On 21/01/20 8:44 pm, Damien Miller wrote:
> On Tue, 21 Jan 2020, Philipp Marek wrote:
>
>>> This makes me think that the syslog approach is probably the way
to go
>>
>> Yeah, right.
>> Another idea is to mirror the current preauth load via
setproctitle()...
>> That makes that data accessible even without a syscall (at least the
>> writing of the data - quering needs syscalls, right), so that can be
>> kept up-to-date and allows a high monitoring frequency as well.
>>
>> Multiple instances of SSHd (on different ports) are easily
distinguished
>> as well.
>
> That's a really, really good idea. Patch below.
That would certainly cover my use case. I wonder if there's a case to
be made to add options.max_startups_begin to the title status (per the
amended patch below). It's more informative, but also a bit more
confusing e.g. when we're above max_startups_begin and beginning to drop
we get a title like:
sshd: [listener] 12/10/100 startups
But my primary goal here is getting that first number exposed and we
could reasonably make max_startups_begin and max_startups the same; for
us, once we start dropping any we may as well be dropping them all,
because it happening at all is a thing to be avoided, and we want
to/should have been alerted before it happens. So the original patch
would certainly be sufficient.
Just an aside: I notice that the original patch got included in commit
f8c11461aa6db168fc5e7eeae448b4cbbf59642a in the portable git repo, along
with another change that matches the commit message. Was that intentional?
-- Craig Miskell Site Reliability Engineer | GitLab | Dunedin, New Zealand
diff --git a/sshd.c b/sshd.c
index 6129b0a..debbdcb 100644
--- a/sshd.c
+++ b/sshd.c
@@ -1005,7 +1005,7 @@ server_accept_loop(int *sock_in, int *sock_out,
int *newsock, int *config_s)
{
fd_set *fdset;
int i, j, ret, maxfd;
- int startups = 0, listening = 0, lameduck = 0;
+ int ostartups = -1, startups = 0, listening = 0, lameduck = 0;
int startup_p[2] = { -1 , -1 };
char c = 0;
struct sockaddr_storage from;
@@ -1029,6 +1029,11 @@ server_accept_loop(int *sock_in, int *sock_out,
int *newsock, int *config_s)
* the daemon is killed with a signal.
*/
for (;;) {
+ if (ostartups != startups) {
+ setproctitle("[listener] %d/%d/%d startups",
+ startups, options.max_startups_begin, options.max_startups);
+ ostartups = startups;
+ }
if (received_sighup) {
if (!lameduck) {
debug("Received SIGHUP; waiting for children");
On Wed, 22 Jan 2020, Craig Miskell wrote:> > On 21/01/20 8:44 pm, Damien Miller wrote: > > On Tue, 21 Jan 2020, Philipp Marek wrote: > > > >>> This makes me think that the syslog approach is probably the way to go > >> > >> Yeah, right. > >> Another idea is to mirror the current preauth load via setproctitle()... > >> That makes that data accessible even without a syscall (at least the > >> writing of the data - quering needs syscalls, right), so that can be > >> kept up-to-date and allows a high monitoring frequency as well. > >> > >> Multiple instances of SSHd (on different ports) are easily distinguished > >> as well. > > > > That's a really, really good idea. Patch below. > > That would certainly cover my use case. I wonder if there's a case to be made > to add options.max_startups_begin to the title status (per the amended patch > below). It's more informative, but also a bit more confusing e.g. when we're > above max_startups_begin and beginning to drop we get a title like: > > sshd: [listener] 12/10/100 startups > > But my primary goal here is getting that first number exposed and we could > reasonably make max_startups_begin and max_startups the same; for us, once we > start dropping any we may as well be dropping them all, because it happening > at all is a thing to be avoided, and we want to/should have been alerted > before it happens. So the original patch would certainly be sufficient.Thanks for the feedback. I've just committed this with the max_startups_begin exposed: 72691 ?? I 0:00.00 sshd: [listener] 0 of 10-100 startups (sshd)> Just an aside: I notice that the original patch got included in commit > f8c11461aa6db168fc5e7eeae448b4cbbf59642a in the portable git repo, along with > another change that matches the commit message. Was that intentional?Nope, and I reverted it before I committed the final one :) -d
>> That would certainly cover my use case. I wonder if there's a case to be made >> to add options.max_startups_begin to the title status (per the amended patch >> below). It's more informative, but also a bit more confusing e.g. when we're >> above max_startups_begin and beginning to drop we get a title like: >> >> sshd: [listener] 12/10/100 startups >> >> But my primary goal here is getting that first number exposed and we could >> reasonably make max_startups_begin and max_startups the same; for us, once we >> start dropping any we may as well be dropping them all, because it happening >> at all is a thing to be avoided, and we want to/should have been alerted >> before it happens. So the original patch would certainly be sufficient. > > Thanks for the feedback. I've just committed this with the max_startups_begin > exposed: > > 72691 ?? I 0:00.00 sshd: [listener] 0 of 10-100 startups (sshd)Magic! Thank you! -- Craig Miskell Site Reliability Engineer | GitLab | Dunedin, New Zealand