diff --git a/ssl/extensions.cc b/ssl/extensions.cc index 029533f58..e9c38a649 100644 --- a/ssl/extensions.cc +++ b/ssl/extensions.cc @@ -4119,15 +4119,16 @@ bool tls1_choose_signature_algorithm(SSL_HANDSHAKE *hs, uint16_t *out) { return true; } - Span sigalgs = kSignSignatureAlgorithms; + Span sigalgs, peer_sigalgs; if (ssl_signing_with_dc(hs)) { sigalgs = MakeConstSpan(&dc->dc_cert_verify_algorithm, 1); - } else if (!cert->sigalgs.empty()) { - sigalgs = cert->sigalgs; + peer_sigalgs = hs->peer_delegated_credential_sigalgs; + } else { + sigalgs = cert->sigalgs.empty() ? MakeConstSpan(kSignSignatureAlgorithms) + : cert->sigalgs; + peer_sigalgs = tls1_get_peer_verify_algorithms(hs); } - Span peer_sigalgs = tls1_get_peer_verify_algorithms(hs); - for (uint16_t sigalg : sigalgs) { if (!ssl_private_key_supports_signature_algorithm(hs, sigalg)) { continue; diff --git a/ssl/internal.h b/ssl/internal.h index 2dd78590f..d64f6c482 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -1630,9 +1630,14 @@ struct DC { // raw is the delegated credential encoded as specified in RFC 9345. UniquePtr raw; - // dc_cert_verify_algorithm is the signature scheme of the DC public key. + // dc_cert_verify_algorithm is the signature scheme of the DC public key. This + // is used for the CertificateVerify message. uint16_t dc_cert_verify_algorithm = 0; + // algorithm is the signature scheme of the signature over the delegated + // credential itself, made by the end-entity certificate's public key. + uint16_t algorithm = 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 17363f328..71baeb77b 100644 --- a/ssl/ssl_cert.cc +++ b/ssl/ssl_cert.cc @@ -118,6 +118,7 @@ #include #include +#include #include #include @@ -689,6 +690,7 @@ UniquePtr DC::Dup() { ret->raw = UpRef(raw); ret->dc_cert_verify_algorithm = dc_cert_verify_algorithm; + ret->algorithm = algorithm; ret->pkey = UpRef(pkey); return ret; } @@ -705,12 +707,11 @@ UniquePtr DC::Parse(CRYPTO_BUFFER *in, uint8_t *out_alert) { CBS pubkey, deleg, sig; uint32_t valid_time; - uint16_t algorithm; CRYPTO_BUFFER_init_CBS(dc->raw.get(), &deleg); if (!CBS_get_u32(&deleg, &valid_time) || !CBS_get_u16(&deleg, &dc->dc_cert_verify_algorithm) || !CBS_get_u24_length_prefixed(&deleg, &pubkey) || - !CBS_get_u16(&deleg, &algorithm) || + !CBS_get_u16(&deleg, &dc->algorithm) || !CBS_get_u16_length_prefixed(&deleg, &sig) || CBS_len(&deleg) != 0) { OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR); @@ -719,7 +720,7 @@ UniquePtr DC::Parse(CRYPTO_BUFFER *in, uint8_t *out_alert) { } dc->pkey.reset(EVP_parse_public_key(&pubkey)); - if (dc->pkey == nullptr) { + if (dc->pkey == nullptr || CBS_len(&pubkey) != 0) { OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR); *out_alert = SSL_AD_DECODE_ERROR; return nullptr; @@ -747,14 +748,21 @@ static bool ssl_can_serve_dc(const SSL_HANDSHAKE *hs) { return false; } - // Check that the DC signature algorithm is supported by the peer. - Span peer_sigalgs = hs->peer_delegated_credential_sigalgs; - for (uint16_t peer_sigalg : peer_sigalgs) { - if (dc->dc_cert_verify_algorithm == peer_sigalg) { - return true; - } + // Check that the peer supports the signature over the delegated credential. + if (std::find(hs->peer_sigalgs.begin(), hs->peer_sigalgs.end(), + dc->algorithm) == hs->peer_sigalgs.end()) { + return false; } - return false; + + // Check that the peer supports the CertificateVerify signature algorithm. + if (std::find(hs->peer_delegated_credential_sigalgs.begin(), + hs->peer_delegated_credential_sigalgs.end(), + dc->dc_cert_verify_algorithm) == + hs->peer_delegated_credential_sigalgs.end()) { + return false; + } + + return true; } bool ssl_signing_with_dc(const SSL_HANDSHAKE *hs) { diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go index 666b4b65b..69d8a695b 100644 --- a/ssl/test/runner/common.go +++ b/ssl/test/runner/common.go @@ -112,7 +112,7 @@ const ( extensionPadding uint16 = 21 extensionExtendedMasterSecret uint16 = 23 extensionCompressedCertAlgs uint16 = 27 - extensionDelegatedCredentials uint16 = 34 + extensionDelegatedCredential uint16 = 34 extensionSessionTicket uint16 = 35 extensionPreSharedKey uint16 = 41 extensionEarlyData uint16 = 42 @@ -591,6 +591,11 @@ type Config struct { // supported signature algorithms that are accepted. VerifySignatureAlgorithms []signatureAlgorithm + // DelegatedCredentialAlgorithms, if not empty, is the set of signature + // algorithms allowed for the delegated credential key. If empty, delegated + // credentials are disabled. + DelegatedCredentialAlgorithms []signatureAlgorithm + // QUICTransportParams, if not empty, will be sent in the QUIC // transport parameters extension. QUICTransportParams []byte @@ -1952,10 +1957,6 @@ type ProtocolBugs struct { // server returns delegated credentials. FailIfDelegatedCredentials bool - // DisableDelegatedCredentials, if true, disables client support for delegated - // credentials. - DisableDelegatedCredentials bool - // CompatModeWithQUIC, if true, enables TLS 1.3 compatibility mode // when running over QUIC. CompatModeWithQUIC bool diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go index 46194568a..94e1db344 100644 --- a/ssl/test/runner/handshake_client.go +++ b/ssl/test/runner/handshake_client.go @@ -39,6 +39,7 @@ type clientHandshakeState struct { session *ClientSessionState finishedBytes []byte peerPublicKey crypto.PublicKey + peerIsDC bool } func mapClientHelloVersion(vers uint16, isDTLS bool) uint16 { @@ -525,7 +526,7 @@ func (hs *clientHandshakeState) createClientHello(innerHello *clientHelloMsg, ec customExtension: c.config.Bugs.CustomExtension, omitExtensions: c.config.Bugs.OmitExtensions, emptyExtensions: c.config.Bugs.EmptyExtensions, - delegatedCredentials: !c.config.Bugs.DisableDelegatedCredentials, + delegatedCredential: c.config.DelegatedCredentialAlgorithms, } // Translate the bugs that modify ClientHello extension order into a @@ -1299,7 +1300,11 @@ func (hs *clientHandshakeState) doTLS13Handshake(msg any) error { c.peerSignatureAlgorithm = certVerifyMsg.signatureAlgorithm input := hs.finishedHash.certificateVerifyInput(serverCertificateVerifyContextTLS13) - err = verifyMessage(c.vers, hs.peerPublicKey, c.config, certVerifyMsg.signatureAlgorithm, input, certVerifyMsg.signature) + if hs.peerIsDC { + err = verifyMessageDC(c.vers, hs.peerPublicKey, c.config, certVerifyMsg.signatureAlgorithm, input, certVerifyMsg.signature) + } else { + err = verifyMessage(c.vers, hs.peerPublicKey, c.config, certVerifyMsg.signatureAlgorithm, input, certVerifyMsg.signature) + } if err != nil { return err } @@ -1836,7 +1841,7 @@ func (hs *clientHandshakeState) verifyCertificates(certMsg *certificateMsg) erro c.sendAlert(alertIllegalParameter) return errors.New("tls: non-leaf certificate has a delegated credential") } - if c.config.Bugs.DisableDelegatedCredentials { + if len(c.config.DelegatedCredentialAlgorithms) == 0 { c.sendAlert(alertIllegalParameter) return errors.New("tls: server sent delegated credential without it being requested") } @@ -1886,20 +1891,17 @@ func (hs *clientHandshakeState) verifyCertificates(certMsg *certificateMsg) erro return errors.New("tls: failed to parse public key from delegated credential: " + err.Error()) } - verifier, err := getSigner(c.vers, hs.peerPublicKey, c.config, dc.algorithm, true) - if err != nil { - c.sendAlert(alertBadCertificate) - return errors.New("tls: failed to get verifier for delegated credential: " + err.Error()) - } - - if err := verifier.verifyMessage(leafPublicKey, delegatedCredentialSignedMessage(dc.signedBytes, dc.algorithm, certs[0].Raw), dc.signature); err != nil { + signedMsg := delegatedCredentialSignedMessage(dc.signedBytes, dc.algorithm, certs[0].Raw) + if err := verifyMessage(c.vers, leafPublicKey, c.config, dc.algorithm, signedMsg, dc.signature); err != nil { c.sendAlert(alertBadCertificate) return errors.New("tls: failed to verify delegated credential: " + err.Error()) } - } else if c.config.Bugs.ExpectDelegatedCredentials { - c.sendAlert(alertInternalError) - return errors.New("tls: delegated credentials missing") + hs.peerIsDC = true } else { + if c.config.Bugs.ExpectDelegatedCredentials { + c.sendAlert(alertInternalError) + return errors.New("tls: delegated credentials missing") + } hs.peerPublicKey = leafPublicKey } diff --git a/ssl/test/runner/handshake_messages.go b/ssl/test/runner/handshake_messages.go index 26e125794..c3d9dd3b2 100644 --- a/ssl/test/runner/handshake_messages.go +++ b/ssl/test/runner/handshake_messages.go @@ -194,7 +194,7 @@ type clientHelloMsg struct { emptyExtensions bool pad int compressedCertAlgs []uint16 - delegatedCredentials bool + delegatedCredential []signatureAlgorithm alpsProtocols []string alpsProtocolsOld []string outerExtensions []uint16 @@ -501,15 +501,15 @@ func (m *clientHelloMsg) marshalBody(hello *cryptobyte.Builder, typ clientHelloT body: body.BytesOrPanic(), }) } - if m.delegatedCredentials { + if len(m.delegatedCredential) > 0 { body := cryptobyte.NewBuilder(nil) body.AddUint16LengthPrefixed(func(signatureSchemeList *cryptobyte.Builder) { - for _, sigAlg := range m.signatureAlgorithms { + for _, sigAlg := range m.delegatedCredential { signatureSchemeList.AddUint16(uint16(sigAlg)) } }) extensions = append(extensions, extension{ - id: extensionDelegatedCredentials, + id: extensionDelegatedCredential, body: body.BytesOrPanic(), }) } @@ -756,7 +756,7 @@ func (m *clientHelloMsg) unmarshal(data []byte) bool { m.alpnProtocols = nil m.extendedMasterSecret = false m.customExtension = "" - m.delegatedCredentials = false + m.delegatedCredential = nil m.alpsProtocols = nil m.alpsProtocolsOld = nil @@ -1029,11 +1029,10 @@ func (m *clientHelloMsg) unmarshal(data []byte) bool { return false } } - case extensionDelegatedCredentials: - if len(body) != 0 { + case extensionDelegatedCredential: + if !parseSignatureAlgorithms(&body, &m.delegatedCredential, false) || len(body) != 0 { return false } - m.delegatedCredentials = true case extensionApplicationSettings: var protocols cryptobyte.String if !body.ReadUint16LengthPrefixed(&protocols) || len(body) != 0 { @@ -2029,7 +2028,7 @@ func (m *certificateMsg) unmarshal(data []byte) bool { } case extensionSignedCertificateTimestamp: cert.sctList = []byte(body) - case extensionDelegatedCredentials: + case extensionDelegatedCredential: // https://www.rfc-editor.org/rfc/rfc9345.html#section-4 if cert.delegatedCredential != nil { return false diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index 5e34dbdd8..4b704db68 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -10347,8 +10347,7 @@ func addSignatureAlgorithmTests() { signatureRSAPKCS1WithSHA1, }, Bugs: ProtocolBugs{ - NoSignatureAlgorithms: true, - DisableDelegatedCredentials: true, + NoSignatureAlgorithms: true, }, }, shouldFail: true, @@ -16460,21 +16459,42 @@ func addDelegatedCredentialTests() { config: Config{ MinVersion: VersionTLS13, MaxVersion: VersionTLS13, + }, + flags: []string{ + "-delegated-credential", ecdsaFlagValue, + }, + }) + + testCases = append(testCases, testCase{ + testType: serverTest, + name: "DelegatedCredentials-Basic", + config: Config{ + MinVersion: VersionTLS13, + MaxVersion: VersionTLS13, + DelegatedCredentialAlgorithms: []signatureAlgorithm{signatureECDSAWithP256AndSHA256}, Bugs: ProtocolBugs{ - DisableDelegatedCredentials: true, + ExpectDelegatedCredentials: true, }, }, flags: []string{ "-delegated-credential", ecdsaFlagValue, + "-expect-delegated-credential-used", }, }) testCases = append(testCases, testCase{ testType: serverTest, - name: "DelegatedCredentials-Basic", + name: "DelegatedCredentials-ExactAlgorithmMatch", config: Config{ MinVersion: VersionTLS13, MaxVersion: VersionTLS13, + // Test that the server doesn't mix up the two signature algorithm + // fields. These options are a match because the signature_algorithms + // extension matches against the signature on the delegated + // credential, while the delegated_credential extension matches + // against the signature made by the delegated credential. + VerifySignatureAlgorithms: []signatureAlgorithm{signatureRSAPSSWithSHA256}, + DelegatedCredentialAlgorithms: []signatureAlgorithm{signatureECDSAWithP256AndSHA256}, Bugs: ProtocolBugs{ ExpectDelegatedCredentials: true, }, @@ -16491,13 +16511,33 @@ func addDelegatedCredentialTests() { config: Config{ MinVersion: VersionTLS13, MaxVersion: VersionTLS13, + // If the client doesn't support the signature in the delegated credential, + // the server should not use delegated credentials. + VerifySignatureAlgorithms: []signatureAlgorithm{signatureRSAPSSWithSHA384}, + DelegatedCredentialAlgorithms: []signatureAlgorithm{signatureECDSAWithP256AndSHA256}, Bugs: ProtocolBugs{ FailIfDelegatedCredentials: true, }, - // If the client doesn't support the delegated credential signature - // algorithm then the handshake should complete without using delegated + }, + flags: []string{ + "-delegated-credential", ecdsaFlagValue, + }, + }) + + testCases = append(testCases, testCase{ + testType: serverTest, + name: "DelegatedCredentials-CertVerifySigAlgoMissing", + config: Config{ + MinVersion: VersionTLS13, + MaxVersion: VersionTLS13, + // If the client doesn't support the delegated credential's + // CertificateVerify algorithm, the server should not use delegated // credentials. - VerifySignatureAlgorithms: []signatureAlgorithm{signatureRSAPSSWithSHA256}, + VerifySignatureAlgorithms: []signatureAlgorithm{signatureRSAPSSWithSHA256}, + DelegatedCredentialAlgorithms: []signatureAlgorithm{signatureECDSAWithP384AndSHA384}, + Bugs: ProtocolBugs{ + FailIfDelegatedCredentials: true, + }, }, flags: []string{ "-delegated-credential", ecdsaFlagValue, diff --git a/ssl/test/runner/sign.go b/ssl/test/runner/sign.go index e0c6a92b1..8b381399f 100644 --- a/ssl/test/runner/sign.go +++ b/ssl/test/runner/sign.go @@ -81,6 +81,19 @@ func verifyMessage(version uint16, key crypto.PublicKey, config *Config, sigAlg return signer.verifyMessage(key, msg, sig) } +func verifyMessageDC(version uint16, key crypto.PublicKey, config *Config, sigAlg signatureAlgorithm, msg, sig []byte) error { + if version >= VersionTLS12 && !slices.Contains(config.DelegatedCredentialAlgorithms, sigAlg) { + return errors.New("tls: unsupported signature algorithm") + } + + signer, err := getSigner(version, key, config, sigAlg, true) + if err != nil { + return err + } + + return signer.verifyMessage(key, msg, sig) +} + type rsaPKCS1Signer struct { hash crypto.Hash }