trying to resurrect discussion about "Cannot signal a process over a channel (rfc 4254, section 6.9)"

[Thierry Parmentelat]

Hi everybody

I’d like to resurrect the discussion on a known issue with the openssh server, regarding signal delivery as described in rfc4254

This has been originally reported about ten years ago in this thread:
https://bugzilla.mindrot.org/show_bug.cgi?id=1424

I am taking he liberty to copy a few people who contributed that thread over time

The discussion does not seem to expose the reasons that lead to the feature being held back for so long
I do share Thomas’s feeling that separating the server and client aspects would be a good way to restart from a clean slate

This is why I would suggest to first address the server side of the matter, which turns out to be our team’s major and single concern here, like I believe all/most of the other requestors

The thread contains, in Comment 44, a link to a patch here - again limited to server side
https://bugzilla.mindrot.org/attachment.cgi?id=3120&action=edit
that we’ve been able to test on a fedora28 box, and that updated version indeed improves the overall behaviour of the openssh server side for our needs, when used against an python/asyncssh client side

Does it make sense for us to submit a PR on the github repo https://github.com/openssh/openssh-portable, or elsewhere ?
or were there deeper concerns about that change that need to be further discussed ?


Many thanks in anticipation — Thierry

PS
I struggled a bit to get this through, so my apologies to people who received multiple copies :)

[Iain Morgan]

Post by Thierry Parmentelat
Hi everybody
I’d like to resurrect the discussion on a known issue with the openssh server, regarding signal delivery as described in rfc4254
https://bugzilla.mindrot.org/show_bug.cgi?id=1424
I am taking he liberty to copy a few people who contributed that thread over time
The discussion does not seem to expose the reasons that lead to the feature being held back for so long
I do share Thomas’s feeling that separating the server and client aspects would be a good way to restart from a clean slate
This is why I would suggest to first address the server side of the matter, which turns out to be our team’s major and single concern here, like I believe all/most of the other requestors

At one point, I had wondered about separating out the client and server
support as well. At first glance, that would seem to help move things
forward and would address most of the reported use cases. However, I
have some users who would need the client support as well.

I suspect that adding the server support first might be a problem for
the developers. Such a feature would need regression and unit tests, and
those would be easier to implement if the client has support for the
feature.

It would be nice to know what the precise technical issues are that have
prevented support for this from being added. From what I recall, it
seemed like the delay was largely due to details of the client
behaviour, and possibly some feature creep.

[Thierry Parmentelat]

Post by Iain Morgan
At one point, I had wondered about separating out the client and server
support as well. At first glance, that would seem to help move things
forward and would address most of the reported use cases. However, I
have some users who would need the client support as well.
I suspect that adding the server support first might be a problem for
the developers. Such a feature would need regression and unit tests, and
those would be easier to implement if the client has support for the
feature.

Fair enough, but apparently having to swallow both aspects in the same move seems to have proven too big a bite at least once :)
I am not at all familiar with the openssh codebase, but if that helps and if that sounds like a doable idea, we can certainly propose to provide some dedicated test stubs addressing the server side, written e.g. in asynchronous python based on asyncssh, that at first sight has all that is needed to carry out fine-grained tests in this area; even if temporarily, i.e. until some agreement can be found on what the client side should look like

Post by Iain Morgan
It would be nice to know what the precise technical issues are that have
prevented support for this from being added. From what I recall, it
seemed like the delay was largely due to details of the client
behaviour, and possibly some feature creep.

agreed ! the initial thread was not exactly very talkative on all this..

— Thierry

[Iain Morgan]

Post by Thierry Parmentelat

Post by Iain Morgan
At one point, I had wondered about separating out the client and server
support as well. At first glance, that would seem to help move things
forward and would address most of the reported use cases. However, I
have some users who would need the client support as well.
I suspect that adding the server support first might be a problem for
the developers. Such a feature would need regression and unit tests, and
those would be easier to implement if the client has support for the
feature.

Fair enough, but apparently having to swallow both aspects in the same move seems to have proven too big a bite at least once :)
I am not at all familiar with the openssh codebase, but if that helps and if that sounds like a doable idea, we can certainly propose to provide some dedicated test stubs addressing the server side, written e.g. in asynchronous python based on asyncssh, that at first sight has all that is needed to carry out fine-grained tests in this area; even if temporarily, i.e. until some agreement can be found on what the client side should look like

Post by Iain Morgan
It would be nice to know what the precise technical issues are that have
prevented support for this from being added. From what I recall, it
seemed like the delay was largely due to details of the client
behaviour, and possibly some feature creep.

agreed ! the initial thread was not exactly very talkative on all this..

One thing to keep in mind is that OpenSSH is primarily developed on
OpenBSD. Any test suite would need to work on a base OpenBSD install,
which does not include Python. Of course, the test suite would need to
work on other platforms as well.

[Ron Frederick]

Post by Iain Morgan

Post by Thierry Parmentelat

Post by Iain Morgan
At one point, I had wondered about separating out the client and server
support as well. At first glance, that would seem to help move things
forward and would address most of the reported use cases. However, I
have some users who would need the client support as well.
I suspect that adding the server support first might be a problem for
the developers. Such a feature would need regression and unit tests, and
those would be easier to implement if the client has support for the
feature.

Fair enough, but apparently having to swallow both aspects in the same move seems to have proven too big a bite at least once :)
I am not at all familiar with the openssh codebase, but if that helps and if that sounds like a doable idea, we can certainly propose to provide some dedicated test stubs addressing the server side, written e.g. in asynchronous python based on asyncssh, that at first sight has all that is needed to carry out fine-grained tests in this area; even if temporarily, i.e. until some agreement can be found on what the client side should look like

Post by Iain Morgan
It would be nice to know what the precise technical issues are that have
prevented support for this from being added. From what I recall, it
seemed like the delay was largely due to details of the client
behaviour, and possibly some feature creep.

agreed ! the initial thread was not exactly very talkative on all this..

One thing to keep in mind is that OpenSSH is primarily developed on
OpenBSD. Any test suite would need to work on a base OpenBSD install,
which does not include Python. Of course, the test suite would need to
work on other platforms as well.

I’m happy to support this effort however I can, as I’ve had multiple requests from AsyncSSH users asking why this functionality doesn’t work, and the reason is usually that AsyncSSH is being used as a client to send signals to an OpenSSH server.

That said, I agree that the unit tests here would probably need to be in C, so they didn’t introduce any new dependencies. The support needed to perform unit testing should be much less work than a full client implementation, though. All that’s needed is a function to send the right SSH message, and a way to trigger that. There doesn’t even need to be any handling of responses, as the “signal” channel request is defined to not request an acknowledgement.

[Yonathan Bleyfuesz]

Hi,

Post by Iain Morgan

Post by Thierry Parmentelat

Post by Iain Morgan
It would be nice to know what the precise technical issues are that have
prevented support for this from being added. From what I recall, it
seemed like the delay was largely due to details of the client
behaviour, and possibly some feature creep.

It would indeed be really great to have some details on this point.

Concerning the test of the server side feature, it should be pretty similar to the test of the functionality of the ~B escape character !
Unfortunately, I was unable to find any reference that such a test exists : is anyone aware of where it could be ?

Moreover, is there any kind of guideline concerning the environment that should be use for the test suite ?

Also in the RFC it is said :”Some systems may not implement signals, in which case they SHOULD ignore this message” . So I think the proposed patch should have some kind of whitelisting.

Thanks in advance for the answers,

Yonathan Bleyfuesz

Post by Iain Morgan

Post by Thierry Parmentelat

Post by Iain Morgan
At one point, I had wondered about separating out the client and server
support as well. At first glance, that would seem to help move things
forward and would address most of the reported use cases. However, I
have some users who would need the client support as well.
I suspect that adding the server support first might be a problem for
the developers. Such a feature would need regression and unit tests, and
those would be easier to implement if the client has support for the
feature.

Fair enough, but apparently having to swallow both aspects in the same move seems to have proven too big a bite at least once :)
I am not at all familiar with the openssh codebase, but if that helps and if that sounds like a doable idea, we can certainly propose to provide some dedicated test stubs addressing the server side, written e.g. in asynchronous python based on asyncssh, that at first sight has all that is needed to carry out fine-grained tests in this area; even if temporarily, i.e. until some agreement can be found on what the client side should look like

Post by Iain Morgan
It would be nice to know what the precise technical issues are that have
prevented support for this from being added. From what I recall, it
seemed like the delay was largely due to details of the client
behaviour, and possibly some feature creep.

agreed ! the initial thread was not exactly very talkative on all this..

One thing to keep in mind is that OpenSSH is primarily developed on
OpenBSD. Any test suite would need to work on a base OpenBSD install,
which does not include Python. Of course, the test suite would need to
work on other platforms as well.
--
Iain Morgan

[Yonathan Bleyfuesz]

Hi all,

I would like to propose some ideas to revivify this subject.

-First, we could add support on the client to send signal thanks to the escape characters.
(code : https://github.com/JawaGL/openssh-portable/commit/5bc9e6bc959b1b0f89d7ca7b4b04d7c37079fef0 ).

With this, in order to send a message requesting the server to send a SIGTERM to the remote process, you need to type “~ST” which is not really invasive client-side.

But this means that the client has to enable TTY.


-Secondly , server-side, there is a problem with the currently suggested patch : it only works when we do an ‘exec’ request to the server (eg : ssh some-host “some; commands;”).

This is because in the other possible configuration, a shell is launched by the server. Then when we launch a process, it is forked by this shell and thus it has its own group-id.

When the user launches a signal-request hoping to reach a blocking process, the pid that is used by the ‘killpg’ function is the one of the shell. So it is this shell that catches the signal resulting in it:
- dying and leaving zombies
- dying and taking its child with him (SIGHUP and SIGKILL)
- ignoring the signal (SIGINT, SIGTERM, SIGQUIT).

Example of ID’s when I connect to a server and launch the script test_signal.sh :
PID PPID PGID SID
4060 1598 4060 1556 sshd sshd: ***@pts/2
4062 4060 4062 4062 bash -bash
4075 4062 4075 4062 sh sh test_signal.sh
4076 4075 4075 4062 sh sh test_signal.sh

So in order to take this use case into account we could use the 'tcgetpgrp()’ function from ‘unistd.h’.
(code : https://github.com/JawaGL/openssh-portable/commit/3667c0d90688c43ac0729083f73afa65102226b4 )

Of course this would still work if there are no TTY present since we can still access the PGID of the forked child in the session attributes.

-Finally, in order to test these functionalities, we could integrate a test case in the regress folder. (code : https://github.com/JawaGL/openssh-portable/commit/02c39b15363c54d0e622e5724c721a474e1cacd6).


I tested all these features on MacOSX and Ubuntu 18.

I hope this helps,
Thanks in advance for your returns,

Yonathan

[Damien Miller]

FWIW, now that privsep is mandatory I have no objection to including
signal support in sshd.

Post by Yonathan Bleyfuesz
Hi all,
I would like to propose some ideas to revivify this subject.
-First, we could add support on the client to send signal thanks to the escape characters.
(code : https://github.com/JawaGL/openssh-portable/commit/5bc9e6bc959b1b0f89d7ca7b4b04d7c37079fef0 ).
With this, in order to send a message requesting the server to send a SIGTERM to the remote process, you need to type “~ST” which is not really invasive client-side.
But this means that the client has to enable TTY.
-Secondly , server-side, there is a problem with the currently suggested patch : it only works when we do an ‘exec’ request to the server (eg : ssh some-host “some; commands;”).
This is because in the other possible configuration, a shell is launched by the server. Then when we launch a process, it is forked by this shell and thus it has its own group-id.
- dying and leaving zombies
- dying and taking its child with him (SIGHUP and SIGKILL)
- ignoring the signal (SIGINT, SIGTERM, SIGQUIT).
PID PPID PGID SID
4062 4060 4062 4062 bash -bash
4075 4062 4075 4062 sh sh test_signal.sh
4076 4075 4075 4062 sh sh test_signal.sh
So in order to take this use case into account we could use the 'tcgetpgrp()’ function from ‘unistd.h’.
(code : https://github.com/JawaGL/openssh-portable/commit/3667c0d90688c43ac0729083f73afa65102226b4 )
Of course this would still work if there are no TTY present since we can still access the PGID of the forked child in the session attributes.
-Finally, in order to test these functionalities, we could integrate a test case in the regress folder. (code : https://github.com/JawaGL/openssh-portable/commit/02c39b15363c54d0e622e5724c721a474e1cacd6).
I tested all these features on MacOSX and Ubuntu 18.
I hope this helps,
Thanks in advance for your returns,
Yonathan
_______________________________________________
openssh-unix-dev mailing list
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

[Iain Morgan]

That's great news! Do you have any input regarding the implementation
details? Any suggestions that would ease inclusion of this feature would
be welcome.
--
Iain

Post by Damien Miller
FWIW, now that privsep is mandatory I have no objection to including
signal support in sshd.

Post by Yonathan Bleyfuesz
Hi all,
I would like to propose some ideas to revivify this subject.
-First, we could add support on the client to send signal thanks to the escape characters.
(code : https://github.com/JawaGL/openssh-portable/commit/5bc9e6bc959b1b0f89d7ca7b4b04d7c37079fef0 ).
With this, in order to send a message requesting the server to send a SIGTERM to the remote process, you need to type “~ST” which is not really invasive client-side.
But this means that the client has to enable TTY.
-Secondly , server-side, there is a problem with the currently suggested patch : it only works when we do an ‘exec’ request to the server (eg : ssh some-host “some; commands;”).
This is because in the other possible configuration, a shell is launched by the server. Then when we launch a process, it is forked by this shell and thus it has its own group-id.
- dying and leaving zombies
- dying and taking its child with him (SIGHUP and SIGKILL)
- ignoring the signal (SIGINT, SIGTERM, SIGQUIT).
PID PPID PGID SID
4062 4060 4062 4062 bash -bash
4075 4062 4075 4062 sh sh test_signal.sh
4076 4075 4075 4062 sh sh test_signal.sh
So in order to take this use case into account we could use the 'tcgetpgrp()’ function from ‘unistd.h’.
(code : https://github.com/JawaGL/openssh-portable/commit/3667c0d90688c43ac0729083f73afa65102226b4 )
Of course this would still work if there are no TTY present since we can still access the PGID of the forked child in the session attributes.
-Finally, in order to test these functionalities, we could integrate a test case in the regress folder. (code : https://github.com/JawaGL/openssh-portable/commit/02c39b15363c54d0e622e5724c721a474e1cacd6).
I tested all these features on MacOSX and Ubuntu 18.
I hope this helps,
Thanks in advance for your returns,
Yonathan
_______________________________________________
openssh-unix-dev mailing list
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

_______________________________________________
openssh-unix-dev mailing list
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

[Yonathan Bleyfuesz]

Hi,

So that you know, I did a pull request on the OpenSSH-portable GitHub to avoid this discussion to fall into oblivion.

Best regards,

Yonathan

Post by Iain Morgan
That's great news! Do you have any input regarding the implementation
details? Any suggestions that would ease inclusion of this feature would
be welcome.
--
Iain

Post by Damien Miller
FWIW, now that privsep is mandatory I have no objection to including
signal support in sshd.

Post by Yonathan Bleyfuesz
Hi all,
I would like to propose some ideas to revivify this subject.
-First, we could add support on the client to send signal thanks to the escape characters.
(code : https://github.com/JawaGL/openssh-portable/commit/5bc9e6bc959b1b0f89d7ca7b4b04d7c37079fef0 ).
With this, in order to send a message requesting the server to send a SIGTERM to the remote process, you need to type “~ST” which is not really invasive client-side.
But this means that the client has to enable TTY.
-Secondly , server-side, there is a problem with the currently suggested patch : it only works when we do an ‘exec’ request to the server (eg : ssh some-host “some; commands;”).
This is because in the other possible configuration, a shell is launched by the server. Then when we launch a process, it is forked by this shell and thus it has its own group-id.
- dying and leaving zombies
- dying and taking its child with him (SIGHUP and SIGKILL)
- ignoring the signal (SIGINT, SIGTERM, SIGQUIT).
PID PPID PGID SID
4062 4060 4062 4062 bash -bash
4075 4062 4075 4062 sh sh test_signal.sh
4076 4075 4075 4062 sh sh test_signal.sh
So in order to take this use case into account we could use the 'tcgetpgrp()’ function from ‘unistd.h’.
(code : https://github.com/JawaGL/openssh-portable/commit/3667c0d90688c43ac0729083f73afa65102226b4 )
Of course this would still work if there are no TTY present since we can still access the PGID of the forked child in the session attributes.
-Finally, in order to test these functionalities, we could integrate a test case in the regress folder. (code : https://github.com/JawaGL/openssh-portable/commit/02c39b15363c54d0e622e5724c721a474e1cacd6).
I tested all these features on MacOSX and Ubuntu 18.
I hope this helps,
Thanks in advance for your returns,
Yonathan
_______________________________________________
openssh-unix-dev mailing list
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

_______________________________________________
openssh-unix-dev mailing list
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

--
Iain Morgan

[Damien Miller]

Post by Yonathan Bleyfuesz
Hi,
So that you know, I did a pull request on the OpenSSH-portable GitHub
to avoid this discussion to fall into oblivion.

FYI:

commit cd98925c6405e972dc9f211afc7e75e838abe81c
Author: ***@openbsd.org <***@openbsd.org>
Date: Tue Oct 2 12:40:07 2018 +0000

upstream: Add server support for signalling sessions via the SSH

channel/ session protocol. Signalling is only supported to sesssions that are
not subsystems and were not started with a forced command.

Long requested in bz#1424

Based on a patch from markus@ and reworked by dtucker@;
ok markus@ dtucker@

OpenBSD-Commit-ID: 4bea826f575862eaac569c4bedd1056a268be1c3