Skip to content

Commit

Permalink
Forbid RSA delegated credentials
Browse files Browse the repository at this point in the history
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 <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit c9a9d8d5a90b55bea3ce019465821478e7036077)
  • Loading branch information
davidben authored and andrewhop committed Nov 25, 2024
1 parent 2098704 commit 4b14a26
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 1 deletion.
10 changes: 10 additions & 0 deletions ssl/ssl_cert.cc
Original file line number Diff line number Diff line change
Expand Up @@ -859,6 +859,16 @@ UniquePtr<DC> 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;
}

Expand Down
21 changes: 20 additions & 1 deletion ssl/test/runner/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 4b14a26

Please sign in to comment.