Support NetBSD, OpenIndiana, Gentoo Linux, and MacOS. #2803

mstone2001 · 2023-09-25T01:43:18Z

I use XRDP extensively on many different machines I manage, but had a few platforms where I couldn't get it to work. So I went though the effort of getting these platforms to work, and wanted to offer the changes. Almost all of the changes were focused on the os_calls file, and also added some additional startup configs for each platform. I made these changes against the development branch, but I also have a working version against the last official release. I originally had problems with shared memory and xorgxrdp, but looks like the posix changes in the current development branch fixed all of that, great work! (Although OpenIndiana requires some build flags, will open a pull request for that ).

NetBSD:

g_sck_get_peer_cred: Added NetBSDs flavor of this.
g_sleep: NetBSD has a quirk where you can't usleep for more than 1,000,000 microseconds( 1 second ), so created a hybrid sleep, usleep method for netbsd.
service scripts are the same as for freebsd, added checks for this to install.
Note I used pkgsrc to install all required libraries. Also note that building against native BSD pam libraries causes undefined symbols, so I built against openpam.

OpenIndiana:

g_sck_get_peer_cred: Added OpenIndiana's flavor of this.
Have to re-register for SIGCHLD after signal is handled.
Getting strange EINTRs, standard way of handling this is to retry on EINTR. Modified the g_waitpid function to do the automatic retry. This resolved a number of strange errors. This should be a standard way to handle EINTR, so I did not #ifdef it. I tried it on Linux platforms and did not see any issue with it. Also modified a few calls doing a direct waitpid to call the g_waitpid instead.
Modified alarm registration to also handle SIGINT. Again, believe this should not need to be #ifdef, but could be.
While testing, added an extra parameter to the new xwait program to allow the wait time to be configurable.
Build flags required -D_XOPEN_SOURCE=600 so posix calls are allowed. Also need this for xorgxrdp.
Added manifest and method files for SMF for services.
Note this code should work on Solaris, but I do not have access to any Solaris environments so can't test.
Commented out line specifying config file when running Xorg, doesn't work on OpenIndiana.
One issue. I could not get PAM to work on OpenIndiana, I had numerous problems. It looks like enable-pam defaults to yes, and I couldn't find a way to turn it off. So I changed enable-pam to default to no. A second option would be to change this to --disable-pam if you want PAM to be the default, or add an explicit --enable-builtin option but I was getting tired of fighting with autoconf to get it right. I can look at this change some more if it is an issue.

GentooLinux:

Added support for OpenRC and startup files to the build. Added --enable-openrc to install them.

MacOS:

Added plist files to run xrdp and xrdp-sesman. Note these rely on a tool available with the macports install, daemondo, which I find is really useful to get traditional services to play nicely with launchd. But of course, user can just modify the installed files to run however they want. All dependent libraries were installed via macports including build tools ( autoconf, automake, etc. )
Added install hook to comment out Xorg and Xvnc blocks automatically, since I believe the only way possible to run on MacOS is through vnc-any.
Made minor enhancement to facilitate how I run it. You have a nice gateway method where you can configure the sesman to act as a gateway for login. However, when I use this with the any-vnc configuration, I could not find a way to configure it because I need the gateway password to be the password the user entered, and the password to be the password for the VNC server. So added the "ask" option for the gateway username and gateway password so they can use the rdp username and password. Running this way, I don't have to rely on VNC with its low security password, and instead can do real authentication. I can also lock down the VNC server to localhost only. My current VNC server of choice is currently OSXvnc-server( now vine server ). I find the performance is quite good.
I was getting a crash in the PAM code. I found one issue where an object was being freed outside of a null check on it. For some reason this only manifested on MacOS.

I believe I have summarized all of the changes. I also added some additional logging various places while I was identifying the issues. I can also offer to help support these in the future, since I plan to actively use xrdp on these platforms. If you have any questions let me know.

Update xrdp to support netbsd, openindiana, macos, gentoo linux.

Fix call to usleep for NetBSD, where value must be < 1000000

metalefty · 2023-09-25T04:01:38Z

This pull request is large a bit. Could you separate into per operating system?

ziggythehamster · 2023-09-25T05:28:54Z

common/os_calls.c

+
+		        LOG(LOG_LEVEL_TRACE, "g_mk_socket_path() returned 1");


This seems to be indented inconsistently with the preceding statement

ziggythehamster · 2023-09-25T05:32:33Z

configure.ac

 AC_ARG_ENABLE(pam, AS_HELP_STRING([--enable-pam],
-              [Build PAM support (default: yes)]),
-              [], [enable_pam=yes])
+              [Build PAM support (default: no)]),
+              [], [enable_pam=no])


I think that on Linux, this should default to yes

I'll probably change the option to --enable-native then, which will effectively disable pam. Since pam is the default I don't see a reason to have an --enable-pam option, correct me if I'm wrong. Pam works everywhere except OpenIndiana, it may be possible to get it working I can take a second look after the changes are merged.

ziggythehamster · 2023-09-25T05:39:15Z

instfiles/launchdaemons/org.xrdp.xrdp-sesman.plist

+<key>Disabled</key><true/>
+<key>KeepAlive</key><true/>
+</dict>
+</plist>


This may be a dumb question, but why use this other binary instead of just writing a plist that works without the extra dependency? That would allow xrdp to work whether installed by MacPorts or Homebrew

I can test this without the extra dependency, the macports daemondo handles launchctl strangeness, but I have seen many configs that don't use it.

ziggythehamster · 2023-09-25T05:42:53Z

waitforx/waitforx.c

+    {
    const char msg[] = "\n<E>Timed out waiting for RandR outputs\n";
    g_file_write(1, msg, g_strlen(msg));
    exit(XW_STATUS_TIMED_OUT);
+    }


Suggested change

{

const char msg[] = "\n<E>Timed out waiting for RandR outputs\n";

g_file_write(1, msg, g_strlen(msg));

exit(XW_STATUS_TIMED_OUT);

}

{

const char msg[] = "\n<E>Timed out waiting for RandR outputs\n";

g_file_write(1, msg, g_strlen(msg));

exit(XW_STATUS_TIMED_OUT);

}

Yep, I'll have to review the indentation.

ziggythehamster · 2023-09-25T05:49:44Z

xrdp/Makefile.am

+if MACOS
+# must be tab below. Common out Xorg and Xvnc since they don't work on MacOs. 
+install-data-hook:
+	sed -i '' '235,249s/^/;/g' $(DESTDIR)$(sysconfdir)/xrdp/xrdp.ini


this breaks if the file changes in potentially minor ways

I can bullet proof it by using grep to look for the [Xorg] and [Xvnc] blocks to get the line numbers.

ziggythehamster · 2023-09-25T05:50:37Z

xrdp/xrdp_mm.c

+		            if( gw_username != NULL )
+		            {


inconsistent indentation

Yep, apologies, I really messed up indentation.

metalefty · 2023-09-25T06:31:19Z

instfiles/openrc/xrdp-sesman

@@ -0,0 +1,4 @@
+#!/sbin/openrc-run
+
+command="/usr/local/sbin/xrdp-sesman"


This shouldn't be hardcoded to '/usr/local' prefix.

Ahh I see. Yes, I believe there are some prefix replacements available in the other scripts, I'll look at this.

metalefty

Anyway, could you separate your PR into per operating system? Gentoo changes seems easy so it can be merged soon if you separate PR into per operating system.

matt335672 · 2023-09-25T09:07:34Z

@mstone2001 - thanks very much for this. You've clearly put a lot of effort into it.

I agree with @metalefty that a PR per operating system is necessary. This will make the reviewing a lot easier:-

A lot of the current work on MacOS is being done by @unstabler, and he will certainly want to look at the Mac portions.
Many years ago I was adminning a Solaris 10 network. A lot of that consisted of back-porting stuff to build on Solaris 10, so I have some expertise in this area although it is a little dated. I'll be very happy to review and even test the OpenIndiana changes, but having reviewed the Oracle OTN license I don't think I'll be downloading Solaris 11 as it isn't absolutely clear that this is suitable for developing open-source software.

Regarding the C source file formatting, I think all you need to do to fix this is run the ./scripts/run_astyle.sh script from the top level.

mstone2001 · 2023-09-25T14:35:43Z

This pull request is large a bit. Could you separate into per operating system?

Sure I can do that. Should I just fork the repo once for each operating system and submit a pull request for each? Or is there a better way?

mstone2001 · 2023-09-25T14:45:06Z

Anyway, could you separate your PR into per operating system? Gentoo changes seems easy so it can be merged soon if you separate PR into per operating system.

Sure, hopefully will have some time this week to break this up. I'll fork 4 versions and submit a pull request for each, unless you can suggest a better way.

mstone2001 · 2023-09-25T14:48:36Z

@mstone2001 - thanks very much for this. You've clearly put a lot of effort into it.

I agree with @metalefty that a PR per operating system is necessary. This will make the reviewing a lot easier:-

A lot of the current work on MacOS is being done by @unstabler, and he will certainly want to look at the Mac portions.

Many years ago I was adminning a Solaris 10 network. A lot of that consisted of back-porting stuff to build on Solaris 10, so I have some expertise in this area although it is a little dated. I'll be very happy to review and even test the OpenIndiana changes, but having reviewed the Oracle OTN license I don't think I'll be downloading Solaris 11 as it isn't absolutely clear that this is suitable for developing open-source software.

Regarding the C source file formatting, I think all you need to do to fix this is run the ./scripts/run_astyle.sh script from the top level.

Thanks for that, wasn't aware you had a script. Yes, Oracle is not the most open source friendly company, so I just wanted to note that it "should work", since OpenIndiana is just a fork of Solaris 10, and I did my changes looking at the Solaris 10 documentation. I targeted this for OpenIndiana since that is what I use.

metalefty · 2023-09-25T14:56:29Z

Sure, hopefully will have some time this week to break this up. I'll fork 4 versions and submit a pull request for each, unless you can suggest a better way.

You don't need to use 4 forks. You can create branches as many as you want in one fork.

mstone2001 · 2023-09-25T15:11:24Z

Sure, hopefully will have some time this week to break this up. I'll fork 4 versions and submit a pull request for each, unless you can suggest a better way.

You don't need to use 4 forks. You can create branches as many as you want in one fork.

That makes sense, I'll do that then, a branch for each OS, and separate out the changes.

metalefty · 2023-09-27T02:40:10Z

Let me close this. Changes will be separated and raised as new PRs per operating system.

mstone2001 and others added 10 commits September 22, 2023 11:09

Support Additional OS.

7d2643b

Update xrdp to support netbsd, openindiana, macos, gentoo linux.

Update flags for Solaris.

08c049d

Add -w option to waitforx to configure wait time.

7f62ea0

Fix call to usleep for NetBSD, where value must be < 1000000

Fix issues with SIGINT on solaris.

e33d626

Retry SIGINT in waitpid.

2f23edb

Retry on EINTR.

9df32cf

Need to re-register for SIGCHLD on Solaris.

f01853b

Add launchdaemons for macos.

74692f1

Fix bad space in LaunchDaemons directory.

adcfc41

Log an info message instead of error if waitpid returns ECHILD.

32b62c2

ziggythehamster reviewed Sep 25, 2023

View reviewed changes

metalefty reviewed Sep 25, 2023

View reviewed changes

metalefty requested changes Sep 25, 2023

View reviewed changes

metalefty closed this Sep 27, 2023

matt335672 mentioned this pull request Oct 6, 2023

Support NetBsd and OpenIndiana #2811

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support NetBSD, OpenIndiana, Gentoo Linux, and MacOS. #2803

Support NetBSD, OpenIndiana, Gentoo Linux, and MacOS. #2803

mstone2001 commented Sep 25, 2023 •

edited

Loading

metalefty commented Sep 25, 2023

ziggythehamster Sep 25, 2023

ziggythehamster Sep 25, 2023

mstone2001 Sep 25, 2023

ziggythehamster Sep 25, 2023

mstone2001 Sep 25, 2023

ziggythehamster Sep 25, 2023

mstone2001 Sep 25, 2023

ziggythehamster Sep 25, 2023

mstone2001 Sep 25, 2023

ziggythehamster Sep 25, 2023

mstone2001 Sep 25, 2023

metalefty Sep 25, 2023

mstone2001 Sep 25, 2023

metalefty left a comment

matt335672 commented Sep 25, 2023

mstone2001 commented Sep 25, 2023

mstone2001 commented Sep 25, 2023

mstone2001 commented Sep 25, 2023

metalefty commented Sep 25, 2023

mstone2001 commented Sep 25, 2023

metalefty commented Sep 27, 2023

		@@ -0,0 +1,4 @@
		#!/sbin/openrc-run

		command="/usr/local/sbin/xrdp-sesman"

Support NetBSD, OpenIndiana, Gentoo Linux, and MacOS. #2803

Support NetBSD, OpenIndiana, Gentoo Linux, and MacOS. #2803

Conversation

mstone2001 commented Sep 25, 2023 • edited Loading

metalefty commented Sep 25, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

metalefty left a comment

Choose a reason for hiding this comment

matt335672 commented Sep 25, 2023

mstone2001 commented Sep 25, 2023

mstone2001 commented Sep 25, 2023

mstone2001 commented Sep 25, 2023

metalefty commented Sep 25, 2023

mstone2001 commented Sep 25, 2023

metalefty commented Sep 27, 2023

mstone2001 commented Sep 25, 2023 •

edited

Loading