split PerSourcePenalties address tracking. Previously it used one

shared table and overflow policy for IPv4 and IPv6 addresses, now
it will use separate tables and optionally different overflow
policies.

This prevents misbehaviour from IPv6 addresses (which are vastly easier
to obtain many of) from affecting IPv4 connections and may allow for
stricter overflow policies.

ok deraadt@

diff --git a/usr.bin/ssh/servconf.c b/usr.bin/ssh/servconf.c
index 2caa160..2926e69 100644 (file)
--- a/usr.bin/ssh/servconf.c
+++ b/usr.bin/ssh/servconf.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: servconf.c,v 1.410 2024/06/11 00:36:20 djm Exp $ */
+/* $OpenBSD: servconf.c,v 1.411 2024/06/12 22:36:00 djm Exp $ */
 /*
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
  *                    All rights reserved
@@ -147,8 +147,10 @@ initialize_server_options(ServerOptions *options)
        options->per_source_masklen_ipv6 = -1;
        options->per_source_penalty_exempt = NULL;
        options->per_source_penalty.enabled = -1;
-       options->per_source_penalty.max_sources = -1;
+       options->per_source_penalty.max_sources4 = -1;
+       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;
@@ -389,10 +391,14 @@ fill_default_server_options(ServerOptions *options)
                options->per_source_masklen_ipv6 = 128;
        if (options->per_source_penalty.enabled == -1)
                options->per_source_penalty.enabled = 1;
-       if (options->per_source_penalty.max_sources == -1)
-               options->per_source_penalty.max_sources = 65536;
+       if (options->per_source_penalty.max_sources4 == -1)
+               options->per_source_penalty.max_sources4 = 65536;
+       if (options->per_source_penalty.max_sources6 == -1)
+               options->per_source_penalty.max_sources6 = 65536;
        if (options->per_source_penalty.overflow_mode == -1)
                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)
@@ -1971,9 +1977,14 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                        } else if (strncmp(arg, "min:", 4) == 0) {
                                p = arg + 4;
                                intptr = &options->per_source_penalty.penalty_min;
-                       } else if (strncmp(arg, "max-sources:", 12) == 0) {
-                               intptr = &options->per_source_penalty.max_sources;
-                               if ((errstr = atoi_err(arg+12, &value)) != NULL)
+                       } else if (strncmp(arg, "max-sources4:", 13) == 0) {
+                               intptr = &options->per_source_penalty.max_sources4;
+                               if ((errstr = atoi_err(arg+13, &value)) != NULL)
+                                       fatal("%s line %d: %s value %s.",
+                                           filename, linenum, keyword, errstr);
+                       } else if (strncmp(arg, "max-sources6:", 13) == 0) {
+                               intptr = &options->per_source_penalty.max_sources6;
+                               if ((errstr = atoi_err(arg+13, &value)) != NULL)
                                        fatal("%s line %d: %s value %s.",
                                            filename, linenum, keyword, errstr);
                        } else if (strcmp(arg, "overflow:deny-all") == 0) {
@@ -1982,6 +1993,12 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                        } else if (strcmp(arg, "overflow:permissive") == 0) {
                                intptr = &options->per_source_penalty.overflow_mode;
                                value = PER_SOURCE_PENALTY_OVERFLOW_PERMISSIVE;
+                       } else if (strcmp(arg, "overflow6:deny-all") == 0) {
+                               intptr = &options->per_source_penalty.overflow_mode6;
+                               value = PER_SOURCE_PENALTY_OVERFLOW_DENY_ALL;
+                       } else if (strcmp(arg, "overflow6:permissive") == 0) {
+                               intptr = &options->per_source_penalty.overflow_mode6;
+                               value = PER_SOURCE_PENALTY_OVERFLOW_PERMISSIVE;
                        } else {
                                fatal("%s line %d: unsupported %s keyword %s",
                                    filename, linenum, keyword, arg);
@@ -3218,16 +3235,21 @@ dump_config(ServerOptions *o)
 
        if (o->per_source_penalty.enabled) {
                printf("persourcepenalties crash:%d authfail:%d noauth:%d "
-                   "grace-exceeded:%d max:%d min:%d max-sources:%d "
-                   "overflow:%s\n", o->per_source_penalty.penalty_crash,
+                   "grace-exceeded:%d max:%d min:%d 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_max,
                    o->per_source_penalty.penalty_min,
-                   o->per_source_penalty.max_sources,
+                   o->per_source_penalty.max_sources4,
+                   o->per_source_penalty.max_sources6,
                    o->per_source_penalty.overflow_mode ==
                    PER_SOURCE_PENALTY_OVERFLOW_DENY_ALL ?
+                   "deny-all" : "permissive",
+                   o->per_source_penalty.overflow_mode6 ==
+                   PER_SOURCE_PENALTY_OVERFLOW_DENY_ALL ?
                    "deny-all" : "permissive");
        } else
                printf("persourcepenalties no\n");

diff --git a/usr.bin/ssh/servconf.h b/usr.bin/ssh/servconf.h
index 4a4ac1c..442dacd 100644 (file)
--- a/usr.bin/ssh/servconf.h
+++ b/usr.bin/ssh/servconf.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: servconf.h,v 1.164 2024/06/06 17:15:25 djm Exp $ */
+/* $OpenBSD: servconf.h,v 1.165 2024/06/12 22:36:00 djm Exp $ */
 
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
@@ -69,8 +69,10 @@ struct listenaddr {
 #define PER_SOURCE_PENALTY_OVERFLOW_PERMISSIVE 2
 struct per_source_penalty {
        int     enabled;
-       int     max_sources;
+       int     max_sources4;
+       int     max_sources6;
        int     overflow_mode;
+       int     overflow_mode6;
        int     penalty_crash;
        int     penalty_grace;
        int     penalty_authfail;

diff --git a/usr.bin/ssh/srclimit.c b/usr.bin/ssh/srclimit.c
index c4bc5fe..4309526 100644 (file)
--- a/usr.bin/ssh/srclimit.c
+++ b/usr.bin/ssh/srclimit.c
@@ -59,11 +59,11 @@ struct penalty {
 };
 static int penalty_addr_cmp(struct penalty *a, struct penalty *b);
 static int penalty_expiry_cmp(struct penalty *a, struct penalty *b);
-RB_HEAD(penalties_by_addr, penalty) penalties_by_addr;
-RB_HEAD(penalties_by_expiry, penalty) penalties_by_expiry;
+RB_HEAD(penalties_by_addr, penalty) penalties_by_addr4, penalties_by_addr6;
+RB_HEAD(penalties_by_expiry, penalty) penalties_by_expiry4, penalties_by_expiry6;
 RB_GENERATE_STATIC(penalties_by_addr, penalty, by_addr, penalty_addr_cmp)
 RB_GENERATE_STATIC(penalties_by_expiry, penalty, by_expiry, penalty_expiry_cmp)
-static size_t npenalties;
+static size_t npenalties4, npenalties6;
 
 static int
 srclimit_mask_addr(const struct xaddr *addr, int bits, struct xaddr *masked)
@@ -104,10 +104,14 @@ srclimit_init(int max, int persource, int ipv4len, int ipv6len,
        ipv6_masklen = ipv6len;
        max_persource = persource;
        penalty_cfg = *penalty_conf;
+       if (penalty_cfg.max_sources4 < 0 || penalty_cfg.max_sources6 < 0)
+               fatal_f("invalid max_sources"); /* shouldn't happen */
        penalty_exempt = penalty_exempt_conf == NULL ?
            NULL : xstrdup(penalty_exempt_conf);
-       RB_INIT(&penalties_by_addr);
-       RB_INIT(&penalties_by_expiry);
+       RB_INIT(&penalties_by_addr4);
+       RB_INIT(&penalties_by_expiry4);
+       RB_INIT(&penalties_by_addr6);
+       RB_INIT(&penalties_by_expiry6);
        if (max_persource == INT_MAX)   /* no limit */
                return;
        debug("%s: max connections %d, per source %d, masks %d,%d", __func__,
@@ -206,26 +210,36 @@ penalty_expiry_cmp(struct penalty *a, struct penalty *b)
 }
 
 static void
-expire_penalties(time_t now)
+expire_penalties_from_tree(time_t now, const char *t,
+    struct penalties_by_expiry *by_expiry,
+    struct penalties_by_addr *by_addr, size_t *npenaltiesp)
 {
        struct penalty *penalty, *tmp;
 
        /* XXX avoid full scan of tree, e.g. min-heap */
-       RB_FOREACH_SAFE(penalty, penalties_by_expiry,
-           &penalties_by_expiry, tmp) {
+       RB_FOREACH_SAFE(penalty, penalties_by_expiry, by_expiry, tmp) {
                if (penalty->expiry >= now)
                        break;
-               if (RB_REMOVE(penalties_by_expiry, &penalties_by_expiry,
+               if (RB_REMOVE(penalties_by_expiry, by_expiry,
                    penalty) != penalty ||
-                   RB_REMOVE(penalties_by_addr, &penalties_by_addr,
+                   RB_REMOVE(penalties_by_addr, by_addr,
                    penalty) != penalty)
-                       fatal_f("internal error: penalty tables corrupt");
+                       fatal_f("internal error: %s penalty table corrupt", t);
                free(penalty);
-               if (npenalties-- == 0)
-                       fatal_f("internal error: npenalties underflow");
+               if ((*npenaltiesp)-- == 0)
+                       fatal_f("internal error: %s npenalties underflow", t);
        }
 }
 
+static void
+expire_penalties(time_t now)
+{
+       expire_penalties_from_tree(now, "ipv4",
+           &penalties_by_expiry4, &penalties_by_addr4, &npenalties4);
+       expire_penalties_from_tree(now, "ipv6",
+           &penalties_by_expiry6, &penalties_by_addr6, &npenalties6);
+}
+
 static void
 addr_masklen_ntop(struct xaddr *addr, int masklen, char *s, size_t slen)
 {
@@ -245,8 +259,10 @@ srclimit_penalty_check_allow(int sock, const char **reason)
        struct xaddr addr;
        struct penalty find, *penalty;
        time_t now;
-       int bits;
+       int bits, max_sources, overflow_mode;
        char addr_s[NI_MAXHOST];
+       struct penalties_by_addr *by_addr;
+       size_t npenalties;
 
        if (!penalty_cfg.enabled)
                return 1;
@@ -259,8 +275,17 @@ srclimit_penalty_check_allow(int sock, const char **reason)
                        return 1;
                }
        }
-       if (npenalties >= (size_t)penalty_cfg.max_sources &&
-           penalty_cfg.overflow_mode == PER_SOURCE_PENALTY_OVERFLOW_DENY_ALL) {
+       now = monotime();
+       expire_penalties(now);
+       by_addr = addr.af == AF_INET ?
+           &penalties_by_addr4 : &penalties_by_addr6;
+       max_sources = addr.af == AF_INET ?
+           penalty_cfg.max_sources4 : penalty_cfg.max_sources6;
+       overflow_mode = addr.af == AF_INET ?
+           penalty_cfg.overflow_mode : penalty_cfg.overflow_mode6;
+       npenalties = addr.af == AF_INET ?  npenalties4 : npenalties6;
+       if (npenalties >= (size_t)max_sources &&
+           overflow_mode == PER_SOURCE_PENALTY_OVERFLOW_DENY_ALL) {
                *reason = "too many penalised addresses";
                return 0;
        }
@@ -268,9 +293,7 @@ srclimit_penalty_check_allow(int sock, const char **reason)
        memset(&find, 0, sizeof(find));
        if (srclimit_mask_addr(&addr, bits, &find.addr) != 0)
                return 1;
-       now = monotime();
-       if ((penalty = RB_FIND(penalties_by_addr,
-           &penalties_by_addr, &find)) == NULL)
+       if ((penalty = RB_FIND(penalties_by_addr, by_addr, &find)) == NULL)
                return 1; /* no penalty */
        if (penalty->expiry < now) {
                expire_penalties(now);
@@ -283,38 +306,52 @@ srclimit_penalty_check_allow(int sock, const char **reason)
 }
 
 static void
-srclimit_remove_expired_penalties(void)
+srclimit_early_expire_penalties_from_tree(const char *t,
+    struct penalties_by_expiry *by_expiry,
+    struct penalties_by_addr *by_addr, size_t *npenaltiesp, size_t max_sources)
 {
        struct penalty *p = NULL;
        int bits;
        char s[NI_MAXHOST + 4];
 
        /* Delete the soonest-to-expire penalties. */
-       while (npenalties > (size_t)penalty_cfg.max_sources) {
-               if ((p = RB_MIN(penalties_by_expiry,
-                   &penalties_by_expiry)) == NULL)
-                       fatal_f("internal error: penalty tables corrupt (find)");
+       while (*npenaltiesp > max_sources) {
+               if ((p = RB_MIN(penalties_by_expiry, by_expiry)) == NULL)
+                       fatal_f("internal error: %s table corrupt (find)", t);
                bits = p->addr.af == AF_INET ? ipv4_masklen : ipv6_masklen;
                addr_masklen_ntop(&p->addr, bits, s, sizeof(s));
-               debug3_f("overflow, remove %s", s);
-               if (RB_REMOVE(penalties_by_expiry,
-                   &penalties_by_expiry, p) != p ||
-                   RB_REMOVE(penalties_by_addr, &penalties_by_addr, p) != p)
-                       fatal_f("internal error: penalty tables corrupt (remove)");
+               debug3_f("%s overflow, remove %s", t, s);
+               if (RB_REMOVE(penalties_by_expiry, by_expiry, p) != p ||
+                   RB_REMOVE(penalties_by_addr, by_addr, p) != p)
+                       fatal_f("internal error: %s table corrupt (remove)", t);
                free(p);
-               npenalties--;
+               (*npenaltiesp)--;
        }
 }
 
+static void
+srclimit_early_expire_penalties(void)
+{
+       srclimit_early_expire_penalties_from_tree("ipv4",
+           &penalties_by_expiry4, &penalties_by_addr4, &npenalties4,
+           (size_t)penalty_cfg.max_sources4);
+       srclimit_early_expire_penalties_from_tree("ipv6",
+           &penalties_by_expiry6, &penalties_by_addr6, &npenalties6,
+           (size_t)penalty_cfg.max_sources6);
+}
+
 void
 srclimit_penalise(struct xaddr *addr, int penalty_type)
 {
        struct xaddr masked;
-       struct penalty *penalty, *existing;
+       struct penalty *penalty = NULL, *existing = NULL;
        time_t now;
-       int bits, penalty_secs;
+       int bits, penalty_secs, max_sources = 0, overflow_mode;
        char addrnetmask[NI_MAXHOST + 4];
-       const char *reason = NULL;
+       const char *reason = NULL, *t;
+       size_t *npenaltiesp = NULL;
+       struct penalties_by_addr *by_addr = NULL;
+       struct penalties_by_expiry *by_expiry = NULL;
 
        if (!penalty_cfg.enabled)
                return;
@@ -356,9 +393,19 @@ srclimit_penalise(struct xaddr *addr, int penalty_type)
 
        now = monotime();
        expire_penalties(now);
-       if (npenalties > (size_t)penalty_cfg.max_sources &&
-           penalty_cfg.overflow_mode == PER_SOURCE_PENALTY_OVERFLOW_DENY_ALL) {
-               verbose_f("penalty table full, cannot penalise %s for %s",
+       by_expiry = addr->af == AF_INET ?
+           &penalties_by_expiry4 : &penalties_by_expiry6;
+       by_addr = addr->af == AF_INET ?
+           &penalties_by_addr4 : &penalties_by_addr6;
+       max_sources = addr->af == AF_INET ?
+           penalty_cfg.max_sources4 : penalty_cfg.max_sources6;
+       overflow_mode = addr->af == AF_INET ?
+           penalty_cfg.overflow_mode : penalty_cfg.overflow_mode6;
+       npenaltiesp = addr->af == AF_INET ?  &npenalties4 : &npenalties6;
+       t = addr->af == AF_INET ? "ipv4" : "ipv6";
+       if (*npenaltiesp > (size_t)max_sources &&
+           overflow_mode == PER_SOURCE_PENALTY_OVERFLOW_DENY_ALL) {
+               verbose_f("%s penalty table full, cannot penalise %s for %s", t,
                    addrnetmask, reason);
                return;
        }
@@ -367,48 +414,48 @@ srclimit_penalise(struct xaddr *addr, int penalty_type)
        penalty->addr = masked;
        penalty->expiry = now + penalty_secs;
        penalty->reason = reason;
-       if ((existing = RB_INSERT(penalties_by_addr, &penalties_by_addr,
+       if ((existing = RB_INSERT(penalties_by_addr, by_addr,
            penalty)) == NULL) {
                /* penalty didn't previously exist */
                if (penalty_secs > penalty_cfg.penalty_min)
                        penalty->active = 1;
-               if (RB_INSERT(penalties_by_expiry, &penalties_by_expiry,
-                   penalty) != NULL)
-                       fatal_f("internal error: penalty tables corrupt");
-               verbose_f("%s: new %s penalty of %d seconds for %s",
+               if (RB_INSERT(penalties_by_expiry, by_expiry, penalty) != NULL)
+                       fatal_f("internal error: %s penalty tables corrupt", t);
+               verbose_f("%s: new %s %s penalty of %d seconds for %s", t,
                    addrnetmask, penalty->active ? "active" : "deferred",
                    penalty_secs, reason);
-               if (++npenalties > (size_t)penalty_cfg.max_sources)
-                       srclimit_remove_expired_penalties(); /* permissive */
+               if (++(*npenaltiesp) > (size_t)max_sources)
+                       srclimit_early_expire_penalties(); /* permissive */
                return;
        }
-       debug_f("%s penalty for %s already exists, %lld seconds remaining",
-           existing->active ? "active" : "inactive",
+       debug_f("%s penalty for %s %s already exists, %lld seconds remaining",
+           existing->active ? "active" : "inactive", t,
            addrnetmask, (long long)(existing->expiry - now));
        /* Expiry information is about to change, remove from tree */
-       if (RB_REMOVE(penalties_by_expiry, &penalties_by_expiry,
-           existing) != existing)
-               fatal_f("internal error: penalty tables corrupt (remove)");
+       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;
        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) {
-               verbose_f("%s: activating penalty of %lld seconds for %s",
-                   addrnetmask, (long long)(existing->expiry - now), reason);
+               verbose_f("%s: activating %s penalty of %lld seconds for %s",
+                   addrnetmask, t, (long long)(existing->expiry - now),
+                   reason);
                existing->active = 1;
        }
        existing->reason = penalty->reason;
        free(penalty);
+       penalty = NULL;
        /* Re-insert into expiry tree */
-       if (RB_INSERT(penalties_by_expiry, &penalties_by_expiry,
-           existing) != NULL)
-               fatal_f("internal error: penalty tables corrupt (insert)");
+       if (RB_INSERT(penalties_by_expiry, by_expiry, existing) != NULL)
+               fatal_f("internal error: %s penalty table corrupt (insert)", t);
 }
 
-void
-srclimit_penalty_info(void)
+static void
+srclimit_penalty_info_for_tree(const char *t,
+    struct penalties_by_expiry *by_expiry, size_t npenalties)
 {
        struct penalty *p = NULL;
        int bits;
@@ -416,8 +463,8 @@ srclimit_penalty_info(void)
        time_t now;
 
        now = monotime();
-       logit("%zu active penalties", npenalties);
-       RB_FOREACH(p, penalties_by_expiry, &penalties_by_expiry) {
+       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;
                addr_masklen_ntop(&p->addr, bits, s, sizeof(s));
                if (p->expiry < now)
@@ -428,3 +475,12 @@ srclimit_penalty_info(void)
                }
        }
 }
+
+void
+srclimit_penalty_info(void)
+{
+       srclimit_penalty_info_for_tree("ipv4",
+           &penalties_by_expiry4, npenalties4);
+       srclimit_penalty_info_for_tree("ipv6",
+           &penalties_by_expiry6, npenalties6);
+}

diff --git a/usr.bin/ssh/sshd_config.5 b/usr.bin/ssh/sshd_config.5
index 390cf98..50e5d8f 100644 (file)
--- a/usr.bin/ssh/sshd_config.5
+++ b/usr.bin/ssh/sshd_config.5
@@ -33,8 +33,8 @@
 .\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
 .\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 .\"
-.\" $OpenBSD: sshd_config.5,v 1.360 2024/06/11 05:24:39 jmc Exp $
-.Dd $Mdocdate: June 11 2024 $
+.\" $OpenBSD: sshd_config.5,v 1.361 2024/06/12 22:36:00 djm Exp $
+.Dd $Mdocdate: June 12 2024 $
 .Dt SSHD_CONFIG 5
 .Os
 .Sh NAME
@@ -1604,12 +1604,14 @@ Repeated penalties will accumulate up to this maximum.
 .It Cm min:duration
 Specifies the minimum penalty that must accrue before enforcement begins
 (default: 15s).
-.It Cm max-sources:number
-Specifies the maximum number of penalise client address ranges to track
-(default: 65536).
+.It Cm max-sources4:number max-sources6:number
+Specifies the maximum number of client IPv4 and IPv6 address ranges to
+track for penalties (default: 65536 for both).
 .It Cm overflow:mode
 Controls how the server behaves when
-.Cm max-sources
+.Cm max-sources4
+or
+.Cm max-sources6
 is exceeded.
 There are two operating modes:
 .Cm deny-all ,
@@ -1619,6 +1621,14 @@ until a penalty expires, and
 .Cm permissive ,
 which allows new connections by removing existing penalties early
 (default: permissive).
+Note that client penalties below the
+.Cm min
+threshold count against the total number of tracked penalties.
+IPv4 and IPv6 addresses are tracked separately, so an overflow in one will
+not affect the other.
+.It Cm overflow6:mode
+Allows specifying a different overflow mode for IPv6 addresses.
+The default it to use the same overflow mode as was specified for IPv4.
 .El
 .It Cm PerSourcePenaltyExemptList
 Specifies a comma-separated list of addresses to exempt from penalties.