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 systemd services (resolves #36, resolves #508) #694

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

simons-public
Copy link

This pull request adds 3 systemd services:

barriers.service - Starts the Barrier server with defaults
[email protected] - Can be used to start a Barrier server instance on another port
[email protected] - Used to start Barrier client instance

The server service will create a Barrier.pem self-signed certificate on first run (all other certificate generation in Barrier happens in the GUI). The server instance (with the @) allows starting with a specific listening port with systemctl start barriers@:<port>' or a specific listening IP/port with systemctl start barriers@:`.

The client service will create a 'TrustedServers.txt' with the specified host on first run. It is started as an instance with systemctl start barrierc@<hostname> or enabled with systemctl enable barrierc@<hostname>. The port can also be specified (i.e. barrierc@<hostname>:<port>).

Since these are for using barrier system-wide via systemd, the configuration file specified is /etc/barrier.conf and the data is stored in /var/db/barrier* instead of /root/.local/share/barrier.

They are installed with CMake in /usr/lib/systemd/system so they can be overridden and customized by creating a copy placed in /etc/systemd/system/.

Copy link
Member

@p12tic p12tic left a comment

Choose a reason for hiding this comment

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

Looks good, but paths need to be updated.

fix hardcoded hostname
@simons-public
Copy link
Author

simons-public commented May 26, 2020

@p12tic Thanks for the review! I pushed the requested change to /var/lib, put all barrier data into a single directory, and got rid of the hardcoded hostname (oops). The other item I added an explanation for.

@p12tic
Copy link
Member

p12tic commented May 27, 2020

Just two tiny nitpicks left, otherwise good. Thank you!

@simons-public
Copy link
Author

@p12tic Got them fixed, thanks again!

grep -oE '([A-Z0-9]{2}:?){20}' > ${FP_DIR}/TrustedServers.txt"

# Main executable
ExecStart=/usr/bin/barrierc --enable-crypto --display ${DISPLAY} --debug ${LOG_LEVEL} --no-daemon %i
Copy link

Choose a reason for hiding this comment

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

Note that if Barrier is installed via Snap, the path to the executable is /snap/bin/barrier.barrierc etc. Are there CMake tokens for this you could use?

Copy link
Author

Choose a reason for hiding this comment

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

I'm not seeing any thing in the CMakeLists.txt, but I'm doing some reading on Snap to see if there's an environment variable or something that can be used to change the path with CMake.

Choose a reason for hiding this comment

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

Snaps have native support for daemon services. Would that help?
https://snapcraft.io/docs/services-and-daemons

@shymega shymega self-requested a review May 28, 2020 14:45
# TrustedServers.txt Directory
Environment=FP_DIR=/var/lib/barrier/barrier@%i/barrier/SSL/Fingerprints
# Ensure the Fingerprints directory exists
ExecStartPre=mkdir -p "${FP_DIR}"
Copy link

Choose a reason for hiding this comment

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

Note that Systemd Exec* commands need to be an absolute path on the current LTS release of Ubuntu (18.04.3) which runs Systemd 237. From that version of the man page ("COMMAND LINES" section):

The command to execute must be an absolute path name. It may contain spaces, but control characters are not allowed.

The current version of Systemd seems to relax this:

If the command is not a full (absolute) path, it will be resolved to a full path using a fixed search path determinted at compilation time. Searched directories include /usr/local/bin/, /usr/bin/, /bin/ on systems using split /usr/bin/ and /bin/ directories, and their sbin/ counterparts on systems using split bin/ and sbin/. It is thus safe to use just the executable name in case of executables located in any of the "standard" directories, and an absolute path must be used in other cases. Using an absolute path is recommended to avoid ambiguity. Hint: this search path may be queried using systemd-path search-binaries-default.

Copy link
Author

Choose a reason for hiding this comment

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

Good point, I'll change it to absolute paths

@detly
Copy link

detly commented May 29, 2020

(Also, if you are concerned with targeting Ubuntu 18.04.3, which you understandably might not be, the ServiceType=exec will be invalid; you will need to use ServiceType=simple.)

@detly
Copy link

detly commented May 29, 2020

I've been playing around with this, running it on an Ubuntu 18.04.3 with Barrier installed via Snap (which might not be a case you want to support). I keep running into #297 . See also the linked issues there, which also mention problems with running it as root.

(Digging into it it seems like this has nothing to do with Snap, since the same problem is reported for a variety of installation sources and also for Synergy/Symless.)

@detly
Copy link

detly commented May 29, 2020

Another note re. Snap (caveat: I know almost nothing about Snap). If barrierc is to run as root, the Snap apparmor policy /var/lib/snapd/apparmor/profiles/snap.barrier.barrierc will probably need to change, in particular access to /root and wherever the Xauthority file is (maybe?).

IIRC Systemd also has the potential to run a service as a dynamic, throwaway user. That might be preferable.

@detly
Copy link

detly commented May 29, 2020

The service still creates /var/lib/barrier/barrier@<server>/barrier/SSL/Fingerprints/ directories fine, but I'm seeing apparmor denials for /root/snap/barrier/83/.local/share/barrier/SSL/Fingerprints/TrustedServers.txt. The service file sets XDG_DATA_HOME=/var/lib/barrier/barrier@%i, but is Barrier using $HOME instead?

Update: it was this commit that made the change, and the same code uses .local/share/barrier instead of .barrier, so I'm a bit baffled by this.

journalctl shows NOTE: trustedServersFilename: /root/snap/barrier/83/.local/share/barrier/SSL/Fingerprints/TrustedServers.txt which comes from here in the code. No idea why it's not actually picking up XDG_DATA_HOME.

@detly
Copy link

detly commented May 29, 2020

Okay, see Snap forum post.

Unrelated: I have also found that I need:

Environment=XAUTHORITY=/var/run/lightdm/root/:0

...and the appropriate lines in /var/lib/snapd/apparmor/profiles/snap.barrier.barrierc to allow access to that. It seems very fragile eg. what if the user is using GDM or KDM or whatever? But I tried years ago to work out a generic "where is the Xauthority file" mechanism with no results 😬.

@simons-public
Copy link
Author

@detly Thanks for all the feedback.

Using simple instead of exec works just fine, I don't mind changing that at all.

As far as #297 goes I think it might have something to do with Xauthority, I'll spin up an Ubuntu VM this weekend to see if I can replicate and figure it out.

The path search happens in DataDirectories.cpp and falls back to using $HOME, so it looks like for some reason it's not reading the $XDG_DATA_HOME environment variable. Maybe Snap is unsetting $XDG_ variables, I'll have to keep reading up on it.

I'm not particularly fond of the idea of running barrier as root anyways, so I'll do some playing around with using those systemd dynamic users. I think the only difficult part of implementing that might be granting the user permissions to the X11 display (possibly using xhost or something).

@detly
Copy link

detly commented May 29, 2020

@simons-public We might have crossed signals there, see my last post. As an addendum, if you find this hack don't get your hopes up, that ACPI stuff has been deprecated for years.

@detly
Copy link

detly commented May 29, 2020

Re. running as root... I wonder if it might be possible to run as whatever user the display manager runs as? And then there might be a way to get XAUTHORITY more easily?

If I get more time to work on this, I'll be sure to sync up with you so we don't duplicate effort.

@simons-public
Copy link
Author

@detly Thanks for the tip on not going down the acpi rabbit hole.

It should be possible to determine which user is running X11 by looking at the owner of the /tmp/.X11-unix/X0 socket, I'm just not sure how portable that path is between distros. I haven't read up yet if there is any way to dynamically set the User= for a systemd service.

@monoprosito
Copy link

So this will be ready soon? It's one of the features I've been looking forward to most for Barrier!

@github-actions
Copy link

This PR has been automatically marked as stale due to inactivity. It will be closed if no further activity occurs. Thank you for your contributions.

@shymega shymega added enhancement New feature or request linux Related to Linux installer/package Related to the Barrier installer and removed no-pr-activity labels Sep 21, 2020
@ryukinix
Copy link

That is a really interesting feature. I have some problems to get client setup working properly automatically at boot time, so a better option I think it would be config as systemd service.

@p12tic
Copy link
Member

p12tic commented Jan 10, 2021

@simons-public @detly Is my understanding correct that the only thing that is left in this PR is that Barrier is unavailable for 1 minute after logout? This is an easy thing to change, the timeout is hardcoded.

@detly
Copy link

detly commented Jan 11, 2021

Hello, doesn't it make sense to offer user based scripts? That way users can start and stop the client/server with the user session.

The problem is that you can't start the user session.

@detly
Copy link

detly commented Jan 11, 2021

@simons-public @detly Is my understanding correct that the only thing that is left in this PR is that Barrier is unavailable for 1 minute after logout? This is an easy thing to change, the timeout is hardcoded.

So the big issue I think is this:

I don't know what you want to do about the Xauthority thing. My addition there is a hack, it will work for LightDM but not others, and I don't know how well it translates between distros.

This was always a problem, Synergy/Barrier was just about the only use case for "sharing" XAUTHORITY between the display manager and another process, so everything I ever made work was a total hack and I never really came up with a reliable method.

@murrant
Copy link

murrant commented Jan 11, 2021

The problem is that you can't start the user session.

I'll give you that it doesn't make as much sense for the server to run only when logged in (although in some cases that might be desirable). But the client absolutely makes sense to start/stop with the user session.

@detly
Copy link

detly commented Jan 12, 2021

But the client absolutely makes sense to start/stop with the user session.

Oh I see what you're saying. Yes, so far I've been using Gnome's startup settings to launch it; a user systemd script might be less DE-dependent.

@fiskhest
Copy link

fiskhest commented Jan 27, 2021

Unrelated: I have also found that I need:

Environment=XAUTHORITY=/var/run/lightdm/root/:0

...and the appropriate lines in /var/lib/snapd/apparmor/profiles/snap.barrier.barrierc to allow access to that. It seems very fragile eg. what if the user is using GDM or KDM or whatever? But I tried years ago to work out a generic "where is the Xauthority file" mechanism with no results grimacing.

I might be misunderstanding something here, but is a xauth info | grep "Authority file" | awk '{print $3}' helpful for this?

@afranke
Copy link

afranke commented Jan 27, 2021

Have you considered adding a systemd socket? That would mean that instead of running Barrier from boot, whether they use it or not, one could have it launched on demand, as soon as a client attempts to connect to the port.

@detly
Copy link

detly commented Jan 29, 2021

I might be misunderstanding something here, but is a xauth info | grep "Authority file" | awk '{print $3}' helpful for this?

@afranke I don't think so, but I haven't tested it. The issue is you need to be able to find the Xauthority file before a user is logged in, as the same user and in the same session as the display manager. So eg. running that command as (say) the lightdm user via su in a framebuffer console (as I once tried) yielded nothing.

@detly
Copy link

detly commented Jan 29, 2021

One possibility would be to:

  • write scripts for each display manager (I think they all have some sort of "run this on start" config, right?) that runs @afranke's @fiskhest's command and writes the results to a file in eg. /var or /etc/defaults or somewhere
  • barrier's systemd unit can then read that file to get the Xauthority file path

...maybe?

@murrant
Copy link

murrant commented Jan 29, 2021

Keep in mind there are different server use cases.
A. start barrier server before login, and keep the connection going. Needs to be a system unit.
B. Login as user, then start barrier server. Stop server after logout (this is the use case I suggest for user systemd unit)

Client should always be the same only start when logged in, so a user unit makes perfect sense.

@Foxboron
Copy link

Foxboron commented May 9, 2021

These services should be changed to exclude any references to the DISPLAY variable and XAUTHORITY. They should be installed as user services towards /usr/lib/systemd/user/.

I think most of the concerns here is a none issue as any X server sessions should be sourcing these variables into the user slice for the services to use. A script similar to the one found on Arch should be on most distros.

λ ~ » cat /etc/X11/xinit/xinitrc.d/50-systemd-user.sh
#!/bin/sh

systemctl --user import-environment DISPLAY XAUTHORITY

if command -v dbus-update-activation-environment >/dev/null 2>&1; then
    dbus-update-activation-environment DISPLAY XAUTHORITY
fi

@detly
Copy link

detly commented May 10, 2021

These services should be changed to exclude any references to the DISPLAY variable and XAUTHORITY. They should be installed as user services towards /usr/lib/systemd/user/.

@Foxboron when do they start if they are installed in this way?

@Foxboron
Copy link

These services should be changed to exclude any references to the DISPLAY variable and XAUTHORITY. They should be installed as user services towards /usr/lib/systemd/user/.

@Foxboron when do they start if they are installed in this way?

You should be capable of specifying PartOf=graphical-session.target or something similar to have it start after X. The network target is not given to the user session so there is no way to say "after network has started" and you'd need to rely on retries for that.

@detly
Copy link

detly commented May 10, 2021

You should be capable of specifying PartOf=graphical-session.target or something similar to have it start after X. The network target is not given to the user session so there is no way to say "after network has started" and you'd need to rely on retries for that.

@Foxboron I'm struggling to find documentation for graphical-session.target, but it looks like this is related to desktop environment sessions, and not the display manager. Is that right?

@Foxboron
Copy link

https://www.freedesktop.org/software/systemd/man/systemd.special.html#Special%20Passive%20User%20Units

It should be ran whenever after a graphical session is started. Not sure I understand the question, is it whether or not the target is activated at the login manager stage?

@detly
Copy link

detly commented May 10, 2021

@Foxboron The problem that remains to be solved in this issue is how to run the Barrier server in a way that doesn't need the user to physically (ie. with a connected keyboard) or automatically log in first ie. how can a user use Barrier to log in to a headless server and start their graphical session. So previous suggestions of having a systemd user unit don't address this.

Now, all of the examples for graphical-session.target seem to involve graphical user login sessions, which makes me think this won't work. But maybe that's just what they chose for the docs, and the target is capable of running under the display manager too?

@Foxboron
Copy link

Then I got the issue. I don't think it's possible.

X needs to know the cookie and display variable. Neither of which you have access too at that stage, you don't even know who the user is going to be. I'm a little bit unsure how the hand-off between the login manager and the X session towards the user work.

I don't think you'll find any solutions here which is going to satisfy people that want cold-boot to work for the barrier server.

@detly
Copy link

detly commented May 10, 2021

X needs to know the cookie and display variable. Neither of which you have access too at that stage, you don't even know who the user is going to be.

I don't understand this. Surely X can know the cookie and display variable for the display manager, because (a) the display manager is running, it must exist and (b) I've written some questionable and DE/DM/distro specific scripts to get that and put it in Barrier's environment. (No one wants to write and maintain thirty or so such scripts for this purpose though, which is why I'd hoped we'd solve it in this issue.) The user is simply whatever user the DM runs as eg. a user named lightdm.

If you're wondering about continuing a Barrier session cleanly across the act of logging in, then yes, that's an issue. But (a) having Barrier disconnect immediately after log in is a much smaller issue than not being able to use it at all and (b) Barrier can automatically reconnect anyway.

Copy link

@kreezxil kreezxil left a comment

Choose a reason for hiding this comment

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

we really need this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request installer/package Related to the Barrier installer linux Related to Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.