How to correctly handle `Transmit::source` and `Candidate::base`? #453

thomaseizinger · 2024-01-27T01:40:48Z

In the connectivity library I built on top of str0m's IceAgent, I copied its model of Transmit and return those messages to the upper layer. I also handle TURN allocations and channels for the user.

Here comes the catch: When str0m wants to make a STUN binding request from one of my relay candidates, it sets Transmit::source to the allocation address. This is a result of Candidate::base being set to the same value as the actual addr in the constructor:

str0m/src/ice/candidate.rs

Line 216 in 30ba27e

Some(addr),

This is actually useful! It allows me to cross-reference Transmit::source with the list of TURN servers and if I detect that I have a TURN server for this IP, I know that I need to wrap the Transmit::contents in a channel data message to Transmit::destination.

The issue gets trickier for server-reflexive addresses. Here, Transmit::source is set to the server-reflexive address. Without keeping a mapping myself, I therefore don't know which local interface this packet needs to be sent from. My first thought was that we should simply change the Candidate constructors to accept an additional base parameter that designates the local socket the data was received on that generated this candidate. However, this will break the case for relay candidates because I now have no way of knowing, whether the data needs to be sent through a relay.

So far, I've come up with the following solutions and I don't like any of them:

Extend the Candidate constructors with base and set it correctly BUT also change Transmit to always use addr. Then, whenever processing a Transmit from str0m, one needs to iterate the local candidates, find the corresponding one and check the base for server-reflexive address candidates to know which socket to use. For host candidates, addr works just fine and for relay candidates, one can look up the mapping similar as to today.
Extend the Candidate constructors with base and split Transmit::source into two fields:
- final_source / perceived_source: The source that the remote should see. This is only useful for relayed candidates.
- local_source: The local socket to use when sending the data.
Extend only the server-reflexive Candidate constructors with base and keep returning the actual allocation socket on the relay as base for relay candidates.

The last one is the easiest but also the most confusing, especially because it is hard to understand and the type system doesn't guide you. Perhaps a variation of the last one would be to model this explicitly by introducing a Source enum that is:

pub enum Source {
	/// The data should be sent from the specified, local socket.
	Local(SocketAddr),
	/// The data should be sent from the specified allocation on the relay.
	Allocation(SocketAddr),
}

The text was updated successfully, but these errors were encountered:

thomaseizinger · 2024-01-27T02:04:00Z

The last one is the easiest but also the most confusing, especially because it is hard to understand and the type system doesn't guide you. Perhaps a variation of the last one would be to model this explicitly by introducing a Source enum that is:
pub enum Source {
	/// The data should be sent from the specified, local socket.
	Local(SocketAddr),
	/// The data should be sent from the specified allocation on the relay.
	Allocation(SocketAddr),
}

I just tried this and it is an absolute rabbit hole haha. See (non-compiling) commit: thomaseizinger@c205c02

algesten · 2024-01-27T08:56:43Z

I'm trying to get my head around it. It was some time since I had the spec clear in my head :D

algesten · 2024-01-27T09:02:05Z

The issue gets trickier for server-reflexive addresses. Here, Transmit::source is set to the server-reflexive address. Without keeping a mapping myself, I therefore don't know which local interface this packet needs to be sent from.

My expectation when I wrote this was that such knowledge would be outside of str0m. Similar problems could occur on multihomed servers where it's important to know which NIC to use, or indeed keeping a TURN server connection active (where I believe you need some additional timers, creds etc).

I.e. str0m has a Candidate with some source address. To the user it says "I want to send from this source address, I don't care how", the user of str0m then needs themselves to keep track of "What additional things do I need to know to send from this source address".

algesten · 2024-01-27T09:16:38Z

One way forward here could potentially be to make a similar mechanism that we have for UserExtensionValues https://github.com/algesten/str0m/blob/main/src/rtp/ext.rs#L916-L921

I'm imagining something like pub user_data: UserData that would let the user store random data on the Candidate. This data would be opaque to str0m.

thomaseizinger · 2024-01-27T15:39:58Z

One way forward here could potentially be to make a similar mechanism that we have for UserExtensionValues https://github.com/algesten/str0m/blob/main/src/rtp/ext.rs#L916-L921

I'm imagining something like pub user_data: UserData that would let the user store random data on the Candidate. This data would be opaque to str0m.

I think the difference here is that it is not opaque user data at all. The Candidate already has a field for base that is used for peer-reflexive addresses. We just don't use it (properly) for all the others.

str0m has all the knowledge about this because I am giving it the local socket address that data was received on when it parses the reply out of a packet. It would be nice if it could properly retain this information and give it back to me "correctly" in source.

thomaseizinger · 2024-01-27T16:21:21Z

str0m has all the knowledge about this because I am giving it the local socket address that data was received on when it parses the reply out of a packet. It would be nice if it could properly retain this information and give it back to me "correctly" in source.

This would also avoid the edge case of: str0m told me to send from an address that we don't have a local candidate for.

In order to keep the mapping, I have to iterate all local candidates on each Transmit to check what kind it is. That is also somewhat inefficient :(

I totally get the "should be handled outside of str0m" point for TURN etc. In this particular case, it just appears that we can do better :)

algesten · 2024-01-27T16:29:06Z

I still don't think I follow. The difference between a relay and srflx is that we use a TURN server or a STUN server. From my perspective (which I guess is kinda "birds eye"), these are pretty much the same. I.e.

Both mean some source address, that needs to be routed via som other server.

And in both cases, it's up to the user of str0m to know how to route the source address via a STUN or TURN server.

algesten · 2024-01-27T16:34:18Z

Here's my mental model:

                                this                    
                              address                   
                                 │                      
                                 │                      
          srflx         ┌──────┐ ▼              ┌──────┐
    ┌ ─ ─ ─ ─ ─ ─ ─ ─ ─ │ STUN │─ ─ ─ ─ ─ ─ ─ ─ │Client│
                        └──────┘                └──────┘
    │                                                   
┌───────┐                       this                    
│ str0m │                     address                   
└───────┘                        │                      
    │                            │                      
                        ┌──────┐ ▼              ┌──────┐
    └ ─ ─ ─ ─ ─ ─ ─ ─ ─ │ TURN │─ ─ ─ ─ ─ ─ ─ ─ │Client│
          relay         └──────┘                └──────┘

thomaseizinger · 2024-01-27T17:05:37Z

I still don't think I follow. The difference between a relay and srflx is that we use a TURN server or a STUN server. From my perspective (which I guess is kinda "birds eye"), these are pretty much the same. I.e.

Both mean some source address, that needs to be routed via som other server.

And in both cases, it's up to the user of str0m to know how to route the source address via a STUN or TURN server.

Conceptually yes. Perhaps I need to improve my design but in practise the two differ because Transmit.src will give me an address I've never seen before for srflx candidates.

To generically handle and transform Transmit.src, I currently do:

Check if I have a TURN server with the IP of Transmit.src
- If yes, wrap it in a channel-data message
- If no, hand out Transmit unmodified

The last bit breaks for srflx candidates because then the lookup for a local socket with this address fails. Additionally, there is no heuristic like "check the IP" like I can do for TURN.

What I could do is keep a mapping every time I discover a srflx candidate on which socket I received that data on. That is doable but since Candidate already has a base, I figured Transmit could just tell me the correct one when it populates source from Candidate!

algesten · 2024-01-27T17:45:12Z

Base: The base of a server reflexive candidate is the host candidate
from which it was derived. A host candidate is also said to have
a base, equal to that candidate itself. Similarly, the base of a
relayed candidate is that candidate itself.

So technically base should be different to addr for server reflexive (and maybe peer reflexive?). And then Transmit needs to use the base for source and the addr for destination. Is that what you're saying maybe?

algesten · 2024-01-27T17:49:49Z

No that's not it either… 😂

algesten · 2024-01-27T17:52:06Z

        let local = pair.local_candidate(&self.local_candidates);
        let proto = local.proto();
        let local_addr = local.base();
        let remote = pair.remote_candidate(&self.remote_candidates);
        let remote_addr = remote.addr();

Maybe you want remote.base() to be like:

        let via = remote.base();

thomaseizinger · 2024-01-27T20:34:58Z

Base: The base of a server reflexive candidate is the host candidate
from which it was derived. A host candidate is also said to have
a base, equal to that candidate itself. Similarly, the base of a
relayed candidate is that candidate itself.

So technically base should be different to addr for server reflexive (and maybe peer reflexive?). And then Transmit needs to use the base for source and the addr for destination. Is that what you're saying maybe?

Is this quoting the RFC? Or where is that text coming from? :)

Because that sounds like what I want!

algesten · 2024-01-27T20:46:30Z

That's the RFC. https://datatracker.ietf.org/doc/html/rfc8445#page-15

thomaseizinger · 2024-01-27T22:01:38Z

That's the RFC. https://datatracker.ietf.org/doc/html/rfc8445#page-15

Okay, well that means we need to fix str0m to adhere to that, right? The ctor for srflx candidates needs another parameter to pass in the base which should be the address the packet was received on.

algesten · 2024-01-27T22:20:19Z

Yes I think the PR I want would be

Passing a base to server_reflexive ctor
Add an optional via field to Transmit
Set via to base for server reflexive and peer reflexive

thomaseizinger · 2024-01-27T23:36:48Z

Yes I think the PR I want would be

1. Passing a `base` to server_reflexive ctor

2. Add an optional `via` field to Transmit

3. Set via to base for server reflexive and peer reflexive

Why not just keep setting source to base in Transmit?

Edit: Or differently asked, if we change base, source will automatically change with it so why introduce via?

algesten · 2024-01-28T07:32:56Z

We shouldn't change the semantics of source.

See my mental model above. To be consistent, source is always the IP the peer see as the source.

The local NIC (or STUN/TURN server IP), or whatever you want the base to be, is extra information.

We don't really need
it internally because it is a concern for the user of str0m. And it makes Transmit more complicated than it needs to be, but also base has a clear meaning in ICE rfc.

Maybe we should provide a lookup of candidate on address instead. That way we can keep Transmit simple.

thomaseizinger · 2024-01-29T04:33:20Z

We shouldn't change the semantics of source.

See my mental model above. To be consistent, source is always the IP the peer see as the source.

Playing devil's advocate: What value does the consistency bring if it overall makes the API harder to use? What would I use the information of source for if not for locating which socket I should send the data from? If we end up adding via, I will simply ignore source unless via is not set.

The mental model I have is:

A relay is an additional "remote" socket that I "bound" to. For the other party, it makes no difference sending data to a relay's allocation or to my socket. In both cases, they just send UDP traffic so it is "just another socket".
All other traffic comes from a local socket.
As the application, I have no idea what my server-reflexive address is. What I do know are the addresses that I bound to locally and "remotely".
Consequently, source would make the most sense if it would always correspond to one of my sockets, either one I bound to locally or one I bound to remotely.

This corresponds directly to how the RFC defines base if I am not mistaken.

Maybe we should provide a lookup of candidate on address instead. That way we can keep Transmit simple.

It keeps Transmit simple but makes the API more cumbersome to use because now we have to teach users that they need to perform a lookup to correctly process Transmit.

…ckets (#3411) It turns out that we need to do some post-processing of the `Transmit.source` attribute from `str0m`. In its current state, `str0m` may also set that to a server-reflexive address which is **not** a local socket. There is a longer discussion around this here: algesten/str0m#453. This depends on an unmerged PR in `str0m`: algesten/str0m#455.

algesten · 2024-02-01T19:54:10Z

                                                        
           NAT                                          
            │                                           
                                                        
     ───────┼──────▶ ┌───────┐                          
         discover    │ STUN  │                          
     ◀──────┼─────── └───────┘                          
                                                        
    ┌───────┼──────────────────────────────────────┐    
    │        srflx                                 │    
    │       │                                      │    
┌───────┐                                      ┌───────┐
│ str0m │   │                                  │ Peer  │
└───────┘                                      └───────┘
    │       │                                      │    
                      ┌───────┐                    │    
    └ ─ ─ ─ ┼ ─ ─ ─ ─ │ TURN  │────────────────────┘    
                      └───────┘relay                    
            │                                           
                                                        
            │

Is this a better mental model?

thomaseizinger · 2024-02-04T02:53:24Z

Is this a better mental model?

I think so? (Assuming I am getting all the important bits you want to convey).

This was referenced Jan 30, 2024

feat: explicitly set base of server-reflexive addresses #455

Merged

fix(connection): only emit Transmit.src that correspond to local sockets firezone/firezone#3411

Merged

algesten closed this as completed in #455 Feb 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

How to correctly handle `Transmit::source` and `Candidate::base`? #453

How to correctly handle `Transmit::source` and `Candidate::base`? #453

thomaseizinger commented Jan 27, 2024

thomaseizinger commented Jan 27, 2024

algesten commented Jan 27, 2024

algesten commented Jan 27, 2024

algesten commented Jan 27, 2024

thomaseizinger commented Jan 27, 2024

thomaseizinger commented Jan 27, 2024

algesten commented Jan 27, 2024

algesten commented Jan 27, 2024 •

edited

Loading

thomaseizinger commented Jan 27, 2024 •

edited

Loading

algesten commented Jan 27, 2024

algesten commented Jan 27, 2024

algesten commented Jan 27, 2024

thomaseizinger commented Jan 27, 2024 •

edited

Loading

algesten commented Jan 27, 2024

thomaseizinger commented Jan 27, 2024

algesten commented Jan 27, 2024

thomaseizinger commented Jan 27, 2024 •

edited

Loading

algesten commented Jan 28, 2024

thomaseizinger commented Jan 29, 2024 •

edited

Loading

algesten commented Feb 1, 2024 •

edited

Loading

thomaseizinger commented Feb 4, 2024

How to correctly handle Transmit::source and Candidate::base? #453

How to correctly handle Transmit::source and Candidate::base? #453

Comments

thomaseizinger commented Jan 27, 2024

thomaseizinger commented Jan 27, 2024

algesten commented Jan 27, 2024

algesten commented Jan 27, 2024

algesten commented Jan 27, 2024

thomaseizinger commented Jan 27, 2024

thomaseizinger commented Jan 27, 2024

algesten commented Jan 27, 2024

algesten commented Jan 27, 2024 • edited Loading

thomaseizinger commented Jan 27, 2024 • edited Loading

algesten commented Jan 27, 2024

algesten commented Jan 27, 2024

algesten commented Jan 27, 2024

thomaseizinger commented Jan 27, 2024 • edited Loading

algesten commented Jan 27, 2024

thomaseizinger commented Jan 27, 2024

algesten commented Jan 27, 2024

thomaseizinger commented Jan 27, 2024 • edited Loading

algesten commented Jan 28, 2024

thomaseizinger commented Jan 29, 2024 • edited Loading

algesten commented Feb 1, 2024 • edited Loading

thomaseizinger commented Feb 4, 2024

How to correctly handle `Transmit::source` and `Candidate::base`? #453

How to correctly handle `Transmit::source` and `Candidate::base`? #453

algesten commented Jan 27, 2024 •

edited

Loading

thomaseizinger commented Jan 27, 2024 •

edited

Loading

thomaseizinger commented Jan 27, 2024 •

edited

Loading

thomaseizinger commented Jan 27, 2024 •

edited

Loading

thomaseizinger commented Jan 29, 2024 •

edited

Loading

algesten commented Feb 1, 2024 •

edited

Loading