add support for modifying secmark via ctnetlink

As for now we only support dumping. This patch adds support to change the secmark from ctnetlink.

Signed-off-by Pablo Neira Ayuso <[email protected]>

Index: net-2.6.git/net/netfilter/nf_conntrack_netlink.c
===================================================================

net/netfilter/nf_conntrack_netlink.c +0 -0

@@ -1121,6 +1121,11 @@ ctnetlink_change_conntrack(struct nf_con
 		ct->mark = ntohl(nla_get_be32(cda[CTA_MARK]));

Quoted 13 lines · Pablo Neira Ayuso

As for now we only support dumping. This patch adds support to change the secmark from ctnetlink.

Signed-off-by Pablo Neira Ayuso <[email protected]>

Index: net-2.6.git/net/netfilter/nf_conntrack_netlink.c
===================================================================
--- net-2.6.git.orig/net/netfilter/nf_conntrack_netlink.c 2008-05-20
22:10:31.000000000 +0200
+++ net-2.6.git/net/netfilter/nf_conntrack_netlink.c 2008-05-20
22:10:56.000000000 +0200

net/netfilter/nf_conntrack_netlink.c +0 -0

@@ -1121,6 +1121,11 @@ ctnetlink_change_conntrack(struct nf_con
 		ct->mark = ntohl(nla_get_be32(cda[CTA_MARK]));

net/netfilter/nf_conntrack_netlink.c ctnetlink_change_conntrack(struct nf_con · line 1131

Quoted 10 lines

net/netfilter/nf_conntrack_netlink.c +5 -0

 #endif
 
+#if defined(CONFIG_NF_CONNTRACK_SECMARK)
+	if (cda[CTA_SECMARK])
+		ct->secmark = ntohl(nla_get_be32(cda[CTA_SECMARK]));
+#endif
+
 #ifdef CONFIG_NF_NAT_NEEDED
  if (cda[CTA_NAT_SEQ_ADJ_ORIG] || cda[CTA_NAT_SEQ_ADJ_REPLY]) {
   err = ctnetlink_change_nat_seq_adj(ct, cda);

I'm wondering whether this isn't subverting the intent of secmark since AFAIK SELinux doesn't have finegrained controls for netlink messages. OTOH, it also doesn't have finegrained control over iptables rulesets.

James, does this patch look OK to you?

Quoted 9 lines · Patrick McHardy

Quoted 3 lines · Pablo Neira Ayuso

As for now we only support dumping. This patch adds support to change the secmark from ctnetlink.

I'm wondering whether this isn't subverting the intent of secmark since AFAIK SELinux doesn't have finegrained controls for netlink messages. OTOH, it also doesn't have finegrained control over iptables rulesets.

Quoted 1 line

James, does this patch look OK to you?

There is some fine-grained netlink coverage, but it is incomplete (the various generic netlink layers likely need to be consolidated first).

Currently, the SECMARK and CONNSECMARK targets call out to selinux_secmark_relabel_packet_permission() when SELinux is active to obtain a permission check. So, detection of the current security model would need to be similarly performed.

The bigger issue perhaps is whether there's really a need to set secmark via ctnetlink.

Quoted 16 lines · James Morris

Quoted 10 lines · Patrick McHardy

Quoted 2 lines · Pablo Neira Ayuso

As for now we only support dumping. This patch adds support to change the secmark from ctnetlink.

I'm wondering whether this isn't subverting the intent of secmark since AFAIK SELinux doesn't have finegrained controls for netlink messages. OTOH, it also doesn't have finegrained control over iptables rulesets.

James, does this patch look OK to you?

There is some fine-grained netlink coverage, but it is incomplete (the various generic netlink layers likely need to be consolidated first).

Quoted 4 lines

Currently, the SECMARK and CONNSECMARK targets call out to selinux_secmark_relabel_packet_permission() when SELinux is active to obtain a permission check. So, detection of the current security model would need to be similarly performed.

Thanks for the explanation.

From what I can tell (I'm no netfilter expert), we need to ensure that

Quoted 2 lines

The bigger issue perhaps is whether there's really a need to set secmark via ctnetlink.

I think Pablo wants to use it for synchronization with conntrackd, but I'm not sure.

Quoted 17 lines · Patrick McHardy

Quoted 16 lines · James Morris

Quoted 10 lines · Patrick McHardy

Quoted 2 lines · Pablo Neira Ayuso

As for now we only support dumping. This patch adds support to change the secmark from ctnetlink.

I'm wondering whether this isn't subverting the intent of secmark since AFAIK SELinux doesn't have finegrained controls for netlink messages. OTOH, it also doesn't have finegrained control over iptables rulesets.

James, does this patch look OK to you?

There is some fine-grained netlink coverage, but it is incomplete (the various generic netlink layers likely need to be consolidated first).

Quoted 6 lines

Quoted 4 lines

Currently, the SECMARK and CONNSECMARK targets call out to selinux_secmark_relabel_packet_permission() when SELinux is active to obtain a permission check. So, detection of the current security model would need to be similarly performed.

Thanks for the explanation.

Sorry, I don't subscribe to netfilter-devel so I missed the original discussion; I'm subscribed now.

I agree with James that we need to perform some access check before setting the ct->secmark field, however, I don't think it is as simple as calling selinux_secmark_relabel_packet_permission(). The problem is that the selinux_secmark_relabel_packet_permission() function checks to see if the currently running task can relabel packets; in this case we don't want to check the currently running task we want to check the sender of the netlink message which we can't really do currently.

The
next best thing is to provide access control around the individual netlink message types which we can currently do.

Quoted 1 line

From what I can tell (I'm no netfilter expert), we need to ensure that

only privileged process have the ability to send netlink messages with type (NFNL_SUBSYS_CTNETLINK | IPCTNL_MSG_CT_NEW) which should be possible using the code in security/selinux/nlmsgtab.c. You would need to create a NETLINK_NETFILTER nlmsg_perm struct first like the others for routing, XFRM, audit, etc.

Quoted 5 lines

Quoted 2 lines

The bigger issue perhaps is whether there's really a need to set secmark via ctnetlink.

I think Pablo wants to use it for synchronization with conntrackd, but I'm not sure.

To be honest I'm a little uneasy about this because we don't have a way to perform any "good" access check (there is no subject in this case since we can't check the security label of the process sending the netlink message). Has anyone checked to see if setting the secmark via conntrackd works? I really doubt it would since the actual secmakr integer value is specific, in the SELinux case, to a particular running kernel and not globally recognized by other systems even if they are running the same kernel and SELinux policy. There are ways to workaround this by passing the string based label between systems but even that can have issues if the security policies differ slightly.

Quoted 2 lines · Paul Moore

I agree with James that we need to perform some access check before setting the ct->secmark field, however, I don't think it is as simple as calling selinux_secmark_relabel_packet_permission(). The problem is that the selinux_secmark_relabel_packet_permission() function checks to see if the currently running task can relabel packets; in this case we don't want to check the currently running task we want to check the sender of the netlink message which we can't really do currently.

Sending task SID is saved in NETLINK_CB(skb).sid at send time, so the information is available (but would need to be passed into the function).

Quoted 10 lines · Stephen Smalley

Quoted 8 lines · Paul Moore

I agree with James that we need to perform some access check before setting the ct->secmark field, however, I don't think it is as simple as calling selinux_secmark_relabel_packet_permission(). The problem is that the selinux_secmark_relabel_packet_permission() function checks to see if the currently running task can relabel packets; in this case we don't want to check the currently running task we want to check the sender of the netlink message which we can't really do currently.

Quoted 3 lines

Sending task SID is saved in NETLINK_CB(skb).sid at send time, so the information is available (but would need to be passed into the function).

This part can actually be removed from af_netlink, see the message I just sent to Paul for reference.

Quoted 10 lines · Patrick McHardy

Quoted 8 lines · Stephen Smalley

Quoted 2 lines · Paul Moore

I agree with James that we need to perform some access check before setting the ct->secmark field, however, I don't think it is as simple as calling selinux_secmark_relabel_packet_permission(). The problem is that the selinux_secmark_relabel_packet_permission() function checks to see if the currently running task can relabel packets; in this case we don't want to check the currently running task we want to check the sender of the netlink message which we can't really do currently.

Sending task SID is saved in NETLINK_CB(skb).sid at send time, so the information is available (but would need to be passed into the function).

Quoted 3 lines

This part can actually be removed from af_netlink, see the message I just sent to Paul for reference.

So eff_cap, loginuid, sessionid, and sid no longer need to be saved in netlink_skb_parms? Current users include security modules and audit.

Quoted 8 lines · Paul Moore

I agree with James that we need to perform some access check before setting the ct->secmark field, however, I don't think it is as simple as calling selinux_secmark_relabel_packet_permission(). The problem is that the selinux_secmark_relabel_packet_permission() function checks to see if the currently running task can relabel packets; in this case we don't want to check the currently running task we want to check the sender of the netlink message which we can't really do currently.

Sending task SID is saved in NETLINK_CB(skb).sid at send time, so the information is available (but would need to be passed into the function).

Sorry, I don't subscribe to netfilter-devel so I missed the original discussion; I'm subscribed now.

I agree with James that we need to perform some access check before setting the ct->secmark field, however, I don't think it is as simple as calling selinux_secmark_relabel_packet_permission(). The problem is that the selinux_secmark_relabel_packet_permission() function checks to see if the currently running task can relabel packets; in this case we don't want to check the currently running task we want to check the sender of the netlink message which we can't really do currently. The next best thing is to provide access control around the individual netlink message types which we can currently do.

From what I can tell (I'm no netfilter expert), we need to ensure that only privileged process have the ability to send netlink messages with type (NFNL_SUBSYS_CTNETLINK | IPCTNL_MSG_CT_NEW) which should be possible using the code in security/selinux/nlmsgtab.c. You would need to create a NETLINK_NETFILTER nlmsg_perm struct first like the others for routing, XFRM, audit, etc.

Quoted 4 lines

Quoted 2 lines

The bigger issue perhaps is whether there's really a need to set secmark via ctnetlink.

I think Pablo wants to use it for synchronization with conntrackd, but I'm not sure.

To be honest I'm a little uneasy about this because we don't have a way to perform any "good" access check (there is no subject in this case since we can't check the security label of the process sending the netlink message). Has anyone checked to see if setting the secmark via conntrackd works? I really doubt it would since the actual secmakr integer value is specific, in the SELinux case, to a particular running kernel and not globally recognized by other systems even if they are running the same kernel and SELinux policy. There are ways to workaround this by passing the string based label between systems but even that can have issues if the security policies differ slightly.

Quoted 13 lines · Paul Moore

Sorry, I don't subscribe to netfilter-devel so I missed the original discussion; I'm subscribed now.

I agree with James that we need to perform some access check before setting the ct->secmark field, however, I don't think it is as simple as calling selinux_secmark_relabel_packet_permission(). The problem is that the selinux_secmark_relabel_packet_permission() function checks to see if the currently running task can relabel packets; in this case we don't want to check the currently running task we want to check the sender of the netlink message which we can't really do currently. The next best thing is to provide access control around the individual netlink message types which we can currently do.

Actually in the current kernel netlink message processing is done synchronously in the context of the sending process (commit cd40b7d3: [NET]: make netlink user -> kernel interface synchronious). So this check should be easy to add.

Quoted 7 lines

From what I can tell (I'm no netfilter expert), we need to ensure that only privileged process have the ability to send netlink messages with type (NFNL_SUBSYS_CTNETLINK | IPCTNL_MSG_CT_NEW) which should be possible using the code in security/selinux/nlmsgtab.c. You would need to create a NETLINK_NETFILTER nlmsg_perm struct first like the others for routing, XFRM, audit, etc.

So far nfnetlink is restricted to CAP_NET_ADMIN and uses security_netlink_recv() for permission checks. I'll add a nlmsg_perm struct for nfnetlink, I guess this makes sense independant of this patch. Will send over for review once its ready.

Quoted 15 lines · Patrick McHardy

Quoted 13 lines · Paul Moore

Sorry, I don't subscribe to netfilter-devel so I missed the original discussion; I'm subscribed now.

I agree with James that we need to perform some access check before setting the ct->secmark field, however, I don't think it is as simple as calling selinux_secmark_relabel_packet_permission(). The problem is that the selinux_secmark_relabel_packet_permission() function checks to see if the currently running task can relabel packets; in this case we don't want to check the currently running task we want to check the sender of the netlink message which we can't really do currently. The next best thing is to provide access control around the individual netlink message types which we can currently do.

Quoted 4 lines

Actually in the current kernel netlink message processing is done synchronously in the context of the sending process (commit cd40b7d3: [NET]: make netlink user -> kernel interface synchronious). So this check should be easy to add.

Thanks, that is definitely handy.

Quoted 8 lines

Quoted 7 lines

From what I can tell (I'm no netfilter expert), we need to ensure that only privileged process have the ability to send netlink messages with type (NFNL_SUBSYS_CTNETLINK | IPCTNL_MSG_CT_NEW) which should be possible using the code in security/selinux/nlmsgtab.c. You would need to create a NETLINK_NETFILTER nlmsg_perm struct first like the others for routing, XFRM, audit, etc.

Quoted 5 lines

So far nfnetlink is restricted to CAP_NET_ADMIN and uses security_netlink_recv() for permission checks. I'll add a nlmsg_perm struct for nfnetlink, I guess this makes sense independant of this patch. Will send over for review once its ready.

Thanks. When developing the patch keep in mind that the new permissions will probably need to made conditional on the SELinux policy capabilitity to preserve backwards compability. Look for the selinux_policycap_{netpeer,openperm} under security/selinux for examples. I can lend a hand if you have any questions.

Quoted 3 lines · Paul Moore

Sorry, I don't subscribe to netfilter-devel so I missed the original discussion; I'm subscribed now.

I agree with James that we need to perform some access check before setting the ct->secmark field, however, I don't think it is as simple as calling selinux_secmark_relabel_packet_permission(). The problem is that the selinux_secmark_relabel_packet_permission() function checks to see if the currently running task can relabel packets; in this case we don't want to check the currently running task we want to check the sender of the netlink message which we can't really do currently. The next best thing is to provide access control around the individual netlink message types which we can currently do.

Actually in the current kernel netlink message processing is done synchronously in the context of the sending process (commit cd40b7d3: [NET]: make netlink user -> kernel interface synchronious). So this check should be easy to add.

Quoted 1 line

From what I can tell (I'm no netfilter expert), we need to ensure that only privileged process have the ability to send netlink messages with type (NFNL_SUBSYS_CTNETLINK | IPCTNL_MSG_CT_NEW) which should be possible using the code in security/selinux/nlmsgtab.c. You would need to create a NETLINK_NETFILTER nlmsg_perm struct first like the others for routing, XFRM, audit, etc.

So far nfnetlink is restricted to CAP_NET_ADMIN and uses security_netlink_recv() for permission checks. I'll add a nlmsg_perm struct for nfnetlink, I guess this makes sense independant of this patch. Will send over for review once its ready.

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

The bigger issue perhaps is whether there's really a need to set secmark via ctnetlink.

Quoted 4 lines

Quoted 2 lines

I think Pablo wants to use it for synchronization with conntrackd, but I'm not sure.

To be honest I'm a little uneasy about this because we don't have a way to perform any "good" access check (there is no subject in this case since we can't check the security label of the process sending the netlink message). Has anyone checked to see if setting the secmark via conntrackd works? I really doubt it would since the actual secmakr integer value is specific, in the SELinux case, to a particular running kernel and not globally recognized by other systems even if they are running the same kernel and SELinux policy. There are ways to workaround this by passing the string based label between systems but even that can have issues if the security policies differ slightly.

In that case it doesn't seem to make sense for synchronization. That was just a guess of mine though, Pablo?