diff --git a/common.go b/common.go index c801535..211fa39 100644 --- a/common.go +++ b/common.go @@ -618,10 +618,16 @@ type Config struct { // non-nil error, the handshake is aborted and that error results. // // If normal verification fails then the handshake will abort before - // considering this callback. If normal verification is disabled by - // 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. + // considering this callback. If normal verification is disabled (on the + // client when InsecureSkipVerify is set, or on a server when ClientAuth is + // RequestClientCert or RequireAnyClientCert), then this callback will be + // considered but the verifiedChains argument will always be nil. When + // ClientAuth is NoClientCert, this callback is not called on the server. + // rawCerts may be empty on the server if ClientAuth is RequestClientCert or + // VerifyClientCertIfGiven. + // + // This callback is not invoked on resumed connections, as certificates are + // not re-verified on resumption. // // verifiedChains and its contents should not be modified. VerifyPeerCertificate func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error @@ -632,8 +638,9 @@ type Config struct { // and that error results. // // If normal verification fails then the handshake will abort before - // considering this callback. This callback will run for all connections - // regardless of InsecureSkipVerify or ClientAuth settings. + // considering this callback. This callback will run for all connections, + // including resumptions, regardless of InsecureSkipVerify or ClientAuth + // settings. VerifyConnection func(ConnectionState) error // RootCAs defines the set of root certificate authorities diff --git a/handshake_client.go b/handshake_client.go index 5b42d30..daed95b 100644 --- a/handshake_client.go +++ b/handshake_client.go @@ -319,18 +319,17 @@ func (c *Conn) loadSession(hello *clientHelloMsg) ( // Check that the cached server certificate is not expired, and that it's // valid for the ServerName. This should be ensured by the cache key, but // protect the application from a faulty ClientSessionCache implementation. + if c.config.time().After(session.peerCertificates[0].NotAfter) { + // Expired certificate, delete the entry. + c.config.ClientSessionCache.Put(cacheKey, nil) + return nil, nil, nil, nil + } if !c.config.InsecureSkipVerify { if len(session.verifiedChains) == 0 { // The original connection had InsecureSkipVerify, while this doesn't. return nil, nil, nil, nil } - serverCert := session.peerCertificates[0] - if c.config.time().After(serverCert.NotAfter) { - // Expired certificate, delete the entry. - c.config.ClientSessionCache.Put(cacheKey, nil) - return nil, nil, nil, nil - } - if err := serverCert.VerifyHostname(c.config.ServerName); err != nil { + if err := session.peerCertificates[0].VerifyHostname(c.config.ServerName); err != nil { return nil, nil, nil, nil } } diff --git a/handshake_server.go b/handshake_server.go index 5fa4281..1c3d74a 100644 --- a/handshake_server.go +++ b/handshake_server.go @@ -471,6 +471,13 @@ func (hs *serverHandshakeState) checkForResumption() error { if sessionHasClientCerts && c.config.ClientAuth == NoClientCert { return nil } + if sessionHasClientCerts && c.config.time().After(sessionState.peerCertificates[0].NotAfter) { + return nil + } + if sessionHasClientCerts && c.config.ClientAuth >= VerifyClientCertIfGiven && + len(sessionState.verifiedChains) == 0 { + return nil + } // RFC 7627, Section 5.3 if !sessionState.extMasterSecret && hs.clientHello.extendedMasterSecret { @@ -482,6 +489,10 @@ func (hs *serverHandshakeState) checkForResumption() error { return errors.New("tls: session supported extended_master_secret but client does not") } + c.peerCertificates = sessionState.peerCertificates + c.ocspResponse = sessionState.ocspResponse + c.scts = sessionState.scts + c.verifiedChains = sessionState.verifiedChains c.extMasterSecret = sessionState.extMasterSecret hs.sessionState = sessionState hs.suite = suite @@ -510,10 +521,6 @@ func (hs *serverHandshakeState) doResumeHandshake() error { return err } - if err := c.processCertsFromClient(hs.sessionState.certificate()); err != nil { - return err - } - if c.config.VerifyConnection != nil { if err := c.config.VerifyConnection(c.connectionStateLocked()); err != nil { c.sendAlert(alertBadCertificate) @@ -847,8 +854,7 @@ func (hs *serverHandshakeState) sendFinished(out []byte) error { } // processCertsFromClient takes a chain of client certificates either from a -// Certificates message or from a sessionState and verifies them. It returns -// the public key of the leaf certificate. +// Certificates message and verifies them. func (c *Conn) processCertsFromClient(certificate Certificate) error { certificates := certificate.Certificate certs := make([]*x509.Certificate, len(certificates)) diff --git a/handshake_server_tls13.go b/handshake_server_tls13.go index 1abf770..f79ce04 100644 --- a/handshake_server_tls13.go +++ b/handshake_server_tls13.go @@ -387,6 +387,13 @@ func (hs *serverHandshakeStateTLS13) checkForResumption() error { if sessionHasClientCerts && c.config.ClientAuth == NoClientCert { continue } + if sessionHasClientCerts && c.config.time().After(sessionState.peerCertificates[0].NotAfter) { + continue + } + if sessionHasClientCerts && c.config.ClientAuth >= VerifyClientCertIfGiven && + len(sessionState.verifiedChains) == 0 { + continue + } hs.earlySecret = hs.suite.extract(sessionState.secret, nil) binderKey := hs.suite.deriveSecret(hs.earlySecret, resumptionBinderLabel, nil) @@ -422,9 +429,10 @@ func (hs *serverHandshakeStateTLS13) checkForResumption() error { } c.didResume = true - if err := c.processCertsFromClient(sessionState.certificate()); err != nil { - return err - } + c.peerCertificates = sessionState.peerCertificates + c.ocspResponse = sessionState.ocspResponse + c.scts = sessionState.scts + c.verifiedChains = sessionState.verifiedChains hs.hello.selectedIdentityPresent = true hs.hello.selectedIdentity = uint16(i) diff --git a/ticket.go b/ticket.go index adb6c3c..e72f4d6 100644 --- a/ticket.go +++ b/ticket.go @@ -37,16 +37,16 @@ type SessionState struct { // uint8 ext_master_secret = { 0, 1 }; // uint8 early_data = { 0, 1 }; // CertificateEntry certificate_list<0..2^24-1>; + // CertificateChain verified_chains<0..2^24-1>; /* excluding leaf */ // select (SessionState.early_data) { // case 0: Empty; // case 1: opaque alpn<1..2^8-1>; // }; // select (SessionState.type) { - // case server: /* empty */; + // case server: Empty; // case client: struct { - // CertificateChain verified_chains<0..2^24-1>; /* excluding leaf */ // select (SessionState.version) { - // case VersionTLS10..VersionTLS12: /* empty */; + // case VersionTLS10..VersionTLS12: Empty; // case VersionTLS13: struct { // uint64 use_by; // uint32 age_add; @@ -87,10 +87,8 @@ type SessionState struct { activeCertHandles []*activeCert ocspResponse []byte scts [][]byte - alpnProtocol string // only set if EarlyData is true - - // Client-side fields. verifiedChains [][]*x509.Certificate + alpnProtocol string // only set if EarlyData is true // Client-side TLS 1.3-only fields. useBy uint64 // seconds since UNIX epoch @@ -129,29 +127,33 @@ func (s *SessionState) Bytes() ([]byte, error) { } else { b.AddUint8(0) } - marshalCertificate(&b, s.certificate()) + marshalCertificate(&b, Certificate{ + Certificate: certificatesToBytesSlice(s.peerCertificates), + OCSPStaple: s.ocspResponse, + SignedCertificateTimestamps: s.scts, + }) + b.AddUint24LengthPrefixed(func(b *cryptobyte.Builder) { + for _, chain := range s.verifiedChains { + b.AddUint24LengthPrefixed(func(b *cryptobyte.Builder) { + // We elide the first certificate because it's always the leaf. + if len(chain) == 0 { + b.SetError(errors.New("tls: internal error: empty verified chain")) + return + } + for _, cert := range chain[1:] { + b.AddUint24LengthPrefixed(func(b *cryptobyte.Builder) { + b.AddBytes(cert.Raw) + }) + } + }) + } + }) if s.EarlyData { b.AddUint8LengthPrefixed(func(b *cryptobyte.Builder) { b.AddBytes([]byte(s.alpnProtocol)) }) } if s.isClient { - b.AddUint24LengthPrefixed(func(b *cryptobyte.Builder) { - for _, chain := range s.verifiedChains { - b.AddUint24LengthPrefixed(func(b *cryptobyte.Builder) { - // We elide the first certificate because it's always the leaf. - if len(chain) == 0 { - b.SetError(errors.New("tls: internal error: empty verified chain")) - return - } - for _, cert := range chain[1:] { - b.AddUint24LengthPrefixed(func(b *cryptobyte.Builder) { - b.AddBytes(cert.Raw) - }) - } - }) - } - }) if s.version >= VersionTLS13 { addUint64(&b, s.useBy) b.AddUint32(s.ageAdd) @@ -160,14 +162,6 @@ func (s *SessionState) Bytes() ([]byte, error) { return b.Bytes() } -func (s *SessionState) certificate() Certificate { - return Certificate{ - Certificate: certificatesToBytesSlice(s.peerCertificates), - OCSPStaple: s.ocspResponse, - SignedCertificateTimestamps: s.scts, - } -} - func certificatesToBytesSlice(certs []*x509.Certificate) [][]byte { s := make([][]byte, 0, len(certs)) for _, c := range certs { @@ -221,6 +215,34 @@ func ParseSessionState(data []byte) (*SessionState, error) { } ss.ocspResponse = cert.OCSPStaple ss.scts = cert.SignedCertificateTimestamps + var chainList cryptobyte.String + if !s.ReadUint24LengthPrefixed(&chainList) { + return nil, errors.New("tls: invalid session encoding") + } + for !chainList.Empty() { + var certList cryptobyte.String + if !chainList.ReadUint24LengthPrefixed(&certList) { + return nil, errors.New("tls: invalid session encoding") + } + var chain []*x509.Certificate + if len(ss.peerCertificates) == 0 { + return nil, errors.New("tls: invalid session encoding") + } + chain = append(chain, ss.peerCertificates[0]) + for !certList.Empty() { + var cert []byte + if !readUint24LengthPrefixed(&certList, &cert) { + return nil, errors.New("tls: invalid session encoding") + } + c, err := globalCertCache.newCert(cert) + if err != nil { + return nil, err + } + ss.activeCertHandles = append(ss.activeCertHandles, c) + chain = append(chain, c.cert) + } + ss.verifiedChains = append(ss.verifiedChains, chain) + } if ss.EarlyData { var alpn []byte if !readUint8LengthPrefixed(&s, &alpn) { @@ -238,31 +260,6 @@ func ParseSessionState(data []byte) (*SessionState, error) { if len(ss.peerCertificates) == 0 { return nil, errors.New("tls: no server certificates in client session") } - var chainList cryptobyte.String - if !s.ReadUint24LengthPrefixed(&chainList) { - return nil, errors.New("tls: invalid session encoding") - } - for !chainList.Empty() { - var certList cryptobyte.String - if !chainList.ReadUint24LengthPrefixed(&certList) { - return nil, errors.New("tls: invalid session encoding") - } - var chain []*x509.Certificate - chain = append(chain, ss.peerCertificates[0]) - for !certList.Empty() { - var cert []byte - if !readUint24LengthPrefixed(&certList, &cert) { - return nil, errors.New("tls: invalid session encoding") - } - c, err := globalCertCache.newCert(cert) - if err != nil { - return nil, err - } - ss.activeCertHandles = append(ss.activeCertHandles, c) - chain = append(chain, c.cert) - } - ss.verifiedChains = append(ss.verifiedChains, chain) - } if ss.version < VersionTLS13 { if !s.Empty() { return nil, errors.New("tls: invalid session encoding") @@ -278,13 +275,6 @@ func ParseSessionState(data []byte) (*SessionState, error) { // sessionState returns a partially filled-out [SessionState] with information // from the current connection. func (c *Conn) sessionState() (*SessionState, error) { - var verifiedChains [][]*x509.Certificate - if c.isClient { - verifiedChains = c.verifiedChains - if len(c.peerCertificates) == 0 { - return nil, errors.New("tls: internal error: empty peer certificates") - } - } return &SessionState{ version: c.vers, cipherSuite: c.cipherSuite, @@ -296,7 +286,7 @@ func (c *Conn) sessionState() (*SessionState, error) { scts: c.scts, isClient: c.isClient, extMasterSecret: c.extMasterSecret, - verifiedChains: verifiedChains, + verifiedChains: c.verifiedChains, }, nil }