I have one more question, there is a field "secmark" in the sk_buff structure, i was wondering if i can
safely use it instead of adding security_sk_getsecid
I did some tests and the secmark revolves exactly to secid if security marking is turned on and smack
has the option "Packet marking using secmarks for netfilter/CONFIG_SECURITY_SMACK_NETFILTER:" turned on
The function smack does it in, seems like it's doing exactly what one would expect:
static unsigned int smack_ipv4_output(const struct nf_hook_ops *ops,
struct sk_buff *skb,
const struct nf_hook_state *state)
{
struct socket_smack *ssp;
struct smack_known *skp;
if (skb && skb->sk && skb->sk->sk_security) {
ssp = skb->sk->sk_security;
skp = ssp->smk_out;
skb->secmark = skp->smk_secid;
}
return NF_ACCEPT;
}
and it's registered as an op
static struct nf_hook_ops smack_nf_ops[] = {
{
.hook = smack_ipv4_output,
.owner = THIS_MODULE,
.pf = NFPROTO_IPV4,
.hooknum = NF_INET_LOCAL_OUT,
.priority = NF_IP_PRI_SELINUX_FIRST,
},
(the same is done for ipv6)
So would it be safe to use it ?
best regards
On 05/27/2015 01:48 PM, Florian Westphal wrote:
> Roman Kubiak <r.kubiak@xxxxxxxxxxx> wrote:
>> This patch adds an additional attribute when sending
>> packet information via netlink in netfilter_queue module.
>> It will send additional security context data, so that
>> userspace applications can verify this context against
>> their own security databases.
>>
>> Signed-off-by: Roman Kubiak <r.kubiak@xxxxxxxxxxx>
>> ---
>> v2:
>> - nfqnl_get_sk_secctx returns seclen now, this changes
>> - updated size calculation
>> - changed NFQA_SECCTX comment
>> - removed duplicate testing of NFQA_CFG_F flags
>>
>> v3:
>> - NULL is not added to the security context anymore
>> - return 0 when socket is invalid in nfqnl_get_sk_secctx
>> - small intent change
>> - removed ret variable in nfqnl_get_sk_secctx
>> ---
>> include/uapi/linux/netfilter/nfnetlink_queue.h | 4 +++-
>> net/netfilter/nfnetlink_queue_core.c | 31 ++++++++++++++++++++++++++
>> 2 files changed, 34 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/uapi/linux/netfilter/nfnetlink_queue.h b/include/uapi/linux/netfilter/nfnetlink_queue.h
>> index 8dd819e..b67a853 100644
>> --- a/include/uapi/linux/netfilter/nfnetlink_queue.h
>> +++ b/include/uapi/linux/netfilter/nfnetlink_queue.h
>> @@ -49,6 +49,7 @@ enum nfqnl_attr_type {
>> NFQA_EXP, /* nf_conntrack_netlink.h */
>> NFQA_UID, /* __u32 sk uid */
>> NFQA_GID, /* __u32 sk gid */
>> + NFQA_SECCTX, /* security context string */
>>
>> __NFQA_MAX
>> };
>> @@ -102,7 +103,8 @@ enum nfqnl_attr_config {
>> #define NFQA_CFG_F_CONNTRACK (1 << 1)
>> #define NFQA_CFG_F_GSO (1 << 2)
>> #define NFQA_CFG_F_UID_GID (1 << 3)
>> -#define NFQA_CFG_F_MAX (1 << 4)
>> +#define NFQA_CFG_F_SECCTX (1 << 4)
>> +#define NFQA_CFG_F_MAX (1 << 5)
>>
>> /* flags for NFQA_SKB_INFO */
>> /* packet appears to have wrong checksums, but they are ok */
>> diff --git a/net/netfilter/nfnetlink_queue_core.c b/net/netfilter/nfnetlink_queue_core.c
>> index 0b98c74..ae4f520 100644
>> --- a/net/netfilter/nfnetlink_queue_core.c
>> +++ b/net/netfilter/nfnetlink_queue_core.c
>> @@ -278,6 +278,24 @@ nla_put_failure:
>> return -1;
>> }
>>
>> +static u32 nfqnl_get_sk_secctx(struct sock *sk, char **secdata)
>> +{
>> + u32 secid = 0;
>> + u32 seclen = 0;
>> +
>> + if (!sk || !sk_fullsock(sk))
>> + return 0;
>
> if (!sk) is not needed anymore
>
>> bool csum_verify;
>> + char *secdata = NULL;
>> + u32 seclen = 0;
>>
>> size = nlmsg_total_size(sizeof(struct nfgenmsg))
>> + nla_total_size(sizeof(struct nfqnl_msg_packet_hdr))
>> @@ -352,6 +372,12 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
>> + nla_total_size(sizeof(u_int32_t))); /* gid */
>> }
>>
>> + if ((queue->flags & NFQA_CFG_F_SECCTX) && entskb->sk) {
>
> ... or remove entskb->sk test here (I have no preference where
> you test this, either caller or callee is fine with me).
>
>> skb = nfnetlink_alloc_skb(net, size, queue->peer_portid,
>> GFP_ATOMIC);
>> if (!skb) {
>> @@ -479,6 +505,11 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
>> nfqnl_put_sk_uidgid(skb, entskb->sk) < 0)
>> goto nla_put_failure;
>>
>> + if (seclen) {
>> + if (nla_put(skb, NFQA_SECCTX, seclen, secdata))
>> + goto nla_put_failure;
>> + }
>> +
>
> Nit: can be written as
>
> if (seclen && nla_put(skb, NFQA_SECCTX, seclen, secdata))
> goto nla_put_failure;
>
> Otherwise I think this looks good. Please consider sending v4 _after_
> security_sk_getsecid situation is resolved.
>
> It would be good to note in the --- section in case there is a
> dependency; we always want to avoid "it won't build (without change
> x from tree y)."
>
> Thanks.
>
--
--------------
Roman Kubiak
--------------
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
