Sign only nonces and peer keys of the size management issues

client/internal/certproof/collect.go

@@ -6,6 +6,7 @@ import (
 	"time"
 
 	log "github.com/sirupsen/logrus"
+	"golang.zx2c4.com/wireguard/wgctrl/wgtypes"
 
 	"github.com/netbirdio/netbird/shared/management/certposture"
 	"github.com/netbirdio/netbird/shared/management/proto"
@@ -31,7 +32,17 @@ func logNoChallenges(checks []*proto.Checks) {
 
 // CollectChallenges answers challenges already extracted from the posture checks, so a
 // caller that ships them across a process boundary reuses the same matching and signing.
+// Only nonces of the size management issues are signed, for a peer key of the size of
+// ours, so the keys behind the store never sign arbitrary caller-chosen data.
 func CollectChallenges(ctx context.Context, store Store, challenges []*proto.CertificateChallenge, peerKey []byte) []certposture.Proof {
+	if len(peerKey) != wgtypes.KeyLen {
+		log.Warnf("certificate posture: refusing to sign for a %d byte peer key", len(peerKey))
+		return nil
+	}
+	challenges = wellFormed(challenges)
+	if len(challenges) == 0 {
+		return nil
+	}
 	log.Debugf("certificate posture: answering %d certificate challenges from store %T", len(challenges), store)
 
 	candidates, err := store.Candidates(ctx)
@@ -94,11 +105,24 @@ func CollectChallenges(ctx context.Context, store Store, challenges []*proto.Cer
 func certificateChallenges(checks []*proto.Checks) []*proto.CertificateChallenge {
 	var challenges []*proto.CertificateChallenge
 	for _, check := range checks {
-		if challenge := check.GetCertificateChallenge(); challenge != nil && len(challenge.GetNonce()) > 0 {
+		if challenge := check.GetCertificateChallenge(); challenge != nil {
 			challenges = append(challenges, challenge)
 		}
 	}
-	return challenges
+	return wellFormed(challenges)
+}
+
+// wellFormed drops challenges whose nonce is not one management could have issued.
+func wellFormed(challenges []*proto.CertificateChallenge) []*proto.CertificateChallenge {
+	var kept []*proto.CertificateChallenge
+	for _, challenge := range challenges {
+		if len(challenge.GetNonce()) != certposture.NonceSize {
+			log.Debugf("certificate posture: skipping challenge with a %d byte nonce", len(challenge.GetNonce()))
+			continue
+		}
+		kept = append(kept, challenge)
+	}
+	return kept
 }
 
 func prove(candidate Candidate, nonce, peerKey []byte) (certposture.Proof, error) {

client/internal/certproof/collect_test.go

@@ -106,3 +106,31 @@ func writeFile(t *testing.T, dir, name, content string) {
 	t.Helper()
 	require.NoError(t, os.WriteFile(filepath.Join(dir, name), []byte(content), 0o600))
 }
+
+func TestCollectChallenges_RefusesMalformedInput(t *testing.T) {
+	ca := certtest.NewCA(t, "corp-root")
+	dir := t.TempDir()
+	key := certtest.ECDSAKey(t)
+	writeFile(t, dir, "device.pem", certtest.CertPEM(ca.Issue(t, key, "device"))+certtest.KeyPEM(t, key))
+	store := NewFileStore(dir)
+	nonce := certposture.NewChallenger([]byte("secret")).Nonce(peerKey, time.Now())
+
+	tests := []struct {
+		name    string
+		nonce   []byte
+		peerKey []byte
+		want    int
+	}{
+		{"issued nonce and peer key are signed", nonce, peerKey, 1},
+		{"short nonce is not signed", nonce[:8], peerKey, 0},
+		{"oversized nonce is not signed", append(append([]byte{}, nonce...), 0), peerKey, 0},
+		{"short peer key is not signed", nonce, peerKey[:16], 0},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			challenges := []*proto.CertificateChallenge{{Nonce: tt.nonce, CaCertificates: []string{ca.PEM}}}
+			assert.Len(t, CollectChallenges(context.Background(), store, challenges, tt.peerKey), tt.want,
+				"the device key signs only what management could have issued")
+		})
+	}
+}

client/internal/certproof/collector_test.go

@@ -12,7 +12,9 @@ import (
 	"github.com/netbirdio/netbird/shared/management/proto"
 )
 
-var challengeChecks = []*proto.Checks{{CertificateChallenge: &proto.CertificateChallenge{Nonce: []byte("nonce")}}}
+var challengeChecks = []*proto.Checks{{CertificateChallenge: &proto.CertificateChallenge{
+	Nonce: certposture.NewChallenger([]byte("secret")).Nonce(peerKey, time.Now()),
+}}}
 
 func TestCollector_SkipsChecksWithoutChallenges(t *testing.T) {
 	var c Collector

shared/management/certposture/challenge.go

@@ -14,6 +14,9 @@ const (
 	challengeDomain = "netbird-cert-challenge-v1"
 	windowLen       = 8
 	nonceLen        = windowLen + sha256.Size
+
+	// NonceSize is the length of every nonce a Challenger issues.
+	NonceSize = nonceLen
 )
 
 var (