NetBSD Problem Report #59200

From www@netbsd.org  Thu Mar 20 21:13:32 2025
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
	 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256
	 client-signature RSA-PSS (2048 bits) client-digest SHA256)
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id B18741A9239
	for <gnats-bugs@gnats.NetBSD.org>; Thu, 20 Mar 2025 21:13:32 +0000 (UTC)
Message-Id: <20250320211331.15FDD1A923D@mollari.NetBSD.org>
Date: Thu, 20 Mar 2025 21:13:31 +0000 (UTC)
From: jlduran@gmail.com
Reply-To: jlduran@gmail.com
To: gnats-bugs@NetBSD.org
Subject: blocklistd: Exit probe location considers a normal session exit as an nfail
X-Send-Pr-Version: www-1.0

>Number:         59200
>Category:       bin
>Synopsis:       blocklistd: Exit probe location considers a normal session exit as an nfail
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    bin-bug-people
>State:          needs-pullups
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Mar 20 21:15:00 +0000 2025
>Closed-Date:    
>Last-Modified:  Tue Mar 25 20:15:00 +0000 2025
>Originator:     Jose Luis Duran
>Release:        10.99.12
>Organization:
>Environment:
NetBSD netbsd.home.arpa 10.99.12 NetBSD 10.99.12 (GENERIC) #1: Wed Mar 19 21:01:56 UTC 2025  jlduran@netbsd.home.arpa:/usr/obj/sys/arch/amd64/compile/GENERIC amd64
>Description:
On a system with blocklistd configured to ban SSH, whenever a normal SSH session exits, a count towards nfail is generated.

>How-To-Repeat:
Configure a system with blocklistd following this procedure:
https://wiki.netbsd.org/tutorials/setting_up_blocklistd/
SSH into the system and exit, inspect the logs (/var/log/messages):

    processing type=4 fd=6 remote=192.0.2.1:1234 msg=ssh uid=0 gid=0
    (type and msg change submitted elsewhere, but irrelevant to this issue)


>Fix:
The fix I have been using is the following:

Subject: [PATCH] blocklistd: Change exit probe location

Move the probe where not only an exit status 255 is checked, but also an
authentication was attempted.  This facility was added by OpenSSH commit
81c1099d2 ("upstream: Add a facility to sshd(8) to penalise particular")
which affords us to remove all occurrences of cleanup exit renumbering,
as well as to avoid counting a normal session exit as an nfail.
---
 crypto/external/bsd/openssh/dist/log.c          | 2 +-
 crypto/external/bsd/openssh/dist/monitor.c      | 2 +-
 crypto/external/bsd/openssh/dist/mux.c          | 2 +-
 crypto/external/bsd/openssh/dist/packet.c       | 2 +-
 crypto/external/bsd/openssh/dist/serverloop.c   | 2 +-
 crypto/external/bsd/openssh/dist/session.c      | 2 +-
 crypto/external/bsd/openssh/dist/sshd-session.c | 7 +++----
 7 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/crypto/external/bsd/openssh/dist/log.c b/crypto/external/bsd/openssh/dist/log.c
index 3476a525bf77..c97f07626fd4 100644
--- a/crypto/external/bsd/openssh/dist/log.c
+++ b/crypto/external/bsd/openssh/dist/log.c
@@ -415,7 +415,7 @@ sshlogdie(const char *file, const char *func, int line, int showfunc,
 	sshlogv(file, func, line, showfunc, SYSLOG_LEVEL_INFO,
 	    suffix, fmt, args);
 	va_end(args);
-	cleanup_exit(254);
+	cleanup_exit(255);
 }

 void
diff --git a/crypto/external/bsd/openssh/dist/monitor.c b/crypto/external/bsd/openssh/dist/monitor.c
index 716cfdb73227..d0222023d9e8 100644
--- a/crypto/external/bsd/openssh/dist/monitor.c
+++ b/crypto/external/bsd/openssh/dist/monitor.c
@@ -1575,7 +1575,7 @@ mm_record_login(struct ssh *ssh, Session *s, struct passwd *pw)
 		if (getpeername(ssh_packet_get_connection_in(ssh),
 		    (struct sockaddr *)&from, &fromlen) == -1) {
 			debug("getpeername: %.100s", strerror(errno));
-			cleanup_exit(254);
+			cleanup_exit(255);
 		}
 	}
 	/* Record that there was a login on that tty from the remote host. */
diff --git a/crypto/external/bsd/openssh/dist/mux.c b/crypto/external/bsd/openssh/dist/mux.c
index b24b838a16cb..ecb366fd57c9 100644
--- a/crypto/external/bsd/openssh/dist/mux.c
+++ b/crypto/external/bsd/openssh/dist/mux.c
@@ -1311,7 +1311,7 @@ muxserver_listen(struct ssh *ssh)
 			return;
 		} else {
 			/* unix_listener() logs the error */
-			cleanup_exit(254);
+			cleanup_exit(255);
 		}
 	}

diff --git a/crypto/external/bsd/openssh/dist/packet.c b/crypto/external/bsd/openssh/dist/packet.c
index dc78a1674283..8ec85ac1fb19 100644
--- a/crypto/external/bsd/openssh/dist/packet.c
+++ b/crypto/external/bsd/openssh/dist/packet.c
@@ -2085,7 +2085,7 @@ ssh_packet_disconnect(struct ssh *ssh, const char *fmt,...)

 	/* Close the connection. */
 	ssh_packet_close(ssh);
-	cleanup_exit(254);
+	cleanup_exit(255);
 }

 /*
diff --git a/crypto/external/bsd/openssh/dist/serverloop.c b/crypto/external/bsd/openssh/dist/serverloop.c
index 9c64ffc21796..39451557e1a5 100644
--- a/crypto/external/bsd/openssh/dist/serverloop.c
+++ b/crypto/external/bsd/openssh/dist/serverloop.c
@@ -289,7 +289,7 @@ process_input(struct ssh *ssh, int connection_in)
 		logit("Read error from remote host %s port %d: %s",
 		    ssh_remote_ipaddr(ssh), ssh_remote_port(ssh),
 		    strerror(errno));
-		cleanup_exit(254);
+		cleanup_exit(255);
 	}
 	return -1;
 }
diff --git a/crypto/external/bsd/openssh/dist/session.c b/crypto/external/bsd/openssh/dist/session.c
index 2c75688750d6..3c162408aadf 100644
--- a/crypto/external/bsd/openssh/dist/session.c
+++ b/crypto/external/bsd/openssh/dist/session.c
@@ -722,7 +722,7 @@ do_login(struct ssh *ssh, Session *s, const char *command)
 		if (getpeername(ssh_packet_get_connection_in(ssh),
 		    (struct sockaddr *)&from, &fromlen) == -1) {
 			debug("getpeername: %.100s", strerror(errno));
-			cleanup_exit(254);
+			cleanup_exit(255);
 		}
 	}

diff --git a/crypto/external/bsd/openssh/dist/sshd-session.c b/crypto/external/bsd/openssh/dist/sshd-session.c
index a9df3d1de4bb..6d94b1ca350f 100644
--- a/crypto/external/bsd/openssh/dist/sshd-session.c
+++ b/crypto/external/bsd/openssh/dist/sshd-session.c
@@ -1452,9 +1452,6 @@ cleanup_exit(int i)
 {
 	extern int auth_attempted; /* monitor.c */

-	if (i == 255)
-		pfilter_notify(1);
-
 	if (the_active_state != NULL && the_authctxt != NULL) {
 		do_cleanup(the_active_state, the_authctxt);
 		if (privsep_is_preauth &&
@@ -1468,7 +1465,9 @@ cleanup_exit(int i)
 		}
 	}
 	/* Override default fatal exit value when auth was attempted */
-	if (i == 255 && auth_attempted)
+	if (i == 255 && auth_attempted) {
+		pfilter_notify(1);
 		_exit(EXIT_AUTH_ATTEMPTED);
+	}
 	_exit(i);
 }
-- 
Jose Luis Duran

>Release-Note:

>Audit-Trail:
From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/59200 CVS commit: src/crypto/external/bsd/openssh/dist
Date: Fri, 21 Mar 2025 10:04:34 -0400

 Module Name:	src
 Committed By:	christos
 Date:		Fri Mar 21 14:04:33 UTC 2025

 Modified Files:
 	src/crypto/external/bsd/openssh/dist: log.c monitor.c mux.c packet.c
 	    serverloop.c session.c sshd-session.c

 Log Message:
 PR/59200: Jose Luis Duran: Change exit probe location

 Move the probe where not only an exit status 255 is checked, but also an
 authentication was attempted.  This facility was added by OpenSSH commit
 81c1099d2 ("upstream: Add a facility to sshd(8) to penalise particular")
 which affords us to remove all occurrences of cleanup exit renumbering,
 as well as to avoid counting a normal session exit as an nfail.


 To generate a diff of this commit:
 cvs rdiff -u -r1.30 -r1.31 src/crypto/external/bsd/openssh/dist/log.c
 cvs rdiff -u -r1.46 -r1.47 src/crypto/external/bsd/openssh/dist/monitor.c
 cvs rdiff -u -r1.36 -r1.37 src/crypto/external/bsd/openssh/dist/mux.c \
     src/crypto/external/bsd/openssh/dist/serverloop.c
 cvs rdiff -u -r1.53 -r1.54 src/crypto/external/bsd/openssh/dist/packet.c
 cvs rdiff -u -r1.40 -r1.41 src/crypto/external/bsd/openssh/dist/session.c
 cvs rdiff -u -r1.6 -r1.7 src/crypto/external/bsd/openssh/dist/sshd-session.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/59200 CVS commit: src/crypto/external/bsd/openssh/dist
Date: Fri, 21 Mar 2025 10:06:14 -0400

 Module Name:	src
 Committed By:	christos
 Date:		Fri Mar 21 14:06:14 UTC 2025

 Modified Files:
 	src/crypto/external/bsd/openssh/dist: sshd-session.c

 Log Message:
 PR/59200: Jose Luis Duran: A lot of bots are checking for SSH version 1
 (or mismatched), and currently blocklistd does not mark them as an nfail,
 spamming the console/logs.


 To generate a diff of this commit:
 cvs rdiff -u -r1.7 -r1.8 src/crypto/external/bsd/openssh/dist/sshd-session.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->needs-pullups
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Tue, 25 Mar 2025 19:58:50 +0000
State-Changed-Why:
Does this warrant pullup-9 or pullup-10?


From: Jose Luis Duran <jlduran@gmail.com>
To: gnats-bugs@netbsd.org
Cc: gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, riastradh@netbsd.org
Subject: Re: bin/59200 (blocklistd: Exit probe location considers a normal
 session exit as an nfail)
Date: Tue, 25 Mar 2025 17:11:45 -0300

 On Tue, Mar 25, 2025 at 4:58=E2=80=AFPM <riastradh@netbsd.org> wrote:
 >
 > Synopsis: blocklistd: Exit probe location considers a normal session exit=
  as an nfail
 >
 > State-Changed-From-To: open->needs-pullups
 > State-Changed-By: riastradh@NetBSD.org
 > State-Changed-When: Tue, 25 Mar 2025 19:58:50 +0000
 > State-Changed-Why:
 > Does this warrant pullup-9 or pullup-10?

 Not that I have a vote, but in my opinion, yes! I'll submit two more
 probe changes soon (one for ftpd and one for sshd, still being
 tested). I also plan on "pulling-up" on FreeBSD.

 Thank you!

>Unformatted:

Home	PR Database Search