CFE-4723: Fixed default route selection to pick the lowest-metric route

libenv/unix_iface.c

@@ -1392,6 +1392,56 @@ JsonElement* GetProcFileInfo(EvalContext *ctx, const char* filename, const char*
 
 /*******************************************************************/
 
+/**
+ * @brief Select the active default route with the lowest metric.
+ *
+ * NetworkingRoutesPostProcessInfo() marks a route with
+ * "active_default_gateway": true when its destination is 0.0.0.0 and its
+ * flags include RTF_UP and RTF_GATEWAY. Among those routes, the one with
+ * the numerically lowest metric is preferred; equal metrics keep the
+ * earliest entry.
+ *
+ * @param routes JSON array of routes parsed from /proc/net/route
+ * @return the preferred default route (owned by @p routes), or NULL if
+ *         there is no active default route with a usable metric
+ */
+static const JsonElement *FindLowestMetricDefaultRoute(const JsonElement *routes)
+{
+    assert(routes != NULL);
+
+    const JsonElement *default_route = NULL;
+    long lowest_metric = 0;
+
+    JsonIterator iter = JsonIteratorInit(routes);
+    const JsonElement *route = NULL;
+    while ((route = JsonIteratorNextValue(&iter)))
+    {
+        JsonElement *active = JsonObjectGet(route, "active_default_gateway");
+        if (active != NULL &&
+            JsonGetElementType(active) == JSON_ELEMENT_TYPE_PRIMITIVE &&
+            JsonGetPrimitiveType(active) == JSON_PRIMITIVE_TYPE_BOOL &&
+            JsonPrimitiveGetAsBool(active))
+        {
+            JsonElement *metric = JsonObjectGet(route, "metric");
+            if (metric != NULL &&
+                JsonGetElementType(metric) == JSON_ELEMENT_TYPE_PRIMITIVE &&
+                JsonGetPrimitiveType(metric) == JSON_PRIMITIVE_TYPE_INTEGER)
+            {
+                long metric_value = JsonPrimitiveGetAsInteger(metric);
+                if (default_route == NULL || metric_value < lowest_metric)
+                {
+                    default_route = route;
+                    lowest_metric = metric_value;
+                }
+            }
+        }
+    }
+
+    return default_route;
+}
+
+/*******************************************************************/
+
 void GetNetworkingInfo(EvalContext *ctx)
 {
     const char *procdir_root = GetRelocatedProcdirRoot();
@@ -1420,29 +1470,7 @@ void GetNetworkingInfo(EvalContext *ctx)
     {
         JsonObjectAppendElement(inet, "routes", routes);
 
-        JsonIterator iter = JsonIteratorInit(routes);
-        const JsonElement *default_route = NULL;
-        long lowest_metric = 0;
-        const JsonElement *route = NULL;
-        while ((route = JsonIteratorNextValue(&iter)))
-        {
-            JsonElement *active = JsonObjectGet(route, "active_default_gateway");
-            if (active != NULL &&
-                JsonGetElementType(active) == JSON_ELEMENT_TYPE_PRIMITIVE &&
-                JsonGetPrimitiveType(active) == JSON_PRIMITIVE_TYPE_BOOL &&
-                JsonPrimitiveGetAsBool(active))
-            {
-                JsonElement *metric = JsonObjectGet(route, "metric");
-                if (metric != NULL &&
-                    JsonGetElementType(metric) == JSON_ELEMENT_TYPE_PRIMITIVE &&
-                    JsonGetPrimitiveType(metric) == JSON_PRIMITIVE_TYPE_INTEGER &&
-                    (default_route == NULL ||
-                     JsonPrimitiveGetAsInteger(metric) < lowest_metric))
-                {
-                    default_route = route;
-                }
-            }
-        }
+        const JsonElement *default_route = FindLowestMetricDefaultRoute(routes);
 
         if (default_route != NULL)
         {

tests/unit/Makefile.am

@@ -420,6 +420,12 @@ check_PROGRAMS += nfs_test
 nfs_test_SOURCES = nfs_test.c
 nfs_test_LDADD = ../../libpromises/libpromises.la libtest.la
 
+check_PROGRAMS += unix_iface_test
+unix_iface_test_SOURCES = unix_iface_test.c
+unix_iface_test_LDADD = libtest.la \
+	../../libenv/libenv.la \
+	../../libpromises/libpromises.la
+
 init_script_test_helper_SOURCES = init_script_test_helper.c
 init_script_test.sh: init_script_test_helper
 CLEANFILES += init_script_test_helper

tests/unit/unix_iface_test.c

@@ -0,0 +1,128 @@
+#include <test.h>
+
+#include <unix_iface.c>
+
+/*
+ * FindLowestMetricDefaultRoute() operates on the JSON produced by parsing
+ * /proc/net/route, after NetworkingRoutesPostProcessInfo() has annotated
+ * each route with "active_default_gateway" and a numeric "metric". The
+ * routes here are constructed directly in that shape, so the selection
+ * logic can be exercised on any platform.
+ */
+static JsonElement *AppendRoute(
+    JsonElement *routes, const char *gateway, int metric, bool active)
+{
+    JsonElement *route = JsonObjectCreate(3);
+    JsonObjectAppendString(route, "gateway", gateway);
+    JsonObjectAppendInteger(route, "metric", metric);
+    JsonObjectAppendBool(route, "active_default_gateway", active);
+    JsonArrayAppendElement(routes, route);
+    return route;
+}
+
+static void test_no_routes(void)
+{
+    JsonElement *routes = JsonArrayCreate(1);
+
+    assert_true(FindLowestMetricDefaultRoute(routes) == NULL);
+
+    JsonDestroy(routes);
+}
+
+static void test_no_active_default_route(void)
+{
+    JsonElement *routes = JsonArrayCreate(2);
+    AppendRoute(routes, "192.168.0.1", 0, false);
+    AppendRoute(routes, "192.168.0.2", 100, false);
+
+    assert_true(FindLowestMetricDefaultRoute(routes) == NULL);
+
+    JsonDestroy(routes);
+}
+
+static void test_single_active_default_route(void)
+{
+    JsonElement *routes = JsonArrayCreate(2);
+    AppendRoute(routes, "192.168.0.1", 0, false);
+    JsonElement *expected = AppendRoute(routes, "192.168.0.2", 1024, true);
+
+    assert_true(FindLowestMetricDefaultRoute(routes) == expected);
+
+    JsonDestroy(routes);
+}
+
+static void test_lowest_metric_first(void)
+{
+    JsonElement *routes = JsonArrayCreate(2);
+    JsonElement *expected = AppendRoute(routes, "192.168.0.1", 100, true);
+    AppendRoute(routes, "192.168.0.2", 600, true);
+
+    assert_true(FindLowestMetricDefaultRoute(routes) == expected);
+
+    JsonDestroy(routes);
+}
+
+static void test_lowest_metric_last(void)
+{
+    /* The lowest metric must win even when it appears after another
+     * active default route; this is the case CFE-4723 got wrong. */
+    JsonElement *routes = JsonArrayCreate(3);
+    AppendRoute(routes, "192.168.0.1", 600, true);
+    AppendRoute(routes, "192.168.0.2", 100, true);
+    JsonElement *expected = AppendRoute(routes, "192.168.0.3", 50, true);
+
+    const JsonElement *found = FindLowestMetricDefaultRoute(routes);
+    assert_true(found != NULL);
+    assert_string_equal(JsonObjectGetAsString(found, "gateway"), "192.168.0.3");
+    assert_true(found == expected);
+
+    JsonDestroy(routes);
+}
+
+static void test_equal_metrics_keep_first(void)
+{
+    JsonElement *routes = JsonArrayCreate(2);
+    JsonElement *expected = AppendRoute(routes, "192.168.0.1", 100, true);
+    AppendRoute(routes, "192.168.0.2", 100, true);
+
+    assert_true(FindLowestMetricDefaultRoute(routes) == expected);
+
+    JsonDestroy(routes);
+}
+
+static void test_inactive_and_incomplete_routes_are_skipped(void)
+{
+    JsonElement *routes = JsonArrayCreate(3);
+
+    /* Lower metric, but not an active default gateway. */
+    AppendRoute(routes, "192.168.0.1", 1, false);
+
+    /* Active, but without a usable metric. */
+    JsonElement *no_metric = JsonObjectCreate(2);
+    JsonObjectAppendString(no_metric, "gateway", "192.168.0.2");
+    JsonObjectAppendBool(no_metric, "active_default_gateway", true);
+    JsonArrayAppendElement(routes, no_metric);
+
+    JsonElement *expected = AppendRoute(routes, "192.168.0.3", 600, true);
+
+    assert_true(FindLowestMetricDefaultRoute(routes) == expected);
+
+    JsonDestroy(routes);
+}
+
+int main()
+{
+    PRINT_TEST_BANNER();
+    const UnitTest tests[] =
+    {
+        unit_test(test_no_routes),
+        unit_test(test_no_active_default_route),
+        unit_test(test_single_active_default_route),
+        unit_test(test_lowest_metric_first),
+        unit_test(test_lowest_metric_last),
+        unit_test(test_equal_metrics_keep_first),
+        unit_test(test_inactive_and_incomplete_routes_are_skipped),
+    };
+
+    return run_tests(tests);
+}