Index | Thread | Search

From:
Damien Miller <djm@mindrot.org>
Subject:
openssh: fractional-second PerSourcePenalties part 2
To:
tech@openbsd.org
Cc:
openssh@openssh.com
Date:
Fri, 28 Nov 2025 16:23:24 +1100

Download raw body.

Thread
Hi,

This is the other part of adding fractional-second PerSourcePenalties
to sshd: actually converting the existing code to microseconds.

This is pretty mechanical, though the config parsing is a little
fiddly as we now need to parse two kinds of integer values from
the config: int64_t intervals and int for everything else.

ok?

diff --git a/servconf.c b/servconf.c
index f9003e6..6c82ab6 100644
--- a/servconf.c
+++ b/servconf.c
@@ -400,19 +400,19 @@ fill_default_server_options(ServerOptions *options)
 	if (options->per_source_penalty.overflow_mode6 == -1)
 		options->per_source_penalty.overflow_mode6 = options->per_source_penalty.overflow_mode;
 	if (options->per_source_penalty.penalty_crash == -1)
-		options->per_source_penalty.penalty_crash = 90;
+		options->per_source_penalty.penalty_crash = 90 * 1000000LL;
 	if (options->per_source_penalty.penalty_grace == -1)
-		options->per_source_penalty.penalty_grace = 10;
+		options->per_source_penalty.penalty_grace = 10 * 1000000LL;
 	if (options->per_source_penalty.penalty_authfail == -1)
-		options->per_source_penalty.penalty_authfail = 5;
+		options->per_source_penalty.penalty_authfail = 5 * 1000000LL;
 	if (options->per_source_penalty.penalty_noauth == -1)
-		options->per_source_penalty.penalty_noauth = 1;
+		options->per_source_penalty.penalty_noauth = 1 * 1000000LL;
 	if (options->per_source_penalty.penalty_refuseconnection == -1)
-		options->per_source_penalty.penalty_refuseconnection = 10;
+		options->per_source_penalty.penalty_refuseconnection = 10 * 1000000LL;
 	if (options->per_source_penalty.penalty_min == -1)
-		options->per_source_penalty.penalty_min = 15;
+		options->per_source_penalty.penalty_min = 15 * 1000000LL;
 	if (options->per_source_penalty.penalty_max == -1)
-		options->per_source_penalty.penalty_max = 600;
+		options->per_source_penalty.penalty_max = 600 * 1000000LL;
 	if (options->max_authtries == -1)
 		options->max_authtries = DEFAULT_AUTH_FAIL_MAX;
 	if (options->max_sessions == -1)
@@ -1264,6 +1264,7 @@ process_server_config_line_depth(ServerOptions *options, char *line,
 	u_int i, *uintptr, flags = 0;
 	size_t len;
 	long long val64;
+	int64_t *val64ptr;
 	const struct multistate *multistate_ptr;
 	const char *errstr;
 	struct include_item *item;
@@ -2012,7 +2013,10 @@ process_server_config_line_depth(ServerOptions *options, char *line,
 
 			found = 1;
 			value = -1;
+			val64 = -1;
 			value2 = 0;
+			intptr = NULL;
+			val64ptr = NULL;
 			/* Allow no/yes only in first position */
 			if (strcasecmp(arg, "no") == 0 ||
 			    (value2 = (strcasecmp(arg, "yes") == 0))) {
@@ -2026,19 +2030,19 @@ process_server_config_line_depth(ServerOptions *options, char *line,
 					options->per_source_penalty.enabled = value2;
 				continue;
 			} else if ((q = strprefix(arg, "crash:", 0)) != NULL) {
-				intptr = &options->per_source_penalty.penalty_crash;
+				val64ptr = &options->per_source_penalty.penalty_crash;
 			} else if ((q = strprefix(arg, "authfail:", 0)) != NULL) {
-				intptr = &options->per_source_penalty.penalty_authfail;
+				val64ptr = &options->per_source_penalty.penalty_authfail;
 			} else if ((q = strprefix(arg, "noauth:", 0)) != NULL) {
-				intptr = &options->per_source_penalty.penalty_noauth;
+				val64ptr = &options->per_source_penalty.penalty_noauth;
 			} else if ((q = strprefix(arg, "grace-exceeded:", 0)) != NULL) {
-				intptr = &options->per_source_penalty.penalty_grace;
+				val64ptr = &options->per_source_penalty.penalty_grace;
 			} else if ((q = strprefix(arg, "refuseconnection:", 0)) != NULL) {
-				intptr = &options->per_source_penalty.penalty_refuseconnection;
+				val64ptr = &options->per_source_penalty.penalty_refuseconnection;
 			} else if ((q = strprefix(arg, "max:", 0)) != NULL) {
-				intptr = &options->per_source_penalty.penalty_max;
+				val64ptr = &options->per_source_penalty.penalty_max;
 			} else if ((q = strprefix(arg, "min:", 0)) != NULL) {
-				intptr = &options->per_source_penalty.penalty_min;
+				val64ptr = &options->per_source_penalty.penalty_min;
 			} else if ((q = strprefix(arg, "max-sources4:", 0)) != NULL) {
 				intptr = &options->per_source_penalty.max_sources4;
 				if ((errstr = atoi_err(q, &value)) != NULL)
@@ -2065,13 +2069,24 @@ process_server_config_line_depth(ServerOptions *options, char *line,
 				fatal("%s line %d: unsupported %s keyword %s",
 				    filename, linenum, keyword, arg);
 			}
-			/* If no value was parsed above, assume it's a time */
-			if (value == -1 && (value = convtime(q)) == -1) {
-				fatal("%s line %d: invalid %s time value.",
-				    filename, linenum, keyword);
+			if (val64ptr != NULL) {
+				/* It's a penalty time */
+				if ((val64 = convtime_usec(q)) == -1) {
+					fatal("%s line %d: invalid %s time value.",
+					    filename, linenum, keyword);
+				}
+				if (*activep && *val64ptr == -1)
+					*val64ptr = val64;
+			} else if (intptr != NULL) {
+				/* It's a non-time integer value */
+				if (*activep && *intptr == -1)
+					*intptr = value;
+			} else {
+				/* It's a screwup */
+				fatal_f("%s line %d: internal error",
+				    filename, linenum);
 			}
-			if (*activep && *intptr == -1) {
-				*intptr = value;
+			if (*activep) {
 				/* any option implicitly enables penalties */
 				options->per_source_penalty.enabled = 1;
 			}
@@ -3319,17 +3334,17 @@ dump_config(ServerOptions *o)
 	printf("\n");
 
 	if (o->per_source_penalty.enabled) {
-		printf("persourcepenalties crash:%d authfail:%d noauth:%d "
-		    "grace-exceeded:%d refuseconnection:%d max:%d min:%d "
+		printf("persourcepenalties crash:%.6f authfail:%.6f noauth:%.6f "
+		    "grace-exceeded:%.6f refuseconnection:%.6f max:%.6f min:%.6f "
 		    "max-sources4:%d max-sources6:%d "
 		    "overflow:%s overflow6:%s\n",
-		    o->per_source_penalty.penalty_crash,
-		    o->per_source_penalty.penalty_authfail,
-		    o->per_source_penalty.penalty_noauth,
-		    o->per_source_penalty.penalty_grace,
-		    o->per_source_penalty.penalty_refuseconnection,
-		    o->per_source_penalty.penalty_max,
-		    o->per_source_penalty.penalty_min,
+		    (double)o->per_source_penalty.penalty_crash / 1000000.0,
+		    (double)o->per_source_penalty.penalty_authfail / 1000000.0,
+		    (double)o->per_source_penalty.penalty_noauth / 1000000.0,
+		    (double)o->per_source_penalty.penalty_grace / 1000000.0,
+		    (double)o->per_source_penalty.penalty_refuseconnection / 1000000.0,
+		    (double)o->per_source_penalty.penalty_max / 1000000.0,
+		    (double)o->per_source_penalty.penalty_min / 1000000.0,
 		    o->per_source_penalty.max_sources4,
 		    o->per_source_penalty.max_sources6,
 		    o->per_source_penalty.overflow_mode ==
diff --git a/servconf.h b/servconf.h
index f05f5f3..462c591 100644
--- a/servconf.h
+++ b/servconf.h
@@ -73,13 +73,13 @@ struct per_source_penalty {
 	int	max_sources6;
 	int	overflow_mode;
 	int	overflow_mode6;
-	int	penalty_crash;
-	int	penalty_grace;
-	int	penalty_authfail;
-	int	penalty_noauth;
-	int	penalty_refuseconnection;
-	int	penalty_max;
-	int	penalty_min;
+	int64_t	penalty_crash;
+	int64_t	penalty_grace;
+	int64_t	penalty_authfail;
+	int64_t	penalty_noauth;
+	int64_t	penalty_refuseconnection;
+	int64_t	penalty_max;
+	int64_t	penalty_min;
 };
 
 typedef struct {
diff --git a/srclimit.c b/srclimit.c
index ddbf767..6b59b0f 100644
--- a/srclimit.c
+++ b/srclimit.c
@@ -51,7 +51,7 @@ static struct child_info {
  */
 struct penalty {
 	struct xaddr addr;
-	time_t expiry;
+	int64_t expiry;
 	int active;
 	const char *reason;
 	RB_ENTRY(penalty) by_addr;
@@ -210,7 +210,7 @@ penalty_expiry_cmp(struct penalty *a, struct penalty *b)
 }
 
 static void
-expire_penalties_from_tree(time_t now, const char *t,
+expire_penalties_from_tree(int64_t now, const char *t,
     struct penalties_by_expiry *by_expiry,
     struct penalties_by_addr *by_addr, size_t *npenaltiesp)
 {
@@ -258,7 +258,7 @@ srclimit_penalty_check_allow(int sock, const char **reason)
 {
 	struct xaddr addr;
 	struct penalty find, *penalty;
-	time_t now;
+	int64_t now;
 	int bits, max_sources, overflow_mode;
 	char addr_s[NI_MAXHOST];
 	struct penalties_by_addr *by_addr;
@@ -275,7 +275,7 @@ srclimit_penalty_check_allow(int sock, const char **reason)
 			return 1;
 		}
 	}
-	now = monotime();
+	now = monotime_usec();
 	expire_penalties(now);
 	by_addr = addr.af == AF_INET ?
 	    &penalties_by_addr4 : &penalties_by_addr6;
@@ -345,8 +345,8 @@ srclimit_penalise(struct xaddr *addr, int penalty_type)
 {
 	struct xaddr masked;
 	struct penalty *penalty = NULL, *existing = NULL;
-	time_t now;
-	int bits, penalty_secs, max_sources = 0, overflow_mode;
+	int64_t now, penalty_usecs;
+	int bits, max_sources = 0, overflow_mode;
 	char addrnetmask[NI_MAXHOST + 4];
 	const char *reason = NULL, *t;
 	size_t *npenaltiesp = NULL;
@@ -368,23 +368,23 @@ srclimit_penalise(struct xaddr *addr, int penalty_type)
 	case SRCLIMIT_PENALTY_NONE:
 		return;
 	case SRCLIMIT_PENALTY_CRASH:
-		penalty_secs = penalty_cfg.penalty_crash;
+		penalty_usecs = penalty_cfg.penalty_crash;
 		reason = "penalty: caused crash";
 		break;
 	case SRCLIMIT_PENALTY_AUTHFAIL:
-		penalty_secs = penalty_cfg.penalty_authfail;
+		penalty_usecs = penalty_cfg.penalty_authfail;
 		reason = "penalty: failed authentication";
 		break;
 	case SRCLIMIT_PENALTY_NOAUTH:
-		penalty_secs = penalty_cfg.penalty_noauth;
+		penalty_usecs = penalty_cfg.penalty_noauth;
 		reason = "penalty: connections without attempting authentication";
 		break;
 	case SRCLIMIT_PENALTY_REFUSECONNECTION:
-		penalty_secs = penalty_cfg.penalty_refuseconnection;
+		penalty_usecs = penalty_cfg.penalty_refuseconnection;
 		reason = "penalty: connection prohibited by RefuseConnection";
 		break;
 	case SRCLIMIT_PENALTY_GRACE_EXCEEDED:
-		penalty_secs = penalty_cfg.penalty_grace;
+		penalty_usecs = penalty_cfg.penalty_grace;
 		reason = "penalty: exceeded LoginGraceTime";
 		break;
 	default:
@@ -395,7 +395,7 @@ srclimit_penalise(struct xaddr *addr, int penalty_type)
 		return;
 	addr_masklen_ntop(addr, bits, addrnetmask, sizeof(addrnetmask));
 
-	now = monotime();
+	now = monotime_usec();
 	expire_penalties(now);
 	by_expiry = addr->af == AF_INET ?
 	    &penalties_by_expiry4 : &penalties_by_expiry6;
@@ -416,38 +416,39 @@ srclimit_penalise(struct xaddr *addr, int penalty_type)
 
 	penalty = xcalloc(1, sizeof(*penalty));
 	penalty->addr = masked;
-	penalty->expiry = now + penalty_secs;
+	penalty->expiry = now + penalty_usecs;
 	penalty->reason = reason;
 	if ((existing = RB_INSERT(penalties_by_addr, by_addr,
 	    penalty)) == NULL) {
 		/* penalty didn't previously exist */
-		if (penalty_secs > penalty_cfg.penalty_min)
+		if (penalty_usecs > penalty_cfg.penalty_min)
 			penalty->active = 1;
 		if (RB_INSERT(penalties_by_expiry, by_expiry, penalty) != NULL)
 			fatal_f("internal error: %s penalty tables corrupt", t);
 		do_log2_f(penalty->active ?
 		    SYSLOG_LEVEL_INFO : SYSLOG_LEVEL_VERBOSE,
-		    "%s: new %s %s penalty of %d seconds for %s", t,
+		    "%s: new %s %s penalty of %.3f seconds for %s", t,
 		    addrnetmask, penalty->active ? "active" : "deferred",
-		    penalty_secs, reason);
+		    (double)penalty_usecs / 1000000.0, reason);
 		if (++(*npenaltiesp) > (size_t)max_sources)
 			srclimit_early_expire_penalties(); /* permissive */
 		return;
 	}
-	debug_f("%s penalty for %s %s already exists, %lld seconds remaining",
+	debug_f("%s penalty for %s %s already exists, %.3f seconds remaining",
 	    existing->active ? "active" : "inactive", t,
-	    addrnetmask, (long long)(existing->expiry - now));
+	    addrnetmask, (double)(existing->expiry - now) / 1000000.0);
 	/* Expiry information is about to change, remove from tree */
 	if (RB_REMOVE(penalties_by_expiry, by_expiry, existing) != existing)
 		fatal_f("internal error: %s penalty table corrupt (remove)", t);
 	/* An entry already existed. Accumulate penalty up to maximum */
-	existing->expiry += penalty_secs;
+	existing->expiry += penalty_usecs;
 	if (existing->expiry - now > penalty_cfg.penalty_max)
 		existing->expiry = now + penalty_cfg.penalty_max;
 	if (existing->expiry - now > penalty_cfg.penalty_min &&
 	    !existing->active) {
-		logit_f("%s: activating %s penalty of %lld seconds for %s",
-		    addrnetmask, t, (long long)(existing->expiry - now),
+		logit_f("%s: activating %s penalty of %.3f seconds for %s",
+		    addrnetmask, t,
+		    (double)((existing->expiry - now) / 1000000.0),
 		    reason);
 		existing->active = 1;
 	}
@@ -466,9 +467,9 @@ srclimit_penalty_info_for_tree(const char *t,
 	struct penalty *p = NULL;
 	int bits;
 	char s[NI_MAXHOST + 4];
-	time_t now;
+	int64_t now;
 
-	now = monotime();
+	now = monotime_usec();
 	logit("%zu active %s penalties", npenalties, t);
 	RB_FOREACH(p, penalties_by_expiry, by_expiry) {
 		bits = p->addr.af == AF_INET ? ipv4_masklen : ipv6_masklen;
@@ -476,8 +477,8 @@ srclimit_penalty_info_for_tree(const char *t,
 		if (p->expiry < now)
 			logit("client %s %s (expired)", s, p->reason);
 		else {
-			logit("client %s %s (%llu secs left)", s, p->reason,
-			   (long long)(p->expiry - now));
+			logit("client %s %s (%.3f secs left)", s, p->reason,
+			   (double)(p->expiry - now) / 1000000.0);
 		}
 	}
 }