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

Add trigger for specific packet_dropped situation #371

Closed
rmarx opened this issue Jan 19, 2024 · 6 comments · Fixed by #381
Closed

Add trigger for specific packet_dropped situation #371

rmarx opened this issue Jan 19, 2024 · 6 comments · Fixed by #381

Comments

@rmarx
Copy link
Contributor

rmarx commented Jan 19, 2024

As reported by @hlandau on the mailing list:

quic:packet_dropped:

What is the intended trigger category here when an EL is discarded and
buffered packets are discarded as a consequence? decryption_failure
could match but feels subtly different, as I guess that covers the
case where a packet is received after an EL is discarded. IMO it would
be better to have a different trigger category for this, and add
language clarifying this case.

@rmarx
Copy link
Contributor Author

rmarx commented Jan 19, 2024

Here, I might be suffering from "my brain is slow because it's friday"-syndrome, but I'm not sure what you mean by "EL" here. Care to explain?

In general, there are many more packet_dropped reasons than we chose to list here, since an exhaustive list would be too much. We have the general category for this, AND because triggers are generally loosely defined, implementations can choose their own custom values as well. Though, we can of course have the discussion if this specific situation is common enough to warrant its own trigger value in the spec.

@hlandau
Copy link

hlandau commented Jan 19, 2024

Here, I might be suffering from "my brain is slow because it's friday"-syndrome, but I'm not sure what you mean by "EL" here. Care to explain?

Encryption level

If custom values are OK I've no objection, though it does seem to me to undermine the point of a standard enum a little.

Actually, there is a broader point of discussion here — the interoperability objectives of QLOG seem slightly fuzzy to me, since there is seemingly a lot of expectation of implementations doing custom things like this, and it seems like it will cause problems for tools. Maybe in practice it will not be a problem and tools will just battle-harden themselves by testing with lots of QLOG output from lots of different implementations. It does undermine the point of a standard a little bit — but maybe QLOG is something that needs to inherently occupy a midpoint between a completely strict standard and a state of total flexibility out of practical realities. (See also the comments on the congestion control algorithm name field, feels like we are touching on the same thing there.) That's something I could definitely see.

@rmarx
Copy link
Contributor Author

rmarx commented Jan 19, 2024

Right, now it all makes more sense :) And it's a fair point that the current categories don't quite cover this properly...

I don't have a strong preference for any of these options (though nr. 3 seems a bit better maybe?):

  1. Do nothing
  2. Add new category for this
  3. Add this as an "example" for the decryption_failure trigger in the text

For the broader point, I think you hit the nail right on the head. It's been a tough line to walk between a fully "interoperable" and consistent specification, and the implementation realities and requirements of individual stacks. I think the general approach has been to provide a stable "framework" as much as possible, that allows for the creation of tools that don't have to be too flexible and that mainly need to deal with some variability in terms of field/trigger values, which is usually not too difficult to manage (e.g., qvis will just display the string it reads, without much interpretation if it doesn't expect the value). If we would try to nail down everything, we'd have a mix of pcaps and a whole second layer of semantics on top (and documents at least 4 times as long probably).

That said, of course, if situations are common enough and information is often critical in common debugging scenarios, we do try to put them into the spec explicitly :)

@LPardue
Copy link
Member

LPardue commented Jan 24, 2024

I think option 3 is the correct one, it would be just a few words

@marten-seemann
Copy link
Collaborator

I think it's useful to be able to tell these two cases apart. Keys being unavailable happens regularly during the handshake, whereas decryption failures shouldn't happen unless there are transmission errors or an active attack.

fwiw, quic-go already uses key_unavailable here.

@LPardue
Copy link
Member

LPardue commented Jan 24, 2024

I can live with a other type

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 a pull request may close this issue.

4 participants