From: djm Date: Wed, 12 Jun 2024 22:36:00 +0000 (+0000) Subject: split PerSourcePenalties address tracking. Previously it used one X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=cd187d0bf5f90a5e9e8ee62ae93d04529dff09e2;p=openbsd 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 2caa16039c3..2926e69fc73 100644 --- 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 , 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 4a4ac1cdb49..442dacdcdf5 100644 --- 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 @@ -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 c4bc5fee273..43095267804 100644 --- 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 390cf9804d8..50e5d8f6e41 100644 --- 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.