GUE dissection support #510

Wilm0r · 2016-03-18T17:38:45Z

Hello,

Here's a patch adding support for GUE (Generic UDP Encapsulation) to tcpdump. The spec can be found at: https://tools.ietf.org/html/draft-ietf-nvo3-gue-02 (header format in 2.2).

This is fairly basic but will show dissection of inner IPv4/IPv6 packets assuming GUE is indeed configured on the officially allocated port 6080. Sadly this is not going to be a firm standard (for example one may want to use a privileged <1024 port instead for security reasons), so I wonder whether, for example, using getservbyname() instead of hardcoded port numbers would be acceptable? There's no prior art for this in your code yet from what I can tell which is why I haven't done this yet.

Kind regards,

Wilmer van der Gaast.

No interpretation of flags and other fields, they are not described in detail in the I-D yet. Do dissect the inner packet which is the most useful.

checks already.

Wilm0r · 2016-04-11T16:13:34Z

I've merged recent changes, this PR should apply cleanly against master now.

infrastation · 2016-07-03T11:42:21Z

print-gue.c

+	case 0:
+		gue_print_0(ndo, bp, len);
+		break;
+	default:


Would you like to add support for version 0x1?

I'll need some dumps to do so, I don't have any so far which makes writing (and testing) that a little bit hard. Will update this PR once that (and the other changes) are done.

infrastation · 2016-07-03T12:13:34Z

Regarding the port numbers, getservbyname() use would not be a good idea because its return value generally may be (and practice will be) volatile and inconsistent across different hosts. tcpdump may be run on a host different from the one that did tunnel termination and the one that did the packet capture.

Section 5.6.2 of the Internet-Draft discusses a use of a magic number. That may be a more reliable means of telling the packets apart but the spec would need to have it as a normative statement rather than as a potential future approach. Maybe you would like to discuss this with the document authors as the root of the problem seems to be there. The magic number support in tcpdump should not be difficult per se.

Wilm0r · 2016-07-06T17:42:26Z

Regarding magic numbers, I'm told that's most likely going to get removed from the next draft version, so better not to add support for it.

So what I'll do is add support for version 1 and squash the commits back into one.

Wilm0r · 2016-10-05T16:44:44Z

Sorry for the long delay on this :-(

So magic numbers are going to get removed, I think it should be okay to not support those then?

And for GUEv1, I've tried to get example dumps but no response so far. One may be able to generate them using a very recent Linux kernel, but I'd rather not do this myself, not being all that familiar with GUEv1 in particular..

infrastation · 2016-10-05T21:31:34Z

Thank you for the update. It would make it easier to review further with the new code rebased on the current master branch.

Wilm0r · 2016-10-05T23:30:59Z

But of course, I was planning to do a merge but wanted to ask first how strongly you feel about the GUEv1 support.

Merged now. Not all tests pass, but the failing ones are not GUE and are failing in the parent branch as well. :-(

I'd squash this branch but I think Github can do that for you at merge time now?

fxlb · 2016-10-06T09:16:51Z

Some warnings here: https://travis-ci.org/the-tcpdump-group/tcpdump/jobs/165389888

./print-gue.c: In function ‘gue_print_0’:
./print-gue.c:119:2: warning: C++ style comments are not allowed in ISO C90 [enabled by default]
./print-gue.c:119:2: warning: (this will be reported only once per input file) [enabled by default]
./print-gue.c:93:8: warning: unused variable ‘control’ [-Wunused-variable]

Wilm0r · 2016-10-07T07:57:00Z

Oops, yeah those didn't appear during my local build. :-/ Fixed and the current build at https://travis-ci.org/the-tcpdump-group/tcpdump/jobs/165633760 looks clean now.

Wilm0r · 2017-01-09T12:00:45Z

So is it okay to merge this one with just GUEv0, not v1? I've not been able to get dumps of GUEv1 traffic so far..

infrastation · 2018-02-01T20:27:13Z

Could you rebase this on the current master branch?

Wilm0r · 2018-02-20T18:46:34Z

Sorry for the delay - I'll try to do this in a few days when I get back into the office. (Been out for a while.)

infrastation · 2018-02-20T22:52:26Z

Sure, take your time.

guyharris · 2018-03-26T18:59:30Z

You need to update CMakeLists.txt to include print-gue.c as well.

Wilm0r · 2018-03-27T00:00:30Z

Oh, thanks for the hint. I was still trying to update the range checking to use your ND_T(TEST|CHECK) macros as well but couldn't fully figure them out. :-( (CHECK vs TEST and also what to do for the check on length>=sizeof(struct ip))

infrastation · 2021-10-31T20:51:48Z

@Wilm0r, if you wish to resume this work, please update this pull request within 14 days. Otherwise it will be closed.

fxlb · 2021-11-02T16:06:47Z

FYI, this is an expired Internet-Draft.
draft-ietf-nvo3-gue-02 -> draft-ietf-intarea-gue-09
(https://datatracker.ietf.org/doc/draft-ietf-intarea-gue/)

Wilm0r · 2021-11-02T16:12:09Z

Correct, there seem to be two versions now. I'll see whether I can get this PR back into decent shape, and how to deal with the versions/support both of them at all. Don't know which one's more common.

I think the two protocols aren't really compatible nor are they detectable easily except maybe with heuristics. Would the usual tcpdump solution be to just name them separately, for example gue0 and gue1 ?

fxlb · 2021-11-02T16:28:44Z

I'll see whether I can get this PR back into decent shape

Is it useful for an expired draft?
If this specification is useful to you, why not find some people to revive it.

fxlb · 2021-11-02T17:42:25Z

there seem to be two versions now.

Do you have specifications about these versions/variants?

infrastation · 2021-11-04T12:06:40Z

@Wilm0r, in case you don't have the motivation to follow the developments of this protocol design closely, it is OK not to implement it, and to get something else done instead. Often it is better not to have a particular protocol support at all than to have out of date code and the cost of its maintenance.

Wilm0r · 2021-11-19T18:51:40Z

Sorry for losing track of this again.

I agree, I don't want to contribute code that is prone to rot. What I will try to do is figure out the status of GUEv0 vs v1 and see what is wise here. AFAIK (and I should add that at work I'm not actually that close to this project/development (anymore)) GUE is definitely used more widely including by some hardware so support would be useful, but I'll need to figure out which of the two versions.

Feel free to close this PR if you've lost patience, I can reopen/open a new one if needed. I'll obviously need to deal with a pretty intense merge anyway.

BTW, my main worry for this one is that GUE can be on ~any port#. (We use a privileged <1024 port instead of the IANA-reserved one ourselves.) When I wrote this patch, tcpdump didn't really support "use this port#" arguments for decoders (while Wireshark does have "reinterpret this stream as protocol X" for example), possibly this has changed?

infrastation · 2021-11-19T20:05:51Z

The traditional workaround to decode a protocol of interest on a non-standard UDP port would be along the lines of tcpdump -T gue udp port NNNN, which would obviously exclude decoding all other traffic, but would be better than nothing.

infrastation · 2021-11-19T22:28:53Z

If in 14 days it is still not clear how to progress this matter in a constructive way, it will be better to close this request.

infrastation · 2021-11-26T21:25:48Z

For reference, a potential alternative way to apply dissectors to non-standard ports was discussed in #495.

infrastation · 2021-12-04T14:11:44Z

It is time.

Wilmer van der Gaast added 3 commits March 4, 2016 16:18

Basic GUE dissection support.

aa99b21

No interpretation of flags and other fields, they are not described in detail in the I-D yet. Do dissect the inner packet which is the most useful.

Move truncation check to just IPv4 handling, ip6_print() does the right

9a250f4

checks already.

Merge remote-tracking branch 'upstream/master'

c99861a

Merge remote-tracking branch 'upstream/master'

2609889

infrastation reviewed Jul 3, 2016
View reviewed changes

Wilm0r changed the title ~~GUE dissection support~~ GUE dissection suppor Jul 4, 2016

Wilm0r changed the title ~~GUE dissection suppor~~ GUE dissection support Jul 6, 2016

Merge remote-tracking branch 'parent/master'

aaab44b

Fix two Travis warnings.

4c0dac4

fxlb force-pushed the master branch from 360124a to 5d938ab Compare March 1, 2018 15:43

Merge remote-tracking branch 'upstream/master'

a239795

infrastation added the reviewed_needswork label Jul 31, 2018

infrastation closed this Dec 4, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GUE dissection support #510

GUE dissection support #510

Wilm0r commented Mar 18, 2016

Wilm0r commented Apr 11, 2016

infrastation Jul 3, 2016

Wilm0r Jul 6, 2016

infrastation commented Jul 3, 2016

Wilm0r commented Jul 6, 2016

Wilm0r commented Oct 5, 2016

infrastation commented Oct 5, 2016

Wilm0r commented Oct 5, 2016

fxlb commented Oct 6, 2016

Wilm0r commented Oct 7, 2016

Wilm0r commented Jan 9, 2017

infrastation commented Feb 1, 2018

Wilm0r commented Feb 20, 2018

infrastation commented Feb 20, 2018

guyharris commented Mar 26, 2018 •

edited

Loading

Wilm0r commented Mar 27, 2018

infrastation commented Oct 31, 2021

fxlb commented Nov 2, 2021

Wilm0r commented Nov 2, 2021

fxlb commented Nov 2, 2021

fxlb commented Nov 2, 2021

infrastation commented Nov 4, 2021

Wilm0r commented Nov 19, 2021

infrastation commented Nov 19, 2021

infrastation commented Nov 19, 2021

infrastation commented Nov 26, 2021

infrastation commented Dec 4, 2021

GUE dissection support #510

GUE dissection support #510

Conversation

Wilm0r commented Mar 18, 2016

Wilm0r commented Apr 11, 2016

infrastation Jul 3, 2016

Choose a reason for hiding this comment

Wilm0r Jul 6, 2016

Choose a reason for hiding this comment

infrastation commented Jul 3, 2016

Wilm0r commented Jul 6, 2016

Wilm0r commented Oct 5, 2016

infrastation commented Oct 5, 2016

Wilm0r commented Oct 5, 2016

fxlb commented Oct 6, 2016

Wilm0r commented Oct 7, 2016

Wilm0r commented Jan 9, 2017

infrastation commented Feb 1, 2018

Wilm0r commented Feb 20, 2018

infrastation commented Feb 20, 2018

guyharris commented Mar 26, 2018 • edited Loading

Wilm0r commented Mar 27, 2018

infrastation commented Oct 31, 2021

fxlb commented Nov 2, 2021

Wilm0r commented Nov 2, 2021

fxlb commented Nov 2, 2021

fxlb commented Nov 2, 2021

infrastation commented Nov 4, 2021

Wilm0r commented Nov 19, 2021

infrastation commented Nov 19, 2021

infrastation commented Nov 19, 2021

infrastation commented Nov 26, 2021

infrastation commented Dec 4, 2021

guyharris commented Mar 26, 2018 •

edited

Loading