support juice_bind_stun to reflect stun requests from unbound client. #248

xicilion · 2024-05-15T06:22:26Z

In order to implement WebRTC Direct PeerB, we need to capture STUN requests from unknown clients and return the client's information. Once PeerB receives this information, it will construct the client's SDP and establish a connection with the client on its own.

xicilion · 2024-06-07T19:38:00Z

change argument at callback function.

achingbrain · 2024-06-13T11:47:13Z

src/conn.c

@@ -247,3 +247,30 @@ int conn_get_addrs(juice_agent_t *agent, addr_record_t *records, size_t size) {

 	return get_mode_entry(agent)->get_addrs_func(agent, records, size);
 }
+
+int juice_bind_stun(const char *bind_address, int local_port, juice_cb_stun_binding_t cb)


Is it possible to unbind from the address to allow closing all open sockets?

Would you just call this again with NULL as the cb argument?

xicilion · 2024-06-13T11:54:33Z

This should be okay, but I need to do some testing to make sure the socket will be closed automatically. I'll add it.

xicilion · 2024-06-13T13:50:11Z

It turned out to be a little more complicated than I expected, but I did it.

achingbrain · 2024-06-13T14:12:45Z

Awesome!

Is the new juice_unbind_stun() function going to unbind all listeners and ports?

I'm thinking of the case where a user specifies multiple listeners running on different ports - they should be stoppable independently of each other.

include/juice/juice.h

xicilion · 2024-06-14T00:29:57Z

I discussed this with @paullouisageneau . Currently, Juice actually only supports one port for mux. After the first mux connects to the specified port, all other mux connections will use this port. So Juice will only have one mux socket at a time.

#247

achingbrain · 2024-06-14T12:21:15Z

So Juice will only have one mux socket at a time

This will prevent people doing things like opening multiple WebRTC-Direct listeners or running more than one js-libp2p node in the same process since they'll try to use the same UDP port for incoming connections.

These aren't very common deployment patterns, though it's not unheard of, and it's done quite often while running unit/integration/interop tests.

It would be good to fix this, though probably not in this PR.

Calls the functions added to libjuice in paullouisageneau/libjuice#248 Exports a `OnUnhandledStunRequest` function that can be passed a callback that will be invoked when an incoming STUN message is received that has no corresponding agent for the ICE ufrag. Closes paullouisageneau#1166

xicilion · 2024-06-14T12:43:42Z

It would be good to fix this

I also hope to achieve this refactoring, but that might be a big project. In the end, it's basically a combination of con_mux and conn_poll.

achingbrain · 2024-07-02T05:59:23Z

@paullouisageneau do you have any feedback on this PR or the functionality it adds?

include/juice/juice.h

src/conn.c

src/conn_mux.c

include/juice/juice.h

src/conn_mux.c

include/juice/juice.h

paullouisageneau · 2024-07-03T08:37:00Z

This will prevent people doing things like opening multiple WebRTC-Direct listeners or running more than one js-libp2p node in the same process since they'll try to use the same UDP port for incoming connections.

These aren't very common deployment patterns, though it's not unheard of, and it's done quite often while running unit/integration/interop tests.

It would be good to fix this, though probably not in this PR.

This is not just a fix, as it affects how the API for this feature is designed. I think it needs to be accounted for, if not implemented, sooner than later. You have to design the interface in accordance now because you can't break compatibility later. This is native library packaged in a few distributions and releasing a new major version is complicated.

xicilion · 2024-07-03T18:59:02Z

@paullouisageneau Thank you very much for your feedback, I will make the changes according to your suggestions as soon as possible.

src/conn_mux.c

include/juice/juice.h

xicilion · 2024-07-11T17:45:11Z

Do you have any more suggestions for this pull request, @paullouisageneau?

achingbrain · 2024-07-15T17:30:31Z

@xicilion this PR still only allows a single muxed port per process, or am I reading it wrong?

@paullouisageneau requested it support multiple ports - #248 (comment)

xicilion · 2024-07-16T14:29:19Z

To support multiple ports, we'll need to almost rewrite the implementation of conn_mux.c. Currently, juice_mux_listen and juice_mux_stop_listen are already compatible with supporting multiple ports in the future. I think this time we can start by only adding the listen feature.

xicilion · 2024-07-30T20:23:49Z

src/conn_mux.c

@@ -295,6 +297,25 @@ static juice_agent_t *lookup_agent(conn_registry_t *registry, char *buf, size_t
 			}
 		}

+		if (registry->cb_mux_incoming) {
+			JLOG_DEBUG("Found STUN request with unknown ICE ufrag");


I found an issue here.

When connecting to the server using a browser and the server closes the peer connection, the browser will continue to send STUN messages, and the incoming event will be triggered again.

According to the RFC document:

[7.1.2](https://www.rfc-editor.org/rfc/rfc8445.html#section-7.1.2). USE-CANDIDATE The controlling agent MUST include the USE-CANDIDATE attribute in order to nominate a candidate pair ([Section 8.1.1](https://www.rfc-editor.org/rfc/rfc8445.html#section-8.1.1)). The controlled agent MUST NOT include the USE-CANDIDATE attribute in a Binding request.

An agent must not include the USE-CANDIDATE attribute when sending a binding request. I tested this in practice, the binding request message received has use_candidate=0. After the connection is closed, the browser continues to send stun messages with use_candidate=1.

I will proceed to test further based on this strategy.

2024-07-31 23:04:44.369 VERB [387904] [rtc::impl::IceTransport::LogCallback@372] juice: conn.c:125: 0 connection left 2024-07-31 23:04:44.369 VERB [387904] [rtc::impl::IceTransport::LogCallback@372] juice: agent.c:192: Destroyed agent 2024-07-31 23:04:44.369 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: conn_mux.c:449: Leaving poll 2024-07-31 23:04:44.369 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: conn_mux.c:408: Receiving datagram 2024-07-31 23:04:44.369 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: conn_mux.c:417: No more datagrams to receive 2024-07-31 23:04:44.369 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: conn_mux.c:447: Entering poll for 60000 ms 2024-07-31 23:04:44.369 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: conn_mux.c:449: Leaving poll 2024-07-31 23:04:44.369 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: conn_mux.c:408: Receiving datagram 2024-07-31 23:04:44.369 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: conn_mux.c:363: Demultiplexing incoming datagram from 127.0.0.1:54664 2024-07-31 23:04:44.369 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: conn_mux.c:255: Looking up agent from address 2024-07-31 23:04:44.369 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:485: Not a STUN message: magic number invalid 2024-07-31 23:04:44.369 INFO [387845] [rtc::impl::IceTransport::LogCallback@372] juice: conn_mux.c:266: Got non-STUN message from unknown source address 2024-07-31 23:04:44.369 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: conn_mux.c:368: Agent not found for incoming datagram, dropping 2024-07-31 23:04:44.369 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: conn_mux.c:408: Receiving datagram 2024-07-31 23:04:44.369 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: conn_mux.c:417: No more datagrams to receive 2024-07-31 23:04:44.369 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: conn_mux.c:447: Entering poll for 60000 ms 2024-07-31 23:04:44.553 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: conn_mux.c:449: Leaving poll 2024-07-31 23:04:44.553 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: conn_mux.c:408: Receiving datagram 2024-07-31 23:04:44.553 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: conn_mux.c:363: Demultiplexing incoming datagram from 127.0.0.1:54664 2024-07-31 23:04:44.553 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: conn_mux.c:255: Looking up agent from address 2024-07-31 23:04:44.553 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: conn_mux.c:270: Looking up agent from STUN message content 2024-07-31 23:04:44.553 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:528: Reading STUN message, class=0x0, method=0x1 2024-07-31 23:04:44.553 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:622: Reading attribute 0x6, length=67 2024-07-31 23:04:44.553 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:700: Reading username 2024-07-31 23:04:44.553 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:707: Got username: libp2p+webrtc+v1/5fhhLR3XJZHejuox:libp2p+webrtc+v1/5fhhLR3XJZHejuox 2024-07-31 23:04:44.553 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:622: Reading attribute 0xC057, length=4 2024-07-31 23:04:44.553 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:1014: Ignoring unknown optional STUN attribute type 0xC057 2024-07-31 23:04:44.553 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:622: Reading attribute 0x802A, length=8 2024-07-31 23:04:44.553 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:906: Found ICE controlling attribute 2024-07-31 23:04:44.553 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:622: Reading attribute 0x25, length=0 2024-07-31 23:04:44.553 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:901: Found use candidate flag 2024-07-31 23:04:44.553 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:622: Reading attribute 0x24, length=4 2024-07-31 23:04:44.553 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:891: Reading priority 2024-07-31 23:04:44.553 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:897: Got priority: 1845501695 2024-07-31 23:04:44.553 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:622: Reading attribute 0x8, length=20 2024-07-31 23:04:44.553 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:711: Reading message integrity 2024-07-31 23:04:44.553 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:622: Reading attribute 0x8028, length=4 2024-07-31 23:04:44.553 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:729: Reading fingerprint 2024-07-31 23:04:44.553 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:745: STUN fingerprint check succeeded 2024-07-31 23:04:44.553 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:545: Finished reading STUN attributes 2024-07-31 23:04:44.553 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: conn_mux.c:368: Agent not found for incoming datagram, dropping

===========================================================================

2024-07-31 23:04:49.570 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: conn_mux.c:408: Receiving datagram 2024-07-31 23:04:49.570 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: conn_mux.c:363: Demultiplexing incoming datagram from 127.0.0.1:54664 2024-07-31 23:04:49.570 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: conn_mux.c:255: Looking up agent from address 2024-07-31 23:04:49.570 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: conn_mux.c:270: Looking up agent from STUN message content 2024-07-31 23:04:49.570 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:528: Reading STUN message, class=0x0, method=0x1 2024-07-31 23:04:49.570 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:622: Reading attribute 0x6, length=67 2024-07-31 23:04:49.570 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:700: Reading username 2024-07-31 23:04:49.570 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:707: Got username: libp2p+webrtc+v1/5fhhLR3XJZHejuox:libp2p+webrtc+v1/5fhhLR3XJZHejuox 2024-07-31 23:04:49.570 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:622: Reading attribute 0xC057, length=4 2024-07-31 23:04:49.570 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:1014: Ignoring unknown optional STUN attribute type 0xC057 2024-07-31 23:04:49.570 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:622: Reading attribute 0x802A, length=8 2024-07-31 23:04:49.570 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:906: Found ICE controlling attribute 2024-07-31 23:04:49.570 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:622: Reading attribute 0x24, length=4 2024-07-31 23:04:49.570 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:891: Reading priority 2024-07-31 23:04:49.570 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:897: Got priority: 1845501695 2024-07-31 23:04:49.570 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:622: Reading attribute 0x8, length=20 2024-07-31 23:04:49.570 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:711: Reading message integrity 2024-07-31 23:04:49.570 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:622: Reading attribute 0x8028, length=4 2024-07-31 23:04:49.570 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:729: Reading fingerprint 2024-07-31 23:04:49.570 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:745: STUN fingerprint check succeeded 2024-07-31 23:04:49.570 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: stun.c:545: Finished reading STUN attributes 2024-07-31 23:04:49.570 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: conn_mux.c:307: Found STUN request with unknown ICE ufrag 2024-07-31 23:04:49.570 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: conn_mux.c:368: Agent not found for incoming datagram, dropping 2024-07-31 23:04:49.570 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: conn_mux.c:408: Receiving datagram 2024-07-31 23:04:49.570 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: conn_mux.c:417: No more datagrams to receive 2024-07-31 23:04:49.570 VERB [387845] [rtc::impl::IceTransport::LogCallback@372] juice: conn_mux.c:447: Entering poll for 60000 ms

After successfully intercepting dozens of messages, the browser still stubbornly sent a binding request message with USE-CANDIDATE set to 0.

I examined numerous attributes in the STUN message, but did not identify any additional characteristics that could be utilized to detect the binding request after the connection is terminated.

@paullouisageneau need your help, do you have any suggestions?

An agent must not include the USE-CANDIDATE attribute when sending a binding request.

I think you misunderstood what USE-CANDIDATE means. The RFC says "The controlled agent MUST NOT include the USE-CANDIDATE attribute in a Binding request". Only the controlling agent includes it to nominate a candidate pair (i.e. to tell the controlled agent which pair to use), otherwise it's not present.

I examined numerous attributes in the STUN message, but did not identify any additional characteristics that could be utilized to detect the binding request after the connection is terminated.

To my knowledge there is no way to know the connection state from STUN attributes, since the ICE layer is unaware of upper layers.

Okay, I have already canceled this commit. I cached the information of the just closed RTCPeerConnection at the business layer for a while, and blocked the same request from being accepted.

SgtPooki · 2024-08-22T14:59:02Z

Is anything else needed here? I'm looking forward to seeing this completed so we can unblock libp2p/js-libp2p#2583

paullouisageneau · 2024-11-22T12:40:09Z

src/conn.h

+	juice_cb_mux_incoming_t cb_mux_incoming;
+	void *mux_incoming_user_ptr;


Since they are specific to the mux implementation, they should be moved to struct registry_impl in conn_mux.c instead, and you can then drop the mux_ prefix in the names.

paullouisageneau · 2024-11-22T13:42:39Z

src/conn.c

@@ -247,3 +247,59 @@ int conn_get_addrs(juice_agent_t *agent, addr_record_t *records, size_t size) {

 	return get_mode_entry(agent)->get_addrs_func(agent, records, size);
 }
+
+int juice_mux_stop_listen(const char *bind_address, int local_port) {


If juice_mux_stop_listen() now has the same bind_address and local_port parameters as juice_mux_listen(), why not simply call juice_mux_listen(bind_address, local_port, NULL) to stop listening? It would also permit multiple ports in the future, while being less clumsy that those two useless parameters.

paullouisageneau · 2024-11-22T14:11:51Z

include/juice/juice.h

@@ -64,6 +64,16 @@ typedef void (*juice_cb_gathering_done_t)(juice_agent_t *agent, void *user_ptr);
 typedef void (*juice_cb_recv_t)(juice_agent_t *agent, const char *data, size_t size,
                                void *user_ptr);

+typedef struct juice_mux_incoming {


Since this does not actually represent incoming connections in the end (for instance in the case of closed connections), I think it should be renamed for clarity. For instance juice_mux_binding_request.

support juice_bind_stun to reflect stun requests from unbound client.

94272e4

xicilion mentioned this pull request Jun 6, 2024

some questions about support for WebRTC Direct paullouisageneau/libdatachannel#1166

Open

feat: Add support for reflecting STUN requests from unbound clients.

59957aa

Return the corresponding error code when binding the port fails.

06a925b

achingbrain mentioned this pull request Jun 12, 2024

feat: WebRTC-Direct support for Node.js libp2p/js-libp2p#2583

Draft

12 tasks

fix compile error on windows.

63651ed

achingbrain reviewed Jun 13, 2024

View reviewed changes

support juice_unbind_stun.

50fa27a

achingbrain reviewed Jun 13, 2024

View reviewed changes

include/juice/juice.h Outdated Show resolved Hide resolved

add user_ptr in callback.

5b818b1

achingbrain mentioned this pull request Jun 14, 2024

feat: add callback for unhandled STUN requests paullouisageneau/libdatachannel#1211

Open

paullouisageneau reviewed Jul 3, 2024

View reviewed changes

feat: Add support for reflecting STUN requests from unbound clients.

f17129c

paullouisageneau reviewed Jul 3, 2024

View reviewed changes

src/conn_mux.c Outdated Show resolved Hide resolved

paullouisageneau reviewed Jul 3, 2024

View reviewed changes

include/juice/juice.h Outdated Show resolved Hide resolved

paullouisageneau reviewed Jul 3, 2024

View reviewed changes

include/juice/juice.h Outdated Show resolved Hide resolved

xicilion added 3 commits July 4, 2024 16:01

feat: Add support for reflecting STUN requests from unbound clients.

20755dc

feat: Add support for reflecting STUN requests from unbound clients.

9ede09b

feat: Add support for reflecting STUN requests from unbound clients.

2c155ae

xicilion commented Jul 30, 2024

View reviewed changes

feat: Add support for reflecting STUN requests from unbound clients.

4a0a896

feat: Add support for reflecting STUN requests from unbound clients.

1fb20f7

achingbrain mentioned this pull request Nov 18, 2024

feat: support setting ICE ufrag and pwd, disabling fingerprint validation and specifying certificates murat-dogan/node-datachannel#256

Draft

paullouisageneau reviewed Nov 22, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support juice_bind_stun to reflect stun requests from unbound client. #248

support juice_bind_stun to reflect stun requests from unbound client. #248

xicilion commented May 15, 2024

xicilion commented Jun 7, 2024

achingbrain Jun 13, 2024 •

edited

Loading

xicilion commented Jun 13, 2024

xicilion commented Jun 13, 2024

achingbrain commented Jun 13, 2024

xicilion commented Jun 14, 2024 •

edited

Loading

achingbrain commented Jun 14, 2024

xicilion commented Jun 14, 2024

achingbrain commented Jul 2, 2024

paullouisageneau commented Jul 3, 2024

xicilion commented Jul 3, 2024

xicilion commented Jul 11, 2024

achingbrain commented Jul 15, 2024

xicilion commented Jul 16, 2024

xicilion Jul 30, 2024

xicilion Jul 31, 2024

xicilion Jul 31, 2024 •

edited

Loading

xicilion Jul 31, 2024 •

edited

Loading

paullouisageneau Aug 10, 2024

xicilion Aug 29, 2024

SgtPooki commented Aug 22, 2024

paullouisageneau Nov 22, 2024

paullouisageneau Nov 22, 2024

paullouisageneau Nov 22, 2024

		juice_cb_mux_incoming_t cb_mux_incoming;
		void *mux_incoming_user_ptr;

support juice_bind_stun to reflect stun requests from unbound client. #248

Are you sure you want to change the base?

support juice_bind_stun to reflect stun requests from unbound client. #248

Conversation

xicilion commented May 15, 2024

xicilion commented Jun 7, 2024

achingbrain Jun 13, 2024 • edited Loading

Choose a reason for hiding this comment

xicilion commented Jun 13, 2024

xicilion commented Jun 13, 2024

achingbrain commented Jun 13, 2024

xicilion commented Jun 14, 2024 • edited Loading

achingbrain commented Jun 14, 2024

xicilion commented Jun 14, 2024

achingbrain commented Jul 2, 2024

paullouisageneau commented Jul 3, 2024

xicilion commented Jul 3, 2024

xicilion commented Jul 11, 2024

achingbrain commented Jul 15, 2024

xicilion commented Jul 16, 2024

xicilion Jul 30, 2024

Choose a reason for hiding this comment

xicilion Jul 31, 2024

Choose a reason for hiding this comment

xicilion Jul 31, 2024 • edited Loading

Choose a reason for hiding this comment

xicilion Jul 31, 2024 • edited Loading

Choose a reason for hiding this comment

paullouisageneau Aug 10, 2024

Choose a reason for hiding this comment

xicilion Aug 29, 2024

Choose a reason for hiding this comment

SgtPooki commented Aug 22, 2024

paullouisageneau Nov 22, 2024

Choose a reason for hiding this comment

paullouisageneau Nov 22, 2024

Choose a reason for hiding this comment

paullouisageneau Nov 22, 2024

Choose a reason for hiding this comment

achingbrain Jun 13, 2024 •

edited

Loading

xicilion commented Jun 14, 2024 •

edited

Loading

xicilion Jul 31, 2024 •

edited

Loading

xicilion Jul 31, 2024 •

edited

Loading