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

Support for multiple listen sockets #27

Closed
Tracked by #34
moritzbuhl opened this issue Mar 14, 2023 · 13 comments · Fixed by #56
Closed
Tracked by #34

Support for multiple listen sockets #27

moritzbuhl opened this issue Mar 14, 2023 · 13 comments · Fixed by #56
Assignees

Comments

@moritzbuhl
Copy link

moritzbuhl commented Mar 14, 2023

I tried following this setup: https://rosenpass.eu/#start

Server: 192.168.2.107/24

rp exchange 192.168.2.107-sec dev rosenpass0 listen 192.168.2.107:9999 peer 192.168.2.106-pub allowed-ips 192.168.123.0/24

Client: 192.168.2.106/24

rp exchange 192.168.2.106-sec dev rosenpass0 peer 192.168.2.107-pub endpoint 192.168.2.107:9999 allowed-ips 192.168.123.0/24

However, I am getting this output from the client:

[2023-03-14T20:59:57Z ERROR rosenpass] unexpected error after processing 0 messages: Address family not supported by protocol family (os error 47) disabled backtrace
[2023-03-14T20:59:57Z ERROR rosenpass] reinitializing networking in 0.01! 10 tries left.
[2023-03-14T20:59:57Z ERROR rosenpass] unexpected error after processing 0 messages: Address family not supported by protocol family (os error 47) disabled backtrace
[2023-03-14T20:59:57Z ERROR rosenpass] reinitializing networking in 0.02! 9 tries left.
[2023-03-14T20:59:58Z ERROR rosenpass] unexpected error after processing 0 messages: Address family not supported by protocol family (os error 47) disabled backtrace
[2023-03-14T20:59:58Z ERROR rosenpass] reinitializing networking in 0.04! 8 tries left.
[2023-03-14T20:59:59Z ERROR rosenpass] unexpected error after processing 0 messages: Address family not supported by protocol family (os error 47) disabled backtrace
[2023-03-14T20:59:59Z ERROR rosenpass] reinitializing networking in 0.08! 7 tries left.
...

To my understanding this is because the socket the rosenpass binary is using for sendig is somehow bound for IPv6:

 98150 rosenpass CALL  socket(AF_INET6,0x8002,0)
 98150 rosenpass RET   socket 3
 98150 rosenpass CALL  bind(3,0x7f7ffffd19a8,28)
 98150 rosenpass STRU  struct sockaddr { AF_INET6, [::]:0 }
 98150 rosenpass RET   bind 0
...
 98150 rosenpass CALL  sendto(3,0x7f7ffffd24b0,0x444,0x400,0x7f7ffffd1a30,0x10)
 98150 rosenpass STRU  struct sockaddr { AF_INET, 192.168.2.107:9999 }
 98150 rosenpass RET   sendto -1 errno 47 Address family not supported by protocol family
 98150 rosenpass CALL  write(2,0x878b524c500,0xd9)
 98150 rosenpass GIO   fd 2 wrote 217 bytes
       "\^[[0m\^[[38;5;8m[\^[[0m2023-03-14T20:59:57Z \^[[0m\^[[1m\^[[31mERROR\
        \^[[0m rosenpass\^[[0m\^[[38;5;8m]\^[[0m unexpected error after proces\
        sing 0 messages: Address family not supported by protocol family (os e\
        rror 47) disabled backtrace
       "
 98150 rosenpass RET   write 217/0xd9
 98150 rosenpass CALL  write(2,0x878b524c500,0x8c)
 98150 rosenpass GIO   fd 2 wrote 140 bytes
       "\^[[0m\^[[38;5;8m[\^[[0m2023-03-14T20:59:57Z \^[[0m\^[[1m\^[[31mERROR\
        \^[[0m rosenpass\^[[0m\^[[38;5;8m]\^[[0m reinitializing networking in \
        0.01! 10 tries left.
       "
This unwrap_or makes the socket IPv6 since `listen` is not set for the client.

To fix this, the client should bind a v4 socket too and depending on endpoint it should use that socket for sending.
Alternatively it could just try the v4 socket in case the v6 one failed.

I will look into implementing that but I have little knowledge on rust.

https://github.com/rosenpass/rosenpass/blob/main/src/main.rs#L356
Adding listen for the client makes it work with IPv4.


Because the client is trying to send via an IPv6 socket, I tried just using IPv6 to connect to the server:

Server:

rp exchange 192.168.2.107-sec dev rosenpass0 listen fe80::fce1:bbff:fed1:3a06%vio0:9999 peer 192.168.2.106-pub allowed-ips 192.168.123.0/24

Client:

rp exchange 192.168.2.106-sec dev rosenpass0 peer 192.168.2.107-pub endpoint fe80::fce1:bbff:fed1:3a06%vio0:9999 allowed-ips 192.168.123.0/24

Now everything works, I can assign IP addresses to the wg/rosenpass interfaces and ping through them.

I had to adjust the rp script sligthly to make it work with OpenBSD, but I did not add any changes to the rosenpass binary:
https://github.com/moritzbuhl/mystuff/tree/master/net/rosenpass

@wucke13
Copy link

wucke13 commented Mar 16, 2023

This unwrap_or makes the socket IPv6 since listen is not set for the client.
To fix this, the client should bind a v4 socket too and depending on endpoint it should use that socket for sending.
Alternatively it could just try the v4 socket in case the v6 one failed.

Hi, I have a couple of questions for you (since you surely are more experienced with OpenBSD than I am):

  1. Do I understand correctly that OpenBSD does not provide a single-socket way of listening on all interfaces, both IPv4 and IPv6?
  2. If so, the correct way would be adding two sockets, one for all interfaces running IPv4, one for all interfaces running IPv6?
  3. On linux (at least with only the Rust std library) I expect spawning of to sockets listening on the same port to cause problems if one of them is for 0.0.0.0 and the other for [0::0]:0. Therefore, we would have to introduce target architecture dependent conditional compilation to either acquire one (all OSs / OpenBSD and ???) or two (OpenBSD and ???) sockets listening on all interfaces?

I will validate 3 quickly. Yup, this fails on Linux:

use std::net::UdpSocket;

fn main() {
    let ipv4 = "0.0.0.0:1024";
    let ipv6 = ":::1024";
    let v4socket = UdpSocket::bind(ipv4).unwrap();
    let v6socket = UdpSocket::bind(ipv6).unwrap();
}

/cc @emilengler @koraa

@wucke13
Copy link

wucke13 commented Mar 16, 2023

Windows apparently also defaults to not being dual stack. So I see two options: either we introduce dynamic detection of dual-stack and on-demand adding of a second socket, or we just default to either IPv4 or IPv6 all-interface listening, while informing the user on how to change the default with an explicit listen flag.

@moritzbuhl
Copy link
Author

  1. OpenBSD does not support that. I was told it boils down to this: https://datatracker.ietf.org/doc/html/draft-itojun-v6ops-v4mapped-harmful-02
  2. Yes. For example here: https://github.com/openbsd/src/blob/master/usr.sbin/ntpd/client.c#L155
  3. I think in Linux there is a 'net.ipv6.bindv6only' sysctl that might affect this. I was told in the past MacOS did not support this single address thing either. It might have changed tho.

@koraa
Copy link
Member

koraa commented Mar 16, 2023

We're currently not testing under openbsd; we should start to add the BSDs and Mac as supported platforms. The rp script also has some compatibility issues…

@koraa koraa changed the title IPv4 based exchange on OpenBSD failing OpenBSD support Mar 16, 2023
@clausecker
Copy link

Ah, this might also explain why I couldn't get a tunnel to work on FreeBSD. It likely has the same behaviour. Treating this as a blocking issue for finishing the port.

@moritzbuhl
Copy link
Author

I think the following makes it work for me but I am not familiar with rust and as discussed it should break Linux.
I was thinking to make sock4 = sock in case the bind for sock4 fails. But I don't know how to express that.

Index: src/main.rs
--- src/main.rs.orig
+++ src/main.rs
@@ -211,6 +211,7 @@ pub enum Verbosity {
 pub struct AppServer {
     pub crypt: CryptoServer,
     pub sock: UdpSocket,
+    pub sock4: UdpSocket,
     pub peers: Vec<AppPeer>,
     pub verbosity: Verbosity,
 }
@@ -449,6 +450,7 @@ impl AppServer {
         Ok(Self {
             crypt: CryptoServer::new(sk, pk),
             sock: UdpSocket::bind(addr)?,
+            sock4: UdpSocket::bind("0.0.0.0:0")?,
             peers: Vec::new(),
             verbosity,
         })
@@ -520,7 +522,10 @@ impl AppServer {
                     let p = $peer.get_app(self);
                     if let Some(addr) = p.tx_addr {
                         let len = $fn()?;
-                        self.sock.send_to(&tx[..len], addr)?;
+                        match self.sock.send_to(&tx[..len], addr) {
+                            Ok(x) => { x },
+                            Err(_) => { self.sock4.send_to(&tx[..len], addr)? },
+                        };
                     }
                     Ok(())
                 })
@@ -547,7 +552,10 @@ impl AppServer {
                                 info!("error processing incoming message from {:?}: {:?} {}", addr, e, e.backtrace())),

                         Ok(HandleMsgResult { resp: Some(len), .. }) => {
-                            self.sock.send_to(&tx[0..len], addr)?
+                            match self.sock.send_to(&tx[..len], addr) {
+                                Ok(x) => { x },
+                                Err(_) => { self.sock4.send_to(&tx[..len], addr)? },
+                            };
                         },

                         Ok(HandleMsgResult { exchanged_with: Some(p), .. }) => {
@@ -634,7 +642,17 @@ impl AppServer {
         }
         self.sock
             .set_read_timeout(Some(Duration::from_secs_f64(timeout)))?;
+        self.sock4
+            .set_read_timeout(Some(Duration::from_secs_f64(timeout)))?;
         match self.sock.recv_from(buf) {
+            Ok(x) => return Ok(Some(x)),
+            Err(e) => match e.kind() {
+                ErrorKind::WouldBlock => {},
+                ErrorKind::TimedOut => {},
+                _ => return Err(anyhow::Error::new(e)),
+            },
+        };
+        match self.sock4.recv_from(buf) {
             Ok(x) => Ok(Some(x)),
             Err(e) => match e.kind() {
                 ErrorKind::WouldBlock => Ok(None),

@koraa koraa changed the title OpenBSD support BSD support Mar 22, 2023
@koraa koraa changed the title BSD support Support for multiple listen sockets Mar 22, 2023
@koraa
Copy link
Member

koraa commented Mar 22, 2023

@moritzbuhl Thank you for your code contribution! Would you mind opening a PR? I don't think that code is what we will arrive, but it is a starting point for development.

I think the correct thing to do is to have support for multiple listen <ADDR> command line parameters and assume listen "[0::0]:0" listen "0.0.0.0:0" by default. This would also a much broader range of configurations. We will need a Vec<UdpSocket> instead of just a single one and the code will need to keep track of which socket a packet came on.

I will also create a "BSD Support" meta ticket to track what needs to be done to fully support BSD.

@koraa koraa mentioned this issue Mar 22, 2023
4 tasks
@cvengler
Copy link
Member

Hello, long-term OpenBSD Desktop user here. I have been learning network development on OpenBSD, so I am quite familiar with that issue :D

The problem lies within the fact, that OpenBSDs IPv6 sockets ONLY support IPv6, not the IPv4 backwards compatibility. Unfortunately, most applications use the latter approach to do a dual stack. The OpenBSD way would be to create two sockets and set the IPv6 socket to IPV6_V6ONLY in order to disable backwards compatibility. This is not necessary on OpenBSD, but on all other systems. Otherwise, a listen (or bind?) would fail.

Years ago, I was working on an OpenBSD-exclusive privilege-separated POP3-Daemon using dual-stack. The code looked like this:

static void
net_sockets_init(uint16_t port)
{
	struct sockaddr_in	sa4;
	struct sockaddr_in6	sa6;

	if ((net_in4 = socket(AF_INET, SOCK_STREAM, 0)) == -1)
		fatal("socket AF_INET");
	if ((net_in6 = socket(AF_INET6, SOCK_STREAM, 0)) == -1)
		fatal("socket AF_INET6");

        /* Portable code would require a setsockopt(2) here */

	if (net_fd_nonblock(net_in4) == -1)
		fatal("net_fd_nonblock AF_INET");
	if (net_fd_nonblock(net_in6) == -1)
		fatal("net_fd_nonblock AF_INET6");

	/* Steps related for bind(2). */
	memset(&sa4, 0, sizeof(sa4));
	sa4.sin_family = AF_INET;
	sa4.sin_port = htons(port);
	sa4.sin_addr.s_addr = INADDR_ANY;
	memset(&sa6, 0, sizeof(sa6));
	sa6.sin6_family = AF_INET6;
	sa6.sin6_port = htons(port);
	sa6.sin6_addr = in6addr_any;

	if (bind(net_in4, (struct sockaddr *)&sa4, sizeof(sa4)) == -1)
		fatal("bind AF_INET");
	if (bind(net_in6, (struct sockaddr *)&sa6, sizeof(sa6)) == -1)
		fatal("bind AF_INET6");

	if (listen(net_in4, 128) == -1)
		fatal("listen AF_INET");
	if (listen(net_in6, 128) == -1)
		fatal("listen AF_INET6");
}

Do I understand correctly that OpenBSD does not provide a single-socket way of listening on all interfaces, both IPv4 and IPv6?

Yes

If so, the correct way would be adding two sockets, one for all interfaces running IPv4, one for all interfaces running IPv6?

Yes, but in order to achieve compatibility with other systems, you have to make the IPv6 socket IPv6 only (default on OpenBSD). It requires a setsockopt(2) IPV6_V6ONLY to 1.

On linux (at least with only the Rust std library) I expect spawning of to sockets listening on the same port to cause problems if one of them is for 0.0.0.0 and the other for [0::0]:0. Therefore, we would have to introduce target architecture dependent conditional compilation to either acquire one (all OSs / OpenBSD and ???) or two (OpenBSD and ???) sockets listening on all interfaces?

I would not introduce conditional compilation. The dual socket approach that OpenBSD uses (one V6-only and one V4-only socket) works cross platform. Sure, it might add a tiny overhead, but having a common codebase across architectures is probably better.

@wucke13
Copy link

wucke13 commented Mar 28, 2023

I see some problems with the multi-socket approach, foremost that we use sock.recv_from together with a read timeout (lets call this t_r when reading from the sockets. This only works with one socket.

  • Sure there is epoll etc., but that is not cross-platform.
  • We could spawn a thread per socket, but that feels ugly and will complicate a lot of things.
  • mio (and async Rust in general) solve the issue easily and cross-platform, but I'm not particular fond of adding 100+ transitive dependencies.
    • if async, maybe smol could help keeping the stack smoler smaller?
  • Best thing that comes to my mind: setting the recv_from timeout to a super low value t_r' and iterating over all sockets once, after sleeping the thread for t_r - t_r' time. This would increase latencies of the protocol, but it would masquerade the current usage to peers better, which after all might be desirable. Feels like a not-so-nice tradeoff as well.

@koraa what is your call on this? The way I see it, our best option is either platform-dependent code (clean, but a high maintenance burden), async for the frontend or just a single listen address per rosenpass process and no happy non-v4-v6-dualstack users.

@clausecker
Copy link

Sure there is epoll etc., but that is not cross-platform.

There are select and poll as cross-platform options. You don't have enough sockets for the limitations of these interfaces to become important.

@cvengler
Copy link
Member

I agree with @clausecker. poll(2) is more than sufficient for two sockets.

@wucke13
Copy link

wucke13 commented Mar 30, 2023

I looked a little more into this. The correct way IMHO would be usin mio. That is a little more effort, but does not blow our dependency quiet as much.

wucke13 pushed a commit that referenced this issue Apr 15, 2023
- adds TOML based configuation files
  - with example configuratios in config-examples
- reimplments arcane CLI argument parser as automaton
- adds a new CLI focused arround configuration files
- moves all file utility stuff from `main.rs` to `util.rs`
- moves all AppServer stuff to dedicated `app_server.rs`
- add mio for multi-listen-socket support (should fix #27)
- consistency: rename private to secret
wucke13 pushed a commit that referenced this issue Apr 15, 2023
- adds TOML based configuation files
  - with example configuratios in config-examples
- reimplments arcane CLI argument parser as automaton
- adds a new CLI focused arround configuration files
- moves all file utility stuff from `main.rs` to `util.rs`
- moves all AppServer stuff to dedicated `app_server.rs`
- add mio for multi-listen-socket support (should fix #27)
- consistency: rename private to secret
wucke13 pushed a commit that referenced this issue Apr 17, 2023
- adds TOML based configuation files
  - with example configuratios in config-examples
- reimplments arcane CLI argument parser as automaton
- adds a new CLI focused arround configuration files
- moves all file utility stuff from `main.rs` to `util.rs`
- moves all AppServer stuff to dedicated `app_server.rs`
- add mio for multi-listen-socket support (should fix #27)
- consistency: rename private to secret
wucke13 pushed a commit that referenced this issue May 5, 2023
- adds TOML based configuation files
  - with example configuratios in config-examples
- reimplments arcane CLI argument parser as automaton
- adds a new CLI focused arround configuration files
- moves all file utility stuff from `main.rs` to `util.rs`
- moves all AppServer stuff to dedicated `app_server.rs`
- add mio for multi-listen-socket support (should fix #27)
- consistency: rename private to secret
wucke13 pushed a commit that referenced this issue May 5, 2023
- adds TOML based configuation files
  - with example configuratios in config-examples
- reimplments arcane CLI argument parser as automaton
- adds a new CLI focused arround configuration files
- moves all file utility stuff from `main.rs` to `util.rs`
- moves all AppServer stuff to dedicated `app_server.rs`
- add mio for multi-listen-socket support (should fix #27)
- consistency: rename private to secret
koraa pushed a commit that referenced this issue May 12, 2023
- adds TOML based configuation files
  - with example configuratios in config-examples
- reimplments arcane CLI argument parser as automaton
- adds a new CLI focused arround configuration files
- moves all file utility stuff from `main.rs` to `util.rs`
- moves all AppServer stuff to dedicated `app_server.rs`
- add mio for multi-listen-socket support (should fix #27)
- consistency: rename private to secret
@koraa koraa closed this as completed in #56 May 22, 2023
koraa pushed a commit that referenced this issue May 22, 2023
- adds TOML based configuation files
  - with example configuratios in config-examples
- reimplments arcane CLI argument parser as automaton
- adds a new CLI focused arround configuration files
- moves all file utility stuff from `main.rs` to `util.rs`
- moves all AppServer stuff to dedicated `app_server.rs`
- add mio for multi-listen-socket support (should fix #27)
- consistency: rename private to secret
@clausecker
Copy link

Looking forwards to the next release! Will try another attempt at packaging then.

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 a pull request may close this issue.

5 participants