From 31a870b29198ddde9d681f42cc90ab6b772d3d47 Mon Sep 17 00:00:00 2001 From: Luis Neto Date: Thu, 30 Jan 2025 05:02:47 -0800 Subject: [PATCH] TUN-8855: Update PQ curve preferences MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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 --- Makefile | 2 - cmd/cloudflared/tunnel/cmd.go | 3 +- cmd/cloudflared/tunnel/configuration.go | 8 +-- cmd/cloudflared/tunnel/fips.go | 3 - cmd/cloudflared/tunnel/subcommands.go | 3 +- fips/fips.go | 11 ++++ fips/fips.go.linux-amd64 | 12 ---- fips/nofips.go | 7 +++ supervisor/pqtunnels.go | 53 +++++++++++----- supervisor/pqtunnels_test.go | 84 +++++++++++++++++++++++++ supervisor/tunnel.go | 5 +- 11 files changed, 150 insertions(+), 41 deletions(-) delete mode 100644 cmd/cloudflared/tunnel/fips.go create mode 100644 fips/fips.go delete mode 100644 fips/fips.go.linux-amd64 create mode 100644 fips/nofips.go create mode 100644 supervisor/pqtunnels_test.go diff --git a/Makefile b/Makefile index 70063f3c..bfdf2ff9 100644 --- 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 index 3c95cc34..9c80ffe8 100644 --- 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 index 353eb1be..46a2bc6f 100644 --- 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 deleted file mode 100644 index 03ae6d26..00000000 --- a/cmd/cloudflared/tunnel/fips.go +++ /dev/null @@ -1,3 +0,0 @@ -package tunnel - -var FipsEnabled bool diff --git a/cmd/cloudflared/tunnel/subcommands.go b/cmd/cloudflared/tunnel/subcommands.go index 7042b6df..cfbffcc9 100644 --- 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 new file mode 100644 index 00000000..cd5d1617 --- /dev/null +++ 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 deleted file mode 100644 index 5075f298..00000000 --- a/fips/fips.go.linux-amd64 +++ /dev/null @@ -1,12 +0,0 @@ -// +build fips - -package main - -import ( - _ "crypto/tls/fipsonly" - "github.com/cloudflare/cloudflared/cmd/cloudflared/tunnel" -) - -func init () { - tunnel.FipsEnabled = true -} diff --git a/fips/nofips.go b/fips/nofips.go new file mode 100644 index 00000000..c8d98ed2 --- /dev/null +++ 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 index 70a3fd69..2eaad9e8 100644 --- 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" ) -func curvePreference(pqMode features.PostQuantumMode, currentCurve []tls.CurveID) ([]tls.CurveID, error) { +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 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 + if fipsEnabled { + return fipsPostQuantumStrictPKex, nil + } + return nonFipsPostQuantumStrictPKex, nil case features.PostQuantumPrefer: - if len(currentCurve) == 0 { - return []tls.CurveID{PQKex}, nil + 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 } - - if currentCurve[0] != PQKex { - return append([]tls.CurveID{PQKex}, currentCurve...), 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 new file mode 100644 index 00000000..383200db --- /dev/null +++ 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 index 6807b56d..4f848fd8 100644 --- 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