Use FIPS-safe default curve (P-256) instead of X25519 by samthesloth · Pull Request #817 · pion/dtls

samthesloth · 2026-03-24T19:38:35Z

Description

On FIPS 140-enabled Linux systems, DTLS handshakes fail because the initial curve is hardcoded to X25519, which Go 1.24+ rejects at runtime (crypto/ecdh: use of X25519 is not allowed in FIPS 140-only mode).

Reference issue

Fixes #816

Changes

conn.go — Changed defaultNamedCurve from X25519 to P256
config.go — Reordered defaultCurves to {P256, P384, X25519} (P-256 first, X25519 kept for non-FIPS compatibility)
flight0handler.go / flight1handler.go — Changed state.namedCurve = defaultNamedCurve to state.namedCurve = cfg.ellipticCurves[0] so the flight handlers respect the caller's configured curves instead of using a hardcoded constant

Testing

I ran into 3 issues when running golangci-lint run referring to file formatting pointing at the comments at the top of the files (authentication_type.go, key_exchange_algorithm.go, and util.go) but they were unmodified files, so I wasn't sure if I should touch them.

codecov · 2026-03-24T19:42:59Z

Codecov Report

❌ Patch coverage is 16.66667% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.49%. Comparing base (848c4bc) to head (5ed8021).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
conn.go	0.00%	5 Missing and 1 partial ⚠️
flight0handler.go	33.33%	1 Missing and 1 partial ⚠️
flight1handler.go	33.33%	1 Missing and 1 partial ⚠️

❌ Your patch status has failed because the patch coverage (16.66%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #817      +/-   ##
==========================================
- Coverage   82.64%   82.49%   -0.15%     
==========================================
  Files         121      121              
  Lines        6858     6868      +10     
==========================================
- Hits         5668     5666       -2     
- Misses        779      787       +8     
- Partials      411      415       +4

Flag	Coverage Δ
go	`82.49% <16.66%> (-0.15%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

theodorsm

Hey, Thanks for discovering this issue and fixing it so fast! Compliance to FIPS is imporant to do right and I'm sure others will be happy for this to be fixed.

I have left some comments on the changing of prefered default order.

theodorsm · 2026-03-24T21:28:31Z

 const defaultMTU = 1200 // bytes

-var defaultCurves = []elliptic.Curve{elliptic.X25519, elliptic.P256, elliptic.P384} //nolint:gochecknoglobals
+var defaultCurves = []elliptic.Curve{elliptic.P256, elliptic.P384, elliptic.X25519} //nolint:gochecknoglobals


We should not change this order, this is the standard followed by other DTLS-implementations such as NSS (firefox), boringssl (chrome). OpenSSL also seem to have X25519 prefered by default. This might affect interopt and allow the library to be fingerprintable more easily.

@theodorsm @samthesloth Okay what if we only switch if fips is enabled? https://pkg.go.dev/crypto/fips140#Enabled i think that would be a good compromise.

theodorsm · 2026-03-24T21:32:00Z

 	cookieLength          = 20
 	sessionLength         = 32
-	defaultNamedCurve     = elliptic.X25519
+	defaultNamedCurve     = elliptic.P256


I guess we could remove this const as it's not needed anymore.

@theodorsm Maybe we use a func instead that goes through the list, skipping X25519 if FIPS is enabled as @JoTurk suggested?

I agree, that sounds like the way to go!

samthesloth · 2026-03-25T14:26:16Z

@theodorsm Hello again! Sorry if I'm doing this wrong. I think I implemented a proper change, but please let me know if I need to change how I went about it or how I pushed it! Thank you!

theodorsm

@samthesloth, no worries! Thinking a bit more about it, we should move the FIPS check as early as possible. Left some comments on how we can approach this.

theodorsm · 2026-03-25T17:16:30Z

+	if fips140.Enabled() {
+		// On FIPS systems, skip non-approved curves
+		for _, c := range curves {
+			if c != elliptic.X25519 {
+				return c
+			}
+		}
+	}


Let's move this check to just after we set the curves, and drop the function:

dtls/conn.go

Lines 178 to 181 in d0e736c

curves := config.EllipticCurves

if len(curves) == 0 {

curves = defaultCurves

}

We are using cfg.ellipticCurves in other flights to set the SupportedEllipticCurves extension, ex:

dtls/flight3handler.go

Lines 297 to 301 in d0e736c

if state.namedCurve != 0 {

extensions = append(extensions, []extension.Extension{

&extension.SupportedEllipticCurves{

EllipticCurves: cfg.ellipticCurves,

},

So if we filter earlier, we avoid announcing non-FIPS curves in the extension also.

theodorsm · 2026-03-25T17:27:48Z

 	state.localEpoch.Store(zeroEpoch)
 	state.remoteEpoch.Store(zeroEpoch)
-	state.namedCurve = defaultNamedCurve
+	state.namedCurve = defaultCurve(cfg.ellipticCurves)


After moving the check in conn.go we should check for length here:

if len(cfg.ellipticCurves) < 1 { return nil,nil,errEmptyEllipticCurves } state.namedCurve = cfg.ellipticCurves[0]

theodorsm · 2026-03-25T17:28:13Z

 	state.localEpoch.Store(zeroEpoch)
 	state.remoteEpoch.Store(zeroEpoch)
-	state.namedCurve = defaultNamedCurve
+	state.namedCurve = defaultCurve(cfg.ellipticCurves)


Also check length here if we move check in conn.go.

theodorsm · 2026-03-25T19:14:43Z

@samthesloth, this looks really good now! Could you also re-try what you reported in your inital issue #816 to verify that this fixes your use case?

samthesloth · 2026-03-26T17:48:12Z

Hello! Sorry for the late reply. I was having some issues recreating everything. I tested on Rocky Linux 9 with Go 1.26.1. I confirmed that crypto/ecdh.X25519() is blocked under GODEBUG=fips140=only while P256() works, so the fix is correct. I wasn't able to do a full end-to-end test in that mode because fips140=only also blocks SHA-1, which the WebSocket protocol requires for connection handshakes, so my program panics before reaching the DTLS layer.

The exact partial-enforcement scenario from my original report (X25519 blocked, SHA-1 allowed) may have been triggered by an earlier Go build. However, the flight handler change to use cfg.ellipticCurves[0] is the right fix as you suggested since it respects the caller's config instead of hardcoding X25519. Happy to add a test if helpful.

JoTurk

thanks.

theodorsm

@samthesloth thanks! You can add a E2E test for FIPS if you want to, but I don't think it's strictly needed for this small and reasonable fix. Could be useful for later if we want to verify FIPS compliance. Can also be added later in another PR if you want to work on it:)

I see that this branch now is behind the master branch, so please rebase - then we can squash and merge.

…urve

samthesloth · 2026-03-30T14:51:36Z

@theodorsm Awesome, thank you! I'd love to! For now, I think I rebased correctly? Let me know if I need to change anything!

samthesloth mentioned this pull request Mar 24, 2026

DTLS handshake fails on FIPS-enabled Linux: defaultNamedCurve hardcoded to X25519 #816

Closed

samthesloth force-pushed the fix-fips-default-curve branch from 158daa0 to 0e14a77 Compare March 24, 2026 19:43

theodorsm requested changes Mar 24, 2026

View reviewed changes

theodorsm requested changes Mar 25, 2026

View reviewed changes

JoTurk approved these changes Mar 26, 2026

View reviewed changes

theodorsm self-requested a review March 30, 2026 14:26

theodorsm approved these changes Mar 30, 2026

View reviewed changes

samthesloth added 3 commits March 30, 2026 09:49

Use P-256 as default DTLS curve for FIPS

40c2c91

Replaced defaultNamedCurve const with func checking FIPS

950b6b9

Moved fips check to right after setting curves, removed func defaultC…

5ed8021

…urve

samthesloth force-pushed the fix-fips-default-curve branch from 10ba769 to 5ed8021 Compare March 30, 2026 14:49

JoTurk merged commit e6950f0 into pion:master Mar 30, 2026
17 of 19 checks passed

	curves := config.EllipticCurves
	if len(curves) == 0 {
	curves = defaultCurves
	}

	if state.namedCurve != 0 {
	extensions = append(extensions, []extension.Extension{
	&extension.SupportedEllipticCurves{
	EllipticCurves: cfg.ellipticCurves,
	},

Conversation

samthesloth commented Mar 24, 2026

Description

Reference issue

Changes

Testing

Uh oh!

codecov Bot commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

theodorsm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samthesloth commented Mar 25, 2026

Uh oh!

theodorsm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theodorsm commented Mar 25, 2026

Uh oh!

samthesloth commented Mar 26, 2026

Uh oh!

JoTurk left a comment

Choose a reason for hiding this comment

Uh oh!

theodorsm left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samthesloth commented Mar 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Mar 24, 2026 •

edited

Loading

theodorsm left a comment •

edited

Loading