]> git.ipfire.org Git - thirdparty/kernel/linux.git/commitdiff
ovpn: reject unexpected netlink attributes
authorAntonio Quartulli <antonio@openvpn.net>
Wed, 25 Jun 2025 14:08:11 +0000 (16:08 +0200)
committerAntonio Quartulli <antonio@openvpn.net>
Wed, 16 Jul 2025 09:51:30 +0000 (11:51 +0200)
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@mandelbit.com>
Closes: https://github.com/OpenVPN/ovpn-net-next/issues/19
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
Documentation/netlink/specs/ovpn.yaml
drivers/net/ovpn/netlink-gen.c
drivers/net/ovpn/netlink-gen.h
drivers/net/ovpn/netlink.c

index 096c51f0c69a8de777281af35e345dd90f27ecd2..ba76426a542d6ed76d03398e8b2a6ce8a29c43a9 100644 (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:
index 58e1a4342378eacc67dec19de34fa4bb45f16fe2..14298188c5f168c0001502edd00c3dd0549115cd 100644 (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 */
index 66a4e4a0a055b4477b67801ded825e9ec068b0e6..220b5b2fdd4f27a7d27e9dda2c69944269fb43bd 100644 (file)
 
 /* 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);
index a4ec53def46ea677f6aab28bf43591b56d92d1b0..c7f382437630292df56fa90df303135f69e5828e 100644 (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;