upstream: 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@

OpenBSD-Commit-ID: 12637ed0aa4d5f1f3e702da42ea967cbd8bfdfd9

diff --git a/servconf.c b/servconf.c
index 8fe56939cede78e916e28cd046f0585f7dc1e41d..22afbededdfef23366e61ab008377a0da1be0a02 100644 (file)
--- a/servconf.c
+++ b/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
@@ -165,8 +165,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;
@@ -414,10 +416,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)
@@ -2028,9 +2034,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) {
@@ -2039,6 +2050,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);
@@ -3288,16 +3305,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/servconf.h b/servconf.h
index e6d7b75b9a9ab668c00f6de5eb7e54239a191945..8984e99661e72cae2f605a88e3739e1cd5580d73 100644 (file)
--- a/servconf.h
+++ b/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/srclimit.c b/srclimit.c
index 962af73f739037f846ceb8d2308787a4975848cd..837e82606e111d0b8852657398adc9e5d8ecccce 100644 (file)
--- a/srclimit.c
+++ b/srclimit.c
@@ -61,11 +61,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)
@@ -106,10 +106,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__,
@@ -208,26 +212,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)
 {
@@ -247,8 +261,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;
@@ -261,8 +277,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;
        }
@@ -270,9 +295,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);
@@ -285,38 +308,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;
@@ -358,9 +395,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;
        }
@@ -369,48 +416,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;
@@ -418,8 +465,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)
@@ -430,3 +477,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/sshd_config.5 b/sshd_config.5
index ff2d116a48d9e5b5fa0dfa51f5bc175aeb37e5a1..550c11cc978ca7c7aa08ef92947523df2d671b70 100644 (file)
--- a/sshd_config.5
+++ b/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
@@ -1603,12 +1603,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 ,
@@ -1618,6 +1620,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.