Add syscall filtering to xrdp systemd unit

[iskunk]

This makes use of systemd's system-call filtering facility to add (Linux) seccomp support to the xrdp daemon in a non-invasive way. This implicitly also enables NoNewPrivileges. (See SystemCallFilter= in systemd.exec(5))

This marks wide swaths of the Linux kernel API as off-limits, significantly reducing the attack surface of the system available to a baddie who manages to compromise the program. If a prohibited syscall is used, the program is immediately killed with SIGSYS.

I've put together this syscall filter through some trial and error, and am able to perform a full login/logout cycle without issue. If there are any unusual codepaths used by xrdp, then those should be tested to ensure the filter covers everything needed.

Some notes on testing the filter:

• When the process uses a prohibited syscall, in addition to the SIGSYS, a message like the following will appear in syslog:

  May 21 00:17:10 darkstar kernel: [ 4789.893316] audit: type=1326 audit(1684642630.723:258): auid=4294967295 uid=124 gid=130 ses=4294967295 subj=unconfined pid=28046 comm="xrdp" exe="/usr/sbin/xrdp" sig=31 arch=c000003e syscall=99 compat=0 ip=0x7fb9ee316f9b code=0x80000000
  

• The syscall=99 bit is salient. The syscall number can be looked up using the scmp_sys_resolver(1) utility:

  $ scmp_sys_resolver 99
  sysinfo
  

• The syscall name can either be added to the filter directly, or if you want to use systemd's syscall sets, the sets can be listed with systemd-analyze syscall-filter. (I generally made use of the sets to keep things simple, save for a couple oddball cases.)
• Alternately, you can test without the threat of SIGSYS by using SystemCallLog=. Comment out SystemCallFilter=, and append its arguments to SystemCallLog=~@default. Then watch syslog for messages like the one above (only they will show sig=0).

Note: This approach to adding seccomp support is easy, but of course it is limited not only to systemd, but also to Linux. It should be considered a stopgap solution at best. A better one would be to add support in the code, making use of seccomp on Linux, pledge on OpenBSD, and similar facilities on other systems. That is what OpenSSH does (see the sandbox-*.c source files).

[matt335672]

Hi @iskunk

This looks like a good idea to me.

As you say, it's Linux only for now, and systemd at that. While it's bedding in however, having something which can be changed by a developer or user to work around an issue seems like a good compromise. In the longer term, we could target Linux in general and FreeBSD (Capsicum?) provided our architectures are up to it. That's a big step however, compared to this one.

I'll need to do a bit of testing before I merge this:-

• To make sure our proxies (VNC, NeutrinoRDP) work OK. It seems unlikely they won't, but it needs to be tested.
• To check our codecs are OK. At the moment we've just got RemoteFX in production. Later though, if there's scope for hardware acceleration within xrdp, the syscall lists may need to be modified.

@metalefty, @jsorg71, @Nexarian - I'd appreciate your thoughts on this PR.

[iskunk]

Sounds good. A common theme between seccomp, AppArmor, SELinux, and other such security layers is that they can break the application, often in subtle ways, if they are not properly vetted. (When they are, then their effect will be unnoticeable to the user.)

It would be helpful to have some kind of automated coverage test suite that exercises the various features and operating modes, so that this kind of thing can be validated/iterated without needing manual intervention. Xrdp definitely has some odd nooks and crannies (e.g. the FUSE drive-sharing stuff) that I've had a hard time getting to work for the purpose of proofing my AppArmor profiles.

[matt335672]

We've got automated tests which we're building up for the libraries. A lot of the application however just isn't suited to this at the moment sadly.

[metalefty]

Overall, I like the idea to make xrdp secure. The same idea can be applied to FreeBSD using Capsicum but unfortunately I'm not familiar with Capsicum.

[matt335672]

I've had a test of this on a few more configurations using Ubuntu 22.04:-

1. The NeutrinoRDP shim isn't working, I get the following error in syslog:-

   $ sudo grep '"xrdp"' /var/log/syslog
   Jun  6 11:21:49 xrdp-test kernel: [ 2013.208199] audit: type=1326 audit(1686046909.223:163): auid=4294967295 uid=0 gid=0 ses=4294967295 subj=unconfined pid=29795 comm="xrdp" exe="/home/mjb/xrdp/xrdp/.libs/xrdp" sig=31 arch=c000003e syscall=16 compat=0 ip=0x7f25f7f1aaff code=0x80000000
   

   syscall=16 maps to ioctl on my system. Adding that seems to fix things.

2. When running an encoder, the following happens at the end of a session:-

   Jun  6 11:42:15 xrdp-test kernel: [ 3239.557002] audit: type=1326 audit(1686048135.576:268): auid=4294967295 uid=0 gid=0 ses=4294967295 subj=unconfined pid=36083 comm="xrdp" exe="/home/mjb/xrdp/xrdp/.libs/xrdp" sig=31 arch=c000003e syscall=28 compat=0 ip=0x7fb92511ed2b code=0x80000000
   

   this is traced back to an madvise call in pthread_create().

So I had to add ioctl madvise to the list of system calls to cover these two cases.

Alternatively we could just use @System-service instead of the existing list, I believe.

I'm out of time today, but I also need to look at CentOS 7. That doesn't support systemd analyse syscall-filter, but supports the directives in the unit file.

[iskunk]

I figured there were a couple edge cases still hiding in there. Thanks for testing this out.

I considered @system-service, but wanted to put in a bit more effort to get a narrower filter. If the current approach ends up being overly fragile or maintenance-heavy, then that would be a reasonable fallback. (It's too bad that ioctl and madvise aren't covered by some other focused syscall set. Those aren't exactly exotic calls.)

It would be so helpful to be able to run through those configurations automatically. Would it improve the testing situation much to have a headless, scriptable RDP testing client based on rdpy?

[matt335672]

Agreed about the narrower filter. I'll have a look at CentOS 7 (which is still taking updates) and see what happens there.

Also agreed about the automated testing. However, it's a massive amount of effort to not only set up, but also to maintain. I can't see the project in its current state being able to bear that kind of cost.

[matt335672]

Following command works well to display the audit messages (Ubuntu 22.04):-

sudo journalctl  _AUDIT_TYPE_NAME=SECCOMP _COMM=xrdp

I'm also wondering if in the main xrdp process (not the forked one) we can catch the SIGCHLD and see if the process was terminated with SIGSYS. If so, we could at least log that the seccomp filter had been triggered. I'll have a play around.

[matt335672]

Out of time today (and I haven't looked at CentOS 7 yet), but here's a log from a modified xrdp instance tripping the filter:-

[2023-06-07T12:16:31.601+0100] [ERROR] [process_pending_sigchld_events(xrdp_listen.c:872)] Child 35041 was terminated with seccomp violation

then, feeding the PID into journalctl:-

$ sudo journalctl --no-pager  _AUDIT_TYPE_NAME=SECCOMP _PID=35041 -S today
Jun 07 12:16:31 xrdp-test audit[35041]: SECCOMP auid=4294967295 uid=0 gid=0 ses=4294967295 subj=unconfined pid=35041 comm="xrdp" exe="/home/mjb/xrdp/xrdp/.libs/xrdp" sig=31 arch=c000003e syscall=28 compat=0 ip=0x7f7fab91ed2b code=0x80000000

which gives the syscall of 28 (madvise)

I'll post my patch later.

[matt335672]

Well, so much for CentOS 7 testing:-

https://bugzilla.redhat.com/show_bug.cgi?id=1546063

In other words, this change has no effect on CentOS 7 as systemd doesn't support SECCOMP.

[iskunk]

Also agreed about the automated testing. However, it's a massive amount of effort to not only set up, but also to maintain. I can't see the project in its current state being able to bear that kind of cost.

Bummer. If there is a way to to minimize the time needed to set up and run the tests you've been doing, then that would also be good. The AppArmor profiles will need the same sort of vetting.

Good to hear that the new directives don't cause breakage on CentOS 7. Folks who want the protection there could use Bubblewrap (bwrap(1)) or the like.

I've updated and rebased the PR commit for freshness.

[matt335672]

I think we're good to merge this now. We're logging SECCOMP failures causing xrdp to exit (see #2719), so any problems with existing systems should be relatively easy to find.