crypto/tls: disable session resumption when VerifyPeerCertificate is in use

[FiloSottile]

In #77217 (comment) we discussed how the combination of the documented behaviors of GetConfigForClient and VerifyPeerCertificate is dangerous.

GetConfigForClient is documented to share session ticket keys with the parent.

// If SessionTicketKey was explicitly set on the returned Config, or if
// SetSessionTicketKeys was called on the returned Config, those keys will
// be used. Otherwise, the original Config keys will be used (and possibly
// rotated if they are automatically managed).
GetConfigForClient func(*ClientHelloInfo) (*Config, error)

VerifyPeerCertificate is documented not to run on resumed sessions.

// This callback is not invoked on resumed connections, as certificates are
// not re-verified on resumption.
VerifyPeerCertificate func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error

That makes the following code work unexpectedly.

&tls.Config{
	ClientAuth: tls.RequireAnyClientCert,
	GetConfigForClient: func(hello *tls.ClientHelloInfo) (*tls.Config, error) {
		if someCondition(hello) {
			return &tls.Config{
				ClientAuth: tls.RequireAnyClientCert,
				VerifyPeerCertificate: customStricterVerification,
			}, nil
		}
		return nil, nil
	},
}

You would think customStricterVerification applies to all clients matching someCondition, but instead a client can resume a connection that originally didn't match someCondition but now does without passing customStricterVerification.

A similar, but less likely, issue is present on the client side if ClientSessionCache is shared across Configs with different VerifyPeerCertificate logic.

The second #77217 fix solved this for native verification by re-checking the inclusion of the root. VerifyConnection is not affected because it always runs, including on resumed connections.

I propose we fix this by disabling session resumption if VerifyPeerCertificate is set. We can ship this in Go 1.27 with a GODEBUG.

If the application wishes to use resumption, they can switch to VerifyConnection.

I am not sure if we should do this only one the server side, or also on the client side. Arguably reusing a ClientSessionCache (which is not enabled by default, unlike server-side session tickets) is a pretty explicit statement of intent?

A more heavy-handed alternative is to deprecate VerifyPeerCertificate entirely in favor of VerifyConnection. We could also break the VerifyPeerCertificate docs promise and run it also on resumed connections, but it's hard to predict what would break.

/cc @golang/security @marten-seemann @espadolini

[gabyhelp]

Related Issues

• crypto/tls: certificate chains aren't re-checked on resumption #77217
• crypto/tls: TLS session resumption re-verifies the client's certificate chain #31641 (closed)
• crypto/tls: VerifyConnection is called twice by tls 1.3 servers if connection is resumed #39012 (closed)
• crypto/tls: don't recheck peer certificate validity on resumption if InsecureSkipVerify is set #77359
• crypto/tls: add VerifyPeerCertificate to tls.Config #16363 (closed)
• crypto/tls: OCSP and SCTs are dropped in resumed connections [freeze exception] #39075 (closed)
• crypto/tls: GetCertificate called on resumed sessions #25352
• crypto/tls: cap lifetime of authentication in TLS 1.3 #77294

Related Code Changes

• crypto/tls: check verifiedChains roots when resuming sessions
• crypto/tls: add identity freshness on server resumption

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

[espadolini]

Could we also reenable resumption if SetSessionTicketKeys is called, no matter what else is going on in the tls.Config? That seems like an even stronger declaration of intent compared to setting ClientSessionCache on the client side.

[espadolini]

There's also the maybe more radical but easier to understand option of disabling resumption if any of GetConfigForClient, VerifyConnection or VerifyPeerCertificate are set unless a new ForceAllowResumption flag is set, similar to how http.Transport requires ForceAttemptHTTP2.

[wwqgtxx]

Since a new GODEBUG can be used to control whether session resumption are disabled when configuring VerifyPeerCertificate, why not use a new GODEBUG to control whether it's allowed to be called in resumed connections? This might also help us understand whether doing so would break existing use cases.