Proposal Details

This is an offshoot of discussion on #74922.

crypto/tls.QUICConn can report errors occurring during the TLS handshake in several places:

  1. QUICConn.Start returns an error if the handshake cannot be started.
  2. QUICConn.HandleData returns an error if processing data from the peer causes a handshake error.
  3. QUICConn.SendSessionTicket and QUICConn.StoreSession also return errors.

If errors can occur at points in the handshake other than during these functions, there is no way for QUICConn to report them.

The proposal is to add an additional way to report TLS errors. We add an Err field to QUICEvent, and produce an event with a type of QUICNoEvent and the Err field set when an error occurs:

type QUICEvent struct {
    // Set for QUICNoEvent
    Err error

    // existing fields as-is
}

const (
    // QUICNoEvent indicates that there are no events available.
    // QUICEvent.Err is set if the connection has encountered a fatal error.
    QUICNoEvent QUICEventKind = iota
)

An open question is what, if any, errors would be reported via this mechanism. It would be nice to have one concrete example of an error that we currently can't repor.

Comment From: aclements

This proposal has been added to the active column of the proposals project and will now be reviewed at the weekly proposal review meetings. — aclements for the proposal review group

Comment From: marten-seemann

An open question is what, if any, errors would be reported via this mechanism. It would be nice to have one concrete example of an error that we currently can't repor.

On the server side, the most important example is the recently introduced tls.Config.GetEncryptedClientHelloKeys callback, which has an error return value. It is called after requesting the QUIC transport parameters, hence the error return value can't be returned from HandleData.

However, it would be equally valid to call GetEncryptedClientHelloKeys before requesting the transport parameters (nothing passed to this callback depends on the QUIC transport parameters), so this particular instance could be resolved without the introduction of new API.

There are two more instances where errors could occur: 1. When signing with the private key fails: https://github.com/golang/go/blob/d86ec924993bfb824dfb6f618a46ea205fdbf261/src/crypto/tls/handshake_server_tls13.go#L886-L896 2. When writing the key log: https://github.com/golang/go/blob/d86ec924993bfb824dfb6f618a46ea205fdbf261/src/crypto/tls/handshake_server_tls13.go#L933-L942

Comment From: aclements

It seems odd to add meaning to the existing QUICNoEvent kind. I'm not sure it would actually break anything, but it doesn't pass the sniff test. My understanding is that this is mostly about compatibility with @marten-seemann 's QUIC implementation.

What about instead adding a new QUICErrorEvent kind? If necessary, we could add a QUICConfig.EnableErrorEvents flag, though maybe that's not necessary.

Comment From: neild

Proposal with a new kind:

type QUICEvent struct {
    // Set for QUICErrorEvent.
    // The error will wrap AlertError.
    Err error

    // existing fields as-is
}

const (
    // QUICErrorEvent indicates that a fatal error has occurred.
    // The handshake cannot proceed and the connection must be closed.
    // QUICEvent.Err is set.
    QUICErrEvent QUICEventKind
)

Comment From: gopherbot

Change https://go.dev/cl/695515 mentions this issue: crypto/tls: implement sync processing of QUIC handshake, with error on QUICEvent

Comment From: marten-seemann

I incorporated the QuicErrEvent as described in https://github.com/golang/go/issues/75108#issuecomment-3250250466 into my CL: https://go-review.googlesource.com/c/go/+/695515/4..5.

My understanding is that this is mostly about compatibility with @marten-seemann 's QUIC implementation.

The fact that errors at certain stages of the TLS handshake cannot be returned to the calling QUIC stack affects all QUIC stacks building on top of crypto/tls.

It also turns out that properly returning the error makes it easier to handle TLS handshake messages synchronously, as I proposed in https://github.com/golang/go/issues/74922, allowing a significant speedup of the QUIC handshake.