Re: [PATCH 1/7] audit: implement generic feature setting and retrieving

Steve Grubb
2013-07-08T20:28:18Z

On Friday, May 24, 2013 12:11:44 PM Eric Paris wrote:
> The audit_status structure was not designed with extensibility in mind.
> Define a new AUDIT_SET_FEATURE message type which takes a new structure
> of bits where things can be enabled/disabled/locked one at a time.

This changes how we have been doing things. The way that the audit system
settings have been done is to use the AUDIT_SET and AUDIT_GET commands. It
takes a bit map as the function to perform. We have only used 5 of the 32
bits.

Do we really need another of the same thing?

-Steve

> This
> structure should be able to grow in the future while maintaining forward
> and backward compatibility (based loosly on the ideas from capabilities
> and prctl)
>
> This does not actually add any features, but is just infrastructure to
> allow new on/off types of audit system features.
>
> Signed-off-by: Eric Paris <eparis@redhat.com>
> ---
> include/linux/audit.h | 2 +
> include/uapi/linux/audit.h | 16 +++++++
> kernel/audit.c | 110
> ++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 127
> insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 729a4d1..7b31bec 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -73,6 +73,8 @@ struct audit_field {
> void *lsm_rule;
> };
>
> +extern int is_audit_feature_set(int which);
> +
> extern int __init audit_register_class(int class, unsigned *list);
> extern int audit_classify_syscall(int abi, unsigned syscall);
> extern int audit_classify_arch(int arch);
> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index b7cb978..a053243 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -68,6 +68,9 @@
> #define AUDIT_MAKE_EQUIV 1015 /* Append to watched tree */
> #define AUDIT_TTY_GET 1016 /* Get TTY auditing status */
> #define AUDIT_TTY_SET 1017 /* Set TTY auditing status */
> +#define AUDIT_SET_FEATURE 1018 /* Turn an audit feature on or off */
> +#define AUDIT_GET_FEATURE 1019 /* Get which features are enabled */
> +#define AUDIT_FEATURE_CHANGE 1020 /* audit log listing feature changes
*/
>
> #define AUDIT_FIRST_USER_MSG 1100 /* Userspace messages mostly
> uninteresting to kernel */ #define AUDIT_USER_AVC 1107 /* We filter
this
> differently */
> @@ -369,6 +372,19 @@ struct audit_status {
> __u32 backlog; /* messages waiting in queue */
> };
>
> +struct audit_features {
> +#define AUDIT_FEATURE_VERSION 1
> + __u32 vers;
> + __u32 mask; /* which bits we are dealing with */
> + __u32 features; /* which feature to enable/disable */
> + __u32 lock; /* which features to lock */
> +};
> +
> +#define AUDIT_LAST_FEATURE -1
> +
> +#define audit_feature_valid(x) ((x) >= 0 && (x) <= AUDIT_LAST_FEATURE)
> +#define AUDIT_FEATURE_TO_MASK(x) (1 << ((x) & 31)) /* mask for __u32 */
> +
> struct audit_tty_status {
> __u32 enabled; /* 1 = enabled, 0 = disabled */
> __u32 log_passwd; /* 1 = enabled, 0 = disabled */
> diff --git a/kernel/audit.c b/kernel/audit.c
> index f2f4666..3acbbc8 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -140,6 +140,15 @@ static struct task_struct *kauditd_task;
> static DECLARE_WAIT_QUEUE_HEAD(kauditd_wait);
> static DECLARE_WAIT_QUEUE_HEAD(audit_backlog_wait);
>
> +static struct audit_features af = {.vers = AUDIT_FEATURE_VERSION,
> + .mask = -1,
> + .features = 0,
> + .lock = 0,};
> +
> +static char *audit_feature_names[0] = {
> +};
> +
> +
> /* Serialize requests from userspace. */
> DEFINE_MUTEX(audit_cmd_mutex);
>
> @@ -584,6 +593,8 @@ static int audit_netlink_ok(struct sk_buff *skb, u16
> msg_type) return -EOPNOTSUPP;
> case AUDIT_GET:
> case AUDIT_SET:
> + case AUDIT_GET_FEATURE:
> + case AUDIT_SET_FEATURE:
> case AUDIT_LIST_RULES:
> case AUDIT_ADD_RULE:
> case AUDIT_DEL_RULE:
> @@ -628,6 +639,94 @@ static int audit_log_common_recv_msg(struct
> audit_buffer **ab, u16 msg_type) return rc;
> }
>
> +int is_audit_feature_set(int i)
> +{
> + return af.features & AUDIT_FEATURE_TO_MASK(i);
> +}
> +
> +
> +static int audit_get_feature(struct sk_buff *skb)
> +{
> + u32 seq;
> +
> + seq = nlmsg_hdr(skb)->nlmsg_seq;
> +
> + audit_send_reply(NETLINK_CB(skb).portid, seq, AUDIT_GET, 0, 0,
> + &af, sizeof(af));
> +
> + return 0;
> +}
> +
> +static void audit_log_feature_change(int which, u32 old_feature, u32
> new_feature, + u32 old_lock, u32 new_lock, int res)
> +{
> + struct audit_buffer *ab;
> +
> + ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_FEATURE_CHANGE);
> + audit_log_format(ab, "feature=%s new=%d old=%d old_lock=%d new_lock=%d
> res=%d", + audit_feature_names[which], !!old_feature,
!!new_feature,
> + !!old_lock, !!new_lock, res);
> + audit_log_end(ab);
> +}
> +
> +static int audit_set_feature(struct sk_buff *skb)
> +{
> + struct audit_features *uaf;
> + int i;
> +
> + BUILD_BUG_ON(AUDIT_LAST_FEATURE + 1 >
> sizeof(audit_feature_names)/sizeof(audit_feature_names[0])); + uaf =
> nlmsg_data(nlmsg_hdr(skb));
> +
> + /* if there is ever a version 2 we should handle that here */
> +
> + for (i = 0; i <= AUDIT_LAST_FEATURE; i++) {
> + u32 feature = AUDIT_FEATURE_TO_MASK(i);
> + u32 old_feature, new_feature, old_lock, new_lock;
> +
> + /* if we are not changing this feature, move along */
> + if (!(feature & uaf->mask))
> + continue;
> +
> + old_feature = af.features & feature;
> + new_feature = uaf->features & feature;
> + new_lock = (uaf->lock | af.lock) & feature;
> + old_lock = af.lock & feature;
> +
> + /* are we changing a locked feature? */
> + if ((af.lock & feature) && (new_feature != old_feature)) {
> + audit_log_feature_change(i, old_feature, new_feature,
> + old_lock, new_lock, 0);
> + return -EPERM;
> + }
> + }
> + /* nothing invalid, do the changes */
> + for (i = 0; i <= AUDIT_LAST_FEATURE; i++) {
> + u32 feature = AUDIT_FEATURE_TO_MASK(i);
> + u32 old_feature, new_feature, old_lock, new_lock;
> +
> + /* if we are not changing this feature, move along */
> + if (!(feature & uaf->mask))
> + continue;
> +
> + old_feature = af.features & feature;
> + new_feature = uaf->features & feature;
> + old_lock = af.lock & feature;
> + new_lock = (uaf->lock | af.lock) & feature;
> +
> + if (new_feature != old_feature)
> + audit_log_feature_change(i, old_feature, new_feature,
> + old_lock, new_lock, 1);
> +
> + if (new_feature)
> + af.features |= feature;
> + else
> + af.features &= ~feature;
> + af.lock |= new_lock;
> + }
> +
> + return 0;
> +}
> +
> static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
> {
> u32 seq;
> @@ -699,7 +798,16 @@ static int audit_receive_msg(struct sk_buff *skb,
> struct nlmsghdr *nlh) if (status_get->mask & AUDIT_STATUS_BACKLOG_LIMIT)
> err = audit_set_backlog_limit(status_get->backlog_limit);
> break;
> - case AUDIT_USER:
> + case AUDIT_GET_FEATURE:
> + err = audit_get_feature(skb);
> + if (err)
> + return err;
> + break;
> + case AUDIT_SET_FEATURE:
> + err = audit_set_feature(skb);
> + if (err)
> + return err;
> + break;
> case AUDIT_FIRST_USER_MSG ... AUDIT_LAST_USER_MSG:
> case AUDIT_FIRST_USER_MSG2 ... AUDIT_LAST_USER_MSG2:
> if (!audit_enabled && msg_type != AUDIT_USER_AVC)