Skip to content

Commit

Permalink
Fix SKI computation in verification tests
Browse files Browse the repository at this point in the history
Currently, we are computing the subject key identifier as the hash
of 0x04 || pubkey.x || pubkey.y and checking that it matches what's
in the cert in the go verification tests. The pubkey is also pulled
from the cert and we get its byte representation through big.int::Bytes.

For our purposes, this function is error-prone as it returns the absolute
value of the pubkey coordinate. If the coordinate's first bit is 1 (i.e.
the coordinate is negative), we would get the wrong byte representation
and our hash would be incorrect. So instead of pulling the pubkey from the
cert, in this PR we just pull it from the certifyKeyResponse since it's
guaranteed to always have the correct pubkey representation. This should
fix this flaky test failures in Caliptra.
  • Loading branch information
sree-revoori1 committed Mar 15, 2024
1 parent 4369214 commit 9594f80
Showing 1 changed file with 7 additions and 11 deletions.
18 changes: 7 additions & 11 deletions verification/testing/certifyKey.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ func TestCertifyKeyCsr(d client.TestDPEInstance, c client.DPEClient, t *testing.
}

// Check fields and extensions in the CSR
checkCertifyKeyExtensions(t, csr.Extensions, flags, label, csr.PublicKey, false, certChain[len(certChain)-1].SubjectKeyId)
checkCertifyKeyExtensions(t, csr.Extensions, flags, label, certifyKeyResp.Pub, false, certChain[len(certChain)-1].SubjectKeyId)
checkPubKey(t, profile, csr.PublicKey, *certifyKeyResp)

// Check that CSR is self-signed
Expand Down Expand Up @@ -333,7 +333,7 @@ func checkCertifyKeyExtendedKeyUsages(t *testing.T, extensions []pkix.Extension,
// Checks for KeyUsage Extension as per spec
// If IsCA = true, KeyUsage extension MUST contain DigitalSignature and KeyCertSign
// If IsCA = false, KeyUsage extension MUST contain only DigitalSignature
func checkCertifyKeyExtensions(t *testing.T, extensions []pkix.Extension, flags client.CertifyKeyFlags, label []byte, pubkey any, IsX509 bool, IssuerSki []byte) {
func checkCertifyKeyExtensions(t *testing.T, extensions []pkix.Extension, flags client.CertifyKeyFlags, label []byte, pubkey client.DPEPubKey, IsX509 bool, IssuerSki []byte) {
t.Helper()

bc, err := getBasicConstraints(extensions)
Expand Down Expand Up @@ -381,7 +381,7 @@ func checkCertifyKeyExtensions(t *testing.T, extensions []pkix.Extension, flags
// against the isCa flag set in the BasicConstraints extension
// The SubjectKeyIdentifier extension MUST be included if isCA is true
// and MUST be excluded if isCA is false.
func checkCertifyKeySubjectKeyIdentifierExtension(t *testing.T, extensions []pkix.Extension, ca bool, pubkey any) {
func checkCertifyKeySubjectKeyIdentifierExtension(t *testing.T, extensions []pkix.Extension, ca bool, pubkey client.DPEPubKey) {
t.Helper()

ski, err := getSubjectKeyIdentifier(extensions)
Expand All @@ -392,19 +392,15 @@ func checkCertifyKeySubjectKeyIdentifierExtension(t *testing.T, extensions []pki
if ski == nil {
t.Errorf("[ERROR]: The certificate is a CA but the SubjectKeyIdentifier extension is not present.")
}
ecdsaPub, ok := pubkey.(*ecdsa.PublicKey)
if !ok {
t.Fatal("[FATAL]: Public key is not a ecdsa key")
}
var hasher hash.Hash
if ecdsaPub.Curve.Params().BitSize == 256 {
if len(pubkey.X) == 32 {
hasher = sha256.New()
} else {
hasher = sha512.New384()
}
hasher.Write([]byte{0x04})
hasher.Write(ecdsaPub.X.Bytes())
hasher.Write(ecdsaPub.Y.Bytes())
hasher.Write(pubkey.X)
hasher.Write(pubkey.Y)
expectedKeyIdentifier := hasher.Sum(nil)[:20]
if !reflect.DeepEqual(ski, expectedKeyIdentifier) {
t.Errorf("[ERROR]: The value of the subject key identifier %v is not equal to the hash of the public key %v", ski, expectedKeyIdentifier)
Expand Down Expand Up @@ -584,7 +580,7 @@ func testCertifyKey(d client.TestDPEInstance, c client.DPEClient, t *testing.T,
checkPubKey(t, profile, leafCert.PublicKey, *certifyKeyResp)

// Check all extensions
checkCertifyKeyExtensions(t, leafCert.Extensions, params.Flags, params.Label, leafCert.PublicKey, true, certChain[len(certChain)-1].SubjectKeyId)
checkCertifyKeyExtensions(t, leafCert.Extensions, params.Flags, params.Label, certifyKeyResp.Pub, true, certChain[len(certChain)-1].SubjectKeyId)

// Ensure full certificate chain has valid signatures
// This also checks certificate lifetime, signatures as part of cert chain validation
Expand Down

0 comments on commit 9594f80

Please sign in to comment.