Implement DTLS 1.2 Connection IDs #558

hasheddan · 2023-07-12T02:47:02Z

Description

Implements DTLS 1.2 Connection ID support as defined in https://datatracker.ietf.org/doc/html/rfc9146.

Remaining work:

Clean up TODO's
Update cbc and ccm suites to support Connection IDs
Expand e2e tests with dedicated Connection ID tests
Expand unit tests to cover all new functionality
Add examples for enabling Connection ID support (dependent on whether UDP connection package is implemented in this PR -- see below)
Expand e2e tests with changing remote IP addresses (dependent on whether UDP connection package is implemented in this PR -- see below)
Clean up commit history (to be completed prior to merge)

Notes for early reviewers:

Currently, if you want an endpoint to be able to update its remote address, you need to implement a custom Listener that uses connection ID's rather than remote address to determine what PacketServer to route packets to. This functionality is dependent on pion/transport/udp, and I demonstrated what an example could look like in pion/transport#252. However, as mentioned on that PR, the intention is to move the functionality to live here in pion/dtls, and I am in progress of doing so. We could feasibly make that a separate PR given that this one is "feature complete" in that it can successfully negotiate and use connection IDs as both a client and server (as demonstrated by the new e2e tests). Either way, the UDP connection package needs to be added in this PR or a subsequent one.

Another area of interest is automated testing against other libraries. The current OpenSSL tests cannot be expanded for this use-case as it does not support connection IDs. I have performed manual validation by expanding the DTLS client example in mbedTLS as demonstrated here: Mbed-TLS/mbedtls@development...hasheddan:mbedtls:feat/cid-example. I would like to expand the e2e testing suite in this repository to include mbedTLS with connection IDs enabled, though we will need to build our own client, which could also be deferred to a subsequent PR.

An area that I do not anticipate expanding scope of this PR to cover quite yet is saving connection state. For my particular use-case is is vital that we able to persist connection state across server restarts. Note that this is different from saving sessions as connection IDs must actually be renegotiated upon session resumption, whereas ideally with connection IDs enabled a server could continue receiving from an endpoint after restart by recovering all state. I would like to ensure this functionality is enabled prior to cutting a v3 release as I believe it could result in breaking changes.

Lastly, I am in progress of increasing unit test coverage across the board, though it is not anticipated that this will result in any functional changes to what is currently implemented as it is already being validated through existing unit tests (testing with connection IDs not enabled), as well as e2e test.

Testing Locally

If reviewers would like to experiment with enabling connection IDs locally, you can use the following commands to only run the CID e2e tests.

docker build -t pion-dtls-e2e -f e2e/Dockerfile .

docker run -i --rm --net=host pion-dtls-e2e go test -run=.*CID.* -v

Using the host's network namespace allows you to capture on the loopback interface in wireshark. Using the dtls filter will help eliminate other noise. Your output should look something like the following:

Reference issue

Fixes #256
Fixes #367

pkg/protocol/recordlayer/fuzz_test.go

pkg/protocol/recordlayer/recordlayer.go

Sean-Der · 2023-07-12T20:02:56Z

pkg/protocol/recordlayer/header.go

 }

 // RecordLayer enums
 const (
-	HeaderSize        = 13
+	DefaultHeaderSize = 13


Instead of Default it would be nice to hint at CID somehow.

HeaderSizePreCID might not be the right name.

@Sean-Der sounds good -- I was actually thinking of just keeping this as HeaderSize when I finish out this PR as this is a public const so any change is technically a breaking change. What do you think?

@hasheddan either works for me!

ConnectionID warrants a V3. It will be easier to get excitement/have people see the feature that way. So feel free to break the API in all the ways that make sense :)

I hope we can merge to master and tag v3.0.0-alpha.1 very soon then!

Went with FixedHeaderSize as it feels like it could age a little better since it isn't directly coupled to the implementation of connection IDs (i.e. in theory there could be another extension in the future that modified header structure).

conn.go

internal/ciphersuite/aes_ccm.go

codecov · 2023-07-18T14:49:09Z

Codecov Report

Patch coverage: 83.48% and project coverage change: +0.21% 🎉

Comparison is base (a1d270f) 76.75% compared to head (ed08ac1) 76.97%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #558      +/-   ##
==========================================
+ Coverage   76.75%   76.97%   +0.21%     
==========================================
  Files          95       99       +4     
  Lines        5675     6011     +336     
==========================================
+ Hits         4356     4627     +271     
- Misses        972     1016      +44     
- Partials      347      368      +21

Flag	Coverage Δ
go	`77.00% <83.48%> (+0.21%)`	⬆️
wasm	`61.74% <44.14%> (-5.24%)`	⬇️

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

Files Changed	Coverage Δ
cipher_suite.go	`98.51% <ø> (ø)`
config.go	`100.00% <ø> (ø)`
handshaker.go	`76.79% <ø> (ø)`
state.go	`90.90% <ø> (ø)`
pkg/protocol/extension/connection_id.go	`68.96% <68.96%> (ø)`
pkg/crypto/ciphersuite/cbc.go	`69.51% <77.96%> (+1.47%)`	⬆️
conn.go	`80.56% <78.10%> (-0.96%)`	⬇️
pkg/protocol/recordlayer/header.go	`85.36% <78.57%> (+5.36%)`	⬆️
connection_id.go	`80.00% <80.00%> (ø)`
pkg/protocol/recordlayer/inner_plaintext.go	`80.95% <80.95%> (ø)`
... and 21 more

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

hasheddan · 2023-07-19T13:38:37Z

conn.go

+// PacketServer listens for incoming DTLS connections.
+// Unlike Server, PacketServer allows for connections to change remote address.
+// The provided rAddr will be used as the initial remote address for sending.
+func PacketServer(ctx context.Context, conn net.PacketConn, rAddr net.Addr, config *Config) (*Conn, error) {


As an alternative to adding PacketServer, we could update the existing Server and Client to require passing a net.PacketConn rather than net.Conn. The reasoning for introducing PacketServer was to minimize breaking changes, but if we are cutting a v3 one could make the case that requiring the user to wrap a net.Conn before passing it to the Server or Client would make it more obvious that remote address updates would not be supported. Said another way, if we are going to have breaking changes, it may be worth doing a larger refactor.

However, I also want to balance getting this functionality out to folks sooner rather than later, and I anticipate that we may have more breaking changes when we add support for DTLS 1.3.

I'm okay going in either direction. My gut is that we stick with the non-breaking approach for this PR and then immediately follow up with an issue (or PR) to go over proposed v3 changes just so that we don't do anything premature/rash here.

Changing to net.PacketConn is something we've wanted to do for a long time, as it's confusing in many places for people to have a net.Conn in the context of UDP. It came up in #367 and we tagged it as a 3.0.0 milestone. So I'd be all in favour for changing that, as that's something we want to do one way or another.

@daenney thanks for this feedback! I'll push up an example of how the change could look to get your thoughts 👍🏻

edaniels

Done with a first pass. DTLS guts are very new to me but this looks great so far after my reading of RFC9146!

edaniels · 2023-07-19T15:54:01Z

conn.go

+// PacketServer listens for incoming DTLS connections.
+// Unlike Server, PacketServer allows for connections to change remote address.
+// The provided rAddr will be used as the initial remote address for sending.
+func PacketServer(ctx context.Context, conn net.PacketConn, rAddr net.Addr, config *Config) (*Conn, error) {


I'm okay going in either direction. My gut is that we stick with the non-breaking approach for this PR and then immediately follow up with an issue (or PR) to go over proposed v3 changes just so that we don't do anything premature/rash here.

edaniels · 2023-07-19T16:01:39Z

pkg/protocol/extension/connection_id.go

+//
+// https://tools.ietf.org/html/rfc9146
+type ConnectionID struct {
+	// Zero-length connection ID indicates that connection ID will be sent but


It might be better to word this in the perspective of client and server as the RFC does. Maybe something like "A zero-length connection ID indicates for a client or server that negotiated connection IDs from the peer will be sent but there is no need to respond with one."?

That sounds like an improvement, thanks! Updated 👍🏻

edaniels · 2023-07-19T16:06:29Z

pkg/protocol/extension/connection_id.go

+
+	var cid cryptobyte.String
+	if !extData.ReadUint8LengthPrefixed(&cid) {
+		return errInvalidSNIFormat


I think we want a specific error message for CID right?

Ah yes -- thank you for catching!

edaniels · 2023-07-19T16:27:48Z

internal/util/util.go

+// AddUint48 appends a big-endian, 48-bit value to the byte string.
+// Remove if / when https://github.com/golang/crypto/pull/265 is merged
+// upstream.
+func AddUint48(b *cryptobyte.Builder, v uint64) {


a unit test for this would be nice, even if very small

Agreed -- unit tests here and a number of other areas are coming 👍🏻

edaniels · 2023-07-19T16:31:23Z

conn.go

+	h := &recordlayer.Header{
+		// Set connection ID size so that records of content type tls12_cid will
+		// be parsed correctly.
+		ConnectionID: make([]byte, len(c.state.localConnectionID)),


this may be worth checking in advance if you should call the make or not to save a few cycles

good call 👍🏻

edaniels · 2023-07-19T16:39:33Z

pkg/protocol/connection_id.go

+
+package protocol
+
+// ConnectionIDData messages are carried by the record layer and wrap an


where is this used?

It was a remnant of an earlier iteration -- removed 👍🏻

edaniels · 2023-07-19T16:43:38Z

pkg/protocol/recordlayer/inner_plaintext.go

+	var out cryptobyte.Builder
+	out.AddBytes(p.Content)
+	out.AddUint8(uint8(p.RealType))
+	out.AddBytes(make([]byte, p.Zeros))


is it worth defining this once so that we don't reallocate on each marshal?

We could use a []byte instead of a uint on the InnerPlaintext, but I opted for a more concise representation given that when we are receiving messages we unmarshal and have no need for the zeros slice. When we are sending, we typically only marshal the InnerPlaintext once, meaning that we don't really get much of an advantage from allocating the slice when we instantiate the structure vs. when we marshal it. One thing we could opt to do is allocate the slice the first time we marshal, save it as an unexported []byte field, then use it in the next marshal if != nil. I'm happy to adjust as desired.

edaniels · 2023-07-19T16:50:21Z

flight1handler.go

+	// If we have a connection ID generator, use it. The CID may be zero length,
+	// in which case we are just requesting that the server send us a CID to
+	// use.
+	if cfg.connectionIDGenerator != nil {


I think it's important to note in the config that you must specify a function that returns a nil/zero length CID in order to get CID support. I'd also be open to the idea of being able to specify you're willing to send back a peers CID (something like SupportPeerCID)

Updated the description on the config to be more explicit. I am also open to an additional config value, although just the generator does allow for specifying support for all modes. One option to make it more explicit could be to define a generator named something like SendOnlyCIDGenerator() that just returns nil in all cases.

I like the generator idea!

Added OnlySendCIDGenerator() and corresponding tests 👍🏻

edaniels · 2023-07-19T16:56:14Z

conn.go

+			inner := &recordlayer.InnerPlaintext{
+				Content:  handshakeFragment,
+				RealType: protocol.ContentTypeHandshake,
+				Zeros:    8,


optional: could have this as a named const

Agreed, though I really should have a TODO here. Using a constant padding size doesn't really provide much value here. There are a variety of both deterministic and non-deterministic padding strategies that can be used, so I was considering allowing for a padding generator to be passed in config similar to the connection ID generator. If none is passed, the default would just be zero-length padding.

For some examples of how other (D)TLS libraries handle this:

mbedTLS has an ssl_compute_padding_length function that takes the content length and a granularity argument. The granularity is configured via the MBEDTLS_SSL_CID_TLS1_3_PADDING_GRANULARITY option.

scandium (californium) allows passing a pad when generating a Record. However, it looks like it is only ever set to 0 currently.

Given these as a small sample, I think the generator strategy which would default to zero-length padding seems like a reasonable solution for initial implementation, while also allowing for flexibility.

/cc @boaks @Sean-Der if either of you want to weigh-in :)

zero length seems fine since yeah, a constant doesn't really add much value when it's known

Added a PaddingLengthGenerator config that defaults to 0 👍🏻

edaniels · 2023-07-19T17:09:17Z

conn.go

+		// Any valid connection ID record is a candidate for updating the remote
+		// address.
+		// https://datatracker.ietf.org/doc/html/rfc9146#peer-address-update
+		if rAddr != c.RemoteAddr() {


Do we need to account for condition 2 of 3 for address update?:

The received datagram is "newer" (in terms of both epoch and sequence number) than the newest datagram received. Reordered datagrams that are sent prior to a change in a peer address might otherwise cause a valid address change to be reverted

Checking a few things here, but will follow up in a moment :)

Following up: from my understanding, we don't currently do any reordering based on sequence number, only on epoch. The latest sequence number is updated in the replay detector when we call markPacketAsValid(). The anti-replay strategy defined in the DTLS 1.2 RFC is used.

I think a reasonable solution without too much churn would be to return from the callback (i.e. markPacketAsValid()) a bool that indicates whether marking the packet as valid resulted in a latest sequence number update. We could then use that value to determine whether we need to update remote address or not.

cc @Sean-Der @daenney

I think that's a perfectly acceptable solution. Honestly if there's a unit test backing it, that's more important to me right now!

@edaniels thanks! PR opened here with implementation pion/transport#256

nice. the change from there looks good here

boaks · 2023-07-19T18:05:16Z

Pretty cool! I like the docker stuff! Is it possible to start a server only with the docker image? And to try some handshakes with clients using other implementations?

daenney · 2023-07-19T22:46:16Z

Pretty cool! I like the docker stuff! Is it possible to start a server only with the docker image? And to try some handshakes with clients using other implementations?

Right now we only use Docker for the E2E CI tests, and they start a specific subset of tests using a build tag. However, you can look at examples/listen for how to start a server using this library to handle DTLS. Since it's Go the resulting binary can be copied into a tiny base image.

So, very crudely, it could be along the lines of:

# Start by building the application.
FROM golang:1.20 as build

WORKDIR /go/src/app
COPY . .

RUN go mod download
RUN CGO_ENABLED=0 go build examples/listen/verify -o /go/bin/verify

# Now copy it into our base image.
FROM gcr.io/distroless/static-debian11
COPY --from=build /go/bin/verify /
CMD ["/verify"]

The only thing you'd really have to do is adapt the accept loop.

boaks · 2023-07-24T09:52:40Z

So, very crudely, it could be along the lines of:

That results on "my machine" in:

package examples/listen/verify is not in GOROOT (/usr/local/go/src/examples/listen/verify)
malformed import path "-o": leading dash
stat /go/bin/verify: directory not found

I'm no GO user, so I'll just wait for others results.

hasheddan · 2023-07-24T12:30:08Z

@boaks apologies for the delay here! I have put together a test server and image that can be found at https://github.com/hasheddan/dtls-interop/tree/pion/dtls/cid/pion/cid_server. I will continue building this out with more functionality as I test against DTLS implementations. There are instructions for building and running the container image, and I have also pushed a preliminary version to DockerHub at hasheddan/dtls-interop-pion-cid-server.

The test server is setup to work out of the box with the example californium DTLS client. I tested by running the following and then executing the californium DTLS client.

docker run --rm --net host hasheddan/dtls-interop-pion-cid-server

The resulting packet capture can be found here. Please let me know if there is anything else I can do to help assist in your testing!

boaks · 2023-07-24T12:48:12Z

Very nice!
I've tested it with an mbedtls client successfully.

boaks · 2023-07-24T12:54:19Z

The resulting packet capture can be found here.

Just to mention, as in my WikiPage for Captures: if the capture is compressed, it can be uploaded to a git comment just by drag&drop.

Please let me know if there is anything else I can do to help assist in your testing!

Thanks. For now, I only test the basic interoperability. That looks pretty good.

hasheddan · 2023-08-02T18:51:20Z

conn.go

-func Client(conn net.Conn, config *Config) (*Conn, error) {
+func Client(conn net.PacketConn, rAddr net.Addr, config *Config) (*Conn, error) {


Note: all client / server constructors are updated to accept net.PacketConn, rather than accepting net.Conn as we are moving towards using net.PacketConn. However, fully switching over to net.PacketConn involves refactoring / porting a fair amount of pion/transport (i.e. udp.Listener, packetio.Buffer, etc.) to live in their new forms in pion/dtls. The current state of this PR allows for any consumers to use all of the client / server constructors directly to exercise existing and new functionality (i.e. connection IDs). The follow-up work is mostly around updating the dtls.Listener (see pion/transport#252 for the general direction of what those changes will look like). However, given that those changes will be fairly large and will include their own rounds of review, which is fairly unrelated to the connection ID work in this PR, I would propose that we work to get this PR reviewed and merged, then I can follow with updates to support the new listener behavior. In the interim, folks will be able to exercise the connection ID functionality and the previous listener behavior will continue to work as expected.

@Sean-Der @edaniels @daenney do y'all have any thoughts in favor or against this proposal? I have the work in progress to port the aforementioned packages, but I also am cognizant that introducing them will likely 3x the size of this PR, which could make reviewing even more cumbersome.

I think sticking with this new Client/Server + FromConn is preferred. It also lets us get other packages updated with the breaking API change and allows it to bake for a bit.

hasheddan · 2023-08-02T21:38:00Z

conn_test.go

+		"BidirectionalConnectionIDs": {
+			clientCfg: &Config{
+				ConnectionIDGenerator: cidEcho(clientCID),
+			},
+			serverCfg: &Config{
+				ConnectionIDGenerator: cidEcho(serverCID),
+			},
+			clientConnectionID: clientCID,
+			serverConnectionID: serverCID,
+		},
+		"BothSupportOnlyClientSends": {
+			clientCfg: &Config{
+				ConnectionIDGenerator: cidEcho(nil),
+			},
+			serverCfg: &Config{
+				ConnectionIDGenerator: cidEcho(serverCID),
+			},
+			serverConnectionID: serverCID,
+		},
+		"BothSupportOnlyServerSends": {
+			clientCfg: &Config{
+				ConnectionIDGenerator: cidEcho(clientCID),
+			},
+			serverCfg: &Config{
+				ConnectionIDGenerator: cidEcho(nil),
+			},
+			clientConnectionID: clientCID,
+		},
+		"ClientDoesNotSupport": {
+			clientCfg: &Config{},
+			serverCfg: &Config{
+				ConnectionIDGenerator: cidEcho(serverCID),
+			},
+		},
+		"ServerDoesNotSupport": {
+			clientCfg: &Config{
+				ConnectionIDGenerator: cidEcho(clientCID),
+			},
+			serverCfg: &Config{},
+		},
+		"NeitherSupport": {
+			clientCfg: &Config{},
+			serverCfg: &Config{},
+		},


This is the most succinct representation of all of the different CID scenarios between a client and server.

edaniels

This LGTM. I have two test suggestions but they do not block merging the code in. Great work!

edaniels · 2023-08-09T02:06:59Z

pkg/protocol/extension/connection_id_test.go

+
+	raw, err := parsedExtensionConnectionID.Marshal()
+	if err != nil {
+		t.Error(err)


are these errors better as Fatal so as to stop the current test immediately?

@edaniels I think so -- as the subsequent results are not useful if we error here. I'll update across other extensions as well 👍🏻

pkg/crypto/ciphersuite/ciphersuite_test.go

Updates pion/transport to v2.2.2-0.20230802201558-f2dffd80896b to consume the new netctx packages and the replaydetector updates. Signed-off-by: Daniel Mangum <[email protected]>

Adds the DTLS 1.2 connection ID extension to supported handshake extensions. Connection ID uses 54 as its identifier and includes a potentially empty connection ID. Signed-off-by: Daniel Mangum <[email protected]>

Adds support for parsing CID records, which wrap an internal in inner plaintext. Consumers of ContentAwareUnpackDatagram must specify the CID length. The HeaderSize of 13 is now referred to as FixedHeaderSize to indicate that CID records will have variable sized headers. Signed-off-by: Daniel Mangum <[email protected]>

ConnectionIDGenerator generates connection identifiers that should be sent by the remote party if it supports the DTLS Connection Identifier extension, as determined during the handshake. Generated connection identifiers must always have the same length. Returning a zero-length connection identifier indicates that the local party supports sending connection identifiers but does not require the remote party to send them. A nil ConnectionIDGenerator indicates that connection identifiers are not supported. https://datatracker.ietf.org/doc/html/rfc9146 PaddingLengthGenerator generates the number of padding bytes used to inflate ciphertext size in order to obscure content size from observers. The length of the content is passed to the generator such that both deterministic and random padding schemes can be applied while not exceeding maximum record size. If no PaddingLengthGenerator is specified, padding will not be applied. https://datatracker.ietf.org/doc/html/rfc9146#section-4 Signed-off-by: Daniel Mangum <[email protected]>

Updates the packet structure to allow for specifying the a given packet should be stuffed in the inner plaintext of a CID record. Signed-off-by: Daniel Mangum <[email protected]>

Adds the connection ID content type, which used 25 as identifier. Signed-off-by: Daniel Mangum <[email protected]>

Adds the AddUint48 utility used to handle sequence numbers in DTLS records. This should be removed when golang/crypto#265 is implemented. Signed-off-by: Daniel Mangum <[email protected]>

Adds local and remote CID to state, but does not serialize them. CIDs should be renogotiated on session resumption. Signed-off-by: Daniel Mangum <[email protected]>

Implements AEAD additional data generation when using connection IDs as described in https://datatracker.ietf.org/doc/html/rfc9146#name-aead-ciphers. Signed-off-by: Daniel Mangum <[email protected]>

Updates ciphersuites to add support for handling connection ID records. Signed-off-by: Daniel Mangum <[email protected]>

Updates handshaker to handle negotiating CIDs. Local connection ID is only set if the local party generates one and the remote indicates support. Remote connection id is only set if remote generates one and connection IDs are supported locally Signed-off-by: Daniel Mangum <[email protected]>

Adds a utility used to translate net.Conn's to net.PacketConn's for use in DTLS package. This utility may be promoted to a public package in the future if consumers require using net.Conn, particularly if they are using connected UDP sockets. Signed-off-by: Daniel Mangum <[email protected]>

Updates DTLS Conn to take a net.PacketConn and support connection IDs. Moving to net.PacketConn allows for updating the remote address when utilizing connection IDs, alleviating the need to re-handshake every time an IP address or port changes. Signed-off-by: Daniel Mangum <[email protected]>

Wraps the net.Conn returned from the UDP listener in the DTLS listener. The underlying UDP listener will be adapted in the future to support returning net.PacketConn. Signed-off-by: Daniel Mangum <[email protected]>

Updates tests that use net.Conn to wrap in net.PacketConn so that new Client / Server constructors may be used. Signed-off-by: Daniel Mangum <[email protected]>

Adds e2e tests for connection ID support. OpenSSL does not currently support connection IDs, so tests are only run between pion/dtls client and server. Support for other libraries, such as californium and mbedTLS will be added in the future. Signed-off-by: Daniel Mangum <[email protected]>

Adds simple connection ID generators that can be used to generate random connection IDs of a given length, or indicate support for connection IDs without requiring the remote to send one. Signed-off-by: Daniel Mangum <[email protected]>

Sean-Der reviewed Jul 12, 2023

View reviewed changes

pkg/protocol/recordlayer/fuzz_test.go Outdated Show resolved Hide resolved

Sean-Der reviewed Jul 12, 2023

View reviewed changes

pkg/protocol/recordlayer/recordlayer.go Outdated Show resolved Hide resolved

Sean-Der reviewed Jul 12, 2023

View reviewed changes

conn.go Outdated Show resolved Hide resolved

hasheddan commented Jul 12, 2023

View reviewed changes

internal/ciphersuite/aes_ccm.go Show resolved Hide resolved

hasheddan force-pushed the feat/cid branch from 4823f71 to 8b8ce87 Compare July 18, 2023 21:12

hasheddan changed the title ~~[DRAFT] Implement DTLS 1.2 Connection IDs~~ Implement DTLS 1.2 Connection IDs Jul 18, 2023

hasheddan marked this pull request as ready for review July 18, 2023 22:24

hasheddan requested review from daenney and Sean-Der July 18, 2023 22:51

hasheddan commented Jul 19, 2023

View reviewed changes

hasheddan mentioned this pull request Jul 19, 2023

Plans for DTLS 1.2 Connection ID #256

Closed

edaniels requested changes Jul 19, 2023

View reviewed changes

hasheddan mentioned this pull request Aug 1, 2023

feat: add latest sequence number notify to replaydetector pion/transport#256

Merged

hasheddan force-pushed the feat/cid branch from 1cda5b9 to 9d01288 Compare August 1, 2023 19:56

hasheddan mentioned this pull request Aug 2, 2023

net.PacketConn #367

Closed

hasheddan force-pushed the feat/cid branch from 09e44fb to 6944aa7 Compare August 2, 2023 18:02

hasheddan commented Aug 2, 2023

View reviewed changes

hasheddan self-assigned this Aug 2, 2023

hasheddan mentioned this pull request Aug 2, 2023

Consume pion/dtls connection ID support plgd-dev/go-coap#467

Closed

hasheddan requested a review from edaniels August 8, 2023 19:39

edaniels approved these changes Aug 9, 2023

View reviewed changes

hasheddan force-pushed the feat/cid branch 4 times, most recently from ff9094f to 7f6325e Compare August 9, 2023 19:05

hasheddan added 17 commits August 9, 2023 15:10

Update pion/transport to latest

cd81c2c

Updates pion/transport to v2.2.2-0.20230802201558-f2dffd80896b to consume the new netctx packages and the replaydetector updates. Signed-off-by: Daniel Mangum <[email protected]>

Add DTLS connection ID extension

44746c5

Adds the DTLS 1.2 connection ID extension to supported handshake extensions. Connection ID uses 54 as its identifier and includes a potentially empty connection ID. Signed-off-by: Daniel Mangum <[email protected]>

Allow packets to specify CID wrapped

f54b529

Updates the packet structure to allow for specifying the a given packet should be stuffed in the inner plaintext of a CID record. Signed-off-by: Daniel Mangum <[email protected]>

Add CID content type

b8391f2

Adds the connection ID content type, which used 25 as identifier. Signed-off-by: Daniel Mangum <[email protected]>

Implement AddUint48 utility

eba51ae

Adds the AddUint48 utility used to handle sequence numbers in DTLS records. This should be removed when golang/crypto#265 is implemented. Signed-off-by: Daniel Mangum <[email protected]>

Add local and remote CID to state

5e5bde9

Adds local and remote CID to state, but does not serialize them. CIDs should be renogotiated on session resumption. Signed-off-by: Daniel Mangum <[email protected]>

Implement AEAD additional data with CID

7888afd

Implements AEAD additional data generation when using connection IDs as described in https://datatracker.ietf.org/doc/html/rfc9146#name-aead-ciphers. Signed-off-by: Daniel Mangum <[email protected]>

Update ciphersuites to support CIDs

6f020b6

Updates ciphersuites to add support for handling connection ID records. Signed-off-by: Daniel Mangum <[email protected]>

Wrap net.Conn in DTLS listener

644e596

Wraps the net.Conn returned from the UDP listener in the DTLS listener. The underlying UDP listener will be adapted in the future to support returning net.PacketConn. Signed-off-by: Daniel Mangum <[email protected]>

Update tests to wrap net.Conn

34822d5

Updates tests that use net.Conn to wrap in net.PacketConn so that new Client / Server constructors may be used. Signed-off-by: Daniel Mangum <[email protected]>

Add basic connection ID generators

ed08ac1

Adds simple connection ID generators that can be used to generate random connection IDs of a given length, or indicate support for connection IDs without requiring the remote to send one. Signed-off-by: Daniel Mangum <[email protected]>

hasheddan force-pushed the feat/cid branch from 7f6325e to ed08ac1 Compare August 9, 2023 19:11

hasheddan merged commit d29c6f0 into pion:master Aug 9, 2023
16 checks passed

hasheddan mentioned this pull request Aug 10, 2023

Support net.PacketConn in DTLS Listener #568

Closed

jaredwolff mentioned this pull request Aug 11, 2023

dtls - Implement DTLS 1.2 Connection IDs webrtc-rs/webrtc#478

Open

hasheddan mentioned this pull request Aug 21, 2023

Support use of net.PacketConn in dtls.Listener #570

Merged


		package protocol

		// ConnectionIDData messages are carried by the record layer and wrap an

		func Client(conn net.Conn, config Config) (Conn, error) {
		func Client(conn net.PacketConn, rAddr net.Addr, config Config) (Conn, error) {

Implement DTLS 1.2 Connection IDs #558

Implement DTLS 1.2 Connection IDs #558

Conversation

hasheddan commented Jul 12, 2023 • edited Loading

Description

Notes for early reviewers:

Testing Locally

Reference issue

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jul 18, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edaniels left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boaks commented Jul 19, 2023

daenney commented Jul 19, 2023

boaks commented Jul 24, 2023

hasheddan commented Jul 24, 2023

boaks commented Jul 24, 2023

boaks commented Jul 24, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edaniels left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hasheddan commented Jul 12, 2023 •

edited

Loading

codecov bot commented Jul 18, 2023 •

edited

Loading

edaniels left a comment •

edited

Loading