Running alternative shell with parameters

[mds99]

Hello.
i'm running xrdp 0.9.6 on linux.
i'm trying to start not a default wm, but pass an alternative shell as a parameter.
if i send just a script name, it runs ok from the sesman.
if i setup to pass the program with parameters, a default WM starts and i see something like this:
[20201002-01:18:03] [CORE ] error starting program /opt/launch/startup "Chrome" for user kioskmin - pid 12833
where /opt/launch/startup is my script, "Chrome" is a parameter and kioskmin - userid i started the session with.

i've found this error message in sessman/session.c:

546:                if (s->program != 0)
547:                {
548:                    if (s->program[0] != 0)
549:                    {
550:                        g_execlp3(s->program, s->program, 0);
551:                        log_message(LOG_LEVEL_ALWAYS,
552:                                    "error starting program %s for user %s - pid %d",
553:                                    s->program, s->username, g_getpid());
554:                    }
555:                }

And i also found the error with strace:

strace.12914:execve("/opt/launch/startup \"Chrome\"", ["/opt/launch/startup \"Chrome\""], [/* 12 vars */]) = -1 ENOENT (No such file or directory)

Looks like second parameter for execve is passed as is, and there's no support for the additional paramenters to pass.
Is it possible to prepare the second parameter as execve() expecting for *argv?

I did my best to look at session.c to understand if there's a way to pass paramenters string better, with no success.

[matt335672]

Hi@mds99,

you're quite right that the parameter is passed as-is with no interpretation.

I've taken the liberty of reformatting your code snippet for v0.9.6 above with line numbers so I can explain this.

The parameter is passed to g_execlp3() at line 550. This code is in common/os_calls.c if you want to follow it through.

The simplest fix is to go via a shim script which makes the transition for you :

#!/bin/bash

exec /opt/launch/startup Chrome

[mds99]

Thanks!
I'm doing this way right now, but i need a way to transfer some parameters from a remote client to adapt the kiosk page accordingly. All i have is a userid, but i need more.
to support new pages it's required to create a lot of scripts like this.
If i will have a way to pass the parameters, i could set it up on client side, as most of the changes a basically some SPA URLSs i need to deliver to people on remote access. Right now they have 5-6 applications available, but there will be more, that's what i'm asking.

i guess i've seen a patch like this earlier in issues, will it be included some day may be?

[mds99]

I've found this change in some fork, but i could not see it in upstream.
https://github.com/d-yacenko/xrdp/blob/devel/sesman/session.c

                }
                if (s->program != 0)
                {
                    if (s->program[0] != 0)
                    {
                        char *params = strchr(s->program, ' ');
                        if(params)
                        {
                            *params = '\0';
                            params++;
                        }
                        if (params && strlen(s->program) > 0 &&  strlen(params) > 0)
                        { 
                           g_execlp3(s->program, "-c", params);
                        }
                        else
                        { 
                           g_execlp3(s->program, s->program, 0);
                        }
                        log_message(LOG_LEVEL_ALWAYS,
                                    "error starting program %s for user %s - pid %d",
                                    s->program, s->username, g_getpid());
                    }
                }

[matt335672]

Looks like it's slipped through the cracks in #1273

I agree that having this functionality on the client side would be useful in some use cases.

@metalefty - I'm had a look at #1273 and I can see no reason not to merge it, apart from a stylistic use of strchr() rather than g_strchr(). Are you happy for me to work with @d-yacenko to get this merged?

[matt335672]

Hmm - I've had a closer look at #1273, and as it is, it has very significant limitations. It's going to work with bash scripts and that's about it. Anything else is going to need to understand '-c' and parse the args themselves which is hardly satisfactory.

@mds99 - I take it you haven't tried this code? Are you in a position to try patches, or are you working with a packaged version of XRDP?

[metalefty]

Maybe #1273 is a wron PR number? Which should I see?

[matt335672]

My bad - sorry. Try #1270

[mds99]

Hmm - I've had a closer look at #1273, and as it is, it has very significant limitations. It's going to work with bash scripts and that's about it. Anything else is going to need to understand '-c' and parse the args themselves which is hardly satisfactory.

@mds99 - I take it you haven't tried this code? Are you in a position to try patches, or are you working with a packaged version of XRDP?

I can rebuild from sources and try on my testbed. I don't feel like i'm capable to produce a patch or to assess the quality of it.

[matt335672]

That's fine - thanks for that.

My plan is to work with @d-yacenko on getting his PR into the main devel branch. I've had a think about this overnight and I don't think it's too much work to achive this. If you can test it as well that would be great.

[d-yacenko]

Hi all, can i help?

[matt335672]

Hi @d-yacenko

Thanks for getting in touch.

You'll realise from the above conversation that @mds99 has a very similar requirement to the one you've addressed with your PR in #1270. If we can work on your PR and sort out a few things we can hopefully get both of these closed together.

Can we resume the conversation in #1270? I'll add some comments there.

[matt335672]

@mds99

How exactly are you trying to start your alternative shell?

There's more than one code path in session.c which can be used to start a WM. We need to check whether #1273 is going to meet your requirement as-is.

[matt335672]

I'll re-open this temporarily until I'm sure we've fixed this issue for @mds99

[mds99]

'devel' works for me, thanks!

[matt335672]

That's great - I'll close this then. The code will be formally released with XRDP v0.9.15.