From: Roland Shoemaker Date: Wed, 31 Aug 2022 19:24:21 +0000 (-0700) Subject: crypto/tls,crypto/x509: clarify certificate ownership X-Git-Tag: go1.20rc1~558 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=ffa03f6bac1a86f85a3d3f16c4711b252dc404e0;p=gostls13.git crypto/tls,crypto/x509: clarify certificate ownership Clarify documentation in cases where certificates returned from various methods are not owned by the caller, and as such should not be modified. Change-Id: I06bdc4cf0f686c3d5e8bbb76fc71f2a4bdb955e3 Reviewed-on: https://go-review.googlesource.com/c/go/+/427155 Auto-Submit: Roland Shoemaker Run-TryBot: Roland Shoemaker Reviewed-by: Filippo Valsorda Reviewed-by: Heschi Kreinick TryBot-Result: Gopher Robot --- diff --git a/src/crypto/tls/common.go b/src/crypto/tls/common.go index f860ac9dfb..62324de513 100644 --- a/src/crypto/tls/common.go +++ b/src/crypto/tls/common.go @@ -246,6 +246,8 @@ type ConnectionState struct { // On the client side, it can't be empty. On the server side, it can be // empty if Config.ClientAuth is not RequireAnyClientCert or // RequireAndVerifyClientCert. + // + // PeerCertificates and its contents should not be modified. PeerCertificates []*x509.Certificate // VerifiedChains is a list of one or more chains where the first element is @@ -255,6 +257,8 @@ type ConnectionState struct { // On the client side, it's set if Config.InsecureSkipVerify is false. On // the server side, it's set if Config.ClientAuth is VerifyClientCertIfGiven // (and the peer provided a certificate) or RequireAndVerifyClientCert. + // + // VerifiedChains and its contents should not be modified. VerifiedChains [][]*x509.Certificate // SignedCertificateTimestamps is a list of SCTs provided by the peer @@ -554,6 +558,8 @@ type Config struct { // If GetCertificate is nil or returns nil, then the certificate is // retrieved from NameToCertificate. If NameToCertificate is nil, the // best element of Certificates will be used. + // + // Once a Certificate is returned it should not be modified. GetCertificate func(*ClientHelloInfo) (*Certificate, error) // GetClientCertificate, if not nil, is called when a server requests a @@ -569,6 +575,8 @@ type Config struct { // // GetClientCertificate may be called multiple times for the same // connection if renegotiation occurs or if TLS 1.3 is in use. + // + // Once a Certificate is returned it should not be modified. GetClientCertificate func(*CertificateRequestInfo) (*Certificate, error) // GetConfigForClient, if not nil, is called after a ClientHello is @@ -597,6 +605,8 @@ type Config struct { // setting InsecureSkipVerify, or (for a server) when ClientAuth is // RequestClientCert or RequireAnyClientCert, then this callback will // be considered but the verifiedChains argument will always be nil. + // + // verifiedChains and its contents should not be modified. VerifyPeerCertificate func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error // VerifyConnection, if not nil, is called after normal certificate diff --git a/src/crypto/x509/verify.go b/src/crypto/x509/verify.go index c49335d225..23b1ec6668 100644 --- a/src/crypto/x509/verify.go +++ b/src/crypto/x509/verify.go @@ -745,6 +745,8 @@ func (c *Certificate) isValid(certType int, currentChain []*Certificate, opts *V // Certificates that use SHA1WithRSA and ECDSAWithSHA1 signatures are not supported, // and will not be used to build chains. // +// Certificates other than c in the returned chains should not be modified. +// // WARNING: this function doesn't do any revocation checking. func (c *Certificate) Verify(opts VerifyOptions) (chains [][]*Certificate, err error) { // Platform-specific verification needs the ASN.1 contents so