From: Damien Miller Subject: Re: openssh: fractional-second PerSourcePenalties part 1 To: tech@openbsd.org Cc: openssh@openssh.com Date: Mon, 1 Dec 2025 15:12:32 +1100 On Fri, 28 Nov 2025, Damien Miller wrote: > > Hi, > > I'd like to add fractional second penalties to PerSourcePenalties, > and this is part 1: > > We use mis.c:convtime() to parse intervals and durations like > "1h30m15s" and so forth. This is used to parse PerSourcePenalties > times. > > To support fractional second penalties, this adds convtime_usec() > that allows things like "1h30m15.123s". > > It only allows fractions in the seconds qualifier, i.e. "0.5w" is > disallowed. Obvious junk like "1.5s123" is also disallowed. > > This also rewrites the existing convtime() function to use > convtime_usec() internally since they're so similar and the > latter is more general. I got rid of the MINUTES/HOURS/etc defines > because they are pretty obvious. > > ok? Darren asked what this would look like with the funky fixed point parsing ripped out and replaced with strtod(). Turns out it's much simpler. I had avoided this initially because I wanted to avoid the need to use libm which seemed necessary to detect fractional seconds in places where we don't want them, but that can be done by just looking for '.' in the string. We need to do that anyway for some other validation. diff --git a/misc.c b/misc.c index 3cc7add..18e56bc 100644 --- a/misc.c +++ b/misc.c @@ -576,25 +576,13 @@ a2tun(const char *s, int *remote) return (tun); } -#define SECONDS 1 -#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 a time string into microseconds; 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: * seconds * s|S seconds @@ -604,79 +592,115 @@ 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 -1 if the time string is invalid. */ -int -convtime(const char *s) +int64_t +convtime_usec(const char *s) { - int secs, total = 0, multiplier; - char *p, *os, *np, c = 0; - const char *errstr; + int64_t val_usec, total_usec = 0; + const char *p, *start_p; + char *endp; + int seen_seconds = 0; + double val, multiplier_sec; if (s == NULL || *s == '\0') return -1; - p = os = strdup(s); /* deal with const */ - if (os == NULL) - return -1; - 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; - multiplier = 1; - switch (c) { + errno = 0; + if ((val = strtod(p, &endp)) < 0 || errno != 0 || p == endp) + return -1; + start_p = p; + p = endp; + + switch (*p) { case '\0': - np--; /* back up */ - break; + /* FALLTHROUGH */ case 's': case 'S': + if (seen_seconds++) + return -1; + multiplier_sec = 1; break; case 'm': case 'M': - multiplier = MINUTES; + multiplier_sec = 60; break; case 'h': case 'H': - multiplier = HOURS; + multiplier_sec = 60 * 60; break; case 'd': case 'D': - multiplier = DAYS; + multiplier_sec = 24 * 60 * 60; break; case 'w': case 'W': - multiplier = WEEKS; + multiplier_sec = 7 * 24 * 60 * 60; break; default: - goto fail; + return -1; } - 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; + + /* Special handling of this was a decimal */ + if (memchr(start_p, '.', endp - start_p) != NULL) { + /* Decimal point present */ + if (multiplier_sec > 1) + return -1; /* No fractionals for non-seconds */ + /* For seconds, ensure digits follow */ + if (!isdigit((unsigned char)*(endp - 1))) + return -1; + } + + double temp_val_usec_d = val * multiplier_sec * 1000000.0; + if (temp_val_usec_d < 0 || temp_val_usec_d > (double)INT64_MAX) + return -1; + + val_usec = (int64_t)temp_val_usec_d; + + if (total_usec > INT64_MAX - val_usec) + return -1; /* overflow */ + total_usec += val_usec; + + if (*p != '\0') + p++; } - free(os); - return total; -fail: - free(os); - return -1; + return total_usec; +} + +/* + * Same as convtime_usec() above but fractional seconds are not supported. + * Return -1 if time string is invalid. + */ +int +convtime(const char *s) +{ + int64_t usec_val; + + if ((usec_val = convtime_usec(s)) == -1) + return -1; + + /* Check for fractional seconds: convtime does not support them */ + if (usec_val % 1000000 != 0) + return -1; + + /* Convert to seconds */ + usec_val /= 1000000; + + /* Check for overflow into int */ + if (usec_val < 0 || usec_val > INT_MAX) + return -1; + + return (int)usec_val; } #define TF_BUFS 8 diff --git a/misc.h b/misc.h index 6dd0dc0..bb8844b 100644 --- a/misc.h +++ b/misc.h @@ -82,6 +82,7 @@ int parse_user_host_path(const char *, char **, char **, 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 *); +int64_t convtime_usec(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);