Standalone ICE agent?

[thomaseizinger]

I am back with another inquiry! :)

Currently, I am trialing usage of str0m's IceAgent in a project that just needs ICE but nothing else around WebRTC. In particular, we don't want to pull in openssl as a dependency. I have a few small patches locally that publicly export a handful APIs around the ice module. However, I also have one that pretty much comments out everything that deals with openssl which is just a hack for now.

I would like to get some opinions on what a collaborative and useful way forward is. I think a SANS-IO & compliant ICE implementation is something really useful so I don't really want to fork str0m longterm. The ice module also depends on the io and parser module which makes extracting it not quite straight-forward. Additionally, splitting up str0m into multiple crates creates an additional maintenance burden.

I think extracting a dedicated str0m-ice crate (name to be bike-shedded) would make the most sense:

• The dependency on parser could be avoided by removing Candidate::from_sdp_string. This one is actually a circular dependency so could be argued shouldn't exist in the first place.
• The dependency on io is a bit trickier. To do message parsing, stun_codec could be used. I've had good experiences with it. Alternatively, something like str0m-stun would have to be introduced that handles message parsing.
• The rest of IO, i.e. all the SANS-IO modelling would either also have to go into a dedicated crate or perhaps inlined into str0m-ice?

Curious to hear your thoughts!

[algesten]

Hi @thomaseizinger!

Interesting idea! I'm not against it, and I have a couple of thoughts.

First is about the quality of str0m's ICE implementation. I implemented it by reading the RFC, and it certainly seems to serve the purpose we use it for, which is client-server. There is however a blind spot for peer-peer (see #405). @pthatcher has mentioned that libWebRTC ICE implementation is top of the class, and that str0m may be better off porting that.

Related to that, str0m's ICE isn't strictly RFC compliant – it's compatible. A simplifying assumption is that there is one agent instance per peer connection, and there is only a single stream/component to negotiate. A fully compliant implementation would be able to use one agent for multiple sessions supporting many streams. How much this matters for crate-ifying the code, I don't know, but it should at least be a conscious choice that isolating the code out from str0m does not make a full ICE agent.

We should consider the long term goals of such crate. Full RFC compliance? Line-by-line translation of libWebRTC? Nothing more than we have?

I don't think refactoring it out would be that hard. We could keep it in the str0m git project, and release a separate crate. The majority of the work is likely to be the API documentation, README, examples etc. I don't have the time to do that myself right now, but I also don't think it's right to release something half baked.

A first step could be to land some patches to export some APIs from the ice module under a feature flag, with the intention to make a separate crate once we ironed out the details of this ICE implementation.

@morajabi might have thoughts too.

[thomaseizinger]

Thank you for the comment!

Related to that, str0m's ICE isn't strictly RFC compliant – it's compatible. A simplifying assumption is that there is one agent instance per peer connection, and there is only a single stream/component to negotiate.

This is okay for us. We don't even want any streams, I just want to use ICE to hole-punch a connection!

We should consider the long term goals of such crate. Full RFC compliance? Line-by-line translation of libWebRTC? Nothing more than we have?

I'd be pragmatic here and suggest that to begin with, it can be whatever is useful to you and whichever other projects come along. Full RFC compliance could be a novel goal although I'd favor compatibility over it. Most notably for our usecase, only str0m <> str0m needs to be compatible!

I don't think refactoring it out would be that hard. We could keep it in the str0m git project, and release a separate crate. The majority of the work is likely to be the API documentation, README, examples etc. I don't have the time to do that myself right now, but I also don't think it's right to release something half baked.

A first step could be to land some patches to export some APIs from the ice module under a feature flag, with the intention to make a separate crate once we ironed out the details of this ICE implementation.

I've done this in my fork but hit a roadblock when it came to openssl. I think it would be great if we don't have to extract a crate to start with. To make openssl optional, a fair amount of feature-flagging would be necessary. Would you be open to that?

The problem with openssl is that we are going to deploy this to all sorts of targets (Android, iOS, Windows, Linux, MacOS) and requiring openssl as a dependency even we don't actually use it is something I'd like to avoid.

[morajabi]

We should consider the long term goals of such crate

I agree. Based on @thomaseizinger's comment, the goal would be to feature flag openssl (and maybe like how reqwest allows rustls as an option? not sure if it's similar). Seems like doable within the str0m git repo and with feature flags. @algesten what do you think, is this something generally useful longterm?

[algesten]

Oh, interesting that openssl is the problem!

I've done this in my fork but hit a roadblock when it came to openssl. I think it would be great if we don't have to extract a crate to start with.

I agree. Based on @thomaseizinger's comment, the goal would be to feature flag openssl

Because I favor a pure Rust library, my long term goal for str0m is to use rustls as default and openssl as an optional alternative. However rustls isn't ready for that (yet).

To make openssl optional, a fair amount of feature-flagging would be necessary. Would you be open to that?

Yes and no! Let's do feature flags, but before that, let's refactor the crypto stuff out to an abstraction so we don't get feature flags everywhere. openssl is a bit all over the place because it exposes API for both higher level crypto stuff like DTLS as well as lower level AES primitives. In a pure Rust solution, this would require more libraries – some combo of rustls + ring + sha1 + …

From the top of my head we use openssl in some capacity for this:

• X509 certificate generation for DTLS – long term maybe ring + something?
• DTLS – rustls hopefully will have at some point, and we should help out to fix that
• SRTP encryption – two algorithms right now: AES 128 CM + SHA1 or AEAD AES 128 GCM – ring + sha1 crate
• STUN hmac - sha1 crate (already somewhat abstracted)

I think we should refactor out these into a number of traits, make str0m use Box<dyn Trait> in the relevant places. I think the best would be 4 traits corresponding to the above, but I'm open to suggestions. That way we can openssl implementation to a central place, and feature flag proliferation kept to a minimum.

It's time for mod crypto! :)

[thomaseizinger]

Okay thank you for your input! I'll see what I can come up with :)

[pthatcher]

Sorry for being late to the conversation. A few thoughts:

1. I would prefer 1 str0m crate with feature flags to change what dependencies are needed. Maybe feature flags for RTP, SCTP, DTLS, etc.

2. I think having pluggable crypto is a good idea. People often have inflexible needs around crypto dependencies.

3. Implementing the full RFC for ICE is neither necessary nor desirable. Only a subset is needed. The short version of that is: be compliant with the format of the ICE checks and responses, always send responses, don't send checks too fast (probably not more often than every 5ms), but not too slow (probably every 5ms), it's best to honor the controlling side's nomination (although not strictly necessary). It's better to use only one "component", and you can probably ignore everything involving components, freezing, foundations, and DSCP.

4. I still think a "good" ICE implementation should exist outside of libwebrtc (one that works well with mobile clients that can change networks frequently), and str0m might one day be that. But for a lot of people, "plain" ICE (without continual nomination) is probably good enough.

[thomaseizinger]

Just as a heads-up, I've now started work on our connectivity layer that is based on str0m. Tracking issue here: https://github.com/firezone/firezone/issues/2966. PRs incoming :)

[algesten]

Cool project!

[thomaseizinger]

After https://github.com/algesten/str0m/pull/446 is merged, I would start on making openssl pluggable. What do people think about the following:

• Introduce on-by-default openssl feature
• Introduce a Dtls trait that covers the entire API from the Dtls struct
• Make the dtls field in Rtc a Box<dyn Dtls>
• Feature-flag the current Rtc::new constructor to require openssl

This would be minimal disruption for existing users. Some alternatives (not necessarily mutually exclusive with each other):

• introduce a dtls parameter on Rtc::new
• Not enable openssl by default
• Introduce multiple constructors: Rtc::with_openssl

Thoughts?

[algesten]

• Introduce on-by-default openssl feature

Sounds good!

• Introduce a Dtls trait that covers the entire API from the Dtls struct

Yep.

• Make the dtls field in Rtc a Box<dyn Dtls>

Generally in str0m we've favored static dispatch. It probably doesn't make a huge difference, but let's keep doing that to keep it consistent.

See for instance how we handle packetizer where a containing enum implements the same type as the enum variants: https://github.com/lolgesten/str0m/blob/main/src/packet/mod.rs#L239

• Feature-flag the current Rtc::new constructor to require openssl

I think feature flags should enable config options, rather than change behavior. I.e. we will have the feature openssl on by default and Rtc::new() behaves like today. When we add another DTLS impl (and feature flag it), Rtc::new() doesn't change behavior, it still instantiates openssl, but there is a config option RtcConfig::use_dtls(DtlsImpl), or some such.

The reason for this is that feature flags are additive, and although it's unlikely str0m will ever be used in a way that pulls it in with different feature flags enabled, I think it's least surprising if behavior stays the same regardless.

If the user disables openssl, then we should probably remove Rtc::new as well.

Sound ok?

[thomaseizinger]

• Feature-flag the current Rtc::new constructor to require openssl

I think feature flags should enable config options, rather than change behavior. I.e. we will have the feature openssl on by default and Rtc::new() behaves like today. When we add another DTLS impl (and feature flag it), Rtc::new() doesn't change behavior, it still instantiates openssl, but there is a config option RtcConfig::use_dtls(DtlsImpl), or some such.

The reason for this is that feature flags are additive, and although it's unlikely str0m will ever be used in a way that pulls it in with different feature flags enabled, I think it's least surprising if behavior stays the same regardless.

If the user disables openssl, then we should probably remove Rtc::new as well.

Fully agreed! I didn't mean to suggest that feature flags should change behviour.

It is just that in the absence of another DTLS impl, having a ctor like Rtc::with_openssl is kind of odd. Hence, my suggestion is to kick that can down the road and decide on how to make DTLS configurable when a 2nd impl is being added. Until then, Rtc::new would only be available if the openssl feature is activated. With it being enabled by default, nothing changes for existing users.

Say a rustls-backed implementation comes along, we could then remove Rtc::new and add two new ctors. Or we could add a parameter to new. Either way will be a breaking change for users, signaling them that they have to make a decision now which backend to use.

[thomaseizinger]

I've submitted a draft here: https://github.com/algesten/str0m/pull/447, please have a look.

[thomaseizinger]

This is resolved, thank you!

[algesten]

Let's keep this as a place to report interest in separating the ice agent into a separate crate.

[thomaseizinger]

Let's keep this as a place to report interest in separating the ice agent into a separate crate.

Has there been any new interest?

[pera]

Thanks for reopening the ticket @algesten. As I mentioned on Zulip I am looking to use str0m's ICE agent alone. At the moment ice is an internal module that is being exported just as a gift for the intrepid :) but given that the only other maintained Rust implementation is the one from WebRTC.rs, which arguably suffers from an awkward/unergonomic API, it could be interesting if str0m's ICE implementation was turned into an independent crate: this could provide much better visibility of the component and potentially bring more collaboration to the project. It could also help to clarify concerns around intended purpose and what level of API stability one may expect. What do you think?

[thomaseizinger]

FWIW, we are using just str0m's ICE agent very successfully in production and I believe the only reason why we didn't end up creating a separate crate is that we didn't want to take on the additional effort of maintaining another public API, versioning multiple crates, etc.

The ICE agent uses multiple internal types from str0m, like Transmit, Protocol etc. Those would likely need to be moved to 3rd crate. The Rtc instance also uses the same Transmit and importing that from str0m-ice would be a bit awkward.

That being said, I am not opposed to it being separated out into another crate. IMO the best way would be to start with a draft PR and see what unfolds when you actually try to attempt this.

[algesten]

The ICE agent uses multiple internal types from str0m, like Transmit, Protocol etc. Those would likely need to be moved to 3rd crate. The Rtc instance also uses the same Transmit and importing that from str0m-ice would be a bit awkward.

This is maybe the biggest obstacle. It be a bit awkward to push them str0m-ice since it has other WebRTC things in there.

but given that the only other maintained Rust implementation is the one from WebRTC.rs, which arguably suffers from an awkward/unergonomic API, it could be interesting if str0m's ICE implementation was turned into an independent crate: this could provide much better visibility of the component and potentially bring more collaboration to the project.

This is an interesting argument. I haven't considered the visibility and use this could get if it was more visible.

[k0nserv]

but given that the only other maintained Rust implementation is the one from WebRTC.rs, which arguably suffers from an awkward/unergonomic API, it could be interesting if str0m's ICE implementation was turned into an independent crate: this could provide much better visibility of the component and potentially bring more collaboration to the project. It could also help to clarify concerns around intended purpose and what level of API stability one may expect. What do you think?

This is not accurate, there's also librice which is also sans-io. I can't speak to the completeness of that crate, but it does strike me that there is duplicated effort, that maybe, could be unified here.

[algesten]

there's also librice which is also sans-io

Looking at github, that project appears to be abandoned.

[ystreet]

just FYI, librice is not abandoned and I am working on some major features and thus have stopped working on the more mundane maintenance tasks for a little while.

[ystreet]

This is not accurate, there's also librice which is also sans-io. I can't speak to the completeness of that crate, but it does strike me that there is duplicated effort, that maybe, could be unified here.

librice can currently communicate just fine with a webrtc.org (browser) peer using host or server reflexive candidates. UDP has been tested extensively, TCP less so but does successfully acquire a server reflexive candidate from a STUN server that handles TCP connections. Connection check code is written for TCP but has not extensively tested against a peer at this stage.

[algesten]

@ystreet Excellent! Sorry I didn't mean to cast aspersions. I should have been more careful in my wording.

Have you looked at str0m's implementation? Do you see significant differences? A couple of things I can think of:

1. str0m is very focused on WebRTC, and thus ignores large parts of the spec. (details here)
2. it specifically doesn't try to be a full RFC compliant
3. str0m uses the word "nomination" incorrectly

[ystreet]

There aren't really that big of a difference between str0m and librice's internal connection checking mechanisms. However librice does expose a very slightly more general purpose interface for handling ICE. Some of the biggest differences:

• librice does and will care about STUN gathering and TURN server allocations (of course that can be ignored if required, local candidates can be added manually).
• librice is generally aiming more at the RFC definition but will add tuning tweaks necessary for a WebRTC context.
• librice aims to expose a stable C ABI.
• librice can do both trickle-ice and non-trickle-ice.
• librice does not currently support ice-restart but that is on the roadmap.
• librice can do multiple streams and components in those streams and has the necessary logic for unfreezing checks in the correct order. aka, frozen check states are handled.
• librice does not currently support RFC3489 as that's only really relevant for communicating with STUN servers for gathering server reflexive candidates. ICE agents must support the newer RFC5389.
• librice doesn't do global Ta. It can be done by having a single agent with multiple streams for each client if necessary. It's also more relevant to SIP servers rather than a peer-to-peer connection like WebRTC.
• librice does not currently support ice-lite. Also on the roadmap.
• librice does support role conflicts
• librice will conclude processing after nomination. Renomination support is on the roadmap.

[algesten]

Thanks!