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

Pure Rust DTLS aka supplementing openssl #326

Open
thomaseizinger opened this issue Sep 3, 2023 · 19 comments
Open

Pure Rust DTLS aka supplementing openssl #326

thomaseizinger opened this issue Sep 3, 2023 · 19 comments

Comments

@thomaseizinger
Copy link
Collaborator

thomaseizinger commented Sep 3, 2023

I wanted to open this issue as a central place for discussing the removal/supplementing of the openssl dependency. As far as I understand, this is desired, see #107 (comment) for example.

It seems that webrtc-rs has a pure-Rust DTLS implementation at https://github.com/webrtc-rs/webrtc/tree/master/dtls but it seems to not be audited.

@xnorpx
Copy link
Collaborator

xnorpx commented Sep 3, 2023

Why is openssl support a problem?

It's performant and some organizations are not allowed to use other crypto libraries.

Even Webrtc-rs has added dependency on it now: webrtc-rs/webrtc#477

I can understand why one would want a feature to build without it, but removing support seems counterproductive....

@algesten
Copy link
Owner

algesten commented Sep 3, 2023

I don't see a reason to remove it, it will be there with a feature flag, but i'd like the default to be a rust native crypto.

I'm a bit hesitant about the webrtc-rs DTLS since, afaik, it's only used by webrtc-rs. I think it's bespoke and it's unclear how many crypto experts have read the code.

This is why my preferred path is for someone to engage with the rustls project and do DTLS there.

@thomaseizinger
Copy link
Collaborator Author

That makes sense, thank you for clarifying. I'll reword the title and description! :)

@thomaseizinger thomaseizinger changed the title Removing openssl dependency Pure Rust DTLS aka supplementing openssl Sep 3, 2023
@k0nserv
Copy link
Collaborator

k0nserv commented Sep 4, 2023

I'm a bit hesitant about the webrtc-rs DTLS since, afaik, it's only used by webrtc-rs. I think it's bespoke and it's unclear how many crypto experts have read the code.

I agree with this concern, but it is worth noting that the webrtc-rs implementation is a port of pion/dtls so not entirely bespoke.

@algesten
Copy link
Owner

algesten commented Dec 9, 2023

Some associated discussion in #413

@algesten
Copy link
Owner

algesten commented Dec 9, 2023

I did some further investigation. openssl does not support the latest DTLS 1.3 standard yet (openssl/openssl#13900). libWebRTC uses boringssl which is a fork of openssl. I think that means most WebRTC traffic uses DTLS 1.2 (though firefox might support 1.3)

That means a pure DTLS 1.3 implementation wouldn't be of much use to str0m – any attempt to build this with rustls or otherwise must do 1.2 compatibility.

@xnorpx
Copy link
Collaborator

xnorpx commented Jan 24, 2024

@thomaseizinger maybe good to summarize what you found regarding full non-openssl feature during your ice-agent work here.

@thomaseizinger
Copy link
Collaborator Author

thomaseizinger commented Jan 24, 2024

@thomaseizinger maybe good to summarize what you found regarding full non-openssl feature during your ice-agent work here.

I am not sure I understand. The only reason I made openssl optional is because we only use the ICE agent which doesn't use any crypto besides a sha1-hmac. The only "finding" I had is that aes_128_cm_sha1_80 seems to be so outdated that neither rustls nor ring support it (for good reasons of broken cryptography I think).

@xnorpx
Copy link
Collaborator

xnorpx commented Jan 24, 2024

RIght that was what I was looking for.

So potentially a rust only implementation could skip the older aes_128_cm_sha1_80 and only implement GCM.

@k0nserv
Copy link
Collaborator

k0nserv commented Jan 24, 2024

Logic needs to be tweaked a little for this, currently we assume the supported SRTP profiles are common across all crypto implementations

@thomaseizinger
Copy link
Collaborator Author

Logic needs to be tweaked a little for this, currently we assume the supported SRTP profiles are common across all crypto implementations

Yeah that is why I argued for not creating too many abstractions regarding the crypto implementations for now. The real requirements will show once another one is integrated :)

@nazar-pc
Copy link

And now that Substrate pulled litep2p that pulled str0m I'm also looking forward to get rid of OpenSSL.
OpenSSL is a huge dependency and is a pain to deal with in cross-platform way, so I was avoiding it at https://github.com/subspace/subspace successfully until today.

Looking forward to ability to disable OpenSSL in str0m, even if second option is not yet audited or as performant as OpenSSL.

@lolgesten
Copy link

Sadly Rustls has taken an awkward direction for us here. They are now primarily relying on aws-lc for crypto instead of ring (which is an optional dep). My vision for str0m would be to have a rust native solution and aws-lc is not that. I have made one attempt at writing DTLS myself, but didn't make it very far and of course, rolling our own is only slightly better than using webrtc-rs variant.

@daxpedda
Copy link

They are now primarily relying on aws-lc for crypto instead of ring (which is an optional dep).

My understanding is that aws-lc is just a crypto provider, so it should be perfectly possible to use Rustls with Ring without relying on aws-lc at all.

@algesten
Copy link
Owner

My understanding is that aws-lc is just a crypto provider, so it should be perfectly possible to use Rustls with Ring without relying on aws-lc at all.

Yeah, I know. My comment is more about being uncertain whether the rustls project is going in a direction is compatible with str0m. The ambitions might be mismatched.

@gilescope
Copy link

gilescope commented Sep 3, 2024

@xnorpx why is openssl a problem? It's native code that isn't rust and thus on a variety of systems it requires more work to make it compile than rustls or similar which I have never had a failed build from. Can you guess why I'm here? It's to implore you to have a pure rust option.

Error building OpenSSL:
*failed* |       Command: cd "/target/release/build/openssl-sys-bf1da25cfeb50714/out/openssl-build/build/src" && MAKEFLAGS="-j --jobserver-fds=16,17 --jobserver-auth=16,17" "make" "build_libs"

I can't see rustls not having a pure rust option in the future. That would be a very retrograde step and would do damage to the wider ecosystem as many tools would only work if you happened to have various native bits properly configured.

@algesten
Copy link
Owner

algesten commented Sep 3, 2024

@gilescope Rustls and aws-lc is in some respects neither here nor there because that project doesn't have a DTLS implementation either. The requirements are:

  • DTLS 1.2 (not 1.3!)
  • Sync
  • Pure Rust

Any solution in that direction is interesting to explore.

@xnorpx
Copy link
Collaborator

xnorpx commented Sep 3, 2024

@xnorpx why is openssl a problem? It's native code that isn't rust and thus on a variety of systems it requires more work to make it compile than rustls or similar which I have never had a failed build from. Can you guess why I'm here? It's to implore you to have a pure rust option.

Error building OpenSSL:
*failed* |       Command: cd "/target/release/build/openssl-sys-bf1da25cfeb50714/out/openssl-build/build/src" && MAKEFLAGS="-j --jobserver-fds=16,17 --jobserver-auth=16,17" "make" "build_libs"

I can't see rustls not having a pure rust option in the future. That would be a very retrograde step and would do damage to the wider ecosystem as many tools would only work if you happened to have various native bits properly configured.

From a build perspective OpenSSL is a pain no disagreement there. I fully support having a feature once someone is motivated enough to implement their own DTLS implementation and get it integrated.

This is probably what is closest to @algesten requirement so far but looks WIP atm.

https://github.com/webrtc-rs/rtc/tree/master/rtc-dtls

Maybe @gilescope Polkadot has people/resources to spare to help finalize that work and integrate it?

Some relevant links:

@k0nserv
Copy link
Collaborator

k0nserv commented Sep 3, 2024

I think we'd need both an implementation(the webrtc-rs one could be a starting point) and then, ideally, a security audit of the resulting code. This requires both a contribution in terms of work and in terms of finances.

webrtc-rs has quite a bit of cash on hand. Maybe an option is to refactor their implementation to be sans-io and ask if they would be open to funding a security audit.

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

No branches or pull requests

8 participants