Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add RFC5764 - SRTP key generation during DTLS handshake #361

Closed
wants to merge 7 commits into from

Conversation

jeannotlapin
Copy link
Contributor

Hi,
here is the updated patch for DTLS-SRTP.

I didn't add any test as I couldn't really found any doc on how to do that.

Simple one shall be, setup a dtls_srtp_profile on server and client side, perform handshake and compare the key material generated on both side.(I manually did that using modified program/ssl/dtls_client/server to check it's working).
Srtp profile selection shall be tested too.
Interop tests with other libraries would be great.

I tested it against the original patch on polarssl1.4(which itself has been tested with chrome and firefox DTLS implementation) and it seems to be ok.

@ciarmcom
Copy link

ciarmcom commented Dec 3, 2015

Automatic CI verification build not done, please verify manually.

@simonbutcher
Copy link
Contributor

I've emailed you directly concerning the CLA which needs to be signed and returned for us to be able to receive the contribution. Thanks!

@jeannotlapin
Copy link
Contributor Author

Ok thanks, as the contribution is just update the one submitted for polarssl in april, I thought the CLA sent then was still valid.
I'll send you the new one today.

@jeannotlapin
Copy link
Contributor Author

Simon,
I made new modifications (mainly moving some dtls_srtp fields from ssl context to ssl_config context and add a function to retrieve the generated key material instead of accessing directly some ssl_context fields).

Shall I push more commits or amend the original one(not exactly sure on how to do this by the way) ?

regards,

johan

@ciarmcom
Copy link

ciarmcom commented Feb 8, 2016

Automatic CI verification build not done, please verify manually.

@mpg
Copy link
Contributor

mpg commented Feb 9, 2016

I think pushing some more commits is the best way to go. It allows us to see the new changes easily.

(Generally speaking, it's best to err on the side of more granular commits, as it's easy to merge them after the fact using git rebase -i and its "squash" feature, while splitting commits after the fact is generally a pain (though it's possible, and also done with git rebase -i, it requires much more manual work and attention).)

@jeannotlapin
Copy link
Contributor Author

Hi Simon,
any news on merging this enhancement into the dev branch?

regards,

johan

@simonbutcher
Copy link
Contributor

Hi @jeannotlapin

We have been holding off merging pull requests while we prepare for a release. You may have noticed the last significant release was 2.2.1 in January, and when we last discussed merging this work, I said it would follow that release - which is unfortunately still pending.

Once that release has shipped, we will enter a window for merging many of these outstanding pull requests.

Hope this helps.

@jeannotlapin
Copy link
Contributor Author

Hi Simon,
any news on merging this enhancement?

thanks,

johan

@jeannotlapin
Copy link
Contributor Author

Hi Simon,
I just updated to latest commit from development branch and manage to get all the checks passed, any chance you would merge it?

regards,
johan

@carsonbaker
Copy link

+1

@simonbutcher
Copy link
Contributor

Hi @jeannotlapin,

We're hoping to get it integrated soon. Apologies for the long, long wait.

Simon

@hanno-becker hanno-becker self-assigned this Jun 7, 2017
@pasichnichenko
Copy link

+1

@jeannotlapin
Copy link
Contributor Author

Hi,

any news on the integration on this merge?

thanks

johan

spli added a commit to spli/esp-idf that referenced this pull request Oct 13, 2017
@simonbutcher simonbutcher requested a review from Patater March 26, 2018 10:32
@simonbutcher
Copy link
Contributor

@Patater - Please review for design, this and @RonEld's rework.

@Patater Patater assigned Patater and RonEld and unassigned hanno-becker and Patater Mar 28, 2018
@Patater
Copy link
Contributor

Patater commented Mar 28, 2018

Reviewed and approved for design.

Still needs more tests and some more thought on run-time vs compile-time configuration.

@RonEld
Copy link
Contributor

RonEld commented Mar 29, 2018

@jeannotlapin I have done some rework on this PR, and created a new PR #1540 which supersedes this one.
Please follow the new PR, as I am closing this PR

@RonEld RonEld closed this Mar 29, 2018
@RonEld RonEld mentioned this pull request Jun 28, 2018
4 tasks
iameli pushed a commit to livepeer/mbedtls that referenced this pull request Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants