diff --git a/AGENTS.md b/AGENTS.md index df52b703..67f6163c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -60,6 +60,16 @@ make vet cd component-tests && python -m pytest test_file.py::test_function_name ``` +Notes on linting: + +- `.golangci.yaml` is configured with `new-from-rev` and `whole-files: true`. + Touching a file triggers linting of the ENTIRE file, not just the changed + hunks. Expect to fix pre-existing issues in files you modify, or add + targeted `// nolint: ` comments with a short justification. +- Prefer `defer func() { _ = resource.Close() }()` over `defer resource.Close()` + for `io.Closer` values whose error truly does not matter — this satisfies + `errcheck` without hiding real failures elsewhere. + ## Project Knowledge ### Package Structure @@ -68,6 +78,24 @@ cd component-tests && python -m pytest test_file.py::test_function_name - Package names should be lowercase, single words when possible - Avoid generic names like `util`, `common`, `helper` +#### Well-known shared packages + +- `crypto/`: Single source of truth for TLS curve preferences and other + cryptographic primitives shared by every edge-facing transport. Import as + `cfdcrypto "github.com/cloudflare/cloudflared/crypto"` to avoid colliding + with the standard library's `crypto` package. Do NOT duplicate TLS curve + or cipher selection logic in other packages. +- `tlsconfig/`: Builds the base `*tls.Config` used for edge connections + (`CreateTunnelConfig`) and loads origin/CA pools. Curve selection is + intentionally NOT set here; it is applied per-connection from the + `crypto/` package so the same config can be cloned and reused across + protocols. +- `features/`: Runtime feature flags including `PostQuantumMode` + (`PostQuantumPrefer` = default, `PostQuantumStrict` = `--post-quantum`). +- `fips/`: Build-tag driven FIPS detection. Only `fips.IsFipsEnabled()` is + exposed; never branch on `fipsEnabled` inside a function if the two + branches return the same value. + ### Function and Method Guidelines ```go @@ -171,6 +199,30 @@ type TunnelProperties struct { - Use channels for goroutine communication - Protect shared state with mutexes - Prefer `sync.RWMutex` for read-heavy workloads +- `*tls.Config` values stored in shared maps (e.g. + `TunnelConfig.EdgeTLSConfigs`) must be `Clone()`d before mutating + per-connection fields like `CurvePreferences` or `NextProtos`. Writing + through the shared pointer races with concurrent connection attempts. + +### TLS & Post-Quantum key exchange + +- Per-connection TLS configuration for edge connections is built via + `cfdcrypto.TLSConfigWithCurvePreferences(tlsConfig, pqMode)`. It clones + the provided `*tls.Config` and sets `CurvePreferences` based on `pqMode`, + so callers never need to clone or mutate `CurvePreferences` themselves. + Do NOT reach for the package-private `getCurvePreferences` helper; the + exported `TLSConfigWithCurvePreferences` is the only supported entry + point. +- Two PQ modes are supported and apply identically to QUIC and HTTP/2: + - `PostQuantumPrefer` (default): `[X25519MLKEM768, P256Kyber768Draft00, CurveP256]` + - `PostQuantumStrict` (`--post-quantum`): `[X25519MLKEM768, P256Kyber768Draft00]` +- FIPS and non-FIPS builds use the same curve list. Do NOT reintroduce a + `fipsEnabled` branch in curve-selection code; if the two modes ever + diverge, express the divergence inside `crypto/` so call sites remain + untouched. +- HTTP/2 supports post-quantum handshakes. Never re-add a + `PostQuantumStrict`-based rejection to H2 code paths, and never force + `--post-quantum` to select QUIC-only in protocol selection. ### Configuration diff --git a/client/config.go b/client/config.go index b4e053e7..4c6650a2 100644 --- a/client/config.go +++ b/client/config.go @@ -72,3 +72,7 @@ func (c ConnectionOptionsSnapshot) ConnectionOptions() *pogs.ConnectionOptions { func (c ConnectionOptionsSnapshot) LogFields(event *zerolog.Event) *zerolog.Event { return event.Strs("features", c.client.Features) } + +func (c *Config) ConnectionFeaturesSnapshot() features.FeatureSnapshot { + return c.featureSelector.Snapshot() +} diff --git a/cmd/cloudflared/tunnel/configuration.go b/cmd/cloudflared/tunnel/configuration.go index 225da347..d9d7ce05 100644 --- a/cmd/cloudflared/tunnel/configuration.go +++ b/cmd/cloudflared/tunnel/configuration.go @@ -140,23 +140,13 @@ func prepareTunnelConfig( } tags = append(tags, pogs.Tag{Name: "ID", Value: clientConfig.ConnectorID.String()}) - clientFeatures := featureSelector.Snapshot() - pqMode := clientFeatures.PostQuantum - if pqMode == features.PostQuantumStrict { - // Error if the user tries to force a non-quic transport protocol - if transportProtocol != connection.AutoSelectFlag && transportProtocol != connection.QUIC.String() { - return nil, nil, fmt.Errorf("post-quantum is only supported with the quic transport") - } - transportProtocol = connection.QUIC.String() - } - cfg := config.GetConfiguration() ingressRules, err := ingress.ParseIngressFromConfigAndCLI(cfg, c, log) if err != nil { return nil, nil, err } - protocolSelector, err := connection.NewProtocolSelector(transportProtocol, namedTunnel.Credentials.AccountTag, c.IsSet(TunnelTokenFlag), isPostQuantumEnforced, edgediscovery.ProtocolPercentage, connection.ResolveTTL, log) + protocolSelector, err := connection.NewProtocolSelector(transportProtocol, namedTunnel.Credentials.AccountTag, c.IsSet(TunnelTokenFlag), edgediscovery.ProtocolPercentage, connection.ResolveTTL, log) if err != nil { return nil, nil, err } diff --git a/connection/protocol.go b/connection/protocol.go index c3070675..72e9b765 100644 --- a/connection/protocol.go +++ b/connection/protocol.go @@ -224,18 +224,10 @@ func NewProtocolSelector( protocolFlag string, accountTag string, tunnelTokenProvided bool, - needPQ bool, protocolFetcher edgediscovery.PercentageFetcher, resolveTTL time.Duration, log *zerolog.Logger, ) (ProtocolSelector, error) { - // With --post-quantum, we force quic - if needPQ { - return &staticProtocolSelector{ - current: QUIC, - }, nil - } - threshold := switchThreshold(accountTag) fetchedProtocol, err := getProtocol(ProtocolList, protocolFetcher, threshold) log.Debug().Msgf("Fetched protocol: %s", fetchedProtocol) diff --git a/connection/protocol_test.go b/connection/protocol_test.go index 9d897ae0..ee0a18b1 100644 --- a/connection/protocol_test.go +++ b/connection/protocol_test.go @@ -31,7 +31,6 @@ func TestNewProtocolSelector(t *testing.T) { name string protocol string tunnelTokenProvided bool - needPQ bool expectedProtocol Protocol hasFallback bool expectedFallback Protocol @@ -59,18 +58,6 @@ func TestNewProtocolSelector(t *testing.T) { hasFallback: true, expectedFallback: HTTP2, }, - { - name: "named tunnel (post quantum)", - protocol: AutoSelectFlag, - needPQ: true, - expectedProtocol: QUIC, - }, - { - name: "named tunnel (post quantum) w/http2", - protocol: "http2", - needPQ: true, - expectedProtocol: QUIC, - }, } fetcher := dynamicMockFetcher{ @@ -79,7 +66,7 @@ func TestNewProtocolSelector(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - selector, err := NewProtocolSelector(test.protocol, testAccountTag, test.tunnelTokenProvided, test.needPQ, fetcher.fetch(), ResolveTTL, &log) + selector, err := NewProtocolSelector(test.protocol, testAccountTag, test.tunnelTokenProvided, fetcher.fetch(), ResolveTTL, &log) if test.wantErr { assert.Error(t, err, "test %s failed", test.name) } else { @@ -97,7 +84,7 @@ func TestNewProtocolSelector(t *testing.T) { func TestAutoProtocolSelectorRefresh(t *testing.T) { fetcher := dynamicMockFetcher{} - selector, err := NewProtocolSelector(AutoSelectFlag, testAccountTag, false, false, fetcher.fetch(), testNoTTL, &log) + selector, err := NewProtocolSelector(AutoSelectFlag, testAccountTag, false, fetcher.fetch(), testNoTTL, &log) require.NoError(t, err) assert.Equal(t, QUIC, selector.Current()) @@ -127,7 +114,7 @@ func TestAutoProtocolSelectorRefresh(t *testing.T) { func TestHTTP2ProtocolSelectorRefresh(t *testing.T) { fetcher := dynamicMockFetcher{} // Since the user chooses http2 on purpose, we always stick to it. - selector, err := NewProtocolSelector(HTTP2.String(), testAccountTag, false, false, fetcher.fetch(), testNoTTL, &log) + selector, err := NewProtocolSelector(HTTP2.String(), testAccountTag, false, fetcher.fetch(), testNoTTL, &log) require.NoError(t, err) assert.Equal(t, HTTP2, selector.Current()) @@ -156,7 +143,7 @@ func TestHTTP2ProtocolSelectorRefresh(t *testing.T) { func TestAutoProtocolSelectorNoRefreshWithToken(t *testing.T) { fetcher := dynamicMockFetcher{} - selector, err := NewProtocolSelector(AutoSelectFlag, testAccountTag, true, false, fetcher.fetch(), testNoTTL, &log) + selector, err := NewProtocolSelector(AutoSelectFlag, testAccountTag, true, fetcher.fetch(), testNoTTL, &log) require.NoError(t, err) assert.Equal(t, QUIC, selector.Current()) diff --git a/crypto/curves.go b/crypto/curves.go new file mode 100644 index 00000000..f816c36f --- /dev/null +++ b/crypto/curves.go @@ -0,0 +1,79 @@ +package crypto + +import ( + "crypto/tls" + "errors" + "fmt" + "slices" + + "github.com/cloudflare/cloudflared/features" +) + +// errUnknownPostQuantumMode is returned by GetCurvePreferences when the +// caller passes a features.PostQuantumMode value that is not one of the +// documented constants. It is intentionally unexported: callers should treat +// any non-nil error as a programming mistake rather than inspecting it. +var errUnknownPostQuantumMode = errors.New("the provided post quantum mode is unknown") + +// P256Kyber768Draft00 is a post-quantum KEM based on Kyber768. +const P256Kyber768Draft00 = tls.CurveID(0xfe32) // ID 65074 + +// Canonical curve lists returned by GetCurvePreferences. They are kept +// package-private so that callers cannot accidentally mutate the shared +// slice; GetCurvePreferences always returns a clone. +var ( + // postQuantumStrictCurves is used when the caller requires a + // post-quantum handshake. Only PQ curves (X25519MLKEM768 and the + // deprecated P256Kyber768Draft00 for backward compatibility) are + // advertised; no classical-only curve is included. + postQuantumStrictCurves = []tls.CurveID{tls.X25519MLKEM768, P256Kyber768Draft00} + // postQuantumPreferCurves is used for the default "prefer" mode: the PQ + // curve is advertised first and the classical CurveP256 is listed as a + // fallback so peers without PQ support can still negotiate. + postQuantumPreferCurves = []tls.CurveID{tls.X25519MLKEM768, P256Kyber768Draft00, tls.CurveP256} +) + +// getCurvePreferences returns the TLS curve preferences that should be +// applied to edge-facing connections for the given post-quantum mode. +// +// The returned slice is the canonical, protocol-agnostic curve list and is +// suitable for direct assignment to tls.Config.CurvePreferences. A fresh +// slice is returned on every call, so callers may mutate it freely without +// affecting other callers. +// +// An error is returned only when profile is not a recognised +// features.PostQuantumMode value, which indicates a programming bug in the +// caller. +func getCurvePreferences(profile features.PostQuantumMode) ([]tls.CurveID, error) { + switch profile { + case features.PostQuantumPrefer: + return slices.Clone(postQuantumPreferCurves), nil + case features.PostQuantumStrict: + return slices.Clone(postQuantumStrictCurves), nil + } + + return nil, errUnknownPostQuantumMode +} + +// TLSConfigWithCurvePreferences clones the provided tls.Config and applies +// curve preferences based on the given post-quantum mode. +// +// The original tls.Config is never modified; a clone is returned so that +// callers can safely use the same base configuration across multiple +// goroutines without racing on CurvePreferences. +// +// Returns an error only when pqMode is not a recognised +// features.PostQuantumMode value. +func TLSConfigWithCurvePreferences(tlsConfig *tls.Config, pqMode features.PostQuantumMode) (*tls.Config, error) { + // Clone the TLS config before applying per-connection curve + // preferences. The TlsConfig may be shared across goroutines; + // mutating it directly would race with concurrent connection attempts. + config := tlsConfig.Clone() + curvePref, err := getCurvePreferences(pqMode) + if err != nil { + return nil, fmt.Errorf("get curve preferences: %w", err) + } + + config.CurvePreferences = curvePref + return config, nil +} diff --git a/crypto/curves_test.go b/crypto/curves_test.go new file mode 100644 index 00000000..dab49f22 --- /dev/null +++ b/crypto/curves_test.go @@ -0,0 +1,126 @@ +package crypto + +import ( + "crypto/tls" + "net/http" + "net/http/httptest" + "runtime" + "slices" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/cloudflare/cloudflared/features" +) + +// TestCurvePreferences verifies that GetCurvePreferences returns the +// documented curve list for each supported PostQuantumMode. The expected +// values correspond to the contract described in the package documentation +// and must be identical under FIPS and non-FIPS builds (see TUN-10413). +func TestCurvePreferences(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + expectedCurves []tls.CurveID + pqMode features.PostQuantumMode + }{ + { + name: "Prefer PQ", + pqMode: features.PostQuantumPrefer, + expectedCurves: []tls.CurveID{tls.X25519MLKEM768, P256Kyber768Draft00, tls.CurveP256}, + }, + { + name: "Strict PQ", + pqMode: features.PostQuantumStrict, + expectedCurves: []tls.CurveID{tls.X25519MLKEM768, P256Kyber768Draft00}, + }, + } + + for _, tcase := range tests { + t.Run(tcase.name, func(t *testing.T) { + t.Parallel() + curves, err := getCurvePreferences(tcase.pqMode) + require.NoError(t, err) + require.Equal(t, tcase.expectedCurves, curves) + }) + } +} + +// TestCurvePreferenceUnknownMode asserts that passing a PostQuantumMode +// value outside of the documented constants produces an error instead of +// silently returning a nil or default curve list. This protects callers +// from accidentally negotiating with an unintended curve set. +func TestCurvePreferenceUnknownMode(t *testing.T) { + t.Parallel() + + _, err := getCurvePreferences(features.PostQuantumMode(255)) + require.Error(t, err) +} + +// TestReturnedSliceIsIndependent ensures GetCurvePreferences returns a +// fresh slice on every call, so that callers cannot corrupt the +// package-level defaults by mutating the result. +func TestReturnedSliceIsIndependent(t *testing.T) { + t.Parallel() + + first, err := getCurvePreferences(features.PostQuantumPrefer) + require.NoError(t, err) + // Mutate the returned slice. + first[0] = tls.CurveP521 + + second, err := getCurvePreferences(features.PostQuantumPrefer) + require.NoError(t, err) + require.Equal(t, tls.X25519MLKEM768, second[0], "package defaults must not be affected by caller mutation") +} + +// runClientServerHandshake drives a TLS 1.3 handshake with the given curve +// preferences set on the client and captures the SupportedCurves list +// advertised by the client in its ClientHello. The helper is used by +// TestSupportedCurvesNegotiation to exercise the curves end-to-end against +// the standard library's TLS stack. +func runClientServerHandshake(t *testing.T, curves []tls.CurveID) []tls.CurveID { + var advertisedCurves []tls.CurveID + ts := httptest.NewUnstartedServer(nil) + ts.TLS = &tls.Config{ // nolint: gosec + GetConfigForClient: func(chi *tls.ClientHelloInfo) (*tls.Config, error) { + advertisedCurves = slices.Clone(chi.SupportedCurves) + return nil, nil + }, + } + ts.StartTLS() + defer ts.Close() + clientTLSConfig := ts.Client().Transport.(*http.Transport).TLSClientConfig + clientTLSConfig.CurvePreferences = curves + resp, err := ts.Client().Head(ts.URL) + if err != nil { + t.Error(err) + return nil + } + defer func() { _ = resp.Body.Close() }() + return advertisedCurves +} + +// TestSupportedCurvesNegotiation verifies that the curves returned by +// GetCurvePreferences survive a real TLS handshake unchanged, i.e. the +// standard library advertises exactly the curves we expect. Currently only +// PostQuantumPrefer is exercised because PostQuantumStrict would cause the +// handshake to fail against httptest servers that do not support +// X25519MLKEM768 server-side. +func TestSupportedCurvesNegotiation(t *testing.T) { + t.Parallel() + for _, tcase := range []features.PostQuantumMode{features.PostQuantumPrefer} { + curves, err := getCurvePreferences(tcase) + require.NoError(t, err) + advertisedCurves := runClientServerHandshake(t, curves) + require.True(t, slices.Contains(advertisedCurves, tls.CurveP256)) + require.True(t, slices.Contains(advertisedCurves, tls.X25519MLKEM768)) + expectedLength := 2 + if runtime.GOOS == "linux" { + // P256Kyber768Draft00 only exists in linux + require.True(t, slices.Contains(advertisedCurves, P256Kyber768Draft00)) + expectedLength = 3 + } + require.Len(t, advertisedCurves, expectedLength) + } +} diff --git a/crypto/doc.go b/crypto/doc.go new file mode 100644 index 00000000..eb969d9d --- /dev/null +++ b/crypto/doc.go @@ -0,0 +1,35 @@ +// Package crypto centralizes the cryptographic primitives and TLS +// configuration used by cloudflared when establishing connections to the +// Cloudflare edge. +// +// The primary responsibility of the package is to expose a single, canonical +// source of TLS curve preferences so that every edge-facing transport (QUIC +// and HTTP/2) negotiates the same key-exchange algorithms regardless of the +// code path that sets up the connection. +// +// # Post-Quantum key exchange +// +// cloudflared supports the X25519MLKEM768 hybrid post-quantum key exchange. +// Two operating modes are exposed via the features.PostQuantumMode flag: +// +// - PostQuantumPrefer: advertise X25519MLKEM768 and the deprecated +// P256Kyber768Draft00 first, then fall back to the classical CurveP256 +// if the peer does not support either PQ curve. This is the default +// used for every outbound edge connection. +// - PostQuantumStrict: advertise only the PQ curves (X25519MLKEM768 and +// P256Kyber768Draft00). Activated by the user via the --post-quantum +// CLI flag. No classical fallback is offered, so a peer that does not +// support any PQ curve will fail the handshake. +// +// The resulting curve lists are identical under FIPS and non-FIPS builds, +// which is why GetCurvePreferences does not take a FIPS toggle. If that +// property ever changes (for example, if a curve stops being FIPS-approved), +// the divergence should be expressed inside this package so callers remain +// unchanged. +// +// # Thread-safety +// +// GetCurvePreferences returns a fresh slice on every call. Callers are free +// to mutate the returned slice without affecting the package-level defaults +// or other callers. +package crypto diff --git a/supervisor/pqtunnels.go b/supervisor/pqtunnels.go deleted file mode 100644 index 30eb2e87..00000000 --- a/supervisor/pqtunnels.go +++ /dev/null @@ -1,60 +0,0 @@ -package supervisor - -import ( - "crypto/tls" - "fmt" - - "github.com/cloudflare/cloudflared/features" -) - -const ( - 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} - nonFipsPostQuantumPreferPKex []tls.CurveID = []tls.CurveID{X25519MLKEM768PQKex} - 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. - if fipsEnabled { - return fipsPostQuantumStrictPKex, 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 - } - 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 deleted file mode 100644 index 3be54460..00000000 --- a/supervisor/pqtunnels_test.go +++ /dev/null @@ -1,119 +0,0 @@ -package supervisor - -import ( - "crypto/tls" - "net/http" - "net/http/httptest" - "slices" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/cloudflare/cloudflared/features" - "github.com/cloudflare/cloudflared/fips" -) - -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, 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, tls.CurveP256, X25519Kyber768Draft00PQKex}, - }, - { - name: "Non FIPS with Strict PQ", - pqMode: features.PostQuantumStrict, - fipsEnabled: false, - currentCurves: []tls.CurveID{tls.CurveP256}, - expectedCurves: []tls.CurveID{X25519MLKEM768PQKex}, - }, - } - - 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) - }) - } -} - -func runClientServerHandshake(t *testing.T, curves []tls.CurveID) []tls.CurveID { - var advertisedCurves []tls.CurveID - ts := httptest.NewUnstartedServer(nil) - ts.TLS = &tls.Config{ // nolint: gosec - GetConfigForClient: func(chi *tls.ClientHelloInfo) (*tls.Config, error) { - advertisedCurves = slices.Clone(chi.SupportedCurves) - return nil, nil - }, - } - ts.StartTLS() - defer ts.Close() - clientTlsConfig := ts.Client().Transport.(*http.Transport).TLSClientConfig - clientTlsConfig.CurvePreferences = curves - resp, err := ts.Client().Head(ts.URL) - if err != nil { - t.Error(err) - return nil - } - defer resp.Body.Close() - return advertisedCurves -} - -func TestSupportedCurvesNegotiation(t *testing.T) { - for _, tcase := range []features.PostQuantumMode{features.PostQuantumPrefer} { - curves, err := curvePreference(tcase, fips.IsFipsEnabled(), make([]tls.CurveID, 0)) - require.NoError(t, err) - advertisedCurves := runClientServerHandshake(t, curves) - assert.Equal(t, curves, advertisedCurves) - } -} diff --git a/supervisor/tunnel.go b/supervisor/tunnel.go index 3641b19c..fc2f0c46 100644 --- a/supervisor/tunnel.go +++ b/supervisor/tunnel.go @@ -20,6 +20,7 @@ import ( "github.com/cloudflare/cloudflared/client" "github.com/cloudflare/cloudflared/connection" "github.com/cloudflare/cloudflared/connection/dialopts" + cfdcrypto "github.com/cloudflare/cloudflared/crypto" "github.com/cloudflare/cloudflared/edgediscovery" "github.com/cloudflare/cloudflared/edgediscovery/allregions" "github.com/cloudflare/cloudflared/features" @@ -90,6 +91,10 @@ func (c *TunnelConfig) connectionOptions(originLocalAddr string, previousAttempt return c.ClientConfig.ConnectionOptionsSnapshot(originIP, previousAttempts) } +func (c *TunnelConfig) connectionFeatures() features.FeatureSnapshot { + return c.ClientConfig.ConnectionFeaturesSnapshot() +} + func StartTunnelDaemon( ctx context.Context, config *TunnelConfig, @@ -473,12 +478,21 @@ func (e *EdgeTunnelServer) serveConnection( connIndex) case connection.HTTP2: - edgeConn, err := edgediscovery.DialEdge(ctx, dialTimeout, e.config.EdgeTLSConfigs[protocol], addr.TCP, e.edgeBindAddr) + tlsConfig, err := cfdcrypto.TLSConfigWithCurvePreferences(e.config.EdgeTLSConfigs[protocol], e.config.connectionFeatures().PostQuantum) + if err != nil { + return fmt.Errorf("could not create TLS configuration: %w", err), true + } + + connLog.Logger().Info().Msgf("Tunnel connection curve preferences: %v", tlsConfig.CurvePreferences) + + edgeConn, err := edgediscovery.DialEdge(ctx, dialTimeout, tlsConfig, addr.TCP, e.edgeBindAddr) if err != nil { connLog.ConnAwareLogger().Err(err).Msg("Unable to establish connection with Cloudflare edge") return err, true } + // Rebuild the connection options with the local address now that the + // edge socket is established. // nolint: gosec connOptions := e.config.connectionOptions(edgeConn.LocalAddr().String(), uint8(backoff.Retries())) // nolint: zerologlint @@ -516,11 +530,9 @@ func (e *EdgeTunnelServer) serveHTTP2( controlStreamHandler connection.ControlStreamHandler, connIndex uint8, ) error { - pqMode := connOptions.FeatureSnapshot.PostQuantum - if pqMode == features.PostQuantumStrict { - return unrecoverableError{errors.New("HTTP/2 transport does not support post-quantum")} - } - + // HTTP/2 supports post-quantum key exchange the same way QUIC does. Curve + // preferences are applied by the caller before the TLS handshake in + // DialEdge (see TUN-10413). connLog.Logger().Debug().Msgf("Connecting via http2") h2conn := connection.NewHTTP2Connection( tlsServerConn, @@ -558,18 +570,12 @@ func (e *EdgeTunnelServer) serveQUIC( controlStreamHandler connection.ControlStreamHandler, connIndex uint8, ) (err error, recoverable bool) { - tlsConfig := e.config.EdgeTLSConfigs[connection.QUIC] - - pqMode := connOptions.FeatureSnapshot.PostQuantum - curvePref, err := curvePreference(pqMode, fips.IsFipsEnabled(), tlsConfig.CurvePreferences) + config, err := cfdcrypto.TLSConfigWithCurvePreferences(e.config.EdgeTLSConfigs[connection.QUIC], connOptions.FeatureSnapshot.PostQuantum) if err != nil { - connLogger.ConnAwareLogger().Err(err).Msgf("failed to get curve preferences") - return err, true + return fmt.Errorf("could not create TLS configuration: %w", err), true } - connLogger.Logger().Info().Msgf("Tunnel connection curve preferences: %v", curvePref) - - tlsConfig.CurvePreferences = curvePref + connLogger.Logger().Info().Msgf("Tunnel connection curve preferences: %v", config.CurvePreferences) // quic-go 0.44 increases the initial packet size to 1280 by default. That breaks anyone running tunnel through WARP // because WARP MTU is 1280. @@ -596,7 +602,7 @@ func (e *EdgeTunnelServer) serveQUIC( conn, err := connection.DialQuic( ctx, quicConfig, - tlsConfig, + config, edgeAddr, e.edgeBindAddr, connIndex, diff --git a/supervisor/tunnel_test.go b/supervisor/tunnel_test.go index b0b6cef5..65cf3366 100644 --- a/supervisor/tunnel_test.go +++ b/supervisor/tunnel_test.go @@ -7,6 +7,7 @@ import ( "github.com/quic-go/quic-go" "github.com/rs/zerolog" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/cloudflare/cloudflared/connection" "github.com/cloudflare/cloudflared/edgediscovery" @@ -43,12 +44,11 @@ func TestWaitForBackoffFallback(t *testing.T) { "auto", "", false, - false, mockFetcher.fetch(), resolveTTL, &log, ) - assert.NoError(t, err) + require.NoError(t, err) initProtocol := protocolSelector.Current() assert.Equal(t, connection.QUIC, initProtocol) @@ -106,12 +106,11 @@ func TestWaitForBackoffFallback(t *testing.T) { "quic", "", false, - false, mockFetcher.fetch(), resolveTTL, &log, ) - assert.NoError(t, err) + require.NoError(t, err) protoFallback = &protocolFallback{backoff, protocolSelector.Current(), false} for i := 0; i < int(maxRetries-1); i++ { protoFallback.BackoffTimer() // simulate retry