Skip certificate files whose key belongs to another certificate

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")
+}

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 "<name>.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