Merge ~chacman/ubuntu/+source/systemd:fix-sd-dhcp6-client into ubuntu/+source/systemd:ubuntu/devel

Proposed by Ciprian Hacman
Status: Needs review
Proposed branch: ~chacman/ubuntu/+source/systemd:fix-sd-dhcp6-client
Merge into: ubuntu/+source/systemd:ubuntu/devel
Diff against target: 295 lines (+63/-42)
4 files modified
src/libsystemd-network/dhcp6-internal.h (+1/-1)
src/libsystemd-network/dhcp6-option.c (+32/-3)
src/libsystemd-network/sd-dhcp6-client.c (+4/-25)
src/libsystemd-network/test-dhcp6-client.c (+26/-13)
Reviewer Review Type Date Requested Status
Dan Streetman (community) Needs Fixing
Review via email: mp+410695@code.launchpad.net

Commit message

sd-dhcp6-client: ignore IAs whose IAID do not match client's IAID

To post a comment you must log in.
Revision history for this message
Dan Streetman (ddstreet) wrote :

the systemd package uses 'quilt' format packaging, which means you can't just directly patch source files, so this MR is not correct.

review: Needs Fixing

Unmerged commits

a13c026... by Ciprian Hacman <email address hidden>

sd-dhcp6-client: ignore IAs whose IAID do not match client's IAID

But do not refuse whole message.

012fa5a... by Lukas Märdian

248.3-1ubuntu8 (patches unapplied)

Imported using git-ubuntu import.

ca74d03... by Michael Hudson-Doyle

248.3-1ubuntu7 (patches unapplied)

Imported using git-ubuntu import.

10decfb... by Michael Hudson-Doyle

248.3-1ubuntu6 (patches unapplied)

Imported using git-ubuntu import.

3267482... by Michael Hudson-Doyle

248.3-1ubuntu5 (patches unapplied)

Imported using git-ubuntu import.

2be65c9... by Lukas Märdian

248.3-1ubuntu4 (patches unapplied)

Imported using git-ubuntu import.

19de481... by Marc Deslauriers

248.3-1ubuntu3 (patches unapplied)

Imported using git-ubuntu import.

b36f293... by Dan Streetman

248.3-1ubuntu2 (patches unapplied)

Imported using git-ubuntu import.

732ad88... by Balint Reczey

248.3-1ubuntu1 (patches unapplied)

Imported using git-ubuntu import.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/libsystemd-network/dhcp6-internal.h b/src/libsystemd-network/dhcp6-internal.h
2index 681c462..942901a 100644
3--- a/src/libsystemd-network/dhcp6-internal.h
4+++ b/src/libsystemd-network/dhcp6-internal.h
5@@ -105,7 +105,7 @@ int dhcp6_option_append_vendor_option(uint8_t **buf, size_t *buflen, OrderedHash
6 int dhcp6_option_parse(uint8_t **buf, size_t *buflen, uint16_t *optcode,
7 size_t *optlen, uint8_t **optvalue);
8 int dhcp6_option_parse_status(DHCP6Option *option, size_t len);
9-int dhcp6_option_parse_ia(DHCP6Option *iaoption, DHCP6IA *ia, uint16_t *ret_status_code);
10+int dhcp6_option_parse_ia(DHCP6Option *iaoption, be32_t iaid, DHCP6IA *ia, uint16_t *ret_status_code);
11 int dhcp6_option_parse_ip6addrs(uint8_t *optval, uint16_t optlen,
12 struct in6_addr **addrs, size_t count,
13 size_t *allocated);
14diff --git a/src/libsystemd-network/dhcp6-option.c b/src/libsystemd-network/dhcp6-option.c
15index 91162d6..1376ca5 100644
16--- a/src/libsystemd-network/dhcp6-option.c
17+++ b/src/libsystemd-network/dhcp6-option.c
18@@ -515,7 +515,12 @@ static int dhcp6_option_parse_pdprefix(DHCP6Option *option, DHCP6IA *ia, uint32_
19 return 0;
20 }
21
22-int dhcp6_option_parse_ia(DHCP6Option *iaoption, DHCP6IA *ia, uint16_t *ret_status_code) {
23+int dhcp6_option_parse_ia(
24+ DHCP6Option *iaoption,
25+ be32_t iaid,
26+ DHCP6IA *ia,
27+ uint16_t *ret_status_code) {
28+
29 uint32_t lt_t1, lt_t2, lt_valid = 0, lt_min = UINT32_MAX;
30 uint16_t iatype, optlen;
31 size_t iaaddr_offset;
32@@ -535,6 +540,14 @@ int dhcp6_option_parse_ia(DHCP6Option *iaoption, DHCP6IA *ia, uint16_t *ret_stat
33 if (len < DHCP6_OPTION_IA_NA_LEN)
34 return -ENOBUFS;
35
36+ /* According to RFC8415, IAs which do not match the client's IAID should be ignored,
37+ * but not necessary to ignore or refuse the whole message. */
38+ if (((const struct ia_na*) iaoption->data)->id != iaid)
39+ /* ENOANO indicates the option should be ignored. */
40+ return log_dhcp6_client_errno(client, SYNTHETIC_ERRNO(ENOANO),
41+ "Received an IA_NA option with a different IAID "
42+ "from the one chosen by the client, ignoring.");
43+
44 iaaddr_offset = DHCP6_OPTION_IA_NA_LEN;
45 memcpy(&ia->ia_na, iaoption->data, sizeof(ia->ia_na));
46
47@@ -553,6 +566,14 @@ int dhcp6_option_parse_ia(DHCP6Option *iaoption, DHCP6IA *ia, uint16_t *ret_stat
48 if (len < sizeof(ia->ia_pd))
49 return -ENOBUFS;
50
51+ /* According to RFC8415, IAs which do not match the client's IAID should be ignored,
52+ * but not necessary to ignore or refuse the whole message. */
53+ if (((const struct ia_pd*) iaoption->data)->id != iaid)
54+ /* ENOANO indicates the option should be ignored. */
55+ return log_dhcp6_client_errno(client, SYNTHETIC_ERRNO(ENOANO),
56+ "Received an IA_PD option with a different IAID "
57+ "from the one chosen by the client, ignoring.");
58+
59 iaaddr_offset = sizeof(ia->ia_pd);
60 memcpy(&ia->ia_pd, iaoption->data, sizeof(ia->ia_pd));
61
62@@ -570,13 +591,21 @@ int dhcp6_option_parse_ia(DHCP6Option *iaoption, DHCP6IA *ia, uint16_t *ret_stat
63 if (len < DHCP6_OPTION_IA_TA_LEN)
64 return -ENOBUFS;
65
66+ /* According to RFC8415, IAs which do not match the client's IAID should be ignored,
67+ * but not necessary to ignore or refuse the whole message. */
68+ if (((const struct ia_ta*) iaoption->data)->id != iaid)
69+ /* ENOANO indicates the option should be ignored. */
70+ return log_dhcp6_client_errno(client, SYNTHETIC_ERRNO(ENOANO),
71+ "Received an IA_TA option with a different IAID "
72+ "from the one chosen by the client, ignoring.");
73+
74 iaaddr_offset = DHCP6_OPTION_IA_TA_LEN;
75- memcpy(&ia->ia_ta.id, iaoption->data, sizeof(ia->ia_ta));
76+ memcpy(&ia->ia_ta, iaoption->data, sizeof(ia->ia_ta));
77
78 break;
79
80 default:
81- return -ENOMSG;
82+ return -EINVAL;
83 }
84
85 ia->type = iatype;
86diff --git a/src/libsystemd-network/sd-dhcp6-client.c b/src/libsystemd-network/sd-dhcp6-client.c
87index 410bfda..1b41795 100644
88--- a/src/libsystemd-network/sd-dhcp6-client.c
89+++ b/src/libsystemd-network/sd-dhcp6-client.c
90@@ -1093,7 +1093,6 @@ static int client_parse_message(
91 while (pos < len) {
92 DHCP6Option *option = (DHCP6Option *) &message->options[pos];
93 uint16_t optcode, optlen;
94- be32_t iaid_lease;
95 int status;
96 uint8_t *optval;
97
98@@ -1172,8 +1171,8 @@ static int client_parse_message(
99 break;
100 }
101
102- r = dhcp6_option_parse_ia(option, &lease->ia, &ia_na_status);
103- if (r < 0 && r != -ENOMSG)
104+ r = dhcp6_option_parse_ia(option, client->ia_pd.ia_na.id, &lease->ia, &ia_na_status);
105+ if (r < 0 && r != -ENOANO)
106 return r;
107
108 if (ia_na_status == DHCP6_STATUS_NO_ADDRS_AVAIL) {
109@@ -1181,16 +1180,6 @@ static int client_parse_message(
110 continue;
111 }
112
113- r = dhcp6_lease_get_iaid(lease, &iaid_lease);
114- if (r < 0)
115- return r;
116-
117- if (client->ia_na.ia_na.id != iaid_lease) {
118- log_dhcp6_client(client, "%s has wrong IAID for IA NA",
119- dhcp6_message_type_to_string(message->type));
120- return -EINVAL;
121- }
122-
123 if (lease->ia.addresses) {
124 lt_t1 = MIN(lt_t1, be32toh(lease->ia.ia_na.lifetime_t1));
125 lt_t2 = MIN(lt_t2, be32toh(lease->ia.ia_na.lifetime_t1));
126@@ -1205,8 +1194,8 @@ static int client_parse_message(
127 break;
128 }
129
130- r = dhcp6_option_parse_ia(option, &lease->pd, &ia_pd_status);
131- if (r < 0 && r != -ENOMSG)
132+ r = dhcp6_option_parse_ia(option, client->ia_pd.ia_pd.id, &lease->pd, &ia_pd_status);
133+ if (r < 0 && r != -ENOANO)
134 return r;
135
136 if (ia_pd_status == DHCP6_STATUS_NO_PREFIX_AVAIL) {
137@@ -1214,16 +1203,6 @@ static int client_parse_message(
138 continue;
139 }
140
141- r = dhcp6_lease_get_pd_iaid(lease, &iaid_lease);
142- if (r < 0)
143- return r;
144-
145- if (client->ia_pd.ia_pd.id != iaid_lease) {
146- log_dhcp6_client(client, "%s has wrong IAID for IA PD",
147- dhcp6_message_type_to_string(message->type));
148- return -EINVAL;
149- }
150-
151 if (lease->pd.addresses) {
152 lt_t1 = MIN(lt_t1, be32toh(lease->pd.ia_pd.lifetime_t1));
153 lt_t2 = MIN(lt_t2, be32toh(lease->pd.ia_pd.lifetime_t2));
154diff --git a/src/libsystemd-network/test-dhcp6-client.c b/src/libsystemd-network/test-dhcp6-client.c
155index 96dc02a..82330f2 100644
156--- a/src/libsystemd-network/test-dhcp6-client.c
157+++ b/src/libsystemd-network/test-dhcp6-client.c
158@@ -287,25 +287,31 @@ static int test_option_status(sd_event *e) {
159 };
160 DHCP6Option *option;
161 DHCP6IA ia, pd;
162+ be32_t iaid;
163 int r = 0;
164
165 log_debug("/* %s */", __func__);
166
167+ memcpy(&iaid, option1 + 4, sizeof(iaid));
168+
169 zero(ia);
170 option = (DHCP6Option *)option1;
171 assert_se(sizeof(option1) == sizeof(DHCP6Option) + be16toh(option->len));
172
173- r = dhcp6_option_parse_ia(option, &ia, NULL);
174+ r = dhcp6_option_parse_ia(option, 0, &ia, NULL);
175+ assert_se(r == -ENOANO);
176+
177+ r = dhcp6_option_parse_ia(option, iaid, &ia, NULL);
178 assert_se(r == 0);
179 assert_se(ia.addresses == NULL);
180
181 option->len = htobe16(17);
182- r = dhcp6_option_parse_ia(option, &ia, NULL);
183+ r = dhcp6_option_parse_ia(option, iaid, &ia, NULL);
184 assert_se(r == -ENOBUFS);
185 assert_se(ia.addresses == NULL);
186
187 option->len = htobe16(sizeof(DHCP6Option));
188- r = dhcp6_option_parse_ia(option, &ia, NULL);
189+ r = dhcp6_option_parse_ia(option, iaid, &ia, NULL);
190 assert_se(r == -ENOBUFS);
191 assert_se(ia.addresses == NULL);
192
193@@ -313,7 +319,7 @@ static int test_option_status(sd_event *e) {
194 option = (DHCP6Option *)option2;
195 assert_se(sizeof(option2) == sizeof(DHCP6Option) + be16toh(option->len));
196
197- r = dhcp6_option_parse_ia(option, &ia, NULL);
198+ r = dhcp6_option_parse_ia(option, iaid, &ia, NULL);
199 assert_se(r >= 0);
200 assert_se(ia.addresses == NULL);
201
202@@ -321,7 +327,7 @@ static int test_option_status(sd_event *e) {
203 option = (DHCP6Option *)option3;
204 assert_se(sizeof(option3) == sizeof(DHCP6Option) + be16toh(option->len));
205
206- r = dhcp6_option_parse_ia(option, &ia, NULL);
207+ r = dhcp6_option_parse_ia(option, iaid, &ia, NULL);
208 assert_se(r >= 0);
209 assert_se(ia.addresses != NULL);
210 dhcp6_lease_free_ia(&ia);
211@@ -330,7 +336,7 @@ static int test_option_status(sd_event *e) {
212 option = (DHCP6Option *)option4;
213 assert_se(sizeof(option4) == sizeof(DHCP6Option) + be16toh(option->len));
214
215- r = dhcp6_option_parse_ia(option, &pd, NULL);
216+ r = dhcp6_option_parse_ia(option, iaid, &pd, NULL);
217 assert_se(r >= 0);
218 assert_se(pd.addresses != NULL);
219 assert_se(memcmp(&pd.ia_pd.id, &option4[4], 4) == 0);
220@@ -342,7 +348,7 @@ static int test_option_status(sd_event *e) {
221 option = (DHCP6Option *)option5;
222 assert_se(sizeof(option5) == sizeof(DHCP6Option) + be16toh(option->len));
223
224- r = dhcp6_option_parse_ia(option, &pd, NULL);
225+ r = dhcp6_option_parse_ia(option, iaid, &pd, NULL);
226 assert_se(r >= 0);
227 assert_se(pd.addresses != NULL);
228 dhcp6_lease_free_ia(&pd);
229@@ -447,13 +453,14 @@ static int test_advertise_option(sd_event *e) {
230 opt_clientid = true;
231 break;
232
233- case SD_DHCP6_OPTION_IA_NA:
234+ case SD_DHCP6_OPTION_IA_NA: {
235+ be32_t iaid = htobe32(0x0ecfa37d);
236+
237 assert_se(optlen == 94);
238 assert_se(optval == &msg_advertise[26]);
239 assert_se(!memcmp(optval, &msg_advertise[26], optlen));
240
241- val = htobe32(0x0ecfa37d);
242- assert_se(!memcmp(optval, &val, sizeof(val)));
243+ assert_se(!memcmp(optval, &iaid, sizeof(val)));
244
245 val = htobe32(80);
246 assert_se(!memcmp(optval + 4, &val, sizeof(val)));
247@@ -461,10 +468,10 @@ static int test_advertise_option(sd_event *e) {
248 val = htobe32(120);
249 assert_se(!memcmp(optval + 8, &val, sizeof(val)));
250
251- assert_se(dhcp6_option_parse_ia(option, &lease->ia, NULL) >= 0);
252+ assert_se(dhcp6_option_parse_ia(option, iaid, &lease->ia, NULL) >= 0);
253
254 break;
255-
256+ }
257 case SD_DHCP6_OPTION_SERVERID:
258 assert_se(optlen == 14);
259 assert_se(optval == &msg_advertise[179]);
260@@ -598,6 +605,8 @@ static void test_client_solicit_cb(sd_dhcp6_client *client, int event,
261 static int test_client_send_reply(DHCP6Message *request) {
262 DHCP6Message reply;
263
264+ log_debug("/* %s */", __func__);
265+
266 reply.transaction_id = request->transaction_id;
267 reply.type = DHCP6_REPLY;
268
269@@ -658,7 +667,7 @@ static int test_client_verify_request(DHCP6Message *request, size_t len) {
270 assert_se(!memcmp(optval + 8, &val, sizeof(val)));
271
272 /* Then, this should refuse all addresses. */
273- assert_se(dhcp6_option_parse_ia(option, &lease->ia, NULL) >= 0);
274+ assert_se(dhcp6_option_parse_ia(option, test_iaid, &lease->ia, NULL) >= 0);
275
276 break;
277
278@@ -704,6 +713,8 @@ static int test_client_verify_request(DHCP6Message *request, size_t len) {
279 static int test_client_send_advertise(DHCP6Message *solicit) {
280 DHCP6Message advertise;
281
282+ log_debug("/* %s */", __func__);
283+
284 advertise.transaction_id = solicit->transaction_id;
285 advertise.type = DHCP6_ADVERTISE;
286
287@@ -899,6 +910,8 @@ int dhcp6_network_send_udp_socket(int s, struct in6_addr *server_address,
288 IN6ADDR_ALL_DHCP6_RELAY_AGENTS_AND_SERVERS_INIT;
289 DHCP6Message *message;
290
291+ log_debug("/* %s */", __func__);
292+
293 assert_se(s == test_dhcp_fd[0]);
294 assert_se(server_address);
295 assert_se(packet);

Subscribers

People subscribed via source and target branches