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

add support for OpenBSD/pf #1567

Merged
merged 1 commit into from
Sep 19, 2024
Merged

add support for OpenBSD/pf #1567

merged 1 commit into from
Sep 19, 2024

Conversation

ge9
Copy link
Contributor

@ge9 ge9 commented Jun 24, 2024

Added support for OpenBSD/pf.
In OpenBSD's pf, divert-to is used for transparent proxying.
As described in https://man.openbsd.org/pf.conf, TCP destination address can be obtained by getsockname() (like Linux+TPROXY or FreeBSD+ipfw), and UDP destination address can be obtained by special socket options like IP_RECVDSTADDR.
send_to using "fake" source address fails, but using "0.0.0.0:0" instead works. Note that this is not related to IPv4-mapped IPv6 address issue (#1543), because OpenBSD doesn't support IPv4-mapped IPv6 address (https://man.openbsd.org/inet6) and shadowsocks can detect it.
Currently IPv6 is not supported. Also, --features "local-tun" doesn't work.

@@ -625,6 +625,8 @@ where
if bypassed { "bypassed" } else { "proxied" },
err
);
#[cfg(target_os = "openbsd")]
let _ = self.respond_writer.send_to(self.peer_addr, &Address::SocketAddress(SocketAddr::new(std::net::IpAddr::V4(std::net::Ipv4Addr::new(0,0,0,0)), 0)), data).await;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why OpenBSD needs this "faked" destination address? addr is the actual address of client.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think peer_addr is client (local) address and addr is remote address, which is not assigned to sslocal itself, resulting an error (Can't assign requested address, error 49).

Copy link
Collaborator

@zonyitoo zonyitoo Jun 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not true. send_received_respond_packet is for sending back packets received from remote server to local client. So:

  1. peer_addr is the receiver (client)'s address
  2. addr is the sender (remote server)'s address

The code in respond_write.send_to, which is redir/udprelay/mod.rs:UdpRedirInboundWriter::send_to were doing:

  1. Create an UDP socket, bind() to addr
  2. Call sendto to peer_addr

This is the way to make an UDP packet that source address is addr and destination address is peer_addr.

If socket's bind() failed right here (EADDRNOTAVAIL), SO_BINDANY was not set or working as expected. Setting addr to 0.0.0.0:0 is actually let system to allocate one for this socket, which is not the expected behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I mistakenly disable set_bindany() function. If I enabled it, the original send_to worked correctly.
https://man.openbsd.org/getsockopt.2 says that we need divert-reply along with SO_BINDANY, but in my environment, it seems working correctly without divert-reply...🤔

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SO_BINDANY allows socket to bind() an address that is not belongs to local environment. pf should be one of the use cases.

@@ -15,7 +15,7 @@ cfg_if! {
#[allow(dead_code, non_upper_case_globals, non_snake_case, non_camel_case_types)]
#[allow(clippy::useless_transmute, clippy::too_many_arguments, clippy::unnecessary_cast)]
mod pfvar;
} else if #[cfg(target_os = "freebsd")] {
} else if #[cfg(any(target_os = "freebsd", target_os = "openbsd"))] {
#[path = "pfvar_bindgen_freebsd.rs"]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pfvar_bindgen_freebsd.rs is generated on FreeBSD. I suggest you to generate one on OpenBSD with bindgen-cli.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah actually, for bindany, I just reversed the comment-out of the old existing code. Maybe that's not working correctly? I'll look into it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WRONG COMMENT. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I'm not familiar with this tool, but how can I do this? Is there a commandline for generating this?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is pfvar.h in OpenBSD included in release? If no, you should first download it from its source.

Create a header file, that only has #include <pfvar.h>.

$ cargo install bindgen-cli
$ bindgen <your_created_header_file> -o pfvar_bindgen_openbsd.rs

More reference: https://rust-lang.github.io/rust-bindgen/command-line-usage.html

@zonyitoo
Copy link
Collaborator

What is that ktrace.out file?

@ge9
Copy link
Contributor Author

ge9 commented Jun 24, 2024

Ah sorry that's a stack trace...
I'll review and resend this PR sorry

@ge9
Copy link
Contributor Author

ge9 commented Jun 24, 2024

And I may be able to add IPv6 support soon.

@zonyitoo
Copy link
Collaborator

You should install an rustfmt, with:

rustup component add rustfmt
rustup component add rustfmt --toolchain nightly

Run rustfmt with:

cargo +nightly fmt

This project have set some unstable options, which requires nightly Rust.

@ge9
Copy link
Contributor Author

ge9 commented Jun 25, 2024

error[E0425]: cannot find value `IPV6_RECVDSTPORT` in crate `libc`
   --> crates/shadowsocks-service/src/local/redir/udprelay/sys/unix/bsd.rs:272:37
    |
272 |         libc::IPPROTO_IPV6 => libc::IPV6_RECVDSTPORT,
    |                                     ^^^^^^^^^^^^^^^^ not found in `libc`
    |
note: constant `crate::local::redir::sys::unix::pfvar::IPV6_RECVDSTPORT` exists but is inaccessible
   --> crates/shadowsocks-service/src/local/redir/sys/unix/pfvar_bindgen_openbsd.rs:589:1
    |
589 | pub const IPV6_RECVDSTPORT: u32 = 64;
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not accessible

Sorry, I can't figure out how to solve this. Do you have any idea?

I think I successfully created pfvar_bindgen_openbsd and specified it in mod.rs

cfg_if! {
    if #[cfg(any(target_os = "macos",
                 target_os = "ios"))] {
        #[path = "pfvar_bindgen_macos.rs"]
        #[allow(dead_code, non_upper_case_globals, non_snake_case, non_camel_case_types)]
        #[allow(clippy::useless_transmute, clippy::too_many_arguments, clippy::unnecessary_cast)]
        mod pfvar;
    } else if #[cfg(target_os = "freebsd")] {
        #[path = "pfvar_bindgen_freebsd.rs"]
        #[allow(dead_code, non_upper_case_globals, non_snake_case, non_camel_case_types)]
        #[allow(clippy::useless_transmute, clippy::too_many_arguments, clippy::unnecessary_cast)]
        mod pfvar;
    } else if #[cfg(target_os = "openbsd")] {
        #[path = "pfvar_bindgen_openbsd.rs"]
        #[allow(dead_code, non_upper_case_globals, non_snake_case, non_camel_case_types)]
        #[allow(clippy::useless_transmute, clippy::too_many_arguments, clippy::unnecessary_cast)]
        mod pfvar;
    }
}

but it seems not working...

@ge9
Copy link
Contributor Author

ge9 commented Jun 25, 2024

There are already some hard-coded constant values for FreeBSD, like

const IP_DONTFRAG: libc::c_int = 67; // don't fragment packet
. So, maybe pfvar_bindgen_freebsd.rs is not actually used either.
Should we write pub mod pfvar; instead of mod pfvar and use the constants in udprelay/sys/unix/bsd.rs?

@zonyitoo
Copy link
Collaborator

The first error indicates that OpenBSD doesn't have libc::IPV6_RECVDSTPORT in libc. Is that true? OpenBSD doesn't have IPV6_RECVDSTPORT?

If so, IPv6 support on OpenBSD have to be disabled.

Those hard-coded constant values exists because they are not defined in libc, so I copied them directly from source.

I would suggest to add IP_DONTFRAG for FreeBSD, OpenBSD independently instead of using the generated pfvar.rs, because they have no relation.

@ge9
Copy link
Contributor Author

ge9 commented Jun 25, 2024

Sorry, I pasted only the part of errors, but similar error happens for libc::IP_RECVDSTPORT.
These constants do exist in OpenBSD's C library, in netinet/in.h, and so included in generated pfvar_xxx.rs, but they seem not defined in Rust's libc.
The situation is exactly the same for IP_DONTFRAG. It's defined in FreeBSD's netinet/in.h and included in pfvar_xxx.rs, but not defined in Rust's libc.

@zonyitoo
Copy link
Collaborator

zonyitoo commented Jun 25, 2024

It would be great to open an PR to Rust's libc crate and add all those contants.

Before it got merge, it is Ok to just hard-coded.

All these constants are added by users, so if nobody is actually using it / depend on it, it won't exist in libc.

@ge9
Copy link
Contributor Author

ge9 commented Jun 25, 2024

Also, I added support for IPv6, but strangely enough, transparent proxying only works every other time. (I don't know detailed conditions)
I think this is OpenBSD's issue, because shadowsocks with -vvvv doesn't detect any packet.

@zonyitoo
Copy link
Collaborator

zonyitoo commented Jun 26, 2024

Consider update your branch with the latest master's commits.

@ge9
Copy link
Contributor Author

ge9 commented Sep 15, 2024

Now I'm working on completing this pull request.
Should I separate bsd.rs into openbsd.rs and freebsd.rs as you suggested?

@zonyitoo
Copy link
Collaborator

Now I'm working on completing this pull request. Should I separate bsd.rs into openbsd.rs and freebsd.rs as you suggested?

I would prefer the separation. openbsd and freebsd has too many differences.

@ge9
Copy link
Contributor Author

ge9 commented Sep 15, 2024

Is it okay if some part of code is shared (copy-pasted) between two xxxxbsd.rs files? (Maybe something is already shared between macos.rs and bsd.rs)

@zonyitoo
Copy link
Collaborator

Is it okay if some part of code is shared (copy-pasted) between two xxxxbsd.rs files? (Maybe something is already shared between macos.rs and bsd.rs)

Your choice. If those features are actually shared between all BSD systems, it could be shared.

@ge9 ge9 force-pushed the master branch 2 times, most recently from 8da6202 to 15d2415 Compare September 18, 2024 05:30
@ge9
Copy link
Contributor Author

ge9 commented Sep 18, 2024

Now, I

  • separated bsd.rs into freebsd.rs and openbsd.rs
  • defined IP_RECVDSTPORT and IPV6_RECVDSTPORT in the global scope of openbsd.rs (as a temporary fix until these two are supported by libc)
  • added a boolean flag to confirm receiving both the address and port. But this still assumes we receive both the address and port only once respectively (in any order). If this is unacceptable, I'll fix it.
  • removed tun2 support

I think I've done all the remaining works (except for the flag issue)

@zonyitoo
Copy link
Collaborator

Current changes look good to me. But the code are not properly formatted. Run cargo fmt in the root directory and push it again.

@ge9
Copy link
Contributor Author

ge9 commented Sep 18, 2024

I applied rustfmt, except for the pfvar file. Is this okay?

@zonyitoo
Copy link
Collaborator

I applied rustfmt, except for the pfvar file. Is this okay?

It's Ok. But some of them are still not formatted.

@ge9
Copy link
Contributor Author

ge9 commented Sep 19, 2024

Oh, sorry, I forgot "git add"...
I pushed it again. Maybe is it fixed now?

@zonyitoo zonyitoo merged commit e691853 into shadowsocks:master Sep 19, 2024
8 of 9 checks passed
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 this pull request may close these issues.

2 participants