Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attempt at "failing gracefully" for dhcpv6 #396

Closed

Conversation

davebarrau
Copy link
Contributor

@hugelgupf Something like this perhaps?

As mentioned in #395, the dhcpv6 library currently causes the whole packet processing to fail if there are any parsing errors in any of the Options. Whilst that MR dealt with a specific issue with a D-Link router, this MR allows processing to continue in general, instead capturing and storing the error in a new "OptionGenericParseFailure" type which includes the option code, option bytes, parser error and original option.

Because the option bytes are returned, it would therefore be possible to detect an OptionGenericParseFailure and write a custom implementation to deal with the specific parser error, if required.

By default, Summary() may therefore look something like this:

Message
  messageType=SOLICIT
  transactionid=0x123456
  options=[
    ClientID: DUID{type=DUID-LL hwtype=Ethernet hwaddr=00:11:22:33:44:55}
    IANA: {IAID=[0 0 0 1], t1=0s, t2=0s, options={[]}}
    Rapid Commit -> []
    ElapsedTime: 34.92s
    RequestedOptions: DNS Recursive Name Server, Domain Search List, SNTP Server List, Vendor Options, Max Solicit Timeout Value, Max Information-Request Timeout Value
    GenericParseFailure(Vendor Class): OptVendorClass{enterprisenum=3561, data=[, , , , , , ]}, Error=buffer too short at position 6: have 12 bytes, want 3072 bytes
    IAPD: {IAID=[0 0 0 0], t1=0s, t2=0s, Options=[{[]}]}
  ]

Signed-off-by: David Barr <38654497+davebarrau@users.noreply.github.com>
Signed-off-by: David Barr <38654497+davebarrau@users.noreply.github.com>
@pmazzini
Copy link
Collaborator

No, this is still trying to parse all the options with a fallback in case the parsing fails.

The idea of lazy parsing is not parsing the options at all, and only do it when the user requests for the specific information.

@pmazzini
Copy link
Collaborator

Moving to lazy parsing is not a trivial change though.

@davebarrau
Copy link
Contributor Author

No, this is still trying to parse all the options with a fallback in case the parsing fails.

The idea of lazy parsing is not parsing the options at all, and only do it when the user requests for the specific information.

Ah okay, I've misunderstood!

Would this MR as a "failing gracefully" one still be useful though? I think it would be beneficial as a standalone feature.

In a "lazy parsing" only world, you may not know that an option is corrupt before you decide to exclude it from parsing, or a wanted option could also be mangled and cause the whole packet to fail to parse as well.

@davebarrau davebarrau changed the title Attempt at "lazy parsing", or "failing gracefully" for dhcpv6 Attempt at "failing gracefully" for dhcpv6 Sep 2, 2020
@pmazzini
Copy link
Collaborator

Dropping in favor of #492

@pmazzini pmazzini closed this Nov 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants