From c6ea802fe24f9ed6ecd8b97c3986b2b55864f991 Mon Sep 17 00:00:00 2001 From: JackDoan Date: Thu, 2 Jul 2026 19:40:44 -0500 Subject: [PATCH 1/2] keys: generate only P256 keys under FIPS 140-only mode Under GODEBUG=fips140=only, crypto/ecdh refuses to generate an X25519 key. As a result keys.New() failed outright, and even with that guarded, Client.Enroll() panicked dereferencing the nil Ed25519 host key. This made it impossible for a FIPS-enforcing build to enroll at all, even into a P256 network. Generate the P256 keypair unconditionally, and only generate the X25519/Ed25519 keypair when crypto/fips140 is not enabled. Enroll() now includes the 25519 public keys only when they were generated. Non-FIPS builds are unchanged: both key sets are generated and sent. FIPS builds send only P256, which the DN API accepts for P256 networks (FIPS networks are always P256). Verified end to end: a fips140=only dnclient now enrolls into a P256 network and completes a P256 Nebula handshake. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Sk4ZZA8bcjVi6FJ3k2opFB --- client.go | 29 ++++++++++++++---------- keys/keys.go | 62 ++++++++++++++++++++++++++++++++-------------------- 2 files changed, 55 insertions(+), 36 deletions(-) diff --git a/client.go b/client.go index bbdee26..305d4d4 100644 --- a/client.go +++ b/client.go @@ -136,24 +136,29 @@ func (c *Client) Enroll(ctx context.Context, logger *slog.Logger, code string) ( return nil, nil, nil, nil, err } - hostEd25519PublicKeyPEM, err := newKeys.HostEd25519PublicKey.MarshalPEM() - if err != nil { - return nil, nil, nil, nil, err - } - hostP256PublicKeyPEM, err := newKeys.HostP256PublicKey.MarshalPEM() if err != nil { return nil, nil, nil, nil, err } - // Make a request to the API with the enrollment code + // Make a request to the API with the enrollment code. P256 keys are always + // generated; the 25519 keys are omitted under FIPS (see keys.New), so only + // include them when they were generated. The DN API selects the key set to + // use based on the enrolled network's curve. payload := message.EnrollRequest{ - Code: code, - NebulaPubkeyX25519: newKeys.NebulaX25519PublicKeyPEM, - HostPubkeyEd25519: hostEd25519PublicKeyPEM, - NebulaPubkeyP256: newKeys.NebulaP256PublicKeyPEM, - HostPubkeyP256: hostP256PublicKeyPEM, - Timestamp: time.Now(), + Code: code, + NebulaPubkeyP256: newKeys.NebulaP256PublicKeyPEM, + HostPubkeyP256: hostP256PublicKeyPEM, + Timestamp: time.Now(), + } + + if newKeys.HostEd25519PublicKey != nil { + hostEd25519PublicKeyPEM, err := newKeys.HostEd25519PublicKey.MarshalPEM() + if err != nil { + return nil, nil, nil, nil, err + } + payload.NebulaPubkeyX25519 = newKeys.NebulaX25519PublicKeyPEM + payload.HostPubkeyEd25519 = hostEd25519PublicKeyPEM } reqID, r, err := callAPI[message.EnrollResponseData](ctx, c, "POST", message.EnrollEndpoint, payload) diff --git a/keys/keys.go b/keys/keys.go index 9c6cc2d..b086016 100644 --- a/keys/keys.go +++ b/keys/keys.go @@ -3,6 +3,7 @@ package keys import ( "crypto/ecdsa" "crypto/ed25519" + "crypto/fips140" "fmt" ) @@ -29,21 +30,10 @@ type Keys struct { } func New() (*Keys, error) { - x25519PublicKeyPEM, x25519PrivateKeyPEM, ed25519PublicKey, ed25519PrivateKey, err := newKeys25519() - if err != nil { - return nil, err - } - - ed25519PublicKeyI, err := NewPublicKey(ed25519PublicKey) - if err != nil { - return nil, err - } - - ed25519PrivateKeyI, err := NewPrivateKey(ed25519PrivateKey) - if err != nil { - return nil, err - } + k := &Keys{} + // P256 keys are FIPS 140-approved and are always generated. The DN API + // selects which key set to use based on the enrolled network's curve. ecdhP256PublicKeyPEM, ecdhP256PrivateKeyPEM, ecdsaP256PublicKey, ecdsaP256PrivateKey, err := newKeysP256() if err != nil { return nil, err @@ -59,16 +49,40 @@ func New() (*Keys, error) { return nil, err } - return &Keys{ - NebulaX25519PublicKeyPEM: x25519PublicKeyPEM, - NebulaX25519PrivateKeyPEM: x25519PrivateKeyPEM, - HostEd25519PublicKey: ed25519PublicKeyI, - HostEd25519PrivateKey: ed25519PrivateKeyI, - NebulaP256PublicKeyPEM: ecdhP256PublicKeyPEM, - NebulaP256PrivateKeyPEM: ecdhP256PrivateKeyPEM, - HostP256PublicKey: ecdsaP256PublicKeyI, - HostP256PrivateKey: ecdsaP256PrivateKeyI, - }, nil + k.NebulaP256PublicKeyPEM = ecdhP256PublicKeyPEM + k.NebulaP256PrivateKeyPEM = ecdhP256PrivateKeyPEM + k.HostP256PublicKey = ecdsaP256PublicKeyI + k.HostP256PrivateKey = ecdsaP256PrivateKeyI + + // X25519/Ed25519 are not FIPS 140-approved. Under GODEBUG=fips140=only, + // crypto/ecdh refuses to generate an X25519 key at all, which would make + // enrollment fail before it ever reaches the API. When FIPS mode is enabled + // we therefore only offer P256 keys - FIPS networks are always P256, and the + // DN API accepts an enroll request that omits the 25519 keys (see the + // server-side NebulaHostPubkeys.HasKeys/GetNebulaPubkey handling). + if !fips140.Enabled() { + x25519PublicKeyPEM, x25519PrivateKeyPEM, ed25519PublicKey, ed25519PrivateKey, err := newKeys25519() + if err != nil { + return nil, err + } + + ed25519PublicKeyI, err := NewPublicKey(ed25519PublicKey) + if err != nil { + return nil, err + } + + ed25519PrivateKeyI, err := NewPrivateKey(ed25519PrivateKey) + if err != nil { + return nil, err + } + + k.NebulaX25519PublicKeyPEM = x25519PublicKeyPEM + k.NebulaX25519PrivateKeyPEM = x25519PrivateKeyPEM + k.HostEd25519PublicKey = ed25519PublicKeyI + k.HostEd25519PrivateKey = ed25519PrivateKeyI + } + + return k, nil } // PublicKey is a wrapper around public keys. From 79262f3cba1b48c169e6d365ececb0d14bfa7a4e Mon Sep 17 00:00:00 2001 From: JackDoan Date: Thu, 2 Jul 2026 19:50:15 -0500 Subject: [PATCH 2/2] keys: test FIPS-aware key generation and run it in CI Add TestNewFIPSAware covering both modes: without FIPS both key sets are generated, and under GODEBUG=fips140=only only P256 keys are produced and New() does not error (the regression that previously broke enrollment for FIPS builds). CI now runs the keys package a second time under GOFIPS140=v1.0.0 GODEBUG=fips140=only to exercise the FIPS branch. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Sk4ZZA8bcjVi6FJ3k2opFB --- .github/workflows/test.yml | 6 +++++ keys/keys_test.go | 49 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) create mode 100644 keys/keys_test.go diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9436b52..b619a5a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -44,6 +44,12 @@ jobs: env: TEST_FLAGS: -race + # Exercises the FIPS branch of keys.New() (P256-only key generation). + # In fips140=only mode crypto/ecdh rejects X25519, so this also guards + # against a regression that would break enrollment for FIPS builds. + - name: Run FIPS key-generation tests + run: GOFIPS140=v1.0.0 GODEBUG=fips140=only go test ./keys/... + - name: Report failures to Slack if: ${{ always() && github.ref == 'refs/heads/main' }} uses: ravsamhq/notify-slack-action@be814b201e233b2dc673608aa46e5447c8ab13f2 # v2 diff --git a/keys/keys_test.go b/keys/keys_test.go new file mode 100644 index 0000000..2fd04d6 --- /dev/null +++ b/keys/keys_test.go @@ -0,0 +1,49 @@ +package keys + +import ( + "crypto/fips140" + "testing" +) + +// TestNewFIPSAware verifies that New() adapts its key generation to FIPS mode. +// +// The FIPS-approved P256 keys are always generated. The non-FIPS-approved +// X25519/Ed25519 keys must only be generated when FIPS is not enabled - under +// GODEBUG=fips140=only, crypto/ecdh refuses to generate an X25519 key at all, +// so generating them would make New() (and therefore enrollment) fail. +// +// This test adapts to the mode it runs under. CI exercises both branches by +// running the package normally (non-FIPS) and again with +// GOFIPS140=v1.0.0 GODEBUG=fips140=only. +func TestNewFIPSAware(t *testing.T) { + k, err := New() + if err != nil { + t.Fatalf("New() failed (fips140.Enabled=%v): %v", fips140.Enabled(), err) + } + + // P256 keys are always expected, in either mode. + if len(k.NebulaP256PublicKeyPEM) == 0 || len(k.NebulaP256PrivateKeyPEM) == 0 { + t.Error("expected P256 Nebula (ECDH) keys to be generated") + } + if k.HostP256PublicKey == nil || k.HostP256PrivateKey == nil { + t.Error("expected P256 host (ECDSA) keys to be generated") + } + + if fips140.Enabled() { + // Under FIPS the 25519 keys must be omitted entirely. + if k.NebulaX25519PublicKeyPEM != nil || k.NebulaX25519PrivateKeyPEM != nil { + t.Error("expected X25519 Nebula keys to be omitted under FIPS") + } + if k.HostEd25519PublicKey != nil || k.HostEd25519PrivateKey != nil { + t.Error("expected Ed25519 host keys to be omitted under FIPS") + } + } else { + // Without FIPS, both key sets are generated (unchanged behavior). + if len(k.NebulaX25519PublicKeyPEM) == 0 || len(k.NebulaX25519PrivateKeyPEM) == 0 { + t.Error("expected X25519 Nebula keys to be generated when not in FIPS mode") + } + if k.HostEd25519PublicKey == nil || k.HostEd25519PrivateKey == nil { + t.Error("expected Ed25519 host keys to be generated when not in FIPS mode") + } + } +}