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

Paul Moore
2024-10-09T22:02:55Z

On Wed, Oct 9, 2024 at 4:33 PM Richard Weinberger <richard@nod.at> 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@nod.at>
> ---
> 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@redhat.com>");
> @@ -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@schaufler-ca.com/

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