Skip to content

Commit af52020

Browse files
ordexordex
committed
ovpn: reject unexpected netlink attributes
Netlink ops do not expect all attributes to be always set, however this condition is not explicitly coded any where, leading the user to believe that all sent attributes are somewhat processed. Fix this behaviour by introducing explicit checks. For CMD_OVPN_PEER_GET and CMD_OVPN_KEY_GET directly open-code the needed condition in the related ops handlers. While for all other ops use attribute subsets in the ovpn.yaml spec file. Fixes: b7a6339 ("ovpn: add basic netlink support") Reported-by: Ralf Lici <[email protected]> Closes: OpenVPN/ovpn-net-next#19 Signed-off-by: Antonio Quartulli <[email protected]>
1 parent 4c88cfc commit af52020

File tree

4 files changed

+251
-20
lines changed

4 files changed

+251
-20
lines changed

‎Documentation/netlink/specs/ovpn.yaml

Lines changed: 147 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,66 @@ attribute-sets:
160160
name: link-tx-packets
161161
type: uint
162162
doc: Number of packets transmitted at the transport level
163+
-
164+
name: peer-new-input
165+
subset-of: peer
166+
attributes:
167+
-
168+
name: id
169+
-
170+
name: remote-ipv4
171+
-
172+
name: remote-ipv6
173+
-
174+
name: remote-ipv6-scope-id
175+
-
176+
name: remote-port
177+
-
178+
name: socket
179+
-
180+
name: vpn-ipv4
181+
-
182+
name: vpn-ipv6
183+
-
184+
name: local-ipv4
185+
-
186+
name: local-ipv6
187+
-
188+
name: keepalive-interval
189+
-
190+
name: keepalive-timeout
191+
-
192+
name: peer-set-input
193+
subset-of: peer
194+
attributes:
195+
-
196+
name: id
197+
-
198+
name: remote-ipv4
199+
-
200+
name: remote-ipv6
201+
-
202+
name: remote-ipv6-scope-id
203+
-
204+
name: remote-port
205+
-
206+
name: vpn-ipv4
207+
-
208+
name: vpn-ipv6
209+
-
210+
name: local-ipv4
211+
-
212+
name: local-ipv6
213+
-
214+
name: keepalive-interval
215+
-
216+
name: keepalive-timeout
217+
-
218+
name: peer-del-input
219+
subset-of: peer
220+
attributes:
221+
-
222+
name: id
163223
-
164224
name: keyconf
165225
attributes:
@@ -216,6 +276,33 @@ attribute-sets:
216276
obtain the actual cipher IV
217277
checks:
218278
exact-len: nonce-tail-size
279+
280+
-
281+
name: keyconf-get
282+
subset-of: keyconf
283+
attributes:
284+
-
285+
name: peer-id
286+
-
287+
name: slot
288+
-
289+
name: key-id
290+
-
291+
name: cipher-alg
292+
-
293+
name: keyconf-swap-input
294+
subset-of: keyconf
295+
attributes:
296+
-
297+
name: peer-id
298+
-
299+
name: keyconf-del-input
300+
subset-of: keyconf
301+
attributes:
302+
-
303+
name: peer-id
304+
-
305+
name: slot
219306
-
220307
name: ovpn
221308
attributes:
@@ -235,12 +322,66 @@ attribute-sets:
235322
type: nest
236323
doc: Peer specific cipher configuration
237324
nested-attributes: keyconf
325+
-
326+
name: ovpn-peer-new-input
327+
subset-of: ovpn
328+
attributes:
329+
-
330+
name: ifindex
331+
-
332+
name: peer
333+
nested-attributes: peer-new-input
334+
-
335+
name: ovpn-peer-set-input
336+
subset-of: ovpn
337+
attributes:
338+
-
339+
name: ifindex
340+
-
341+
name: peer
342+
nested-attributes: peer-set-input
343+
-
344+
name: ovpn-peer-del-input
345+
subset-of: ovpn
346+
attributes:
347+
-
348+
name: ifindex
349+
-
350+
name: peer
351+
nested-attributes: peer-del-input
352+
-
353+
name: ovpn-keyconf-get
354+
subset-of: ovpn
355+
attributes:
356+
-
357+
name: ifindex
358+
-
359+
name: keyconf
360+
nested-attributes: keyconf-get
361+
-
362+
name: ovpn-keyconf-swap-input
363+
subset-of: ovpn
364+
attributes:
365+
-
366+
name: ifindex
367+
-
368+
name: keyconf
369+
nested-attributes: keyconf-swap-input
370+
-
371+
name: ovpn-keyconf-del-input
372+
subset-of: ovpn
373+
attributes:
374+
-
375+
name: ifindex
376+
-
377+
name: keyconf
378+
nested-attributes: keyconf-del-input
238379

239380
operations:
240381
list:
241382
-
242383
name: peer-new
243-
attribute-set: ovpn
384+
attribute-set: ovpn-peer-new-input
244385
flags: [ admin-perm ]
245386
doc: Add a remote peer
246387
do:
@@ -252,7 +393,7 @@ operations:
252393
- peer
253394
-
254395
name: peer-set
255-
attribute-set: ovpn
396+
attribute-set: ovpn-peer-set-input
256397
flags: [ admin-perm ]
257398
doc: modify a remote peer
258399
do:
@@ -286,7 +427,7 @@ operations:
286427
- peer
287428
-
288429
name: peer-del
289-
attribute-set: ovpn
430+
attribute-set: ovpn-peer-del-input
290431
flags: [ admin-perm ]
291432
doc: Delete existing remote peer
292433
do:
@@ -316,7 +457,7 @@ operations:
316457
- keyconf
317458
-
318459
name: key-get
319-
attribute-set: ovpn
460+
attribute-set: ovpn-keyconf-get
320461
flags: [ admin-perm ]
321462
doc: Retrieve non-sensitive data about peer key and cipher
322463
do:
@@ -331,7 +472,7 @@ operations:
331472
- keyconf
332473
-
333474
name: key-swap
334-
attribute-set: ovpn
475+
attribute-set: ovpn-keyconf-swap-input
335476
flags: [ admin-perm ]
336477
doc: Swap primary and secondary session keys for a specific peer
337478
do:
@@ -350,7 +491,7 @@ operations:
350491
mcgrp: peers
351492
-
352493
name: key-del
353-
attribute-set: ovpn
494+
attribute-set: ovpn-keyconf-del-input
354495
flags: [ admin-perm ]
355496
doc: Delete cipher key for a specific peer
356497
do:

‎drivers/net/ovpn/netlink-gen.c

Lines changed: 55 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,22 @@ const struct nla_policy ovpn_keyconf_nl_policy[OVPN_A_KEYCONF_DECRYPT_DIR + 1] =
2929
[OVPN_A_KEYCONF_DECRYPT_DIR] = NLA_POLICY_NESTED(ovpn_keydir_nl_policy),
3030
};
3131

32+
const struct nla_policy ovpn_keyconf_del_input_nl_policy[OVPN_A_KEYCONF_SLOT + 1] = {
33+
[OVPN_A_KEYCONF_PEER_ID] = NLA_POLICY_FULL_RANGE(NLA_U32, &ovpn_a_keyconf_peer_id_range),
34+
[OVPN_A_KEYCONF_SLOT] = NLA_POLICY_MAX(NLA_U32, 1),
35+
};
36+
37+
const struct nla_policy ovpn_keyconf_get_nl_policy[OVPN_A_KEYCONF_CIPHER_ALG + 1] = {
38+
[OVPN_A_KEYCONF_PEER_ID] = NLA_POLICY_FULL_RANGE(NLA_U32, &ovpn_a_keyconf_peer_id_range),
39+
[OVPN_A_KEYCONF_SLOT] = NLA_POLICY_MAX(NLA_U32, 1),
40+
[OVPN_A_KEYCONF_KEY_ID] = NLA_POLICY_MAX(NLA_U32, 7),
41+
[OVPN_A_KEYCONF_CIPHER_ALG] = NLA_POLICY_MAX(NLA_U32, 2),
42+
};
43+
44+
const struct nla_policy ovpn_keyconf_swap_input_nl_policy[OVPN_A_KEYCONF_PEER_ID + 1] = {
45+
[OVPN_A_KEYCONF_PEER_ID] = NLA_POLICY_FULL_RANGE(NLA_U32, &ovpn_a_keyconf_peer_id_range),
46+
};
47+
3248
const struct nla_policy ovpn_keydir_nl_policy[OVPN_A_KEYDIR_NONCE_TAIL + 1] = {
3349
[OVPN_A_KEYDIR_CIPHER_KEY] = NLA_POLICY_MAX_LEN(256),
3450
[OVPN_A_KEYDIR_NONCE_TAIL] = NLA_POLICY_EXACT_LEN(OVPN_NONCE_TAIL_SIZE),
@@ -60,16 +76,49 @@ const struct nla_policy ovpn_peer_nl_policy[OVPN_A_PEER_LINK_TX_PACKETS + 1] = {
6076
[OVPN_A_PEER_LINK_TX_PACKETS] = { .type = NLA_UINT, },
6177
};
6278

79+
const struct nla_policy ovpn_peer_del_input_nl_policy[OVPN_A_PEER_ID + 1] = {
80+
[OVPN_A_PEER_ID] = NLA_POLICY_FULL_RANGE(NLA_U32, &ovpn_a_peer_id_range),
81+
};
82+
83+
const struct nla_policy ovpn_peer_new_input_nl_policy[OVPN_A_PEER_KEEPALIVE_TIMEOUT + 1] = {
84+
[OVPN_A_PEER_ID] = NLA_POLICY_FULL_RANGE(NLA_U32, &ovpn_a_peer_id_range),
85+
[OVPN_A_PEER_REMOTE_IPV4] = { .type = NLA_BE32, },
86+
[OVPN_A_PEER_REMOTE_IPV6] = NLA_POLICY_EXACT_LEN(16),
87+
[OVPN_A_PEER_REMOTE_IPV6_SCOPE_ID] = { .type = NLA_U32, },
88+
[OVPN_A_PEER_REMOTE_PORT] = NLA_POLICY_MIN(NLA_BE16, 1),
89+
[OVPN_A_PEER_SOCKET] = { .type = NLA_U32, },
90+
[OVPN_A_PEER_VPN_IPV4] = { .type = NLA_BE32, },
91+
[OVPN_A_PEER_VPN_IPV6] = NLA_POLICY_EXACT_LEN(16),
92+
[OVPN_A_PEER_LOCAL_IPV4] = { .type = NLA_BE32, },
93+
[OVPN_A_PEER_LOCAL_IPV6] = NLA_POLICY_EXACT_LEN(16),
94+
[OVPN_A_PEER_KEEPALIVE_INTERVAL] = { .type = NLA_U32, },
95+
[OVPN_A_PEER_KEEPALIVE_TIMEOUT] = { .type = NLA_U32, },
96+
};
97+
98+
const struct nla_policy ovpn_peer_set_input_nl_policy[OVPN_A_PEER_KEEPALIVE_TIMEOUT + 1] = {
99+
[OVPN_A_PEER_ID] = NLA_POLICY_FULL_RANGE(NLA_U32, &ovpn_a_peer_id_range),
100+
[OVPN_A_PEER_REMOTE_IPV4] = { .type = NLA_BE32, },
101+
[OVPN_A_PEER_REMOTE_IPV6] = NLA_POLICY_EXACT_LEN(16),
102+
[OVPN_A_PEER_REMOTE_IPV6_SCOPE_ID] = { .type = NLA_U32, },
103+
[OVPN_A_PEER_REMOTE_PORT] = NLA_POLICY_MIN(NLA_BE16, 1),
104+
[OVPN_A_PEER_VPN_IPV4] = { .type = NLA_BE32, },
105+
[OVPN_A_PEER_VPN_IPV6] = NLA_POLICY_EXACT_LEN(16),
106+
[OVPN_A_PEER_LOCAL_IPV4] = { .type = NLA_BE32, },
107+
[OVPN_A_PEER_LOCAL_IPV6] = NLA_POLICY_EXACT_LEN(16),
108+
[OVPN_A_PEER_KEEPALIVE_INTERVAL] = { .type = NLA_U32, },
109+
[OVPN_A_PEER_KEEPALIVE_TIMEOUT] = { .type = NLA_U32, },
110+
};
111+
63112
/* OVPN_CMD_PEER_NEW - do */
64113
static const struct nla_policy ovpn_peer_new_nl_policy[OVPN_A_PEER + 1] = {
65114
[OVPN_A_IFINDEX] = { .type = NLA_U32, },
66-
[OVPN_A_PEER] = NLA_POLICY_NESTED(ovpn_peer_nl_policy),
115+
[OVPN_A_PEER] = NLA_POLICY_NESTED(ovpn_peer_new_input_nl_policy),
67116
};
68117

69118
/* OVPN_CMD_PEER_SET - do */
70119
static const struct nla_policy ovpn_peer_set_nl_policy[OVPN_A_PEER + 1] = {
71120
[OVPN_A_IFINDEX] = { .type = NLA_U32, },
72-
[OVPN_A_PEER] = NLA_POLICY_NESTED(ovpn_peer_nl_policy),
121+
[OVPN_A_PEER] = NLA_POLICY_NESTED(ovpn_peer_set_input_nl_policy),
73122
};
74123

75124
/* OVPN_CMD_PEER_GET - do */
@@ -86,7 +135,7 @@ static const struct nla_policy ovpn_peer_get_dump_nl_policy[OVPN_A_IFINDEX + 1]
86135
/* OVPN_CMD_PEER_DEL - do */
87136
static const struct nla_policy ovpn_peer_del_nl_policy[OVPN_A_PEER + 1] = {
88137
[OVPN_A_IFINDEX] = { .type = NLA_U32, },
89-
[OVPN_A_PEER] = NLA_POLICY_NESTED(ovpn_peer_nl_policy),
138+
[OVPN_A_PEER] = NLA_POLICY_NESTED(ovpn_peer_del_input_nl_policy),
90139
};
91140

92141
/* OVPN_CMD_KEY_NEW - do */
@@ -98,19 +147,19 @@ static const struct nla_policy ovpn_key_new_nl_policy[OVPN_A_KEYCONF + 1] = {
98147
/* OVPN_CMD_KEY_GET - do */
99148
static const struct nla_policy ovpn_key_get_nl_policy[OVPN_A_KEYCONF + 1] = {
100149
[OVPN_A_IFINDEX] = { .type = NLA_U32, },
101-
[OVPN_A_KEYCONF] = NLA_POLICY_NESTED(ovpn_keyconf_nl_policy),
150+
[OVPN_A_KEYCONF] = NLA_POLICY_NESTED(ovpn_keyconf_get_nl_policy),
102151
};
103152

104153
/* OVPN_CMD_KEY_SWAP - do */
105154
static const struct nla_policy ovpn_key_swap_nl_policy[OVPN_A_KEYCONF + 1] = {
106155
[OVPN_A_IFINDEX] = { .type = NLA_U32, },
107-
[OVPN_A_KEYCONF] = NLA_POLICY_NESTED(ovpn_keyconf_nl_policy),
156+
[OVPN_A_KEYCONF] = NLA_POLICY_NESTED(ovpn_keyconf_swap_input_nl_policy),
108157
};
109158

110159
/* OVPN_CMD_KEY_DEL - do */
111160
static const struct nla_policy ovpn_key_del_nl_policy[OVPN_A_KEYCONF + 1] = {
112161
[OVPN_A_IFINDEX] = { .type = NLA_U32, },
113-
[OVPN_A_KEYCONF] = NLA_POLICY_NESTED(ovpn_keyconf_nl_policy),
162+
[OVPN_A_KEYCONF] = NLA_POLICY_NESTED(ovpn_keyconf_del_input_nl_policy),
114163
};
115164

116165
/* Ops table for ovpn */

‎drivers/net/ovpn/netlink-gen.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,14 @@
1313

1414
/* Common nested types */
1515
extern const struct nla_policy ovpn_keyconf_nl_policy[OVPN_A_KEYCONF_DECRYPT_DIR + 1];
16+
extern const struct nla_policy ovpn_keyconf_del_input_nl_policy[OVPN_A_KEYCONF_SLOT + 1];
17+
extern const struct nla_policy ovpn_keyconf_get_nl_policy[OVPN_A_KEYCONF_CIPHER_ALG + 1];
18+
extern const struct nla_policy ovpn_keyconf_swap_input_nl_policy[OVPN_A_KEYCONF_PEER_ID + 1];
1619
extern const struct nla_policy ovpn_keydir_nl_policy[OVPN_A_KEYDIR_NONCE_TAIL + 1];
1720
extern const struct nla_policy ovpn_peer_nl_policy[OVPN_A_PEER_LINK_TX_PACKETS + 1];
21+
extern const struct nla_policy ovpn_peer_del_input_nl_policy[OVPN_A_PEER_ID + 1];
22+
extern const struct nla_policy ovpn_peer_new_input_nl_policy[OVPN_A_PEER_KEEPALIVE_TIMEOUT + 1];
23+
extern const struct nla_policy ovpn_peer_set_input_nl_policy[OVPN_A_PEER_KEEPALIVE_TIMEOUT + 1];
1824

1925
int ovpn_nl_pre_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
2026
struct genl_info *info);

0 commit comments

Comments
 (0)