crypto/tls: QUICConn.SendSessionTicket should check Config.SessionTicketsDisabled

[marten-seemann]

What version of Go are you using (go version)?

$ go version
go version go1.21.0 darwin/arm64

Does this issue reproduce with the latest release?

Yes

What did you do?

When QUICConn.SendSessionTicket is called even though Config.SessionTicketsDisabled is set, an error is returned ("session ticket keys unavailable").

Now one could argue that the QUIC stack do that check before calling SendSessionTicket (and that's what quic-go does), however, the QUIC stack won't have access to the tls.Config used on the connection if it is a config returned by GetConfigForClient (unless the QUIC stack wraps that callback and keeps track of the invocations, which would be quite a bit of complexity).

What did you expect to see?

Instead, it would be easy for crypto/tls to check the Config.SessionTicketsDisabled flag on the config that's in use on the connection.

API-wise, there are two options here:

1. return nil
2. return a sentinel error

No strong preference in either direction.

cc @neild @FiloSottile

[neild]

Seems reasonable to me. The TLS layer should be checking SessionTicketsDisabled, since it owns that configuration, and the QUIC layer shouldn't be left holding an uninterpretable error when tickets are disabled.

I think its safe at the moment for the QUIC layer to just ignore all errors from SendSessionTicket, but it's reasonable to want a way to identify the expected error case.

I don't have a strong opinion on whether SendSessionTicket should return nil or a distinguishable error when tickets are disabled. nil is less API surface, but doesn't give the QUIC layer any way to identify the case where a ticket wasn't sent. I don't know why the QUIC layer would need to care, though; I presume all it's going to do is ignore the error, so maybe nil is simple and sufficient.

[gopherbot]

Change https://go.dev/cl/528755 mentions this issue: crypto/tls: check if quic conn can send session ticket