colin/NetworkManager
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

dhcp: fix parsing of the search list option

Browse Source 
The DHCP search list option (119) can use the "message compression"
algorithm specified in RFC 1035 section 4.1.4 to reduce the size of
the message in presence of subdomains that appear multiple times.

When using the compression a label starts with:

    +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
    | 1  1|                OFFSET                   |
    +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+

where the offset points to a previous domain.

Previously, the parsing code was taking the lower 6 bits of the first
byte, shifting them left 16 bits, and adding the next byte. Instead,
the shift should be of 8 bits.

The effect of this bug was that when the offset was greater than 255,
it was incorrectly parsed as a number larger than the message size,
and the parsing failed.

Note that while a single DHCP option can be at most 255 bytes, a DHCP
message can contain multiple instances of the same option. The
receiver must concatenate all the occurrences according to RFC 3396
and parse the resulting buffer.

Fixes: 6adade6f21 ('dhcp: add nettools dhcp4 client')
This commit is contained in:
Beniamino Galvani
2025-07-07 09:32:16 +02:00
parent 43f738473c
commit a9d7abbc50
2 changed files with 71 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/core/dhcp/nm-dhcp-utils.c
View File

@@ -1224,7 +1224,7 @@ lease_option_print_domain_name(const uint8_t *cache,
}
case 0xC0: /* back pointer */
{
size_t offset = (c & 0x3F) << 16;
size_t offset = (c & 0x3F) << 8;
/*
* The offset is given as two bytes (in big endian), where the

70
src/core/dhcp/tests/test-dhcp-utils.c
View File

@@ -239,6 +239,76 @@ test_parse_search_list(void)
g_assert_cmpint(g_strv_length(domains), ==, 1);
g_assert_cmpstr(domains[0], ==, "okay");
g_strfreev(domains);
/* Test that the message compression works when the offset uses both bytes */
data = (guint8[]) {
/* clang-format off */
/* offset 0 */
0x3e,
'a','a','a','a','a','a','a','a','a','a','a','a','a','a','a','a',
'a','a','a','a','a','a','a','a','a','a','a','a','a','a','a','a',
'a','a','a','a','a','a','a','a','a','a','a','a','a','a','a','a',
'a','a','a','a','a','a','a','a','a','a','a','a','a','a',
0x00,
/* offset 0x40 */
0x3e,
'b','b','b','b','b','b','b','b','b','b','b','b','b','b','b','b',
'b','b','b','b','b','b','b','b','b','b','b','b','b','b','b','b',
'b','b','b','b','b','b','b','b','b','b','b','b','b','b','b','b',
'b','b','b','b','b','b','b','b','b','b','b','b','b','b',
0x00,
/* offset 0x80 */
0x3e,
'c','c','c','c','c','c','c','c','c','c','c','c','c','c','c','c',
'c','c','c','c','c','c','c','c','c','c','c','c','c','c','c','c',
'c','c','c','c','c','c','c','c','c','c','c','c','c','c','c','c',
'c','c','c','c','c','c','c','c','c','c','c','c','c','c',
0x00,
/* offset 0xc0 */
0x3e,
'd','d','d','d','d','d','d','d','d','d','d','d','d','d','d','d',
'd','d','d','d','d','d','d','d','d','d','d','d','d','d','d','d',
'd','d','d','d','d','d','d','d','d','d','d','d','d','d','d','d',
'd','d','d','d','d','d','d','d','d','d','d','d','d','d',
0x00,
/* offset 0x100 */
0x3e,
'e','e','e','e','e','e','e','e','e','e','e','e','e','e','e','e',
'e','e','e','e','e','e','e','e','e','e','e','e','e','e','e','e',
'e','e','e','e','e','e','e','e','e','e','e','e','e','e','e','e',
'e','e','e','e','e','e','e','e','e','e','e','e','e','e',
0x00,
/* offset 0x140 */
0x06, 'f','o','o','b','a','r', 0x03, 'c', 'o', 'm', 0x00,
0x04, 't', 'e', 's', 't', 0xc1, 0x40, /* back pointer to offset 0x140*/
/* clang-format on */
};
domains = nm_dhcp_lease_data_parse_search_list(data,
0x153,
"eth0",
AF_INET,
NM_DHCP_OPTION_DHCP4_DOMAIN_SEARCH_LIST);
g_assert(domains);
g_assert_cmpint(g_strv_length(domains), ==, 7);
g_assert_cmpstr(domains[0],
==,
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
g_assert_cmpstr(domains[1],
==,
"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb");
g_assert_cmpstr(domains[2],
==,
"cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc");
g_assert_cmpstr(domains[3],
==,
"dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd");
g_assert_cmpstr(domains[4],
==,
"eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee");
g_assert_cmpstr(domains[5], ==, "foobar.com");
g_assert_cmpstr(domains[6], ==, "test.foobar.com");
g_strfreev(domains);
}
static void