Skip to content

Commit f674b82

Browse files
committed
TUN-10413: Centralize TLS curve configuration in crypto/ and adopt X25519MLKEM768 for QUIC/H2
Introduce a new crypto/ package as the single source of truth for TLS curve preferences used on every edge-facing connection, and adopt X25519MLKEM768 as the primary post-quantum key exchange for both QUIC and HTTP/2: PQ Prefer (default): X25519MLKEM768, P256Kyber768Draft00, CurveP256 PQ Strict (--post-quantum): X25519MLKEM768, P256Kyber768Draft00 The curve list is identical under FIPS and non-FIPS builds, so crypto.GetCurvePreferences takes only a features.PostQuantumMode and returns a fresh slice on every call. HTTP/2 now applies these curve preferences the same way QUIC does. The previous PostQuantumStrict rejection in serveHTTP2 and the forced QUIC-only selection in NewProtocolSelector are removed since both transports support the same post-quantum curves; the needPQ parameter is dropped from NewProtocolSelector accordingly. Also fix a shared tls.Config race: both the QUIC and HTTP/2 paths now Clone() the per-protocol entry from TunnelConfig.EdgeTLSConfigs before mutating CurvePreferences instead of writing through the shared map entry. Legacy Kyber draft curve X25519Kyber768Draft00 and the unused removeDuplicates helper are removed along with the old supervisor/pqtunnels.go / _test.go files. AGENTS.md is updated with guidance on the new crypto/ package, the cfdcrypto import alias, the tls.Config cloning rule, and the lint workflow implications of .golangci.yaml's whole-files: true setting.
1 parent ae3799a commit f674b82

12 files changed

Lines changed: 326 additions & 235 deletions

File tree

AGENTS.md

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,16 @@ make vet
6060
cd component-tests && python -m pytest test_file.py::test_function_name
6161
```
6262

63+
Notes on linting:
64+
65+
- `.golangci.yaml` is configured with `new-from-rev` and `whole-files: true`.
66+
Touching a file triggers linting of the ENTIRE file, not just the changed
67+
hunks. Expect to fix pre-existing issues in files you modify, or add
68+
targeted `// nolint: <linter>` comments with a short justification.
69+
- Prefer `defer func() { _ = resource.Close() }()` over `defer resource.Close()`
70+
for `io.Closer` values whose error truly does not matter — this satisfies
71+
`errcheck` without hiding real failures elsewhere.
72+
6373
## Project Knowledge
6474

6575
### Package Structure
@@ -68,6 +78,24 @@ cd component-tests && python -m pytest test_file.py::test_function_name
6878
- Package names should be lowercase, single words when possible
6979
- Avoid generic names like `util`, `common`, `helper`
7080

81+
#### Well-known shared packages
82+
83+
- `crypto/`: Single source of truth for TLS curve preferences and other
84+
cryptographic primitives shared by every edge-facing transport. Import as
85+
`cfdcrypto "github.com/cloudflare/cloudflared/crypto"` to avoid colliding
86+
with the standard library's `crypto` package. Do NOT duplicate TLS curve
87+
or cipher selection logic in other packages.
88+
- `tlsconfig/`: Builds the base `*tls.Config` used for edge connections
89+
(`CreateTunnelConfig`) and loads origin/CA pools. Curve selection is
90+
intentionally NOT set here; it is applied per-connection from the
91+
`crypto/` package so the same config can be cloned and reused across
92+
protocols.
93+
- `features/`: Runtime feature flags including `PostQuantumMode`
94+
(`PostQuantumPrefer` = default, `PostQuantumStrict` = `--post-quantum`).
95+
- `fips/`: Build-tag driven FIPS detection. Only `fips.IsFipsEnabled()` is
96+
exposed; never branch on `fipsEnabled` inside a function if the two
97+
branches return the same value.
98+
7199
### Function and Method Guidelines
72100

73101
```go
@@ -171,6 +199,30 @@ type TunnelProperties struct {
171199
- Use channels for goroutine communication
172200
- Protect shared state with mutexes
173201
- Prefer `sync.RWMutex` for read-heavy workloads
202+
- `*tls.Config` values stored in shared maps (e.g.
203+
`TunnelConfig.EdgeTLSConfigs`) must be `Clone()`d before mutating
204+
per-connection fields like `CurvePreferences` or `NextProtos`. Writing
205+
through the shared pointer races with concurrent connection attempts.
206+
207+
### TLS & Post-Quantum key exchange
208+
209+
- Per-connection TLS configuration for edge connections is built via
210+
`cfdcrypto.TLSConfigWithCurvePreferences(tlsConfig, pqMode)`. It clones
211+
the provided `*tls.Config` and sets `CurvePreferences` based on `pqMode`,
212+
so callers never need to clone or mutate `CurvePreferences` themselves.
213+
Do NOT reach for the package-private `getCurvePreferences` helper; the
214+
exported `TLSConfigWithCurvePreferences` is the only supported entry
215+
point.
216+
- Two PQ modes are supported and apply identically to QUIC and HTTP/2:
217+
- `PostQuantumPrefer` (default): `[X25519MLKEM768, P256Kyber768Draft00, CurveP256]`
218+
- `PostQuantumStrict` (`--post-quantum`): `[X25519MLKEM768, P256Kyber768Draft00]`
219+
- FIPS and non-FIPS builds use the same curve list. Do NOT reintroduce a
220+
`fipsEnabled` branch in curve-selection code; if the two modes ever
221+
diverge, express the divergence inside `crypto/` so call sites remain
222+
untouched.
223+
- HTTP/2 supports post-quantum handshakes. Never re-add a
224+
`PostQuantumStrict`-based rejection to H2 code paths, and never force
225+
`--post-quantum` to select QUIC-only in protocol selection.
174226

175227
### Configuration
176228

client/config.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,3 +72,7 @@ func (c ConnectionOptionsSnapshot) ConnectionOptions() *pogs.ConnectionOptions {
7272
func (c ConnectionOptionsSnapshot) LogFields(event *zerolog.Event) *zerolog.Event {
7373
return event.Strs("features", c.client.Features)
7474
}
75+
76+
func (c *Config) ConnectionFeaturesSnapshot() features.FeatureSnapshot {
77+
return c.featureSelector.Snapshot()
78+
}

cmd/cloudflared/tunnel/configuration.go

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -140,23 +140,13 @@ func prepareTunnelConfig(
140140
}
141141
tags = append(tags, pogs.Tag{Name: "ID", Value: clientConfig.ConnectorID.String()})
142142

143-
clientFeatures := featureSelector.Snapshot()
144-
pqMode := clientFeatures.PostQuantum
145-
if pqMode == features.PostQuantumStrict {
146-
// Error if the user tries to force a non-quic transport protocol
147-
if transportProtocol != connection.AutoSelectFlag && transportProtocol != connection.QUIC.String() {
148-
return nil, nil, fmt.Errorf("post-quantum is only supported with the quic transport")
149-
}
150-
transportProtocol = connection.QUIC.String()
151-
}
152-
153143
cfg := config.GetConfiguration()
154144
ingressRules, err := ingress.ParseIngressFromConfigAndCLI(cfg, c, log)
155145
if err != nil {
156146
return nil, nil, err
157147
}
158148

159-
protocolSelector, err := connection.NewProtocolSelector(transportProtocol, namedTunnel.Credentials.AccountTag, c.IsSet(TunnelTokenFlag), isPostQuantumEnforced, edgediscovery.ProtocolPercentage, connection.ResolveTTL, log)
149+
protocolSelector, err := connection.NewProtocolSelector(transportProtocol, namedTunnel.Credentials.AccountTag, c.IsSet(TunnelTokenFlag), edgediscovery.ProtocolPercentage, connection.ResolveTTL, log)
160150
if err != nil {
161151
return nil, nil, err
162152
}

connection/protocol.go

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -224,18 +224,10 @@ func NewProtocolSelector(
224224
protocolFlag string,
225225
accountTag string,
226226
tunnelTokenProvided bool,
227-
needPQ bool,
228227
protocolFetcher edgediscovery.PercentageFetcher,
229228
resolveTTL time.Duration,
230229
log *zerolog.Logger,
231230
) (ProtocolSelector, error) {
232-
// With --post-quantum, we force quic
233-
if needPQ {
234-
return &staticProtocolSelector{
235-
current: QUIC,
236-
}, nil
237-
}
238-
239231
threshold := switchThreshold(accountTag)
240232
fetchedProtocol, err := getProtocol(ProtocolList, protocolFetcher, threshold)
241233
log.Debug().Msgf("Fetched protocol: %s", fetchedProtocol)

connection/protocol_test.go

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ func TestNewProtocolSelector(t *testing.T) {
3131
name string
3232
protocol string
3333
tunnelTokenProvided bool
34-
needPQ bool
3534
expectedProtocol Protocol
3635
hasFallback bool
3736
expectedFallback Protocol
@@ -59,18 +58,6 @@ func TestNewProtocolSelector(t *testing.T) {
5958
hasFallback: true,
6059
expectedFallback: HTTP2,
6160
},
62-
{
63-
name: "named tunnel (post quantum)",
64-
protocol: AutoSelectFlag,
65-
needPQ: true,
66-
expectedProtocol: QUIC,
67-
},
68-
{
69-
name: "named tunnel (post quantum) w/http2",
70-
protocol: "http2",
71-
needPQ: true,
72-
expectedProtocol: QUIC,
73-
},
7461
}
7562

7663
fetcher := dynamicMockFetcher{
@@ -79,7 +66,7 @@ func TestNewProtocolSelector(t *testing.T) {
7966

8067
for _, test := range tests {
8168
t.Run(test.name, func(t *testing.T) {
82-
selector, err := NewProtocolSelector(test.protocol, testAccountTag, test.tunnelTokenProvided, test.needPQ, fetcher.fetch(), ResolveTTL, &log)
69+
selector, err := NewProtocolSelector(test.protocol, testAccountTag, test.tunnelTokenProvided, fetcher.fetch(), ResolveTTL, &log)
8370
if test.wantErr {
8471
assert.Error(t, err, "test %s failed", test.name)
8572
} else {
@@ -97,7 +84,7 @@ func TestNewProtocolSelector(t *testing.T) {
9784

9885
func TestAutoProtocolSelectorRefresh(t *testing.T) {
9986
fetcher := dynamicMockFetcher{}
100-
selector, err := NewProtocolSelector(AutoSelectFlag, testAccountTag, false, false, fetcher.fetch(), testNoTTL, &log)
87+
selector, err := NewProtocolSelector(AutoSelectFlag, testAccountTag, false, fetcher.fetch(), testNoTTL, &log)
10188
require.NoError(t, err)
10289
assert.Equal(t, QUIC, selector.Current())
10390

@@ -127,7 +114,7 @@ func TestAutoProtocolSelectorRefresh(t *testing.T) {
127114
func TestHTTP2ProtocolSelectorRefresh(t *testing.T) {
128115
fetcher := dynamicMockFetcher{}
129116
// Since the user chooses http2 on purpose, we always stick to it.
130-
selector, err := NewProtocolSelector(HTTP2.String(), testAccountTag, false, false, fetcher.fetch(), testNoTTL, &log)
117+
selector, err := NewProtocolSelector(HTTP2.String(), testAccountTag, false, fetcher.fetch(), testNoTTL, &log)
131118
require.NoError(t, err)
132119
assert.Equal(t, HTTP2, selector.Current())
133120

@@ -156,7 +143,7 @@ func TestHTTP2ProtocolSelectorRefresh(t *testing.T) {
156143

157144
func TestAutoProtocolSelectorNoRefreshWithToken(t *testing.T) {
158145
fetcher := dynamicMockFetcher{}
159-
selector, err := NewProtocolSelector(AutoSelectFlag, testAccountTag, true, false, fetcher.fetch(), testNoTTL, &log)
146+
selector, err := NewProtocolSelector(AutoSelectFlag, testAccountTag, true, fetcher.fetch(), testNoTTL, &log)
160147
require.NoError(t, err)
161148
assert.Equal(t, QUIC, selector.Current())
162149

crypto/curves.go

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
package crypto
2+
3+
import (
4+
"crypto/tls"
5+
"errors"
6+
"fmt"
7+
"slices"
8+
9+
"github.com/cloudflare/cloudflared/features"
10+
)
11+
12+
// errUnknownPostQuantumMode is returned by GetCurvePreferences when the
13+
// caller passes a features.PostQuantumMode value that is not one of the
14+
// documented constants. It is intentionally unexported: callers should treat
15+
// any non-nil error as a programming mistake rather than inspecting it.
16+
var errUnknownPostQuantumMode = errors.New("the provided post quantum mode is unknown")
17+
18+
// P256Kyber768Draft00 is a post-quantum KEM based on Kyber768.
19+
const P256Kyber768Draft00 = tls.CurveID(0xfe32) // ID 65074
20+
21+
// Canonical curve lists returned by GetCurvePreferences. They are kept
22+
// package-private so that callers cannot accidentally mutate the shared
23+
// slice; GetCurvePreferences always returns a clone.
24+
var (
25+
// postQuantumStrictCurves is used when the caller requires a
26+
// post-quantum handshake. Only PQ curves (X25519MLKEM768 and the
27+
// deprecated P256Kyber768Draft00 for backward compatibility) are
28+
// advertised; no classical-only curve is included.
29+
postQuantumStrictCurves = []tls.CurveID{tls.X25519MLKEM768, P256Kyber768Draft00}
30+
// postQuantumPreferCurves is used for the default "prefer" mode: the PQ
31+
// curve is advertised first and the classical CurveP256 is listed as a
32+
// fallback so peers without PQ support can still negotiate.
33+
postQuantumPreferCurves = []tls.CurveID{tls.X25519MLKEM768, P256Kyber768Draft00, tls.CurveP256}
34+
)
35+
36+
// getCurvePreferences returns the TLS curve preferences that should be
37+
// applied to edge-facing connections for the given post-quantum mode.
38+
//
39+
// The returned slice is the canonical, protocol-agnostic curve list and is
40+
// suitable for direct assignment to tls.Config.CurvePreferences. A fresh
41+
// slice is returned on every call, so callers may mutate it freely without
42+
// affecting other callers.
43+
//
44+
// An error is returned only when profile is not a recognised
45+
// features.PostQuantumMode value, which indicates a programming bug in the
46+
// caller.
47+
func getCurvePreferences(profile features.PostQuantumMode) ([]tls.CurveID, error) {
48+
switch profile {
49+
case features.PostQuantumPrefer:
50+
return slices.Clone(postQuantumPreferCurves), nil
51+
case features.PostQuantumStrict:
52+
return slices.Clone(postQuantumStrictCurves), nil
53+
}
54+
55+
return nil, errUnknownPostQuantumMode
56+
}
57+
58+
// TLSConfigWithCurvePreferences clones the provided tls.Config and applies
59+
// curve preferences based on the given post-quantum mode.
60+
//
61+
// The original tls.Config is never modified; a clone is returned so that
62+
// callers can safely use the same base configuration across multiple
63+
// goroutines without racing on CurvePreferences.
64+
//
65+
// Returns an error only when pqMode is not a recognised
66+
// features.PostQuantumMode value.
67+
func TLSConfigWithCurvePreferences(tlsConfig *tls.Config, pqMode features.PostQuantumMode) (*tls.Config, error) {
68+
// Clone the TLS config before applying per-connection curve
69+
// preferences. The TlsConfig may be shared across goroutines;
70+
// mutating it directly would race with concurrent connection attempts.
71+
config := tlsConfig.Clone()
72+
curvePref, err := getCurvePreferences(pqMode)
73+
if err != nil {
74+
return nil, fmt.Errorf("get curve preferences: %w", err)
75+
}
76+
77+
config.CurvePreferences = curvePref
78+
return config, nil
79+
}

crypto/curves_test.go

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
package crypto
2+
3+
import (
4+
"crypto/tls"
5+
"net/http"
6+
"net/http/httptest"
7+
"runtime"
8+
"slices"
9+
"testing"
10+
11+
"github.com/stretchr/testify/require"
12+
13+
"github.com/cloudflare/cloudflared/features"
14+
)
15+
16+
// TestCurvePreferences verifies that GetCurvePreferences returns the
17+
// documented curve list for each supported PostQuantumMode. The expected
18+
// values correspond to the contract described in the package documentation
19+
// and must be identical under FIPS and non-FIPS builds (see TUN-10413).
20+
func TestCurvePreferences(t *testing.T) {
21+
t.Parallel()
22+
23+
tests := []struct {
24+
name string
25+
expectedCurves []tls.CurveID
26+
pqMode features.PostQuantumMode
27+
}{
28+
{
29+
name: "Prefer PQ",
30+
pqMode: features.PostQuantumPrefer,
31+
expectedCurves: []tls.CurveID{tls.X25519MLKEM768, P256Kyber768Draft00, tls.CurveP256},
32+
},
33+
{
34+
name: "Strict PQ",
35+
pqMode: features.PostQuantumStrict,
36+
expectedCurves: []tls.CurveID{tls.X25519MLKEM768, P256Kyber768Draft00},
37+
},
38+
}
39+
40+
for _, tcase := range tests {
41+
t.Run(tcase.name, func(t *testing.T) {
42+
t.Parallel()
43+
curves, err := getCurvePreferences(tcase.pqMode)
44+
require.NoError(t, err)
45+
require.Equal(t, tcase.expectedCurves, curves)
46+
})
47+
}
48+
}
49+
50+
// TestCurvePreferenceUnknownMode asserts that passing a PostQuantumMode
51+
// value outside of the documented constants produces an error instead of
52+
// silently returning a nil or default curve list. This protects callers
53+
// from accidentally negotiating with an unintended curve set.
54+
func TestCurvePreferenceUnknownMode(t *testing.T) {
55+
t.Parallel()
56+
57+
_, err := getCurvePreferences(features.PostQuantumMode(255))
58+
require.Error(t, err)
59+
}
60+
61+
// TestReturnedSliceIsIndependent ensures GetCurvePreferences returns a
62+
// fresh slice on every call, so that callers cannot corrupt the
63+
// package-level defaults by mutating the result.
64+
func TestReturnedSliceIsIndependent(t *testing.T) {
65+
t.Parallel()
66+
67+
first, err := getCurvePreferences(features.PostQuantumPrefer)
68+
require.NoError(t, err)
69+
// Mutate the returned slice.
70+
first[0] = tls.CurveP521
71+
72+
second, err := getCurvePreferences(features.PostQuantumPrefer)
73+
require.NoError(t, err)
74+
require.Equal(t, tls.X25519MLKEM768, second[0], "package defaults must not be affected by caller mutation")
75+
}
76+
77+
// runClientServerHandshake drives a TLS 1.3 handshake with the given curve
78+
// preferences set on the client and captures the SupportedCurves list
79+
// advertised by the client in its ClientHello. The helper is used by
80+
// TestSupportedCurvesNegotiation to exercise the curves end-to-end against
81+
// the standard library's TLS stack.
82+
func runClientServerHandshake(t *testing.T, curves []tls.CurveID) []tls.CurveID {
83+
var advertisedCurves []tls.CurveID
84+
ts := httptest.NewUnstartedServer(nil)
85+
ts.TLS = &tls.Config{ // nolint: gosec
86+
GetConfigForClient: func(chi *tls.ClientHelloInfo) (*tls.Config, error) {
87+
advertisedCurves = slices.Clone(chi.SupportedCurves)
88+
return nil, nil
89+
},
90+
}
91+
ts.StartTLS()
92+
defer ts.Close()
93+
clientTLSConfig := ts.Client().Transport.(*http.Transport).TLSClientConfig
94+
clientTLSConfig.CurvePreferences = curves
95+
resp, err := ts.Client().Head(ts.URL)
96+
if err != nil {
97+
t.Error(err)
98+
return nil
99+
}
100+
defer func() { _ = resp.Body.Close() }()
101+
return advertisedCurves
102+
}
103+
104+
// TestSupportedCurvesNegotiation verifies that the curves returned by
105+
// GetCurvePreferences survive a real TLS handshake unchanged, i.e. the
106+
// standard library advertises exactly the curves we expect. Currently only
107+
// PostQuantumPrefer is exercised because PostQuantumStrict would cause the
108+
// handshake to fail against httptest servers that do not support
109+
// X25519MLKEM768 server-side.
110+
func TestSupportedCurvesNegotiation(t *testing.T) {
111+
t.Parallel()
112+
for _, tcase := range []features.PostQuantumMode{features.PostQuantumPrefer} {
113+
curves, err := getCurvePreferences(tcase)
114+
require.NoError(t, err)
115+
advertisedCurves := runClientServerHandshake(t, curves)
116+
require.True(t, slices.Contains(advertisedCurves, tls.CurveP256))
117+
require.True(t, slices.Contains(advertisedCurves, tls.X25519MLKEM768))
118+
expectedLength := 2
119+
if runtime.GOOS == "linux" {
120+
// P256Kyber768Draft00 only exists in linux
121+
require.True(t, slices.Contains(advertisedCurves, P256Kyber768Draft00))
122+
expectedLength = 3
123+
}
124+
require.Len(t, advertisedCurves, expectedLength)
125+
}
126+
}

0 commit comments

Comments
 (0)