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

Move padding to the crypto layer #197

Closed
ekr opened this issue Jan 23, 2017 · 9 comments
Closed

Move padding to the crypto layer #197

ekr opened this issue Jan 23, 2017 · 9 comments
Labels
-transport design An issue that affects the design of the protocol; resolution requires consensus. has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list.

Comments

@ekr
Copy link
Collaborator

ekr commented Jan 23, 2017

I think it would be good to make padding the responsibility of the crypto layer. TLS already has a reasonable padding scheme which we can just steal here.

@davidben
Copy link

This would probably require a tweak to TLS 1.3. Right now the specification implies that a second ClientHello after HelloRetryRequest can have no modifications besides what extensions imply. TLS 1.3 would need to allow the padding extension change between first and second ClientHello.

@ekr
Copy link
Collaborator Author

ekr commented Jan 23, 2017

Sorry, I'm not following your comment here, so maybe you need to expand a bit. Is your point that you intend to use the S 7.7 padding to force the CH-containing frame to be ~1MTU and that you need to adjust it during CH/HRR?

In any case, in the same way as TLS makes a distinction between padding for the first handshake message (CH) and generic record padding, it seems like it might be good for QUIC to do the same, and pad all encrypted data in the encryption layer.

@davidben
Copy link

Oh, sorry, I completely misunderstood your ticket! Please ignore me. (I thought you were referring to using RFC 7685 to pad the initial packet which wouldn't work without changes.)

@marten-seemann
Copy link
Contributor

I think PADDINGs are supposed to be a countermeasure to traffic pattern analysis. For example, a peer could pretend to be doing an upload when in reality all it doing is sending ACKs for a download and a lot of PADDINGs.

I'm not sure if this is possible with TLS 1.3, but since the spec doesn't require TLS, but just any appropriate crypto protocol, this shouldn't be an argument to remove the PADDING frame (unless you make the ability to send arbitrary padding a requirement for any crypto protocol that can be used for QUIC).

@martinthomson martinthomson added design An issue that affects the design of the protocol; resolution requires consensus. -transport labels Jan 24, 2017
@martinthomson
Copy link
Member

@ekr, is your intent to just use this for the initial handshake packets? As in #164. Padding is available at both layers (PADDING, the padding TLS extension, and TLS record layer padding for the server flight only).

I didn't port the record padding from TLS into our packet protection because PADDING existed. If that is what you are talking about, we should discuss.

@ekr
Copy link
Collaborator Author

ekr commented Jan 24, 2017 via email

@mnot mnot added this to Odd Jobs in QUIC Apr 28, 2017
@mnot mnot moved this from Odd Jobs to Crypto in QUIC Jun 21, 2017
@ianswett
Copy link
Contributor

ianswett commented Oct 1, 2017

Since padding is used to ensure the MTU is large enough in both directions in QUIC, is it going to be hard to ensure the crypto layer adds the right amount of padding to generate a packet with the desired size?

And it seems like this suggestion only applies to the Crypto Handshake. If so, can we change the title of this issue to "Move crypto handshake padding to the crypto layer"?

@ianswett
Copy link
Contributor

At some point, we changed padding to be a single byte to allow for padding to be placed between frames at the request of @larseggert This seems like a useful feature and we'd remove that if we got rid of padding entirely, so I don't think QUIC padding is going away.

Given that, this issue becomes 'sometimes move padding to the crypto layer', which is going to make congestion control and bytes in flight counting potentially more complex, since sometimes we'll do padding in one place and sometimes the other.

This is orthogonal to #837, since we need to decide how we want QUIC to work with padding. Where to put the padding doesn't change that. If we want padding that counts towards bytes in flight and one that doesn't, we can use PADDING and PING for the two different cases(or other approaches).

Given the above, @ekr is there a reason to keep this open?

@ekr
Copy link
Collaborator Author

ekr commented Nov 12, 2017 via email

@ianswett ianswett removed this from Crypto in QUIC Nov 12, 2017
@mnot mnot added the has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list. label Mar 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-transport design An issue that affects the design of the protocol; resolution requires consensus. has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list.
Projects
None yet
Development

No branches or pull requests

6 participants