From: Roy Marples Date: Tue, 10 Mar 2009 17:28:18 +0000 (+0000) Subject: We need to blacklist IP addresses at the packet level X-Git-Tag: v5.0.0~45 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ce6b39df64069a367cf62fd2bf450613ea54fc1a;p=thirdparty%2Fdhcpcd.git 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 3eac18c0..93bd3ea4 100644 --- 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 c1b8f637..4efa0e0d 100644 --- 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 664d0d11..752ca555 100644 --- 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 3c6ca1a0..4f71de15 100644 --- 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 7e79633a..c49031df 100644 --- 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 b6d1f6bf..b9cb280c 100644 --- 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 3ebc454a..19abb91d 100644 --- 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,