glib-aux: avoid using inet_aton()

nm_inet_parse_bin_full() supports a legacy mode for IPv4, which used
inet_aton(). This is only used by initrd reader, which parses the
kernel command line as defined by dracut. Since that dracut API is old
and not defined by us, we want to be more forgiving in case a user
specifies something that used to work in the past. In particular,
we want to parse "255.256.256.000" as netmask (which inet_pton() would
reject).

inet_aton() trips off some ABI checkers that we shouldn't use this ABI.
It was anyway only used as *additional* guard when we parsed certain
legacy formats for IPv4 addresses. We can drop that and just use our
parser.

Note that there is still an nm_assert() path, which loads inet_aton()
dynamically, just to ensure that our legacy parser implementation is in
agree with inet_aton().

https://bugzilla.redhat.com/show_bug.cgi?id=2049134

4 files changed, 127 insertions, 40 deletions

diff --git a/src/libnm-glib-aux/nm-inet-utils.c b/src/libnm-glib-aux/nm-inet-utils.c
index 7f710f3527..87fe3ce050 100644
--- a/src/libnm-glib-aux/nm-inet-utils.c
+++ b/src/libnm-glib-aux/nm-inet-utils.c
@@ -6,6 +6,7 @@
 
 #include <netinet/in.h>
 #include <arpa/inet.h>
+#include <dlfcn.h>
 
 /*****************************************************************************/
 
@@ -266,34 +267,118 @@ nm_ip6_addr_same_prefix_cmp(const struct in6_addr *addr_a,
 
 /*****************************************************************************/
 
-static gboolean
-_parse_legacy_addr4(const char *text, in_addr_t *out_addr, GError **error)
+static int
+_inet_aton(const char *text, in_addr_t *out_addr)
 {
-    gs_free char  *s_free = NULL;
-    struct in_addr a1;
-    guint8         bin[sizeof(a1)];
-    char          *s;
-    int            i;
+    /* Call inet_aton() via dlopen.
+     *
+     * The inet_aton() API is discouraged, and ABI checkers warn when we call
+     * it.
+     *
+     * We want to use this function, but only for testing/asserting. To avoid
+     * the ABI checker's complain, dlopen() the symbol. This is not used for
+     * production.
+     */
+    static gpointer mod_handle  = NULL;
+    static gpointer fcn_sym     = NULL;
+    static gsize    initialized = 0;
+    int (*fcn)(const char *text, struct in_addr *out_addr);
+    int       r;
+    in_addr_t a;
+
+    if (g_once_init_enter(&initialized)) {
+        mod_handle = dlopen(NULL, RTLD_LAZY);
+        if (mod_handle) {
+            fcn_sym = dlsym(mod_handle, "inet_aton");
+            if (!fcn_sym)
+                dlclose(g_steal_pointer(&mod_handle));
+        }
+        g_once_init_leave(&initialized, 1);
+    }
 
-    if (inet_aton(text, &a1) != 1) {
-        g_set_error_literal(error,
-                            NM_UTILS_ERROR,
-                            NM_UTILS_ERROR_INVALID_ARGUMENT,
-                            "address invalid according to inet_aton()");
-        return FALSE;
+    if (!fcn_sym)
+        return -ENOSYS;
+
+    g_assert(mod_handle);
+
+    fcn = fcn_sym;
+    r   = fcn(text, (gpointer) &a);
+
+    if (r != 1)
+        return -EINVAL;
+
+    NM_SET_OUT(out_addr, a);
+    return 0;
+}
+
+int
+nmtst_inet_aton(const char *text, in_addr_t *out_addr)
+{
+    return _inet_aton(text, out_addr);
+}
+
+static void
+_nm_assert_legacy_addr4(const char *text, in_addr_t addr)
+{
+#if NM_MORE_ASSERTS > 20
+    char      buf1[NM_INET_ADDRSTRLEN];
+    char      buf2[NM_INET_ADDRSTRLEN];
+    int       r;
+    in_addr_t a;
+
+    /* Our legacy parser accepted "text" as "addr".
+     *
+     * However, we want to ensure that whatever we parse is also parsed by old
+     * inet_aton(). So we want to be strictly more strict than inet_aton() in
+     * what we accept.
+     */
+
+    r = _inet_aton(text, &a);
+
+    if (r != 0) {
+        if (r == -ENOSYS)
+            return;
+        g_error("inet_aton(\"%s\") failed with \"%s\", but we expected %s",
+                text,
+                nm_strerror_native(-r),
+                nm_inet4_ntop(addr, buf2));
     }
 
-    /* OK, inet_aton() accepted the format. That's good, because we want
-     * to accept IPv4 addresses in octal format, like 255.255.000.000.
-     * That's what "legacy" means here. inet_pton() doesn't accept those.
+    if (a != addr) {
+        g_error("inet_aton(\"%s\") parsed %s, but we expected %s",
+                text,
+                nm_inet4_ntop(a, buf1),
+                nm_inet4_ntop(addr, buf2));
+    }
+#endif
+}
+
+static gboolean
+_parse_legacy_addr4(const char *text, in_addr_t *out_addr, GError **error)
+{
+    gs_free char *s_free = NULL;
+    union {
+        guint8    b[sizeof(in_addr_t)];
+        in_addr_t a;
+    } addr;
+    char *s;
+    int   i;
+
+    /* inet_pton() does strict parsing of IPv4 address. Good.
      *
-     * But inet_aton() also ignores trailing garbage and formats with fewer than
-     * 4 digits. That is just too crazy and we don't do that. Perform additional checks
-     * and reject some forms that inet_aton() accepted.
+     * However, inet_aton() used to accept much more relaxed forms (e.g. octal
+     * and hex numbers, not having 4 components but fewer, ignore any trailing
+     * garbage).
+     *
+     * Some places where we accept input, we want to be slightly more forgiving
+     * than inet_pton() and accept some (not all!) forms of what inet_aton()
+     * would accept. For example, we want to accept 255.000.000.000.
+     *
+     * We reimplement that below.
      *
      * Note that we still should (of course) accept everything that inet_pton()
-     * accepts. However this code never gets called if inet_pton() succeeds
-     * (see below, aside the assertion code). */
+     * accepts. This is ensured because the caller only calls this function
+     * after inet_pton() failed. */
 
     if (NM_STRCHAR_ANY(text, ch, (!(ch >= '0' && ch <= '9') && !NM_IN_SET(ch, '.', 'x')))) {
         /* We only accepts '.', digits, and 'x' for "0x". */
@@ -306,7 +391,7 @@ _parse_legacy_addr4(const char *text, in_addr_t *out_addr, GError **error)
 
     s = nm_memdup_maybe_a(300, text, strlen(text) + 1, &s_free);
 
-    for (i = 0; i < G_N_ELEMENTS(bin); i++) {
+    for (i = 0; i < G_N_ELEMENTS(addr.b); i++) {
         char  *current_token = s;
         gint32 v;
 
@@ -316,7 +401,7 @@ _parse_legacy_addr4(const char *text, in_addr_t *out_addr, GError **error)
             s++;
         }
 
-        if ((i == G_N_ELEMENTS(bin) - 1) != (s == NULL)) {
+        if ((i == G_N_ELEMENTS(addr.b) - 1) != (s == NULL)) {
             /* Exactly for the last digit, we expect to have no more following token.
              * But this isn't the case. Abort. */
             g_set_error(error,
@@ -344,26 +429,12 @@ _parse_legacy_addr4(const char *text, in_addr_t *out_addr, GError **error)
             return FALSE;
         }
 
-        bin[i] = v;
+        addr.b[i] = v;
     }
 
-    if (memcmp(bin, &a1, sizeof(bin)) != 0) {
-        /* our parsing did not agree with what inet_aton() gave. Something
-         * is wrong. Abort. */
-        g_set_error(
-            error,
-            NM_UTILS_ERROR,
-            NM_UTILS_ERROR_INVALID_ARGUMENT,
-            "inet_aton() result 0x%08x differs from computed value 0x%02hhx%02hhx%02hhx%02hhx",
-            a1.s_addr,
-            bin[0],
-            bin[1],
-            bin[2],
-            bin[3]);
-        return FALSE;
-    }
+    _nm_assert_legacy_addr4(text, addr.a);
 
-    *out_addr = a1.s_addr;
+    *out_addr = addr.a;
     return TRUE;
 }
 
diff --git a/src/libnm-glib-aux/nm-inet-utils.h b/src/libnm-glib-aux/nm-inet-utils.h
index 087a8af179..65ceeb2e04 100644
--- a/src/libnm-glib-aux/nm-inet-utils.h
+++ b/src/libnm-glib-aux/nm-inet-utils.h
@@ -369,6 +369,8 @@ nm_inet6_ntop_dup(const struct in6_addr *addr)
 
 /*****************************************************************************/
 
+int nmtst_inet_aton(const char *text, in_addr_t *out_addr);
+
 gboolean nm_inet_parse_bin_full(int         addr_family,
                                 gboolean    accept_legacy,
                                 const char *text,
diff --git a/src/libnm-glib-aux/tests/test-shared-general.c b/src/libnm-glib-aux/tests/test-shared-general.c
index 82a0d3e415..84d87d4848 100644
--- a/src/libnm-glib-aux/tests/test-shared-general.c
+++ b/src/libnm-glib-aux/tests/test-shared-general.c
@@ -2382,6 +2382,15 @@ _inet_parse(int addr_family, const char *str, gboolean accept_legacy, gpointer o
             nmtst_assert_ip6_address(&_addr[0].addr6, _expected);               \
         else                                                                    \
             nmtst_assert_ip4_address(_addr[1].addr4, _expected);                \
+                                                                                \
+        if (_success[1]) {                                                      \
+            in_addr_t _a4;                                                      \
+            int       _r;                                                       \
+                                                                                \
+            _r = nmtst_inet_aton(_check, &_a4);                                 \
+            g_assert_cmpint(_r, ==, 0);                                         \
+            nmtst_assert_ip4_address(_a4, _expected);                           \
+        }                                                                       \
     }                                                                           \
     G_STMT_END
 
diff --git a/src/nm-initrd-generator/nmi-dt-reader.c b/src/nm-initrd-generator/nmi-dt-reader.c
index f1279d5807..b273822a36 100644
--- a/src/nm-initrd-generator/nmi-dt-reader.c
+++ b/src/nm-initrd-generator/nmi-dt-reader.c
@@ -96,6 +96,11 @@ str_addr(const char *str, int *family)
 {
     NMIPAddr addr_bin;
 
+    /* For IPv4, we need to be more tolerant than inet_pton() to recognize
+     * things like the extra zeroes in "255.255.255.000".
+     *
+     * Pass accept_legacy=TRUE to nm_inet_parse_bin_full(), which also accepts
+     * such forms (but not everything which inet_aton() accepts). */
     if (!nm_inet_parse_bin_full(*family, TRUE, str, family, &addr_bin)) {
         _LOGW(LOGD_CORE, "Malformed IP address: '%s'", str);
         return NULL;