* [PATCH 2/3] dhcp-server: Validate chaddr against source MAC
2021-08-06 23:52 [PATCH 1/3] dhcp-server: Fix double free in l_dhcp_server_expire_by_mac Andrew Zaborowski
@ 2021-08-06 23:52 ` Andrew Zaborowski
2021-08-06 23:52 ` [PATCH 3/3] unit: In test-dhcp update rx callback parameters Andrew Zaborowski
2021-08-13 1:27 ` [PATCH 1/3] dhcp-server: Fix double free in l_dhcp_server_expire_by_mac Denis Kenzior
2 siblings, 0 replies; 4+ messages in thread
From: Andrew Zaborowski @ 2021-08-06 23:52 UTC (permalink / raw)
To: ell
[-- Attachment #1: Type: text/plain, Size: 3538 bytes --]
In DHCP over ethernet -- the only thing we currently support -- the
chaddr in a DHCP message should always match the client's address, i.e.
the source address in messages from client to server. On wifi, where
the client's frame source can't be easily spoofed, validating chaddr
may add a little security. Some other DHCP servers support this too,
either on or off by default.
---
ell/dhcp-private.h | 3 ++-
ell/dhcp-server.c | 6 +++++-
ell/dhcp-transport.c | 16 +++++++++++++---
ell/dhcp.c | 3 ++-
4 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/ell/dhcp-private.h b/ell/dhcp-private.h
index 5a09f5e..bc10765 100644
--- a/ell/dhcp-private.h
+++ b/ell/dhcp-private.h
@@ -107,7 +107,8 @@ const char *_dhcp_option_to_string(uint8_t option);
uint16_t _dhcp_checksum(const void *buf, size_t len);
uint16_t _dhcp_checksumv(const struct iovec *iov, size_t iov_cnt);
-typedef void (*dhcp_transport_rx_cb_t)(const void *, size_t, void *);
+typedef void (*dhcp_transport_rx_cb_t)(const void *, size_t, void *,
+ const uint8_t *);
struct dhcp_transport {
int (*open)(struct dhcp_transport *s, uint32_t xid);
diff --git a/ell/dhcp-server.c b/ell/dhcp-server.c
index 84dc41d..05f9b73 100644
--- a/ell/dhcp-server.c
+++ b/ell/dhcp-server.c
@@ -705,7 +705,8 @@ static void send_ack(struct l_dhcp_server *server,
server->user_data, lease);
}
-static void listener_event(const void *data, size_t len, void *user_data)
+static void listener_event(const void *data, size_t len, void *user_data,
+ const uint8_t *saddr)
{
struct l_dhcp_server *server = user_data;
const struct dhcp_message *message = data;
@@ -722,6 +723,9 @@ static void listener_event(const void *data, size_t len, void *user_data)
SERVER_DEBUG("");
+ if (saddr && memcmp(saddr, message->chaddr, ETH_ALEN))
+ return;
+
if (!_dhcp_message_iter_init(&iter, message, len))
return;
diff --git a/ell/dhcp-transport.c b/ell/dhcp-transport.c
index 68108e2..7c73fee 100644
--- a/ell/dhcp-transport.c
+++ b/ell/dhcp-transport.c
@@ -111,8 +111,11 @@ static bool _dhcp_default_transport_read_handler(struct l_io *io,
ssize_t len;
struct dhcp_packet *p;
uint16_t c;
+ struct sockaddr_ll saddr;
+ socklen_t saddr_len = sizeof(saddr);
- len = read(fd, buf, sizeof(buf));
+ len = recvfrom(fd, buf, sizeof(buf), 0, (struct sockaddr *) &saddr,
+ &saddr_len);
if (len < 0)
return false;
@@ -148,8 +151,15 @@ static bool _dhcp_default_transport_read_handler(struct l_io *io,
len -= sizeof(struct udphdr) - sizeof(struct iphdr);
- if (transport->super.rx_cb)
- transport->super.rx_cb(&p->dhcp, len, transport->super.rx_data);
+ if (transport->super.rx_cb) {
+ const uint8_t *src_mac = NULL;
+
+ if (saddr_len >= sizeof(saddr) && saddr.sll_halen == ETH_ALEN)
+ src_mac = saddr.sll_addr;
+
+ transport->super.rx_cb(&p->dhcp, len, transport->super.rx_data,
+ src_mac);
+ }
return true;
}
diff --git a/ell/dhcp.c b/ell/dhcp.c
index 2efbf39..32e4a7c 100644
--- a/ell/dhcp.c
+++ b/ell/dhcp.c
@@ -775,7 +775,8 @@ static int dhcp_client_receive_offer(struct l_dhcp_client *client,
return 0;
}
-static void dhcp_client_rx_message(const void *data, size_t len, void *userdata)
+static void dhcp_client_rx_message(const void *data, size_t len, void *userdata,
+ const uint8_t *saddr)
{
struct l_dhcp_client *client = userdata;
const struct dhcp_message *message = data;
--
2.30.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 3/3] unit: In test-dhcp update rx callback parameters
2021-08-06 23:52 [PATCH 1/3] dhcp-server: Fix double free in l_dhcp_server_expire_by_mac Andrew Zaborowski
2021-08-06 23:52 ` [PATCH 2/3] dhcp-server: Validate chaddr against source MAC Andrew Zaborowski
@ 2021-08-06 23:52 ` Andrew Zaborowski
2021-08-13 1:27 ` [PATCH 1/3] dhcp-server: Fix double free in l_dhcp_server_expire_by_mac Denis Kenzior
2 siblings, 0 replies; 4+ messages in thread
From: Andrew Zaborowski @ 2021-08-06 23:52 UTC (permalink / raw)
To: ell
[-- Attachment #1: Type: text/plain, Size: 3591 bytes --]
---
unit/test-dhcp.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/unit/test-dhcp.c b/unit/test-dhcp.c
index 11ee10e..e502be7 100644
--- a/unit/test-dhcp.c
+++ b/unit/test-dhcp.c
@@ -721,7 +721,7 @@ static void test_discover(const void *data)
assert(dhcp_message_compare(discover_data_1, sizeof(discover_data_1),
client_packet, client_packet_len));
- transport->rx_cb(offer_data_1, sizeof(offer_data_1), client);
+ transport->rx_cb(offer_data_1, sizeof(offer_data_1), client, NULL);
assert(client_send_called);
client_send_called = false;
@@ -729,7 +729,7 @@ static void test_discover(const void *data)
client_packet, client_packet_len));
event_handler_called = false;
- transport->rx_cb(ack_data_1, sizeof(ack_data_1), client);
+ transport->rx_cb(ack_data_1, sizeof(ack_data_1), client, NULL);
assert(!client_send_called);
assert(event_handler_called);
@@ -829,30 +829,36 @@ static void client_connect(struct l_dhcp_client *client,
_dhcp_server_get_transport(server);
struct dhcp_transport *cli_transport =
_dhcp_client_get_transport(client);
+ uint8_t cli_addr[ETH_ALEN];
+ const struct dhcp_message *msg = (struct dhcp_message *) client_packet;
assert(l_dhcp_client_start(client));
assert(client_send_called);
client_send_called = false;
+ memcpy(cli_addr, msg->chaddr, ETH_ALEN);
/* RX DISCOVER */
- srv_transport->rx_cb(client_packet, client_packet_len, server);
+ srv_transport->rx_cb(client_packet, client_packet_len, server,
+ cli_addr);
assert(l2_send_called);
l2_send_called = false;
if (!rapid_commit) {
/* RX OFFER */
- cli_transport->rx_cb(server_packet, server_packet_len, client);
+ cli_transport->rx_cb(server_packet, server_packet_len, client,
+ NULL);
assert(client_send_called);
client_send_called = false;
/* RX REQUEST */
- srv_transport->rx_cb(client_packet, client_packet_len, server);
+ srv_transport->rx_cb(client_packet, client_packet_len, server,
+ cli_addr);
assert(l2_send_called);
l2_send_called = false;
}
/* RX ACK */
- cli_transport->rx_cb(server_packet, server_packet_len, client);
+ cli_transport->rx_cb(server_packet, server_packet_len, client, NULL);
assert(!client_send_called);
assert(event_handler_called);
@@ -966,7 +972,7 @@ static void test_complete_run(const void *data)
l_dhcp_client_stop(client1);
assert(client_send_called);
client_send_called = false;
- srv_transport->rx_cb(client_packet, client_packet_len, server);
+ srv_transport->rx_cb(client_packet, client_packet_len, server, addr1);
assert(expired_client);
assert(!strcmp(expired_client, "192.168.1.2"));
l_free(expired_client);
@@ -975,7 +981,7 @@ static void test_complete_run(const void *data)
l_dhcp_client_stop(client2);
assert(client_send_called);
client_send_called = false;
- srv_transport->rx_cb(client_packet, client_packet_len, server);
+ srv_transport->rx_cb(client_packet, client_packet_len, server, addr2);
assert(expired_client);
assert(!strcmp(expired_client, "192.168.1.3"));
l_free(expired_client);
@@ -1023,7 +1029,8 @@ static void test_expired_ip_reuse(const void *data)
l_dhcp_client_destroy(client);
assert(client_send_called);
client_send_called = false;
- srv_transport->rx_cb(client_packet, client_packet_len, server);
+ srv_transport->rx_cb(client_packet, client_packet_len, server,
+ addr);
l_free(expired_client);
expired_client = NULL;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 4+ messages in thread