We need to blacklist IP addresses at the packet level

so we can ignore NAKs from rogue servers who don't
supply a ServerID, or supply a fake one.

diff --git a/arp.c b/arp.c
index 3eac18c0b01498d3431e70c1bf032bb8362a36a4..93bd3ea4c7a17c303206b5cccec28aa7a361b1c0 100644 (file)
--- a/arp.c
+++ b/arp.c
@@ -40,7 +40,7 @@
 #include "ipv4ll.h"
 #include "net.h"
 
-#define ARP_LEN                                                                \
+#define ARP_LEN                                                                      \
        (sizeof(struct arphdr) + (2 * sizeof(uint32_t)) + (2 * HWADDR_LEN))
 
 static int
@@ -140,7 +140,7 @@ handle_arp_packet(void *arg)
                memcpy(&reply_t, hw_t + ar.ar_hln, ar.ar_pln);
 
                /* Check for conflict */
-               if (state->offer && 
+               if (state->offer &&
                    (reply_s == state->offer->yiaddr ||
                        (reply_s == 0 && reply_t == state->offer->yiaddr)))
                        state->fail.s_addr = state->offer->yiaddr;

diff --git a/dhcpcd.8.in b/dhcpcd.8.in
index c1b8f63773441e954f68abe3c5e431c20f9ada0e..4efa0e0d1ecaefcb4fa7d87b106309c9b5cd0718 100644 (file)
--- a/dhcpcd.8.in
+++ b/dhcpcd.8.in
@@ -22,7 +22,7 @@
 .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 .\" SUCH DAMAGE.
 .\"
-.Dd February 27, 2009
+.Dd March 10, 2009
 .Dt DHCPCD 8 SMM
 .Sh NAME
 .Nm dhcpcd
@@ -452,15 +452,8 @@ files.
 Display a list of option codes and the associated variable for use in
 .Xr dhcpcd-run-hooks 8 .
 .It Fl X, -blacklist Ar address Ns Op Ar /cidr
-Ignores all DHCP messages which have this
-.Ar address
-as the server ID or offered address.
-If
-.Ar cidr
-is given then we match against that network as well.
-This may be expanded in future releases to ignore all packets
-matching either the IP or hardware
-.Ar address .
+Ignore all packets from
+.Ar address Ns Op Ar /cidr .
 .It Fl Z , -denyinterfaces Ar pattern
 When discovering interfaces, the interface name must not match
 .Ar pattern

diff --git a/dhcpcd.c b/dhcpcd.c
index 664d0d11804bc4950f2a4ee2bb314e10288d7240..752ca5554be632bf8eae86b1617bdfe0d5e48236 100644 (file)
--- a/dhcpcd.c
+++ b/dhcpcd.c
@@ -411,6 +411,17 @@ log_dhcp(int lvl, const char *msg,
        free(a);
 }
 
+static int
+blacklisted_ip(const struct if_options *ifo, in_addr_t addr)
+{
+       size_t i;
+       
+       for (i = 0; i < ifo->blacklist_len; i += 2)
+               if (ifo->blacklist[i] == (addr & ifo->blacklist[i + 1]))
+                       return 1;
+       return 0;
+}
+
 static void
 handle_dhcp(struct interface *iface, struct dhcp_message **dhcpp)
 {
@@ -419,9 +430,8 @@ handle_dhcp(struct interface *iface, struct dhcp_message **dhcpp)
        struct dhcp_message *dhcp = *dhcpp;
        struct dhcp_lease *lease = &state->lease;
        uint8_t type, tmp;
-       struct in_addr addr, addr2;
+       struct in_addr addr;
        size_t i;
-       char *a;
 
        /* reset the message counter */
        state->interval = 0;
@@ -430,52 +440,6 @@ handle_dhcp(struct interface *iface, struct dhcp_message **dhcpp)
        if (get_option_uint8(&type, dhcp, DHO_MESSAGETYPE) == -1) 
                type = 0;
 
-       /* Ensure that it's not from a blacklisted server.
-        * We should expand this to check IP and/or hardware address
-        * at the packet level. */
-       if (ifo->blacklist_len != 0) {
-               if (get_option_addr(&addr.s_addr, dhcp, DHO_SERVERID) != 0)
-                       addr.s_addr = 0;
-               for (i = 0; i < ifo->blacklist_len; i += 2) {
-                       if (ifo->blacklist[i] ==
-                           (addr.s_addr & ifo->blacklist[i + 1]))
-                       {
-                               if (dhcp->servername[0])
-                                       syslog(LOG_WARNING,
-                                           "%s: blacklisted server %s `%s'",
-                                           iface->name,
-                                           inet_ntoa(addr), dhcp->servername);
-                               else
-                                       syslog(LOG_WARNING,
-                                           "%s: blacklisted server %s",
-                                           iface->name, inet_ntoa(addr));
-                               return;
-                       }
-                       if (ifo->blacklist[i] ==
-                           (dhcp->yiaddr & ifo->blacklist[i + 1]))
-                       {
-                               addr2.s_addr = dhcp->yiaddr;
-                               a = xstrdup(inet_ntoa(addr2));
-                               if (dhcp->servername[0])
-                                       syslog(LOG_WARNING,
-                                           "%s: blacklisted offer"
-                                           " %s from %s `%s'",
-                                           iface->name, a,
-                                           inet_ntoa(addr), dhcp->servername);
-                               else if (addr.s_addr)
-                                       syslog(LOG_WARNING,
-                                           "%s: blacklisted offer %s from %s",
-                                           iface->name, a, inet_ntoa(addr));
-                               else
-                                       syslog(LOG_WARNING,
-                                           "%s: blacklisted offer %s",
-                                           iface->name, a);
-                               free(a);
-                               return;
-                       }
-               }
-       }
-
        /* We should restart on a NAK */
        if (type == DHCP_NAK) {
                log_dhcp(LOG_WARNING, "NAK:", iface, dhcp);
@@ -514,7 +478,8 @@ handle_dhcp(struct interface *iface, struct dhcp_message **dhcpp)
                lease->addr.s_addr = dhcp->yiaddr;
                lease->server.s_addr = 0;
                if (type)
-                       get_option_addr(&lease->server.s_addr, dhcp, DHO_SERVERID);
+                       get_option_addr(&lease->server.s_addr, dhcp,
+                           DHO_SERVERID);
                log_dhcp(LOG_INFO, "offered", iface, dhcp);
                free(state->offer);
                state->offer = dhcp;
@@ -595,6 +560,7 @@ handle_dhcp_packet(void *arg)
        struct dhcp_message *dhcp = NULL;
        const uint8_t *pp;
        ssize_t bytes;
+       struct in_addr from;
 
        /* We loop through until our buffer is empty.
         * The benefit is that if we get >1 DHCP packet in our buffer and
@@ -605,35 +571,45 @@ handle_dhcp_packet(void *arg)
                    packet, udp_dhcp_len);
                if (bytes == 0 || bytes == -1)
                        break;
-               if (valid_udp_packet(packet, bytes) == -1)
+               if (valid_udp_packet(packet, bytes, &from) == -1) {
+                       syslog(LOG_ERR, "%s: invalid UDP packet from %s",
+                           iface->name, inet_ntoa(from));
                        continue;
+               }
+               if (blacklisted_ip(iface->state->options, from.s_addr)) {
+                       syslog(LOG_WARNING,
+                           "%s: blacklisted DHCP packet from %s",
+                           iface->name, inet_ntoa(from));
+                       continue;
+               }
                bytes = get_udp_data(&pp, packet);
                if ((size_t)bytes > sizeof(*dhcp)) {
-                       syslog(LOG_ERR, "%s: packet greater than DHCP size",
-                           iface->name);
+                       syslog(LOG_ERR,
+                           "%s: packet greater than DHCP size from %s",
+                           iface->name, inet_ntoa(from));
                        continue;
                }
                if (!dhcp)
                        dhcp = xzalloc(sizeof(*dhcp));
                memcpy(dhcp, pp, bytes);
                if (dhcp->cookie != htonl(MAGIC_COOKIE)) {
-                       syslog(LOG_DEBUG, "%s: bogus cookie, ignoring",
-                           iface->name);
+                       syslog(LOG_DEBUG, "%s: bogus cookie from %s",
+                           iface->name, inet_ntoa(from));
                        continue;
                }
                /* Ensure it's the right transaction */
                if (iface->state->xid != dhcp->xid) {
                        syslog(LOG_DEBUG,
-                           "%s: ignoring packet with xid 0x%x as"
-                           " it's not ours (0x%x)",
-                           iface->name, dhcp->xid, iface->state->xid);
+                           "%s: wrong xid 0x%x (expecting 0x%x) from %s",
+                           iface->name, dhcp->xid, iface->state->xid,
+                           inet_ntoa(from));
                        continue;
                }
                /* Ensure packet is for us */
                if (iface->hwlen <= sizeof(dhcp->chaddr) &&
                    memcmp(dhcp->chaddr, iface->hwaddr, iface->hwlen))
                {
-                       syslog(LOG_DEBUG, "%s: xid 0x%x is not for our hwaddr %s",
+                       syslog(LOG_DEBUG, "%s: xid 0x%x is not for hwaddr %s",
                            iface->name, dhcp->xid,
                            hwaddr_ntoa(dhcp->chaddr, sizeof(dhcp->chaddr)));
                        continue;

diff --git a/dhcpcd.conf.5.in b/dhcpcd.conf.5.in
index 3c6ca1a0b897b9bec8a8294cd966fce51cf92542..4f71de156d89d32116e24cffc17d65985f0ab4b6 100644 (file)
--- a/dhcpcd.conf.5.in
+++ b/dhcpcd.conf.5.in
@@ -22,7 +22,7 @@
 .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 .\" SUCH DAMAGE.
 .\"
-.Dd February 27, 2009
+.Dd March 10, 2009
 .Dt DHCPCD.CONF 5 SMM
 .Sh NAME
 .Nm dhcpcd.conf
@@ -61,15 +61,8 @@ Background immediately.
 This is useful for startup scripts which don't disable link messages for
 carrier status.
 .It Ic blacklist Ar address Ns Op Ar /cidr
-Ignores all DHCP messages which have this
-.Ar address
-as the server ID or offered address.
-If
-.Ar cidr
-is given then we match against that network as well.
-This may be expanded in future releases to ignore all packets
-matching either the IP or hardware
-.Ar address .
+Ignores all packets from
+.Ar address Ns Op Ar /cidr .
 .It Ic clientid Ar string
 Send the
 .Ar clientid .

diff --git a/if-options.c b/if-options.c
index 7e79633abcd2e5c2e4b4100af49a1787bc02507d..c49031dff1784a26d5e4028374b3319dcd5ae5cc 100644 (file)
--- a/if-options.c
+++ b/if-options.c
@@ -634,9 +634,10 @@ parse_option(struct if_options *ifo, int opt, const char *arg)
                }
                break;
        case 'X':
-               addr2.s_addr = ~0U;
                if (parse_addr(&addr, &addr2, arg) != 0)
                        return -1;
+               if (strchr(arg, '/') == NULL)
+                       addr2.s_addr = INADDR_BROADCAST;
                ifo->blacklist = xrealloc(ifo->blacklist,
                    sizeof(in_addr_t) * (ifo->blacklist_len + 2));
                ifo->blacklist[ifo->blacklist_len++] = addr.s_addr;

diff --git a/net.c b/net.c
index b6d1f6bfd0e2c81230b986c4b59a1267c936d283..b9cb280c2d1861ab7df0bc09bbdd1f3759dc626a 100644 (file)
--- a/net.c
+++ b/net.c
@@ -639,16 +639,24 @@ get_udp_data(const uint8_t **data, const uint8_t *udp)
 }
 
 int
-valid_udp_packet(const uint8_t *data, size_t data_len)
+valid_udp_packet(const uint8_t *data, size_t data_len, struct in_addr *from)
 {
        struct udp_dhcp_packet packet;
        uint16_t bytes, udpsum;
 
+       if (data_len < sizeof(packet.ip)) {
+               if (from)
+                       from->s_addr = INADDR_ANY;
+               errno = EINVAL;
+               return -1;
+       }
+       memcpy(&packet, data, MIN(data_len, sizeof(packet)));
+       if (from)
+               from->s_addr = packet.ip.ip_src.s_addr;
        if (data_len > sizeof(packet)) {
                errno = EINVAL;
                return -1;
        }
-       memcpy(&packet, data, data_len);
        if (checksum(&packet.ip, sizeof(packet.ip)) != 0) {
                errno = EINVAL;
                return -1;

diff --git a/net.h b/net.h
index 3ebc454aa063b07af44e2daf352261665277db49..19abb91da408dac4e767643456c9bf65dea17d5f 100644 (file)
--- a/net.h
+++ b/net.h
@@ -137,7 +137,7 @@ const size_t udp_dhcp_len;
 ssize_t make_udp_packet(uint8_t **, const uint8_t *, size_t,
     struct in_addr, struct in_addr);
 ssize_t get_udp_data(const uint8_t **, const uint8_t *);
-int valid_udp_packet(const uint8_t *, size_t);
+int valid_udp_packet(const uint8_t *, size_t, struct in_addr *);
 
 int open_socket(struct interface *, int);
 ssize_t send_packet(const struct interface *, struct in_addr,