[PATCH 3/4] add support for modifying secmark via ctnetlink

[PATCH 3/4] add support for modifying secmark via ctnetlink

[Pablo Neira Ayuso]

[-- Attachment #1: Type: text/plain, Size: 215 bytes --]

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

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

[-- Attachment #2: 04.patch --]
[-- Type: text/x-patch, Size: 917 bytes --]

[PATCH] 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 <pablo@netfilter.org>

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
@@ -1121,6 +1121,11 @@ ctnetlink_change_conntrack(struct nf_con
 		ct->mark = ntohl(nla_get_be32(cda[CTA_MARK]));
 #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);

Re: [PATCH 3/4] add support for modifying secmark via ctnetlink

[Patrick McHardy]

Pablo Neira Ayuso wrote:
> As for now we only support dumping. This patch adds support to change
> the secmark from ctnetlink.
> 
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> 
> 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
> @@ -1121,6 +1121,11 @@ ctnetlink_change_conntrack(struct nf_con
>  		ct->mark = ntohl(nla_get_be32(cda[CTA_MARK]));
>  #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?

Re: [PATCH 3/4] add support for modifying secmark via ctnetlink

[James Morris]

On Wed, 21 May 2008, Patrick McHardy wrote:

> Pablo Neira Ayuso wrote:
> > As for now we only support dumping. This patch adds support to change
> > the secmark from ctnetlink.
> > 
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > 
> > 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
> > @@ -1121,6 +1121,11 @@ ctnetlink_change_conntrack(struct nf_con
> >  		ct->mark = ntohl(nla_get_be32(cda[CTA_MARK]));
> >  #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?

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.


- James
-- 
James Morris
<jmorris@namei.org>

Re: [PATCH 3/4] add support for modifying secmark via ctnetlink

[Patrick McHardy]

James Morris wrote:
> On Wed, 21 May 2008, Patrick McHardy wrote:
> 
>> Pablo Neira Ayuso wrote:
>>> 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).
> 
> 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.

> 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.

Re: [PATCH 3/4] add support for modifying secmark via ctnetlink

[Paul Moore]

On Wednesday 21 May 2008 8:00:09 am Patrick McHardy wrote:
> James Morris wrote:
> > On Wed, 21 May 2008, Patrick McHardy wrote:
> >> Pablo Neira Ayuso wrote:
> >>> 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).
> >
> > 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.

>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.

> > 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.

-- 
paul moore
linux @ hp

Re: [PATCH 3/4] add support for modifying secmark via ctnetlink

[Stephen Smalley]

On Wed, 2008-05-21 at 12:46 -0400, Paul Moore wrote:
> On Wednesday 21 May 2008 8:00:09 am Patrick McHardy wrote:
> > James Morris wrote:
> > > On Wed, 21 May 2008, Patrick McHardy wrote:
> > >> Pablo Neira Ayuso wrote:
> > >>> 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).
> > >
> > > 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.

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).

Not that I'm in favor of letting ctnetlink modify the secmark, but just
FYI.

>   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.
> 
> > > 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.

-- 
Stephen Smalley
National Security Agency

Re: [PATCH 3/4] add support for modifying secmark via ctnetlink

[Patrick McHardy]

Paul Moore wrote:
> 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.

> 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.

>>> 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.

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

Re: [PATCH 3/4] add support for modifying secmark via ctnetlink

[Patrick McHardy]

Stephen Smalley wrote:
> On Wed, 2008-05-21 at 12:46 -0400, Paul Moore wrote:
>>
>> 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).

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

Re: [PATCH 3/4] add support for modifying secmark via ctnetlink

[Paul Moore]

On Wednesday 21 May 2008 12:54:16 pm Stephen Smalley wrote:
> On Wed, 2008-05-21 at 12:46 -0400, Paul Moore wrote:
> > 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).

Thanks, that is good to know, I missed that.

-- 
paul moore
linux @ hp

Re: [PATCH 3/4] add support for modifying secmark via ctnetlink

[Paul Moore]

On Wednesday 21 May 2008 1:11:59 pm Patrick McHardy wrote:
> Paul Moore wrote:
> > 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.

Thanks, that is definitely handy.

> > 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.

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.

-- 
paul moore
linux @ hp

Re: [PATCH 3/4] add support for modifying secmark via ctnetlink

[Stephen Smalley]

On Wed, 2008-05-21 at 13:51 -0400, Paul Moore wrote:
> On Wednesday 21 May 2008 1:11:59 pm Patrick McHardy wrote:
> > Paul Moore wrote:
> > > 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.
> 
> Thanks, that is definitely handy.
> 
> > > 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.
> 
> 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.

I'd prefer to only do that if there is actual breakage of existing
applications in widespread use from the new checks.

-- 
Stephen Smalley
National Security Agency

Re: [PATCH 3/4] add support for modifying secmark via ctnetlink

[Pablo Neira Ayuso]

Patrick McHardy wrote:
> Paul Moore wrote:
>> 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.
> 
>> 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.
> 
>>>> 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.
> 
> In that case it doesn't seem to make sense for synchronization.
> That was just a guess of mine though, Pablo?

Indeed. I thought that the secmark was global instead of 
system-specific. My idea was that you could use the conntrack command 
line tool to change the secmark on the fly (as you can do with skb-mark 
and ct-mark), however, as the SELinux guys said, this goes against the 
access-check-before-setting idea.

With regards to synchronization, conntrackd should run in a dedicated 
system that acts as firewall with no local services (as forwarder), so 
AFAICS the secmark would not be of any use there.

I don't have any particular interest in pushing this patch forward, 
actually I just look at source code yesterday late at night and I 
noticed some missing unsupported bits in ctnetlink and I tried to finish 
it :). So, I'm fine with dropping this patch.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: [PATCH 3/4] add support for modifying secmark via ctnetlink

[Stephen Smalley]

On Wed, 2008-05-21 at 19:13 +0200, Patrick McHardy wrote:
> Stephen Smalley wrote:
> > On Wed, 2008-05-21 at 12:46 -0400, Paul Moore wrote:
> >>
> >> 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).
> 
> 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.

-- 
Stephen Smalley
National Security Agency

Re: [PATCH 3/4] add support for modifying secmark via ctnetlink

[Patrick McHardy]

Stephen Smalley wrote:
> On Wed, 2008-05-21 at 19:13 +0200, Patrick McHardy wrote:
>   
>> Stephen Smalley wrote:
>>     
>>> On Wed, 2008-05-21 at 12:46 -0400, Paul Moore wrote:
>>>       
>>>> 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).
>>>       
>> 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.
>   

Yes, these four members can be removed.