[PATCH net v2] macsec: require CAP_NET_ADMIN in the device netns for changelink

macsec_changelink() operates on two netns: dev_net(dev) and the netns
of macsec->real_dev. They differ once the macsec device is created in
or moved to a netns other than the one holding the real device. The
rtnl changelink path checks CAP_NET_ADMIN only against dev_net(dev), so
a caller privileged there but not in dev_net(real_dev) can reach
real_dev's driver:

  - IFLA_MACSEC_OFFLOAD -> macsec_update_offload(), which calls
    ops->mdo_add_secy() / mdo_del_secy() on real_dev.
  - the tail "propagate to the device" block, which calls
    ops->mdo_upd_secy() on real_dev for any request against an already
    offloaded device that does not change the offload mode. This one
    needs no IFLA_MACSEC_OFFLOAD attribute at all.

Gate both with rtnl_dev_link_net_capable(), as the tunnel, geneve,
vxlan and macvlan handlers already do. It fires only when the request
would reach real_dev: when IFLA_MACSEC_OFFLOAD asks for a mode other
than the current one, or when the device is already offloaded. The
local attributes of a non-offloaded device stay configurable from its
own netns.

The check is in the changelink handler rather than in
macsec_update_offload(), because the mdo_upd_secy() propagation does
not go through that function. Its other caller, macsec_upd_offload()
for MACSEC_CMD_UPD_OFFLOAD, is GENL_ADMIN_PERM. The ndo and dellink
paths that reach real_dev's driver as a side effect of operating the
macsec device itself (open/stop, set_mac_address, dellink) are
unchanged here.

rtnl_dev_link_net_capable() was added by commit 8165f7ff57d9 ("net:
ip_gre: require CAP_NET_ADMIN in the device netns for changelink"); a
stable tree without it needs that first.

Fixes: 3cf3227a21d1 ("net: macsec: hardware offloading infrastructure")
Cc: <redacted> # see patch description
Cc: Antoine Tenart <atenart@kernel.org>
Assisted-by: LLM
Signed-off-by: Aamir Ahmed <redacted>
---
v2:
  - factor the check out into macsec_changelink_check_netns() (Jakub)
  - fire the offload term only on an actual transition, so restating
    the current mode on a non-offloaded device is still allowed
    (Sashiko, Jakub)
  - commit message: say why the check stays in the changelink handler
    (Sashiko)
  - Cc Antoine Tenart, author of the Fixes: commit (netdev CI)
  - add the Assisted-by: LLM trailer
v1: https://lore.kernel.org/netdev/AS8P251MB000190AAD82CD30EFBC57B4DC8B42@AS8P251MB0001.EURP251.PROD.OUTLOOK.COM/ (local)

Found and tested with LLM assistance, by auditing rtnl_link_ops
changelink handlers for this check. The helper and the changelog were
reviewed by hand.

Where 8165f7ff57d9 is missing, the call can be open-coded as
  net_eq(dev_net(dev), dev_net(macsec->real_dev)) ||
  ns_capable(dev_net(macsec->real_dev)->user_ns, CAP_NET_ADMIN)

Tested on the base-commit below against three kernels - unpatched, v1
and this v2 - with netdevsim as the offload-capable NIC, and macsec0
(offload mac) and macsec1 (no offload) moved into an unshare -Urn
namespace; the netdevsim port and an offloaded macsec2 (row 7) stayed
in init_net. ftrace on nsim_macsec_* showed rows 1, 2, 3 and 6 reaching
real_dev's driver unpatched (upd_secy, upd_secy, del_secy, add_secy)
and no row reaching it on v1 or v2 except rows 7 and 8. Every EPERM
carried the new extack.

                                                  unpatched  v1     v2
 1 offloaded  + unpriv "encrypt off"              ok         EPERM  EPERM
 2 offloaded  + unpriv "offload mac encrypt off"  ok         EPERM  EPERM
 3 offloaded  + unpriv "offload off"              ok         EPERM  EPERM
 4 !offloaded + unpriv "encrypt off"              ok         ok     ok
 5 !offloaded + unpriv "offload off encrypt off"  ok         EPERM  ok
 6 !offloaded + unpriv "offload mac"              ok         EPERM  EPERM
 7 same-netns privileged "encrypt off"            ok         ok     ok
 8 privileged cross-netns "offload off"           ok         ok     ok

Rows 1, 2, 3 and 6 unpatched are the bug; row 5 is what v1 wrongly refused.

 drivers/net/macsec.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 6f9f3acef..e019e7a25 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c

@@ -19,6 +19,7 @@
 #include <net/gro_cells.h>
 #include <net/macsec.h>
 #include <net/dst_metadata.h>
+#include <net/rtnetlink.h>
 #include <net/netdev_lock.h>
 #include <linux/phy.h>
 #include <linux/byteorder/generic.h>

@@ -3945,6 +3946,33 @@ static int macsec_changelink_common(struct net_device *dev,
 	return 0;
 }
 
+/* Both a change of the offload state (mdo_add_secy()/mdo_del_secy()) and
+ * any request against an already offloaded device (mdo_upd_secy()) reach
+ * the driver of real_dev, which may live in a different network namespace
+ * from the macsec device. Require CAP_NET_ADMIN over real_dev's namespace
+ * for those; local attributes of a non-offloaded device are unaffected.
+ */
+static int macsec_changelink_check_netns(struct net_device *dev,
+					 struct nlattr *data[],
+					 struct netlink_ext_ack *extack)
+{
+	struct macsec_dev *macsec = macsec_priv(dev);
+	bool reaches_real_dev;
+
+	reaches_real_dev = macsec_is_offloaded(macsec);
+	if (data[IFLA_MACSEC_OFFLOAD] &&
+	    nla_get_u8(data[IFLA_MACSEC_OFFLOAD]) != macsec->offload)
+		reaches_real_dev = true;
+
+	if (!reaches_real_dev ||
+	    rtnl_dev_link_net_capable(dev, dev_net(macsec->real_dev)))
+		return 0;
+
+	NL_SET_ERR_MSG(extack,
+		       "Changing a MACsec device whose real device is in another network namespace requires CAP_NET_ADMIN in that namespace");
+	return -EPERM;
+}
+
 static int macsec_changelink(struct net_device *dev, struct nlattr *tb[],
 			     struct nlattr *data[],
 			     struct netlink_ext_ack *extack)

@@ -3959,6 +3987,10 @@ static int macsec_changelink(struct net_device *dev, struct nlattr *tb[],
 	if (!data)
 		return 0;
 
+	ret = macsec_changelink_check_netns(dev, data, extack);
+	if (ret)
+		return ret;
+
 	if (data[IFLA_MACSEC_CIPHER_SUITE] ||
 	    data[IFLA_MACSEC_ICV_LEN] ||
 	    data[IFLA_MACSEC_SCI] ||

base-commit: 78445023439506ebd83b86d40b1e428a3b309d4a
-- 
2.43.0