Stateful session resumption

[taoso]

Add session resumption support.

You could use the examples/listen/psk/main.go and openssl to test this feature.

First you need to change the server's config and run the server.

    config := &dtls.Config{
          ...
          CipherSuites: []dtls.CipherSuiteID{dtls.TLS_PSK_WITH_AES_128_GCM_SHA256},
          SessionStore: ...,
          ServerName: "example.com",
          ...
    }

And then, run openssl one time to store the session data.

openssl s_client -dtls -cipher PSK-AES128-GCM-SHA256 -psk abc123 -sess_out /tmp/s.txt  -connect 127.0.0.1:4444

Finally, run openssl in the resumption mode.

openssl s_client -dtls -cipher PSK-AES128-GCM-SHA256 -psk abc123 -sess_in /tmp/s.txt  -connect 127.0.0.1:4444

[daenney]

RFC 5077 is indeed obsolete since RFC 8446. But the RFC that obsoleted it is about TLS 1.3, whereas we support (D)TLS 1.2. So generally speaking supporting session resumption based on session ID is not unreasonable for us to have. DTLS 1.3 also still has some support for it in the form of the legacy_session_id in ClientHello even though it has a new way to do session resumption.

With respect to your PR: we can't accept this addition of functionality without tests that verify it works as intended. This will also ensure we don't break this feature in the future. This is especially important for features we don't expect to be used/exercised regularly by users of the library.

On a personal note; we have made it this far without it, and I am a little frustrated by the idea of supporting this b/c a company with resources like Cisco can't be bothered to follow an RFC and implement PSK support. However, that should not stop us from supporting session resumption in general.

[Sean-Der]

Very cool project! Would love to help get this merged :) 100% agree with @daenney we need tests.

Also DTLS resumption has been known to cause security issues so want to be especially careful with this change!

[taoso]

this pr just make some basic feature which will be needed to implement session resume. If it will be accepted, i will try my best to add the test case.

[daenney]

I don't have strong objections to it. I'd be inclined to accept the PR seeing it purely implements session resumption.

[taoso]

Hi @daenney @Sean-Der
I have implemented session resumption. Please make your comments. Thank you.

This PR has a small test. And it could work with openssl's dtls session resumption.

[taoso]

Anybody is here?

[daenney]

We're here. But please keep in mind we are open source maintainers, doing this work in our free time. We can't always respond within hours, and review will take time.

[taoso]

We're here. But please keep in mind we are open source maintainers, doing this work in our free time. We can't always respond within hours, and review will take time.

Hi @daenney

There are nobody expecting hourly respond. I do understand most contributors make contributions in their free time.

However, I made this PR three days ago and asked yesterday. I just want to make sure there are maintainers who have interest in this PR. And it will be great encouragement to me if some maintainers could make some response or put this MR review in their plan.

Thank you.

[taoso]

@pf512 94a8dfd will fix the error you mentationed.

[taoso]

Hi @daenney @Sean-Der , one month passed. Would this PR going to be placed in any maintainer schedule?

[grokker001]

This PR only implements the session ID based session resumption which requires the server to save the session info (https://datatracker.ietf.org/doc/html/rfc5246#appendix-F.1.4). It doesn't implement the session ticket based session resumption: https://datatracker.ietf.org/doc/html/rfc5077

[taoso]

@grokker001

This PR was originally designed for Cisco's AnyConnect VPN protocol, which used session resumption to establish DTLS session. And according @daenney 's advice, the full stateful session resumption has been implemented.

If the ticket based session resumption feature is required, it may be a little better to implement it in a dedicate PR. This depends on the maintainers' requirement.

However, it seems no maintainer has time to make a review or give any feedback. It needs to wait the maintainers' bandwidth.

[grokker001]

How does this PR prevent data replay attack of the resumed "ClientHello" packet?

[taoso]

@grokker001
The attacker cannot get the master secret, so the session will terminated by the following code:
https://github.com/pion/dtls/pull/369/files#diff-4f427d2b022907c552328e63f137561f6de92396d7a6e8f6c2ea1bcf0db52654R686-R688

[taoso]

Hi @daenney @Sean-Der , would you like to offer any feedback about this PR. Even a rejection is welcome.

[daenney]

I don't have the time to review this right now. In general I'm OK with having this code. But it's going to be a while before I personally have time to give it the review it needs before it goes in.

[taoso]

@daenney thank you for reply. Waiting for your free time.

[taoso]

Hi @daenney almost 6 months has past. I not intend to press you make action. But would you like put this MR into your schedule? Or make any clue for your following plan?

Thanks.

[daenney]

I honestly don't know. I'm not particularly invested in getting session support in here myself since I don't have any need for it in practice. It's also a big change and once we accept it something we'll need to keep supporting going forwards.

Idk if other maintainers like @Sean-Der, @at-wat or @backkem might have time to review this.

Coincidentally, the tests for this thing haven't run yet. Can you rebase and push this again and see if that unblocks it?

[taoso]

Coincidentally, the tests for this thing haven't run yet. Can you rebase and push this again and see if that unblocks it?

It seems the workflows will not run without approval. @daenney

[at-wat]

I'll review the code tomorrow

[daenney]

Coincidentally, the tests for this thing haven't run yet. Can you rebase and push this again and see if that unblocks it?

It seems the workflows will not run without approval. @daenney

Yeah, but for some reason GitHub wasn't showing the approval buttons. It did after you pushed, so I've OKed them to run now.

[taoso]

@daenney @at-wat I will fix the workflow as soon as possible.

[taoso]

@daenney thanks. But please approve again. In order to pass the git message lint, I have squashed all the commits.

[at-wat]

Main logic of the handshake with resumption looks good!
Added some more comments.

[taoso]

hi @at-wat , do you have time to review the latest changes?

[taoso]

@at-wat please check again, all the issues have been fixed.

[at-wat]

LGTM:+1:

[taoso]

Hi @daenney @Sean-Der , as this PR has been reviewed by @at-wat , what else need I do to make it merged?

[daenney]

Hi @daenney @Sean-Der , as this PR has been reviewed by @at-wat , what else need I do to make it merged?

Nothing left for you to do. If @at-wat is happy with it, then I think we can go ahead and merge it.

Since a lot of work on this PR happened during the winter holidays when a lot of us were probably not keeping track of this I'll let it soak for another week so other maintainers have a chance to comment. If nothing shows up, I'll merge it on the 10th of January.

[taoso]

Hi @daenney , as there is no further comment, would you like to merge this PR?

[daenney]

Released as v2.1.0

[taoso]

Thank you @daenney

And thank you @at-wat for your detailed review and mentor. Is is a great pleasant procedure to corporate with you and you make me learn a lot. Thank you.

By the way, @Sean-Der, @at-wat, @backkem, @daenney, @at-wat is there any roadmap to support connection id for dtls 1.2 and the dtls 1.3?

[daenney]

There isn't, no. We don't really have a roadmap. It's a volunteer driven project so people contribute features if and when the need arises for whatever use case they're facing.

[swechencheng]

Please forgive me if I am asking some stupid question:
Is this feature related to RFC6347#section-4.2.8?
@taoso @daenney