Download raw body.
openssh: fractional-second PerSourcePenalties part 1
On Mon, 1 Dec 2025, Theo de Raadt wrote:
> > index 20eb305..b2276bb 100644
> > --- a/misc.c
> > +++ b/misc.c
> > @@ -619,6 +619,9 @@ convtime_usec(const char *s)
> > errno = 0;
> > if ((val = strtod(p, &endp)) < 0 || errno != 0 || p == endp)
> > return -1;
> > + /* Allow only decimal forms */
> > + if (p + strspn(p, "0123456789.") != endp)
> > + return -1;
> > start_p = p;
> > p = endp;
>
> That looks like a reasonable way of catching the problem. It removes the
> ability to exercise -, +, NaN Infinity, ( ). It assumes the whitespace is
> earlier. It pins the trailing multiplier character.
finally (?), Darren asked for a version that skips the usecs entirely
and passes intervals as double throughout. This turns out to be a bit
simpler again.
ok?
Index: usr.bin/ssh/misc.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/misc.c,v
diff -u -p -r1.209 misc.c
--- usr.bin/ssh/misc.c 6 Nov 2025 01:31:11 -0000 1.209
+++ usr.bin/ssh/misc.c 2 Dec 2025 03:49:58 -0000
@@ -576,25 +576,22 @@ a2tun(const char *s, int *remote)
return (tun);
}
-#define SECONDS 1
+#define SECONDS 1.0
#define MINUTES (SECONDS * 60)
#define HOURS (MINUTES * 60)
#define DAYS (HOURS * 24)
#define WEEKS (DAYS * 7)
-static char *
-scandigits(char *s)
-{
- while (isdigit((unsigned char)*s))
- s++;
- return s;
-}
-
/*
- * Convert a time string into seconds; format is
- * a sequence of:
+ * Convert an interval/duration time string into seconds, which may include
+ * fractional seconds.
+ *
+ * The format is a sequence of:
* time[qualifier]
*
+ * This supports fractional values for the seconds value only. All other
+ * values must be integers.
+ *
* Valid time qualifiers are:
* <none> seconds
* s|S seconds
@@ -604,44 +601,46 @@ scandigits(char *s)
* w|W weeks
*
* Examples:
- * 90m 90 minutes
- * 1h30m 90 minutes
- * 2d 2 days
- * 1w 1 week
+ * 90m 90 minutes
+ * 1h30m 90 minutes
+ * 1.5s 1.5 seconds
+ * 2d 2 days
+ * 1w 1 week
*
- * Return -1 if time string is invalid.
+ * Returns <0.0 if the time string is invalid.
*/
-int
-convtime(const char *s)
+double
+convtime_double(const char *s)
{
- int secs, total = 0, multiplier;
- char *p, *os, *np, c = 0;
- const char *errstr;
+ double val, total_sec = 0.0, multiplier;
+ const char *p, *start_p;
+ char *endp;
+ int seen_seconds = 0;
if (s == NULL || *s == '\0')
- return -1;
- p = os = strdup(s); /* deal with const */
- if (os == NULL)
- return -1;
+ return -1.0;
- while (*p) {
- np = scandigits(p);
- if (np) {
- c = *np;
- *np = '\0';
- }
- secs = (int)strtonum(p, 0, INT_MAX, &errstr);
- if (errstr)
- goto fail;
- *np = c;
+ for (p = s; *p != '\0';) {
+ if (!isdigit((unsigned char)*p) && *p != '.')
+ return -1.0;
+
+ errno = 0;
+ if ((val = strtod(p, &endp)) < 0 || errno != 0 || p == endp)
+ return -1.0;
+ /* Allow only decimal forms */
+ if (p + strspn(p, "0123456789.") != endp)
+ return -1.0;
+ start_p = p;
+ p = endp;
- multiplier = 1;
- switch (c) {
+ switch (*p) {
case '\0':
- np--; /* back up */
- break;
+ /* FALLTHROUGH */
case 's':
case 'S':
+ if (seen_seconds++)
+ return -1.0;
+ multiplier = SECONDS;
break;
case 'm':
case 'M':
@@ -660,23 +659,44 @@ convtime(const char *s)
multiplier = WEEKS;
break;
default:
- goto fail;
+ return -1.0;
+ }
+
+ /* Special handling if this was a decimal */
+ if (memchr(start_p, '.', endp - start_p) != NULL) {
+ /* Decimal point present */
+ if (multiplier > 1.0)
+ return -1.0; /* No fractionals for non-seconds */
+ /* For seconds, ensure digits follow */
+ if (!isdigit((unsigned char)*(endp - 1)))
+ return -1.0;
}
- if (secs > INT_MAX / multiplier)
- goto fail;
- secs *= multiplier;
- if (total > INT_MAX - secs)
- goto fail;
- total += secs;
- if (total < 0)
- goto fail;
- p = ++np;
+
+ total_sec += val * multiplier;
+
+ if (*p != '\0')
+ p++;
}
- free(os);
- return total;
-fail:
- free(os);
- return -1;
+ return total_sec;
+}
+
+/*
+ * Same as convtime_double() above but fractional seconds are ignored.
+ * Return -1 if time string is invalid.
+ */
+int
+convtime(const char *s)
+{
+ double sec_val;
+
+ if ((sec_val = convtime_double(s)) < 0.0)
+ return -1;
+
+ /* Check for overflow into int */
+ if (sec_val < 0 || sec_val > INT_MAX)
+ return -1;
+
+ return (int)sec_val;
}
#define TF_BUFS 8
Index: usr.bin/ssh/misc.h
===================================================================
RCS file: /cvs/src/usr.bin/ssh/misc.h,v
diff -u -p -r1.113 misc.h
--- usr.bin/ssh/misc.h 6 Nov 2025 01:31:11 -0000 1.113
+++ usr.bin/ssh/misc.h 2 Dec 2025 03:49:58 -0000
@@ -82,6 +82,7 @@ int parse_user_host_path(const char *,
int parse_user_host_port(const char *, char **, char **, int *);
int parse_uri(const char *, const char *, char **, char **, int *, char **);
int convtime(const char *);
+double convtime_double(const char *);
const char *fmt_timeframe(time_t t);
int tilde_expand(const char *, uid_t, char **);
char *tilde_expand_filename(const char *, uid_t);
Index: usr.bin/ssh/servconf.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/servconf.c,v
diff -u -p -r1.437 servconf.c
--- usr.bin/ssh/servconf.c 25 Nov 2025 00:52:00 -0000 1.437
+++ usr.bin/ssh/servconf.c 2 Dec 2025 03:49:58 -0000
@@ -152,13 +152,13 @@ initialize_server_options(ServerOptions
options->per_source_penalty.max_sources6 = -1;
options->per_source_penalty.overflow_mode = -1;
options->per_source_penalty.overflow_mode6 = -1;
- options->per_source_penalty.penalty_crash = -1;
- options->per_source_penalty.penalty_authfail = -1;
- options->per_source_penalty.penalty_noauth = -1;
- options->per_source_penalty.penalty_grace = -1;
- options->per_source_penalty.penalty_refuseconnection = -1;
- options->per_source_penalty.penalty_max = -1;
- options->per_source_penalty.penalty_min = -1;
+ options->per_source_penalty.penalty_crash = -1.0;
+ options->per_source_penalty.penalty_authfail = -1.0;
+ options->per_source_penalty.penalty_noauth = -1.0;
+ options->per_source_penalty.penalty_grace = -1.0;
+ options->per_source_penalty.penalty_refuseconnection = -1.0;
+ options->per_source_penalty.penalty_max = -1.0;
+ options->per_source_penalty.penalty_min = -1.0;
options->max_authtries = -1;
options->max_sessions = -1;
options->banner = NULL;
@@ -399,20 +399,20 @@ fill_default_server_options(ServerOption
options->per_source_penalty.overflow_mode = PER_SOURCE_PENALTY_OVERFLOW_PERMISSIVE;
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;
- if (options->per_source_penalty.penalty_grace == -1)
- options->per_source_penalty.penalty_grace = 10;
- if (options->per_source_penalty.penalty_authfail == -1)
- options->per_source_penalty.penalty_authfail = 5;
- if (options->per_source_penalty.penalty_noauth == -1)
- options->per_source_penalty.penalty_noauth = 1;
- if (options->per_source_penalty.penalty_refuseconnection == -1)
- options->per_source_penalty.penalty_refuseconnection = 10;
- if (options->per_source_penalty.penalty_min == -1)
- options->per_source_penalty.penalty_min = 15;
- if (options->per_source_penalty.penalty_max == -1)
- options->per_source_penalty.penalty_max = 600;
+ if (options->per_source_penalty.penalty_crash < 0.0)
+ options->per_source_penalty.penalty_crash = 90.0;
+ if (options->per_source_penalty.penalty_grace < 0.0)
+ options->per_source_penalty.penalty_grace = 10.0;
+ if (options->per_source_penalty.penalty_authfail < 0.0)
+ options->per_source_penalty.penalty_authfail = 5.0;
+ if (options->per_source_penalty.penalty_noauth < 0.0)
+ options->per_source_penalty.penalty_noauth = 1.0;
+ if (options->per_source_penalty.penalty_refuseconnection < 0.0)
+ options->per_source_penalty.penalty_refuseconnection = 10.0;
+ if (options->per_source_penalty.penalty_min < 0.0)
+ options->per_source_penalty.penalty_min = 15.0;
+ if (options->per_source_penalty.penalty_max < 0.0)
+ options->per_source_penalty.penalty_max = 600.0;
if (options->max_authtries == -1)
options->max_authtries = DEFAULT_AUTH_FAIL_MAX;
if (options->max_sessions == -1)
@@ -1257,6 +1257,7 @@ process_server_config_line_depth(ServerO
{
char *str, ***chararrayptr, **charptr, *arg, *arg2, *p, *keyword;
int cmdline = 0, *intptr, value, value2, value3, n, port, oactive, r;
+ double dvalue, *doubleptr = NULL;
int ca_only = 0, found = 0;
SyslogFacility *log_facility_ptr;
LogLevel *log_level_ptr;
@@ -2011,7 +2012,8 @@ process_server_config_line_depth(ServerO
const char *q = NULL;
found = 1;
- value = -1;
+ intptr = NULL;
+ doubleptr = NULL;
value2 = 0;
/* Allow no/yes only in first position */
if (strcasecmp(arg, "no") == 0 ||
@@ -2026,19 +2028,19 @@ process_server_config_line_depth(ServerO
options->per_source_penalty.enabled = value2;
continue;
} else if ((q = strprefix(arg, "crash:", 0)) != NULL) {
- intptr = &options->per_source_penalty.penalty_crash;
+ doubleptr = &options->per_source_penalty.penalty_crash;
} else if ((q = strprefix(arg, "authfail:", 0)) != NULL) {
- intptr = &options->per_source_penalty.penalty_authfail;
+ doubleptr = &options->per_source_penalty.penalty_authfail;
} else if ((q = strprefix(arg, "noauth:", 0)) != NULL) {
- intptr = &options->per_source_penalty.penalty_noauth;
+ doubleptr = &options->per_source_penalty.penalty_noauth;
} else if ((q = strprefix(arg, "grace-exceeded:", 0)) != NULL) {
- intptr = &options->per_source_penalty.penalty_grace;
+ doubleptr = &options->per_source_penalty.penalty_grace;
} else if ((q = strprefix(arg, "refuseconnection:", 0)) != NULL) {
- intptr = &options->per_source_penalty.penalty_refuseconnection;
+ doubleptr = &options->per_source_penalty.penalty_refuseconnection;
} else if ((q = strprefix(arg, "max:", 0)) != NULL) {
- intptr = &options->per_source_penalty.penalty_max;
+ doubleptr = &options->per_source_penalty.penalty_max;
} else if ((q = strprefix(arg, "min:", 0)) != NULL) {
- intptr = &options->per_source_penalty.penalty_min;
+ doubleptr = &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,15 +2067,24 @@ process_server_config_line_depth(ServerO
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 (*activep && *intptr == -1) {
- *intptr = value;
- /* any option implicitly enables penalties */
- options->per_source_penalty.enabled = 1;
+
+ if (doubleptr != NULL) {
+ if ((dvalue = convtime_double(q)) < 0) {
+ fatal("%s line %d: invalid %s time value.",
+ filename, linenum, keyword);
+ }
+ if (*activep && *doubleptr < 0.0) {
+ *doubleptr = dvalue;
+ options->per_source_penalty.enabled = 1;
+ }
+ } else if (intptr != NULL) {
+ if (*activep && *intptr == -1) {
+ *intptr = value;
+ options->per_source_penalty.enabled = 1;
+ }
+ } else {
+ fatal_f("%s line %d: internal error",
+ filename, linenum);
}
}
if (!found) {
@@ -3319,8 +3330,8 @@ 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:%f authfail:%f noauth:%f "
+ "grace-exceeded:%f refuseconnection:%f max:%f min:%f "
"max-sources4:%d max-sources6:%d "
"overflow:%s overflow6:%s\n",
o->per_source_penalty.penalty_crash,
Index: usr.bin/ssh/servconf.h
===================================================================
RCS file: /cvs/src/usr.bin/ssh/servconf.h,v
diff -u -p -r1.169 servconf.h
--- usr.bin/ssh/servconf.h 14 Oct 2024 01:57:50 -0000 1.169
+++ usr.bin/ssh/servconf.h 2 Dec 2025 03:49:58 -0000
@@ -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;
+ double penalty_crash;
+ double penalty_grace;
+ double penalty_authfail;
+ double penalty_noauth;
+ double penalty_refuseconnection;
+ double penalty_max;
+ double penalty_min;
};
typedef struct {
Index: usr.bin/ssh/srclimit.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/srclimit.c,v
diff -u -p -r1.13 srclimit.c
--- usr.bin/ssh/srclimit.c 19 Sep 2025 01:32:45 -0000 1.13
+++ usr.bin/ssh/srclimit.c 2 Dec 2025 03:49:58 -0000
@@ -346,7 +346,8 @@ srclimit_penalise(struct xaddr *addr, in
struct xaddr masked;
struct penalty *penalty = NULL, *existing = NULL;
time_t now;
- int bits, penalty_secs, max_sources = 0, overflow_mode;
+ int bits, max_sources = 0, overflow_mode;
+ double penalty_secs;
char addrnetmask[NI_MAXHOST + 4];
const char *reason = NULL, *t;
size_t *npenaltiesp = NULL;
@@ -427,7 +428,7 @@ srclimit_penalise(struct xaddr *addr, in
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 %f seconds for %s", t,
addrnetmask, penalty->active ? "active" : "deferred",
penalty_secs, reason);
if (++(*npenaltiesp) > (size_t)max_sources)
Index: regress/usr.bin/ssh/unittests/misc/test_convtime.c
===================================================================
RCS file: /cvs/src/regress/usr.bin/ssh/unittests/misc/test_convtime.c,v
diff -u -p -r1.3 test_convtime.c
--- regress/usr.bin/ssh/unittests/misc/test_convtime.c 11 Aug 2022 01:57:50 -0000 1.3
+++ regress/usr.bin/ssh/unittests/misc/test_convtime.c 2 Dec 2025 03:49:58 -0000
@@ -55,6 +55,43 @@ test_convtime(void)
#endif
TEST_DONE();
+ TEST_START("misc_convtime_double");
+ ASSERT_DOUBLE_EQ(convtime_double("0"), 0);
+ ASSERT_DOUBLE_EQ(convtime_double("1"), 1.0);
+ ASSERT_DOUBLE_EQ(convtime_double("2s"), 2.0);
+ ASSERT_DOUBLE_EQ(convtime_double("3m"), 180.0);
+ ASSERT_DOUBLE_EQ(convtime_double("1m30s"), 90.0);
+ ASSERT_DOUBLE_EQ(convtime_double("1.5s"), 1.5);
+ ASSERT_DOUBLE_EQ(convtime_double(".5s"), 0.5);
+ ASSERT_DOUBLE_EQ(convtime_double("0.5s"), 0.5);
+ ASSERT_DOUBLE_EQ(convtime_double("1.123456s"), 1.123456);
+ ASSERT_DOUBLE_EQ(convtime_double("1.1234567s"), 1.1234567);
+ ASSERT_DOUBLE_EQ(convtime_double("1.123s"), 1.123);
+ ASSERT_DOUBLE_EQ(convtime_double("1m0.5s"), 60.5);
+ ASSERT_DOUBLE_EQ(convtime_double("1m.5s"), 60.5);
+ ASSERT_DOUBLE_EQ(convtime_double("1.5"), 1.5);
+ ASSERT_DOUBLE_EQ(convtime_double("1.123456"), 1.123456);
+ ASSERT_DOUBLE_EQ(convtime_double("1.1234567"), 1.1234567);
+ /* errors */
+ ASSERT_DOUBLE_LT(convtime_double(""), 0.0);
+ ASSERT_DOUBLE_LT(convtime_double("trout"), 0.0);
+ ASSERT_DOUBLE_LT(convtime_double("1.s"), 0.0);
+ ASSERT_DOUBLE_LT(convtime_double("0x1"), 0.0);
+ ASSERT_DOUBLE_LT(convtime_double("inf"), 0.0);
+ ASSERT_DOUBLE_LT(convtime_double("nan"), 0.0);
+ ASSERT_DOUBLE_LT(convtime_double("1e10"), 0.0);
+ ASSERT_DOUBLE_LT(convtime_double("-1"), 0.0);
+ ASSERT_DOUBLE_LT(convtime_double("3.w0.5s"), 0.0);
+ ASSERT_DOUBLE_LT(convtime_double("1.0d0.5s"), 0.0);
+ ASSERT_DOUBLE_LT(convtime_double("1.5m"), 0.0);
+ ASSERT_DOUBLE_LT(convtime_double("1.5h"), 0.0);
+ ASSERT_DOUBLE_LT(convtime_double("1.5d"), 0.0);
+ ASSERT_DOUBLE_LT(convtime_double("1.5w"), 0.0);
+ ASSERT_DOUBLE_LT(convtime_double("1s1.5"), 0.0);
+ snprintf(buf, sizeof(buf), "%llds", (long long)(INT64_MAX / 1000000) + 1);
+ ASSERT_DOUBLE_NE(convtime_double(buf), -1.0);
+ TEST_DONE();
+
/* XXX timezones/DST make verification of this tricky */
/* XXX maybe setenv TZ and tzset() to make it unambiguous? */
TEST_START("misc_parse_absolute_time");
Index: regress/usr.bin/ssh/unittests/test_helper/test_helper.c
===================================================================
RCS file: /cvs/src/regress/usr.bin/ssh/unittests/test_helper/test_helper.c,v
diff -u -p -r1.14 test_helper.c
--- regress/usr.bin/ssh/unittests/test_helper/test_helper.c 15 Apr 2025 04:00:42 -0000 1.14
+++ regress/usr.bin/ssh/unittests/test_helper/test_helper.c 2 Dec 2025 03:49:58 -0000
@@ -569,6 +569,47 @@ assert_u64(const char *file, int line, c
}
void
+assert_double(const char *file, int line, const char *a1, const char *a2,
+ double aa1, double aa2, enum test_predicate pred)
+{
+ const double epsilon = 0.000000001;
+
+ switch (pred) {
+ case TEST_EQ:
+ if (fabs(aa1 - aa2) < epsilon)
+ return;
+ break;
+ case TEST_NE:
+ if (fabs(aa1 - aa2) >= epsilon)
+ return;
+ break;
+ case TEST_LT:
+ if (aa1 < aa2)
+ return;
+ break;
+ case TEST_LE:
+ if (aa1 <= aa2)
+ return;
+ break;
+ case TEST_GT:
+ if (aa1 > aa2)
+ return;
+ break;
+ case TEST_GE:
+ if (aa1 >= aa2)
+ return;
+ break;
+ default:
+ abort();
+ }
+
+ test_header(file, line, a1, a2, "DOUBLE", pred);
+ fprintf(stderr, "%12s = %f\n", a1, aa1);
+ fprintf(stderr, "%12s = %f\n", a2, aa2);
+ test_die();
+}
+
+void
assert_ptr(const char *file, int line, const char *a1, const char *a2,
const void *aa1, const void *aa2, enum test_predicate pred)
{
Index: regress/usr.bin/ssh/unittests/test_helper/test_helper.h
===================================================================
RCS file: /cvs/src/regress/usr.bin/ssh/unittests/test_helper/test_helper.h,v
diff -u -p -r1.10 test_helper.h
--- regress/usr.bin/ssh/unittests/test_helper/test_helper.h 15 Apr 2025 04:00:42 -0000 1.10
+++ regress/usr.bin/ssh/unittests/test_helper/test_helper.h 2 Dec 2025 03:49:58 -0000
@@ -93,6 +93,9 @@ void assert_u32(const char *file, int li
void assert_u64(const char *file, int line,
const char *a1, const char *a2,
u_int64_t aa1, u_int64_t aa2, enum test_predicate pred);
+void assert_double(const char *file, int line,
+ const char *a1, const char *a2,
+ double aa1, double aa2, enum test_predicate pred);
#define TEST_START(n) test_start(n)
#define TEST_DONE() test_done()
@@ -278,6 +281,19 @@ void assert_u64(const char *file, int li
assert_u32(__FILE__, __LINE__, #a1, #a2, a1, a2, TEST_GE)
#define ASSERT_U64_GE(a1, a2) \
assert_u64(__FILE__, __LINE__, #a1, #a2, a1, a2, TEST_GE)
+
+#define ASSERT_DOUBLE_EQ(a1, a2) \
+ assert_double(__FILE__, __LINE__, #a1, #a2, a1, a2, TEST_EQ)
+#define ASSERT_DOUBLE_NE(a1, a2) \
+ assert_double(__FILE__, __LINE__, #a1, #a2, a1, a2, TEST_NE)
+#define ASSERT_DOUBLE_LT(a1, a2) \
+ assert_double(__FILE__, __LINE__, #a1, #a2, a1, a2, TEST_LT)
+#define ASSERT_DOUBLE_LE(a1, a2) \
+ assert_double(__FILE__, __LINE__, #a1, #a2, a1, a2, TEST_LE)
+#define ASSERT_DOUBLE_GT(a1, a2) \
+ assert_double(__FILE__, __LINE__, #a1, #a2, a1, a2, TEST_GT)
+#define ASSERT_DOUBLE_GE(a1, a2) \
+ assert_double(__FILE__, __LINE__, #a1, #a2, a1, a2, TEST_GE)
/* Benchmarking support */
#define BENCH_START(name) \
openssh: fractional-second PerSourcePenalties part 1