guacamole-server icon indicating copy to clipboard operation
guacamole-server copied to clipboard

GUACAMOLE-1981: Add configure argument for systemd user

Open morganwillcock opened this issue 1 year ago • 4 comments

This adds an additional configure argument --with-systemd-user.

I've tried to follow the pattern of how other configure arguments and variables are defined. The indentation style for the changes in Makefile.am is taken from an example in the Automake manual, using an additional space character to indent a multi-line command.

Here are some examples which show the additional configure output:

$ ./configure --help | grep -A2 with-systemd-user
  --with-systemd-user=<username>
                          the user configured in the systemd unit to run guacd
                          [default=daemon]

$ 2>/dev/null ./configure | grep -B1 'Systemd user:'
   Systemd units: no
   Systemd user: no

$ 2>/dev/null ./configure --with-systemd-dir=/etc/systemd/system | grep -B1 'Systemd user:'
   Systemd units: /etc/systemd/system
   Systemd user: daemon

$ 2>/dev/null ./configure --with-systemd-dir=/etc/systemd/system --with-systemd-user=guacd | grep -B1 'Systemd user:'
   Systemd units: /etc/systemd/system
   Systemd user: guacd

morganwillcock avatar Sep 03 '24 20:09 morganwillcock

Thanks @morganwillcock, this looks pretty good. Does it make sense to also make the group configurable, and add that in?

necouchman avatar Sep 03 '24 21:09 necouchman

I'm not sure about doing the same for the group. If no group is specified it should automatically use the default group of the user.

It probably isn't a good idea to try and guess ahead of time which Systemd settings will be useful and then start adding additional configure arguments for each. It could be argued that setting the user with a configure argument is already an abuse of the build system - personally I think this single case is justified to avoid running [a remote access service] as root by default, while still allowing the unit file to be installed without additional changes.

morganwillcock avatar Sep 03 '24 22:09 morganwillcock

@morganwillcock : That's fair - I was just thinking of other software builds that I've seen, and often when a user option is available at build-time a group option is also available. But I'm not feeling particularly strongly that it has to be there.

necouchman avatar Sep 13 '24 21:09 necouchman

I think the problem would be that creating the unit file with a default group name set in it will probably break people's existing deployment mechanisms which wouldn't expect it to be there.

i.e. There may already be something in place to modify the user before starting the service, but there wouldn't necessarily be anything configured to set the group.

Just on the grounds of compatibility with previous releases, it is probably best to omit the group unless there was some compelling case to always set it.

morganwillcock avatar Sep 14 '24 12:09 morganwillcock

On debian bookworm with latest 1.6.0:

août 08 11:59:45 sv1 guacd[1629]: guacd[1629]: INFO:        Creating new client for protocol "rdp"
août 08 11:59:45 sv1 guacd[1629]: Connection ID is "$6cc13365-1523-40ab-aa49-120c318af223"
août 08 11:59:45 sv1 guacd[1629]: guacd[1629]: INFO:        Connection ID is "$6cc13365-1523-40ab-aa49-120c318af223"
août 08 11:59:45 sv1 guacd[14781]: FreeRDP initialization may fail: The current user's home directory ("/usr/sbin") is not writable, but FreeRDP generally requires a writable home directory for storage of configuration files and certificates.
août 08 11:59:45 sv1 guacd[14781]: guacd[14781]: WARNING:        FreeRDP initialization may fail: The current user's home directory ("/usr/sbin") is not writable, but FreeRDP generally requires a writable home directory for storage of configuration files and certificates.
août 08 11:59:45 sv1 guacd[1629]: Connection "$6cc13365-1523-40ab-aa49-120c318af223" removed.
août 08 11:59:45 sv1 guacd[1629]: guacd[1629]: INFO:        Connection "$6cc13365-1523-40ab-aa49-120c318af223" removed.

After switching to guacd user into systemd unit, no more warning, it connect:

août 08 12:04:25 sv1 guacd[15600]: Creating new client for protocol "rdp"
août 08 12:04:25 sv1 guacd[15600]: guacd[15600]: INFO:        Creating new client for protocol "rdp"
août 08 12:04:25 sv1 guacd[15600]: guacd[15600]: INFO:        Connection ID is "$fffc02f2-c80a-4dc0-bbe3-12b242f53a36"
août 08 12:04:25 sv1 guacd[15600]: Connection ID is "$fffc02f2-c80a-4dc0-bbe3-12b242f53a36"
août 08 12:04:25 sv1 guacd[15609]: No security mode specified. Defaulting to security mode negotiation with server.
août 08 12:04:25 sv1 guacd[15609]: guacd[15609]: INFO:        No security mode specified. Defaulting to security mode negotiation with server.
août 08 12:04:25 sv1 guacd[15609]: guacd[15609]: INFO:        Resize method: none
août 08 12:04:25 sv1 guacd[15609]: guacd[15609]: INFO:        No clipboard line-ending normalization specified. Defaulting to preserving the format of all line endings.
août 08 12:04:25 sv1 guacd[15609]: guacd[15609]: INFO:        User "@a5697992-9f08-459f-8fdf-23cc38f1025b" joined connection "$fffc02f2-c80a-4dc0-bbe3-12b242f53a36" (1 users now present)
août 08 12:04:25 sv1 guacd[15609]: guacd[15609]: INFO:        Local system reports 6 processor(s) are available.
août 08 12:04:25 sv1 guacd[15609]: guacd[15609]: INFO:        Graphical updates will be encoded using 6 worker thread(s).

Any news on this PR ?

Best regards

Vebryn avatar Aug 08 '25 10:08 Vebryn

@morganwillcock I'm good with these changes - could you re-base this on the patch branch?

necouchman avatar Aug 08 '25 12:08 necouchman

@necouchman It should now be mergable on the patch branch.

morganwillcock avatar Aug 16 '25 09:08 morganwillcock