diff --git a/client/internal/certproof/collect.go b/client/internal/certproof/collect.go index 40dce1bc9..367ec7dc6 100644 --- a/client/internal/certproof/collect.go +++ b/client/internal/certproof/collect.go @@ -2,7 +2,9 @@ package certproof import ( "context" + "crypto" "crypto/sha256" + "crypto/x509" "time" log "github.com/sirupsen/logrus" @@ -73,18 +75,22 @@ func CollectChallenges(ctx context.Context, store Store, challenges []*proto.Cer continue } leaf := candidate.Chain[0] - if err := certposture.VerifyChain(candidate.Chain, roots, now); err != nil { - log.Debugf("certificate posture: challenge %d rejected %q issued by %q, chain of %d: %v", i, leaf.Subject, leaf.Issuer, len(candidate.Chain), err) + chain, err := certposture.VerifiedChain(leaf, candidate.issuers(), roots, now) + if err != nil { + log.Debugf("certificate posture: challenge %d rejected %q issued by %q: %v", i, leaf.Subject, leaf.Issuer, err) continue } matched = true - fingerprint := sha256.Sum256(leaf.Raw) + // The same leaf can chain to different CAs for different challenges, and + // management checks each chain against each check's CAs, so a proof is + // deduplicated by its whole chain rather than by its leaf. + fingerprint := chainFingerprint(chain) if _, done := proven[fingerprint]; done { log.Debugf("certificate posture: challenge %d matched %q, already proven for an earlier challenge", i, leaf.Subject) break } - proof, err := prove(candidate, challenge.GetNonce(), peerKey) + proof, err := prove(candidate.Signer, chain, challenge.GetNonce(), peerKey) if err != nil { log.Warnf("failed signing certificate proof for %s: %v", leaf.Subject, err) continue @@ -125,14 +131,23 @@ func wellFormed(challenges []*proto.CertificateChallenge) []*proto.CertificateCh return kept } -func prove(candidate Candidate, nonce, peerKey []byte) (certposture.Proof, error) { - sigAlg, sig, err := certposture.Sign(candidate.Signer, nonce, peerKey) +func prove(signer crypto.Signer, chain []*x509.Certificate, nonce, peerKey []byte) (certposture.Proof, error) { + sigAlg, sig, err := certposture.Sign(signer, nonce, peerKey) if err != nil { return certposture.Proof{}, err } - chain := make([][]byte, 0, len(candidate.Chain)) - for _, cert := range candidate.Chain { - chain = append(chain, cert.Raw) + der := make([][]byte, 0, len(chain)) + for _, cert := range chain { + der = append(der, cert.Raw) } - return certposture.Proof{Nonce: nonce, Chain: chain, SigAlg: sigAlg, Signature: sig}, nil + return certposture.Proof{Nonce: nonce, Chain: der, SigAlg: sigAlg, Signature: sig}, nil +} + +func chainFingerprint(chain []*x509.Certificate) [sha256.Size]byte { + buf := make([]byte, 0, len(chain)*sha256.Size) + for _, cert := range chain { + certHash := sha256.Sum256(cert.Raw) + buf = append(buf, certHash[:]...) + } + return sha256.Sum256(buf) } diff --git a/client/internal/certproof/collect_test.go b/client/internal/certproof/collect_test.go index bc6594355..344e8b7a8 100644 --- a/client/internal/certproof/collect_test.go +++ b/client/internal/certproof/collect_test.go @@ -2,6 +2,9 @@ package certproof import ( "context" + "crypto/rand" + "crypto/x509" + "math/big" "os" "path/filepath" "testing" @@ -160,3 +163,87 @@ func TestFileStore_SkipsKeyOfAnotherCertificate(t *testing.T) { _, err = challenger.Verify(proofs[0], peerKey, now) assert.NoError(t, err, "the proof sent is one management accepts") } + +// staticStore hands out fixed candidates, for scenarios no on-disk layout can express. +type staticStore []Candidate + +func (s staticStore) Candidates(context.Context) ([]Candidate, error) { return s, nil } + +func TestCollectChallenges_RoutesAroundExpiredCopyOfRenewedIntermediate(t *testing.T) { + root := certtest.NewCA(t, "root") + intermediate := certtest.NewIntermediate(t, root, "issuing-ca") + + // Renewing a CA with the same key pair leaves two certificates with the same + // subject and key in the store. The expired one sorts first here. + expiredTmpl := *intermediate.Cert + expiredTmpl.SerialNumber = big.NewInt(1) + expiredTmpl.NotBefore = time.Now().Add(-72 * time.Hour) + expiredTmpl.NotAfter = time.Now().Add(-48 * time.Hour) + der, err := x509.CreateCertificate(rand.Reader, &expiredTmpl, root.Cert, intermediate.Key.Public(), root.Key) + require.NoError(t, err) + expired, err := x509.ParseCertificate(der) + require.NoError(t, err) + + key := certtest.ECDSAKey(t) + leaf := intermediate.Issue(t, key, "device") + pool := []*x509.Certificate{expired, intermediate.Cert} + + chain := buildChain(leaf, pool) + require.Len(t, chain, 2) + require.True(t, expired.Equal(chain[1]), "precondition: the first-match chain runs through the expired copy") + + challenger := certposture.NewChallenger([]byte("secret")) + now := time.Now() + nonce := challenger.Nonce(peerKey, now) + store := staticStore{{Chain: chain, Signer: key, Intermediates: pool}} + + proofs := CollectChallenges(context.Background(), store, []*proto.CertificateChallenge{{Nonce: nonce, CaCertificates: []string{root.PEM}}}, peerKey) + + require.Len(t, proofs, 1, "a valid path through the renewed intermediate exists, so the challenge is answered") + verified, err := challenger.Verify(proofs[0], peerKey, now) + require.NoError(t, err) + assert.True(t, intermediate.Cert.Equal(verified[1]), "the proof carries the valid intermediate, not the expired copy") + assert.True(t, certposture.ChainMatchesCAs(certposture.EncodeChainPEM(verified), []string{root.PEM}, now), + "management's own check accepts the chain the proof carries") +} + +func TestCollectChallenges_ProvesALeafOncePerDistinctChain(t *testing.T) { + rootA := certtest.NewCA(t, "root-a") + rootB := certtest.NewCA(t, "root-b") + issuer := certtest.NewIntermediate(t, rootA, "issuing-ca") + + // The same issuing CA cross-signed by a second root: one leaf, two valid paths. + crossTmpl := *issuer.Cert + crossTmpl.SerialNumber = big.NewInt(2) + der, err := x509.CreateCertificate(rand.Reader, &crossTmpl, rootB.Cert, issuer.Key.Public(), rootB.Key) + require.NoError(t, err) + cross, err := x509.ParseCertificate(der) + require.NoError(t, err) + + key := certtest.ECDSAKey(t) + leaf := issuer.Issue(t, key, "device") + pool := []*x509.Certificate{issuer.Cert, cross} + store := staticStore{{Chain: buildChain(leaf, pool), Signer: key, Intermediates: pool}} + + challenger := certposture.NewChallenger([]byte("secret")) + now := time.Now() + nonce := challenger.Nonce(peerKey, now) + challenges := []*proto.CertificateChallenge{ + {Nonce: nonce, CaCertificates: []string{rootA.PEM}}, + {Nonce: nonce, CaCertificates: []string{rootB.PEM}}, + {Nonce: nonce, CaCertificates: []string{rootA.PEM}}, + } + + proofs := CollectChallenges(context.Background(), store, challenges, peerKey) + + require.Len(t, proofs, 2, "one proof per distinct chain, the repeated root-a challenge reuses the first") + for _, root := range []*certtest.CA{rootA, rootB} { + matched := false + for _, p := range proofs { + chain, err := challenger.Verify(p, peerKey, now) + require.NoError(t, err) + matched = matched || certposture.ChainMatchesCAs(certposture.EncodeChainPEM(chain), []string{root.PEM}, now) + } + assert.True(t, matched, "management can match a chain for the check that trusts %s", root.Cert.Subject.CommonName) + } +} diff --git a/client/internal/certproof/keychain_darwin.go b/client/internal/certproof/keychain_darwin.go index 4613d1e9c..ac817002f 100644 --- a/client/internal/certproof/keychain_darwin.go +++ b/client/internal/certproof/keychain_darwin.go @@ -103,7 +103,7 @@ func (s *KeychainStore) Candidates(_ context.Context) ([]Candidate, error) { if len(chain) == 1 && leaf.CheckSignatureFrom(leaf) != nil { log.Debugf("keychain candidate %q has no issuer in the keychain, its proof carries the leaf alone and only verifies if the challenge supplies %q", leaf.Subject, leaf.Issuer) } - candidates = append(candidates, Candidate{Chain: chain, Signer: &keychainSigner{leaf: leaf}}) + candidates = append(candidates, Candidate{Chain: chain, Signer: &keychainSigner{leaf: leaf}, Intermediates: pool}) } return candidates, nil } diff --git a/client/internal/certproof/pkcs11store.go b/client/internal/certproof/pkcs11store.go index 6f17be319..cf3e3bd50 100644 --- a/client/internal/certproof/pkcs11store.go +++ b/client/internal/certproof/pkcs11store.go @@ -113,7 +113,7 @@ func (s *PKCS11Store) Candidates(_ context.Context) ([]Candidate, error) { func (s *PKCS11Store) candidate(leaf *x509.Certificate, id []byte, pool []*x509.Certificate) Candidate { chain := buildChain(leaf, pool) log.Debugf("%s candidate %q issued by %q built a chain of %d certificates", s, leaf.Subject, leaf.Issuer, len(chain)) - return Candidate{Chain: chain, Signer: &pkcs11Signer{store: s, leaf: leaf, id: id}} + return Candidate{Chain: chain, Signer: &pkcs11Signer{store: s, leaf: leaf, id: id}, Intermediates: pool} } // fileChains reads the certificate files in the PEM directory that carry no key of their diff --git a/client/internal/certproof/store.go b/client/internal/certproof/store.go index e7f222fe0..11b758cd4 100644 --- a/client/internal/certproof/store.go +++ b/client/internal/certproof/store.go @@ -9,6 +9,7 @@ import ( "fmt" "os" "path/filepath" + "slices" "strings" log "github.com/sirupsen/logrus" @@ -26,9 +27,18 @@ const ( var errKeyMismatch = errors.New("private key does not match the certificate") // Candidate is a certificate chain the peer can sign for. Signer never exposes the key. +// Chain is leaf first. Intermediates holds every other certificate the store has, so a +// path to a challenge's CAs can be found even where Chain followed a different issuer, +// such as an expired copy of a renewed intermediate. type Candidate struct { - Chain []*x509.Certificate - Signer crypto.Signer + Chain []*x509.Certificate + Signer crypto.Signer + Intermediates []*x509.Certificate +} + +// issuers is every certificate other than the leaf that a path may run through. +func (c Candidate) issuers() []*x509.Certificate { + return append(slices.Clip(c.Chain[1:]), c.Intermediates...) } // Store yields the certificates a peer may prove possession of. FileStore is the PEM diff --git a/client/internal/certproof/systemstore_windows.go b/client/internal/certproof/systemstore_windows.go index c7d29da95..1b8bb608e 100644 --- a/client/internal/certproof/systemstore_windows.go +++ b/client/internal/certproof/systemstore_windows.go @@ -88,7 +88,7 @@ func (s *SystemStore) Candidates(_ context.Context) ([]Candidate, error) { for _, leaf := range leaves { chain := buildChain(leaf, pool) log.Debugf("certificate store %s candidate %q issued by %q built a chain of %d certificates", s, leaf.Subject, leaf.Issuer, len(chain)) - candidates = append(candidates, Candidate{Chain: chain, Signer: &systemStoreSigner{leaf: leaf, location: s.location}}) + candidates = append(candidates, Candidate{Chain: chain, Signer: &systemStoreSigner{leaf: leaf, location: s.location}, Intermediates: pool}) } return candidates, nil } diff --git a/shared/management/certposture/chain.go b/shared/management/certposture/chain.go index d0e4bc27d..80fb46b4b 100644 --- a/shared/management/certposture/chain.go +++ b/shared/management/certposture/chain.go @@ -28,17 +28,32 @@ func VerifyChain(chain []*x509.Certificate, roots *x509.CertPool, now time.Time) if len(chain) == 0 { return ErrEmptyChain } - intermediates := x509.NewCertPool() - for _, cert := range chain[1:] { - intermediates.AddCert(cert) + _, err := VerifiedChain(chain[0], chain[1:], roots, now) + return err +} + +// VerifiedChain finds a path from leaf to one of roots through any of intermediates, +// trying every candidate issuer rather than the first that matches, and returns it +// without the root, leaf first. It applies the same rules as VerifyChain. +func VerifiedChain(leaf *x509.Certificate, intermediates []*x509.Certificate, roots *x509.CertPool, now time.Time) ([]*x509.Certificate, error) { + pool := x509.NewCertPool() + for _, cert := range intermediates { + pool.AddCert(cert) } - _, err := chain[0].Verify(x509.VerifyOptions{ + chains, err := leaf.Verify(x509.VerifyOptions{ Roots: roots, - Intermediates: intermediates, + Intermediates: pool, CurrentTime: now, KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageAny}, }) - return err + if err != nil { + return nil, err + } + chain := chains[0] + if len(chain) > 1 { + chain = chain[:len(chain)-1] + } + return chain, nil } // ChainMatchesCAs is VerifyChain over the PEM forms stored in peer meta and check config.