merge: branch 'bg/ndisc-ratelimit-warns'

ndisc: rate limit messages about invalid RAs

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/2294

src/core/ndisc/nm-lndp-ndisc.c

@@ -19,6 +19,7 @@
 #include "libnm-systemd-shared/nm-sd-utils-shared.h"
 #include "nm-l3cfg.h"
 #include "nm-ndisc-private.h"
+#include "nm-core-utils.h"
 
 #define _NMLOG_PREFIX_NAME "ndisc-lndp"
 
@@ -27,6 +28,14 @@
 typedef struct {
     struct ndp *ndp;
     GSource    *event_source;
+
+    struct {
+        NMRateLimit pio_lft;
+        NMRateLimit mtu;
+        NMRateLimit omit_prefix;
+        NMRateLimit omit_dns;
+        NMRateLimit omit_dnssl;
+    } msg_ratelimit;
 } NMLndpNDiscPrivate;
 
 /*****************************************************************************/
@@ -49,6 +58,36 @@ G_DEFINE_TYPE(NMLndpNDisc, nm_lndp_ndisc, NM_TYPE_NDISC)
 
 /*****************************************************************************/
 
+/*
+ * If we log a message about an invalid RA packet, don't repeat the same message
+ * at every packet received or sent. Rate limit the message to 6 every 12 hours
+ * per type and per ndisc instance.
+ */
+
+#define LOG_INV_RA_WINDOW (12 * 3600)
+#define LOG_INV_RA_BURST  6
+
+#define _LOG_INVALID_RA(ndisc, rate_limit, ...)                                  \
+    G_STMT_START                                                                 \
+    {                                                                            \
+        NMNDisc     *__ndisc  = (ndisc);                                         \
+        NMRateLimit *__rl     = (rate_limit);                                    \
+        const char  *__ifname = nm_ndisc_get_ifname(__ndisc);                    \
+                                                                                 \
+        if (__ifname && nm_logging_enabled(LOGL_WARN, LOGD_IP6)                  \
+            && nm_rate_limit_check(__rl, LOG_INV_RA_WINDOW, LOG_INV_RA_BURST)) { \
+            nm_log(LOGL_WARN,                                                    \
+                   LOGD_IP6,                                                     \
+                   __ifname,                                                     \
+                   NULL,                                                         \
+                   "ndisc (%s): " _NM_UTILS_MACRO_FIRST(__VA_ARGS__),            \
+                   __ifname _NM_UTILS_MACRO_REST(__VA_ARGS__));                  \
+        }                                                                        \
+    }                                                                            \
+    G_STMT_END
+
+/*****************************************************************************/
+
 static gboolean
 send_rs(NMNDisc *ndisc, GError **error)
 {
@@ -113,6 +152,7 @@ static int
 receive_ra(struct ndp *ndp, struct ndp_msg *msg, gpointer user_data)
 {
     NMNDisc             *ndisc   = (NMNDisc *) user_data;
+    NMLndpNDiscPrivate  *priv    = NM_LNDP_NDISC_GET_PRIVATE(ndisc);
     NMNDiscDataInternal *rdata   = ndisc->rdata;
     NMNDiscConfigMap     changed = 0;
     NMNDiscGateway       gateway;
@@ -229,7 +269,11 @@ receive_ra(struct ndp *ndp, struct ndp_msg *msg, gpointer user_data)
              * log a system management error in this case.
              */
             if (preferred_time > valid_time) {
-                _LOGW("skipping PIO - preferred lifetime > valid lifetime");
+                _LOG_INVALID_RA(
+                    ndisc,
+                    &priv->msg_ratelimit.pio_lft,
+                    "ignoring Prefix Information Option with invalid lifetimes in received IPv6 "
+                    "router advertisement");
                 continue;
             }
 
@@ -349,7 +393,11 @@ receive_ra(struct ndp *ndp, struct ndp_msg *msg, gpointer user_data)
              * Kernel would set it, but would flush out all IPv6 addresses away
              * from the link, even the link-local, and we wouldn't be able to
              * listen for further RAs that could fix the MTU. */
-            _LOGW("MTU too small for IPv6 ignored: %d", mtu);
+            _LOG_INVALID_RA(ndisc,
+                            &priv->msg_ratelimit.mtu,
+                            "ignoring too small MTU %u in received IPv6 "
+                            "router advertisement",
+                            mtu);
         }
     }
 
@@ -445,8 +493,11 @@ send_ra(NMNDisc *ndisc, GError **error)
 
         prefix = _ndp_msg_add_option(msg, sizeof(*prefix));
         if (!prefix) {
-            /* Maybe we could sent separate RAs, but why bother... */
+            /* Maybe we could send separate RAs, but why bother... */
-            _LOGW("The RA is too big, had to omit some some prefixes.");
+            _LOG_INVALID_RA(
+                ndisc,
+                &priv->msg_ratelimit.omit_prefix,
+                "the outgoing IPv6 router advertisement is too big: omitting some prefixes");
             break;
         }
 
@@ -475,7 +526,10 @@ send_ra(NMNDisc *ndisc, GError **error)
 
         option = _ndp_msg_add_option(msg, len);
         if (!option) {
-            _LOGW("The RA is too big, had to omit DNS information.");
+            _LOG_INVALID_RA(
+                ndisc,
+                &priv->msg_ratelimit.omit_dns,
+                "the outgoing IPv6 router advertisement is too big: omitting DNS information");
             goto dns_servers_done;
         }
 
@@ -553,7 +607,10 @@ dns_servers_done:
         nm_assert(len / 8u >= 2u);
 
         if (len / 8u >= 256u || !(option = _ndp_msg_add_option(msg, len))) {
-            _LOGW("The RA is too big, had to omit DNS search list.");
+            _LOG_INVALID_RA(
+                ndisc,
+                &priv->msg_ratelimit.omit_dnssl,
+                "the outgoing IPv6 router advertisement is too big: omitting DNS search list");
             goto dns_domains_done;
         }
 

src/core/nm-core-utils.c

@@ -5568,3 +5568,59 @@ out_removed:
                 feature);
     return FALSE;
 }
+
+/*****************************************************************************/
+
+/**
+ * nm_rate_limit_check():
+ * @rate_limit: the NMRateLimit instance
+ * @window_sec: the time window in seconds, between 1 and 864000 (ten days)
+ * @burst: the number of max allowed event occurrences in the given time
+ *   window
+ *
+ * The function rate limits an event. Call it multiple times with the
+ * same @window_sec, and @burst values.
+ *
+ * Returns: TRUE if the event is allowed, FALSE if it is rate-limited
+ */
+gboolean
+nm_rate_limit_check(NMRateLimit *rate_limit, gint32 window_sec, gint32 burst)
+{
+    gint64 now;
+    gint64 old_ts_msec;
+    gint64 window_msec;
+    gint64 capacity;
+    gint64 elapsed;
+
+    nm_assert(window_sec >= 1 && window_sec <= 864000);
+    nm_assert(burst >= 1);
+
+    /* This implements a simple token bucket algorithm. For each millisecond,
+     * refill "burst" tokens. Thus, during a full time window we
+     * refill (window_msec * burst) tokens. Each event consumes @window_msec
+     * tokens. */
+
+    window_msec         = (gint64) window_sec * NM_UTILS_MSEC_PER_SEC;
+    capacity            = window_msec * (gint64) burst;
+    old_ts_msec         = rate_limit->ts_msec;
+    now                 = nm_utils_get_monotonic_timestamp_msec();
+    rate_limit->ts_msec = now;
+
+    elapsed = now - old_ts_msec;
+    if (old_ts_msec == 0 || elapsed > window_msec) {
+        /* On the first call, or in case a whole window passed, (re)start with
+         * a full budget */
+        rate_limit->tokens = capacity;
+    } else {
+        rate_limit->tokens += elapsed * (gint64) burst;
+        rate_limit->tokens = NM_MIN(rate_limit->tokens, capacity);
+    }
+
+    /* Consume the tokens */
+    if (rate_limit->tokens >= window_msec) {
+        rate_limit->tokens -= window_msec;
+        return TRUE;
+    }
+
+    return FALSE;
+}

src/core/nm-core-utils.h

@@ -494,4 +494,13 @@ gid_t nm_utils_get_nm_gid(void);
 
 gboolean nm_utils_connection_supported(NMConnection *connection, GError **error);
 
+/*****************************************************************************/
+
+typedef struct {
+    gint64 ts_msec;
+    gint64 tokens;
+} NMRateLimit;
+
+gboolean nm_rate_limit_check(NMRateLimit *rate_limit, gint32 window_sec, gint32 burst);
+
 #endif /* __NM_CORE_UTILS_H__ */

src/core/tests/test-utils.c

@@ -260,6 +260,73 @@ test_shorten_hostname(void)
     do_test_shorten_hostname(".name1", FALSE, NULL);
 }
 
+/*****************************************************************************/
+typedef struct {
+    NMRateLimit ratelimit;
+    GMainLoop  *loop;
+    GSource    *source1;
+    GSource    *source2;
+    guint       num;
+} RateLimitData;
+
+static int
+rate_limit_window_expire_cb(gpointer user_data)
+{
+    RateLimitData *data = user_data;
+
+    g_assert(nm_rate_limit_check(&data->ratelimit, 1, 5));
+    g_assert(nm_rate_limit_check(&data->ratelimit, 1, 5));
+    g_assert(nm_rate_limit_check(&data->ratelimit, 1, 5));
+    g_assert(nm_rate_limit_check(&data->ratelimit, 1, 5));
+    g_assert(nm_rate_limit_check(&data->ratelimit, 1, 5));
+
+    g_assert(!nm_rate_limit_check(&data->ratelimit, 1, 5));
+    g_assert(!nm_rate_limit_check(&data->ratelimit, 1, 5));
+
+    g_main_loop_quit(data->loop);
+
+    nm_clear_g_source_inst(&data->source1);
+
+    return G_SOURCE_CONTINUE;
+}
+
+static int
+rate_limit_check_cb(gpointer user_data)
+{
+    RateLimitData *data = user_data;
+
+    g_assert(nm_rate_limit_check(&data->ratelimit, 1, 5));
+    g_assert(nm_rate_limit_check(&data->ratelimit, 1, 5));
+    g_assert(nm_rate_limit_check(&data->ratelimit, 1, 5));
+    g_assert(nm_rate_limit_check(&data->ratelimit, 1, 5));
+    g_assert(nm_rate_limit_check(&data->ratelimit, 1, 5));
+
+    g_assert(!nm_rate_limit_check(&data->ratelimit, 1, 5));
+    g_assert(!nm_rate_limit_check(&data->ratelimit, 1, 5));
+
+    nm_clear_g_source_inst(&data->source2);
+
+    return G_SOURCE_CONTINUE;
+}
+
+static void
+test_rate_limit_check(void)
+{
+    RateLimitData data;
+
+    data = (RateLimitData) {
+        .loop      = g_main_loop_new(NULL, FALSE),
+        .ratelimit = {},
+        .num       = 0,
+    };
+
+    data.source1 = nm_g_timeout_add_source(1100, rate_limit_window_expire_cb, &data);
+    data.source2 = nm_g_timeout_add_source(10, rate_limit_check_cb, &data);
+
+    g_main_loop_run(data.loop);
+    g_main_loop_unref(data.loop);
+}
+
 /*****************************************************************************/
 
 NMTST_DEFINE();
@@ -272,6 +339,7 @@ main(int argc, char **argv)
     g_test_add_func("/utils/stable_privacy", test_stable_privacy);
     g_test_add_func("/utils/hw_addr_gen_stable_eth", test_hw_addr_gen_stable_eth);
     g_test_add_func("/utils/shorten-hostname", test_shorten_hostname);
+    g_test_add_func("/utils/rate-limit-check", test_rate_limit_check);
 
     return g_test_run();
 }