DTLS 1.3 FSM architecture

[theodorsm]

Description

I have made a WIP DTLS 1.3 configuration and architecture.

As suggested by @JoTurk, it implementes a functional options pattern to provide flexibility in the future. A DTLS 1.3 configuration is created with NewConfigVersion13(c Config, opts ...OptionVersion13) (*Config, error) where an option/config can be implemented like this:

func WithOption13(f func(b bool)) OptionVersion13 {
	return func(c *Config) error {
		c.comeConfig = b
		return nil
	}
}
*/

The codebase for DTLS 1.2 and DTLS 1.3 splits off in conn.go with the new handshake13 function and the new handshakeFSM in handshaker_13.go (note that neither reflects DTLS 1.3 logic yet, just a copy/paste). I suggest we split off as much code as possible suffixed with _13 to keep the development of DTLS 1.3 separate from 1.2.

This PR contains a test of enabling DTLS 1.3 and verifies that an error is returned correctly, as we have not yet started to implement DTLS 1.3 flights.

I would appreciate some input from other maintainers on this! @JoTurk, @daenney, @Sean-Der.

Reference issue

Fixes #731

[JoTurk]

I'm just concerned that if we expose a public APIs, it'll be extremely difficult to change or remove it later, just my personal take about the dtls 1.3 suffix, maybe a sub package? what do you think?

[theodorsm]

@JoTurk thanks!

I agree that we should keep the API private until the DTLS 1.3 implementation is ready; we should only make it public as the last thing to do. If that is what you meant by it?

Regarding the sub-package or suffix: It depends. Using a sub-package I am afraid to loose access to some private fields in structs or functions that can be valuable for the implementation. Using a suffix, we can move fast without those problems, but it will be a bit messy file-wise (especially with flights). Will definitely have another look to see what's possible. I think a combination might be a good solution.

[JoTurk]

I agree that we should keep the API private until the DTLS 1.3 implementation is ready; we should only make it public as the last thing to do. If that is what you meant by it?

Yeah this will be a good call, private or sub package, but private will be better, I agree , for example: if we expose NewConfigVersion13 we'll not be able to remove it in the future :(

[codecov]

Codecov Report

❌ Patch coverage is 57.81638% with 170 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.13%. Comparing base (fd27a52) to head (ff2fe74).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
conn.go	59.71%	56 Missing and 29 partials ⚠️
flight_13.go	14.28%	36 Missing ⚠️
flighthandlers_client_13.go	71.76%	14 Missing and 10 partials ⚠️
handshaker_13.go	51.51%	16 Missing ⚠️
flighthandler_13.go	0.00%	8 Missing ⚠️
util.go	93.75%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #738      +/-   ##
==========================================
- Coverage   82.44%   81.13%   -1.31%     
==========================================
  Files         121      125       +4     
  Lines        6928     7290     +362     
==========================================
+ Hits         5712     5915     +203     
- Misses        805      927     +122     
- Partials      411      448      +37     

Flag	Coverage Δ
go	81.13% <57.81%> (-1.31%)	⬇️

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]

The goal here is not to be complete, but to provide a best-effort skeleton with my current understanding of how the DTLS 1.3 implementation would look. It still contains some duplicate code in conn.go (the linter loves to complain about it), which shall be modified when we implement more of the DTLS 1.3 spec (such as the new record layer encoding #755 ).

I still think the _13 suffix is a good way of structuring to move on without large refactorings. This reduces the number of new public APIs (now private) we would need to expose when using a sub-package. I reduced the flight handlers files by grouping them into flighthandlers_client_13 and flighthandlers_server_13. We can always decide to make a sub-package later if we see it necessary.

[theodorsm]

There are some TODOs here, but they are mainly regarding the new record demultiplexing. This is addressed in #793.

This PR is not supposed to be complete, but the changes are private so we can get it merged to unblock working on the DTLS 1.3 implementation further. It will affect #766

[Copilot]

Pull request overview

This PR introduces initial scaffolding to split DTLS 1.2 and DTLS 1.3 handshake logic, adding a placeholder DTLS 1.3 FSM/flight structure and wiring a “DTLS 1.3 enabled” path in Conn.HandshakeContext.

Changes:

• Refactor DTLS 1.2 handshake FSM to handshakeFSM12 and introduce a handshakeFSM interface for selecting between 1.2 and 1.3 implementations.
• Add DTLS 1.3 WIP files (*_13.go) for flights, parsers/generators, and a placeholder FSM that currently returns unimplemented errors.
• Add a WIP test intended to validate DTLS 1.3 enablement and expected error behavior.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
state.go	Adds a protocol.Version field to connection state.
handshaker.go	Renames DTLS 1.2 FSM to handshakeFSM12, adds handshakeFSM interface, and adds onFlightState13 hook.
handshaker_13.go	New placeholder DTLS 1.3 FSM (handshakeFSM13) that returns unimplemented errors.
flight_13.go	New DTLS 1.3 flight enum and helpers (isLastSendFlight/isLastRecvFlight).
flighthandler_13.go	New DTLS 1.3 parser/generator type stubs and placeholder getters.
flighthandlers_client_13.go	Placeholder file for DTLS 1.3 client flight handlers.
flighthandlers_server_13.go	Placeholder file for DTLS 1.3 server flight handlers.
errors.go	Adds DTLS 1.3 unimplemented error values.
conn.go	Wires DTLS 1.3 path selection into HandshakeContext, stores FSM as an interface, and adds version-enabled helper.
handshaker_test.go	Updates tests to use newHandshakeFSM12.
conn_test.go	Updates an elliptic curve test for FSM interface and adds WIP DTLS 1.3 config test.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[JoTurk]

Thank you. Great work.

Sorry it took me so long, I had to refresh my memory with the specs again because it has been a while, I think i have a fresh memory of rfc9147 and 6347 now :)

[JoTurk]

I think copilot is correct, I don't think should set state.version if multiple version are enabled to version1_3, this early.

Even if we got it to work somehow now, I think we should be very explicit here, And it shouldn't be a 1.3 DTLS with 1.2 fallback logic, we should be in:

1. DTLS 1.2 only mode.
2. DTLS 1.3 only mode.
3. dual stack 1.2/1.3 with version not selected yet (if we're early in the connection).

I'm concerned because after seeing all the bugs introduced by dual stack libraries (mainly the two bugs introduced by firefox) I'm worried we'll introduce similar bugs if we don't write this very explicitly.

This dual-stack pattern should make it easier to implement the client and the server path, and have the code expected to interact with DTLS 1.2

Also it will make it easier to reasoon about

In addition to the handshake messages that are deprecated by the TLS
1.3 specification, DTLS 1.3 furthermore deprecates the
HelloVerifyRequest message originally defined in DTLS 1.0. DTLS
1.3-compliant implementations MUST NOT use the HelloVerifyRequest to
execute a return-routability check. A dual-stack DTLS 1.2 / DTLS 1.3
client MUST, however, be prepared to interact with a DTLS 1.2 server.

[theodorsm]

I have added prepareHandshakeStart to try to handle this better.

Both the DTLS 1.2 and 1.3 only mode should be quite clean, but dual-stack is a bit messier. The dual-stack mode negotiates the version without starting a FSM by sending a CH and waiting for the responding SH or waiting for a CH as a server, then pass the state/flights to the correct FSM.

I think doing this slighly messy operation up front and then having clean FSM would be a good way to reduce such bugs. We should write some tests for this behavior tho (which I haven't really done here).

[JoTurk]

We should write some tests for this behavior tho (which I haven't really done here).

Yeah and more DTLS 1.2 interops targeted at the dual-stack mode, because even if we follow the spec, we might break some incorrect DTLS 1.2 libraries.

[theodorsm]

Should we also consider a refactoring? Right now the handshake path starts the FSM together with a main read loop (and the reason for needing readAndBufferNoFSM). Perhaps we could decouple it better to make it easier to reason about when to launch the handshake and the current state. I am a little unsure how it would look, or if I am missing some reason why the current architecture is needed. I am somewhat afraid that such an effort will introduce more bugs and require a lot of time, on the other hand we already have seen some FSM bugs recently with the timers.

[JoTurk]

I think yes, I can start working on the refactor, Also i think we're in a close stage, so it's fine to start breaking APIs, I'll make a branch to lock the current state, and tag from there if we need to fix bugs or issues targeted at 1.2, what do you think?

[theodorsm]

@JoTurk, sounds like good plan, let's do it! Thanks:)

[theodorsm]

I have also added some dual-stack tests (negotiating DTLS 1.2) that are passing now.

[JoTurk]

@theodorsm sorry i missed this, this is great thank you.