| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250716115443.16763-3-antonio@openvpn.net>
Date: Wed, 16 Jul 2025 13:54:42 +0200
From: Antonio Quartulli <antonio@...nvpn.net>
To: netdev@...r.kernel.org
Cc: Antonio Quartulli <antonio@...nvpn.net>,
Sabrina Dubroca <sd@...asysnail.net>,
"David S . Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Ralf Lici <ralf@...delbit.com>
Subject: [PATCH net 2/3] 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: b7a63391aa98 ("ovpn: add basic netlink support")
Reported-by: Ralf Lici <ralf@...delbit.com>
Closes: https://github.com/OpenVPN/ovpn-net-next/issues/19
Signed-off-by: Antonio Quartulli <antonio@...nvpn.net>
---
Documentation/netlink/specs/ovpn.yaml | 153 +++++++++++++++++++++++++-
drivers/net/ovpn/netlink-gen.c | 61 +++++++++-
drivers/net/ovpn/netlink-gen.h | 6 +
drivers/net/ovpn/netlink.c | 51 +++++++--
4 files changed, 251 insertions(+), 20 deletions(-)
diff --git a/Documentation/netlink/specs/ovpn.yaml b/Documentation/netlink/specs/ovpn.yaml
index 096c51f0c69a..ba76426a542d 100644
--- a/Documentation/netlink/specs/ovpn.yaml
+++ b/Documentation/netlink/specs/ovpn.yaml
@@ -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:
diff --git a/drivers/net/ovpn/netlink-gen.c b/drivers/net/ovpn/netlink-gen.c
index 58e1a4342378..14298188c5f1 100644
--- a/drivers/net/ovpn/netlink-gen.c
+++ b/drivers/net/ovpn/netlink-gen.c
@@ -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 */
diff --git a/drivers/net/ovpn/netlink-gen.h b/drivers/net/ovpn/netlink-gen.h
index 66a4e4a0a055..220b5b2fdd4f 100644
--- a/drivers/net/ovpn/netlink-gen.h
+++ b/drivers/net/ovpn/netlink-gen.h
@@ -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);
diff --git a/drivers/net/ovpn/netlink.c b/drivers/net/ovpn/netlink.c
index a4ec53def46e..c7f382437630 100644
--- a/drivers/net/ovpn/netlink.c
+++ b/drivers/net/ovpn/netlink.c
@@ -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;
--
2.49.1
Powered by blists - more mailing lists