dtls icon indicating copy to clipboard operation
dtls copied to clipboard

DTLS 1.3 configuration and architecture

Open theodorsm opened this issue 3 months ago • 5 comments

Description

I have made a WIP DTLS 1.3 configuration and architecture.

As suggested by @JoeTurki, 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! @JoeTurki, @daenney, @Sean-Der.

Reference issue

Fixes #731

theodorsm avatar Oct 05 '25 15:10 theodorsm

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?

JoTurk avatar Oct 08 '25 21:10 JoTurk

@JoeTurki 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.

theodorsm avatar Oct 08 '25 22:10 theodorsm

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 :(

JoTurk avatar Oct 08 '25 22:10 JoTurk

Codecov Report

:x: Patch coverage is 40.38462% with 155 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 79.07%. Comparing base (7b68bd9) to head (2dc8c37). :warning: Report is 7 commits behind head on master.

Files with missing lines Patch % Lines
conn.go 45.09% 78 Missing and 6 partials :warning:
flight_13.go 9.52% 38 Missing :warning:
handshaker_13.go 52.72% 26 Missing :warning:
flighthandler_13.go 0.00% 4 Missing :warning:
config.go 50.00% 2 Missing and 1 partial :warning:

:x: Your patch check has failed because the patch coverage (40.38%) 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     #738      +/-   ##
==========================================
- Coverage   79.13%   79.07%   -0.07%     
==========================================
  Files         102      104       +2     
  Lines        6917     5855    -1062     
==========================================
- Hits         5474     4630     -844     
+ Misses       1068      837     -231     
- Partials      375      388      +13     
Flag Coverage Δ
go 79.07% <40.38%> (-0.09%) :arrow_down:
wasm ?

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Oct 16 '25 19:10 codecov[bot]

This PR is ready for review. 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 avatar Nov 17 '25 23:11 theodorsm