From 2f213f643f11cb45af5e405b596ff00ab007613b Mon Sep 17 00:00:00 2001 From: Watson Ladd Date: Tue, 12 Feb 2019 16:59:54 -0800 Subject: [PATCH] Update delegated credentials to draft-03 Change-Id: I0c648340ac7bb134fcda42c56a83f4815bbaa557 Reviewed-on: https://boringssl-review.googlesource.com/c/34884 Commit-Queue: David Benjamin Reviewed-by: David Benjamin --- include/openssl/ssl.h | 2 +- ssl/internal.h | 5 +---- ssl/ssl_cert.cc | 15 +++----------- ssl/test/runner/handshake_client.go | 7 +------ ssl/test/runner/handshake_messages.go | 8 +++----- ssl/test/runner/runner.go | 29 ++++----------------------- 6 files changed, 13 insertions(+), 53 deletions(-) diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index fa0f6b2b..70ad930b 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -3072,7 +3072,7 @@ OPENSSL_EXPORT void SSL_get_peer_quic_transport_params(const SSL *ssl, // // Servers configure a DC for use in the handshake via // |SSL_set1_delegated_credential|. It must be signed by the host's end-entity -// certificate as defined in draft-ietf-tls-subcerts-02. +// certificate as defined in draft-ietf-tls-subcerts-03. // SSL_set1_delegated_credential configures the delegated credential (DC) that // will be sent to the peer for the current connection. |dc| is the DC in wire diff --git a/ssl/internal.h b/ssl/internal.h index 2c7f606e..37d3177e 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -1378,7 +1378,7 @@ enum handback_t { // Delegated credentials. // This structure stores a delegated credential (DC) as defined by -// draft-ietf-tls-subcerts-02. +// draft-ietf-tls-subcerts-03. struct DC { static constexpr bool kAllowUniquePtr = true; ~DC(); @@ -1399,9 +1399,6 @@ struct DC { // key. uint16_t expected_cert_verify_algorithm = 0; - // expected_version is the protocol in which the DC must be used. - uint16_t expected_version = 0; - // pkey is the public key parsed from |public_key|. UniquePtr pkey; diff --git a/ssl/ssl_cert.cc b/ssl/ssl_cert.cc index 52a1ddf3..1b01e7fe 100644 --- a/ssl/ssl_cert.cc +++ b/ssl/ssl_cert.cc @@ -763,7 +763,6 @@ UniquePtr DC::Dup() { ret->raw = UpRef(raw); ret->expected_cert_verify_algorithm = expected_cert_verify_algorithm; - ret->expected_version = expected_version; ret->pkey = UpRef(pkey); return ret; } @@ -784,7 +783,6 @@ UniquePtr DC::Parse(CRYPTO_BUFFER *in, uint8_t *out_alert) { CRYPTO_BUFFER_init_CBS(dc->raw.get(), &deleg); if (!CBS_get_u32(&deleg, &valid_time) || !CBS_get_u16(&deleg, &dc->expected_cert_verify_algorithm) || - !CBS_get_u16(&deleg, &dc->expected_version) || !CBS_get_u24_length_prefixed(&deleg, &pubkey) || !CBS_get_u16(&deleg, &algorithm) || !CBS_get_u16_length_prefixed(&deleg, &sig) || @@ -818,17 +816,10 @@ static bool ssl_can_serve_dc(const SSL_HANDSHAKE *hs) { return false; } - // Check that the negotiated version matches the protocol version to which the - // DC is bound, and that 1.3 or higher has been negotiated. - // - // NOTE: We use |hs->ssl->version| for checking the DC expected version. We - // don't call |ssl_protocol_version| because we need the version sent on the - // wire. For example, a delegated credential can be bound to a draft of TLS - // 1.3. + // Check that 1.3 or higher has been negotiated. const DC *dc = cert->dc.get(); assert(hs->ssl->s3->have_version); - if (hs->ssl->version != dc->expected_version || - ssl_protocol_version(hs->ssl) < TLS1_3_VERSION) { + if (ssl_protocol_version(hs->ssl) < TLS1_3_VERSION) { return false; } @@ -846,7 +837,7 @@ static bool ssl_can_serve_dc(const SSL_HANDSHAKE *hs) { } bool ssl_signing_with_dc(const SSL_HANDSHAKE *hs) { - // As of draft-ietf-tls-subcert-02, only the server may use delegated + // As of draft-ietf-tls-subcert-03, only the server may use delegated // credentials to authenticate itself. return hs->ssl->server && hs->delegated_credential_requested && diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go index 2472fc9e..45dc75d1 100644 --- a/ssl/test/runner/handshake_client.go +++ b/ssl/test/runner/handshake_client.go @@ -1382,7 +1382,7 @@ func (hs *clientHandshakeState) doFullHandshake() error { // delegatedCredentialSignedMessage returns the bytes that are signed in order // to authenticate a delegated credential. func delegatedCredentialSignedMessage(credBytes []byte, algorithm signatureAlgorithm, leafDER []byte) []byte { - // https://tools.ietf.org/html/draft-ietf-tls-subcerts-02#section-3 + // https://tools.ietf.org/html/draft-ietf-tls-subcerts-03#section-3 ret := make([]byte, 64, 128) for i := range ret { ret[i] = 0x20 @@ -1467,11 +1467,6 @@ func (hs *clientHandshakeState) verifyCertificates(certMsg *certificateMsg) erro if dc != nil { // Note that this doesn't check a) the delegated credential temporal // validity nor b) that the certificate has the special OID asserted. - if dc.expectedTLSVersion != c.wireVersion { - c.sendAlert(alertBadCertificate) - return errors.New("tls: delegated credential is for wrong TLS version") - } - hs.skxAlgo = dc.expectedCertVerifyAlgo var err error diff --git a/ssl/test/runner/handshake_messages.go b/ssl/test/runner/handshake_messages.go index 8ff1defd..f12ca1ac 100644 --- a/ssl/test/runner/handshake_messages.go +++ b/ssl/test/runner/handshake_messages.go @@ -1618,11 +1618,10 @@ type certificateEntry struct { } type delegatedCredential struct { - // https://tools.ietf.org/html/draft-ietf-tls-subcerts-02#section-3 + // https://tools.ietf.org/html/draft-ietf-tls-subcerts-03#section-3 signedBytes []byte lifetimeSecs uint32 expectedCertVerifyAlgo signatureAlgorithm - expectedTLSVersion uint16 pkixPublicKey []byte algorithm signatureAlgorithm signature []byte @@ -1725,7 +1724,7 @@ func (m *certificateMsg) unmarshal(data []byte) bool { case extensionSignedCertificateTimestamp: cert.sctList = []byte(body) case extensionDelegatedCredentials: - // https://tools.ietf.org/html/draft-ietf-tls-subcerts-02#section-3 + // https://tools.ietf.org/html/draft-ietf-tls-subcerts-03#section-3 if cert.delegatedCredential != nil { return false } @@ -1736,7 +1735,6 @@ func (m *certificateMsg) unmarshal(data []byte) bool { if !body.readU32(&dc.lifetimeSecs) || !body.readU16(&expectedCertVerifyAlgo) || - !body.readU16(&dc.expectedTLSVersion) || !body.readU24LengthPrefixedBytes(&dc.pkixPublicKey) || !body.readU16(&algorithm) || !body.readU16LengthPrefixedBytes(&dc.signature) || @@ -1746,7 +1744,7 @@ func (m *certificateMsg) unmarshal(data []byte) bool { dc.expectedCertVerifyAlgo = signatureAlgorithm(expectedCertVerifyAlgo) dc.algorithm = signatureAlgorithm(algorithm) - dc.signedBytes = []byte(origBody)[:4+2+2+3+len(dc.pkixPublicKey)] + dc.signedBytes = []byte(origBody)[:4+2+3+len(dc.pkixPublicKey)] cert.delegatedCredential = dc default: return false diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index ff4efb63..8461bd86 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -346,7 +346,6 @@ func createDelegatedCredential(config delegatedCredentialConfig, parentDER []byt if lifetimeSecs > 1<<32 { return nil, nil, fmt.Errorf("lifetime %s is too long to be expressed", lifetime) } - tlsVersion := config.tlsVersion if tlsVersion == 0 { tlsVersion = VersionTLS13 @@ -356,10 +355,9 @@ func createDelegatedCredential(config delegatedCredentialConfig, parentDER []byt return nil, nil, fmt.Errorf("delegated credentials require TLS 1.3") } - // https://tools.ietf.org/html/draft-ietf-tls-subcerts-02#section-3 + // https://tools.ietf.org/html/draft-ietf-tls-subcerts-03#section-3 dc = append(dc, byte(lifetimeSecs>>24), byte(lifetimeSecs>>16), byte(lifetimeSecs>>8), byte(lifetimeSecs)) dc = append(dc, byte(expectedAlgo>>8), byte(expectedAlgo)) - dc = append(dc, byte(tlsVersion>>8), byte(tlsVersion)) pubBytes, err := x509.MarshalPKIXPublicKey(pub) if err != nil { @@ -14987,34 +14985,15 @@ func addDelegatedCredentialTests() { }, }) - badTLSVersionDC, badTLSVersionPKCS8, err := createDelegatedCredential(delegatedCredentialConfig{ + // This flag value has mismatched public and private keys which should cause a + // configuration error in the shim. + _, badTLSVersionPKCS8, err := createDelegatedCredential(delegatedCredentialConfig{ algo: signatureRSAPSSWithSHA256, tlsVersion: 0x1234, }, parentDER, rsaPriv) if err != nil { panic(err) } - badTLSVersionFlagValue := fmt.Sprintf("%x,%x", badTLSVersionDC, badTLSVersionPKCS8) - - testCases = append(testCases, testCase{ - testType: serverTest, - name: "DelegatedCredentials-BadTLSVersion", - config: Config{ - // The delegated credential specifies a crazy TLS version, which should - // prevent its use. - MinVersion: VersionTLS13, - MaxVersion: VersionTLS13, - Bugs: ProtocolBugs{ - FailIfDelegatedCredentials: true, - }, - }, - flags: []string{ - "-delegated-credential", badTLSVersionFlagValue, - }, - }) - - // This flag value has mismatched public and private keys which should cause a - // configuration error in the shim. mismatchFlagValue := fmt.Sprintf("%x,%x", ecdsaDC, badTLSVersionPKCS8) testCases = append(testCases, testCase{ testType: serverTest,