Re: [PATCH] netfilter: Record uid and gid in xt_AUDIT

[Paul Moore]

On Wed, Oct 9, 2024 at 4:33 PM Richard Weinberger <richard@xxxxxx> wrote:
>
> When recording audit events for new outgoing connections,
> it is helpful to log the user info of the associated socket,
> if available.
> Therefore, check if the skb has a socket, and if it does,
> log the owning fsuid/fsgid.
>
> Signed-off-by: Richard Weinberger <richard@xxxxxx>
> ---
>  net/netfilter/xt_AUDIT.c | 27 +++++++++++++++++++++++++--
>  1 file changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c
> index b6a015aee0cec..d88b5442beaa6 100644
> --- a/net/netfilter/xt_AUDIT.c
> +++ b/net/netfilter/xt_AUDIT.c
> @@ -9,16 +9,19 @@
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
>  #include <linux/audit.h>
> +#include <linux/cred.h>
> +#include <linux/file.h>
> +#include <linux/if_arp.h>
>  #include <linux/module.h>
>  #include <linux/skbuff.h>
>  #include <linux/tcp.h>
>  #include <linux/udp.h>
> -#include <linux/if_arp.h>
>  #include <linux/netfilter/x_tables.h>
>  #include <linux/netfilter/xt_AUDIT.h>
>  #include <linux/netfilter_bridge/ebtables.h>
> -#include <net/ipv6.h>
>  #include <net/ip.h>
> +#include <net/ipv6.h>
> +#include <net/sock.h>
>
>  MODULE_LICENSE("GPL");
>  MODULE_AUTHOR("Thomas Graf <tgraf@xxxxxxxxxx>");
> @@ -66,7 +69,9 @@ static bool audit_ip6(struct audit_buffer *ab, struct sk_buff *skb)
>  static unsigned int
>  audit_tg(struct sk_buff *skb, const struct xt_action_param *par)
>  {
> +       struct sock *sk = skb->sk;
>         struct audit_buffer *ab;
> +       bool got_uidgid = false;
>         int fam = -1;
>
>         if (audit_enabled == AUDIT_OFF)
> @@ -99,6 +104,24 @@ audit_tg(struct sk_buff *skb, const struct xt_action_param *par)
>         if (fam == -1)
>                 audit_log_format(ab, " saddr=? daddr=? proto=-1");
>
> +       if (sk && sk_fullsock(sk)) {
> +               read_lock_bh(&sk->sk_callback_lock);
> +               if (sk->sk_socket && sk->sk_socket->file) {
> +                       const struct file *file = sk->sk_socket->file;
> +                       const struct cred *cred = file->f_cred;
> +
> +                       audit_log_format(ab, " uid=%u gid=%u",
> +                                        from_kuid(&init_user_ns, cred->fsuid),
> +                                        from_kgid(&init_user_ns, cred->fsgid));

[CC'ing the audit and LSM lists for obvious reasons]

If we're logging the subjective credentials of the skb's associated
socket, we really should also log the socket's LSM secctx similar to
what we do with audit_log_task() and audit_log_task_context().
Unfortunately, I don't believe we currently have a LSM interface that
return the secctx from a sock/socket, although we do have
security_inode_getsecctx() which *should* yield the same result using
SOCK_INODE(sk->sk_socket).

I should also mention that I'm currently reviewing a patchset which is
going to add proper support for multiple LSMs in audit which will
likely impact this work.

https://lore.kernel.org/linux-security-module/20241009173222.12219-1-casey@xxxxxxxxxxxxxxxx/

> +                       got_uidgid = true;
> +               }
> +               read_unlock_bh(&sk->sk_callback_lock);
> +       }
> +
> +       if (!got_uidgid)
> +               audit_log_format(ab, " uid=? gid=?");
> +
>         audit_log_end(ab);
>
>  errout:
> --
> 2.35.3

-- 
paul-moore.com