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

feat: enable fwmark (SO_MARK) for outgoing sockets #202

Merged
merged 10 commits into from
Dec 16, 2024

Conversation

sabify
Copy link

@sabify sabify commented Aug 25, 2024

No description provided.

@sabify sabify requested a review from a team as a code owner August 25, 2024 17:41
service/tcp.go Outdated Show resolved Hide resolved
internal/integration_test/integration_test.go Outdated Show resolved Hide resolved
cmd/outline-ss-server/main.go Outdated Show resolved Hide resolved
cmd/outline-ss-server/main.go Outdated Show resolved Hide resolved
@sbruens
Copy link

sbruens commented Sep 4, 2024

I have merged some large refactors we've been working on recently; apologies for the merge conflicts, and thank you for this contribution!

We may want to have some more dialer config options in future, so I suggest maybe a dialer wrapper in the config:

dialer:
  fwmark: ...

/cc @fortuna to weigh in on that

@fortuna
Copy link

fortuna commented Sep 4, 2024

@sabify thanks for the changes to inject the dialer. That's solid. As @sbruens pointed out, we need to pull the changes and I like the idea of putting the fwmark in a dialer config. That could be a place for other settings, like interface binding, routing, enable local network, etc)

@sabify
Copy link
Author

sabify commented Sep 9, 2024

@sbruens @fortuna I just made changes based on the refactored code.

service/tcp.go Outdated Show resolved Hide resolved
service/udp.go Outdated Show resolved Hide resolved
service/udp.go Outdated Show resolved Hide resolved
@fortuna
Copy link

fortuna commented Sep 11, 2024

By the way, for your use case, have you considered using a firewall rule based on the PID?

You can probably do things like:

sudo nft add rule filter output meta pid $PID mark set 0x1234

With iptables you can use cgroups and add the pid to it.

It's also possible to use network namespaces.

@sabify
Copy link
Author

sabify commented Sep 12, 2024

By the way, for your use case, have you considered using a firewall rule based on the PID?

You can probably do things like:

sudo nft add rule filter output meta pid $PID mark set 0x1234

With iptables you can use cgroups and add the pid to it.

It's also possible to use network namespaces.

This already adds too much complexity for even simple routing logic.

I made the changes to be linux-specific but it also opens room for other similar functionality in other platforms like freebsd's SO_USER_COOKIE.

Copy link

@fortuna fortuna left a comment

Choose a reason for hiding this comment

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

Thanks for the changes. Looking good. I just have a few more tweaks.

service/udp_linux.go Outdated Show resolved Hide resolved
service/udp_linux.go Outdated Show resolved Hide resolved
service/tcp_other.go Show resolved Hide resolved
@sabify
Copy link
Author

sabify commented Sep 12, 2024

Changes applied.

cmd/outline-ss-server/main.go Outdated Show resolved Hide resolved
cmd/outline-ss-server/main.go Outdated Show resolved Hide resolved
service/socketopts_linux.go Outdated Show resolved Hide resolved
@sabify
Copy link
Author

sabify commented Sep 28, 2024

@fortuna @sbruens I just gave you maintainer access to my fork and you are able to apply any of your concerns and code styles that fit best with the codebase. I may not be able to keep up with the rapid changes and requests in the codebase and this PR due to time constraints. Sorry for that and appreciate your work to land this feature. Thank you!

@fortuna fortuna requested a review from sbruens November 21, 2024 15:54
@sbruens
Copy link

sbruens commented Dec 16, 2024

@fortuna @sbruens I just gave you maintainer access to my fork and you are able to apply any of your concerns and code styles that fit best with the codebase. I may not be able to keep up with the rapid changes and requests in the codebase and this PR due to time constraints. Sorry for that and appreciate your work to land this feature. Thank you!

Thanks for all your hard work on this @sabify. I finally found some time to pick this up and merge in the changes.

@fortuna PTAL

@sbruens sbruens requested a review from fortuna December 16, 2024 16:39
Copy link

@fortuna fortuna left a comment

Choose a reason for hiding this comment

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

Thanks for updating this! We should probably release it soon after

@sbruens sbruens merged commit 98db5b4 into Jigsaw-Code:master Dec 16, 2024
3 checks passed
@sabify
Copy link
Author

sabify commented Dec 17, 2024

@sbruens Thanks for taking your time to land this feature.

How is it possible to take control of this feature from outline server (CLI and/or GUI)? https://github.com/Jigsaw-Code/outline-server

@sbruens
Copy link

sbruens commented Dec 17, 2024

You can't right now.

There is some work to be done to get this into outline-server, namely:

  • Move outline-server to the new service config format introduced in 9992735 to which you added this dial option. We haven't gotten around to this yet.
  • Add some new API endpoint to actually set/update the dial option(s) so you can call it via the command line.
  • Expose this in the Manager/GUI.

The first 2 seem feasible, but the latter requires more UX research and we may not want to expose such an advanced feature in the Manager anyway.

@sbruens
Copy link

sbruens commented Dec 20, 2024

  • Move outline-server to the new service config format introduced in 9992735 to which you added this dial option. We haven't gotten around to this yet.

Jigsaw-Code/outline-server#1628

62w71st added a commit to JinaVPN/outline-ss-server-v2 that referenced this pull request Dec 29, 2024
feat: enable `fwmark` (`SO_MARK`) for outgoing sockets (Jigsaw-Code#202)
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.

3 participants