`set_system_proxy` not working as expected when using SystemD

demarcush commented 3 months ago

Operating system

Linux (Arch deriv.)

System version

latest

Installation type

Original sing-box Command Line (From AUR)

If you are using a graphical client, please provide the version of the client.

No response

Version

sing-box version 1.9.3

Environment: go1.22.4 linux/amd64
Tags: with_gvisor,with_quic,with_wireguard,with_utls,with_reality_server,with_clash_api,with_ech,with_acme,with_dhcp
Revision: 4619f2280afffb2e045f499511e69dc116476f23
CGO: enabled

Description

When declaring set_system_proxy in the config file, default system unit file fails, whether running as user root or sing-box.

Providing a user unit file is the first solution that came to my mind, cause I suspect this would also be the case under GNOME too.

Reproduction

Install sing-box with default systemd unit files in /release/config under KDE6
Declare set_system_proxy for mixed inbound.
sudo systemctl enable --now sing-box.service
As of version 1.9.0, no problems due to the fact that said option was bogus prior to https://github.com/SagerNet/sing-box/commit/69dc87aa6d27a24ace82d4d786f882544e093b27
As of 1.9.1 and later, encountering error and panic.

Logs

sing-box[24514]: FATAL[0000] start service: initialize inbound/mixed[0]: set system proxy: execute (/usr/bin/kwriteconfig6) kwriteconfig6 --file kioslaverc --group Proxy Settings --key ProxyType 1: exit status 2
systemd[1]: sing-box.service: Main process exited, code=exited, status=1/FAILURE

Supporter

[ ] I am a sponsor

Integrity requirements

[X] I confirm that I have read the documentation, understand the meaning of all the configuration items I wrote, and did not pile up seemingly useful options or default values.
[X] I confirm that I have provided the server and client configuration files and process that can be reproduced locally, instead of a complicated client configuration file that has been stripped of sensitive data.
[X] I confirm that I have provided the simplest configuration that can be used to reproduce the error I reported, instead of depending on remote servers, TUN, graphical interface clients, or other closed-source software.
[X] I confirm that I have provided the complete configuration files and logs, rather than just providing parts I think are useful out of confidence in my own intelligence.

demarcush commented 3 months ago

I Will tinker around to see if I can get it to work with a user unit. Will open a PR if successful.

lgjint commented 3 months ago

I Will tinker around to see if I can get it to work with a user unit. Will open a PR if successful.

This is not a bug. In your KDE6, there was no kwriteconfig5, so actually the KDE system proxy was not successfully set in the previous version, leading you to mistakenly believe that the KDE system proxy can be set by systemd unit.

And please specify your linux distro before asking because normally if you run sing-box by systemd unit it will tell you that set system proxy: unable to set as root. https://github.com/SagerNet/sing-box/blob/a18400503366a46445cf5cefea83a2f2e6ddd134/common/settings/proxy_linux.go#L130-L138

kwriteconfig and dbus-send should be run as current user in order to set up the KDE system proxy correctly.

If you really want to use systemd, then just create a user unit by yourself.

demarcush commented 3 months ago

The installation is from the AUR and there's this line in the pkgbuild:

    sed -i "/^\[Service\]$/a StateDirectory=$pkgname"    release/config/$pkgname.service
    sed -i "/^\[Service\]$/a StateDirectory=$pkgname-%i" release/config/$pkgname@.service
    sed -i "/^\[Service\]$/a User=$pkgname"              release/config/$pkgname*.service

Which makes the system service run as sing-box user instead of root (current .service files omit User=). So with no access to dbus under sing-box user, I simply encountered that error instead of the defined unable to set as root. Still, I think my pull request (which includes DynamicUser=true) is something that should be considered. And yes, I'm aware that sing-box drops privileges during start on its own.

lgjint commented 3 months ago

The installation is from the AUR and there's this line in the pkgbuild:
    sed -i "/^\[Service\]$/a StateDirectory=$pkgname"    release/config/$pkgname.service
    sed -i "/^\[Service\]$/a StateDirectory=$pkgname-%i" release/config/$pkgname@.service
    sed -i "/^\[Service\]$/a User=$pkgname"              release/config/$pkgname*.service
Which makes the system service run as sing-box user instead of root (current .service files omit User=). So with no access to dbus under sing-box user, I simply encountered that error instead of the defined unable to set as root. Still, I think my pull request (which includes DynamicUser=true) is something that should be considered. And yes, I'm aware that sing-box drops privileges during start on its own.

dbus-send requires the current user's DBUS_SESSION_BUS_ADDRESS, and kwriteconfig requires the current user's XDG_CONFIG_HOME, which I think should these enviroment variables can only be obtained through the user unit.

Your PR has too many commits, you need to remove the useless commits and merge them together.

And I don't know whether the systemd versions of other distributions are new enough to support these security options.

demarcush commented 3 months ago

I'll turn the PR to draft until I merge them. Will test the new service files under Debian Stable. Does that count?

demarcush commented 3 months ago

@nekohasekai: What do you think?

demarcush commented 3 months ago

[x] you need to remove the useless commits

demarcush commented 3 months ago

[x] Both system and user units tested on Debian 12 with systemd version 252.22

SagerNet / sing-box