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: b7a63391aa ("ovpn: add basic netlink support")
Reported-by: Ralf Lici <ralf@mandelbit.com>
Closes: https://github.com/OpenVPN/ovpn-net-next/issues/19
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
This commit is contained in:
Antonio Quartulli 2025-06-25 16:08:11 +02:00
parent 4c88cfcc67
commit af52020fc5
4 changed files with 251 additions and 20 deletions

View file

@ -160,6 +160,66 @@ attribute-sets:
name: link-tx-packets
type: uint
doc: Number of packets transmitted at the transport level
-
name: peer-new-input
subset-of: peer
attributes:
-
name: id
-
name: remote-ipv4
-
name: remote-ipv6
-
name: remote-ipv6-scope-id
-
name: remote-port
-
name: socket
-
name: vpn-ipv4
-
name: vpn-ipv6
-
name: local-ipv4
-
name: local-ipv6
-
name: keepalive-interval
-
name: keepalive-timeout
-
name: peer-set-input
subset-of: peer
attributes:
-
name: id
-
name: remote-ipv4
-
name: remote-ipv6
-
name: remote-ipv6-scope-id
-
name: remote-port
-
name: vpn-ipv4
-
name: vpn-ipv6
-
name: local-ipv4
-
name: local-ipv6
-
name: keepalive-interval
-
name: keepalive-timeout
-
name: peer-del-input
subset-of: peer
attributes:
-
name: id
-
name: keyconf
attributes:
@ -216,6 +276,33 @@ attribute-sets:
obtain the actual cipher IV
checks:
exact-len: nonce-tail-size
-
name: keyconf-get
subset-of: keyconf
attributes:
-
name: peer-id
-
name: slot
-
name: key-id
-
name: cipher-alg
-
name: keyconf-swap-input
subset-of: keyconf
attributes:
-
name: peer-id
-
name: keyconf-del-input
subset-of: keyconf
attributes:
-
name: peer-id
-
name: slot
-
name: ovpn
attributes:
@ -235,12 +322,66 @@ attribute-sets:
type: nest
doc: Peer specific cipher configuration
nested-attributes: keyconf
-
name: ovpn-peer-new-input
subset-of: ovpn
attributes:
-
name: ifindex
-
name: peer
nested-attributes: peer-new-input
-
name: ovpn-peer-set-input
subset-of: ovpn
attributes:
-
name: ifindex
-
name: peer
nested-attributes: peer-set-input
-
name: ovpn-peer-del-input
subset-of: ovpn
attributes:
-
name: ifindex
-
name: peer
nested-attributes: peer-del-input
-
name: ovpn-keyconf-get
subset-of: ovpn
attributes:
-
name: ifindex
-
name: keyconf
nested-attributes: keyconf-get
-
name: ovpn-keyconf-swap-input
subset-of: ovpn
attributes:
-
name: ifindex
-
name: keyconf
nested-attributes: keyconf-swap-input
-
name: ovpn-keyconf-del-input
subset-of: ovpn
attributes:
-
name: ifindex
-
name: keyconf
nested-attributes: keyconf-del-input
operations:
list:
-
name: peer-new
attribute-set: ovpn
attribute-set: ovpn-peer-new-input
flags: [ admin-perm ]
doc: Add a remote peer
do:
@ -252,7 +393,7 @@ operations:
- peer
-
name: peer-set
attribute-set: ovpn
attribute-set: ovpn-peer-set-input
flags: [ admin-perm ]
doc: modify a remote peer
do:
@ -286,7 +427,7 @@ operations:
- peer
-
name: peer-del
attribute-set: ovpn
attribute-set: ovpn-peer-del-input
flags: [ admin-perm ]
doc: Delete existing remote peer
do:
@ -316,7 +457,7 @@ operations:
- keyconf
-
name: key-get
attribute-set: ovpn
attribute-set: ovpn-keyconf-get
flags: [ admin-perm ]
doc: Retrieve non-sensitive data about peer key and cipher
do:
@ -331,7 +472,7 @@ operations:
- keyconf
-
name: key-swap
attribute-set: ovpn
attribute-set: ovpn-keyconf-swap-input
flags: [ admin-perm ]
doc: Swap primary and secondary session keys for a specific peer
do:
@ -350,7 +491,7 @@ operations:
mcgrp: peers
-
name: key-del
attribute-set: ovpn
attribute-set: ovpn-keyconf-del-input
flags: [ admin-perm ]
doc: Delete cipher key for a specific peer
do:

View file

@ -29,6 +29,22 @@ const struct nla_policy ovpn_keyconf_nl_policy[OVPN_A_KEYCONF_DECRYPT_DIR + 1] =
[OVPN_A_KEYCONF_DECRYPT_DIR] = NLA_POLICY_NESTED(ovpn_keydir_nl_policy),
};
const struct nla_policy ovpn_keyconf_del_input_nl_policy[OVPN_A_KEYCONF_SLOT + 1] = {
[OVPN_A_KEYCONF_PEER_ID] = NLA_POLICY_FULL_RANGE(NLA_U32, &ovpn_a_keyconf_peer_id_range),
[OVPN_A_KEYCONF_SLOT] = NLA_POLICY_MAX(NLA_U32, 1),
};
const struct nla_policy ovpn_keyconf_get_nl_policy[OVPN_A_KEYCONF_CIPHER_ALG + 1] = {
[OVPN_A_KEYCONF_PEER_ID] = NLA_POLICY_FULL_RANGE(NLA_U32, &ovpn_a_keyconf_peer_id_range),
[OVPN_A_KEYCONF_SLOT] = NLA_POLICY_MAX(NLA_U32, 1),
[OVPN_A_KEYCONF_KEY_ID] = NLA_POLICY_MAX(NLA_U32, 7),
[OVPN_A_KEYCONF_CIPHER_ALG] = NLA_POLICY_MAX(NLA_U32, 2),
};
const struct nla_policy ovpn_keyconf_swap_input_nl_policy[OVPN_A_KEYCONF_PEER_ID + 1] = {
[OVPN_A_KEYCONF_PEER_ID] = NLA_POLICY_FULL_RANGE(NLA_U32, &ovpn_a_keyconf_peer_id_range),
};
const struct nla_policy ovpn_keydir_nl_policy[OVPN_A_KEYDIR_NONCE_TAIL + 1] = {
[OVPN_A_KEYDIR_CIPHER_KEY] = NLA_POLICY_MAX_LEN(256),
[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] = {
[OVPN_A_PEER_LINK_TX_PACKETS] = { .type = NLA_UINT, },
};
const struct nla_policy ovpn_peer_del_input_nl_policy[OVPN_A_PEER_ID + 1] = {
[OVPN_A_PEER_ID] = NLA_POLICY_FULL_RANGE(NLA_U32, &ovpn_a_peer_id_range),
};
const struct nla_policy ovpn_peer_new_input_nl_policy[OVPN_A_PEER_KEEPALIVE_TIMEOUT + 1] = {
[OVPN_A_PEER_ID] = NLA_POLICY_FULL_RANGE(NLA_U32, &ovpn_a_peer_id_range),
[OVPN_A_PEER_REMOTE_IPV4] = { .type = NLA_BE32, },
[OVPN_A_PEER_REMOTE_IPV6] = NLA_POLICY_EXACT_LEN(16),
[OVPN_A_PEER_REMOTE_IPV6_SCOPE_ID] = { .type = NLA_U32, },
[OVPN_A_PEER_REMOTE_PORT] = NLA_POLICY_MIN(NLA_BE16, 1),
[OVPN_A_PEER_SOCKET] = { .type = NLA_U32, },
[OVPN_A_PEER_VPN_IPV4] = { .type = NLA_BE32, },
[OVPN_A_PEER_VPN_IPV6] = NLA_POLICY_EXACT_LEN(16),
[OVPN_A_PEER_LOCAL_IPV4] = { .type = NLA_BE32, },
[OVPN_A_PEER_LOCAL_IPV6] = NLA_POLICY_EXACT_LEN(16),
[OVPN_A_PEER_KEEPALIVE_INTERVAL] = { .type = NLA_U32, },
[OVPN_A_PEER_KEEPALIVE_TIMEOUT] = { .type = NLA_U32, },
};
const struct nla_policy ovpn_peer_set_input_nl_policy[OVPN_A_PEER_KEEPALIVE_TIMEOUT + 1] = {
[OVPN_A_PEER_ID] = NLA_POLICY_FULL_RANGE(NLA_U32, &ovpn_a_peer_id_range),
[OVPN_A_PEER_REMOTE_IPV4] = { .type = NLA_BE32, },
[OVPN_A_PEER_REMOTE_IPV6] = NLA_POLICY_EXACT_LEN(16),
[OVPN_A_PEER_REMOTE_IPV6_SCOPE_ID] = { .type = NLA_U32, },
[OVPN_A_PEER_REMOTE_PORT] = NLA_POLICY_MIN(NLA_BE16, 1),
[OVPN_A_PEER_VPN_IPV4] = { .type = NLA_BE32, },
[OVPN_A_PEER_VPN_IPV6] = NLA_POLICY_EXACT_LEN(16),
[OVPN_A_PEER_LOCAL_IPV4] = { .type = NLA_BE32, },
[OVPN_A_PEER_LOCAL_IPV6] = NLA_POLICY_EXACT_LEN(16),
[OVPN_A_PEER_KEEPALIVE_INTERVAL] = { .type = NLA_U32, },
[OVPN_A_PEER_KEEPALIVE_TIMEOUT] = { .type = NLA_U32, },
};
/* OVPN_CMD_PEER_NEW - do */
static const struct nla_policy ovpn_peer_new_nl_policy[OVPN_A_PEER + 1] = {
[OVPN_A_IFINDEX] = { .type = NLA_U32, },
[OVPN_A_PEER] = NLA_POLICY_NESTED(ovpn_peer_nl_policy),
[OVPN_A_PEER] = NLA_POLICY_NESTED(ovpn_peer_new_input_nl_policy),
};
/* OVPN_CMD_PEER_SET - do */
static const struct nla_policy ovpn_peer_set_nl_policy[OVPN_A_PEER + 1] = {
[OVPN_A_IFINDEX] = { .type = NLA_U32, },
[OVPN_A_PEER] = NLA_POLICY_NESTED(ovpn_peer_nl_policy),
[OVPN_A_PEER] = NLA_POLICY_NESTED(ovpn_peer_set_input_nl_policy),
};
/* OVPN_CMD_PEER_GET - do */
@ -86,7 +135,7 @@ static const struct nla_policy ovpn_peer_get_dump_nl_policy[OVPN_A_IFINDEX + 1]
/* OVPN_CMD_PEER_DEL - do */
static const struct nla_policy ovpn_peer_del_nl_policy[OVPN_A_PEER + 1] = {
[OVPN_A_IFINDEX] = { .type = NLA_U32, },
[OVPN_A_PEER] = NLA_POLICY_NESTED(ovpn_peer_nl_policy),
[OVPN_A_PEER] = NLA_POLICY_NESTED(ovpn_peer_del_input_nl_policy),
};
/* OVPN_CMD_KEY_NEW - do */
@ -98,19 +147,19 @@ static const struct nla_policy ovpn_key_new_nl_policy[OVPN_A_KEYCONF + 1] = {
/* OVPN_CMD_KEY_GET - do */
static const struct nla_policy ovpn_key_get_nl_policy[OVPN_A_KEYCONF + 1] = {
[OVPN_A_IFINDEX] = { .type = NLA_U32, },
[OVPN_A_KEYCONF] = NLA_POLICY_NESTED(ovpn_keyconf_nl_policy),
[OVPN_A_KEYCONF] = NLA_POLICY_NESTED(ovpn_keyconf_get_nl_policy),
};
/* OVPN_CMD_KEY_SWAP - do */
static const struct nla_policy ovpn_key_swap_nl_policy[OVPN_A_KEYCONF + 1] = {
[OVPN_A_IFINDEX] = { .type = NLA_U32, },
[OVPN_A_KEYCONF] = NLA_POLICY_NESTED(ovpn_keyconf_nl_policy),
[OVPN_A_KEYCONF] = NLA_POLICY_NESTED(ovpn_keyconf_swap_input_nl_policy),
};
/* OVPN_CMD_KEY_DEL - do */
static const struct nla_policy ovpn_key_del_nl_policy[OVPN_A_KEYCONF + 1] = {
[OVPN_A_IFINDEX] = { .type = NLA_U32, },
[OVPN_A_KEYCONF] = NLA_POLICY_NESTED(ovpn_keyconf_nl_policy),
[OVPN_A_KEYCONF] = NLA_POLICY_NESTED(ovpn_keyconf_del_input_nl_policy),
};
/* Ops table for ovpn */

View file

@ -13,8 +13,14 @@
/* Common nested types */
extern const struct nla_policy ovpn_keyconf_nl_policy[OVPN_A_KEYCONF_DECRYPT_DIR + 1];
extern const struct nla_policy ovpn_keyconf_del_input_nl_policy[OVPN_A_KEYCONF_SLOT + 1];
extern const struct nla_policy ovpn_keyconf_get_nl_policy[OVPN_A_KEYCONF_CIPHER_ALG + 1];
extern const struct nla_policy ovpn_keyconf_swap_input_nl_policy[OVPN_A_KEYCONF_PEER_ID + 1];
extern const struct nla_policy ovpn_keydir_nl_policy[OVPN_A_KEYDIR_NONCE_TAIL + 1];
extern const struct nla_policy ovpn_peer_nl_policy[OVPN_A_PEER_LINK_TX_PACKETS + 1];
extern const struct nla_policy ovpn_peer_del_input_nl_policy[OVPN_A_PEER_ID + 1];
extern const struct nla_policy ovpn_peer_new_input_nl_policy[OVPN_A_PEER_KEEPALIVE_TIMEOUT + 1];
extern const struct nla_policy ovpn_peer_set_input_nl_policy[OVPN_A_PEER_KEEPALIVE_TIMEOUT + 1];
int ovpn_nl_pre_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
struct genl_info *info);

View file

@ -352,7 +352,7 @@ int ovpn_nl_peer_new_doit(struct sk_buff *skb, struct genl_info *info)
return -EINVAL;
ret = nla_parse_nested(attrs, OVPN_A_PEER_MAX, info->attrs[OVPN_A_PEER],
ovpn_peer_nl_policy, info->extack);
ovpn_peer_new_input_nl_policy, info->extack);
if (ret)
return ret;
@ -476,7 +476,7 @@ int ovpn_nl_peer_set_doit(struct sk_buff *skb, struct genl_info *info)
return -EINVAL;
ret = nla_parse_nested(attrs, OVPN_A_PEER_MAX, info->attrs[OVPN_A_PEER],
ovpn_peer_nl_policy, info->extack);
ovpn_peer_set_input_nl_policy, info->extack);
if (ret)
return ret;
@ -654,7 +654,7 @@ int ovpn_nl_peer_get_doit(struct sk_buff *skb, struct genl_info *info)
struct ovpn_peer *peer;
struct sk_buff *msg;
u32 peer_id;
int ret;
int ret, i;
if (GENL_REQ_ATTR_CHECK(info, OVPN_A_PEER))
return -EINVAL;
@ -668,6 +668,23 @@ int ovpn_nl_peer_get_doit(struct sk_buff *skb, struct genl_info *info)
OVPN_A_PEER_ID))
return -EINVAL;
/* OVPN_CMD_PEER_GET expects only the PEER_ID, therefore
* ensure that the user hasn't specified any other attribute.
*
* Unfortunately this check cannot be performed via netlink
* spec/policy and must be open-coded.
*/
for (i = 0; i < OVPN_A_PEER_MAX + 1; i++) {
if (i == OVPN_A_PEER_ID)
continue;
if (attrs[i]) {
NL_SET_ERR_MSG_FMT_MOD(info->extack,
"unexpected attribute %u", i);
return -EINVAL;
}
}
peer_id = nla_get_u32(attrs[OVPN_A_PEER_ID]);
peer = ovpn_peer_get_by_id(ovpn, peer_id);
if (!peer) {
@ -768,7 +785,7 @@ int ovpn_nl_peer_del_doit(struct sk_buff *skb, struct genl_info *info)
return -EINVAL;
ret = nla_parse_nested(attrs, OVPN_A_PEER_MAX, info->attrs[OVPN_A_PEER],
ovpn_peer_nl_policy, info->extack);
ovpn_peer_del_input_nl_policy, info->extack);
if (ret)
return ret;
@ -969,14 +986,14 @@ int ovpn_nl_key_get_doit(struct sk_buff *skb, struct genl_info *info)
struct ovpn_peer *peer;
struct sk_buff *msg;
u32 peer_id;
int ret;
int ret, i;
if (GENL_REQ_ATTR_CHECK(info, OVPN_A_KEYCONF))
return -EINVAL;
ret = nla_parse_nested(attrs, OVPN_A_KEYCONF_MAX,
info->attrs[OVPN_A_KEYCONF],
ovpn_keyconf_nl_policy, info->extack);
ovpn_keyconf_get_nl_policy, info->extack);
if (ret)
return ret;
@ -988,6 +1005,24 @@ int ovpn_nl_key_get_doit(struct sk_buff *skb, struct genl_info *info)
OVPN_A_KEYCONF_SLOT))
return -EINVAL;
/* OVPN_CMD_KEY_GET expects only the PEER_ID and the SLOT, therefore
* ensure that the user hasn't specified any other attribute.
*
* Unfortunately this check cannot be performed via netlink
* spec/policy and must be open-coded.
*/
for (i = 0; i < OVPN_A_KEYCONF_MAX + 1; i++) {
if (i == OVPN_A_KEYCONF_PEER_ID ||
i == OVPN_A_KEYCONF_SLOT)
continue;
if (attrs[i]) {
NL_SET_ERR_MSG_FMT_MOD(info->extack,
"unexpected attribute %u", i);
return -EINVAL;
}
}
peer_id = nla_get_u32(attrs[OVPN_A_KEYCONF_PEER_ID]);
peer = ovpn_peer_get_by_id(ovpn, peer_id);
if (!peer) {
@ -1037,7 +1072,7 @@ int ovpn_nl_key_swap_doit(struct sk_buff *skb, struct genl_info *info)
ret = nla_parse_nested(attrs, OVPN_A_KEYCONF_MAX,
info->attrs[OVPN_A_KEYCONF],
ovpn_keyconf_nl_policy, info->extack);
ovpn_keyconf_swap_input_nl_policy, info->extack);
if (ret)
return ret;
@ -1074,7 +1109,7 @@ int ovpn_nl_key_del_doit(struct sk_buff *skb, struct genl_info *info)
ret = nla_parse_nested(attrs, OVPN_A_KEYCONF_MAX,
info->attrs[OVPN_A_KEYCONF],
ovpn_keyconf_nl_policy, info->extack);
ovpn_keyconf_del_input_nl_policy, info->extack);
if (ret)
return ret;