Re: sshd proctitle [Re: CVS: cvs.openbsd.org: src]

2020-01-24 Thread Damien Miller
On Fri, 24 Jan 2020, Stuart Henderson wrote:

> That works - etc/rc.d/sshd diff to match as follows:
> 
> Index: sshd
> ===
> RCS file: /cvs/src/etc/rc.d/sshd,v
> retrieving revision 1.5
> diff -u -p -r1.5 sshd
> --- sshd  22 Jan 2020 13:14:51 -  1.5
> +++ sshd  24 Jan 2020 11:59:52 -
> @@ -6,7 +6,7 @@ daemon="/usr/sbin/sshd"
>  
>  . /etc/rc.d/rc.subr
>  
> -pexp="sshd: \[listener\].*"
> +pexp="sshd: ${daemon}${daemon_flags:+ ${daemon_flags}} \[listener\].*"
>  
>  rc_reload() {
>   ${daemon} ${daemon_flags} -t && pkill -HUP -xf "${pexp}"

ok djm



Re: sshd proctitle [Re: CVS: cvs.openbsd.org: src]

2020-01-24 Thread Damien Miller
On Fri, 24 Jan 2020, Antoine Jacoutot wrote:

> Great :-)
> Ok aja 

committed, the proctitle looks like this now in case the rc scripts need
further tweaking:

$ pgrep -lf sshd
12844 sshd: /usr/sbin/sshd -f /etc/ssh/sshd_config [listener] 0 of 10-100 
startups



Re: sshd proctitle [Re: CVS: cvs.openbsd.org: src]

2020-01-24 Thread Antoine Jacoutot
Great :-)
Ok aja 

—
Antoine

> On 24 Jan 2020, at 13:02, Stuart Henderson  wrote:
> 
> On 2020/01/23 21:20, Damien Miller wrote:
>>> On Thu, 23 Jan 2020, Damien Miller wrote:
>>> 
 On Thu, 23 Jan 2020, Damien Miller wrote:
>>> 
 What information would you like there? We could put the first N listen
 addrs in the proctitle if that would help.
>>> 
>>> Maybe like this:
>>> 
>>> 63817 ??  S0:00.05 sshd: [listen] on [0.0.0.0]:22, [::]:22, 0 of 
>>> 10-100
>> 
>> antoine@ said this was not sufficient, so please try the following:
>> 
>> 63817 ??  I0:00.09 sshd: /usr/sbin/sshd [listener] 0 of 10-100 
>> startups
> 
> That works - etc/rc.d/sshd diff to match as follows:
> 
> Index: sshd
> ===
> RCS file: /cvs/src/etc/rc.d/sshd,v
> retrieving revision 1.5
> diff -u -p -r1.5 sshd
> --- sshd22 Jan 2020 13:14:51 -1.5
> +++ sshd24 Jan 2020 11:59:52 -
> @@ -6,7 +6,7 @@ daemon="/usr/sbin/sshd"
> 
> . /etc/rc.d/rc.subr
> 
> -pexp="sshd: \[listener\].*"
> +pexp="sshd: ${daemon}${daemon_flags:+ ${daemon_flags}} \[listener\].*"
> 
> rc_reload() {
>${daemon} ${daemon_flags} -t && pkill -HUP -xf "${pexp}"
> 
> 
> 
>> 
>> diff --git a/sshd.c b/sshd.c
>> index f6139fe..b7ed0f3 100644
>> --- a/sshd.c
>> +++ b/sshd.c
>> @@ -240,6 +240,8 @@ void destroy_sensitive_data(void);
>> void demote_sensitive_data(void);
>> static void do_ssh2_kex(struct ssh *);
>> 
>> +static char *listener_proctitle;
>> +
>> /*
>>  * Close all listening sockets
>>  */
>> @@ -1032,9 +1034,9 @@ server_accept_loop(int *sock_in, int *sock_out, int 
>> *newsock, int *config_s)
>> */
>>for (;;) {
>>if (ostartups != startups) {
>> -setproctitle("[listener] %d of %d-%d startups",
>> -startups, options.max_startups_begin,
>> -options.max_startups);
>> +setproctitle("%s [listener] %d of %d-%d startups",
>> +listener_proctitle, startups,
>> +options.max_startups_begin, options.max_startups);
>>ostartups = startups;
>>}
>>if (received_sighup) {
>> @@ -1347,6 +1349,41 @@ accumulate_host_timing_secret(struct sshbuf 
>> *server_cfg,
>>sshbuf_free(buf);
>> }
>> 
>> +static void
>> +xextendf(char **s, const char *sep, const char *fmt, ...)
>> +__attribute__((__format__ (printf, 3, 4))) __attribute__((__nonnull__ 
>> (3)));
>> +static void
>> +xextendf(char **sp, const char *sep, const char *fmt, ...)
>> +{
>> +va_list ap;
>> +char *tmp1, *tmp2;
>> +
>> +va_start(ap, fmt);
>> +xvasprintf(, fmt, ap);
>> +va_end(ap);
>> +
>> +if (*sp == NULL || **sp == '\0') {
>> +free(*sp);
>> +*sp = tmp1;
>> +return;
>> +}
>> +xasprintf(, "%s%s%s", *sp, sep, tmp1);
>> +free(tmp1);
>> +free(*sp);
>> +*sp = tmp2;
>> +}
>> +
>> +static char *
>> +prepare_proctitle(int ac, char **av)
>> +{
>> +char *ret = NULL;
>> +int i;
>> +
>> +for (i = 0; i < ac; i++)
>> +xextendf(, " ", "%s", av[i]);
>> +return ret;
>> +}
>> +
>> /*
>>  * Main program for the daemon.
>>  */
>> @@ -1774,6 +1811,7 @@ main(int ac, char **av)
>>rexec_argv[rexec_argc] = "-R";
>>rexec_argv[rexec_argc + 1] = NULL;
>>}
>> +listener_proctitle = prepare_proctitle(ac, av);
>> 
>>/* Ensure that umask disallows at least group and world write */
>>new_umask = umask(0077) | 0022;
>> 
> 



Re: sshd proctitle [Re: CVS: cvs.openbsd.org: src]

2020-01-24 Thread Stuart Henderson
On 2020/01/23 21:20, Damien Miller wrote:
> On Thu, 23 Jan 2020, Damien Miller wrote:
> 
> > On Thu, 23 Jan 2020, Damien Miller wrote:
> > 
> > > What information would you like there? We could put the first N listen
> > > addrs in the proctitle if that would help.
> > 
> > Maybe like this:
> > 
> > 63817 ??  S0:00.05 sshd: [listen] on [0.0.0.0]:22, [::]:22, 0 of 
> > 10-100
> 
> antoine@ said this was not sufficient, so please try the following:
> 
> 63817 ??  I0:00.09 sshd: /usr/sbin/sshd [listener] 0 of 10-100 
> startups

That works - etc/rc.d/sshd diff to match as follows:

Index: sshd
===
RCS file: /cvs/src/etc/rc.d/sshd,v
retrieving revision 1.5
diff -u -p -r1.5 sshd
--- sshd22 Jan 2020 13:14:51 -  1.5
+++ sshd24 Jan 2020 11:59:52 -
@@ -6,7 +6,7 @@ daemon="/usr/sbin/sshd"
 
 . /etc/rc.d/rc.subr
 
-pexp="sshd: \[listener\].*"
+pexp="sshd: ${daemon}${daemon_flags:+ ${daemon_flags}} \[listener\].*"
 
 rc_reload() {
${daemon} ${daemon_flags} -t && pkill -HUP -xf "${pexp}"



> 
> diff --git a/sshd.c b/sshd.c
> index f6139fe..b7ed0f3 100644
> --- a/sshd.c
> +++ b/sshd.c
> @@ -240,6 +240,8 @@ void destroy_sensitive_data(void);
>  void demote_sensitive_data(void);
>  static void do_ssh2_kex(struct ssh *);
>  
> +static char *listener_proctitle;
> +
>  /*
>   * Close all listening sockets
>   */
> @@ -1032,9 +1034,9 @@ server_accept_loop(int *sock_in, int *sock_out, int 
> *newsock, int *config_s)
>*/
>   for (;;) {
>   if (ostartups != startups) {
> - setproctitle("[listener] %d of %d-%d startups",
> - startups, options.max_startups_begin,
> - options.max_startups);
> + setproctitle("%s [listener] %d of %d-%d startups",
> + listener_proctitle, startups,
> + options.max_startups_begin, options.max_startups);
>   ostartups = startups;
>   }
>   if (received_sighup) {
> @@ -1347,6 +1349,41 @@ accumulate_host_timing_secret(struct sshbuf 
> *server_cfg,
>   sshbuf_free(buf);
>  }
>  
> +static void
> +xextendf(char **s, const char *sep, const char *fmt, ...)
> +__attribute__((__format__ (printf, 3, 4))) __attribute__((__nonnull__ 
> (3)));
> +static void
> +xextendf(char **sp, const char *sep, const char *fmt, ...)
> +{
> + va_list ap;
> + char *tmp1, *tmp2;
> +
> + va_start(ap, fmt);
> + xvasprintf(, fmt, ap);
> + va_end(ap);
> +
> + if (*sp == NULL || **sp == '\0') {
> + free(*sp);
> + *sp = tmp1;
> + return;
> + }
> + xasprintf(, "%s%s%s", *sp, sep, tmp1);
> + free(tmp1);
> + free(*sp);
> + *sp = tmp2;
> +}
> +
> +static char *
> +prepare_proctitle(int ac, char **av)
> +{
> + char *ret = NULL;
> + int i;
> +
> + for (i = 0; i < ac; i++)
> + xextendf(, " ", "%s", av[i]);
> + return ret;
> +}
> +
>  /*
>   * Main program for the daemon.
>   */
> @@ -1774,6 +1811,7 @@ main(int ac, char **av)
>   rexec_argv[rexec_argc] = "-R";
>   rexec_argv[rexec_argc + 1] = NULL;
>   }
> + listener_proctitle = prepare_proctitle(ac, av);
>  
>   /* Ensure that umask disallows at least group and world write */
>   new_umask = umask(0077) | 0022;
> 



Re: sshd proctitle [Re: CVS: cvs.openbsd.org: src]

2020-01-23 Thread Damien Miller
On Thu, 23 Jan 2020, Damien Miller wrote:

> On Thu, 23 Jan 2020, Damien Miller wrote:
> 
> > What information would you like there? We could put the first N listen
> > addrs in the proctitle if that would help.
> 
> Maybe like this:
> 
> 63817 ??  S0:00.05 sshd: [listen] on [0.0.0.0]:22, [::]:22, 0 of 
> 10-100

antoine@ said this was not sufficient, so please try the following:

63817 ??  I0:00.09 sshd: /usr/sbin/sshd [listener] 0 of 10-100 startups


diff --git a/sshd.c b/sshd.c
index f6139fe..b7ed0f3 100644
--- a/sshd.c
+++ b/sshd.c
@@ -240,6 +240,8 @@ void destroy_sensitive_data(void);
 void demote_sensitive_data(void);
 static void do_ssh2_kex(struct ssh *);
 
+static char *listener_proctitle;
+
 /*
  * Close all listening sockets
  */
@@ -1032,9 +1034,9 @@ server_accept_loop(int *sock_in, int *sock_out, int 
*newsock, int *config_s)
 */
for (;;) {
if (ostartups != startups) {
-   setproctitle("[listener] %d of %d-%d startups",
-   startups, options.max_startups_begin,
-   options.max_startups);
+   setproctitle("%s [listener] %d of %d-%d startups",
+   listener_proctitle, startups,
+   options.max_startups_begin, options.max_startups);
ostartups = startups;
}
if (received_sighup) {
@@ -1347,6 +1349,41 @@ accumulate_host_timing_secret(struct sshbuf *server_cfg,
sshbuf_free(buf);
 }
 
+static void
+xextendf(char **s, const char *sep, const char *fmt, ...)
+__attribute__((__format__ (printf, 3, 4))) __attribute__((__nonnull__ 
(3)));
+static void
+xextendf(char **sp, const char *sep, const char *fmt, ...)
+{
+   va_list ap;
+   char *tmp1, *tmp2;
+
+   va_start(ap, fmt);
+   xvasprintf(, fmt, ap);
+   va_end(ap);
+
+   if (*sp == NULL || **sp == '\0') {
+   free(*sp);
+   *sp = tmp1;
+   return;
+   }
+   xasprintf(, "%s%s%s", *sp, sep, tmp1);
+   free(tmp1);
+   free(*sp);
+   *sp = tmp2;
+}
+
+static char *
+prepare_proctitle(int ac, char **av)
+{
+   char *ret = NULL;
+   int i;
+
+   for (i = 0; i < ac; i++)
+   xextendf(, " ", "%s", av[i]);
+   return ret;
+}
+
 /*
  * Main program for the daemon.
  */
@@ -1774,6 +1811,7 @@ main(int ac, char **av)
rexec_argv[rexec_argc] = "-R";
rexec_argv[rexec_argc + 1] = NULL;
}
+   listener_proctitle = prepare_proctitle(ac, av);
 
/* Ensure that umask disallows at least group and world write */
new_umask = umask(0077) | 0022;



Re: sshd proctitle [Re: CVS: cvs.openbsd.org: src]

2020-01-22 Thread Antoine Jacoutot
On Thu, Jan 23, 2020 at 02:36:43PM +1100, Damien Miller wrote:
> On Wed, 22 Jan 2020, Stuart Henderson wrote:
> 
> > On 2020/01/21 15:39, Damien Miller wrote:
> > > CVSROOT:  /cvs
> > > Module name:  src
> > > Changes by:   d...@cvs.openbsd.org2020/01/21 15:39:57
> > > 
> > > Modified files:
> > >   usr.bin/ssh: sshd.c 
> > > 
> > > Log message:
> > > expose the number of currently-authenticating connections
> > > along with the MaxStartups limit in the proctitle;
> > > suggestion from Philipp Marek, w/ feedback from Craig Miskell
> > > ok dtucker@
> > > 
> > 
> > It's nice to have this information visible, but it brings some problems.
> > You can't now distinguish between multiple sshd processes (e.g. if you
> > run several on different ports it's hard to figure out which one to
> > signal if needed).
> 
> How could you discern between different sshd processes before? Just the
> command-line args?

Yes.
e.g. for 2 different sshd running:
root 92105  0.0  0.0  1360  1296 ??  I  Wed07AM0:00.05 
/usr/sbin/sshd
root 68236  0.0  0.0  1372  1364 ??  S   7:08AM0:00.00 
/usr/sbin/sshd -f /etc/ssh/sshd_config2

> What information would you like there? We could put the first N listen
> addrs in the proctitle if that would help.

Can't we put the args back and append the new things we expose?
That will also be easier to know which currently-authenticating / MaxStartups
sshd process we are talking about if we run several.
proctitle bit us in the arse several times in the past with rc.d.

My 2 cents, maybe I am talking garbage.

-- 
Antoine



Re: sshd proctitle [Re: CVS: cvs.openbsd.org: src]

2020-01-22 Thread Damien Miller
On Thu, 23 Jan 2020, Damien Miller wrote:

> On Wed, 22 Jan 2020, Stuart Henderson wrote:
> 
> > On 2020/01/21 15:39, Damien Miller wrote:
> > > CVSROOT:  /cvs
> > > Module name:  src
> > > Changes by:   d...@cvs.openbsd.org2020/01/21 15:39:57
> > > 
> > > Modified files:
> > >   usr.bin/ssh: sshd.c 
> > > 
> > > Log message:
> > > expose the number of currently-authenticating connections
> > > along with the MaxStartups limit in the proctitle;
> > > suggestion from Philipp Marek, w/ feedback from Craig Miskell
> > > ok dtucker@
> > > 
> > 
> > It's nice to have this information visible, but it brings some problems.
> > You can't now distinguish between multiple sshd processes (e.g. if you
> > run several on different ports it's hard to figure out which one to
> > signal if needed).
> 
> How could you discern between different sshd processes before? Just the
> command-line args?
> 
> What information would you like there? We could put the first N listen
> addrs in the proctitle if that would help.

Maybe like this:

63817 ??  S0:00.05 sshd: [listen] on [0.0.0.0]:22, [::]:22, 0 of 10-100

ok?

diff --git a/sshd.c b/sshd.c
index ec644c9..15014d1 100644
--- a/sshd.c
+++ b/sshd.c
@@ -240,6 +240,9 @@ void destroy_sensitive_data(void);
 void demote_sensitive_data(void);
 static void do_ssh2_kex(struct ssh *);
 
+/* Listen info for proctitle */
+static char *proctitle_listen_addr;
+
 /*
  * Close all listening sockets
  */
@@ -913,7 +916,7 @@ listen_on_addrs(struct listenaddr *la)
 {
int ret, listen_sock;
struct addrinfo *ai;
-   char ntop[NI_MAXHOST], strport[NI_MAXSERV];
+   char *cp, ntop[NI_MAXHOST], strport[NI_MAXSERV];
 
for (ai = la->addrs; ai; ai = ai->ai_next) {
if (ai->ai_family != AF_INET && ai->ai_family != AF_INET6)
@@ -973,6 +976,15 @@ listen_on_addrs(struct listenaddr *la)
ntop, strport,
la->rdomain == NULL ? "" : " rdomain ",
la->rdomain == NULL ? "" : la->rdomain);
+   if (num_listen_socks < 3) {
+   cp = proctitle_listen_addr;
+   xasprintf(_listen_addr, "%s%s[%s]:%s%s%s",
+   cp == NULL ? "" : cp, cp == NULL ? "" : ", ",
+   ntop, strport,
+   la->rdomain == NULL ? "" : " rdomain ",
+   la->rdomain == NULL ? "" : la->rdomain);
+   free(cp);
+   }
}
 }
 
@@ -1030,7 +1042,9 @@ server_accept_loop(int *sock_in, int *sock_out, int 
*newsock, int *config_s)
 */
for (;;) {
if (ostartups != startups) {
-   setproctitle("[listener] %d of %d-%d startups",
+   setproctitle("[listen] on %s%s, "
+   "%d of %d-%d startups", proctitle_listen_addr,
+   num_listen_socks > 3 ? " [...]" : "",
startups, options.max_startups_begin,
options.max_startups);
ostartups = startups;



Re: sshd proctitle [Re: CVS: cvs.openbsd.org: src]

2020-01-22 Thread Damien Miller
On Wed, 22 Jan 2020, Stuart Henderson wrote:

> On 2020/01/21 15:39, Damien Miller wrote:
> > CVSROOT:/cvs
> > Module name:src
> > Changes by: d...@cvs.openbsd.org2020/01/21 15:39:57
> > 
> > Modified files:
> > usr.bin/ssh: sshd.c 
> > 
> > Log message:
> > expose the number of currently-authenticating connections
> > along with the MaxStartups limit in the proctitle;
> > suggestion from Philipp Marek, w/ feedback from Craig Miskell
> > ok dtucker@
> > 
> 
> It's nice to have this information visible, but it brings some problems.
> You can't now distinguish between multiple sshd processes (e.g. if you
> run several on different ports it's hard to figure out which one to
> signal if needed).

How could you discern between different sshd processes before? Just the
command-line args?

What information would you like there? We could put the first N listen
addrs in the proctitle if that would help.

-d



Re: sshd proctitle [Re: CVS: cvs.openbsd.org: src]

2020-01-22 Thread Theo de Raadt
I guess this needs to be changed again, to retain more info from the
original title.

Stuart Henderson  wrote:

> On 2020/01/21 15:39, Damien Miller wrote:
> > CVSROOT:/cvs
> > Module name:src
> > Changes by: d...@cvs.openbsd.org2020/01/21 15:39:57
> > 
> > Modified files:
> > usr.bin/ssh: sshd.c 
> > 
> > Log message:
> > expose the number of currently-authenticating connections
> > along with the MaxStartups limit in the proctitle;
> > suggestion from Philipp Marek, w/ feedback from Craig Miskell
> > ok dtucker@
> > 
> 
> It's nice to have this information visible, but it brings some problems.
> You can't now distinguish between multiple sshd processes (e.g. if you
> run several on different ports it's hard to figure out which one to
> signal if needed).
> 
> The rc.d script also needs updating because it uses pgrep to find the
> matching process:
> 
> Index: sshd
> ===
> RCS file: /cvs/src/etc/rc.d/sshd,v
> retrieving revision 1.4
> diff -u -p -r1.4 sshd
> --- sshd  11 Jan 2018 19:52:12 -  1.4
> +++ sshd  22 Jan 2020 12:52:15 -
> @@ -6,6 +6,8 @@ daemon="/usr/sbin/sshd"
>  
>  . /etc/rc.d/rc.subr
>  
> +pexp="sshd: \[listener\].*"
> +
>  rc_reload() {
>   ${daemon} ${daemon_flags} -t && pkill -HUP -xf "${pexp}"
>  }
> 
> 



sshd proctitle [Re: CVS: cvs.openbsd.org: src]

2020-01-22 Thread Stuart Henderson
On 2020/01/21 15:39, Damien Miller wrote:
> CVSROOT:  /cvs
> Module name:  src
> Changes by:   d...@cvs.openbsd.org2020/01/21 15:39:57
> 
> Modified files:
>   usr.bin/ssh: sshd.c 
> 
> Log message:
> expose the number of currently-authenticating connections
> along with the MaxStartups limit in the proctitle;
> suggestion from Philipp Marek, w/ feedback from Craig Miskell
> ok dtucker@
> 

It's nice to have this information visible, but it brings some problems.
You can't now distinguish between multiple sshd processes (e.g. if you
run several on different ports it's hard to figure out which one to
signal if needed).

The rc.d script also needs updating because it uses pgrep to find the
matching process:

Index: sshd
===
RCS file: /cvs/src/etc/rc.d/sshd,v
retrieving revision 1.4
diff -u -p -r1.4 sshd
--- sshd11 Jan 2018 19:52:12 -  1.4
+++ sshd22 Jan 2020 12:52:15 -
@@ -6,6 +6,8 @@ daemon="/usr/sbin/sshd"
 
 . /etc/rc.d/rc.subr
 
+pexp="sshd: \[listener\].*"
+
 rc_reload() {
${daemon} ${daemon_flags} -t && pkill -HUP -xf "${pexp}"
 }