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 EC-DSA, custom certificate common names for DTLS #607

Merged
merged 2 commits into from
Jan 10, 2025

Conversation

efer-ms
Copy link
Contributor

@efer-ms efer-ms commented Jan 9, 2025

  • New Features -

Update Str0m to support EC-DSA certification for DTLS. See: https://developer.chrome.com/blog/webrtc-ecdsa/

This updates both the OpenSSL and Wincrypto providers to allow both RSA or EC-DSA certificates. The default remains RSA, although perhaps it should change to EC-DSA at some point.

Took the opportunity to also allow the CN of the certificate to be configured (as this was an existing TODO).


  • Other changes -

OpenSSL is configured to indicate SHA1 is not acceptable for the DTLS certificate signing, but then proceeded to use SHA1 and for some reason accepts it. This changes it to use SHA256 which matches WinCrypto, since the Windows APIs refused to offer a SHA1 signed cert if the DTLS handshake indicated it wasn't supported.

WinCrypto was updated to use the Owned struct that windows-rs offers for cleaning up the SrtpKey, rather than having a custom Drop impl.

WinCrypto had an issue when compiled in --release, where DTLS handshake would fail as a server. This was related to buffers being marked as immutable, but then cast to mutable in unsafe code. The compiler (I believe) was using the immutability to avoid reading the values multiple times, so although things were mutated in the unsafe code, the code did not recognize the changes. This just updated those buffers to be marked mut, it also eliminated some of the double-casts in unsafe code.

…me buffer handling in wincrypto (#11)

* Clippy fixes

Add options for DTLS certs. Implement ECDSA

Pass flag

Add EC generation

generatesecdsa

oops

ugg

firs time?

oops

generatesecdsa

generatesecdsa

cleanup dtls cert and windows resource ownership

fix things that are mut to be mut

grrrrrrrrr

ok I think it's working now

pass silent falg

* tweaks after merge

* ok fix test breaks

* another tweak

* add cerifications that cert subject/issuer is set correctly

* expand TODO comment

* blah blah blah

---------

Co-authored-by: Martin Algesten <martin@algesten.se>
@efer-ms efer-ms changed the title Add EC-DSA, custom certificate common names for DTLS in Str0m, fix so… Add EC-DSA, custom certificate common names for DTLS Jan 9, 2025
@efer-ms
Copy link
Contributor Author

efer-ms commented Jan 9, 2025

The public API change is mostly set_dtls_config in RtcConfig to allow changing the default cert generation options. The default behavior matches the current Str0m (RSA with a certificate common name of "WebRTC").

There is a breaking API change for Pregenerated DTLS certs, since to create a cert, you need to supply the certification options. I'm not sure how widespread the use of that API is.

Copy link
Owner

@algesten algesten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

A couple of minor comments.

Thanks for pushing ECDSA!

src/crypto/dtls.rs Outdated Show resolved Hide resolved
src/crypto/ossl/cert.rs Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Show resolved Hide resolved
@algesten algesten merged commit c72d48c into algesten:main Jan 10, 2025
24 of 25 checks passed
@algesten
Copy link
Owner

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants