From bf943a9d49941801b45e4631f010359619173d12 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 10 Jul 2023 13:28:59 +0900 Subject: [PATCH 1/2] network/ndisc: do not store too many captive portals provided through RA Prompted by https://github.com/systemd/systemd/pull/28285#issuecomment-1627585140. --- src/network/networkd-ndisc.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index c6ca7f95e2..de03abad22 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -27,6 +27,9 @@ #define NDISC_DNSSL_MAX 64U #define NDISC_RDNSS_MAX 64U +/* Not defined RFC, but let's set an upper limit to make not consume much memory. + * This should be safe as typically there should be at most 1 portal per network. */ +#define NDISC_CAPTIVE_PORTAL_MAX 64U bool link_ipv6_accept_ra_enabled(Link *link) { assert(link); @@ -914,6 +917,19 @@ static int ndisc_router_process_captive_portal(Link *link, sd_ndisc_router *rt) return 0; } + if (set_size(link->ndisc_captive_portals) >= NDISC_CAPTIVE_PORTAL_MAX) { + NDiscCaptivePortal *c, *target = NULL; + + /* Find the portal who has the minimal lifetime and drop it to store new one. */ + SET_FOREACH(c, link->ndisc_captive_portals) + if (!target || c->lifetime_usec < target->lifetime_usec) + target = c; + + assert(target); + assert(set_remove(link->ndisc_captive_portals, target) == target); + ndisc_captive_portal_free(target); + } + new_entry = new(NDiscCaptivePortal, 1); if (!new_entry) return log_oom(); From 6df82d128cae55dc23839b1cc9673062a1beeb37 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 10 Jul 2023 13:37:46 +0900 Subject: [PATCH 2/2] network/ndisc: use the first captive portal in each RA To handle malicious RA packets gracefully. Also prompted by https://github.com/systemd/systemd/pull/28285#issuecomment-1627585140. --- src/network/networkd-ndisc.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index de03abad22..ae267ce5ca 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -900,21 +900,21 @@ static int ndisc_router_process_captive_portal(Link *link, sd_ndisc_router *rt) return log_link_warning_errno(link, r, "Failed to get captive portal from RA: %m"); if (len == 0) - return 0; + return log_link_warning_errno(link, SYNTHETIC_ERRNO(EBADMSG), "Received empty captive portal, ignoring."); r = make_cstring(uri, len, MAKE_CSTRING_REFUSE_TRAILING_NUL, &captive_portal); if (r < 0) return log_link_warning_errno(link, r, "Failed to convert captive portal URI: %m"); if (!in_charset(captive_portal, URI_VALID)) - return log_link_debug_errno(link, SYNTHETIC_ERRNO(EBADMSG), "Received invalid captive portal, ignoring."); + return log_link_warning_errno(link, SYNTHETIC_ERRNO(EBADMSG), "Received invalid captive portal, ignoring."); exist = set_get(link->ndisc_captive_portals, &(NDiscCaptivePortal) { .captive_portal = captive_portal }); if (exist) { /* update existing entry */ exist->router = router; exist->lifetime_usec = lifetime_usec; - return 0; + return 1; } if (set_size(link->ndisc_captive_portals) >= NDISC_CAPTIVE_PORTAL_MAX) { @@ -947,10 +947,11 @@ static int ndisc_router_process_captive_portal(Link *link, sd_ndisc_router *rt) TAKE_PTR(new_entry); link_dirty(link); - return 0; + return 1; } static int ndisc_router_process_options(Link *link, sd_ndisc_router *rt) { + size_t n_captive_portal = 0; int r; assert(link); @@ -986,7 +987,16 @@ static int ndisc_router_process_options(Link *link, sd_ndisc_router *rt) { r = ndisc_router_process_dnssl(link, rt); break; case SD_NDISC_OPTION_CAPTIVE_PORTAL: + if (n_captive_portal > 0) { + if (n_captive_portal == 1) + log_link_notice(link, "Received RA with multiple captive portals, only using the first one."); + + n_captive_portal++; + continue; + } r = ndisc_router_process_captive_portal(link, rt); + if (r > 0) + n_captive_portal++; break; } if (r < 0 && r != -EBADMSG)