daemon/interfaces: fix management address selection when negative

When we had a negative IP address, and nothing positive, the address was
still selected as it didn't match an interface. When all negative, we
should only select an address if both IP and interface are allowed.

This mechanism is becoming a bit complex.

diff --git a/NEWS b/NEWS
index ed93554a90d9b88c022f16ca8334195f774d9fb9..142f89e87a40c59b199c8aec95e1111e4af2f01a 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -5,6 +5,7 @@ lldpd (1.0.20)
  * Fix:
    + Do not break zero-copy traffic on Linux (#732 and #733)
    + Fix crash on rapid addition/removal of interfaces (#744)
+   + Fix management address selection when pattern is a negative IP address
 
 lldpd (1.0.19)
  * Changes:

diff --git a/src/daemon/interfaces.c b/src/daemon/interfaces.c
index 58195b855b83ed950e23919c16774040713b89b1..2aaec7693806bddb67900a66079ca3fb9eb1b778 100644 (file)
--- a/src/daemon/interfaces.c
+++ b/src/daemon/interfaces.c
@@ -400,9 +400,29 @@ interfaces_helper_chassis(struct lldpd *cfg, struct interfaces_device_list *inte
 #undef IN6_IS_ADDR_GLOBAL
 #define IN6_IS_ADDR_GLOBAL(a) (!IN6_IS_ADDR_LOOPBACK(a) && !IN6_IS_ADDR_LINKLOCAL(a))
 
+static int
+interfaces_allowed_mgt(struct lldpd *cfg, struct interfaces_device_list *interfaces,
+    struct interfaces_address *addr, char *addrstrbuf, int allnegative)
+{
+       struct interfaces_device *device;
+       int addr_match, device_match;
+       if (cfg->g_config.c_mgmt_pattern == NULL) {
+               return 1;
+       }
+       device = interfaces_indextointerface(interfaces, addr->index);
+       if (allnegative) {
+               addr_match = pattern_match(addrstrbuf, cfg->g_config.c_mgmt_pattern, PATTERN_MATCH_ALLOWED);
+               device_match = device && pattern_match(device->name, cfg->g_config.c_mgmt_pattern, PATTERN_MATCH_ALLOWED);
+               return addr_match && device_match;
+       }
+       addr_match = pattern_match(addrstrbuf, cfg->g_config.c_mgmt_pattern, PATTERN_MATCH_DENIED);
+       device_match = device && pattern_match(device->name, cfg->g_config.c_mgmt_pattern, PATTERN_MATCH_DENIED);
+       return addr_match || device_match;
+}
+
 /* Add management addresses for the given family. We only take one of each
    address family, unless a pattern is provided and is not all negative. For
-   example !*:*,!10.* will only deny addresses. We will pick the first IPv4
+   example !*:*,!10.* will only deny IPv6 addresses. We will pick the first IPv4
    address not matching 10.*.
 */
 static int
@@ -411,7 +431,6 @@ interfaces_helper_mgmt_for_af(struct lldpd *cfg, int af,
     int global, int allnegative)
 {
        struct interfaces_address *addr;
-       struct interfaces_device *device;
        struct lldpd_mgmt *mgmt;
        char addrstrbuf[INET6_ADDRSTRLEN];
        int found = 0;
@@ -454,14 +473,7 @@ interfaces_helper_mgmt_for_af(struct lldpd *cfg, int af,
                            "unable to convert IP address to a string");
                        continue;
                }
-               if (cfg->g_config.c_mgmt_pattern == NULL ||
-                   /* Match on IP address */
-                   pattern_match(addrstrbuf, cfg->g_config.c_mgmt_pattern,
-                       allnegative) ||
-                   /* Match on interface name */
-                   ((device = interfaces_indextointerface(interfaces, addr->index)) &&
-                       pattern_match(device->name, cfg->g_config.c_mgmt_pattern,
-                           allnegative))) {
+               if (interfaces_allowed_mgt(cfg, interfaces, addr, addrstrbuf, allnegative)) {
                        mgmt =
                            lldpd_alloc_mgmt(af, &in_addr, in_addr_size, addr->index);
                        if (mgmt == NULL) {

diff --git a/src/daemon/pattern.c b/src/daemon/pattern.c
index 0f9885d2d602e390070f23e7f44f5ab293809309..e88ff8f6b98909e9e6deef09d278c60473fb414b 100644 (file)
--- a/src/daemon/pattern.c
+++ b/src/daemon/pattern.c
@@ -30,7 +30,7 @@
  *                 case, it is allowed back. Each pattern will then be
  *                 matched against `fnmatch()` function.
  * @param found    Value to return if the pattern isn't found. Should be either
- *                 PATTERN_MATCH_DENIED or PATTERN_MACTH_DENIED.
+ *                 PATTERN_MATCH_ALLOWED or PATTERN_MACTH_DENIED.
  *
  * If a pattern is found matching and denied at the same time, it
  * will be denied. If it is both allowed and denied, it

diff --git a/tests/integration/test_basic.py b/tests/integration/test_basic.py
index 9dd34803be1b992f4cff41ae51ab87e76ac96977..642f44f608107db3ee3a2fc4b9a59b6ecfb86a1f 100644 (file)
--- a/tests/integration/test_basic.py
+++ b/tests/integration/test_basic.py
@@ -216,6 +216,32 @@ def test_management_address(lldpd1, lldpd, lldpcli, links, namespaces):
         assert out["lldp.eth0.chassis.mgmt-iface"] == "2"
 
 
+def test_negative_management_address(lldpd1, lldpd, lldpcli, links, namespaces):
+    with namespaces(2):
+        with pyroute2.IPRoute() as ipr:
+            idx = ipr.link_lookup(ifname="eth1")[0]
+            ipr.addr("add", index=idx, address="192.168.14.2", prefixlen=24)
+            ipr.addr("add", index=idx, address="172.25.21.47", prefixlen=24)
+        lldpd("-m", "!192.168.14.2,!*:*")
+    with namespaces(1):
+        out = lldpcli("-f", "keyvalue", "show", "neighbors")
+        assert out["lldp.eth0.chassis.mgmt-ip"] == "172.25.21.47"
+        assert out["lldp.eth0.chassis.mgmt-iface"] == "2"
+
+
+def test_negative_unknown_management_address(lldpd1, lldpd, lldpcli, namespaces):
+    with namespaces(2):
+        with pyroute2.IPRoute() as ipr:
+            idx = ipr.link_lookup(ifname="eth1")[0]
+            ipr.addr("add", index=idx, address="192.168.14.2", prefixlen=24)
+            ipr.addr("add", index=idx, address="172.25.21.47", prefixlen=24)
+        lldpd("-m", "!192.168.14.2,!*:*,192.0.2.15")
+    with namespaces(1):
+        out = lldpcli("-f", "keyvalue", "show", "neighbors")
+        assert "lldp.eth0.chassis.mgmt-ip" not in out
+        assert "lldp.eth0.chassis.mgmt-iface" not in out
+
+
 def test_management_interface(lldpd1, lldpd, lldpcli, links, namespaces):
     links(namespaces(1), namespaces(2), 4)
     with namespaces(2):