[jgbernalp]

No description provided.

[Nexucis]

Thanks for adding this! The parsing and the TLS 1.3-only logic look right: isTLS13OnlyCurve matches isTLS13OnlyKeyExchange in Go 1.27, supportedCurves matches the internal list of crypto/tls, and the tests are a nice way to detect a change after a Go upgrade.

Two things to address before merging (details inline):

1. FIPS 140-3 mode: with GODEBUG=fips140=on, crypto/tls doesn't use X25519.
   • TestIsTLS13OnlyCurve/X25519 fails, while the echo tests pass with this setting on main.
   • --web.tls-curve-preferences=X25519 passes Build, but every TLS handshake then fails: the server starts but can't be used.
2. TLS 1.3-only curves while TLS 1.2 is allowed: --web.tls-curve-preferences=X25519MLKEM768 without --web.tls-min-version=1.3 is accepted without any warning, and every TLS 1.2 client then fails with remote error: tls: handshake failure.

[Nexucis]

With GODEBUG=fips140=on, crypto/tls ignores the curves that aren't FIPS-approved (with the Go FIPS 140-3 module, X25519 is currently the only one of supportedCurves). So --web.tls-curve-preferences=X25519 passes Build, but no key exchange is possible: every TLS handshake fails, and the server starts but can't be used.

Could checkCurvePreferences reject a list without any FIPS-approved curve when fips140.Enabled(), like it rejects a list without any TLS 1.2 curve when TLS 1.3 is disabled? Or at least, could the flag help mention it?

[Nexucis]

This only covers TLS 1.3 being disabled. The opposite case isn't detected: when all the curves require TLS 1.3 while TLS 1.2 is still allowed (--web.tls-min-version unset or 1.2), for example --web.tls-curve-preferences=X25519MLKEM768 alone, Build succeeds silently, and every TLS 1.2 client then fails with remote error: tls: handshake failure.

Like the warning below, could we also pass parsedMinVersion and warn in this case (with a test case)? For example:

func checkCurvePreferences(minVersion, maxVersion uint16, curves []tls.CurveID) error {
	var tls13Only []string
	for _, id := range curves {
		if isTLS13OnlyCurve(id) {
			tls13Only = append(tls13Only, id.String())
		}
	}
	if len(tls13Only) == 0 {
		return nil
	}
	onlyTLS13 := len(tls13Only) == len(curves)
	// The versions are 0 when unset: TLS 1.2 and TLS 1.3 are then both enabled.
	tls13Disabled := maxVersion != 0 && maxVersion < tls.VersionTLS13
	switch {
	case tls13Disabled && onlyTLS13:
		return fmt.Errorf(...) // unchanged
	case tls13Disabled:
		logrus.Warnf(...) // unchanged
	case onlyTLS13 && minVersion < tls.VersionTLS13:
		logrus.Warn("TLS 1.2 clients cannot connect, because all the curves of --web.tls-curve-preferences require TLS 1.3: set --web.tls-min-version to 1.3 to make it explicit")
	}
	return nil
}

[Nexucis]

This test fails with GODEBUG=fips140=on go test ./echo/ (X25519 subtest): in FIPS 140-3 mode, crypto/tls doesn't use X25519, so even the TLS 1.3 handshake fails. The echo tests pass with this setting on main.

As the regular run already checks the mapping, the test could be skipped in FIPS mode (with the crypto/fips140 import):

func TestIsTLS13OnlyCurve(t *testing.T) {
	if fips140.Enabled() {
		t.Skip("crypto/tls only uses the FIPS-approved curves in FIPS 140-3 mode")
	}

[jgbernalp]

@Nexucis comments fixed, PTAL. Also rebased with the CA changes

[Nexucis]

Thanks for the changes! The three points are fixed:

• isFIPSApprovedCurve matches allowedCurvePreferencesFIPS of crypto/tls in Go 1.27, and the note about Go+BoringCrypto is right (crypto/tls/fipsonly only forces fips140tls).
• The warning for the curves that require TLS 1.3 while TLS 1.2 is allowed works, and it uses the curves that are left in FIPS 140-3 mode.
• The echo tests pass with GODEBUG=fips140=on and GODEBUG=fips140=only (also with -race).

Two non-blocking suggestions inline.

[Nexucis]

Non-blocking: the CI runs make test without GODEBUG, so this test is always skipped there, like the fips140.Enabled() branch of build(). As isFIPSApprovedCurve copies the unexported allowedCurvePreferencesFIPS, a change of this list after a Go upgrade wouldn't be detected. For example, if Go allows X25519 in FIPS 140-3 mode one day, --web.tls-curve-preferences=X25519 would make Build fail, while crypto/tls would accept it.

Could make test also run the echo tests in FIPS 140-3 mode? It passes locally, also with -race:

test:
	@echo ">> running all tests"
	$(GO) test -race -count=1 -v $(pkgs)
	@echo ">> running the echo tests in FIPS 140-3 mode"
	GODEBUG=fips140=on $(GO) test -race -count=1 ./echo/...

Then, the comment of TestSupportedCurves could also mention TestIsFIPSApprovedCurve, next to TestIsTLS13OnlyCurve.

[Nexucis]

Nit (non-blocking): these checks also run when HTTPS isn't enabled. Without --web.tls-cert-file:

• --web.tls-curve-preferences=X25519MLKEM768 logs TLS 1.2 clients cannot connect, ..., and then that the TLS flags have no effect because HTTPS is not enabled.
• In FIPS 140-3 mode, --web.tls-curve-preferences=X25519 makes Build fail, although the flag has no effect.

Could they only run when HTTPS is enabled (cert != "")?