From 59aeeb1c94e3147e0e6d698cf0c8e393bcc5d010 Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Thu, 1 Oct 2026 09:07:56 +0200 Subject: [PATCH] Skip certificate files whose key belongs to another certificate --- client/internal/certproof/collect_test.go | 26 ++++++++++++ client/internal/certproof/store.go | 52 +++++++++++++++++------ 2 files changed, 64 insertions(+), 14 deletions(-) diff --git a/client/internal/certproof/collect_test.go b/client/internal/certproof/collect_test.go index 53bfebe02..bc6594355 100644 --- a/client/internal/certproof/collect_test.go +++ b/client/internal/certproof/collect_test.go @@ -134,3 +134,29 @@ func TestCollectChallenges_RefusesMalformedInput(t *testing.T) { }) } } + +func TestFileStore_SkipsKeyOfAnotherCertificate(t *testing.T) { + ca := certtest.NewCA(t, "corp-root") + dir := t.TempDir() + + // A stale key next to a renewed certificate, sorted before the good pair, must not + // produce a proof that management rejects and stop the search there. + writeFile(t, dir, "a-renewed.crt", certtest.CertPEM(ca.Issue(t, certtest.ECDSAKey(t), "renewed"))) + writeFile(t, dir, "a-renewed.key", certtest.KeyPEM(t, certtest.ECDSAKey(t))) + goodKey := certtest.ECDSAKey(t) + good := ca.Issue(t, goodKey, "good") + writeFile(t, dir, "b-good.pem", certtest.CertPEM(good)+certtest.KeyPEM(t, goodKey)) + + candidates, err := NewFileStore(dir).Candidates(context.Background()) + require.NoError(t, err) + require.Len(t, candidates, 1, "only the certificate whose key matches is a candidate") + assert.True(t, good.Equal(candidates[0].Chain[0]), "the matching pair is kept") + + challenger := certposture.NewChallenger([]byte("secret")) + now := time.Now() + nonce := challenger.Nonce(peerKey, now) + proofs := CollectChallenges(context.Background(), NewFileStore(dir), []*proto.CertificateChallenge{{Nonce: nonce, CaCertificates: []string{ca.PEM}}}, peerKey) + require.Len(t, proofs, 1) + _, err = challenger.Verify(proofs[0], peerKey, now) + assert.NoError(t, err, "the proof sent is one management accepts") +} diff --git a/client/internal/certproof/store.go b/client/internal/certproof/store.go index 666bb1be3..e7f222fe0 100644 --- a/client/internal/certproof/store.go +++ b/client/internal/certproof/store.go @@ -21,6 +21,10 @@ const ( defaultStoreDir = "/etc/netbird/certs" ) +// errKeyMismatch rejects a key that does not belong to the certificate it sits with: it +// would sign a proof management can only reject, in place of a usable later candidate. +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. type Candidate struct { Chain []*x509.Certificate @@ -120,25 +124,45 @@ func loadPEM(path string) ([]*x509.Certificate, crypto.Signer, error) { if len(chain) == 0 { return nil, nil, errors.New("no certificate") } - if signer != nil { - return chain, signer, nil - } - keyData, err := os.ReadFile(strings.TrimSuffix(path, filepath.Ext(path)) + ".key") - if errors.Is(err, os.ErrNotExist) { - return chain, nil, nil - } - if err != nil { - return nil, nil, fmt.Errorf("read key file: %w", err) - } - if _, signer, err = parsePEM(keyData); err != nil { - return nil, nil, err - } if signer == nil { - return nil, nil, errors.New("no private key in key file") + if signer, err = siblingKey(path); err != nil { + return nil, nil, err + } + if signer == nil { + return chain, nil, nil + } + } + if !samePublicKey(signer.Public(), chain[0].PublicKey) { + return nil, nil, errKeyMismatch } return chain, signer, nil } +// siblingKey reads the private key from the ".key" file next to a certificate +// file, or returns nil when there is no such file. +func siblingKey(path string) (crypto.Signer, error) { + keyData, err := os.ReadFile(strings.TrimSuffix(path, filepath.Ext(path)) + ".key") + if errors.Is(err, os.ErrNotExist) { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("read key file: %w", err) + } + _, signer, err := parsePEM(keyData) + if err != nil { + return nil, err + } + if signer == nil { + return nil, errors.New("no private key in key file") + } + return signer, nil +} + +func samePublicKey(a, b crypto.PublicKey) bool { + equaler, ok := a.(interface{ Equal(crypto.PublicKey) bool }) + return ok && equaler.Equal(b) +} + func parsePEM(data []byte) ([]*x509.Certificate, crypto.Signer, error) { var chain []*x509.Certificate var signer crypto.Signer