cloudflare / cloudflare/cloudflared

TestSupportedCurvesNegotiation fails with standard Go on Linux

Open
#1,700 0 comments 0 reactions 0 assignees View on GitHub
Priority: Normal Type: Bug
Dominant language
Go
Stars
15.6k
Forks
1.4k
PR merge metrics
No merged PRs in 30d

Description

# Summary

`crypto/TestSupportedCurvesNegotiation` assumes that every Linux Go toolchain advertises Cloudflare's private `P256Kyber768Draft00` curve. That curve is implemented by Cloudflare's Go fork, not by the standard Go TLS stack. Standard `crypto/tls` filters the unknown private curve ID from the ClientHello, so the test fails and makes `make test` fail for the standard Go development setup documented in the README.

# Reproduction

At commit `2206516c3b693357d101b9a4b7a1e39c11cc265c`:

```bash
go version
# go version go1.26.4 linux/arm64

go test -mod=vendor -race -count=1 \
-run TestSupportedCurvesNegotiation -v ./crypto
```

Actual result:

```text
=== RUN TestSupportedCurvesNegotiation
curves_test.go:121:
Error: Should be true
--- FAIL: TestSupportedCurvesNegotiation (0.02s)
FAIL github.com/cloudflare/cloudflared/crypto
```

The same failure reproduces with standard Go 1.26.5. Running the full `make test` suite fails for the same reason.

# Expected behavior

The test should pass with both supported toolchain variants:

- Cloudflare's Go fork advertises `X25519MLKEM768`, `P256Kyber768Draft00`, and `P-256`.
- Standard Go filters the unsupported private ID and advertises `X25519MLKEM768` and `P-256`.

In both cases the test should continue to verify the exact advertised sequence, including preference order, and reject any other missing or added curve.

# Environment

- cloudflared commit: `2206516c3b693357d101b9a4b7a1e39c11cc265c`
- OS: Ubuntu 24.04.4 LTS
- Architecture: arm64
- Reproduced with: standard Go 1.26.4 and 1.26.5

# Root cause

The test uses `runtime.GOOS == "linux"` as a proxy for TLS capability. `P256Kyber768Draft00` support is a property of Cloudflare's Go fork, not of the operating system. An earlier repository change (`d0a6318334b5cb0421b17c4478defac6356f4533`, TUN-9161) likewise distinguished the non-FIPS/standard curve behavior.

# Suggested fix

Build the expected standard-Go sequence as `[X25519MLKEM768, P-256]`. If the captured ClientHello contains `P256Kyber768Draft00`, compare it with the full configured sequence instead. This preserves exact-order validation on both TLS implementations without changing production curve configuration.

# Verification of the attached fix

- Focused test passes on standard Go 1.26.4 and 1.26.5.
- Full `make test` passes on Go 1.26.4 (`go vet` plus race-enabled tests).
- `make fmt-check` passes.
- golangci-lint 2.12.2 reports `0 issues`.

Fix commit prepared locally: `824ac288a0faca138a81968dddda2c6471eb85e2` on branch `fix/standard-go-curve-negotiation-test`.

Contributor guide

Open the contributing guide

Research direction

Start with crypto/curves_test.go and run the focused TestSupportedCurvesNegotiation command from the issue on standard Go. Compare the advertised curve sequence for standard Go with the Cloudflare fork, preserving exact order; done means the focused test and the reported full-suite checks pass on both toolchain variants.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
cryptography, testing-qa
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
50/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.