[systemd-devel] [PATCH 1/2] dhcp-network: add check for DHCP.chaddr

Tom Gundersen teg at jklm.no
Tue Jun 24 06:54:54 PDT 2014


On Tue, Jun 24, 2014 at 12:58 PM, Michal Sekletar <msekleta at redhat.com> wrote:
> On Thu, Jun 19, 2014 at 12:02:39PM -0500, Dan Williams wrote:
>> On Thu, 2014-06-19 at 18:59 +0200, Michal Sekletar wrote:
>> > Check that received DHCP packets actually include our MAC address in
>> > chaddr field. BPF interpreter has 32 bit wide registers but MAC address
>> > is 48 bits long so we have to do check in two steps.
>>
>> And if the MAC address is not 48 bits (Infiniband) or the interface type
>> doesn't really have a MAC address (PPP/PPPoE)?  DHCP can be used in both
>> cases...  If anything, the code that uses the MAC address should be
>> if-ed so it only executes when a MAC address is provided.
>
> I reconsidered. In filter code I can very easily find out type of device to which
> filter is attached and skip ethernet specific bits or fallback to something
> else. However filter already assumes ethernet data-link type moreover there is a ton of
> code in networkd which does likewise.
>
> I think we should merge patch as is right now. If somebody will hack up patches
> for supporting various data-link types I will be happy to patch filter
> accordingly.

Ok. I'm fine with that. Please push.

Just to make clear: I'd be happy if anyone wanted to turn all our libs
to work also on non-ethernet (and test it!), and would merge such
patches.

Cheers,

Tom

>> >  src/libsystemd-network/dhcp-internal.h    |  3 ++-
>> >  src/libsystemd-network/dhcp-network.c     | 15 +++++++++++++--
>> >  src/libsystemd-network/sd-dhcp-client.c   |  4 ++--
>> >  src/libsystemd-network/test-dhcp-client.c |  2 +-
>> >  4 files changed, 18 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/src/libsystemd-network/dhcp-internal.h b/src/libsystemd-network/dhcp-internal.h
>> > index 03cc824..fd86a70 100644
>> > --- a/src/libsystemd-network/dhcp-internal.h
>> > +++ b/src/libsystemd-network/dhcp-internal.h
>> > @@ -24,12 +24,13 @@
>> >
>> >  #include <stdint.h>
>> >  #include <linux/if_packet.h>
>> > +#include <net/ethernet.h>
>> >
>> >  #include "socket-util.h"
>> >
>> >  #include "dhcp-protocol.h"
>> >
>> > -int dhcp_network_bind_raw_socket(int index, union sockaddr_union *link, uint32_t xid);
>> > +int dhcp_network_bind_raw_socket(int index, union sockaddr_union *link, uint32_t xid, struct ether_addr mac_addr);
>> >  int dhcp_network_bind_udp_socket(be32_t address, uint16_t port);
>> >  int dhcp_network_send_raw_socket(int s, const union sockaddr_union *link,
>> >                                   const void *packet, size_t len);
>> > diff --git a/src/libsystemd-network/dhcp-network.c b/src/libsystemd-network/dhcp-network.c
>> > index d7ba0ef..e5fd6a8 100644
>> > --- a/src/libsystemd-network/dhcp-network.c
>> > +++ b/src/libsystemd-network/dhcp-network.c
>> > @@ -33,7 +33,7 @@
>> >  #include "dhcp-internal.h"
>> >
>> >  int dhcp_network_bind_raw_socket(int index, union sockaddr_union *link,
>> > -                                 uint32_t xid) {
>> > +                                 uint32_t xid, struct ether_addr mac_addr) {
>> >          struct sock_filter filter[] = {
>> >              BPF_STMT(BPF_LD + BPF_W + BPF_LEN, 0),                                 /* A <- packet length */
>> >              BPF_JUMP(BPF_JMP + BPF_JGE + BPF_K, sizeof(DHCPPacket), 1, 0),         /* packet >= DHCPPacket ? */
>> > @@ -56,7 +56,18 @@ int dhcp_network_bind_raw_socket(int index, union sockaddr_union *link,
>> >              BPF_STMT(BPF_LD + BPF_W + BPF_ABS, offsetof(DHCPPacket, dhcp.xid)),    /* A <- client identifier */
>> >              BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, xid, 1, 0),                        /* client identifier == xid ? */
>> >              BPF_STMT(BPF_RET + BPF_K, 0),                                          /* ignore */
>> > -            /* TODO: match chaddr */
>> > +            BPF_STMT(BPF_LD + BPF_IMM, htobe32(*((unsigned int *) &mac_addr))),                    /* A <- 4 bytes of client's MAC */
>> > +            BPF_STMT(BPF_MISC + BPF_TAX, 0),                                                       /* X <- A */
>> > +            BPF_STMT(BPF_LD + BPF_W + BPF_ABS, offsetof(DHCPPacket, dhcp.chaddr)),                 /* A <- 4 bytes of MAC from dhcp.chaddr */
>> > +            BPF_STMT(BPF_ALU + BPF_XOR + BPF_X, 0),                                                /* A xor X */
>> > +            BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, 0, 1, 0),                                          /* A == 0 ? */
>> > +            BPF_STMT(BPF_RET + BPF_K, 0),                                                          /* ignore */
>> > +            BPF_STMT(BPF_LD + BPF_IMM, htobe16(*((unsigned short *) (((char *) &mac_addr) + 4)))), /* A <- remainder of client's MAC */
>> > +            BPF_STMT(BPF_MISC + BPF_TAX, 0),                                                       /* X <- A */
>> > +            BPF_STMT(BPF_LD + BPF_H + BPF_ABS, offsetof(DHCPPacket, dhcp.chaddr) + 4),             /* A <- remainder of MAC from dhcp.chaddr */
>> > +            BPF_STMT(BPF_ALU + BPF_XOR + BPF_X, 0),                                                /* A xor X */
>> > +            BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, 0, 1, 0),                                          /* A == 0 ? */
>> > +            BPF_STMT(BPF_RET + BPF_K, 0),                                                          /* ignore */
>> >              BPF_STMT(BPF_LD + BPF_W + BPF_ABS, offsetof(DHCPPacket, dhcp.magic)),  /* A <- DHCP magic cookie */
>> >              BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, DHCP_MAGIC_COOKIE, 1, 0),          /* cookie == DHCP magic cookie ? */
>> >              BPF_STMT(BPF_RET + BPF_K, 0),                                          /* ignore */
>> > diff --git a/src/libsystemd-network/sd-dhcp-client.c b/src/libsystemd-network/sd-dhcp-client.c
>> > index c1af6df..d44cad2 100644
>> > --- a/src/libsystemd-network/sd-dhcp-client.c
>> > +++ b/src/libsystemd-network/sd-dhcp-client.c
>> > @@ -719,7 +719,7 @@ static int client_start(sd_dhcp_client *client) {
>> >
>> >          client->xid = random_u32();
>> >
>> > -        r = dhcp_network_bind_raw_socket(client->index, &client->link, client->xid);
>> > +        r = dhcp_network_bind_raw_socket(client->index, &client->link, client->xid, client->client_id.mac_addr);
>> >          if (r < 0) {
>> >                  client_stop(client, r);
>> >                  return r;
>> > @@ -761,7 +761,7 @@ static int client_timeout_t2(sd_event_source *s, uint64_t usec, void *userdata)
>> >          client->state = DHCP_STATE_REBINDING;
>> >          client->attempt = 1;
>> >
>> > -        r = dhcp_network_bind_raw_socket(client->index, &client->link, client->xid);
>> > +        r = dhcp_network_bind_raw_socket(client->index, &client->link, client->xid, client->client_id.mac_addr);
>> >          if (r < 0) {
>> >                  client_stop(client, r);
>> >                  return 0;
>> > diff --git a/src/libsystemd-network/test-dhcp-client.c b/src/libsystemd-network/test-dhcp-client.c
>> > index 450b6d4..7cbe10d 100644
>> > --- a/src/libsystemd-network/test-dhcp-client.c
>> > +++ b/src/libsystemd-network/test-dhcp-client.c
>> > @@ -196,7 +196,7 @@ int dhcp_network_send_raw_socket(int s, const union sockaddr_union *link,
>> >          return 575;
>> >  }
>> >
>> > -int dhcp_network_bind_raw_socket(int index, union sockaddr_union *link, uint32_t id)
>> > +int dhcp_network_bind_raw_socket(int index, union sockaddr_union *link, uint32_t id, struct ether_addr mac)
>> >  {
>> >          if (socketpair(AF_UNIX, SOCK_STREAM, 0, test_fd) < 0)
>> >                  return -errno;
>>
>>


More information about the systemd-devel mailing list