TUN-8855: Update PQ curve preferences

## Summary Nowadays, Cloudflared only supports X25519Kyber768Draft00 (0x6399,25497) but older versions may use different preferences. For FIPS compliance we are required to use P256Kyber768Draft00 (0xfe32,65074) which is supported in our internal fork of [Go-Boring-1.22.10](https://bitbucket.cfdata.org/projects/PLAT/repos/goboring/browse?at=refs/heads/go-boring/1.22.10 "Follow link"). In the near future, Go will support by default the X25519MLKEM768 (0x11ec,4588) given this we may drop the usage of our public fork of GO. To summarise: * Cloudflared FIPS: QUIC_CURVE_PREFERENCES=65074 * Cloudflared non-FIPS: QUIC_CURVE_PREFERENCES=4588 Closes TUN-8855
cloudflare · Jan 30, 2025 · 31a870b · 31a870b
1 parent bfdb0c7
commit 31a870b
Show file tree

Hide file tree

Showing 11 changed files with 150 additions and 41 deletions.
diff --git a/Makefile b/Makefile
@@ -133,11 +133,9 @@ clean:
 cloudflared:
 ifeq ($(FIPS), true)
 	$(info Building cloudflared with go-fips)
-	cp -f fips/fips.go.linux-amd64 cmd/cloudflared/fips.go
 endif
 	GOOS=$(TARGET_OS) GOARCH=$(TARGET_ARCH) $(ARM_COMMAND) go build -mod=vendor $(GO_BUILD_TAGS) $(LDFLAGS) $(IMPORT_PATH)/cmd/cloudflared
 ifeq ($(FIPS), true)
-	rm -f cmd/cloudflared/fips.go
 	./check-fips.sh cloudflared
 endif
 

diff --git a/cmd/cloudflared/tunnel/cmd.go b/cmd/cloudflared/tunnel/cmd.go
@@ -31,6 +31,7 @@ import (
 	"github.com/cloudflare/cloudflared/credentials"
 	"github.com/cloudflare/cloudflared/diagnostic"
 	"github.com/cloudflare/cloudflared/edgediscovery"
+	"github.com/cloudflare/cloudflared/fips"
 	"github.com/cloudflare/cloudflared/ingress"
 	"github.com/cloudflare/cloudflared/logger"
 	"github.com/cloudflare/cloudflared/management"
@@ -925,7 +926,7 @@ func tunnelFlags(shouldHide bool) []cli.Flag {
 			Usage:   "When given creates an experimental post-quantum secure tunnel",
 			Aliases: []string{"pq"},
 			EnvVars: []string{"TUNNEL_POST_QUANTUM"},
-			Hidden:  FipsEnabled,
+			Hidden:  fips.IsFipsEnabled(),
 		}),
 		altsrc.NewBoolFlag(&cli.BoolFlag{
 			Name:    "management-diagnostics",

diff --git a/cmd/cloudflared/tunnel/configuration.go b/cmd/cloudflared/tunnel/configuration.go
@@ -23,6 +23,7 @@ import (
 	"github.com/cloudflare/cloudflared/edgediscovery"
 	"github.com/cloudflare/cloudflared/edgediscovery/allregions"
 	"github.com/cloudflare/cloudflared/features"
+	"github.com/cloudflare/cloudflared/fips"
 	"github.com/cloudflare/cloudflared/ingress"
 	"github.com/cloudflare/cloudflared/orchestration"
 	"github.com/cloudflare/cloudflared/supervisor"
@@ -124,7 +125,7 @@ func prepareTunnelConfig(
 
 	transportProtocol := c.String("protocol")
 
-	if c.Bool("post-quantum") && FipsEnabled {
+	if c.Bool("post-quantum") && fips.IsFipsEnabled() {
 		return nil, nil, fmt.Errorf("post-quantum not supported in FIPS mode")
 	}
 
@@ -140,11 +141,6 @@ func prepareTunnelConfig(
 			return nil, nil, fmt.Errorf("post-quantum is only supported with the quic transport")
 		}
 		transportProtocol = connection.QUIC.String()
-
-		log.Info().Msgf(
-			"Using hybrid post-quantum key agreement %s",
-			supervisor.PQKexName,
-		)
 	}
 
 	namedTunnel.Client = pogs.ClientInfo{

diff --git a/cmd/cloudflared/tunnel/fips.go b/cmd/cloudflared/tunnel/fips.go
diff --git a/cmd/cloudflared/tunnel/subcommands.go b/cmd/cloudflared/tunnel/subcommands.go
@@ -29,6 +29,7 @@ import (
 	"github.com/cloudflare/cloudflared/config"
 	"github.com/cloudflare/cloudflared/connection"
 	"github.com/cloudflare/cloudflared/diagnostic"
+	"github.com/cloudflare/cloudflared/fips"
 	"github.com/cloudflare/cloudflared/metrics"
 )
 
@@ -148,7 +149,7 @@ var (
 		Usage:   "When given creates an experimental post-quantum secure tunnel",
 		Aliases: []string{"pq"},
 		EnvVars: []string{"TUNNEL_POST_QUANTUM"},
-		Hidden:  FipsEnabled,
+		Hidden:  fips.IsFipsEnabled(),
 	})
 	sortInfoByFlag = &cli.StringFlag{
 		Name:    "sort-by",

diff --git a/fips/fips.go b/fips/fips.go
@@ -0,0 +1,11 @@
+//go:build fips
+
+package fips
+
+import (
+	_ "crypto/tls/fipsonly"
+)
+
+func IsFipsEnabled() bool {
+	return true
+}
diff --git a/fips/fips.go.linux-amd64 b/fips/fips.go.linux-amd64
diff --git a/fips/nofips.go b/fips/nofips.go
@@ -0,0 +1,7 @@
+//go:build !fips
+
+package fips
+
+func IsFipsEnabled() bool {
+	return false
+}
diff --git a/supervisor/pqtunnels.go b/supervisor/pqtunnels.go
@@ -7,30 +7,53 @@ import (
 	"github.com/cloudflare/cloudflared/features"
 )
 
-// When experimental post-quantum tunnels are enabled, and we're hitting an
-// issue creating the tunnel, we'll report the first error
-// to https://pqtunnels.cloudflareresearch.com.
-
 const (
-	PQKex     = tls.CurveID(0x6399) // X25519Kyber768Draft00
-	PQKexName = "X25519Kyber768Draft00"
+	X25519Kyber768Draft00PQKex     = tls.CurveID(0x6399) // X25519Kyber768Draft00
+	X25519Kyber768Draft00PQKexName = "X25519Kyber768Draft00"
+	P256Kyber768Draft00PQKex       = tls.CurveID(0xfe32) // P256Kyber768Draft00
+	P256Kyber768Draft00PQKexName   = "P256Kyber768Draft00"
+	X25519MLKEM768PQKex            = tls.CurveID(0x11ec) // X25519MLKEM768
+	X25519MLKEM768PQKexName        = "X25519MLKEM768"
+)
+
+var (
+	nonFipsPostQuantumStrictPKex []tls.CurveID = []tls.CurveID{X25519MLKEM768PQKex, X25519Kyber768Draft00PQKex}
+	nonFipsPostQuantumPreferPKex []tls.CurveID = []tls.CurveID{X25519MLKEM768PQKex, X25519Kyber768Draft00PQKex}
+	fipsPostQuantumStrictPKex    []tls.CurveID = []tls.CurveID{P256Kyber768Draft00PQKex}
+	fipsPostQuantumPreferPKex    []tls.CurveID = []tls.CurveID{P256Kyber768Draft00PQKex, tls.CurveP256}
 )
 
-func curvePreference(pqMode features.PostQuantumMode, currentCurve []tls.CurveID) ([]tls.CurveID, error) {
+func removeDuplicates(curves []tls.CurveID) []tls.CurveID {
+	bucket := make(map[tls.CurveID]bool)
+	var result []tls.CurveID
+	for _, curve := range curves {
+		if _, ok := bucket[curve]; !ok {
+			bucket[curve] = true
+			result = append(result, curve)
+		}
+	}
+	return result
+}
+
+func curvePreference(pqMode features.PostQuantumMode, fipsEnabled bool, currentCurve []tls.CurveID) ([]tls.CurveID, error) {
 	switch pqMode {
 	case features.PostQuantumStrict:
 		// If the user passes the -post-quantum flag, we override
 		// CurvePreferences to only support hybrid post-quantum key agreements.
-		return []tls.CurveID{PQKex}, nil
-	case features.PostQuantumPrefer:
-		if len(currentCurve) == 0 {
-			return []tls.CurveID{PQKex}, nil
+		if fipsEnabled {
+			return fipsPostQuantumStrictPKex, nil
 		}
-
-		if currentCurve[0] != PQKex {
-			return append([]tls.CurveID{PQKex}, currentCurve...), nil
+		return nonFipsPostQuantumStrictPKex, nil
+	case features.PostQuantumPrefer:
+		if fipsEnabled {
+			// Ensure that all curves returned are FIPS compliant.
+			// Moreover the first curves are post-quantum and then the
+			// non post-quantum.
+			return fipsPostQuantumPreferPKex, nil
 		}
-		return currentCurve, nil
+		curves := append(nonFipsPostQuantumPreferPKex, currentCurve...)
+		curves = removeDuplicates(curves)
+		return curves, nil
 	default:
 		return nil, fmt.Errorf("Unexpected post quantum mode")
 	}

diff --git a/supervisor/pqtunnels_test.go b/supervisor/pqtunnels_test.go
@@ -0,0 +1,84 @@
+package supervisor
+
+import (
+	"crypto/tls"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/cloudflare/cloudflared/features"
+)
+
+func TestCurvePreferences(t *testing.T) {
+	// This tests if the correct curves are returned
+	// given a PostQuantumMode and a FIPS enabled bool
+	t.Parallel()
+
+	tests := []struct {
+		name           string
+		currentCurves  []tls.CurveID
+		expectedCurves []tls.CurveID
+		pqMode         features.PostQuantumMode
+		fipsEnabled    bool
+	}{
+		{
+			name:           "FIPS with Prefer PQ",
+			pqMode:         features.PostQuantumPrefer,
+			fipsEnabled:    true,
+			currentCurves:  []tls.CurveID{tls.CurveP384},
+			expectedCurves: []tls.CurveID{P256Kyber768Draft00PQKex, tls.CurveP256},
+		},
+		{
+			name:           "FIPS with Strict PQ",
+			pqMode:         features.PostQuantumStrict,
+			fipsEnabled:    true,
+			currentCurves:  []tls.CurveID{tls.CurveP256, tls.CurveP384},
+			expectedCurves: []tls.CurveID{P256Kyber768Draft00PQKex},
+		},
+		{
+			name:           "FIPS with Prefer PQ - no duplicates",
+			pqMode:         features.PostQuantumPrefer,
+			fipsEnabled:    true,
+			currentCurves:  []tls.CurveID{tls.CurveP256},
+			expectedCurves: []tls.CurveID{P256Kyber768Draft00PQKex, tls.CurveP256},
+		},
+		{
+			name:           "Non FIPS with Prefer PQ",
+			pqMode:         features.PostQuantumPrefer,
+			fipsEnabled:    false,
+			currentCurves:  []tls.CurveID{tls.CurveP256},
+			expectedCurves: []tls.CurveID{X25519MLKEM768PQKex, X25519Kyber768Draft00PQKex, tls.CurveP256},
+		},
+		{
+			name:           "Non FIPS with Prefer PQ - no duplicates",
+			pqMode:         features.PostQuantumPrefer,
+			fipsEnabled:    false,
+			currentCurves:  []tls.CurveID{X25519Kyber768Draft00PQKex, tls.CurveP256},
+			expectedCurves: []tls.CurveID{X25519MLKEM768PQKex, X25519Kyber768Draft00PQKex, tls.CurveP256},
+		},
+		{
+			name:           "Non FIPS with Prefer PQ - correct preference order",
+			pqMode:         features.PostQuantumPrefer,
+			fipsEnabled:    false,
+			currentCurves:  []tls.CurveID{tls.CurveP256, X25519Kyber768Draft00PQKex},
+			expectedCurves: []tls.CurveID{X25519MLKEM768PQKex, X25519Kyber768Draft00PQKex, tls.CurveP256},
+		},
+		{
+			name:           "Non FIPS with Strict PQ",
+			pqMode:         features.PostQuantumStrict,
+			fipsEnabled:    false,
+			currentCurves:  []tls.CurveID{tls.CurveP256},
+			expectedCurves: []tls.CurveID{X25519MLKEM768PQKex, X25519Kyber768Draft00PQKex},
+		},
+	}
+
+	for _, tcase := range tests {
+		t.Run(tcase.name, func(t *testing.T) {
+			t.Parallel()
+			curves, err := curvePreference(tcase.pqMode, tcase.fipsEnabled, tcase.currentCurves)
+			require.NoError(t, err)
+			assert.Equal(t, tcase.expectedCurves, curves)
+		})
+	}
+}
diff --git a/supervisor/tunnel.go b/supervisor/tunnel.go
@@ -20,6 +20,7 @@ import (
 	"github.com/cloudflare/cloudflared/edgediscovery"
 	"github.com/cloudflare/cloudflared/edgediscovery/allregions"
 	"github.com/cloudflare/cloudflared/features"
+	"github.com/cloudflare/cloudflared/fips"
 	"github.com/cloudflare/cloudflared/ingress"
 	"github.com/cloudflare/cloudflared/management"
 	"github.com/cloudflare/cloudflared/orchestration"
@@ -555,11 +556,13 @@ func (e *EdgeTunnelServer) serveQUIC(
 	tlsConfig := e.config.EdgeTLSConfigs[connection.QUIC]
 
 	pqMode := e.config.FeatureSelector.PostQuantumMode()
-	curvePref, err := curvePreference(pqMode, tlsConfig.CurvePreferences)
+	curvePref, err := curvePreference(pqMode, fips.IsFipsEnabled(), tlsConfig.CurvePreferences)
 	if err != nil {
 		return err, true
 	}
 
+	connLogger.Logger().Info().Msgf("Using %v as curve preferences", curvePref)
+
 	tlsConfig.CurvePreferences = curvePref
 
 	// quic-go 0.44 increases the initial packet size to 1280 by default. That breaks anyone running tunnel through WARP