Skip to content

Commit

Permalink
Make DelegatedCredentials-KeyMismatch test less confusing
Browse files Browse the repository at this point in the history
This is passing in a different TLS version, but the TLS version is both
nonsense and doesn't figure into the delegated credential anyway. All
this test is doing is generating a different keypair and mixing them up.
Probably we should move it to ssl_test, as it's not really testing
anything about the protocol, but I've just left it alone and fixed the
test.

Also fix another issue in the test: the getSigner / signMessage chord
should just be a plain signMessage call. There were a few other issues
of that shape, but they'll be fixed in a follow-up change because they
reveal a deeper problem with
https://boringssl-review.googlesource.com/c/34884

Change-Id: I090b41a081f694b4ff8d97f3895645d6a620904d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66549
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
(cherry picked from commit 9f376b0694dfb8528fa2200369b48632563e972f)
  • Loading branch information
davidben authored and andrewhop committed Nov 25, 2024
1 parent 186d177 commit 2098704
Showing 1 changed file with 5 additions and 30 deletions.
35 changes: 5 additions & 30 deletions ssl/test/runner/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,9 +301,6 @@ type delegatedCredentialConfig struct {
// dcAlgo is the signature scheme that should be used with this delegated
// credential. If zero, ECDSA with P-256 is assumed.
dcAlgo signatureAlgorithm
// tlsVersion is the version of TLS that should be used with this delegated
// credential. If zero, TLS 1.3 is assumed.
tlsVersion uint16
// algo is the signature algorithm that the delegated credential itself is
// signed with. Cannot be zero.
algo signatureAlgorithm
Expand Down Expand Up @@ -379,14 +376,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
}

if tlsVersion < VersionTLS13 {
return nil, nil, fmt.Errorf("delegated credentials require TLS 1.3")
}

// https://www.rfc-editor.org/rfc/rfc9345.html#section-4
dc = append(dc, byte(lifetimeSecs>>24), byte(lifetimeSecs>>16), byte(lifetimeSecs>>8), byte(lifetimeSecs))
Expand All @@ -401,12 +390,7 @@ func createDelegatedCredential(config delegatedCredentialConfig, parentDER []byt
dc = append(dc, pubBytes...)

var dummyConfig Config
parentSigner, err := getSigner(tlsVersion, parentPriv, &dummyConfig, config.algo, false /* not for verification */)
if err != nil {
return nil, nil, err
}

parentSignature, err := parentSigner.signMessage(parentPriv, &dummyConfig, delegatedCredentialSignedMessage(dc, config.algo, parentDER))
parentSignature, err := signMessage(VersionTLS13, parentPriv, &dummyConfig, config.algo, delegatedCredentialSignedMessage(dc, config.algo, parentDER))
if err != nil {
return nil, nil, err
}
Expand Down Expand Up @@ -16950,26 +16934,17 @@ func addDelegatedCredentialTests() {
},
})

// 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,
// Generate another delegated credential, so we can get the keys out of sync.
_, ecdsaPKCS8Wrong, err := createDelegatedCredential(delegatedCredentialConfig{
algo: signatureRSAPSSWithSHA256,
}, rsaCertificate.Leaf.Raw, rsaCertificate.PrivateKey)
if err != nil {
panic(err)
}
mismatchFlagValue := fmt.Sprintf("%x,%x", ecdsaDC, badTLSVersionPKCS8)
mismatchFlagValue := fmt.Sprintf("%x,%x", ecdsaDC, ecdsaPKCS8Wrong)
testCases = append(testCases, testCase{
testType: serverTest,
name: "DelegatedCredentials-KeyMismatch",
config: Config{
MinVersion: VersionTLS13,
MaxVersion: VersionTLS13,
Bugs: ProtocolBugs{
FailIfDelegatedCredentials: true,
},
},
flags: []string{
"-delegated-credential", mismatchFlagValue,
},
Expand Down

0 comments on commit 2098704

Please sign in to comment.