On Wed, Dec 07, 2016 at 09:36:24PM +0100, Jeremie Courreges-Anglas wrote:
> Rafael Zalamena <rzalam...@gmail.com> writes:
> 
> > I'm implementing some features for dhcrelay and to make them fit I need
> > some clean ups in the dhcrelay(8) first. This diff changes most of the
> > input/output functions prototypes to take one parameter with all addresses
> > instead of passing multiple parameters.
> >
> > Basically this will make input functions gather more information (source/
> > destination MACs, source/destination IPs, source/destination ports) and
> > use it in the output instead of trying to figure out this information along
> > the way.
> >
> > With this we will be able to add IPv6 support and layer 2 relaying.
> 
> Nice. :)
> 
> [...]
> 
> > ok?
> 
> This conflicts with a diff that has been committed by patrick@, you'll
> need to refresh it.

I updated the diff with the latest commits from patrick@. Basically instead
of cleaning hto to trigger the memset(, 0xff,) on destination mac we just
update the pc dmac field (see assemble_hw_header()).

> 
> I didn't review it entirely, but please address the point below.

I changed the struct fields sss and dss to src and dst respectively.

ok?

Index: bpf.c
===================================================================
RCS file: /home/obsdcvs/src/usr.sbin/dhcrelay/bpf.c,v
retrieving revision 1.11
diff -u -p -r1.11 bpf.c
--- bpf.c       28 May 2016 07:00:18 -0000      1.11
+++ bpf.c       8 Dec 2016 09:10:23 -0000
@@ -258,24 +258,23 @@ if_register_receive(struct interface_inf
 
 ssize_t
 send_packet(struct interface_info *interface,
-    struct dhcp_packet *raw, size_t len, struct in_addr from,
-    struct sockaddr_in *to, struct hardware *hto)
+    struct dhcp_packet *raw, size_t len, struct packet_ctx *pc)
 {
        unsigned char buf[256];
        struct iovec iov[2];
        int result, bufp = 0;
 
        if (interface->hw_address.htype == HTYPE_IPSEC_TUNNEL) {
-               socklen_t slen = sizeof(*to);
+               socklen_t slen = pc->pc_dst.ss_len;
                result = sendto(server_fd, raw, len, 0,
-                   (struct sockaddr *)to, slen);
+                   (struct sockaddr *)&pc->pc_dst, slen);
                goto done;
        }
 
        /* Assemble the headers... */
-       assemble_hw_header(interface, buf, &bufp, hto);
-       assemble_udp_ip_header(interface, buf, &bufp, from.s_addr,
-           to->sin_addr.s_addr, to->sin_port, (unsigned char *)raw, len);
+       assemble_hw_header(interface, buf, &bufp, pc);
+       assemble_udp_ip_header(interface, buf, &bufp, pc,
+           (unsigned char *)raw, len);
 
        /* Fire it off */
        iov[0].iov_base = (char *)buf;
@@ -292,7 +291,7 @@ send_packet(struct interface_info *inter
 
 ssize_t
 receive_packet(struct interface_info *interface, unsigned char *buf,
-    size_t len, struct sockaddr_in *from, struct hardware *hfrom)
+    size_t len, struct packet_ctx *pc)
 {
        int length = 0, offset = 0;
        struct bpf_hdr hdr;
@@ -358,7 +357,7 @@ receive_packet(struct interface_info *in
 
                /* Decode the physical header... */
                offset = decode_hw_header(interface,
-                   interface->rbuf, interface->rbuf_offset, hfrom);
+                   interface->rbuf, interface->rbuf_offset, pc);
 
                /*
                 * If a physical layer checksum failed (dunno of any
@@ -374,7 +373,7 @@ receive_packet(struct interface_info *in
 
                /* Decode the IP and UDP headers... */
                offset = decode_udp_ip_header(interface, interface->rbuf,
-                   interface->rbuf_offset, from, hdr.bh_caplen);
+                   interface->rbuf_offset, pc, hdr.bh_caplen);
 
                /* If the IP or UDP checksum was bad, skip the packet... */
                if (offset < 0) {
Index: dhcpd.h
===================================================================
RCS file: /home/obsdcvs/src/usr.sbin/dhcrelay/dhcpd.h,v
retrieving revision 1.15
diff -u -p -r1.15 dhcpd.h
--- dhcpd.h     7 Dec 2016 13:19:18 -0000       1.15
+++ dhcpd.h     8 Dec 2016 09:10:23 -0000
@@ -42,15 +42,28 @@
 #define        SERVER_PORT     67
 #define        CLIENT_PORT     68
 
+/* Maximum size of client hardware address. */
+#define CHADDR_SIZE    16
+
+struct packet_ctx {
+       uint8_t                          pc_htype;
+       uint8_t                          pc_hlen;
+       uint8_t                          pc_smac[CHADDR_SIZE];
+       uint8_t                          pc_dmac[CHADDR_SIZE];
+
+       struct sockaddr_storage          pc_src;
+       struct sockaddr_storage          pc_dst;
+};
+
 struct iaddr {
        int len;
-       unsigned char iabuf[16];
+       unsigned char iabuf[CHADDR_SIZE];
 };
 
 struct hardware {
        u_int8_t htype;
        u_int8_t hlen;
-       u_int8_t haddr[16];
+       u_int8_t haddr[CHADDR_SIZE];
 };
 
 /* Possible states in which the client can be. */
@@ -112,15 +125,13 @@ int if_register_bpf(struct interface_inf
 void if_register_send(struct interface_info *);
 void if_register_receive(struct interface_info *);
 ssize_t send_packet(struct interface_info *,
-    struct dhcp_packet *, size_t, struct in_addr,
-    struct sockaddr_in *, struct hardware *);
+    struct dhcp_packet *, size_t, struct packet_ctx *);
 ssize_t receive_packet(struct interface_info *, unsigned char *, size_t,
-    struct sockaddr_in *, struct hardware *);
+    struct packet_ctx *);
 
 /* dispatch.c */
 extern void (*bootp_packet_handler)(struct interface_info *,
-    struct dhcp_packet *, int, unsigned int, struct iaddr,
-    struct hardware *);
+    struct dhcp_packet *, int, struct packet_ctx *);
 struct interface_info *get_interface(const char *,
     void (*)(struct protocol *));
 void dispatch(void);
@@ -129,14 +140,15 @@ void add_protocol(char *, int, void (*)(
 void remove_protocol(struct protocol *);
 
 /* packet.c */
+struct sockaddr_in *ss2sin(struct sockaddr_storage *);
 void assemble_hw_header(struct interface_info *, unsigned char *,
-    int *, struct hardware *);
+    int *, struct packet_ctx *);
 void assemble_udp_ip_header(struct interface_info *, unsigned char *,
-    int *, u_int32_t, u_int32_t, unsigned int, unsigned char *, int);
+    int *, struct packet_ctx *pc, unsigned char *, int);
 ssize_t decode_hw_header(struct interface_info *, unsigned char *,
-    int, struct hardware *);
+    int, struct packet_ctx *);
 ssize_t decode_udp_ip_header(struct interface_info *, unsigned char *,
-    int, struct sockaddr_in *, int);
+    int, struct packet_ctx *, int);
 
 /* dhcrelay.c */
 extern u_int16_t server_port;
Index: dhcrelay.c
===================================================================
RCS file: /home/obsdcvs/src/usr.sbin/dhcrelay/dhcrelay.c,v
retrieving revision 1.47
diff -u -p -r1.47 dhcrelay.c
--- dhcrelay.c  7 Dec 2016 20:03:22 -0000       1.47
+++ dhcrelay.c  8 Dec 2016 09:10:23 -0000
@@ -47,6 +47,8 @@
 
 #include <net/if.h>
 
+#include <netinet/if_ether.h>
+
 #include <errno.h>
 #include <fcntl.h>
 #include <netdb.h>
@@ -65,7 +67,7 @@
 void    usage(void);
 int     rdaemon(int);
 void    relay(struct interface_info *, struct dhcp_packet *, int,
-           unsigned int, struct iaddr, struct hardware *);
+           struct packet_ctx *);
 char   *print_hw_addr(int, int, unsigned char *);
 void    got_response(struct protocol *);
 int     get_rdomain(char *);
@@ -259,11 +261,10 @@ main(int argc, char *argv[])
 
 void
 relay(struct interface_info *ip, struct dhcp_packet *packet, int length,
-    unsigned int from_port, struct iaddr from, struct hardware *hfrom)
+    struct packet_ctx *pc)
 {
        struct server_list      *sp;
        struct sockaddr_in       to;
-       struct hardware          hto;
 
        if (packet->hlen > sizeof packet->chaddr) {
                note("Discarding packet with invalid hlen.");
@@ -282,6 +283,8 @@ relay(struct interface_info *ip, struct 
                }
                to.sin_family = AF_INET;
                to.sin_len = sizeof to;
+               memcpy(&ss2sin(&pc->pc_dst)->sin_addr, &to.sin_addr,
+                   sizeof(ss2sin(&pc->pc_dst)->sin_addr));
 
                /*
                 * Set up the hardware destination address.  If it's a reply
@@ -289,13 +292,13 @@ relay(struct interface_info *ip, struct 
                 * cast as well.
                 */
                if (!(packet->flags & htons(BOOTP_BROADCAST))) {
-                       hto.hlen = packet->hlen;
-                       if (hto.hlen > sizeof hto.haddr)
-                               hto.hlen = sizeof hto.haddr;
-                       memcpy(hto.haddr, packet->chaddr, hto.hlen);
-                       hto.htype = packet->htype;
+                       pc->pc_hlen = packet->hlen;
+                       if (pc->pc_hlen > CHADDR_SIZE)
+                               pc->pc_hlen = CHADDR_SIZE;
+                       memcpy(pc->pc_dmac, packet->chaddr, pc->pc_hlen);
+                       pc->pc_htype = packet->htype;
                } else {
-                       bzero(&hto, sizeof(hto));
+                       memset(pc->pc_dmac, 0xff, sizeof(pc->pc_dmac));
                }
 
                if ((length = relay_agentinfo(interfaces,
@@ -316,8 +319,7 @@ relay(struct interface_info *ip, struct 
                 */
                packet->giaddr.s_addr = 0x0;
 
-               if (send_packet(interfaces, packet, length,
-                   interfaces->primary_address, &to, &hto) != -1)
+               if (send_packet(interfaces, packet, length, pc) != -1)
                        debug("forwarded BOOTREPLY for %s to %s",
                            print_hw_addr(packet->htype, packet->hlen,
                            packet->chaddr), inet_ntoa(to.sin_addr));
@@ -346,7 +348,7 @@ relay(struct interface_info *ip, struct 
                packet->giaddr = ip->primary_address;
 
        if ((length = relay_agentinfo(ip, packet, length,
-           (struct in_addr *)from.iabuf, NULL)) == -1) {
+           &ss2sin(&pc->pc_src)->sin_addr, NULL)) == -1) {
                note("ignoring BOOTREQUEST with invalid "
                    "relay agent information");
                return;
@@ -434,8 +436,8 @@ bad:
 void
 got_response(struct protocol *l)
 {
+       struct packet_ctx pc;
        ssize_t result;
-       struct iaddr ifrom;
        union {
                /*
                 * Packet input buffer.  Must be as large as largest
@@ -468,13 +470,19 @@ got_response(struct protocol *l)
                return;
        }
 
-       if (bootp_packet_handler) {
-               ifrom.len = 4;
-               memcpy(ifrom.iabuf, &sp->to.sin_addr, ifrom.len);
+       memset(&pc, 0, sizeof(pc));
+       pc.pc_src.ss_family = AF_INET;
+       pc.pc_src.ss_len = sizeof(struct sockaddr_in);
+       memcpy(&ss2sin(&pc.pc_src)->sin_addr, &sp->to.sin_addr,
+           sizeof(ss2sin(&pc.pc_src)->sin_addr));
+       ss2sin(&pc.pc_src)->sin_port = server_port;
+
+       pc.pc_dst.ss_family = AF_INET;
+       pc.pc_dst.ss_len = sizeof(struct sockaddr_in);
+       ss2sin(&pc.pc_dst)->sin_port = client_port;
 
-               (*bootp_packet_handler)(NULL, &u.packet, result,
-                   sp->to.sin_port, ifrom, NULL);
-       }
+       if (bootp_packet_handler)
+               (*bootp_packet_handler)(NULL, &u.packet, result, &pc);
 }
 
 ssize_t
Index: dispatch.c
===================================================================
RCS file: /home/obsdcvs/src/usr.sbin/dhcrelay/dispatch.c,v
retrieving revision 1.12
diff -u -p -r1.12 dispatch.c
--- dispatch.c  7 Dec 2016 13:19:18 -0000       1.12
+++ dispatch.c  8 Dec 2016 09:10:23 -0000
@@ -49,6 +49,7 @@
 #include <net/if_types.h>
 
 #include <netinet/in.h>
+#include <netinet/if_ether.h>
 
 #include <errno.h>
 #include <ifaddrs.h>
@@ -69,8 +70,7 @@ static struct timeout *free_timeouts;
 static int interfaces_invalidated;
 
 void (*bootp_packet_handler)(struct interface_info *,
-    struct dhcp_packet *, int, unsigned int,
-    struct iaddr, struct hardware *);
+    struct dhcp_packet *, int, struct packet_ctx *);
 
 static int interface_status(struct interface_info *ifinfo);
 
@@ -261,9 +261,7 @@ another:
 void
 got_one(struct protocol *l)
 {
-       struct sockaddr_in from;
-       struct hardware hfrom;
-       struct iaddr ifrom;
+       struct packet_ctx pc;
        size_t result;
        union {
                /*
@@ -275,8 +273,9 @@ got_one(struct protocol *l)
        } u;
        struct interface_info *ip = l->local;
 
-       if ((result = receive_packet(ip, u.packbuf, sizeof(u), &from,
-           &hfrom)) == -1) {
+       memset(&pc, 0, sizeof(pc));
+
+       if ((result = receive_packet(ip, u.packbuf, sizeof(u), &pc)) == -1) {
                warning("receive_packet failed on %s: %s", ip->name,
                    strerror(errno));
                ip->errors++;
@@ -296,13 +295,8 @@ got_one(struct protocol *l)
        if (result == 0)
                return;
 
-       if (bootp_packet_handler) {
-               ifrom.len = 4;
-               memcpy(ifrom.iabuf, &from.sin_addr, ifrom.len);
-
-               (*bootp_packet_handler)(ip, &u.packet, result,
-                   from.sin_port, ifrom, &hfrom);
-       }
+       if (bootp_packet_handler)
+               (*bootp_packet_handler)(ip, &u.packet, result, &pc);
 }
 
 int
Index: errwarn.c
===================================================================
RCS file: /home/obsdcvs/src/usr.sbin/dhcrelay/errwarn.c,v
retrieving revision 1.6
diff -u -p -r1.6 errwarn.c
--- errwarn.c   7 Feb 2016 00:49:28 -0000       1.6
+++ errwarn.c   8 Dec 2016 09:10:23 -0000
@@ -46,6 +46,7 @@
 #include <net/if.h>
 
 #include <netinet/in.h>
+#include <netinet/if_ether.h>
 
 #include <errno.h>
 #include <stdarg.h>
Index: packet.c
===================================================================
RCS file: /home/obsdcvs/src/usr.sbin/dhcrelay/packet.c,v
retrieving revision 1.11
diff -u -p -r1.11 packet.c
--- packet.c    7 Feb 2016 00:49:28 -0000       1.11
+++ packet.c    8 Dec 2016 09:10:23 -0000
@@ -62,6 +62,12 @@
 u_int32_t      checksum(unsigned char *, unsigned, u_int32_t);
 u_int32_t      wrapsum(u_int32_t);
 
+struct sockaddr_in *
+ss2sin(struct sockaddr_storage *ss)
+{
+       return ((struct sockaddr_in *)ss);
+}
+
 u_int32_t
 checksum(unsigned char *buf, unsigned nbytes, u_int32_t sum)
 {
@@ -97,17 +103,13 @@ wrapsum(u_int32_t sum)
 
 void
 assemble_hw_header(struct interface_info *interface, unsigned char *buf,
-    int *bufix, struct hardware *to)
+    int *bufix, struct packet_ctx *pc)
 {
        struct ether_header eh;
 
-       if (to != NULL && to->hlen == 6) /* XXX */
-               memcpy(eh.ether_dhost, to->haddr, sizeof(eh.ether_dhost));
-       else
-               memset(eh.ether_dhost, 0xff, sizeof(eh.ether_dhost));
-
-       /* source address is filled in by the kernel */
-       memset(eh.ether_shost, 0x00, sizeof(eh.ether_shost));
+       /* Use the supplied address or let the kernel fill it. */
+       memcpy(eh.ether_shost, pc->pc_smac, ETHER_ADDR_LEN);
+       memcpy(eh.ether_dhost, pc->pc_dmac, ETHER_ADDR_LEN);
 
        eh.ether_type = htons(ETHERTYPE_IP);
 
@@ -117,8 +119,7 @@ assemble_hw_header(struct interface_info
 
 void
 assemble_udp_ip_header(struct interface_info *interface, unsigned char *buf,
-    int *bufix, u_int32_t from, u_int32_t to, unsigned int port,
-    unsigned char *data, int len)
+    int *bufix, struct packet_ctx *pc, unsigned char *data, int len)
 {
        struct ip ip;
        struct udphdr udp;
@@ -132,15 +133,15 @@ assemble_udp_ip_header(struct interface_
        ip.ip_ttl = 16;
        ip.ip_p = IPPROTO_UDP;
        ip.ip_sum = 0;
-       ip.ip_src.s_addr = from;
-       ip.ip_dst.s_addr = to;
+       ip.ip_src.s_addr = ss2sin(&pc->pc_src)->sin_addr.s_addr;
+       ip.ip_dst.s_addr = ss2sin(&pc->pc_dst)->sin_addr.s_addr;
 
        ip.ip_sum = wrapsum(checksum((unsigned char *)&ip, sizeof(ip), 0));
        memcpy(&buf[*bufix], &ip, sizeof(ip));
        *bufix += sizeof(ip);
 
-       udp.uh_sport = server_port;     /* XXX */
-       udp.uh_dport = port;                    /* XXX */
+       udp.uh_sport = ss2sin(&pc->pc_src)->sin_port;
+       udp.uh_dport = ss2sin(&pc->pc_dst)->sin_port;
        udp.uh_ulen = htons(sizeof(udp) + len);
        memset(&udp.uh_sum, 0, sizeof(udp.uh_sum));
 
@@ -155,9 +156,8 @@ assemble_udp_ip_header(struct interface_
 
 ssize_t
 decode_hw_header(struct interface_info *interface, unsigned char *buf,
-    int bufix, struct hardware *from)
+    int bufix, struct packet_ctx *pc)
 {
-       struct ether_header eh;
        size_t offset = 0;
 
        if (interface->hw_address.htype == HTYPE_IPSEC_TUNNEL) {
@@ -172,23 +172,24 @@ decode_hw_header(struct interface_info *
                if (ip->ip_p != IPPROTO_IPIP)
                        return (-1);
 
-               bzero(&eh, sizeof(eh));
+               memset(pc->pc_dmac, 0xff, ETHER_ADDR_LEN);
                offset = ENC_HDRLEN + ip_len;
-       } else {        
-               memcpy(&eh, buf + bufix, ETHER_HDR_LEN);
-               offset = sizeof(eh);
+       } else {
+               memcpy(pc->pc_dmac, buf + bufix, ETHER_ADDR_LEN);
+               memcpy(pc->pc_smac, buf + bufix + ETHER_ADDR_LEN,
+                   ETHER_ADDR_LEN);
+               offset = sizeof(struct ether_header);
        }
 
-       memcpy(from->haddr, eh.ether_shost, sizeof(eh.ether_shost));
-       from->htype = ARPHRD_ETHER;
-       from->hlen = sizeof(eh.ether_shost);
+       pc->pc_htype = ARPHRD_ETHER;
+       pc->pc_hlen = ETHER_ADDR_LEN;
 
        return (offset);
 }
 
 ssize_t
 decode_udp_ip_header(struct interface_info *interface, unsigned char *buf,
-    int bufix, struct sockaddr_in *from, int buflen)
+    int bufix, struct packet_ctx *pc, int buflen)
 {
        struct ip *ip;
        struct udphdr *udp;
@@ -224,7 +225,15 @@ decode_udp_ip_header(struct interface_in
                return (-1);
        }
 
-       memcpy(&from->sin_addr, &ip->ip_src, sizeof(from->sin_addr));
+       pc->pc_src.ss_len = sizeof(struct sockaddr_in);
+       pc->pc_src.ss_family = AF_INET;
+       memcpy(&ss2sin(&pc->pc_src)->sin_addr, &ip->ip_src,
+           sizeof(ss2sin(&pc->pc_src)->sin_addr));
+
+       pc->pc_dst.ss_len = sizeof(struct sockaddr_in);
+       pc->pc_dst.ss_family = AF_INET;
+       memcpy(&ss2sin(&pc->pc_dst)->sin_addr, &ip->ip_dst,
+           sizeof(ss2sin(&pc->pc_dst)->sin_addr));
 
 #ifdef DEBUG
        if (ntohs(ip->ip_len) != buflen)
@@ -291,7 +300,8 @@ decode_udp_ip_header(struct interface_in
                return (-1);
        }
 
-       memcpy(&from->sin_port, &udp->uh_sport, sizeof(udp->uh_sport));
+       ss2sin(&pc->pc_src)->sin_port = udp->uh_sport;
+       ss2sin(&pc->pc_dst)->sin_port = udp->uh_dport;
 
        return (ip_len + sizeof(*udp));
 }

Reply via email to