From 4b14a26a3d133b08c7f033cfd9f937f1e0c86349 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Fri, 23 Feb 2024 17:56:50 -0500 Subject: [PATCH] Forbid RSA delegated credentials RFC 9345 has this bizarre special case forbiding the rsaEncryption OID for delegated credentials. This doesn't make much sense as DCs already constrain to a single signature algorithm. In fact, they didn't need to use SPKIs at all and could have just encoded the type-specific values. Nonetheless, this is where the spec went up. We have long rejected the RSASSA-PSS OID as being unusably complex, so this effectively means we will never permit RSA delegated credentials. This was another oversight in https://boringssl-review.googlesource.com/c/34884. Fix it separately before everything is reworked to SSL_CREDENTIAL. Bug: 249 Change-Id: I7eae1e8da9da8052b8d985e78388ef8f2b235942 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66567 Reviewed-by: Bob Beck Commit-Queue: David Benjamin (cherry picked from commit c9a9d8d5a90b55bea3ce019465821478e7036077) --- ssl/ssl_cert.cc | 10 ++++++++++ ssl/test/runner/runner.go | 21 ++++++++++++++++++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/ssl/ssl_cert.cc b/ssl/ssl_cert.cc index cd09d847bc..932b4801cb 100644 --- a/ssl/ssl_cert.cc +++ b/ssl/ssl_cert.cc @@ -859,6 +859,16 @@ UniquePtr DC::Parse(CRYPTO_BUFFER *in, uint8_t *out_alert) { return nullptr; } + // RFC 9345 forbids algorithms that use the rsaEncryption OID. As the + // RSASSA-PSS OID is unusably complicated, this effectively means we will not + // support RSA delegated credentials. + if (SSL_get_signature_algorithm_key_type(dc->dc_cert_verify_algorithm) == + EVP_PKEY_RSA) { + OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SIGNATURE_ALGORITHM); + *out_alert = SSL_AD_ILLEGAL_PARAMETER; + return nullptr; + } + return dc; } diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index 2d1d9d6bbd..9d53ab6fbc 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -335,7 +335,7 @@ func createDelegatedCredential(config delegatedCredentialConfig, parentDER []byt switch dcAlgo { case signatureRSAPKCS1WithMD5, signatureRSAPKCS1WithSHA1, signatureRSAPKCS1WithSHA256, signatureRSAPKCS1WithSHA384, signatureRSAPKCS1WithSHA512, signatureRSAPSSWithSHA256, signatureRSAPSSWithSHA384, signatureRSAPSSWithSHA512: pub = &rsa2048Key.PublicKey - privPKCS8, err = x509.MarshalPKCS8PrivateKey(rsa2048Key) + privPKCS8, err = x509.MarshalPKCS8PrivateKey(&rsa2048Key) if err != nil { return nil, nil, err } @@ -16951,6 +16951,25 @@ func addDelegatedCredentialTests() { shouldFail: true, expectedError: ":KEY_VALUES_MISMATCH:", }) + + // RSA delegated credentials should be rejected at configuration time. + rsaDC, rsaPKCS8, err := createDelegatedCredential(delegatedCredentialConfig{ + algo: signatureRSAPSSWithSHA256, + dcAlgo: signatureRSAPSSWithSHA256, + }, rsaCertificate.Leaf.Raw, rsaCertificate.PrivateKey) + if err != nil { + panic(err) + } + rsaFlagValue := fmt.Sprintf("%x,%x", rsaDC, rsaPKCS8) + testCases = append(testCases, testCase{ + testType: serverTest, + name: "DelegatedCredentials-NoRSA", + flags: []string{ + "-delegated-credential", rsaFlagValue, + }, + shouldFail: true, + expectedError: ":INVALID_SIGNATURE_ALGORITHM:", + }) } type echCipher struct {