The subject line only tells half the story. The other half is that
instead of hoping it only identifies Ethernet interfaces it now actually
checks and restricts itself to Ethernet interfaces. I couldn't help
myself.

If it helps my case for this code, I've been using this semantic for a
few years in a home grown dhcp-relay we run on our firewalls at
work. We used to run the dhcp-relay on a bunch of vlan(4) interfaces,
but we currently run them on carp(4). There's no significant difference
between using an Ethernet or carp interface from a dhcpd or relay point
of view, so the code accepts either.

The kernel lets you bind to addresses on down interfaces, so I don't
know why dhcpd has to be precious about it. We often start the
firewalls up with whole groups of these interfaces forced down.
We want DHCP packets to start moving when then interface comes up
though, therefore the daemon should start and be running even if
the interface is down.

Should I warn if the interface is down on startup?

There's some other questionable code in here, but I'm going to take
some deep breaths and try to ignore them for now.

OK?

Index: dispatch.c
===================================================================
RCS file: /cvs/src/usr.sbin/dhcpd/dispatch.c,v
retrieving revision 1.43
diff -u -p -r1.43 dispatch.c
--- dispatch.c  12 Apr 2017 19:17:30 -0000      1.43
+++ dispatch.c  15 Nov 2021 03:50:46 -0000
@@ -47,6 +47,7 @@
 #include <net/if.h>
 #include <net/if_dl.h>
 #include <net/if_media.h>
+#include <net/if_types.h>
 
 #include <netinet/in.h>
 
@@ -112,14 +113,10 @@ discover_interfaces(int *rdomain)
        for (ifa = ifap; ifa != NULL; ifa = ifa->ifa_next) {
                /*
                 * See if this is the sort of interface we want to
-                * deal with.  Skip loopback, point-to-point and down
-                * interfaces, except don't skip down interfaces if we're
-                * trying to get a list of configurable interfaces.
+                * deal with, which is basically Ethernet.
                 */
                if ((ifa->ifa_flags & IFF_LOOPBACK) ||
-                   (ifa->ifa_flags & IFF_POINTOPOINT) ||
-                   (!(ifa->ifa_flags & IFF_UP)) ||
-                   (!(ifa->ifa_flags & IFF_BROADCAST)))
+                   (ifa->ifa_flags & IFF_POINTOPOINT))
                        continue;
 
                /* See if we've seen an interface that matches this one. */
@@ -156,11 +153,27 @@ discover_interfaces(int *rdomain)
                /* If we have the capability, extract link information
                   and record it in a linked list. */
                if (ifa->ifa_addr->sa_family == AF_LINK) {
-                       struct sockaddr_dl *sdl =
-                           ((struct sockaddr_dl *)(ifa->ifa_addr));
+                       struct if_data *ifi;
+                       struct sockaddr_dl *sdl;
+
+                       ifi = (struct if_data *)ifa->ifa_data;
+                       if (ifi->ifi_type != IFT_ETHER &&
+                           ifi->ifi_type != IFT_CARP) {
+                               /* this interface will be cleaned up later */
+                               continue;
+                       }
+
+                       sdl = ((struct sockaddr_dl *)(ifa->ifa_addr));
+                       if (sdl->sdl_alen > sizeof(tmp->hw_address.haddr)) {
+                               log_warnx("interface %s long hardware address",
+                                   tmp->name);
+                               /* this interface will be cleaned up later */
+                               continue;
+                       }
+
                        tmp->index = sdl->sdl_index;
                        tmp->hw_address.hlen = sdl->sdl_alen;
-                       tmp->hw_address.htype = HTYPE_ETHER; /* XXX */
+                       tmp->hw_address.htype = HTYPE_ETHER;
                        memcpy(tmp->hw_address.haddr,
                            LLADDR(sdl), sdl->sdl_alen);
                } else if (ifa->ifa_addr->sa_family == AF_INET) {
@@ -236,6 +249,17 @@ discover_interfaces(int *rdomain)
        last = NULL;
        for (tmp = interfaces; tmp; tmp = next) {
                next = tmp->next;
+
+               if (tmp->index == 0) {
+                       log_warnx("Can't listen on %s - wrong interface type",
+                           tmp->name);
+                       /* Remove tmp from the list of interfaces. */
+                       if (!last)
+                               interfaces = interfaces->next;
+                       else
+                               last->next = tmp->next;
+                       continue;
+               }
 
                if (!tmp->ifp) {
                        log_warnx("Can't listen on %s - it has no IP address.",

Reply via email to