-
Notifications
You must be signed in to change notification settings - Fork 204
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 an appendix containing test vectors for "Initial". #1573
Conversation
I tried @ekr's test vector on the list and this test vector. My code passed with ekr's test vector, but failed with this test vector.
I doubt that there is a bug around QUIC draft-12
TLS 1.3 (QUIC draft-13)
|
That makes sense, thanks for testing. I can try to get @ekr's vectors into this PR and include the PNE vectors EDIT: actually that might not be the best idea since we wanted to change the SNI to something more sensible before putting it in the spec. I'll use this PR to update the vectors and leave PNE to a different PR. |
IV: 27 92 24 e5 85 fd 84 e9 e0 70 ea 9a | ||
~~~ | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, having something like that in the spec would be useful. We need an independent verification before publishing these values.
Also, having the PN key is fine, but we probably need a complete packet example to ensure that the nonce is properly extracted and the PN properly decrypted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact, I think that it would be better to start with the test vector in EKR's message "PNE Test Vector" sent on the quic list on 7/18/2018.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@huitema my objective was to have the PN key in this section and then use it for PNE but this PR was just for initial test vectors. I agree we need independent verification before we publish.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that these are incorrect.
draft-ietf-quic-tls.md
Outdated
|
||
~~~ | ||
quic client in: 00 20 0e 71 75 69 63 20 63 6c 69 65 6e 74 20 | ||
69 6e |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These labels all omit the trailing zero octet for the context.
I think we would be better off with the test vector that EKR published. I use it as unit test in Picoquic, and the implementation works against Quicly, NGTCP2 and NGX. Here are the values that I tested, based on EKR's message:
|
I have the same test vector in https://gist.github.com/martinthomson/7f4b615070254644f4c5335014980cbc - the only problem I can see is that it is very big. (The other problem being that it isn't in a PR yet, but that's a fixable problem.) |
Yes, it may well be that including a "real" packet as a test vector is a bad idea. If we do that, then we will need to update the version number (bytes 2 to 5) from 0xff, 0x00, 0x00, 0x0d to 0xff, 0x00, 0x00, 0x0E for draft 14, and so on for each draft. This does not affect the computation of AEAD and PNE keys, as they are only a function of the salt and connection ID, but it does affect the sample used for PNE in the example. Suppose that instead of a full packet we had a truncated packet, with just enough bytes to get the PNE sample. We will lose the capability of doing a full test "up to packet decoding", but in my experience that's not a very big deal. Implementations can use different unit tests for verifying the implementation of AEAD. But for that loss we get a big gain: we can update the test vector by just flipping the version bits, without having to worry about the rest. Unless we change the salt or the algorithm of course. But then if we do that we know that we have to change the whole test vector no matter what. |
I have a much smaller example in the script I linked. That is a mere 98 octets of payload. It has an ACK frame and a valid ServerHello. It's not as easy to feed into a stack, though. The alternative is to build the much-requested ACK frame example and just encrypt a packet with that in it. Whatever we build, the example needs to be rebuilt as we update things (like the fixed salt). |
I think using client's Initial is beneficial because you can test the construction of AEAD keys. If the size of the test vectors and the need to update them for every revision of the draft (prior to publishing it!) are problemsome, we can list the vectors in a separate document, as @martinthomson has done for TLS :-) https://datatracker.ietf.org/doc/draft-ietf-tls-tls13-vectors/ |
I'm aware they are about to change again, but at least this should be correct now for -15 drafts. I can update these vectors once @martinthomson PR is merged. (Looks like the consensus right now is to move to "tls13 " and to add a QUIC version in the context field.) |
I'm also working on a PNE appendix.
ping @ekr @huitema