<div dir="ltr">sure<div><br></div><div>Reviewed-by Jérémy Rosen <<a href="mailto:jeremy.rosen@smile.fr">jeremy.rosen@smile.fr</a>></div><div><br></div><div>Sorry for noticing that everything was adressed</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Le jeu. 11 juin 2020 à 02:04, Norbert Lange <<a href="mailto:nolange79@gmail.com">nolange79@gmail.com</a>> a écrit :<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Am So., 7. Juni 2020 um 21:42 Uhr schrieb Jérémy ROSEN <<a href="mailto:jeremy.rosen@smile.fr" target="_blank">jeremy.rosen@smile.fr</a>>:<br>
><br>
><br>
><br>
> Le dim. 7 juin 2020 à 21:24, Norbert Lange <<a href="mailto:nolange79@gmail.com" target="_blank">nolange79@gmail.com</a>> a écrit :<br>
>><br>
>> Am So., 7. Juni 2020 um 21:16 Uhr schrieb Jérémy ROSEN <<a href="mailto:jeremy.rosen@smile.fr" target="_blank">jeremy.rosen@smile.fr</a>>:<br>
>> ><br>
>> ><br>
>> ><br>
>> > Le dim. 7 juin 2020 à 21:03, Norbert Lange <<a href="mailto:nolange79@gmail.com" target="_blank">nolange79@gmail.com</a>> a écrit :<br>
>> >><br>
>> >> Am So., 7. Juni 2020 um 12:54 Uhr schrieb Jérémy ROSEN <<a href="mailto:jeremy.rosen@smile.fr" target="_blank">jeremy.rosen@smile.fr</a>>:<br>
>> >> ><br>
>> >> ><br>
>> >> ><br>
>> >> > Le sam. 6 juin 2020 à 00:59, Norbert Lange <<a href="mailto:nolange79@gmail.com" target="_blank">nolange79@gmail.com</a>> a écrit :<br>
>> >> >><br>
>> >> >> the openssh daemon is not suited for systemd's simple<br>
>> >> >> service type. dependend services should only start<br>
>> >> >> when sshd is ready to accept connections.<br>
>> >> >><br>
>> >> >> A patch is added from debian to allow openssh<br>
>> >> >> to communicate this state.<br>
>> >> >><br>
>> >> >> Restarts are prevented if the reason is a faulty<br>
>> >> >> config file (errocode 255).<br>
>> >> >><br>
>> >> >> The "user confinement directory" is changed to<br>
>> >> >> '/run/sshd' which is automatically managed by systemd.<br>
>> >> >><br>
>> >> >> Signed-off-by: Norbert Lange <<a href="mailto:nolange79@gmail.com" target="_blank">nolange79@gmail.com</a>><br>
>> >> >> ---<br>
>> >> >>  package/openssh/00-systemd-readiness.patch | 84 ++++++++++++++++++++++<br>
>> >> >>  package/openssh/<a href="http://openssh.mk" rel="noreferrer" target="_blank">openssh.mk</a>                 | 14 +++-<br>
>> >> >>  package/openssh/sshd-sysusers.conf         |  2 +-<br>
>> >> >>  package/openssh/sshd.service               | 13 +++-<br>
>> >> >>  4 files changed, 109 insertions(+), 4 deletions(-)<br>
>> >> >>  create mode 100644 package/openssh/00-systemd-readiness.patch<br>
>> >> >><br>
>> >> >> diff --git a/package/openssh/00-systemd-readiness.patch b/package/openssh/00-systemd-readiness.patch<br>
>> >> >> new file mode 100644<br>
>> >> >> index 0000000000..be3b6b0074<br>
>> >> >> --- /dev/null<br>
>> >> >> +++ b/package/openssh/00-systemd-readiness.patch<br>
>> >> >> @@ -0,0 +1,84 @@<br>
>> >> >> +From ab765b2bd55062a704f09da8f8c1c4ad1d6630a7 Mon Sep 17 00:00:00 2001<br>
>> >> >> +From: Michael Biebl <<a href="mailto:biebl@debian.org" target="_blank">biebl@debian.org</a>><br>
>> >> >> +Date: Mon, 21 Dec 2015 16:08:47 +0000<br>
>> >> >> +Subject: Add systemd readiness notification support<br>
>> >> >> +<br>
>> >> >> +Bug-Debian: <a href="https://bugs.debian.org/778913" rel="noreferrer" target="_blank">https://bugs.debian.org/778913</a><br>
>> >> >> +Forwarded: no<br>
>> >> >> +Last-Update: 2017-08-22<br>
>> >> >> +<br>
>> >> >> +Patch-Name: systemd-readiness.patch<br>
>> >> >> +---<br>
>> >> >> + <a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a> | 24 ++++++++++++++++++++++++<br>
>> >> >> + sshd.c       |  9 +++++++++<br>
>> >> >> + 2 files changed, 33 insertions(+)<br>
>> >> >> +<br>
>> >> >> +diff --git a/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a> b/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a><br>
>> >> >> +index e894db9fc..c119d6fd1 100644<br>
>> >> >> +--- a/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a><br>
>> >> >> ++++ b/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a><br>
>> >> >> +@@ -4499,6 +4499,29 @@ AC_ARG_WITH([kerberos5],<br>
>> >> >> + AC_SUBST([GSSLIBS])<br>
>> >> >> + AC_SUBST([K5LIBS])<br>
>> >> >> +<br>
>> >> >> ++# Check whether user wants systemd support<br>
>> >> >> ++SYSTEMD_MSG="no"<br>
>> >> >> ++AC_ARG_WITH(systemd,<br>
>> >> >> ++      [  --with-systemd          Enable systemd support],<br>
>> >> >> ++      [ if test "x$withval" != "xno" ; then<br>
>> >> >> ++              AC_PATH_TOOL([PKGCONFIG], [pkg-config], [no])<br>
>> >> >> ++              if test "$PKGCONFIG" != "no"; then<br>
>> >> >> ++                      AC_MSG_CHECKING([for libsystemd])<br>
>> >> >> ++                      if $PKGCONFIG --exists libsystemd; then<br>
>> >> >> ++                              SYSTEMD_CFLAGS=`$PKGCONFIG --cflags libsystemd`<br>
>> >> >> ++                              SYSTEMD_LIBS=`$PKGCONFIG --libs libsystemd`<br>
>> >> >> ++                              CPPFLAGS="$CPPFLAGS $SYSTEMD_CFLAGS"<br>
>> >> >> ++                              SSHDLIBS="$SSHDLIBS $SYSTEMD_LIBS"<br>
>> >> >> ++                              AC_MSG_RESULT([yes])<br>
>> >> >> ++                              AC_DEFINE(HAVE_SYSTEMD, 1, [Define if you want systemd support.])<br>
>> >> >> ++                              SYSTEMD_MSG="yes"<br>
>> >> >> ++                      else<br>
>> >> >> ++                              AC_MSG_RESULT([no])<br>
>> >> >> ++                      fi<br>
>> >> >> ++              fi<br>
>> >> >> ++      fi ]<br>
>> >> >> ++)<br>
>> >> >> ++<br>
>> >> >> + # Looking for programs, paths and files<br>
>> >> >> +<br>
>> >> >> + PRIVSEP_PATH=/var/empty<br>
>> >> >> +@@ -5305,6 +5328,7 @@ echo "                   libldns support: $LDNS_MSG"<br>
>> >> >> + echo "  Solaris process contract support: $SPC_MSG"<br>
>> >> >> + echo "           Solaris project support: $SP_MSG"<br>
>> >> >> + echo "         Solaris privilege support: $SPP_MSG"<br>
>> >> >> ++echo "                   systemd support: $SYSTEMD_MSG"<br>
>> >> >> + echo "       IP address in \$DISPLAY hack: $DISPLAY_HACK_MSG"<br>
>> >> >> + echo "           Translate v4 in v6 hack: $IPV4_IN6_HACK_MSG"<br>
>> >> >> + echo "                  BSD Auth support: $BSD_AUTH_MSG"<br>
>> >> >> +diff --git a/sshd.c b/sshd.c<br>
>> >> >> +index 4e8ff0662..5e7679a33 100644<br>
>> >> >> +--- a/sshd.c<br>
>> >> >> ++++ b/sshd.c<br>
>> >> >> +@@ -85,6 +85,10 @@<br>
>> >> >> + #include <prot.h><br>
>> >> >> + #endif<br>
>> >> >> +<br>
>> >> >> ++#ifdef HAVE_SYSTEMD<br>
>> >> >> ++#include <systemd/sd-daemon.h><br>
>> >> >> ++#endif<br>
>> >> >> ++<br>
>> >> >> + #include "xmalloc.h"<br>
>> >> >> + #include "ssh.h"<br>
>> >> >> + #include "ssh2.h"<br>
>> >> >> +@@ -1951,6 +1955,11 @@ main(int ac, char **av)<br>
>> >> >> +                       }<br>
>> >> >> +               }<br>
>> >> >> +<br>
>> >> >> ++#ifdef HAVE_SYSTEMD<br>
>> >> >> ++              /* Signal systemd that we are ready to accept connections */<br>
>> >> >> ++              sd_notify(0, "READY=1");<br>
>> >> >> ++#endif<br>
>> >> >> ++<br>
>> >> >> +               /* Accept a connection and return in a forked child */<br>
>> >> >> +               server_accept_loop(&sock_in, &sock_out,<br>
>> >> >> +                   &newsock, config_s);<br>
>> >> >> diff --git a/package/openssh/<a href="http://openssh.mk" rel="noreferrer" target="_blank">openssh.mk</a> b/package/openssh/<a href="http://openssh.mk" rel="noreferrer" target="_blank">openssh.mk</a><br>
>> >> >> index 55b917e20a..d425db1428 100644<br>
>> >> >> --- a/package/openssh/<a href="http://openssh.mk" rel="noreferrer" target="_blank">openssh.mk</a><br>
>> >> >> +++ b/package/openssh/<a href="http://openssh.mk" rel="noreferrer" target="_blank">openssh.mk</a><br>
>> >> >> @@ -12,6 +12,7 @@ OPENSSH_CONF_ENV = \<br>
>> >> >>         LD="$(TARGET_CC)" \<br>
>> >> >>         LDFLAGS="$(TARGET_CFLAGS)" \<br>
>> >> >>         LIBS=`$(PKG_CONFIG_HOST_BINARY) --libs openssl`<br>
>> >> >> +OPENSSH_AUTORECONF = YES<br>
>> >> >>  OPENSSH_CONF_OPTS = \<br>
>> >> >>         --sysconfdir=/etc/ssh \<br>
>> >> >>         --with-default-path=$(BR2_SYSTEM_DEFAULT_PATH) \<br>
>> >> >> @@ -22,9 +23,20 @@ OPENSSH_CONF_OPTS = \<br>
>> >> >>         --disable-wtmpx \<br>
>> >> >>         --disable-strip<br>
>> >> >><br>
>> >> >> +ifeq ($(BR2_PACKAGE_SYSTEMD),y)<br>
>> >> >> +OPENSSH_DEPENDENCIES = systemd<br>
>> >> >> +<br>
>> >> >> +OPENSSH_CONF_OPTS += \<br>
>> >> >> +       --with-privsep-path=/run/sshd \<br>
>> >> >> +       --with-pid-dir=/run \<br>
>> >> >> +       --with-systemd<br>
>> >> >> +<br>
>> >> >> +else<br>
>> >> >> +<br>
>> >> >>  define OPENSSH_PERMISSIONS<br>
>> >> >>         /var/empty d 755 root root - - - - -<br>
>> >> >>  endef<br>
>> >> ><br>
>> >> ><br>
>> >> > Do we still need this when using systemd, or can it be commented out ?<br>
>> >><br>
>> >> Not sure what you mean with "this"?<br>
>> >> The OPENSSH_PERMISSIONS block is needed when not using systemd and it<br>
>> >> is only active then.<br>
>> >><br>
>> ><br>
>> > my bad, I missed the enclosing ifeq()<br>
>> ><br>
>> >><br>
>> >> ><br>
>> >> ><br>
>> >> >><br>
>> >> >> +endif<br>
>> >> >><br>
>> >> >>  ifeq ($(BR2_TOOLCHAIN_SUPPORTS_PIE),)<br>
>> >> >>  OPENSSH_CONF_OPTS += --without-pie<br>
>> >> >> @@ -72,7 +84,7 @@ define OPENSSH_INSTALL_SYSTEMD_SYSUSERS<br>
>> >> >>  endef<br>
>> >> >>  else<br>
>> >> >>  define OPENSSH_USERS<br>
>> >> >> -       sshd -1 sshd -1 * /var/empty - - SSH drop priv user<br>
>> >> >> +       sshd -1 sshd -1 * $(if $(BR2_PACKAGE_SYSTEMD),/run/sshd,/var/empty) - - SSH drop priv user<br>
>> >> >>  endef<br>
>> >> >>  endif<br>
>> >> >><br>
>> >> >> diff --git a/package/openssh/sshd-sysusers.conf b/package/openssh/sshd-sysusers.conf<br>
>> >> >> index ac77aec065..303d0dbb63 100644<br>
>> >> >> --- a/package/openssh/sshd-sysusers.conf<br>
>> >> >> +++ b/package/openssh/sshd-sysusers.conf<br>
>> >> >> @@ -1 +1 @@<br>
>> >> >> -u sshd - "SSH drop priv user" /var/empty<br>
>> >> >> +u sshd - "SSH drop priv user" /run/sshd<br>
>> >> >> diff --git a/package/openssh/sshd.service b/package/openssh/sshd.service<br>
>> >> >> index b5e96b3a25..715bd3f7eb 100644<br>
>> >> >> --- a/package/openssh/sshd.service<br>
>> >> >> +++ b/package/openssh/sshd.service<br>
>> >> >> @@ -1,11 +1,20 @@<br>
>> >> >>  [Unit]<br>
>> >> >>  Description=OpenSSH server daemon<br>
>> >> >> -After=syslog.target network.target auditd.service<br>
>> >> >> +Documentation=man:sshd(8) man:sshd_config(5)<br>
>> >> >> +After=network.target auditd.service<br>
>> >> >><br>
>> >> >><br>
>> >> >>  [Service]<br>
>> >> >>  ExecStartPre=/usr/bin/ssh-keygen -A<br>
>> >> >> -ExecStart=/usr/sbin/sshd -D -e<br>
>> >> >> +ExecStartPre=/usr/sbin/sshd -t<br>
>> >> >> +ExecStart=/usr/sbin/sshd -D<br>
>> >> ><br>
>> >> > You droped the -e, so  you are logging to syslog<br>
>> >> > However you droped the dependency on syslog.target earlier...<br>
>> >> > (maybe it should be syslog.socket instead of .target, btw)<br>
>> >><br>
>> >><br>
>> >> syslog.target is long long gone, and the syslog will be<br>
>> >> unconditionally available<br>
>> >> <a href="https://www.freedesktop.org/wiki/Software/systemd/syslog/" rel="noreferrer" target="_blank">https://www.freedesktop.org/wiki/Software/systemd/syslog/</a><br>
>> >><br>
>> >><br>
>> >> ><br>
>> >> ><br>
>> >> > how exactly do you want to log  ? (I think logging to stdout is better, it will be<br>
>> >> > redirected to the journal.<br>
>> >><br>
>> >><br>
>> >> stdout is not really useful if syslog is supported.<br>
>> >><br>
>> > i'd go the other way round<br>
>> ><br>
>> > syslog is not really necessary if stdout is available,<br>
>> > but it's a matter of taste :P so let's go your way.<br>
>><br>
>> Its more the point, that Openssh already implemented syslog, and thats<br>
>> a clear functional superset of listening to stdout.<br>
>><br>
>> ><br>
>> >><br>
>> >> ><br>
>> >> ><br>
>> >> >><br>
>> >> >> +ExecReload=/usr/sbin/sshd -t<br>
>> >> >>  ExecReload=/bin/kill -HUP $MAINPID<br>
>> >> >> +KillMode=process<br>
>> >> ><br>
>> >> ><br>
>> >> > Wouldn't mixed be better here ?<br>
>> >> > I'm not really sure what the use-case for procss is anyway...<br>
>> >><br>
>> >><br>
>> >> I taken that from debian, I could not argue against it (there is a<br>
>> >> long discussion which I linked above).<br>
>> >> Can you argue *for* mixed?<br>
>> >><br>
>> ><br>
>> ><br>
>> > I didn't see any link<br>
>> > * process : SIGTERM and SIGKILL is sent only to MainPID<br>
>> > * mixed : SIGTERM is sent to MainPID, SIGKILL is sent to every process in the service cgroup.<br>
>> ><br>
>> > This means that if all works well, they do the same thing<br>
>> ><br>
>> > in case the MainPID fails to properly terminate its children, process would leave children alive<br>
>> > but mixed woul kill everybody<br>
>> ><br>
>> > Since we are trying to terminate the service, it makes sense to me to make sur all child process<br>
>> > are killed.<br>
>> ><br>
>> > but I don't see your link so I may be missing something<br>
>><br>
>> The link is in the added patch: <a href="https://bugs.debian.org/778913" rel="noreferrer" target="_blank">https://bugs.debian.org/778913</a><br>
>><br>
>> As said, I could not argue either way, but I got some respect for the<br>
>> debian guys ;)<br>
>><br>
><br>
> The thread does not actually discuss process vs mixed...<br>
><br>
> so doesn't really help here.<br>
> OTOH, the debian version has been vetted by mbiel which is a systemc core-maintainer.<br>
><br>
> so i would go with mixed if I were to write the service from scratch, but since I don't have an<br>
> explanation for the choice of process, I'm not entirely sure...<br>
><br>
> A possibility is that ssh creates a process per connection. in that case<br>
> * process would not kill all ongoing connections<br>
> * mixed would<br>
><br>
> maybe it was chosen to protect existing connection. that would make some sense.<br>
<br>
Well, debian and arch seem to agree on using "process", I guess it<br>
means to just prevent<br>
new connections and not kill existing ones?<br>
<br>
Can I get a "reviewed-by" for this patch, so this and #3 can be merged?<br>
<br>
Norbert<br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><table border="0" style="border-collapse:collapse;border-spacing:0px;color:rgb(51,51,51);font-family:Raleway,regular;font-size:14px;padding:20px"><tbody><tr><td style="padding:0px 30px 0px 0px;font-size:13px;color:rgb(255,128,84);text-align:center"><a href="http://www.smile.eu/" style="background-color:transparent;color:rgb(221,72,20)" target="_blank"><img src="http://ftp.smile.fr/client/Communication/signature/img/Logo-new.png" alt="SMILE" style="border: 0px; vertical-align: middle; max-width: 100%; height: auto;"> </a><br><br><p style="margin:0px 0px 10px">20 rue des Jardins<br>92600 Asnières-sur-Seine</p></td><td style="padding:0px 0px 0px 20px;border-left:1px solid rgb(59,127,254);font-size:13px;color:rgb(255,128,84)"><div style="font-size:14px"><b>Jérémy ROSEN</b></div><div style="color:rgb(59,127,254)">Architecte technique<br></div><br><div style="color:rgb(59,127,254)"><span style="white-space:nowrap"><img src="http://ftp.smile.fr/client/Communication/signature/img/mail.png" alt="email" width="12" height="10" style="border: 0px; vertical-align: middle; margin-right: 5px;"> <a href="mailto:jeremy.rosen@smile.fr" style="background-color:transparent;color:rgb(59,127,254)" target="_blank">jeremy.rosen@smile.fr</a> </span><br><span style="white-space:nowrap"><img src="http://ftp.smile.fr/client/Communication/signature/img/phone.png" alt="phone" width="10" height="10" style="border: 0px; vertical-align: middle; margin-right: 5px;"></span>  +33 6 88 25 87 42 <br><span style="white-space:nowrap"><img src="http://ftp.smile.fr/client/Communication/signature/img/web.png" alt="url" width="12" height="12" style="border: 0px; vertical-align: middle; margin-right: 5px;"> <a href="http://www.smile.eu/" style="background-color:transparent;color:rgb(59,127,254)" target="_blank">http://www.smile.eu</a></span></div><br><div><span><a href="https://twitter.com/GroupeSmile" style="background-color:transparent;color:rgb(221,72,20);margin-right:5px" target="_blank"><img src="http://ftp.smile.fr/client/Communication/signature/img/rs-twitter.png" alt="Twitter" style="border: 0px; vertical-align: middle; max-width: 100%; height: auto;"></a></span> <span><a href="https://www.facebook.com/smileopensource" style="background-color:transparent;color:rgb(221,72,20);margin-right:5px" target="_blank"><img src="http://ftp.smile.fr/client/Communication/signature/img/rs-facebook.png" alt="Facebook" style="border: 0px; vertical-align: middle; max-width: 100%; height: auto;"></a></span> <span><a href="https://www.linkedin.com/company/smile" style="background-color:transparent;color:rgb(221,72,20);margin-right:5px" target="_blank"><img src="http://ftp.smile.fr/client/Communication/signature/img/rs-linkedin.png" alt="LinkedIn" style="border: 0px; vertical-align: middle; max-width: 100%; height: auto;"></a></span> <span><a href="https://github.com/Smile-SA" style="background-color:transparent;color:rgb(221,72,20);margin-right:5px" target="_blank"><img src="http://ftp.smile.fr/client/Communication/signature/img/rs-github.png" alt="Github" style="border: 0px; vertical-align: middle; max-width: 100%; height: auto;"></a></span></div></td></tr></tbody></table><br style="color:rgb(51,51,51);font-family:Raleway,regular;font-size:14px"><div style="color:rgb(51,51,51);font-family:Raleway,regular;font-size:14px"><a href="https://www.smile.eu/fr/publications/livres-blancs/yocto?utm_source=signature&utm_medium=email&utm_campaign=signature" style="background-color:transparent;color:rgb(221,72,20)" target="_blank"><img src="https://signature.smile.eu/assets/img/bandeaux_signature_mail_yocto.gif.gif" alt="Découvrez l’univers Smile, rendez-vous sur smile.eu" border="0" style="border: 0px; vertical-align: middle;"></a></div></div></div></div></div>