-
Notifications
You must be signed in to change notification settings - Fork 17.6k
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
crypto/tls: support QUIC as a transport #44886
Comments
Thank you @neild! I really like the API you're proposing. A few thoughts:
|
|
|
Any progress? |
Going to try to move this along. Updated proposal after some discussions with @rolandshoemaker and @FiloSottile: // EncryptionLevel represents a QUIC encryption level used to transmit
// handshake messages.
type EncryptionLevel int
const (
EncryptionLevelInitial = iota
EncryptionLevelHandshake
EncryptionLevelApplication
)
// An AlertError is a TLS alert.
type AlertError uint8
// A QUICConn represents a connection which uses a QUIC implementation as the underlying
// transport as described in RFC 9001.
type QUICConn
// QUICClient returns a new TLS client side connection using QUICTransport as the
// underlying transport. The config cannot be nil.
//
// The config's MinVersion must be at least TLS 1.3.
func QUICClient(t *QUICTransport, config *Config) *QUICConn
// QUICClient returns a new TLS server side connection using QUICTransport as the
// underlying transport. The config cannot be nil.
//
// The config's MinVersion must be at least TLS 1.3.
func QUICServer(t *QUICTransport, config *Config) *QUICConn
// Handshake runs the client or server handshake protocol.
//
// When the handshake ends with a TLS alert, Handshake returns an error wrapping AlertError.
func (q *QUICConn) Handshake() error
// QUICTransport describes hooks used by a QUIC implementation.
//
// If any QUICTransport function returns an error, the QUIC handshake will
// be terminated.
type QUICTransport struct {
// SetReadSecret configures the read secret and cipher suite for the given
// encryption level. It will be called at most once per encryption level.
//
// QUIC ACKs packets at the same level they were received at, except that
// early data (0-RTT) packets trigger application (1-RTT) acks. ACK-writing
// keys will always be installed with SetWriteSecret before the
// packet-reading keys with SetReadSecret, ensuring that QUIC can always
// ACK any packet that it decrypts.
//
// After SetReadSecret is called for an encryption level, no more data is
// expected at previous levels.
SetReadSecret func(level EncryptionLevel, suite uint16, secret []byte) error
// SetWriteSecret configures the write secret and cipher suite for the
// given encryption level. It will be called at most once per encryption
// level.
//
// See SetReadSecret for additional invariants between packets and their
// ACKs.
//
// After SetWriteSecret is called for an encryption level, all future writes
// will be at that level until the next call to SetWriteSecret.
SetWriteSecret func(level EncryptionLevel, suite uint16, secret []byte) error
// WriteHandshakeData adds handshake data to the current flight at the
// given encryption level.
//
// A single handshake flight may include data from multiple encryption
// levels. QUIC implementations should defer writing data to the network
// until FlushHandshakeData to better pack QUIC packets into transport
// datagrams.
WriteHandshakeData func(level EncryptionLevel, data []byte) error
// FlushHandshakeData is called when the current flight is complete and
// should be written to the transport. Note that a flight may contain
// data at several encryption levels.
FlushHandshakeData func() error
// ReadHandshakeData is called to request handshake data. It follows the
// same contract as io.Reader's Read method, but returns the encryption
// level of the data as well as the number of bytes read and error.
//
// ReadHandshakeData must not combine data from multiple encryption levels.
//
// ReadHandshakeData must block until at least one byte of data is
// available, and must return as soon as least one byte of data is
// available.
ReadHandshakeData func(p []byte) (level EncryptionLevel, n int, err error)
// SetTransportParameters provides the extension_data field of the
// quic_transport_parameters extension sent by the peer. It will
// always be called before the successful completion of a handshake.
SetTransportParameters func([]byte) error
// GetTransportParameters returns the extension_data field of the
// quic_transport_parameters extension to send to the peer.
//
// Server connections always call SetTransportParameters before
// GetTransportParameters.
GetTransportParameters func() ([]byte, error)
} This moves the QUIC support into a new In QUIC, TLS alerts are always fatal and terminate the connection. The The progression of encryption levels is more explicitly spelled out: Once a read or write secret is provided for a level, no more data is read/written at previous levels. This means the I've added the guarantee that servers call @marten-seemann points out (#44886 (comment)) that RFC 9001 requires connections to be terminated with a TLS alert when ALPN negotiation fails. The specific language in the RFC is "unless another mechanism is used for agreeing on an application protocol, endpoints MUST use ALPN for this purpose." Since this allows for an unspecified "another mechanism", I think it's best for the An open question is how to extend this to support early data / 0-RTT. Since
I don't see any obstacle to adding this functionality to the proposed API above, once Edit: Added an |
I thought crypto/tls deliberately does not support 0-RTT to avoid replay attacks? |
I'm not sure that it's so clear. I found #9671 (comment) and go/src/crypto/tls/handshake_server_tls13.go Lines 278 to 280 in 80c5bbc
It looks to me like support just hasn't been implemented yet, not that there's any particular opposition to it. |
Perhaps I should say if My goal is to ensure we don't paint ourselves into a corner with the QUIC APIs such that it would be difficult to support 0-RTT with them in the future, in the event |
This proposal has been added to the active column of the proposals project |
@neild Two questions about the proposed API, why is And secondly who provides the |
I think it's a Note that this proposal is specifically for |
Indeed, this is about the crypto/tls API needed by HTTP/3 implementations in other packages. It's a struct of callbacks because that can be extended in the future with other optional callbacks, while interfaces can't ever add method backwards-compatibly. I agree it's a little weird, but I don't have a better suggestion. |
Yes, The
|
A few thoughts about the API suggested in #44886 (comment):
// SetTransportParameters provides the extension_data field of the
// quic_transport_parameters extension sent by the peer. It will
// always be called before the successful completion of a handshake.
SetTransportParameters func([]byte) error Is there any reason to not make the contract stricter here, e.g. "It will be called before the keys for the next encryption level are installed.". I imagine this is how it would be implemented anyway. This would be a requirement if one wanted to implemented the QUIC Version Negotiation extension. This would also be a requirement to send
Sending the @neild Please let me know once you have a branch to play around with. I'd be more than happy to integrate it into a quic-go branch (even at an early stage), so we can how the new API works in practice. |
@neild think functional constructors will work better than the struct based approach here. will let us hide the internals of the structure layout. |
Change https://go.dev/cl/461608 mentions this issue: |
@marten-seemann Thanks for the comments!
I was thinking that there isn't any use for post-handshake TLS messages until
Alternatively,
My view is that the QUIC implementation should detect CRYPTO data sent at a previous encryption level, but
Isn't this
This seems like it'd be useful to have, but I think it's out of scope for this proposal.
Seems reasonable. How about:
I'm convinced. How about: If This requires no API changes, and the only behavioral change is to return an error when QUIC is enabled,
https://go.dev/cl/461608 is a very-draft implementation of the proposal. (No tests, doesn't enforce all the constraints defined in RFC 9001.) |
What's the motivation for hiding the internals of |
@neild because it'll lock in the API for all time. functional constructors wont. |
Implementing The design of The An alternative would be to have
( Internally, This has the advantage of being (I think) a simpler interface for a QUIC implementation to use, and leaving the door open for us adding a non-blocking handshake implementation some day. A disadvantage is that |
Talked to @FiloSottile and we decided on the QUICEvent version. The allocation is not a show-stopper, and it will be easier to understand and extend in the future, which should lead to fewer bugs. |
Updating https://go.dev/cl/493655 with the tests now. I'm wondering if it's simpler to provide events via a separate method:
This avoids questions of ownership of the |
I like @neild's proposal, as it provides an easy way to avoid allocations. The fact that we don't need Any opinions / decision on |
I'd rather stick with a single concrete I've updated https://go.dev/cl/493655 with tests, and changed it to report events with |
Sounds good to me.
I've made the (minimal) change to quic-go to use |
Oh I like Agreed on a single |
Based on the discussion above, this proposal seems like a likely accept. |
No change in consensus, so accepted. 🎉 |
Out of curiosity, is this still aiming for Go 1.21, or might it be pushed back to 1.22? I ask because I believe the 1.21 tree is meant to be frozen today. |
We are indeed attempting to slide this in for 1.21 before the window closes. |
Add a QUICConn type for use by QUIC implementations. A QUICConn provides unencrypted handshake bytes and connection secrets to the QUIC layer, and receives handshake bytes. For #44886 Change-Id: I859dda4cc6d466a1df2fb863a69d3a2a069110d5 Reviewed-on: https://go-review.googlesource.com/c/go/+/493655 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Filippo Valsorda <filippo@golang.org> Run-TryBot: Damien Neil <dneil@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Marten Seemann <martenseemann@gmail.com>
Change https://go.dev/cl/501303 mentions this issue: |
For #60105 For #44886 Change-Id: I8f6cfc4490535979ee8c0d8381c03b03c9c7b9a0 Reviewed-on: https://go-review.googlesource.com/c/go/+/501303 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org> Run-TryBot: Damien Neil <dneil@google.com>
Should this issue be closed? |
QUIC is the transport protocol underlying HTTP/3, as detailed in https://tools.ietf.org/html/draft-ietf-quic-tls-34. A QUIC implementation requires an unusually tight coupling with TLS; quoting the draft:
The
crypto/tls
package does not currently provide an API suitable for use by a QUIC implementation. I propose that we add one.Related background
BoringSSL provides a set of functions specifically for use by QUIC implementations:
https://commondatastorage.googleapis.com/chromium-boringssl-docs/ssl.h.html#QUIC-integration
The
quic-go
QUIC implementation uses a fork ofcrypto/tls
. Thequic-go
fork adds aRecordLayer
interface, which is quite similar to the BoringSSL API.A QUIC implementation needs to:
quic_transport_parameters
TLS extensionBoringSSL and the
quic-go
fork ofcrypto/tls
both provide these capabilities.The
quic-go
fork provide additional extensions tocrypto/tls
around the handling of early data and session tickets. Those changes are out of scope for this proposal, which addresses only the QUIC-specific need to replace the record layer.Proposed API changes
The text was updated successfully, but these errors were encountered: