crypto/tls: SetSessionTicketKeys can race with automatic ticket-key rotation state

[jvdprng]

Description

The built-in session-ticket key path has a narrow concurrency window between automatic key rotation and an application call to SetSessionTicketKeys. A handshake can continue using stale automatic rotation state after manual keys have been installed.

This is not a Go data race and does not disclose ticket keys. The plausible impact is at most one stale or unusable ticket around the transition.

Specification or documentation

This is a Go API consistency issue. Config.SetSessionTicketKeys is documented as updating the keys used by the built-in session-ticket mechanism. New ticket encryption after the call should consistently reflect the manual key set.

Code references

• go/src/crypto/tls/handshake_server_tls13.go:999 encrypts TLS 1.3 session ticket state with c.config.encryptTicket.
• go/src/crypto/tls/common.go:1085 through go/src/crypto/tls/common.go:1138 implements Config.ticketKeys, including automatic key rotation and the lock upgrade path.
• go/src/crypto/tls/common.go:1141 through go/src/crypto/tls/common.go:1166 implements SetSessionTicketKeys and documents that calling it turns off automatic session ticket key rotation.
• go/src/crypto/tls/handshake_server_test.go:1672 has existing coverage around SetSessionTicketKeys.

Test case

This should be a deterministic Go unit test with a hook at the internal read-lock-to-write-lock transition in the automatic ticket-key rotation path. The hook can call SetSessionTicketKeys and then assert that the ticket-key path returns manual keys rather than continuing with automatic rotation.

[gabyhelp]

Related Issues

• crypto/tls: TLS 1.3 ticket_age_add accepts partial random reads #79853
• crypto/tls: automatically rotated ticket keys are valid for longer than documented #77309
• crypto/tls: TLS 1.3 post-handshake messages can bypass non-advancing record limit #79866
• crypto/tls: add missing RUnlock in ticketKeys #68607 (closed)
• crypto/tls: don't overwrite Config.sessionTicketKeys if already set #15421 (closed)
• crypto/tls: Support for session ticket key rotation #9994 (closed)
• crypto/tls: QUICConn.SendSessionTicket should check Config.SessionTicketsDisabled #62032 (closed)
• net/http: decide how users should use SetSessionTicketKeys with ServeTLS, etc #22018

Related Code Changes

• crypto/tls: drop ClientForConfig mutex when session tickets disabled
• crypto/tls: add support for session ticket key rotation

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[jvdprng]

I do not have a useful standalone Playground reproducer for this one. The reported behavior is a race between Config.SetSessionTicketKeys and automatic ticket-key rotation state. A deterministic reproducer needs an in-tree hook or stress test around that internal rotation state; a normal public-API Playground snippet would be timing-dependent and flaky rather than useful evidence.

[cherrymui]

cc @golang/security

[rolandshoemaker]

I think the described way to trigger this violates the API contract for tls.Config. In particular: "After [a tls.Config] has been passed to a TLS function it must not be modified."

This case requires passing a config to a TLS function and then later calling SetSessionTicketKeys. Now technically this doesn't direct mutation of the Config, but in practice it is mutating things under the hood. Probably we should just explicitly document that you should not call this API after passing the Config to a TLS function.

[jvdprng]

Yeah, the current docs state that it's safe to call this function while a server is running.