JulianSchmid / etherparse

A rust library for parsing ethernet & ethernet using protocols.
Apache License 2.0
285 stars 54 forks source link

RFC: Adding support for parsing IPv4 Record Route Option #56

Closed robs-zeynet closed 1 year ago

robs-zeynet commented 1 year ago

Hi all,

See title. Record route is very cool and very useful (can provide a bunch of citations if this is a disputed point), but it looks like it will require some surgery to add it to the existing extensions parsing logic.

Specifically,

0) [minor?] In v4, there are options (e.g., from rfc791 - RR, NO-OP, source route, etc.) and extensions (e.g., AUTH - currently implemented), which seem to populate the same space but use different terms - I'm assuming we don't need to disambiguate, but open to opinions here. 1) the main read() call for IPv4 extensions (https://github.com/JulianSchmid/etherparse/blob/master/etherparse/src/internet/ipv4_exts.rs#L35) doesn't take the options/extensions length as a parameter so that needs to be computed from the call sites 2) The actual read() logic needs to be updated with a cursor, while( index < length) logic to parse variable length options 3) The write() logic needs to be updated to understand if the sum of the options specified exceeds the total header length available (e.g., 4 bits of header * 4 bytes is max header size of 64 bytes - 20 bytes v4 base header --> max options length is 44 bytes)

I'm happy to put together a PR to put all of this together the way I think it should go, but wanted to start a thread here first to see if others agree/have different ideas.

Thoughts?

robs-zeynet commented 1 year ago

Sigh... it turns out that I was completely confused about extensions vs. options and that all of the existing APIs for managing options are already available, so I'm going to sheepishly close this issue and hope no one reads it :-)