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 <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

diff --git a/src/crypto/tls/common.go b/src/crypto/tls/common.go
index f860ac9dfbdc0318a93dceecb5f81b64684bd824..62324de5139b46343a2300ef11f909578ce6324d 100644 (file)
--- 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 c49335d2259123d2243f3997f203256007456c97..23b1ec66680ec22f811989eddfb7db7c8bcbce9c 100644 (file)
--- 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