From 41ef4d1cd5ee14b2000a01f1f2b9a939b444804d Mon Sep 17 00:00:00 2001 From: marctrem <1619947+marctrem@users.noreply.github.com> Date: Fri, 15 Aug 2025 11:36:01 -0700 Subject: [PATCH] simple chain validation --- README.md | 13 +++ appattest/appattest_impl.go | 50 ++++------ appattest/appattest_impl_test.go | 50 +++------- appattest/verify_attestation.go | 87 +++++++---------- appattest/verify_chain.go | 163 +++++++++++++++++++++++++++++++ mint/mint_attestation.go | 2 +- mint/mint_attestation_test.go | 9 +- 7 files changed, 247 insertions(+), 127 deletions(-) create mode 100644 appattest/verify_chain.go diff --git a/README.md b/README.md index 11c4b14..d2e549b 100644 --- a/README.md +++ b/README.md @@ -62,6 +62,19 @@ func main() { log.Fatalf("attestation: attested bundle differs from the expected one") } + // Verify validity instant + instant := time.Now() + if !(instant.After(res.LeafCert.NotBefore) && instant.Before(res.LeafCert.NotAfter)) { + log.Fatalf("attestation: not valid at expected time") + } + + // (optional) verify the key id of the signer + expectedKeyID := []byte("myexpectedkeyid") + if !bytes.Equal(expectedKeyID, res.KeyID) { + log.Fatalf("attestation: unexpected signer id ") + } + + fmt.Printf("Attestation successful. Sign count: %d\n", res.AuthenticatorData.SignCount) } ``` diff --git a/appattest/appattest_impl.go b/appattest/appattest_impl.go index 6eef7c7..a166664 100644 --- a/appattest/appattest_impl.go +++ b/appattest/appattest_impl.go @@ -2,7 +2,8 @@ package appattest import ( "crypto/x509" - "time" + "encoding/pem" + "fmt" "github.com/pkg/errors" "github.com/splitsecure/go-app-attest/authenticatordata" @@ -28,13 +29,11 @@ type Attestor interface { } type AttestorImpl struct { - aaroots *x509.CertPool - nowfn func() time.Time + aaroots []*x509.Certificate } type optionsState struct { - aaroots *x509.CertPool - nowfn func() time.Time + aaroots []*x509.Certificate } type option struct { @@ -47,17 +46,10 @@ func newoption(fn func(*optionsState)) option { } } -// WithAppAttestRoots lets the user provide its own authoritative certs pool -func WithAppAttestRoots(pool *x509.CertPool) option { +// WithAppAttestRoots lets the user provide its own authoritative certificates +func WithAppAttestRoots(certs []*x509.Certificate) option { return newoption(func(s *optionsState) { - s.aaroots = pool - }) -} - -// WithNowFn lets the user provide its own time.Now function -func WithNowFn(now func() time.Time) option { - return newoption(func(os *optionsState) { - os.nowfn = now + s.aaroots = certs }) } @@ -73,41 +65,37 @@ func New( option.apply(&optionsState) } - // determine pool + // determine root certificates if optionsState.aaroots == nil { // use the certificate provided by the library - att.aaroots = x509.NewCertPool() - if !att.aaroots.AppendCertsFromPEM([]byte(appattestRootCAPEM)) { - return nil, errors.New("loading library provided app attest ca") + block, _ := pem.Decode([]byte(appattestRootCAPEM)) + if block == nil { + return nil, errors.New("failed to parse app attest root CA PEM") + } + cert, err := x509.ParseCertificate(block.Bytes) + if err != nil { + return nil, fmt.Errorf("parsing app attest root CA: %w", err) } + att.aaroots = []*x509.Certificate{cert} } else { - // use the user provided pool + // use the user provided certificates att.aaroots = optionsState.aaroots } - // determine timefn - if optionsState.nowfn == nil { - att.nowfn = time.Now - } else { - att.nowfn = optionsState.nowfn - } - return att, nil } type VerifyAttestationInput struct { ServerChallenge []byte AttestationCBOR []byte - KeyIdentifier []byte OutAuthenticatorData *authenticatordata.T } func (at *AttestorImpl) VerifyAttestation(in *VerifyAttestationInput) (VerifyAttestationOutput, error) { - subtleIn := VerifyAttestationInputStateless{ + subtleIn := VerifyAttestationInputPure{ AttestationInput: in, - Time: at.nowfn(), AARoots: at.aaroots, } - return VerifyAttestationStateless(&subtleIn) + return VerifyAttestationPure(&subtleIn) } diff --git a/appattest/appattest_impl_test.go b/appattest/appattest_impl_test.go index 3d2ee6d..8438de4 100644 --- a/appattest/appattest_impl_test.go +++ b/appattest/appattest_impl_test.go @@ -13,21 +13,11 @@ import ( ) func TestAppAttest(t *testing.T) { - nowFn := func() time.Time { - t, err := time.Parse(time.DateOnly, "2024-04-18") - if err != nil { - panic(err) - } - return t - } - // create an attestor bundleDigest, err := base64.StdEncoding.AppendDecode(nil, []byte("FVhAM8lQuf6dUUziohGjJtcaprEBSrTG+i+9qdmqGKY=")) require.NoError(t, err) - attestor, err := appattest.New( - appattest.WithNowFn(nowFn), - ) + attestor, err := appattest.New() require.NoError(t, err) // deserialize the sample case @@ -41,7 +31,6 @@ func TestAppAttest(t *testing.T) { req := appattest.VerifyAttestationInput{ ServerChallenge: []byte("test_server_challenge"), AttestationCBOR: attestationCBOR, - KeyIdentifier: keyIdentifier, } res, err := attestor.VerifyAttestation(&req) @@ -49,24 +38,19 @@ func TestAppAttest(t *testing.T) { assert.Equal(t, uint32(0), res.AuthenticatorData.SignCount) require.Equal(t, bundleDigest, res.BundleDigest) require.Equal(t, appattest.AAGUIDProd, res.EnvironmentGUID) + + validInstant := time.Date(2024, 4, 18, 0, 0, 0, 0, time.UTC) + assert.True(t, validInstant.Before(res.LeafCert.NotAfter)) + assert.True(t, validInstant.After(res.LeafCert.NotBefore)) + assert.Equal(t, keyIdentifier, res.KeyID) } func TestAppAttestDev(t *testing.T) { - nowFn := func() time.Time { - t, err := time.Parse(time.DateOnly, "2024-09-05") - if err != nil { - panic(err) - } - return t - } - // pre-hash has the following shape: ABC6DEF.com.example.fooapp bundleDigest, err := base64.StdEncoding.AppendDecode(nil, []byte("FcoOH+2hZbXEsTrH0Orwx24jatXg6mk7q+38tfqkUbg=")) require.NoError(t, err) - attestor, err := appattest.New( - appattest.WithNowFn(nowFn), - ) + attestor, err := appattest.New() require.NoError(t, err) // deserialize the sample case @@ -81,7 +65,6 @@ func TestAppAttestDev(t *testing.T) { req := appattest.VerifyAttestationInput{ ServerChallenge: chalSum[:], AttestationCBOR: attestationCBOR, - KeyIdentifier: keyIdentifier, } res, err := attestor.VerifyAttestation(&req) @@ -89,6 +72,10 @@ func TestAppAttestDev(t *testing.T) { assert.Equal(t, uint32(0), res.AuthenticatorData.SignCount) assert.Equal(t, bundleDigest, res.BundleDigest) assert.Equal(t, appattest.AAGUIDDev, res.EnvironmentGUID) + validInstant := time.Date(2025, 4, 5, 0, 0, 0, 0, time.UTC) + assert.True(t, validInstant.Before(res.LeafCert.NotAfter)) + assert.True(t, validInstant.After(res.LeafCert.NotBefore)) + assert.Equal(t, keyIdentifier, res.KeyID) } func FuzzAttestationData(f *testing.F) { @@ -104,28 +91,15 @@ func FuzzAttestationData(f *testing.F) { f.Add(tc) // Use f.Add to provide a seed corpus } - // prepare an attestor - nowFn := func() time.Time { - t, err := time.Parse(time.DateOnly, "2024-09-05") - if err != nil { - panic(err) - } - return t - } - - attestor, err := appattest.New( - appattest.WithNowFn(nowFn), - ) + attestor, err := appattest.New() require.NoError(f, err) chalSum := sha256.Sum256([]byte("server_challenge")) - keyIdentifier, err := base64.StdEncoding.AppendDecode(nil, []byte("B3hMn1CG/4of7s+TUkKLrS/6FAxsGft2N6PJhrzXI4E=")) require.NoError(f, err) out := authenticatordata.T{} req := appattest.VerifyAttestationInput{ ServerChallenge: chalSum[:], - KeyIdentifier: keyIdentifier, OutAuthenticatorData: &out, } diff --git a/appattest/verify_attestation.go b/appattest/verify_attestation.go index 21bc47d..88794cc 100644 --- a/appattest/verify_attestation.go +++ b/appattest/verify_attestation.go @@ -7,7 +7,9 @@ import ( "crypto/x509" "encoding/asn1" "encoding/hex" + "encoding/pem" "fmt" + "os" "reflect" "slices" "time" @@ -21,10 +23,18 @@ const ( Format = "apple-appattest" ) -type VerifyAttestationInputStateless struct { +var ( + NonceOID = asn1.ObjectIdentifier{1, 2, 840, 113635, 100, 8, 2} + AAGUIDProd = Environment("appattest\x00\x00\x00\x00\x00\x00\x00") + AAGUIDDev = Environment("appattestdevelop") +) + +type Environment = []byte + +type VerifyAttestationInputPure struct { AttestationInput *VerifyAttestationInput Time time.Time - AARoots *x509.CertPool + AARoots []*x509.Certificate } type VerifyAttestationOutput struct { @@ -33,6 +43,7 @@ type VerifyAttestationOutput struct { EnvironmentGUID Environment BundleDigest []byte + KeyID []byte } // AttestedPubkey returns the key from the leaf certificate @@ -40,8 +51,8 @@ func (o *VerifyAttestationOutput) AttestedPubkey() *ecdsa.PublicKey { return o.LeafCert.PublicKey.(*ecdsa.PublicKey) } -// VerifyAttestationStateless performs attestation without the guardrails provided by AppAttestImpl. -func VerifyAttestationStateless(in *VerifyAttestationInputStateless) (VerifyAttestationOutput, error) { +// VerifyAttestationPure performs attestation without the guardrails provided by AppAttestImpl. +func VerifyAttestationPure(in *VerifyAttestationInputPure) (VerifyAttestationOutput, error) { // unmarshal the attestation object attestObj := AttestationObject{} err := cbor.Unmarshal(in.AttestationInput.AttestationCBOR, &attestObj) @@ -54,24 +65,30 @@ func VerifyAttestationStateless(in *VerifyAttestationInputStateless) (VerifyAtte return VerifyAttestationOutput{}, fmt.Errorf("attestation object format mismatch: expected '%s', got '%s'", Format, attestObj.Format) } - // create a new cert verifier using the intermediates provided in the attestation object - verifyOpts := x509.VerifyOptions{} - if err := populateVerifyOpts(&verifyOpts, &attestObj, in.AARoots); err != nil { - return VerifyAttestationOutput{}, fmt.Errorf("populating verify opts: %w", err) + // Parse certificate chain from bytes to certificates + chain := make([]*x509.Certificate, len(attestObj.AttestationStatement.X509CertChain)) + for i, certBytes := range attestObj.AttestationStatement.X509CertChain { + cert, err := x509.ParseCertificate(certBytes) + if err != nil { + return VerifyAttestationOutput{}, fmt.Errorf("parsing certificate at index %d: %w", i, err) + } + chain[i] = cert } - verifyOpts.CurrentTime = in.Time - // parse the leaf certificate - leafCert, err := x509.ParseCertificate(attestObj.AttestationStatement.X509CertChain[0]) - if err != nil { - return VerifyAttestationOutput{}, fmt.Errorf("parsing leaf certificate: %w", err) + if err := VerifyChain(chain, in.AARoots); err != nil { + return VerifyAttestationOutput{}, fmt.Errorf("verifying certificate chain: %w", err) } - // verify the leaf certificate - _, err = leafCert.Verify(verifyOpts) - if err != nil { - return VerifyAttestationOutput{}, fmt.Errorf("verifying leaf certificate: %w", err) + // get the leaf certificate (first in chain) + leafCert := chain[0] + pblock := pem.Block{ + Type: "CERTIFICATE", + Bytes: leafCert.Raw, } + if err := pem.Encode(os.Stdout, &pblock); err != nil { + panic(err) + } + fmt.Println() // > 2. Create clientDataHash as the SHA256 hash of the one-time challenge your server sends // > to your app before performing the attestation, @@ -102,11 +119,6 @@ func VerifyAttestationStateless(in *VerifyAttestationInputStateless) (VerifyAtte computedPubkeyHash := ComputeKeyHash(certPubKey) - // assert that the public key of the leaf certificate matches the key handle returned by the app - if !bytes.Equal(in.AttestationInput.KeyIdentifier, computedPubkeyHash[:]) { - return VerifyAttestationOutput{}, fmt.Errorf("key identifier did not match public key of leaf certificate: %s != %s", hex.EncodeToString(computedPubkeyHash[:]), hex.EncodeToString(in.AttestationInput.KeyIdentifier)) - } - authenticatorData := in.AttestationInput.OutAuthenticatorData if authenticatorData == nil { authenticatorData = &authenticatordata.T{} @@ -117,7 +129,7 @@ func VerifyAttestationStateless(in *VerifyAttestationInputStateless) (VerifyAtte } // > 9. Verify that the authenticator data’s credentialId field is the same as the key identifier. - if !bytes.Equal(in.AttestationInput.KeyIdentifier, authenticatorData.AttestedCredentialData.CredentialID) { + if !bytes.Equal(computedPubkeyHash[:], authenticatorData.AttestedCredentialData.CredentialID) { return VerifyAttestationOutput{}, fmt.Errorf("key identifier did not match attested credential id of authenticator data") } @@ -127,29 +139,10 @@ func VerifyAttestationStateless(in *VerifyAttestationInputStateless) (VerifyAtte EnvironmentGUID: authenticatorData.AttestedCredentialData.AAGUID, BundleDigest: authenticatorData.RelayingPartyHash, + KeyID: computedPubkeyHash[:], }, nil } -func populateVerifyOpts(dst *x509.VerifyOptions, attObj *AttestationObject, aaroots *x509.CertPool) (err error) { - if len(attObj.AttestationStatement.X509CertChain) < 1 { - return errors.New("expected at least one certificate in x509 cert chain") - } - - // set the intermediates - dst.Intermediates = x509.NewCertPool() - // skip the first element, it's the leaf certificate - for _, inter := range attObj.AttestationStatement.X509CertChain[1:] { - cert, err := x509.ParseCertificate(inter) - if err != nil { - return errors.Wrap(err, "parsing intermediate") - } - dst.Intermediates.AddCert(cert) - dst.Roots = aaroots - } - - return nil -} - func extractNonceFromCert(c *x509.Certificate) ([]byte, error) { var oidValue []byte for _, ext := range c.Extensions { @@ -216,11 +209,3 @@ func ComputeNonce(authData, clientDataHash []byte) (res [sha256.Size]byte, err e func ComputeKeyHash(key *ecdsa.PublicKey) [sha256.Size]byte { return sha256.Sum256(ellipticPointToX962Uncompressed(key)) } - -type Environment = []byte - -var ( - NonceOID = asn1.ObjectIdentifier{1, 2, 840, 113635, 100, 8, 2} - AAGUIDProd = Environment("appattest\x00\x00\x00\x00\x00\x00\x00") - AAGUIDDev = Environment("appattestdevelop") -) diff --git a/appattest/verify_chain.go b/appattest/verify_chain.go new file mode 100644 index 0000000..0d3fd53 --- /dev/null +++ b/appattest/verify_chain.go @@ -0,0 +1,163 @@ +package appattest + +import ( + "bytes" + "crypto/x509" + "fmt" + + "github.com/pkg/errors" +) + +// validateSignatureAlgorithm checks if the signature algorithm is acceptable +func validateSignatureAlgorithm(cert *x509.Certificate) error { + switch cert.SignatureAlgorithm { + case x509.MD2WithRSA, x509.MD5WithRSA, x509.SHA1WithRSA: + return fmt.Errorf("weak signature algorithm: %v", cert.SignatureAlgorithm) + } + return nil +} + +// validateBasicConstraints checks basic constraints for CA certificates +func validateBasicConstraints(cert *x509.Certificate, isCA bool, pathLen int) error { + if isCA { + if !cert.IsCA { + return fmt.Errorf("certificate must be a CA certificate") + } + if cert.MaxPathLen >= 0 && pathLen > cert.MaxPathLen { + return fmt.Errorf("path length %d exceeds maximum allowed %d", pathLen, cert.MaxPathLen) + } + } else { + // End entity certificate should not be a CA + if cert.IsCA { + return fmt.Errorf("end entity certificate cannot be a CA") + } + } + return nil +} + +// validateKeyUsage checks key usage for certificates +func validateKeyUsage(cert *x509.Certificate, isCA bool) error { + if isCA { + if cert.KeyUsage&x509.KeyUsageCertSign == 0 { + return fmt.Errorf("CA certificate missing Certificate Sign key usage") + } + } else { + fmt.Println(cert.KeyUsage) + if cert.KeyUsage&x509.KeyUsageDigitalSignature == 0 { + return fmt.Errorf("end entity certificate missing required key usage (Digital Signature)") + } + } + return nil +} + +// validateCriticalExtensions checks for unknown critical extensions +func validateCriticalExtensions(cert *x509.Certificate) error { + knownCriticalExtensions := map[string]struct{}{ + "2.5.29.19": {}, // Basic Constraints + "2.5.29.15": {}, // Key Usage + "2.5.29.37": {}, // Extended Key Usage + "2.5.29.17": {}, // Subject Alternative Name + "2.5.29.35": {}, // Authority Key Identifier + "2.5.29.14": {}, // Subject Key Identifier + "2.5.29.32": {}, // Certificate Policies + "2.5.29.31": {}, // CRL Distribution Points + "1.3.6.1.5.5.7.1.1": {}, // Authority Information Access + } + + for _, ext := range cert.Extensions { + if ext.Critical { + if _, ok := knownCriticalExtensions[ext.Id.String()]; !ok { + return fmt.Errorf("unknown critical extension: %s", ext.Id.String()) + } + } + } + return nil +} + +// VerifyChain is a simplified verification routine. +// Since it might be desireable to verify the validity of an attestation +// beyond its lifetime, this function returns the timerange in which it was valid. +// It assumes that the chain will be passed in the order of leaf to root. +func VerifyChain(chain []*x509.Certificate, roots []*x509.Certificate) error { + if len(chain) == 0 { + return fmt.Errorf("chain is empty") + } + + // Validate each certificate in the chain + for i, cert := range chain { + // Validate signature algorithm + if err := validateSignatureAlgorithm(cert); err != nil { + return fmt.Errorf("certificate at index %d has invalid signature algorithm: %w", i, err) + } + + // Validate critical extensions + if err := validateCriticalExtensions(cert); err != nil { + return fmt.Errorf("certificate at index %d has invalid critical extensions: %w", i, err) + } + + // Determine if this certificate is a CA (all except leaf are CAs) + isCA := i != 0 + pathLen := len(chain) - 1 - i // Path length remaining after this cert + + // Validate basic constraints + if err := validateBasicConstraints(cert, isCA, pathLen); err != nil { + return fmt.Errorf("certificate at index %d fails basic constraints validation: %w", i, err) + } + + // Validate key usage + if err := validateKeyUsage(cert, isCA); err != nil { + return fmt.Errorf("certificate at index %d fails key usage validation: %w", i, err) + } + } + + if len(chain) > 1 { + for i := len(chain) - 1; i >= 1; i-- { + parent := chain[i] + child := chain[i-1] + + // Validate Subject/Issuer name chaining + if !bytes.Equal(parent.RawSubject, child.RawIssuer) { + return fmt.Errorf("certificate at index %d: issuer name does not match parent subject at index %d", i-1, i) + } + + if err := child.CheckSignatureFrom(parent); err != nil { + return fmt.Errorf("certificate at index %d not signed by parent at index %d: %w", i-1, i, err) + } + + // Ensure child certificate validity period is within parent's validity period + if child.NotBefore.Before(parent.NotBefore) { + return fmt.Errorf("certificate at index %d NotBefore (%v) is before parent NotBefore (%v)", i-1, child.NotBefore, parent.NotBefore) + } + if child.NotAfter.After(parent.NotAfter) { + return fmt.Errorf("certificate at index %d NotAfter (%v) is after parent NotAfter (%v)", i-1, child.NotAfter, parent.NotAfter) + } + } + } + + // top of chain must be valid against one of the roots + topOfChain := chain[len(chain)-1] + var validRoot *x509.Certificate + + for _, root := range roots { + // Check signature from root to top of chain + if err := topOfChain.CheckSignatureFrom(root); err == nil { + // Validate Subject/Issuer name chaining with root + if !bytes.Equal(root.RawSubject, topOfChain.RawIssuer) { + continue // Skip root with mismatched subject/issuer + } + + // Also check that the top certificate's validity period is within the root's validity period + if topOfChain.NotBefore.Before(root.NotBefore) || topOfChain.NotAfter.After(root.NotAfter) { + continue // This root can't validate this certificate due to timing constraints + } + validRoot = root + break + } + } + + if validRoot == nil { + return errors.New("chain is not issued by any known CA or validity periods don't align") + } + + return nil +} diff --git a/mint/mint_attestation.go b/mint/mint_attestation.go index 1cceb01..9d8dec3 100644 --- a/mint/mint_attestation.go +++ b/mint/mint_attestation.go @@ -139,7 +139,7 @@ func generateLeafCert( Subject: pkix.Name{CommonName: commonName}, NotBefore: notBefore, NotAfter: notAfter, - KeyUsage: x509.KeyUsageCertSign, + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageContentCommitment | x509.KeyUsageKeyEncipherment | x509.KeyUsageDataEncipherment, ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, BasicConstraintsValid: true, ExtraExtensions: exts, diff --git a/mint/mint_attestation_test.go b/mint/mint_attestation_test.go index 026ceee..8641e19 100644 --- a/mint/mint_attestation_test.go +++ b/mint/mint_attestation_test.go @@ -46,19 +46,16 @@ func TestMint(t *testing.T) { keyid := appattest.ComputeKeyHash(&attKey.PublicKey) - caPool := x509.NewCertPool() - caPool.AddCert(caCert) - - attverif, err := appattest.VerifyAttestationStateless(&appattest.VerifyAttestationInputStateless{ + attverif, err := appattest.VerifyAttestationPure(&appattest.VerifyAttestationInputPure{ AttestationInput: &appattest.VerifyAttestationInput{ ServerChallenge: []byte("server data"), AttestationCBOR: mintout.Attestation, - KeyIdentifier: keyid[:], }, Time: time.Now(), - AARoots: caPool, + AARoots: []*x509.Certificate{caCert}, }) require.NoError(t, err) require.Equal(t, appIDDigest[:], attverif.BundleDigest) require.Equal(t, appattest.AAGUIDDev, attverif.EnvironmentGUID) + require.Equal(t, keyid[:], attverif.KeyID) }