xrdp only binding to ipv6 and not using port settings from ini

gotjoshua commented 6 months ago

Describe the bug

xrdp only binding to ipv6 and not using port settings from ini as the default port setting is passed via cli and trumps settings from the ini

Steps To Reproduce

Steps to reproduce the behavior:

use this gist https://gist.github.com/gotjoshua/3d7c51a76b3d74b213fc86e32cea3935
exclude the systemd section https://gist.github.com/gotjoshua/3d7c51a76b3d74b213fc86e32cea3935#file-configuration-nix-L38-L46
notice that the /etc/xrdp/ files are patched, but the port settings from xrdp.ini is not used

Expected behavior

the port variable ought to be a string and be used to patch the ini (not passed to the cli)
- then one could just use ini syntax directly like services.xrdp.port = "tcp://:3389";
the service should use the /etc/xrdp ini files
by default the service should bind to both ipv4 and 6

Screenshots

without the systemd ExecStart patch it only binds ipv6

the ini file is used, via the direct nix store path (rather than /etc/xrdp/

$ sudo systemctl status xrdp.service
● xrdp.service - xrdp daemon
     Loaded: loaded (/etc/systemd/system/xrdp.service; enabled; preset: enabled)
     Active: active (running) since Wed 2024-04-17 20:37:42 WEST; 3min 36s ago
    Process: 325011 ExecStartPre=/nix/store/dyf3vgbq1lhpks3qrc4i5zjvaj4xzmv8-unit-script-xrdp-pre-start/bin/xrdp-pre-start (code=exited, status=0/SUCCESS)
   Main PID: 325055 (xrdp)
         IP: 0B in, 0B out
      Tasks: 1 (limit: 76411)
     Memory: 1.0M ()
        CPU: 9ms
     CGroup: /system.slice/xrdp.service
             └─325055 /nix/store/01zd6sxkym5a3aj6miwn2ip1id57bn82-xrdp-0.9.25.1/bin/xrdp --nodaemon --port 3389 --config /nix/store/l8i6gamqjdn4h0f8n7b6jl69ks7kg0zp-xrdp.conf/xrdp.ini

Apr 17 20:37:42 gotjosh-icase systemd[1]: Starting xrdp daemon...
Apr 17 20:37:42 gotjosh-icase systemd[1]: Started xrdp daemon.

Notify maintainers

@nagy @pennae @chvp

Metadata

$ nix-shell -p nix-info --run "nix-info -m"
 - system: `"x86_64-linux"`
 - host os: `Linux 6.6.25, NixOS, 24.05 (Uakari), 24.05.20240406.ff0dbd9`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.18.2`
 - channels(gotjosh): `""`
 - nixpkgs: `/nix/store/j10523yhkcc34478azkgcl70yzcx6j2j-source`

SuperSandro2000 commented 6 months ago

I am not a xrdp maintainer

gotjoshua commented 6 months ago

I am not a xrdp maintainer

sorry bout that, i went with the last three commits that touched the file...

found this now https://search.nixos.org/packages?channel=23.11&show=xrdp&from=0&size=50&sort=relevance&type=packages&query=xrdp

Maintainers

Charlotte Van Petegem nixpkgs@cvpetegem.be @chvp are you still an active maintainer?

gotjoshua commented 6 months ago

The section that would need updating is blamed on @lucasew, who is also not a formal maintainer...

chvp commented 6 months ago

I maintain the package, I have not touched the NixOS module yet (and I don't actively use the package anymore).

SuperSandro2000 commented 5 months ago

the port variable ought to be a string and be used to patch the ini (not passed to the cli)

I agree with that but that would require a bigger rewrite of the module. Right now the overrides are for the config files are created in a complicated way and they should be converted to just freeform settings, probably in an ini format.

Could you open a PR for that? Otherwise I would mark this point as wont-fix.

then one could just use ini syntax directly like services.xrdp.port = "tcp://:3389";

please test #315101

the service should use the /etc/xrdp ini files

Yeah true but that falls under my first comment.

4. by default the service should bind to both ipv4 and 6

is also fixed in my PR

NixOS / nixpkgs