* Re: [PATCH 1/1] NetLabel: add audit support for configuration changes
@ 2006-09-29 14:08 ` Steve Grubb
0 siblings, 0 replies; 24+ messages in thread
From: Steve Grubb @ 2006-09-29 14:08 UTC (permalink / raw)
To: linux-audit; +Cc: paul.moore, netdev, selinux
On Thursday 28 September 2006 14:03, paul.moore@hp.com wrote:
> This patch adds audit support to NetLabel, including six new audit message
> types shown below.
>
> #define AUDIT_MAC_UNLBL_ACCEPT 1406
> #define AUDIT_MAC_UNLBL_DENY 1407
> #define AUDIT_MAC_CIPSOV4_ADD 1408
> #define AUDIT_MAC_CIPSOV4_DEL 1409
> #define AUDIT_MAC_MAP_ADD 1410
> #define AUDIT_MAC_MAP_DEL 1411
>
> Please consider this for inclusion into 2.6.19.
>
> Signed-off-by: Paul Moore <paul.moore@hp.com>
> ---
> include/linux/audit.h | 6 ++
> include/net/cipso_ipv4.h | 5 +-
> include/net/netlabel.h | 2
> net/ipv4/cipso_ipv4.c | 8 ++-
> net/netlabel/netlabel_cipso_v4.c | 43 +++++++++++++----
> net/netlabel/netlabel_domainhash.c | 54 +++++++++++++++++++--
> net/netlabel/netlabel_domainhash.h | 6 +-
> net/netlabel/netlabel_mgmt.c | 14 +++--
> net/netlabel/netlabel_unlabeled.c | 36 ++++++++++++--
> net/netlabel/netlabel_user.c | 91
> +++++++++++++++++++++++++++++++++++++ net/netlabel/netlabel_user.h |
> 6 ++
> 11 files changed, 235 insertions(+), 36 deletions(-)
>
> Index: net-2.6/include/linux/audit.h
> ===================================================================
> --- net-2.6.orig/include/linux/audit.h
> +++ net-2.6/include/linux/audit.h
> @@ -95,6 +95,12 @@
> #define AUDIT_MAC_POLICY_LOAD 1403 /* Policy file load */
> #define AUDIT_MAC_STATUS 1404 /* Changed enforcing,permissive,off */
> #define AUDIT_MAC_CONFIG_CHANGE 1405 /* Changes to booleans */
> +#define AUDIT_MAC_UNLBL_ACCEPT 1406 /* NetLabel: allow unlabeled traffic */
> +#define AUDIT_MAC_UNLBL_DENY 1407 /* NetLabel: deny unlabeled traffic */
Please drop the use of DENY per comments later down.
> +#define AUDIT_MAC_CIPSOV4_ADD 1408 /* NetLabel: add CIPSOv4 DOI entry */
> +#define AUDIT_MAC_CIPSOV4_DEL 1409 /* NetLabel: del CIPSOv4 DOI entry */
> +#define AUDIT_MAC_MAP_ADD 1410 /* NetLabel: add LSM domain mapping */
> +#define AUDIT_MAC_MAP_DEL 1411 /* NetLabel: del LSM domain mapping */
>
> #define AUDIT_FIRST_KERN_ANOM_MSG 1700
> #define AUDIT_LAST_KERN_ANOM_MSG 1799
> Index: net-2.6/include/net/cipso_ipv4.h
> ===================================================================
> --- net-2.6.orig/include/net/cipso_ipv4.h
> +++ net-2.6/include/net/cipso_ipv4.h
> @@ -128,7 +128,9 @@ extern int cipso_v4_rbm_strictvalid;
>
> #ifdef CONFIG_NETLABEL
> int cipso_v4_doi_add(struct cipso_v4_doi *doi_def);
> -int cipso_v4_doi_remove(u32 doi, void (*callback) (struct rcu_head *
> head)); +int cipso_v4_doi_remove(u32 doi,
> + u32 audit_secid,
> + void (*callback) (struct rcu_head * head));
> struct cipso_v4_doi *cipso_v4_doi_getdef(u32 doi);
> int cipso_v4_doi_walk(u32 *skip_cnt,
> int (*callback) (struct cipso_v4_doi *doi_def, void *arg),
> @@ -143,6 +145,7 @@ static inline int cipso_v4_doi_add(struc
> }
>
> static inline int cipso_v4_doi_remove(u32 doi,
> + u32 audit_secid,
> void (*callback) (struct rcu_head * head))
> {
> return 0;
> Index: net-2.6/include/net/netlabel.h
> ===================================================================
> --- net-2.6.orig/include/net/netlabel.h
> +++ net-2.6/include/net/netlabel.h
> @@ -96,7 +96,7 @@
> struct netlbl_dom_map;
>
> /* Domain mapping operations */
> -int netlbl_domhsh_remove(const char *domain);
> +int netlbl_domhsh_remove(const char *domain, u32 audit_secid);
>
> /* LSM security attributes */
> struct netlbl_lsm_cache {
> Index: net-2.6/net/ipv4/cipso_ipv4.c
> ===================================================================
> --- net-2.6.orig/net/ipv4/cipso_ipv4.c
> +++ net-2.6/net/ipv4/cipso_ipv4.c
> @@ -474,6 +474,7 @@ doi_add_failure_rlock:
> /**
> * cipso_v4_doi_remove - Remove an existing DOI from the CIPSO protocol
> engine * @doi: the DOI value
> + * @audit_secid: the LSM secid to use in the audit message
> * @callback: the DOI cleanup/free callback
> *
> * Description:
> @@ -483,7 +484,9 @@ doi_add_failure_rlock:
> * success and negative values on failure.
> *
> */
> -int cipso_v4_doi_remove(u32 doi, void (*callback) (struct rcu_head *
> head)) +int cipso_v4_doi_remove(u32 doi,
> + u32 audit_secid,
> + void (*callback) (struct rcu_head * head))
> {
> struct cipso_v4_doi *doi_def;
> struct cipso_v4_domhsh_entry *dom_iter;
> @@ -502,7 +505,8 @@ int cipso_v4_doi_remove(u32 doi, void (*
> spin_unlock(&cipso_v4_doi_list_lock);
> list_for_each_entry_rcu(dom_iter, &doi_def->dom_list, list)
> if (dom_iter->valid)
> - netlbl_domhsh_remove(dom_iter->domain);
> + netlbl_domhsh_remove(dom_iter->domain,
> + audit_secid);
> cipso_v4_cache_invalidate();
> rcu_read_unlock();
>
> Index: net-2.6/net/netlabel/netlabel_cipso_v4.c
> ===================================================================
> --- net-2.6.orig/net/netlabel/netlabel_cipso_v4.c
> +++ net-2.6/net/netlabel/netlabel_cipso_v4.c
> @@ -32,6 +32,7 @@
> #include <linux/socket.h>
> #include <linux/string.h>
> #include <linux/skbuff.h>
> +#include <linux/audit.h>
> #include <net/sock.h>
> #include <net/netlink.h>
> #include <net/genetlink.h>
> @@ -162,8 +163,7 @@ static int netlbl_cipsov4_add_std(struct
> int nla_a_rem;
> int nla_b_rem;
>
> - if (!info->attrs[NLBL_CIPSOV4_A_DOI] ||
> - !info->attrs[NLBL_CIPSOV4_A_TAGLST] ||
> + if (!info->attrs[NLBL_CIPSOV4_A_TAGLST] ||
> !info->attrs[NLBL_CIPSOV4_A_MLSLVLLST])
> return -EINVAL;
>
> @@ -344,8 +344,7 @@ static int netlbl_cipsov4_add_pass(struc
> int ret_val;
> struct cipso_v4_doi *doi_def = NULL;
>
> - if (!info->attrs[NLBL_CIPSOV4_A_DOI] ||
> - !info->attrs[NLBL_CIPSOV4_A_TAGLST])
> + if (!info->attrs[NLBL_CIPSOV4_A_TAGLST])
> return -EINVAL;
>
> doi_def = kmalloc(sizeof(*doi_def), GFP_KERNEL);
> @@ -381,21 +380,35 @@ static int netlbl_cipsov4_add(struct sk_
>
> {
> int ret_val = -EINVAL;
> - u32 map_type;
> + u32 type;
> + u32 doi;
> + const char *type_str = "(unknown)";
> + struct audit_buffer *audit_buf;
>
> - if (!info->attrs[NLBL_CIPSOV4_A_MTYPE])
> + if (!info->attrs[NLBL_CIPSOV4_A_DOI] ||
> + !info->attrs[NLBL_CIPSOV4_A_MTYPE])
> return -EINVAL;
>
> - map_type = nla_get_u32(info->attrs[NLBL_CIPSOV4_A_MTYPE]);
> - switch (map_type) {
> + type = nla_get_u32(info->attrs[NLBL_CIPSOV4_A_MTYPE]);
> + switch (type) {
> case CIPSO_V4_MAP_STD:
> + type_str = "std";
> ret_val = netlbl_cipsov4_add_std(info);
> break;
> case CIPSO_V4_MAP_PASS:
> + type_str = "pass";
> ret_val = netlbl_cipsov4_add_pass(info);
> break;
> }
>
> + if (ret_val == 0) {
> + doi = nla_get_u32(info->attrs[NLBL_CIPSOV4_A_DOI]);
> + audit_buf = netlbl_audit_start_common(AUDIT_MAC_CIPSOV4_ADD,
> + NETLINK_CB(skb).sid);
> + audit_log_format(audit_buf, " doi=%u type=%s", doi, type_str);
type field is already taken for another purpose, it needs to be renamed.
> + audit_log_end(audit_buf);
> + }
Normally, we have a field res= that records whether or not the user was
successful in removing the rule. You would probably do something like
ret_val==0?1:0 but send a audit event regardless
> +
> return ret_val;
> }
>
> @@ -653,11 +666,21 @@ static int netlbl_cipsov4_listall(struct
> static int netlbl_cipsov4_remove(struct sk_buff *skb, struct genl_info
> *info) {
> int ret_val = -EINVAL;
> - u32 doi;
> + u32 doi = 0;
> + struct audit_buffer *audit_buf;
>
> if (info->attrs[NLBL_CIPSOV4_A_DOI]) {
> doi = nla_get_u32(info->attrs[NLBL_CIPSOV4_A_DOI]);
> - ret_val = cipso_v4_doi_remove(doi, netlbl_cipsov4_doi_free);
> + ret_val = cipso_v4_doi_remove(doi,
> + NETLINK_CB(skb).sid,
> + netlbl_cipsov4_doi_free);
> + }
> +
> + if (ret_val == 0) {
> + audit_buf = netlbl_audit_start_common(AUDIT_MAC_CIPSOV4_DEL,
> + NETLINK_CB(skb).sid);
> + audit_log_format(audit_buf, " doi=%u", doi);
> + audit_log_end(audit_buf);
> }
Same as above, global comment about res= being recorded no matter what.
>
> return ret_val;
> Index: net-2.6/net/netlabel/netlabel_domainhash.c
> ===================================================================
> --- net-2.6.orig/net/netlabel/netlabel_domainhash.c
> +++ net-2.6/net/netlabel/netlabel_domainhash.c
> @@ -35,12 +35,14 @@
> #include <linux/skbuff.h>
> #include <linux/spinlock.h>
> #include <linux/string.h>
> +#include <linux/audit.h>
> #include <net/netlabel.h>
> #include <net/cipso_ipv4.h>
> #include <asm/bug.h>
>
> #include "netlabel_mgmt.h"
> #include "netlabel_domainhash.h"
> +#include "netlabel_user.h"
>
> struct netlbl_domhsh_tbl {
> struct list_head *tbl;
> @@ -186,6 +188,7 @@ int netlbl_domhsh_init(u32 size)
> /**
> * netlbl_domhsh_add - Adds a entry to the domain hash table
> * @entry: the entry to add
> + * @audit_secid: the LSM secid to use in the audit message
> *
> * Description:
> * Adds a new entry to the domain hash table and handles any updates to
> the @@ -193,10 +196,12 @@ int netlbl_domhsh_init(u32 size)
> * negative on failure.
> *
> */
> -int netlbl_domhsh_add(struct netlbl_dom_map *entry)
> +int netlbl_domhsh_add(struct netlbl_dom_map *entry, u32 audit_secid)
> {
> int ret_val;
> u32 bkt;
> + struct audit_buffer *audit_buf;
> + char *audit_domain;
>
> switch (entry->type) {
> case NETLBL_NLTYPE_UNLABELED:
> @@ -236,6 +241,26 @@ int netlbl_domhsh_add(struct netlbl_dom_
> spin_unlock(&netlbl_domhsh_def_lock);
> } else
> ret_val = -EINVAL;
> + if (ret_val == 0) {
> + if (entry->domain != NULL)
> + audit_domain = entry->domain;
> + else
> + audit_domain = "(default)";
> + audit_buf = netlbl_audit_start_common(AUDIT_MAC_MAP_ADD,
> + audit_secid);
> + audit_log_format(audit_buf, " domain=%s", audit_domain);
> + switch (entry->type) {
> + case NETLBL_NLTYPE_UNLABELED:
> + audit_log_format(audit_buf, " protocol=unlbl");
> + break;
> + case NETLBL_NLTYPE_CIPSOV4:
> + audit_log_format(audit_buf,
> + " protocol=cipsov4 doi=%u",
> + entry->type_def.cipsov4->doi);
> + break;
> + }
> + audit_log_end(audit_buf);
> + }
> rcu_read_unlock();
>
> if (ret_val != 0) {
> @@ -254,6 +279,7 @@ int netlbl_domhsh_add(struct netlbl_dom_
> /**
> * netlbl_domhsh_add_default - Adds the default entry to the domain hash
> table * @entry: the entry to add
> + * @audit_secid: the LSM secid to use in the audit message
> *
> * Description:
> * Adds a new default entry to the domain hash table and handles any
> updates @@ -261,14 +287,15 @@ int netlbl_domhsh_add(struct netlbl_dom_
> * negative on failure.
> *
> */
> -int netlbl_domhsh_add_default(struct netlbl_dom_map *entry)
> +int netlbl_domhsh_add_default(struct netlbl_dom_map *entry, u32
> audit_secid) {
> - return netlbl_domhsh_add(entry);
> + return netlbl_domhsh_add(entry, audit_secid);
> }
>
> /**
> * netlbl_domhsh_remove - Removes an entry from the domain hash table
> * @domain: the domain to remove
> + * @audit_secid: the LSM secid to use in the audit message
> *
> * Description:
> * Removes an entry from the domain hash table and handles any updates to
> the @@ -276,10 +303,12 @@ int netlbl_domhsh_add_default(struct net
> * negative on failure.
> *
> */
> -int netlbl_domhsh_remove(const char *domain)
> +int netlbl_domhsh_remove(const char *domain, u32 audit_secid)
> {
> int ret_val = -ENOENT;
> struct netlbl_dom_map *entry;
> + struct audit_buffer *audit_buf;
> + char *audit_domain;
>
> rcu_read_lock();
> if (domain != NULL)
> @@ -316,8 +345,18 @@ int netlbl_domhsh_remove(const char *dom
> ret_val = -ENOENT;
> spin_unlock(&netlbl_domhsh_def_lock);
> }
> - if (ret_val == 0)
> + if (ret_val == 0) {
> + if (entry->domain != NULL)
> + audit_domain = entry->domain;
> + else
> + audit_domain = "(default)";
> + audit_buf = netlbl_audit_start_common(AUDIT_MAC_MAP_DEL,
> + audit_secid);
> + audit_log_format(audit_buf, " domain=%s", audit_domain);
> + audit_log_end(audit_buf);
> +
> call_rcu(&entry->rcu, netlbl_domhsh_free_entry);
> + }
>
> remove_return:
> rcu_read_unlock();
> @@ -326,6 +365,7 @@ remove_return:
>
> /**
> * netlbl_domhsh_remove_default - Removes the default entry from the table
> + * @audit_secid: the LSM secid to use in the audit message
> *
> * Description:
> * Removes/resets the default entry for the domain hash table and handles
> any @@ -333,9 +373,9 @@ remove_return:
> * success, non-zero on failure.
> *
> */
> -int netlbl_domhsh_remove_default(void)
> +int netlbl_domhsh_remove_default(u32 audit_secid)
> {
> - return netlbl_domhsh_remove(NULL);
> + return netlbl_domhsh_remove(NULL, audit_secid);
> }
>
> /**
> Index: net-2.6/net/netlabel/netlabel_domainhash.h
> ===================================================================
> --- net-2.6.orig/net/netlabel/netlabel_domainhash.h
> +++ net-2.6/net/netlabel/netlabel_domainhash.h
> @@ -57,9 +57,9 @@ struct netlbl_dom_map {
> int netlbl_domhsh_init(u32 size);
>
> /* Manipulate the domain hash table */
> -int netlbl_domhsh_add(struct netlbl_dom_map *entry);
> -int netlbl_domhsh_add_default(struct netlbl_dom_map *entry);
> -int netlbl_domhsh_remove_default(void);
> +int netlbl_domhsh_add(struct netlbl_dom_map *entry, u32 audit_secid);
> +int netlbl_domhsh_add_default(struct netlbl_dom_map *entry, u32
> audit_secid); +int netlbl_domhsh_remove_default(u32 audit_secid);
> struct netlbl_dom_map *netlbl_domhsh_getentry(const char *domain);
> int netlbl_domhsh_walk(u32 *skip_bkt,
> u32 *skip_chain,
> Index: net-2.6/net/netlabel/netlabel_mgmt.c
> ===================================================================
> --- net-2.6.orig/net/netlabel/netlabel_mgmt.c
> +++ net-2.6/net/netlabel/netlabel_mgmt.c
> @@ -108,7 +108,7 @@ static int netlbl_mgmt_add(struct sk_buf
>
> switch (entry->type) {
> case NETLBL_NLTYPE_UNLABELED:
> - ret_val = netlbl_domhsh_add(entry);
> + ret_val = netlbl_domhsh_add(entry, NETLINK_CB(skb).sid);
> break;
> case NETLBL_NLTYPE_CIPSOV4:
> if (!info->attrs[NLBL_MGMT_A_CV4DOI])
> @@ -125,7 +125,7 @@ static int netlbl_mgmt_add(struct sk_buf
> rcu_read_unlock();
> goto add_failure;
> }
> - ret_val = netlbl_domhsh_add(entry);
> + ret_val = netlbl_domhsh_add(entry, NETLINK_CB(skb).sid);
> rcu_read_unlock();
> break;
> default:
> @@ -161,7 +161,7 @@ static int netlbl_mgmt_remove(struct sk_
> return -EINVAL;
>
> domain = nla_data(info->attrs[NLBL_MGMT_A_DOMAIN]);
> - return netlbl_domhsh_remove(domain);
> + return netlbl_domhsh_remove(domain, NETLINK_CB(skb).sid);
> }
>
> /**
> @@ -277,7 +277,8 @@ static int netlbl_mgmt_adddef(struct sk_
>
> switch (entry->type) {
> case NETLBL_NLTYPE_UNLABELED:
> - ret_val = netlbl_domhsh_add_default(entry);
> + ret_val = netlbl_domhsh_add_default(entry,
> + NETLINK_CB(skb).sid);
> break;
> case NETLBL_NLTYPE_CIPSOV4:
> if (!info->attrs[NLBL_MGMT_A_CV4DOI])
> @@ -294,7 +295,8 @@ static int netlbl_mgmt_adddef(struct sk_
> rcu_read_unlock();
> goto adddef_failure;
> }
> - ret_val = netlbl_domhsh_add_default(entry);
> + ret_val = netlbl_domhsh_add_default(entry,
> + NETLINK_CB(skb).sid);
> rcu_read_unlock();
> break;
> default:
> @@ -322,7 +324,7 @@ adddef_failure:
> */
> static int netlbl_mgmt_removedef(struct sk_buff *skb, struct genl_info
> *info) {
> - return netlbl_domhsh_remove_default();
> + return netlbl_domhsh_remove_default(NETLINK_CB(skb).sid);
> }
>
> /**
> Index: net-2.6/net/netlabel/netlabel_unlabeled.c
> ===================================================================
> --- net-2.6.orig/net/netlabel/netlabel_unlabeled.c
> +++ net-2.6/net/netlabel/netlabel_unlabeled.c
> @@ -64,6 +64,27 @@ static struct nla_policy netlbl_unlabel_
> };
>
> /*
> + * Helper Functions
> + */
> +
> +/**
> + * netlbl_unlabel_acceptflg_set - Set the unlabeled accept flag
> + * @value: desired value
> + * @audit_secid: the LSM secid to use in the audit message
> + *
> + * Description:
> + * Set the value of the unlabeled accept flag to @value.
> + *
> + */
> +static void netlbl_unlabel_acceptflg_set(u8 value, u32 audit_secid)
> +{
> + atomic_set(&netlabel_unlabel_accept_flg, value);
> + netlbl_audit_nomsg((value ?
> + AUDIT_MAC_UNLBL_ACCEPT : AUDIT_MAC_UNLBL_DENY),
> + audit_secid);
Looking at how this is being used, I think only 1 message type should be used.
There are places in the audit system where we set a flag to 1 or 0, but only
have 1 message type. We record the old and new value. So, you'd need to pass
that to the logger.
> +}
> +
> +/*
> * NetLabel Command Handlers
> */
>
> @@ -79,18 +100,18 @@ static struct nla_policy netlbl_unlabel_
> */
> static int netlbl_unlabel_accept(struct sk_buff *skb, struct genl_info
> *info) {
> - int ret_val = -EINVAL;
> u8 value;
>
> if (info->attrs[NLBL_UNLABEL_A_ACPTFLG]) {
> value = nla_get_u8(info->attrs[NLBL_UNLABEL_A_ACPTFLG]);
> if (value == 1 || value == 0) {
> - atomic_set(&netlabel_unlabel_accept_flg, value);
> - ret_val = 0;
> + netlbl_unlabel_acceptflg_set(value,
> + NETLINK_CB(skb).sid);
> + return 0;
> }
> }
>
> - return ret_val;
> + return -EINVAL;
> }
>
> /**
> @@ -229,16 +250,19 @@ int netlbl_unlabel_defconf(void)
> {
> int ret_val;
> struct netlbl_dom_map *entry;
> + u32 secid;
> +
> + security_task_getsecid(current, &secid);
>
> entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> if (entry == NULL)
> return -ENOMEM;
> entry->type = NETLBL_NLTYPE_UNLABELED;
> - ret_val = netlbl_domhsh_add_default(entry);
> + ret_val = netlbl_domhsh_add_default(entry, secid);
> if (ret_val != 0)
> return ret_val;
>
> - atomic_set(&netlabel_unlabel_accept_flg, 1);
> + netlbl_unlabel_acceptflg_set(1, secid);
>
> return 0;
> }
> Index: net-2.6/net/netlabel/netlabel_user.c
> ===================================================================
> --- net-2.6.orig/net/netlabel/netlabel_user.c
> +++ net-2.6/net/netlabel/netlabel_user.c
> @@ -32,6 +32,9 @@
> #include <linux/types.h>
> #include <linux/list.h>
> #include <linux/socket.h>
> +#include <linux/audit.h>
> +#include <linux/tty.h>
> +#include <linux/security.h>
> #include <net/sock.h>
> #include <net/netlink.h>
> #include <net/genetlink.h>
> @@ -74,3 +77,91 @@ int netlbl_netlink_init(void)
>
> return 0;
> }
> +
> +/*
> + * NetLabel Audit Functions
> + */
> +
> +/**
> + * netlbl_audit_start_common - Start an audit message
> + * @type: audit message type
> + * @secid: LSM context ID
> + *
> + * Description:
> + * Start an audit message using the type specified in @type and fill the
> audit + * message with some fields common to all NetLabel audit messages.
> Returns + * a pointer to the audit buffer on success, NULL on failure.
> + *
> + */
> +struct audit_buffer *netlbl_audit_start_common(int type, u32 secid)
> +{
Generally, logging functions are moved into auditsc.c where the context and
other functions are defined.
> + struct audit_context *audit_ctx = current->audit_context;
> + struct audit_buffer *audit_buf;
> + uid_t audit_loginuid;
> + const char *audit_tty;
> + char audit_comm[sizeof(current->comm)];
> + struct vm_area_struct *vma;
> + char *secctx;
> + u32 secctx_len;
> +
> + audit_buf = audit_log_start(audit_ctx, GFP_ATOMIC, type);
> + if (audit_buf == NULL)
> + return NULL;
> +
> + audit_loginuid = audit_get_loginuid(audit_ctx);
Netlink is async protocol, you should use the loginuid of the netlink packet
and pass it into this function.
> + if (current->signal &&
> + current->signal->tty &&
> + current->signal->tty->name)
> + audit_tty = current->signal->tty->name;
> + else
> + audit_tty = "(none)";
Netlink is an async protocol. How do you know that the sender is current? This
is a global comment, please check use of current everywhere.
> + get_task_comm(audit_comm, current);
> +
> + audit_log_format(audit_buf,
> + "netlabel: auid=%u uid=%u tty=%s pid=%d",
> + audit_loginuid,
> + current->uid,
> + audit_tty,
> + current->pid);
Why are you logging all this? When we add audit rules, all that we log is the
auid, and subj. If we need to log all this, we should probably have a helper
function that gets called by other config change loggers.
> + audit_log_format(audit_buf, " comm=");
> + audit_log_untrustedstring(audit_buf, audit_comm);
> + if (current->mm) {
> + down_read(¤t->mm->mmap_sem);
> + vma = current->mm->mmap;
> + while (vma) {
> + if ((vma->vm_flags & VM_EXECUTABLE) &&
> + vma->vm_file) {
> + audit_log_d_path(audit_buf,
> + " exe=",
> + vma->vm_file->f_dentry,
> + vma->vm_file->f_vfsmnt);
> + break;
> + }
> + vma = vma->vm_next;
> + }
> + up_read(¤t->mm->mmap_sem);
> + }
> +
If this function was moved inside auditsc.c you could use a function there
that does this. But the question remains why all this data?
> + if (secid != 0 &&
> + security_secid_to_secctx(secid, &secctx, &secctx_len) == 0)
> + audit_log_format(audit_buf, " subj=%s", secctx);
> +
> + return audit_buf;
> +}
> +
> +/**
> + * netlbl_audit_nomsg - Send an audit message without additional text
> + * @type: audit message type
> + * @secid: LSM context ID
> + *
> + * Description:
> + * Send an audit message with only the common NetLabel audit fields.
> + *
> + */
> +void netlbl_audit_nomsg(int type, u32 secid)
> +{
> + struct audit_buffer *audit_buf;
> +
> + audit_buf = netlbl_audit_start_common(type, secid);
> + audit_log_end(audit_buf);
We normally record old and new value when changing flags like this.
> +}
> Index: net-2.6/net/netlabel/netlabel_user.h
> ===================================================================
> --- net-2.6.orig/net/netlabel/netlabel_user.h
> +++ net-2.6/net/netlabel/netlabel_user.h
> @@ -34,6 +34,7 @@
> #include <linux/types.h>
> #include <linux/skbuff.h>
> #include <linux/capability.h>
> +#include <linux/audit.h>
> #include <net/netlink.h>
> #include <net/genetlink.h>
> #include <net/netlabel.h>
> @@ -75,4 +76,9 @@ static inline void *netlbl_netlink_hdr_p
>
> int netlbl_netlink_init(void);
>
> +/* NetLabel Audit Functions */
> +
> +struct audit_buffer *netlbl_audit_start_common(int type, u32 secid);
> +void netlbl_audit_nomsg(int type, u32 secid);
> +
> #endif
>
> --
> paul moore
> linux security @ hp
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 1/1] NetLabel: add audit support for configuration changes
2006-09-29 14:08 ` Steve Grubb
@ 2006-09-29 14:13 ` James Morris
-1 siblings, 0 replies; 24+ messages in thread
From: James Morris @ 2006-09-29 14:13 UTC (permalink / raw)
To: Steve Grubb; +Cc: netdev, linux-audit, David S. Miller, selinux
Dave,
Looks like this patch needs to be reverted until these issues are
resolved.
On Fri, 29 Sep 2006, Steve Grubb wrote:
> On Thursday 28 September 2006 14:03, paul.moore@hp.com wrote:
> > This patch adds audit support to NetLabel, including six new audit message
> > types shown below.
> >
> > #define AUDIT_MAC_UNLBL_ACCEPT 1406
> > #define AUDIT_MAC_UNLBL_DENY 1407
> > #define AUDIT_MAC_CIPSOV4_ADD 1408
> > #define AUDIT_MAC_CIPSOV4_DEL 1409
> > #define AUDIT_MAC_MAP_ADD 1410
> > #define AUDIT_MAC_MAP_DEL 1411
> >
> > Please consider this for inclusion into 2.6.19.
> >
> > Signed-off-by: Paul Moore <paul.moore@hp.com>
> > ---
> > include/linux/audit.h | 6 ++
> > include/net/cipso_ipv4.h | 5 +-
> > include/net/netlabel.h | 2
> > net/ipv4/cipso_ipv4.c | 8 ++-
> > net/netlabel/netlabel_cipso_v4.c | 43 +++++++++++++----
> > net/netlabel/netlabel_domainhash.c | 54 +++++++++++++++++++--
> > net/netlabel/netlabel_domainhash.h | 6 +-
> > net/netlabel/netlabel_mgmt.c | 14 +++--
> > net/netlabel/netlabel_unlabeled.c | 36 ++++++++++++--
> > net/netlabel/netlabel_user.c | 91
> > +++++++++++++++++++++++++++++++++++++ net/netlabel/netlabel_user.h |
> > 6 ++
> > 11 files changed, 235 insertions(+), 36 deletions(-)
> >
> > Index: net-2.6/include/linux/audit.h
> > ===================================================================
> > --- net-2.6.orig/include/linux/audit.h
> > +++ net-2.6/include/linux/audit.h
> > @@ -95,6 +95,12 @@
> > #define AUDIT_MAC_POLICY_LOAD 1403 /* Policy file load */
> > #define AUDIT_MAC_STATUS 1404 /* Changed enforcing,permissive,off */
> > #define AUDIT_MAC_CONFIG_CHANGE 1405 /* Changes to booleans */
> > +#define AUDIT_MAC_UNLBL_ACCEPT 1406 /* NetLabel: allow unlabeled traffic */
> > +#define AUDIT_MAC_UNLBL_DENY 1407 /* NetLabel: deny unlabeled traffic */
>
> Please drop the use of DENY per comments later down.
>
> > +#define AUDIT_MAC_CIPSOV4_ADD 1408 /* NetLabel: add CIPSOv4 DOI entry */
> > +#define AUDIT_MAC_CIPSOV4_DEL 1409 /* NetLabel: del CIPSOv4 DOI entry */
> > +#define AUDIT_MAC_MAP_ADD 1410 /* NetLabel: add LSM domain mapping */
> > +#define AUDIT_MAC_MAP_DEL 1411 /* NetLabel: del LSM domain mapping */
> >
> > #define AUDIT_FIRST_KERN_ANOM_MSG 1700
> > #define AUDIT_LAST_KERN_ANOM_MSG 1799
> > Index: net-2.6/include/net/cipso_ipv4.h
> > ===================================================================
> > --- net-2.6.orig/include/net/cipso_ipv4.h
> > +++ net-2.6/include/net/cipso_ipv4.h
> > @@ -128,7 +128,9 @@ extern int cipso_v4_rbm_strictvalid;
> >
> > #ifdef CONFIG_NETLABEL
> > int cipso_v4_doi_add(struct cipso_v4_doi *doi_def);
> > -int cipso_v4_doi_remove(u32 doi, void (*callback) (struct rcu_head *
> > head)); +int cipso_v4_doi_remove(u32 doi,
> > + u32 audit_secid,
> > + void (*callback) (struct rcu_head * head));
> > struct cipso_v4_doi *cipso_v4_doi_getdef(u32 doi);
> > int cipso_v4_doi_walk(u32 *skip_cnt,
> > int (*callback) (struct cipso_v4_doi *doi_def, void *arg),
> > @@ -143,6 +145,7 @@ static inline int cipso_v4_doi_add(struc
> > }
> >
> > static inline int cipso_v4_doi_remove(u32 doi,
> > + u32 audit_secid,
> > void (*callback) (struct rcu_head * head))
> > {
> > return 0;
> > Index: net-2.6/include/net/netlabel.h
> > ===================================================================
> > --- net-2.6.orig/include/net/netlabel.h
> > +++ net-2.6/include/net/netlabel.h
> > @@ -96,7 +96,7 @@
> > struct netlbl_dom_map;
> >
> > /* Domain mapping operations */
> > -int netlbl_domhsh_remove(const char *domain);
> > +int netlbl_domhsh_remove(const char *domain, u32 audit_secid);
> >
> > /* LSM security attributes */
> > struct netlbl_lsm_cache {
> > Index: net-2.6/net/ipv4/cipso_ipv4.c
> > ===================================================================
> > --- net-2.6.orig/net/ipv4/cipso_ipv4.c
> > +++ net-2.6/net/ipv4/cipso_ipv4.c
> > @@ -474,6 +474,7 @@ doi_add_failure_rlock:
> > /**
> > * cipso_v4_doi_remove - Remove an existing DOI from the CIPSO protocol
> > engine * @doi: the DOI value
> > + * @audit_secid: the LSM secid to use in the audit message
> > * @callback: the DOI cleanup/free callback
> > *
> > * Description:
> > @@ -483,7 +484,9 @@ doi_add_failure_rlock:
> > * success and negative values on failure.
> > *
> > */
> > -int cipso_v4_doi_remove(u32 doi, void (*callback) (struct rcu_head *
> > head)) +int cipso_v4_doi_remove(u32 doi,
> > + u32 audit_secid,
> > + void (*callback) (struct rcu_head * head))
> > {
> > struct cipso_v4_doi *doi_def;
> > struct cipso_v4_domhsh_entry *dom_iter;
> > @@ -502,7 +505,8 @@ int cipso_v4_doi_remove(u32 doi, void (*
> > spin_unlock(&cipso_v4_doi_list_lock);
> > list_for_each_entry_rcu(dom_iter, &doi_def->dom_list, list)
> > if (dom_iter->valid)
> > - netlbl_domhsh_remove(dom_iter->domain);
> > + netlbl_domhsh_remove(dom_iter->domain,
> > + audit_secid);
> > cipso_v4_cache_invalidate();
> > rcu_read_unlock();
> >
> > Index: net-2.6/net/netlabel/netlabel_cipso_v4.c
> > ===================================================================
> > --- net-2.6.orig/net/netlabel/netlabel_cipso_v4.c
> > +++ net-2.6/net/netlabel/netlabel_cipso_v4.c
> > @@ -32,6 +32,7 @@
> > #include <linux/socket.h>
> > #include <linux/string.h>
> > #include <linux/skbuff.h>
> > +#include <linux/audit.h>
> > #include <net/sock.h>
> > #include <net/netlink.h>
> > #include <net/genetlink.h>
> > @@ -162,8 +163,7 @@ static int netlbl_cipsov4_add_std(struct
> > int nla_a_rem;
> > int nla_b_rem;
> >
> > - if (!info->attrs[NLBL_CIPSOV4_A_DOI] ||
> > - !info->attrs[NLBL_CIPSOV4_A_TAGLST] ||
> > + if (!info->attrs[NLBL_CIPSOV4_A_TAGLST] ||
> > !info->attrs[NLBL_CIPSOV4_A_MLSLVLLST])
> > return -EINVAL;
> >
> > @@ -344,8 +344,7 @@ static int netlbl_cipsov4_add_pass(struc
> > int ret_val;
> > struct cipso_v4_doi *doi_def = NULL;
> >
> > - if (!info->attrs[NLBL_CIPSOV4_A_DOI] ||
> > - !info->attrs[NLBL_CIPSOV4_A_TAGLST])
> > + if (!info->attrs[NLBL_CIPSOV4_A_TAGLST])
> > return -EINVAL;
> >
> > doi_def = kmalloc(sizeof(*doi_def), GFP_KERNEL);
> > @@ -381,21 +380,35 @@ static int netlbl_cipsov4_add(struct sk_
> >
> > {
> > int ret_val = -EINVAL;
> > - u32 map_type;
> > + u32 type;
> > + u32 doi;
> > + const char *type_str = "(unknown)";
> > + struct audit_buffer *audit_buf;
> >
> > - if (!info->attrs[NLBL_CIPSOV4_A_MTYPE])
> > + if (!info->attrs[NLBL_CIPSOV4_A_DOI] ||
> > + !info->attrs[NLBL_CIPSOV4_A_MTYPE])
> > return -EINVAL;
> >
> > - map_type = nla_get_u32(info->attrs[NLBL_CIPSOV4_A_MTYPE]);
> > - switch (map_type) {
> > + type = nla_get_u32(info->attrs[NLBL_CIPSOV4_A_MTYPE]);
> > + switch (type) {
> > case CIPSO_V4_MAP_STD:
> > + type_str = "std";
> > ret_val = netlbl_cipsov4_add_std(info);
> > break;
> > case CIPSO_V4_MAP_PASS:
> > + type_str = "pass";
> > ret_val = netlbl_cipsov4_add_pass(info);
> > break;
> > }
> >
> > + if (ret_val == 0) {
> > + doi = nla_get_u32(info->attrs[NLBL_CIPSOV4_A_DOI]);
> > + audit_buf = netlbl_audit_start_common(AUDIT_MAC_CIPSOV4_ADD,
> > + NETLINK_CB(skb).sid);
> > + audit_log_format(audit_buf, " doi=%u type=%s", doi, type_str);
>
> type field is already taken for another purpose, it needs to be renamed.
>
> > + audit_log_end(audit_buf);
> > + }
>
> Normally, we have a field res= that records whether or not the user was
> successful in removing the rule. You would probably do something like
> ret_val==0?1:0 but send a audit event regardless
>
> > +
> > return ret_val;
> > }
> >
> > @@ -653,11 +666,21 @@ static int netlbl_cipsov4_listall(struct
> > static int netlbl_cipsov4_remove(struct sk_buff *skb, struct genl_info
> > *info) {
> > int ret_val = -EINVAL;
> > - u32 doi;
> > + u32 doi = 0;
> > + struct audit_buffer *audit_buf;
> >
> > if (info->attrs[NLBL_CIPSOV4_A_DOI]) {
> > doi = nla_get_u32(info->attrs[NLBL_CIPSOV4_A_DOI]);
> > - ret_val = cipso_v4_doi_remove(doi, netlbl_cipsov4_doi_free);
> > + ret_val = cipso_v4_doi_remove(doi,
> > + NETLINK_CB(skb).sid,
> > + netlbl_cipsov4_doi_free);
> > + }
> > +
> > + if (ret_val == 0) {
> > + audit_buf = netlbl_audit_start_common(AUDIT_MAC_CIPSOV4_DEL,
> > + NETLINK_CB(skb).sid);
> > + audit_log_format(audit_buf, " doi=%u", doi);
> > + audit_log_end(audit_buf);
> > }
>
> Same as above, global comment about res= being recorded no matter what.
>
> >
> > return ret_val;
> > Index: net-2.6/net/netlabel/netlabel_domainhash.c
> > ===================================================================
> > --- net-2.6.orig/net/netlabel/netlabel_domainhash.c
> > +++ net-2.6/net/netlabel/netlabel_domainhash.c
> > @@ -35,12 +35,14 @@
> > #include <linux/skbuff.h>
> > #include <linux/spinlock.h>
> > #include <linux/string.h>
> > +#include <linux/audit.h>
> > #include <net/netlabel.h>
> > #include <net/cipso_ipv4.h>
> > #include <asm/bug.h>
> >
> > #include "netlabel_mgmt.h"
> > #include "netlabel_domainhash.h"
> > +#include "netlabel_user.h"
> >
> > struct netlbl_domhsh_tbl {
> > struct list_head *tbl;
> > @@ -186,6 +188,7 @@ int netlbl_domhsh_init(u32 size)
> > /**
> > * netlbl_domhsh_add - Adds a entry to the domain hash table
> > * @entry: the entry to add
> > + * @audit_secid: the LSM secid to use in the audit message
> > *
> > * Description:
> > * Adds a new entry to the domain hash table and handles any updates to
> > the @@ -193,10 +196,12 @@ int netlbl_domhsh_init(u32 size)
> > * negative on failure.
> > *
> > */
> > -int netlbl_domhsh_add(struct netlbl_dom_map *entry)
> > +int netlbl_domhsh_add(struct netlbl_dom_map *entry, u32 audit_secid)
> > {
> > int ret_val;
> > u32 bkt;
> > + struct audit_buffer *audit_buf;
> > + char *audit_domain;
> >
> > switch (entry->type) {
> > case NETLBL_NLTYPE_UNLABELED:
> > @@ -236,6 +241,26 @@ int netlbl_domhsh_add(struct netlbl_dom_
> > spin_unlock(&netlbl_domhsh_def_lock);
> > } else
> > ret_val = -EINVAL;
> > + if (ret_val == 0) {
> > + if (entry->domain != NULL)
> > + audit_domain = entry->domain;
> > + else
> > + audit_domain = "(default)";
> > + audit_buf = netlbl_audit_start_common(AUDIT_MAC_MAP_ADD,
> > + audit_secid);
> > + audit_log_format(audit_buf, " domain=%s", audit_domain);
> > + switch (entry->type) {
> > + case NETLBL_NLTYPE_UNLABELED:
> > + audit_log_format(audit_buf, " protocol=unlbl");
> > + break;
> > + case NETLBL_NLTYPE_CIPSOV4:
> > + audit_log_format(audit_buf,
> > + " protocol=cipsov4 doi=%u",
> > + entry->type_def.cipsov4->doi);
> > + break;
> > + }
> > + audit_log_end(audit_buf);
> > + }
> > rcu_read_unlock();
> >
> > if (ret_val != 0) {
> > @@ -254,6 +279,7 @@ int netlbl_domhsh_add(struct netlbl_dom_
> > /**
> > * netlbl_domhsh_add_default - Adds the default entry to the domain hash
> > table * @entry: the entry to add
> > + * @audit_secid: the LSM secid to use in the audit message
> > *
> > * Description:
> > * Adds a new default entry to the domain hash table and handles any
> > updates @@ -261,14 +287,15 @@ int netlbl_domhsh_add(struct netlbl_dom_
> > * negative on failure.
> > *
> > */
> > -int netlbl_domhsh_add_default(struct netlbl_dom_map *entry)
> > +int netlbl_domhsh_add_default(struct netlbl_dom_map *entry, u32
> > audit_secid) {
> > - return netlbl_domhsh_add(entry);
> > + return netlbl_domhsh_add(entry, audit_secid);
> > }
> >
> > /**
> > * netlbl_domhsh_remove - Removes an entry from the domain hash table
> > * @domain: the domain to remove
> > + * @audit_secid: the LSM secid to use in the audit message
> > *
> > * Description:
> > * Removes an entry from the domain hash table and handles any updates to
> > the @@ -276,10 +303,12 @@ int netlbl_domhsh_add_default(struct net
> > * negative on failure.
> > *
> > */
> > -int netlbl_domhsh_remove(const char *domain)
> > +int netlbl_domhsh_remove(const char *domain, u32 audit_secid)
> > {
> > int ret_val = -ENOENT;
> > struct netlbl_dom_map *entry;
> > + struct audit_buffer *audit_buf;
> > + char *audit_domain;
> >
> > rcu_read_lock();
> > if (domain != NULL)
> > @@ -316,8 +345,18 @@ int netlbl_domhsh_remove(const char *dom
> > ret_val = -ENOENT;
> > spin_unlock(&netlbl_domhsh_def_lock);
> > }
> > - if (ret_val == 0)
> > + if (ret_val == 0) {
> > + if (entry->domain != NULL)
> > + audit_domain = entry->domain;
> > + else
> > + audit_domain = "(default)";
> > + audit_buf = netlbl_audit_start_common(AUDIT_MAC_MAP_DEL,
> > + audit_secid);
> > + audit_log_format(audit_buf, " domain=%s", audit_domain);
> > + audit_log_end(audit_buf);
> > +
> > call_rcu(&entry->rcu, netlbl_domhsh_free_entry);
> > + }
> >
> > remove_return:
> > rcu_read_unlock();
> > @@ -326,6 +365,7 @@ remove_return:
> >
> > /**
> > * netlbl_domhsh_remove_default - Removes the default entry from the table
> > + * @audit_secid: the LSM secid to use in the audit message
> > *
> > * Description:
> > * Removes/resets the default entry for the domain hash table and handles
> > any @@ -333,9 +373,9 @@ remove_return:
> > * success, non-zero on failure.
> > *
> > */
> > -int netlbl_domhsh_remove_default(void)
> > +int netlbl_domhsh_remove_default(u32 audit_secid)
> > {
> > - return netlbl_domhsh_remove(NULL);
> > + return netlbl_domhsh_remove(NULL, audit_secid);
> > }
> >
> > /**
> > Index: net-2.6/net/netlabel/netlabel_domainhash.h
> > ===================================================================
> > --- net-2.6.orig/net/netlabel/netlabel_domainhash.h
> > +++ net-2.6/net/netlabel/netlabel_domainhash.h
> > @@ -57,9 +57,9 @@ struct netlbl_dom_map {
> > int netlbl_domhsh_init(u32 size);
> >
> > /* Manipulate the domain hash table */
> > -int netlbl_domhsh_add(struct netlbl_dom_map *entry);
> > -int netlbl_domhsh_add_default(struct netlbl_dom_map *entry);
> > -int netlbl_domhsh_remove_default(void);
> > +int netlbl_domhsh_add(struct netlbl_dom_map *entry, u32 audit_secid);
> > +int netlbl_domhsh_add_default(struct netlbl_dom_map *entry, u32
> > audit_secid); +int netlbl_domhsh_remove_default(u32 audit_secid);
> > struct netlbl_dom_map *netlbl_domhsh_getentry(const char *domain);
> > int netlbl_domhsh_walk(u32 *skip_bkt,
> > u32 *skip_chain,
> > Index: net-2.6/net/netlabel/netlabel_mgmt.c
> > ===================================================================
> > --- net-2.6.orig/net/netlabel/netlabel_mgmt.c
> > +++ net-2.6/net/netlabel/netlabel_mgmt.c
> > @@ -108,7 +108,7 @@ static int netlbl_mgmt_add(struct sk_buf
> >
> > switch (entry->type) {
> > case NETLBL_NLTYPE_UNLABELED:
> > - ret_val = netlbl_domhsh_add(entry);
> > + ret_val = netlbl_domhsh_add(entry, NETLINK_CB(skb).sid);
> > break;
> > case NETLBL_NLTYPE_CIPSOV4:
> > if (!info->attrs[NLBL_MGMT_A_CV4DOI])
> > @@ -125,7 +125,7 @@ static int netlbl_mgmt_add(struct sk_buf
> > rcu_read_unlock();
> > goto add_failure;
> > }
> > - ret_val = netlbl_domhsh_add(entry);
> > + ret_val = netlbl_domhsh_add(entry, NETLINK_CB(skb).sid);
> > rcu_read_unlock();
> > break;
> > default:
> > @@ -161,7 +161,7 @@ static int netlbl_mgmt_remove(struct sk_
> > return -EINVAL;
> >
> > domain = nla_data(info->attrs[NLBL_MGMT_A_DOMAIN]);
> > - return netlbl_domhsh_remove(domain);
> > + return netlbl_domhsh_remove(domain, NETLINK_CB(skb).sid);
> > }
> >
> > /**
> > @@ -277,7 +277,8 @@ static int netlbl_mgmt_adddef(struct sk_
> >
> > switch (entry->type) {
> > case NETLBL_NLTYPE_UNLABELED:
> > - ret_val = netlbl_domhsh_add_default(entry);
> > + ret_val = netlbl_domhsh_add_default(entry,
> > + NETLINK_CB(skb).sid);
> > break;
> > case NETLBL_NLTYPE_CIPSOV4:
> > if (!info->attrs[NLBL_MGMT_A_CV4DOI])
> > @@ -294,7 +295,8 @@ static int netlbl_mgmt_adddef(struct sk_
> > rcu_read_unlock();
> > goto adddef_failure;
> > }
> > - ret_val = netlbl_domhsh_add_default(entry);
> > + ret_val = netlbl_domhsh_add_default(entry,
> > + NETLINK_CB(skb).sid);
> > rcu_read_unlock();
> > break;
> > default:
> > @@ -322,7 +324,7 @@ adddef_failure:
> > */
> > static int netlbl_mgmt_removedef(struct sk_buff *skb, struct genl_info
> > *info) {
> > - return netlbl_domhsh_remove_default();
> > + return netlbl_domhsh_remove_default(NETLINK_CB(skb).sid);
> > }
> >
> > /**
> > Index: net-2.6/net/netlabel/netlabel_unlabeled.c
> > ===================================================================
> > --- net-2.6.orig/net/netlabel/netlabel_unlabeled.c
> > +++ net-2.6/net/netlabel/netlabel_unlabeled.c
> > @@ -64,6 +64,27 @@ static struct nla_policy netlbl_unlabel_
> > };
> >
> > /*
> > + * Helper Functions
> > + */
> > +
> > +/**
> > + * netlbl_unlabel_acceptflg_set - Set the unlabeled accept flag
> > + * @value: desired value
> > + * @audit_secid: the LSM secid to use in the audit message
> > + *
> > + * Description:
> > + * Set the value of the unlabeled accept flag to @value.
> > + *
> > + */
> > +static void netlbl_unlabel_acceptflg_set(u8 value, u32 audit_secid)
> > +{
> > + atomic_set(&netlabel_unlabel_accept_flg, value);
> > + netlbl_audit_nomsg((value ?
> > + AUDIT_MAC_UNLBL_ACCEPT : AUDIT_MAC_UNLBL_DENY),
> > + audit_secid);
>
> Looking at how this is being used, I think only 1 message type should be used.
> There are places in the audit system where we set a flag to 1 or 0, but only
> have 1 message type. We record the old and new value. So, you'd need to pass
> that to the logger.
>
> > +}
> > +
> > +/*
> > * NetLabel Command Handlers
> > */
> >
> > @@ -79,18 +100,18 @@ static struct nla_policy netlbl_unlabel_
> > */
> > static int netlbl_unlabel_accept(struct sk_buff *skb, struct genl_info
> > *info) {
> > - int ret_val = -EINVAL;
> > u8 value;
> >
> > if (info->attrs[NLBL_UNLABEL_A_ACPTFLG]) {
> > value = nla_get_u8(info->attrs[NLBL_UNLABEL_A_ACPTFLG]);
> > if (value == 1 || value == 0) {
> > - atomic_set(&netlabel_unlabel_accept_flg, value);
> > - ret_val = 0;
> > + netlbl_unlabel_acceptflg_set(value,
> > + NETLINK_CB(skb).sid);
> > + return 0;
> > }
> > }
> >
> > - return ret_val;
> > + return -EINVAL;
> > }
> >
> > /**
> > @@ -229,16 +250,19 @@ int netlbl_unlabel_defconf(void)
> > {
> > int ret_val;
> > struct netlbl_dom_map *entry;
> > + u32 secid;
> > +
> > + security_task_getsecid(current, &secid);
> >
> > entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> > if (entry == NULL)
> > return -ENOMEM;
> > entry->type = NETLBL_NLTYPE_UNLABELED;
> > - ret_val = netlbl_domhsh_add_default(entry);
> > + ret_val = netlbl_domhsh_add_default(entry, secid);
> > if (ret_val != 0)
> > return ret_val;
> >
> > - atomic_set(&netlabel_unlabel_accept_flg, 1);
> > + netlbl_unlabel_acceptflg_set(1, secid);
> >
> > return 0;
> > }
> > Index: net-2.6/net/netlabel/netlabel_user.c
> > ===================================================================
> > --- net-2.6.orig/net/netlabel/netlabel_user.c
> > +++ net-2.6/net/netlabel/netlabel_user.c
> > @@ -32,6 +32,9 @@
> > #include <linux/types.h>
> > #include <linux/list.h>
> > #include <linux/socket.h>
> > +#include <linux/audit.h>
> > +#include <linux/tty.h>
> > +#include <linux/security.h>
> > #include <net/sock.h>
> > #include <net/netlink.h>
> > #include <net/genetlink.h>
> > @@ -74,3 +77,91 @@ int netlbl_netlink_init(void)
> >
> > return 0;
> > }
> > +
> > +/*
> > + * NetLabel Audit Functions
> > + */
> > +
> > +/**
> > + * netlbl_audit_start_common - Start an audit message
> > + * @type: audit message type
> > + * @secid: LSM context ID
> > + *
> > + * Description:
> > + * Start an audit message using the type specified in @type and fill the
> > audit + * message with some fields common to all NetLabel audit messages.
> > Returns + * a pointer to the audit buffer on success, NULL on failure.
> > + *
> > + */
> > +struct audit_buffer *netlbl_audit_start_common(int type, u32 secid)
> > +{
>
> Generally, logging functions are moved into auditsc.c where the context and
> other functions are defined.
>
> > + struct audit_context *audit_ctx = current->audit_context;
> > + struct audit_buffer *audit_buf;
> > + uid_t audit_loginuid;
> > + const char *audit_tty;
> > + char audit_comm[sizeof(current->comm)];
> > + struct vm_area_struct *vma;
> > + char *secctx;
> > + u32 secctx_len;
> > +
> > + audit_buf = audit_log_start(audit_ctx, GFP_ATOMIC, type);
> > + if (audit_buf == NULL)
> > + return NULL;
> > +
> > + audit_loginuid = audit_get_loginuid(audit_ctx);
>
> Netlink is async protocol, you should use the loginuid of the netlink packet
> and pass it into this function.
>
> > + if (current->signal &&
> > + current->signal->tty &&
> > + current->signal->tty->name)
> > + audit_tty = current->signal->tty->name;
> > + else
> > + audit_tty = "(none)";
>
> Netlink is an async protocol. How do you know that the sender is current? This
> is a global comment, please check use of current everywhere.
>
> > + get_task_comm(audit_comm, current);
> > +
> > + audit_log_format(audit_buf,
> > + "netlabel: auid=%u uid=%u tty=%s pid=%d",
> > + audit_loginuid,
> > + current->uid,
> > + audit_tty,
> > + current->pid);
>
> Why are you logging all this? When we add audit rules, all that we log is the
> auid, and subj. If we need to log all this, we should probably have a helper
> function that gets called by other config change loggers.
>
> > + audit_log_format(audit_buf, " comm=");
> > + audit_log_untrustedstring(audit_buf, audit_comm);
> > + if (current->mm) {
> > + down_read(¤t->mm->mmap_sem);
> > + vma = current->mm->mmap;
> > + while (vma) {
> > + if ((vma->vm_flags & VM_EXECUTABLE) &&
> > + vma->vm_file) {
> > + audit_log_d_path(audit_buf,
> > + " exe=",
> > + vma->vm_file->f_dentry,
> > + vma->vm_file->f_vfsmnt);
> > + break;
> > + }
> > + vma = vma->vm_next;
> > + }
> > + up_read(¤t->mm->mmap_sem);
> > + }
> > +
>
> If this function was moved inside auditsc.c you could use a function there
> that does this. But the question remains why all this data?
>
> > + if (secid != 0 &&
> > + security_secid_to_secctx(secid, &secctx, &secctx_len) == 0)
> > + audit_log_format(audit_buf, " subj=%s", secctx);
> > +
> > + return audit_buf;
> > +}
> > +
> > +/**
> > + * netlbl_audit_nomsg - Send an audit message without additional text
> > + * @type: audit message type
> > + * @secid: LSM context ID
> > + *
> > + * Description:
> > + * Send an audit message with only the common NetLabel audit fields.
> > + *
> > + */
> > +void netlbl_audit_nomsg(int type, u32 secid)
> > +{
> > + struct audit_buffer *audit_buf;
> > +
> > + audit_buf = netlbl_audit_start_common(type, secid);
> > + audit_log_end(audit_buf);
>
> We normally record old and new value when changing flags like this.
>
> > +}
> > Index: net-2.6/net/netlabel/netlabel_user.h
> > ===================================================================
> > --- net-2.6.orig/net/netlabel/netlabel_user.h
> > +++ net-2.6/net/netlabel/netlabel_user.h
> > @@ -34,6 +34,7 @@
> > #include <linux/types.h>
> > #include <linux/skbuff.h>
> > #include <linux/capability.h>
> > +#include <linux/audit.h>
> > #include <net/netlink.h>
> > #include <net/genetlink.h>
> > #include <net/netlabel.h>
> > @@ -75,4 +76,9 @@ static inline void *netlbl_netlink_hdr_p
> >
> > int netlbl_netlink_init(void);
> >
> > +/* NetLabel Audit Functions */
> > +
> > +struct audit_buffer *netlbl_audit_start_common(int type, u32 secid);
> > +void netlbl_audit_nomsg(int type, u32 secid);
> > +
> > #endif
> >
> > --
> > paul moore
> > linux security @ hp
> >
> > --
> > Linux-audit mailing list
> > Linux-audit@redhat.com
> > https://www.redhat.com/mailman/listinfo/linux-audit
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 1/1] NetLabel: add audit support for configuration changes
@ 2006-09-29 14:13 ` James Morris
0 siblings, 0 replies; 24+ messages in thread
From: James Morris @ 2006-09-29 14:13 UTC (permalink / raw)
To: Steve Grubb; +Cc: linux-audit, David S. Miller, Paul Moore, netdev, selinux
Dave,
Looks like this patch needs to be reverted until these issues are
resolved.
On Fri, 29 Sep 2006, Steve Grubb wrote:
> On Thursday 28 September 2006 14:03, paul.moore@hp.com wrote:
> > This patch adds audit support to NetLabel, including six new audit message
> > types shown below.
> >
> > #define AUDIT_MAC_UNLBL_ACCEPT 1406
> > #define AUDIT_MAC_UNLBL_DENY 1407
> > #define AUDIT_MAC_CIPSOV4_ADD 1408
> > #define AUDIT_MAC_CIPSOV4_DEL 1409
> > #define AUDIT_MAC_MAP_ADD 1410
> > #define AUDIT_MAC_MAP_DEL 1411
> >
> > Please consider this for inclusion into 2.6.19.
> >
> > Signed-off-by: Paul Moore <paul.moore@hp.com>
> > ---
> > include/linux/audit.h | 6 ++
> > include/net/cipso_ipv4.h | 5 +-
> > include/net/netlabel.h | 2
> > net/ipv4/cipso_ipv4.c | 8 ++-
> > net/netlabel/netlabel_cipso_v4.c | 43 +++++++++++++----
> > net/netlabel/netlabel_domainhash.c | 54 +++++++++++++++++++--
> > net/netlabel/netlabel_domainhash.h | 6 +-
> > net/netlabel/netlabel_mgmt.c | 14 +++--
> > net/netlabel/netlabel_unlabeled.c | 36 ++++++++++++--
> > net/netlabel/netlabel_user.c | 91
> > +++++++++++++++++++++++++++++++++++++ net/netlabel/netlabel_user.h |
> > 6 ++
> > 11 files changed, 235 insertions(+), 36 deletions(-)
> >
> > Index: net-2.6/include/linux/audit.h
> > ===================================================================
> > --- net-2.6.orig/include/linux/audit.h
> > +++ net-2.6/include/linux/audit.h
> > @@ -95,6 +95,12 @@
> > #define AUDIT_MAC_POLICY_LOAD 1403 /* Policy file load */
> > #define AUDIT_MAC_STATUS 1404 /* Changed enforcing,permissive,off */
> > #define AUDIT_MAC_CONFIG_CHANGE 1405 /* Changes to booleans */
> > +#define AUDIT_MAC_UNLBL_ACCEPT 1406 /* NetLabel: allow unlabeled traffic */
> > +#define AUDIT_MAC_UNLBL_DENY 1407 /* NetLabel: deny unlabeled traffic */
>
> Please drop the use of DENY per comments later down.
>
> > +#define AUDIT_MAC_CIPSOV4_ADD 1408 /* NetLabel: add CIPSOv4 DOI entry */
> > +#define AUDIT_MAC_CIPSOV4_DEL 1409 /* NetLabel: del CIPSOv4 DOI entry */
> > +#define AUDIT_MAC_MAP_ADD 1410 /* NetLabel: add LSM domain mapping */
> > +#define AUDIT_MAC_MAP_DEL 1411 /* NetLabel: del LSM domain mapping */
> >
> > #define AUDIT_FIRST_KERN_ANOM_MSG 1700
> > #define AUDIT_LAST_KERN_ANOM_MSG 1799
> > Index: net-2.6/include/net/cipso_ipv4.h
> > ===================================================================
> > --- net-2.6.orig/include/net/cipso_ipv4.h
> > +++ net-2.6/include/net/cipso_ipv4.h
> > @@ -128,7 +128,9 @@ extern int cipso_v4_rbm_strictvalid;
> >
> > #ifdef CONFIG_NETLABEL
> > int cipso_v4_doi_add(struct cipso_v4_doi *doi_def);
> > -int cipso_v4_doi_remove(u32 doi, void (*callback) (struct rcu_head *
> > head)); +int cipso_v4_doi_remove(u32 doi,
> > + u32 audit_secid,
> > + void (*callback) (struct rcu_head * head));
> > struct cipso_v4_doi *cipso_v4_doi_getdef(u32 doi);
> > int cipso_v4_doi_walk(u32 *skip_cnt,
> > int (*callback) (struct cipso_v4_doi *doi_def, void *arg),
> > @@ -143,6 +145,7 @@ static inline int cipso_v4_doi_add(struc
> > }
> >
> > static inline int cipso_v4_doi_remove(u32 doi,
> > + u32 audit_secid,
> > void (*callback) (struct rcu_head * head))
> > {
> > return 0;
> > Index: net-2.6/include/net/netlabel.h
> > ===================================================================
> > --- net-2.6.orig/include/net/netlabel.h
> > +++ net-2.6/include/net/netlabel.h
> > @@ -96,7 +96,7 @@
> > struct netlbl_dom_map;
> >
> > /* Domain mapping operations */
> > -int netlbl_domhsh_remove(const char *domain);
> > +int netlbl_domhsh_remove(const char *domain, u32 audit_secid);
> >
> > /* LSM security attributes */
> > struct netlbl_lsm_cache {
> > Index: net-2.6/net/ipv4/cipso_ipv4.c
> > ===================================================================
> > --- net-2.6.orig/net/ipv4/cipso_ipv4.c
> > +++ net-2.6/net/ipv4/cipso_ipv4.c
> > @@ -474,6 +474,7 @@ doi_add_failure_rlock:
> > /**
> > * cipso_v4_doi_remove - Remove an existing DOI from the CIPSO protocol
> > engine * @doi: the DOI value
> > + * @audit_secid: the LSM secid to use in the audit message
> > * @callback: the DOI cleanup/free callback
> > *
> > * Description:
> > @@ -483,7 +484,9 @@ doi_add_failure_rlock:
> > * success and negative values on failure.
> > *
> > */
> > -int cipso_v4_doi_remove(u32 doi, void (*callback) (struct rcu_head *
> > head)) +int cipso_v4_doi_remove(u32 doi,
> > + u32 audit_secid,
> > + void (*callback) (struct rcu_head * head))
> > {
> > struct cipso_v4_doi *doi_def;
> > struct cipso_v4_domhsh_entry *dom_iter;
> > @@ -502,7 +505,8 @@ int cipso_v4_doi_remove(u32 doi, void (*
> > spin_unlock(&cipso_v4_doi_list_lock);
> > list_for_each_entry_rcu(dom_iter, &doi_def->dom_list, list)
> > if (dom_iter->valid)
> > - netlbl_domhsh_remove(dom_iter->domain);
> > + netlbl_domhsh_remove(dom_iter->domain,
> > + audit_secid);
> > cipso_v4_cache_invalidate();
> > rcu_read_unlock();
> >
> > Index: net-2.6/net/netlabel/netlabel_cipso_v4.c
> > ===================================================================
> > --- net-2.6.orig/net/netlabel/netlabel_cipso_v4.c
> > +++ net-2.6/net/netlabel/netlabel_cipso_v4.c
> > @@ -32,6 +32,7 @@
> > #include <linux/socket.h>
> > #include <linux/string.h>
> > #include <linux/skbuff.h>
> > +#include <linux/audit.h>
> > #include <net/sock.h>
> > #include <net/netlink.h>
> > #include <net/genetlink.h>
> > @@ -162,8 +163,7 @@ static int netlbl_cipsov4_add_std(struct
> > int nla_a_rem;
> > int nla_b_rem;
> >
> > - if (!info->attrs[NLBL_CIPSOV4_A_DOI] ||
> > - !info->attrs[NLBL_CIPSOV4_A_TAGLST] ||
> > + if (!info->attrs[NLBL_CIPSOV4_A_TAGLST] ||
> > !info->attrs[NLBL_CIPSOV4_A_MLSLVLLST])
> > return -EINVAL;
> >
> > @@ -344,8 +344,7 @@ static int netlbl_cipsov4_add_pass(struc
> > int ret_val;
> > struct cipso_v4_doi *doi_def = NULL;
> >
> > - if (!info->attrs[NLBL_CIPSOV4_A_DOI] ||
> > - !info->attrs[NLBL_CIPSOV4_A_TAGLST])
> > + if (!info->attrs[NLBL_CIPSOV4_A_TAGLST])
> > return -EINVAL;
> >
> > doi_def = kmalloc(sizeof(*doi_def), GFP_KERNEL);
> > @@ -381,21 +380,35 @@ static int netlbl_cipsov4_add(struct sk_
> >
> > {
> > int ret_val = -EINVAL;
> > - u32 map_type;
> > + u32 type;
> > + u32 doi;
> > + const char *type_str = "(unknown)";
> > + struct audit_buffer *audit_buf;
> >
> > - if (!info->attrs[NLBL_CIPSOV4_A_MTYPE])
> > + if (!info->attrs[NLBL_CIPSOV4_A_DOI] ||
> > + !info->attrs[NLBL_CIPSOV4_A_MTYPE])
> > return -EINVAL;
> >
> > - map_type = nla_get_u32(info->attrs[NLBL_CIPSOV4_A_MTYPE]);
> > - switch (map_type) {
> > + type = nla_get_u32(info->attrs[NLBL_CIPSOV4_A_MTYPE]);
> > + switch (type) {
> > case CIPSO_V4_MAP_STD:
> > + type_str = "std";
> > ret_val = netlbl_cipsov4_add_std(info);
> > break;
> > case CIPSO_V4_MAP_PASS:
> > + type_str = "pass";
> > ret_val = netlbl_cipsov4_add_pass(info);
> > break;
> > }
> >
> > + if (ret_val == 0) {
> > + doi = nla_get_u32(info->attrs[NLBL_CIPSOV4_A_DOI]);
> > + audit_buf = netlbl_audit_start_common(AUDIT_MAC_CIPSOV4_ADD,
> > + NETLINK_CB(skb).sid);
> > + audit_log_format(audit_buf, " doi=%u type=%s", doi, type_str);
>
> type field is already taken for another purpose, it needs to be renamed.
>
> > + audit_log_end(audit_buf);
> > + }
>
> Normally, we have a field res= that records whether or not the user was
> successful in removing the rule. You would probably do something like
> ret_val==0?1:0 but send a audit event regardless
>
> > +
> > return ret_val;
> > }
> >
> > @@ -653,11 +666,21 @@ static int netlbl_cipsov4_listall(struct
> > static int netlbl_cipsov4_remove(struct sk_buff *skb, struct genl_info
> > *info) {
> > int ret_val = -EINVAL;
> > - u32 doi;
> > + u32 doi = 0;
> > + struct audit_buffer *audit_buf;
> >
> > if (info->attrs[NLBL_CIPSOV4_A_DOI]) {
> > doi = nla_get_u32(info->attrs[NLBL_CIPSOV4_A_DOI]);
> > - ret_val = cipso_v4_doi_remove(doi, netlbl_cipsov4_doi_free);
> > + ret_val = cipso_v4_doi_remove(doi,
> > + NETLINK_CB(skb).sid,
> > + netlbl_cipsov4_doi_free);
> > + }
> > +
> > + if (ret_val == 0) {
> > + audit_buf = netlbl_audit_start_common(AUDIT_MAC_CIPSOV4_DEL,
> > + NETLINK_CB(skb).sid);
> > + audit_log_format(audit_buf, " doi=%u", doi);
> > + audit_log_end(audit_buf);
> > }
>
> Same as above, global comment about res= being recorded no matter what.
>
> >
> > return ret_val;
> > Index: net-2.6/net/netlabel/netlabel_domainhash.c
> > ===================================================================
> > --- net-2.6.orig/net/netlabel/netlabel_domainhash.c
> > +++ net-2.6/net/netlabel/netlabel_domainhash.c
> > @@ -35,12 +35,14 @@
> > #include <linux/skbuff.h>
> > #include <linux/spinlock.h>
> > #include <linux/string.h>
> > +#include <linux/audit.h>
> > #include <net/netlabel.h>
> > #include <net/cipso_ipv4.h>
> > #include <asm/bug.h>
> >
> > #include "netlabel_mgmt.h"
> > #include "netlabel_domainhash.h"
> > +#include "netlabel_user.h"
> >
> > struct netlbl_domhsh_tbl {
> > struct list_head *tbl;
> > @@ -186,6 +188,7 @@ int netlbl_domhsh_init(u32 size)
> > /**
> > * netlbl_domhsh_add - Adds a entry to the domain hash table
> > * @entry: the entry to add
> > + * @audit_secid: the LSM secid to use in the audit message
> > *
> > * Description:
> > * Adds a new entry to the domain hash table and handles any updates to
> > the @@ -193,10 +196,12 @@ int netlbl_domhsh_init(u32 size)
> > * negative on failure.
> > *
> > */
> > -int netlbl_domhsh_add(struct netlbl_dom_map *entry)
> > +int netlbl_domhsh_add(struct netlbl_dom_map *entry, u32 audit_secid)
> > {
> > int ret_val;
> > u32 bkt;
> > + struct audit_buffer *audit_buf;
> > + char *audit_domain;
> >
> > switch (entry->type) {
> > case NETLBL_NLTYPE_UNLABELED:
> > @@ -236,6 +241,26 @@ int netlbl_domhsh_add(struct netlbl_dom_
> > spin_unlock(&netlbl_domhsh_def_lock);
> > } else
> > ret_val = -EINVAL;
> > + if (ret_val == 0) {
> > + if (entry->domain != NULL)
> > + audit_domain = entry->domain;
> > + else
> > + audit_domain = "(default)";
> > + audit_buf = netlbl_audit_start_common(AUDIT_MAC_MAP_ADD,
> > + audit_secid);
> > + audit_log_format(audit_buf, " domain=%s", audit_domain);
> > + switch (entry->type) {
> > + case NETLBL_NLTYPE_UNLABELED:
> > + audit_log_format(audit_buf, " protocol=unlbl");
> > + break;
> > + case NETLBL_NLTYPE_CIPSOV4:
> > + audit_log_format(audit_buf,
> > + " protocol=cipsov4 doi=%u",
> > + entry->type_def.cipsov4->doi);
> > + break;
> > + }
> > + audit_log_end(audit_buf);
> > + }
> > rcu_read_unlock();
> >
> > if (ret_val != 0) {
> > @@ -254,6 +279,7 @@ int netlbl_domhsh_add(struct netlbl_dom_
> > /**
> > * netlbl_domhsh_add_default - Adds the default entry to the domain hash
> > table * @entry: the entry to add
> > + * @audit_secid: the LSM secid to use in the audit message
> > *
> > * Description:
> > * Adds a new default entry to the domain hash table and handles any
> > updates @@ -261,14 +287,15 @@ int netlbl_domhsh_add(struct netlbl_dom_
> > * negative on failure.
> > *
> > */
> > -int netlbl_domhsh_add_default(struct netlbl_dom_map *entry)
> > +int netlbl_domhsh_add_default(struct netlbl_dom_map *entry, u32
> > audit_secid) {
> > - return netlbl_domhsh_add(entry);
> > + return netlbl_domhsh_add(entry, audit_secid);
> > }
> >
> > /**
> > * netlbl_domhsh_remove - Removes an entry from the domain hash table
> > * @domain: the domain to remove
> > + * @audit_secid: the LSM secid to use in the audit message
> > *
> > * Description:
> > * Removes an entry from the domain hash table and handles any updates to
> > the @@ -276,10 +303,12 @@ int netlbl_domhsh_add_default(struct net
> > * negative on failure.
> > *
> > */
> > -int netlbl_domhsh_remove(const char *domain)
> > +int netlbl_domhsh_remove(const char *domain, u32 audit_secid)
> > {
> > int ret_val = -ENOENT;
> > struct netlbl_dom_map *entry;
> > + struct audit_buffer *audit_buf;
> > + char *audit_domain;
> >
> > rcu_read_lock();
> > if (domain != NULL)
> > @@ -316,8 +345,18 @@ int netlbl_domhsh_remove(const char *dom
> > ret_val = -ENOENT;
> > spin_unlock(&netlbl_domhsh_def_lock);
> > }
> > - if (ret_val == 0)
> > + if (ret_val == 0) {
> > + if (entry->domain != NULL)
> > + audit_domain = entry->domain;
> > + else
> > + audit_domain = "(default)";
> > + audit_buf = netlbl_audit_start_common(AUDIT_MAC_MAP_DEL,
> > + audit_secid);
> > + audit_log_format(audit_buf, " domain=%s", audit_domain);
> > + audit_log_end(audit_buf);
> > +
> > call_rcu(&entry->rcu, netlbl_domhsh_free_entry);
> > + }
> >
> > remove_return:
> > rcu_read_unlock();
> > @@ -326,6 +365,7 @@ remove_return:
> >
> > /**
> > * netlbl_domhsh_remove_default - Removes the default entry from the table
> > + * @audit_secid: the LSM secid to use in the audit message
> > *
> > * Description:
> > * Removes/resets the default entry for the domain hash table and handles
> > any @@ -333,9 +373,9 @@ remove_return:
> > * success, non-zero on failure.
> > *
> > */
> > -int netlbl_domhsh_remove_default(void)
> > +int netlbl_domhsh_remove_default(u32 audit_secid)
> > {
> > - return netlbl_domhsh_remove(NULL);
> > + return netlbl_domhsh_remove(NULL, audit_secid);
> > }
> >
> > /**
> > Index: net-2.6/net/netlabel/netlabel_domainhash.h
> > ===================================================================
> > --- net-2.6.orig/net/netlabel/netlabel_domainhash.h
> > +++ net-2.6/net/netlabel/netlabel_domainhash.h
> > @@ -57,9 +57,9 @@ struct netlbl_dom_map {
> > int netlbl_domhsh_init(u32 size);
> >
> > /* Manipulate the domain hash table */
> > -int netlbl_domhsh_add(struct netlbl_dom_map *entry);
> > -int netlbl_domhsh_add_default(struct netlbl_dom_map *entry);
> > -int netlbl_domhsh_remove_default(void);
> > +int netlbl_domhsh_add(struct netlbl_dom_map *entry, u32 audit_secid);
> > +int netlbl_domhsh_add_default(struct netlbl_dom_map *entry, u32
> > audit_secid); +int netlbl_domhsh_remove_default(u32 audit_secid);
> > struct netlbl_dom_map *netlbl_domhsh_getentry(const char *domain);
> > int netlbl_domhsh_walk(u32 *skip_bkt,
> > u32 *skip_chain,
> > Index: net-2.6/net/netlabel/netlabel_mgmt.c
> > ===================================================================
> > --- net-2.6.orig/net/netlabel/netlabel_mgmt.c
> > +++ net-2.6/net/netlabel/netlabel_mgmt.c
> > @@ -108,7 +108,7 @@ static int netlbl_mgmt_add(struct sk_buf
> >
> > switch (entry->type) {
> > case NETLBL_NLTYPE_UNLABELED:
> > - ret_val = netlbl_domhsh_add(entry);
> > + ret_val = netlbl_domhsh_add(entry, NETLINK_CB(skb).sid);
> > break;
> > case NETLBL_NLTYPE_CIPSOV4:
> > if (!info->attrs[NLBL_MGMT_A_CV4DOI])
> > @@ -125,7 +125,7 @@ static int netlbl_mgmt_add(struct sk_buf
> > rcu_read_unlock();
> > goto add_failure;
> > }
> > - ret_val = netlbl_domhsh_add(entry);
> > + ret_val = netlbl_domhsh_add(entry, NETLINK_CB(skb).sid);
> > rcu_read_unlock();
> > break;
> > default:
> > @@ -161,7 +161,7 @@ static int netlbl_mgmt_remove(struct sk_
> > return -EINVAL;
> >
> > domain = nla_data(info->attrs[NLBL_MGMT_A_DOMAIN]);
> > - return netlbl_domhsh_remove(domain);
> > + return netlbl_domhsh_remove(domain, NETLINK_CB(skb).sid);
> > }
> >
> > /**
> > @@ -277,7 +277,8 @@ static int netlbl_mgmt_adddef(struct sk_
> >
> > switch (entry->type) {
> > case NETLBL_NLTYPE_UNLABELED:
> > - ret_val = netlbl_domhsh_add_default(entry);
> > + ret_val = netlbl_domhsh_add_default(entry,
> > + NETLINK_CB(skb).sid);
> > break;
> > case NETLBL_NLTYPE_CIPSOV4:
> > if (!info->attrs[NLBL_MGMT_A_CV4DOI])
> > @@ -294,7 +295,8 @@ static int netlbl_mgmt_adddef(struct sk_
> > rcu_read_unlock();
> > goto adddef_failure;
> > }
> > - ret_val = netlbl_domhsh_add_default(entry);
> > + ret_val = netlbl_domhsh_add_default(entry,
> > + NETLINK_CB(skb).sid);
> > rcu_read_unlock();
> > break;
> > default:
> > @@ -322,7 +324,7 @@ adddef_failure:
> > */
> > static int netlbl_mgmt_removedef(struct sk_buff *skb, struct genl_info
> > *info) {
> > - return netlbl_domhsh_remove_default();
> > + return netlbl_domhsh_remove_default(NETLINK_CB(skb).sid);
> > }
> >
> > /**
> > Index: net-2.6/net/netlabel/netlabel_unlabeled.c
> > ===================================================================
> > --- net-2.6.orig/net/netlabel/netlabel_unlabeled.c
> > +++ net-2.6/net/netlabel/netlabel_unlabeled.c
> > @@ -64,6 +64,27 @@ static struct nla_policy netlbl_unlabel_
> > };
> >
> > /*
> > + * Helper Functions
> > + */
> > +
> > +/**
> > + * netlbl_unlabel_acceptflg_set - Set the unlabeled accept flag
> > + * @value: desired value
> > + * @audit_secid: the LSM secid to use in the audit message
> > + *
> > + * Description:
> > + * Set the value of the unlabeled accept flag to @value.
> > + *
> > + */
> > +static void netlbl_unlabel_acceptflg_set(u8 value, u32 audit_secid)
> > +{
> > + atomic_set(&netlabel_unlabel_accept_flg, value);
> > + netlbl_audit_nomsg((value ?
> > + AUDIT_MAC_UNLBL_ACCEPT : AUDIT_MAC_UNLBL_DENY),
> > + audit_secid);
>
> Looking at how this is being used, I think only 1 message type should be used.
> There are places in the audit system where we set a flag to 1 or 0, but only
> have 1 message type. We record the old and new value. So, you'd need to pass
> that to the logger.
>
> > +}
> > +
> > +/*
> > * NetLabel Command Handlers
> > */
> >
> > @@ -79,18 +100,18 @@ static struct nla_policy netlbl_unlabel_
> > */
> > static int netlbl_unlabel_accept(struct sk_buff *skb, struct genl_info
> > *info) {
> > - int ret_val = -EINVAL;
> > u8 value;
> >
> > if (info->attrs[NLBL_UNLABEL_A_ACPTFLG]) {
> > value = nla_get_u8(info->attrs[NLBL_UNLABEL_A_ACPTFLG]);
> > if (value == 1 || value == 0) {
> > - atomic_set(&netlabel_unlabel_accept_flg, value);
> > - ret_val = 0;
> > + netlbl_unlabel_acceptflg_set(value,
> > + NETLINK_CB(skb).sid);
> > + return 0;
> > }
> > }
> >
> > - return ret_val;
> > + return -EINVAL;
> > }
> >
> > /**
> > @@ -229,16 +250,19 @@ int netlbl_unlabel_defconf(void)
> > {
> > int ret_val;
> > struct netlbl_dom_map *entry;
> > + u32 secid;
> > +
> > + security_task_getsecid(current, &secid);
> >
> > entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> > if (entry == NULL)
> > return -ENOMEM;
> > entry->type = NETLBL_NLTYPE_UNLABELED;
> > - ret_val = netlbl_domhsh_add_default(entry);
> > + ret_val = netlbl_domhsh_add_default(entry, secid);
> > if (ret_val != 0)
> > return ret_val;
> >
> > - atomic_set(&netlabel_unlabel_accept_flg, 1);
> > + netlbl_unlabel_acceptflg_set(1, secid);
> >
> > return 0;
> > }
> > Index: net-2.6/net/netlabel/netlabel_user.c
> > ===================================================================
> > --- net-2.6.orig/net/netlabel/netlabel_user.c
> > +++ net-2.6/net/netlabel/netlabel_user.c
> > @@ -32,6 +32,9 @@
> > #include <linux/types.h>
> > #include <linux/list.h>
> > #include <linux/socket.h>
> > +#include <linux/audit.h>
> > +#include <linux/tty.h>
> > +#include <linux/security.h>
> > #include <net/sock.h>
> > #include <net/netlink.h>
> > #include <net/genetlink.h>
> > @@ -74,3 +77,91 @@ int netlbl_netlink_init(void)
> >
> > return 0;
> > }
> > +
> > +/*
> > + * NetLabel Audit Functions
> > + */
> > +
> > +/**
> > + * netlbl_audit_start_common - Start an audit message
> > + * @type: audit message type
> > + * @secid: LSM context ID
> > + *
> > + * Description:
> > + * Start an audit message using the type specified in @type and fill the
> > audit + * message with some fields common to all NetLabel audit messages.
> > Returns + * a pointer to the audit buffer on success, NULL on failure.
> > + *
> > + */
> > +struct audit_buffer *netlbl_audit_start_common(int type, u32 secid)
> > +{
>
> Generally, logging functions are moved into auditsc.c where the context and
> other functions are defined.
>
> > + struct audit_context *audit_ctx = current->audit_context;
> > + struct audit_buffer *audit_buf;
> > + uid_t audit_loginuid;
> > + const char *audit_tty;
> > + char audit_comm[sizeof(current->comm)];
> > + struct vm_area_struct *vma;
> > + char *secctx;
> > + u32 secctx_len;
> > +
> > + audit_buf = audit_log_start(audit_ctx, GFP_ATOMIC, type);
> > + if (audit_buf == NULL)
> > + return NULL;
> > +
> > + audit_loginuid = audit_get_loginuid(audit_ctx);
>
> Netlink is async protocol, you should use the loginuid of the netlink packet
> and pass it into this function.
>
> > + if (current->signal &&
> > + current->signal->tty &&
> > + current->signal->tty->name)
> > + audit_tty = current->signal->tty->name;
> > + else
> > + audit_tty = "(none)";
>
> Netlink is an async protocol. How do you know that the sender is current? This
> is a global comment, please check use of current everywhere.
>
> > + get_task_comm(audit_comm, current);
> > +
> > + audit_log_format(audit_buf,
> > + "netlabel: auid=%u uid=%u tty=%s pid=%d",
> > + audit_loginuid,
> > + current->uid,
> > + audit_tty,
> > + current->pid);
>
> Why are you logging all this? When we add audit rules, all that we log is the
> auid, and subj. If we need to log all this, we should probably have a helper
> function that gets called by other config change loggers.
>
> > + audit_log_format(audit_buf, " comm=");
> > + audit_log_untrustedstring(audit_buf, audit_comm);
> > + if (current->mm) {
> > + down_read(¤t->mm->mmap_sem);
> > + vma = current->mm->mmap;
> > + while (vma) {
> > + if ((vma->vm_flags & VM_EXECUTABLE) &&
> > + vma->vm_file) {
> > + audit_log_d_path(audit_buf,
> > + " exe=",
> > + vma->vm_file->f_dentry,
> > + vma->vm_file->f_vfsmnt);
> > + break;
> > + }
> > + vma = vma->vm_next;
> > + }
> > + up_read(¤t->mm->mmap_sem);
> > + }
> > +
>
> If this function was moved inside auditsc.c you could use a function there
> that does this. But the question remains why all this data?
>
> > + if (secid != 0 &&
> > + security_secid_to_secctx(secid, &secctx, &secctx_len) == 0)
> > + audit_log_format(audit_buf, " subj=%s", secctx);
> > +
> > + return audit_buf;
> > +}
> > +
> > +/**
> > + * netlbl_audit_nomsg - Send an audit message without additional text
> > + * @type: audit message type
> > + * @secid: LSM context ID
> > + *
> > + * Description:
> > + * Send an audit message with only the common NetLabel audit fields.
> > + *
> > + */
> > +void netlbl_audit_nomsg(int type, u32 secid)
> > +{
> > + struct audit_buffer *audit_buf;
> > +
> > + audit_buf = netlbl_audit_start_common(type, secid);
> > + audit_log_end(audit_buf);
>
> We normally record old and new value when changing flags like this.
>
> > +}
> > Index: net-2.6/net/netlabel/netlabel_user.h
> > ===================================================================
> > --- net-2.6.orig/net/netlabel/netlabel_user.h
> > +++ net-2.6/net/netlabel/netlabel_user.h
> > @@ -34,6 +34,7 @@
> > #include <linux/types.h>
> > #include <linux/skbuff.h>
> > #include <linux/capability.h>
> > +#include <linux/audit.h>
> > #include <net/netlink.h>
> > #include <net/genetlink.h>
> > #include <net/netlabel.h>
> > @@ -75,4 +76,9 @@ static inline void *netlbl_netlink_hdr_p
> >
> > int netlbl_netlink_init(void);
> >
> > +/* NetLabel Audit Functions */
> > +
> > +struct audit_buffer *netlbl_audit_start_common(int type, u32 secid);
> > +void netlbl_audit_nomsg(int type, u32 secid);
> > +
> > #endif
> >
> > --
> > paul moore
> > linux security @ hp
> >
> > --
> > Linux-audit mailing list
> > Linux-audit@redhat.com
> > https://www.redhat.com/mailman/listinfo/linux-audit
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
James Morris
<jmorris@namei.org>
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 1/1] NetLabel: add audit support for configuration changes
2006-09-29 14:13 ` James Morris
@ 2006-09-29 15:43 ` Paul Moore
-1 siblings, 0 replies; 24+ messages in thread
From: Paul Moore @ 2006-09-29 15:43 UTC (permalink / raw)
To: David S. Miller; +Cc: James Morris, Steve Grubb, linux-audit, netdev, selinux
James Morris wrote:
> Dave,
>
> Looks like this patch needs to be reverted until these issues are
> resolved.
Yes, please revert this patch.
I posted an earlier version to the linux-audit list and waited for a day
to see if there any comments before submitting for inclusion, but
unfortunately it seems I didn't wait long enough for Steve. I'll deal
with Steve's comments once I handle the secid reconciliation patches as
I think they are more important right now.
> On Fri, 29 Sep 2006, Steve Grubb wrote:
>
>
>>On Thursday 28 September 2006 14:03, paul.moore@hp.com wrote:
>>
>>>This patch adds audit support to NetLabel, including six new audit message
>>>types shown below.
>>>
>>> #define AUDIT_MAC_UNLBL_ACCEPT 1406
>>> #define AUDIT_MAC_UNLBL_DENY 1407
>>> #define AUDIT_MAC_CIPSOV4_ADD 1408
>>> #define AUDIT_MAC_CIPSOV4_DEL 1409
>>> #define AUDIT_MAC_MAP_ADD 1410
>>> #define AUDIT_MAC_MAP_DEL 1411
>>>
>>>Please consider this for inclusion into 2.6.19.
>>>
>>>Signed-off-by: Paul Moore <paul.moore@hp.com>
>>>---
>>> include/linux/audit.h | 6 ++
>>> include/net/cipso_ipv4.h | 5 +-
>>> include/net/netlabel.h | 2
>>> net/ipv4/cipso_ipv4.c | 8 ++-
>>> net/netlabel/netlabel_cipso_v4.c | 43 +++++++++++++----
>>> net/netlabel/netlabel_domainhash.c | 54 +++++++++++++++++++--
>>> net/netlabel/netlabel_domainhash.h | 6 +-
>>> net/netlabel/netlabel_mgmt.c | 14 +++--
>>> net/netlabel/netlabel_unlabeled.c | 36 ++++++++++++--
>>> net/netlabel/netlabel_user.c | 91
>>>+++++++++++++++++++++++++++++++++++++ net/netlabel/netlabel_user.h |
>>> 6 ++
>>> 11 files changed, 235 insertions(+), 36 deletions(-)
>>>
>>>Index: net-2.6/include/linux/audit.h
>>>===================================================================
>>>--- net-2.6.orig/include/linux/audit.h
>>>+++ net-2.6/include/linux/audit.h
>>>@@ -95,6 +95,12 @@
>>> #define AUDIT_MAC_POLICY_LOAD 1403 /* Policy file load */
>>> #define AUDIT_MAC_STATUS 1404 /* Changed enforcing,permissive,off */
>>> #define AUDIT_MAC_CONFIG_CHANGE 1405 /* Changes to booleans */
>>>+#define AUDIT_MAC_UNLBL_ACCEPT 1406 /* NetLabel: allow unlabeled traffic */
>>>+#define AUDIT_MAC_UNLBL_DENY 1407 /* NetLabel: deny unlabeled traffic */
>>
>>Please drop the use of DENY per comments later down.
>>
>>
>>>+#define AUDIT_MAC_CIPSOV4_ADD 1408 /* NetLabel: add CIPSOv4 DOI entry */
>>>+#define AUDIT_MAC_CIPSOV4_DEL 1409 /* NetLabel: del CIPSOv4 DOI entry */
>>>+#define AUDIT_MAC_MAP_ADD 1410 /* NetLabel: add LSM domain mapping */
>>>+#define AUDIT_MAC_MAP_DEL 1411 /* NetLabel: del LSM domain mapping */
>>>
>>> #define AUDIT_FIRST_KERN_ANOM_MSG 1700
>>> #define AUDIT_LAST_KERN_ANOM_MSG 1799
>>>Index: net-2.6/include/net/cipso_ipv4.h
>>>===================================================================
>>>--- net-2.6.orig/include/net/cipso_ipv4.h
>>>+++ net-2.6/include/net/cipso_ipv4.h
>>>@@ -128,7 +128,9 @@ extern int cipso_v4_rbm_strictvalid;
>>>
>>> #ifdef CONFIG_NETLABEL
>>> int cipso_v4_doi_add(struct cipso_v4_doi *doi_def);
>>>-int cipso_v4_doi_remove(u32 doi, void (*callback) (struct rcu_head *
>>>head)); +int cipso_v4_doi_remove(u32 doi,
>>>+ u32 audit_secid,
>>>+ void (*callback) (struct rcu_head * head));
>>> struct cipso_v4_doi *cipso_v4_doi_getdef(u32 doi);
>>> int cipso_v4_doi_walk(u32 *skip_cnt,
>>> int (*callback) (struct cipso_v4_doi *doi_def, void *arg),
>>>@@ -143,6 +145,7 @@ static inline int cipso_v4_doi_add(struc
>>> }
>>>
>>> static inline int cipso_v4_doi_remove(u32 doi,
>>>+ u32 audit_secid,
>>> void (*callback) (struct rcu_head * head))
>>> {
>>> return 0;
>>>Index: net-2.6/include/net/netlabel.h
>>>===================================================================
>>>--- net-2.6.orig/include/net/netlabel.h
>>>+++ net-2.6/include/net/netlabel.h
>>>@@ -96,7 +96,7 @@
>>> struct netlbl_dom_map;
>>>
>>> /* Domain mapping operations */
>>>-int netlbl_domhsh_remove(const char *domain);
>>>+int netlbl_domhsh_remove(const char *domain, u32 audit_secid);
>>>
>>> /* LSM security attributes */
>>> struct netlbl_lsm_cache {
>>>Index: net-2.6/net/ipv4/cipso_ipv4.c
>>>===================================================================
>>>--- net-2.6.orig/net/ipv4/cipso_ipv4.c
>>>+++ net-2.6/net/ipv4/cipso_ipv4.c
>>>@@ -474,6 +474,7 @@ doi_add_failure_rlock:
>>> /**
>>> * cipso_v4_doi_remove - Remove an existing DOI from the CIPSO protocol
>>>engine * @doi: the DOI value
>>>+ * @audit_secid: the LSM secid to use in the audit message
>>> * @callback: the DOI cleanup/free callback
>>> *
>>> * Description:
>>>@@ -483,7 +484,9 @@ doi_add_failure_rlock:
>>> * success and negative values on failure.
>>> *
>>> */
>>>-int cipso_v4_doi_remove(u32 doi, void (*callback) (struct rcu_head *
>>>head)) +int cipso_v4_doi_remove(u32 doi,
>>>+ u32 audit_secid,
>>>+ void (*callback) (struct rcu_head * head))
>>> {
>>> struct cipso_v4_doi *doi_def;
>>> struct cipso_v4_domhsh_entry *dom_iter;
>>>@@ -502,7 +505,8 @@ int cipso_v4_doi_remove(u32 doi, void (*
>>> spin_unlock(&cipso_v4_doi_list_lock);
>>> list_for_each_entry_rcu(dom_iter, &doi_def->dom_list, list)
>>> if (dom_iter->valid)
>>>- netlbl_domhsh_remove(dom_iter->domain);
>>>+ netlbl_domhsh_remove(dom_iter->domain,
>>>+ audit_secid);
>>> cipso_v4_cache_invalidate();
>>> rcu_read_unlock();
>>>
>>>Index: net-2.6/net/netlabel/netlabel_cipso_v4.c
>>>===================================================================
>>>--- net-2.6.orig/net/netlabel/netlabel_cipso_v4.c
>>>+++ net-2.6/net/netlabel/netlabel_cipso_v4.c
>>>@@ -32,6 +32,7 @@
>>> #include <linux/socket.h>
>>> #include <linux/string.h>
>>> #include <linux/skbuff.h>
>>>+#include <linux/audit.h>
>>> #include <net/sock.h>
>>> #include <net/netlink.h>
>>> #include <net/genetlink.h>
>>>@@ -162,8 +163,7 @@ static int netlbl_cipsov4_add_std(struct
>>> int nla_a_rem;
>>> int nla_b_rem;
>>>
>>>- if (!info->attrs[NLBL_CIPSOV4_A_DOI] ||
>>>- !info->attrs[NLBL_CIPSOV4_A_TAGLST] ||
>>>+ if (!info->attrs[NLBL_CIPSOV4_A_TAGLST] ||
>>> !info->attrs[NLBL_CIPSOV4_A_MLSLVLLST])
>>> return -EINVAL;
>>>
>>>@@ -344,8 +344,7 @@ static int netlbl_cipsov4_add_pass(struc
>>> int ret_val;
>>> struct cipso_v4_doi *doi_def = NULL;
>>>
>>>- if (!info->attrs[NLBL_CIPSOV4_A_DOI] ||
>>>- !info->attrs[NLBL_CIPSOV4_A_TAGLST])
>>>+ if (!info->attrs[NLBL_CIPSOV4_A_TAGLST])
>>> return -EINVAL;
>>>
>>> doi_def = kmalloc(sizeof(*doi_def), GFP_KERNEL);
>>>@@ -381,21 +380,35 @@ static int netlbl_cipsov4_add(struct sk_
>>>
>>> {
>>> int ret_val = -EINVAL;
>>>- u32 map_type;
>>>+ u32 type;
>>>+ u32 doi;
>>>+ const char *type_str = "(unknown)";
>>>+ struct audit_buffer *audit_buf;
>>>
>>>- if (!info->attrs[NLBL_CIPSOV4_A_MTYPE])
>>>+ if (!info->attrs[NLBL_CIPSOV4_A_DOI] ||
>>>+ !info->attrs[NLBL_CIPSOV4_A_MTYPE])
>>> return -EINVAL;
>>>
>>>- map_type = nla_get_u32(info->attrs[NLBL_CIPSOV4_A_MTYPE]);
>>>- switch (map_type) {
>>>+ type = nla_get_u32(info->attrs[NLBL_CIPSOV4_A_MTYPE]);
>>>+ switch (type) {
>>> case CIPSO_V4_MAP_STD:
>>>+ type_str = "std";
>>> ret_val = netlbl_cipsov4_add_std(info);
>>> break;
>>> case CIPSO_V4_MAP_PASS:
>>>+ type_str = "pass";
>>> ret_val = netlbl_cipsov4_add_pass(info);
>>> break;
>>> }
>>>
>>>+ if (ret_val == 0) {
>>>+ doi = nla_get_u32(info->attrs[NLBL_CIPSOV4_A_DOI]);
>>>+ audit_buf = netlbl_audit_start_common(AUDIT_MAC_CIPSOV4_ADD,
>>>+ NETLINK_CB(skb).sid);
>>>+ audit_log_format(audit_buf, " doi=%u type=%s", doi, type_str);
>>
>>type field is already taken for another purpose, it needs to be renamed.
>>
>>
>>>+ audit_log_end(audit_buf);
>>>+ }
>>
>>Normally, we have a field res= that records whether or not the user was
>>successful in removing the rule. You would probably do something like
>>ret_val==0?1:0 but send a audit event regardless
>>
>>
>>>+
>>> return ret_val;
>>> }
>>>
>>>@@ -653,11 +666,21 @@ static int netlbl_cipsov4_listall(struct
>>> static int netlbl_cipsov4_remove(struct sk_buff *skb, struct genl_info
>>>*info) {
>>> int ret_val = -EINVAL;
>>>- u32 doi;
>>>+ u32 doi = 0;
>>>+ struct audit_buffer *audit_buf;
>>>
>>> if (info->attrs[NLBL_CIPSOV4_A_DOI]) {
>>> doi = nla_get_u32(info->attrs[NLBL_CIPSOV4_A_DOI]);
>>>- ret_val = cipso_v4_doi_remove(doi, netlbl_cipsov4_doi_free);
>>>+ ret_val = cipso_v4_doi_remove(doi,
>>>+ NETLINK_CB(skb).sid,
>>>+ netlbl_cipsov4_doi_free);
>>>+ }
>>>+
>>>+ if (ret_val == 0) {
>>>+ audit_buf = netlbl_audit_start_common(AUDIT_MAC_CIPSOV4_DEL,
>>>+ NETLINK_CB(skb).sid);
>>>+ audit_log_format(audit_buf, " doi=%u", doi);
>>>+ audit_log_end(audit_buf);
>>> }
>>
>>Same as above, global comment about res= being recorded no matter what.
>>
>>
>>> return ret_val;
>>>Index: net-2.6/net/netlabel/netlabel_domainhash.c
>>>===================================================================
>>>--- net-2.6.orig/net/netlabel/netlabel_domainhash.c
>>>+++ net-2.6/net/netlabel/netlabel_domainhash.c
>>>@@ -35,12 +35,14 @@
>>> #include <linux/skbuff.h>
>>> #include <linux/spinlock.h>
>>> #include <linux/string.h>
>>>+#include <linux/audit.h>
>>> #include <net/netlabel.h>
>>> #include <net/cipso_ipv4.h>
>>> #include <asm/bug.h>
>>>
>>> #include "netlabel_mgmt.h"
>>> #include "netlabel_domainhash.h"
>>>+#include "netlabel_user.h"
>>>
>>> struct netlbl_domhsh_tbl {
>>> struct list_head *tbl;
>>>@@ -186,6 +188,7 @@ int netlbl_domhsh_init(u32 size)
>>> /**
>>> * netlbl_domhsh_add - Adds a entry to the domain hash table
>>> * @entry: the entry to add
>>>+ * @audit_secid: the LSM secid to use in the audit message
>>> *
>>> * Description:
>>> * Adds a new entry to the domain hash table and handles any updates to
>>>the @@ -193,10 +196,12 @@ int netlbl_domhsh_init(u32 size)
>>> * negative on failure.
>>> *
>>> */
>>>-int netlbl_domhsh_add(struct netlbl_dom_map *entry)
>>>+int netlbl_domhsh_add(struct netlbl_dom_map *entry, u32 audit_secid)
>>> {
>>> int ret_val;
>>> u32 bkt;
>>>+ struct audit_buffer *audit_buf;
>>>+ char *audit_domain;
>>>
>>> switch (entry->type) {
>>> case NETLBL_NLTYPE_UNLABELED:
>>>@@ -236,6 +241,26 @@ int netlbl_domhsh_add(struct netlbl_dom_
>>> spin_unlock(&netlbl_domhsh_def_lock);
>>> } else
>>> ret_val = -EINVAL;
>>>+ if (ret_val == 0) {
>>>+ if (entry->domain != NULL)
>>>+ audit_domain = entry->domain;
>>>+ else
>>>+ audit_domain = "(default)";
>>>+ audit_buf = netlbl_audit_start_common(AUDIT_MAC_MAP_ADD,
>>>+ audit_secid);
>>>+ audit_log_format(audit_buf, " domain=%s", audit_domain);
>>>+ switch (entry->type) {
>>>+ case NETLBL_NLTYPE_UNLABELED:
>>>+ audit_log_format(audit_buf, " protocol=unlbl");
>>>+ break;
>>>+ case NETLBL_NLTYPE_CIPSOV4:
>>>+ audit_log_format(audit_buf,
>>>+ " protocol=cipsov4 doi=%u",
>>>+ entry->type_def.cipsov4->doi);
>>>+ break;
>>>+ }
>>>+ audit_log_end(audit_buf);
>>>+ }
>>> rcu_read_unlock();
>>>
>>> if (ret_val != 0) {
>>>@@ -254,6 +279,7 @@ int netlbl_domhsh_add(struct netlbl_dom_
>>> /**
>>> * netlbl_domhsh_add_default - Adds the default entry to the domain hash
>>>table * @entry: the entry to add
>>>+ * @audit_secid: the LSM secid to use in the audit message
>>> *
>>> * Description:
>>> * Adds a new default entry to the domain hash table and handles any
>>>updates @@ -261,14 +287,15 @@ int netlbl_domhsh_add(struct netlbl_dom_
>>> * negative on failure.
>>> *
>>> */
>>>-int netlbl_domhsh_add_default(struct netlbl_dom_map *entry)
>>>+int netlbl_domhsh_add_default(struct netlbl_dom_map *entry, u32
>>>audit_secid) {
>>>- return netlbl_domhsh_add(entry);
>>>+ return netlbl_domhsh_add(entry, audit_secid);
>>> }
>>>
>>> /**
>>> * netlbl_domhsh_remove - Removes an entry from the domain hash table
>>> * @domain: the domain to remove
>>>+ * @audit_secid: the LSM secid to use in the audit message
>>> *
>>> * Description:
>>> * Removes an entry from the domain hash table and handles any updates to
>>>the @@ -276,10 +303,12 @@ int netlbl_domhsh_add_default(struct net
>>> * negative on failure.
>>> *
>>> */
>>>-int netlbl_domhsh_remove(const char *domain)
>>>+int netlbl_domhsh_remove(const char *domain, u32 audit_secid)
>>> {
>>> int ret_val = -ENOENT;
>>> struct netlbl_dom_map *entry;
>>>+ struct audit_buffer *audit_buf;
>>>+ char *audit_domain;
>>>
>>> rcu_read_lock();
>>> if (domain != NULL)
>>>@@ -316,8 +345,18 @@ int netlbl_domhsh_remove(const char *dom
>>> ret_val = -ENOENT;
>>> spin_unlock(&netlbl_domhsh_def_lock);
>>> }
>>>- if (ret_val == 0)
>>>+ if (ret_val == 0) {
>>>+ if (entry->domain != NULL)
>>>+ audit_domain = entry->domain;
>>>+ else
>>>+ audit_domain = "(default)";
>>>+ audit_buf = netlbl_audit_start_common(AUDIT_MAC_MAP_DEL,
>>>+ audit_secid);
>>>+ audit_log_format(audit_buf, " domain=%s", audit_domain);
>>>+ audit_log_end(audit_buf);
>>>+
>>> call_rcu(&entry->rcu, netlbl_domhsh_free_entry);
>>>+ }
>>>
>>> remove_return:
>>> rcu_read_unlock();
>>>@@ -326,6 +365,7 @@ remove_return:
>>>
>>> /**
>>> * netlbl_domhsh_remove_default - Removes the default entry from the table
>>>+ * @audit_secid: the LSM secid to use in the audit message
>>> *
>>> * Description:
>>> * Removes/resets the default entry for the domain hash table and handles
>>>any @@ -333,9 +373,9 @@ remove_return:
>>> * success, non-zero on failure.
>>> *
>>> */
>>>-int netlbl_domhsh_remove_default(void)
>>>+int netlbl_domhsh_remove_default(u32 audit_secid)
>>> {
>>>- return netlbl_domhsh_remove(NULL);
>>>+ return netlbl_domhsh_remove(NULL, audit_secid);
>>> }
>>>
>>> /**
>>>Index: net-2.6/net/netlabel/netlabel_domainhash.h
>>>===================================================================
>>>--- net-2.6.orig/net/netlabel/netlabel_domainhash.h
>>>+++ net-2.6/net/netlabel/netlabel_domainhash.h
>>>@@ -57,9 +57,9 @@ struct netlbl_dom_map {
>>> int netlbl_domhsh_init(u32 size);
>>>
>>> /* Manipulate the domain hash table */
>>>-int netlbl_domhsh_add(struct netlbl_dom_map *entry);
>>>-int netlbl_domhsh_add_default(struct netlbl_dom_map *entry);
>>>-int netlbl_domhsh_remove_default(void);
>>>+int netlbl_domhsh_add(struct netlbl_dom_map *entry, u32 audit_secid);
>>>+int netlbl_domhsh_add_default(struct netlbl_dom_map *entry, u32
>>>audit_secid); +int netlbl_domhsh_remove_default(u32 audit_secid);
>>> struct netlbl_dom_map *netlbl_domhsh_getentry(const char *domain);
>>> int netlbl_domhsh_walk(u32 *skip_bkt,
>>> u32 *skip_chain,
>>>Index: net-2.6/net/netlabel/netlabel_mgmt.c
>>>===================================================================
>>>--- net-2.6.orig/net/netlabel/netlabel_mgmt.c
>>>+++ net-2.6/net/netlabel/netlabel_mgmt.c
>>>@@ -108,7 +108,7 @@ static int netlbl_mgmt_add(struct sk_buf
>>>
>>> switch (entry->type) {
>>> case NETLBL_NLTYPE_UNLABELED:
>>>- ret_val = netlbl_domhsh_add(entry);
>>>+ ret_val = netlbl_domhsh_add(entry, NETLINK_CB(skb).sid);
>>> break;
>>> case NETLBL_NLTYPE_CIPSOV4:
>>> if (!info->attrs[NLBL_MGMT_A_CV4DOI])
>>>@@ -125,7 +125,7 @@ static int netlbl_mgmt_add(struct sk_buf
>>> rcu_read_unlock();
>>> goto add_failure;
>>> }
>>>- ret_val = netlbl_domhsh_add(entry);
>>>+ ret_val = netlbl_domhsh_add(entry, NETLINK_CB(skb).sid);
>>> rcu_read_unlock();
>>> break;
>>> default:
>>>@@ -161,7 +161,7 @@ static int netlbl_mgmt_remove(struct sk_
>>> return -EINVAL;
>>>
>>> domain = nla_data(info->attrs[NLBL_MGMT_A_DOMAIN]);
>>>- return netlbl_domhsh_remove(domain);
>>>+ return netlbl_domhsh_remove(domain, NETLINK_CB(skb).sid);
>>> }
>>>
>>> /**
>>>@@ -277,7 +277,8 @@ static int netlbl_mgmt_adddef(struct sk_
>>>
>>> switch (entry->type) {
>>> case NETLBL_NLTYPE_UNLABELED:
>>>- ret_val = netlbl_domhsh_add_default(entry);
>>>+ ret_val = netlbl_domhsh_add_default(entry,
>>>+ NETLINK_CB(skb).sid);
>>> break;
>>> case NETLBL_NLTYPE_CIPSOV4:
>>> if (!info->attrs[NLBL_MGMT_A_CV4DOI])
>>>@@ -294,7 +295,8 @@ static int netlbl_mgmt_adddef(struct sk_
>>> rcu_read_unlock();
>>> goto adddef_failure;
>>> }
>>>- ret_val = netlbl_domhsh_add_default(entry);
>>>+ ret_val = netlbl_domhsh_add_default(entry,
>>>+ NETLINK_CB(skb).sid);
>>> rcu_read_unlock();
>>> break;
>>> default:
>>>@@ -322,7 +324,7 @@ adddef_failure:
>>> */
>>> static int netlbl_mgmt_removedef(struct sk_buff *skb, struct genl_info
>>>*info) {
>>>- return netlbl_domhsh_remove_default();
>>>+ return netlbl_domhsh_remove_default(NETLINK_CB(skb).sid);
>>> }
>>>
>>> /**
>>>Index: net-2.6/net/netlabel/netlabel_unlabeled.c
>>>===================================================================
>>>--- net-2.6.orig/net/netlabel/netlabel_unlabeled.c
>>>+++ net-2.6/net/netlabel/netlabel_unlabeled.c
>>>@@ -64,6 +64,27 @@ static struct nla_policy netlbl_unlabel_
>>> };
>>>
>>> /*
>>>+ * Helper Functions
>>>+ */
>>>+
>>>+/**
>>>+ * netlbl_unlabel_acceptflg_set - Set the unlabeled accept flag
>>>+ * @value: desired value
>>>+ * @audit_secid: the LSM secid to use in the audit message
>>>+ *
>>>+ * Description:
>>>+ * Set the value of the unlabeled accept flag to @value.
>>>+ *
>>>+ */
>>>+static void netlbl_unlabel_acceptflg_set(u8 value, u32 audit_secid)
>>>+{
>>>+ atomic_set(&netlabel_unlabel_accept_flg, value);
>>>+ netlbl_audit_nomsg((value ?
>>>+ AUDIT_MAC_UNLBL_ACCEPT : AUDIT_MAC_UNLBL_DENY),
>>>+ audit_secid);
>>
>>Looking at how this is being used, I think only 1 message type should be used.
>>There are places in the audit system where we set a flag to 1 or 0, but only
>>have 1 message type. We record the old and new value. So, you'd need to pass
>>that to the logger.
>>
>>
>>>+}
>>>+
>>>+/*
>>> * NetLabel Command Handlers
>>> */
>>>
>>>@@ -79,18 +100,18 @@ static struct nla_policy netlbl_unlabel_
>>> */
>>> static int netlbl_unlabel_accept(struct sk_buff *skb, struct genl_info
>>>*info) {
>>>- int ret_val = -EINVAL;
>>> u8 value;
>>>
>>> if (info->attrs[NLBL_UNLABEL_A_ACPTFLG]) {
>>> value = nla_get_u8(info->attrs[NLBL_UNLABEL_A_ACPTFLG]);
>>> if (value == 1 || value == 0) {
>>>- atomic_set(&netlabel_unlabel_accept_flg, value);
>>>- ret_val = 0;
>>>+ netlbl_unlabel_acceptflg_set(value,
>>>+ NETLINK_CB(skb).sid);
>>>+ return 0;
>>> }
>>> }
>>>
>>>- return ret_val;
>>>+ return -EINVAL;
>>> }
>>>
>>> /**
>>>@@ -229,16 +250,19 @@ int netlbl_unlabel_defconf(void)
>>> {
>>> int ret_val;
>>> struct netlbl_dom_map *entry;
>>>+ u32 secid;
>>>+
>>>+ security_task_getsecid(current, &secid);
>>>
>>> entry = kzalloc(sizeof(*entry), GFP_KERNEL);
>>> if (entry == NULL)
>>> return -ENOMEM;
>>> entry->type = NETLBL_NLTYPE_UNLABELED;
>>>- ret_val = netlbl_domhsh_add_default(entry);
>>>+ ret_val = netlbl_domhsh_add_default(entry, secid);
>>> if (ret_val != 0)
>>> return ret_val;
>>>
>>>- atomic_set(&netlabel_unlabel_accept_flg, 1);
>>>+ netlbl_unlabel_acceptflg_set(1, secid);
>>>
>>> return 0;
>>> }
>>>Index: net-2.6/net/netlabel/netlabel_user.c
>>>===================================================================
>>>--- net-2.6.orig/net/netlabel/netlabel_user.c
>>>+++ net-2.6/net/netlabel/netlabel_user.c
>>>@@ -32,6 +32,9 @@
>>> #include <linux/types.h>
>>> #include <linux/list.h>
>>> #include <linux/socket.h>
>>>+#include <linux/audit.h>
>>>+#include <linux/tty.h>
>>>+#include <linux/security.h>
>>> #include <net/sock.h>
>>> #include <net/netlink.h>
>>> #include <net/genetlink.h>
>>>@@ -74,3 +77,91 @@ int netlbl_netlink_init(void)
>>>
>>> return 0;
>>> }
>>>+
>>>+/*
>>>+ * NetLabel Audit Functions
>>>+ */
>>>+
>>>+/**
>>>+ * netlbl_audit_start_common - Start an audit message
>>>+ * @type: audit message type
>>>+ * @secid: LSM context ID
>>>+ *
>>>+ * Description:
>>>+ * Start an audit message using the type specified in @type and fill the
>>>audit + * message with some fields common to all NetLabel audit messages.
>>>Returns + * a pointer to the audit buffer on success, NULL on failure.
>>>+ *
>>>+ */
>>>+struct audit_buffer *netlbl_audit_start_common(int type, u32 secid)
>>>+{
>>
>>Generally, logging functions are moved into auditsc.c where the context and
>>other functions are defined.
>>
>>
>>>+ struct audit_context *audit_ctx = current->audit_context;
>>>+ struct audit_buffer *audit_buf;
>>>+ uid_t audit_loginuid;
>>>+ const char *audit_tty;
>>>+ char audit_comm[sizeof(current->comm)];
>>>+ struct vm_area_struct *vma;
>>>+ char *secctx;
>>>+ u32 secctx_len;
>>>+
>>>+ audit_buf = audit_log_start(audit_ctx, GFP_ATOMIC, type);
>>>+ if (audit_buf == NULL)
>>>+ return NULL;
>>>+
>>>+ audit_loginuid = audit_get_loginuid(audit_ctx);
>>
>>Netlink is async protocol, you should use the loginuid of the netlink packet
>>and pass it into this function.
>>
>>
>>>+ if (current->signal &&
>>>+ current->signal->tty &&
>>>+ current->signal->tty->name)
>>>+ audit_tty = current->signal->tty->name;
>>>+ else
>>>+ audit_tty = "(none)";
>>
>>Netlink is an async protocol. How do you know that the sender is current? This
>>is a global comment, please check use of current everywhere.
>>
>>
>>>+ get_task_comm(audit_comm, current);
>>>+
>>>+ audit_log_format(audit_buf,
>>>+ "netlabel: auid=%u uid=%u tty=%s pid=%d",
>>>+ audit_loginuid,
>>>+ current->uid,
>>>+ audit_tty,
>>>+ current->pid);
>>
>>Why are you logging all this? When we add audit rules, all that we log is the
>>auid, and subj. If we need to log all this, we should probably have a helper
>>function that gets called by other config change loggers.
>>
>>
>>>+ audit_log_format(audit_buf, " comm=");
>>>+ audit_log_untrustedstring(audit_buf, audit_comm);
>>>+ if (current->mm) {
>>>+ down_read(¤t->mm->mmap_sem);
>>>+ vma = current->mm->mmap;
>>>+ while (vma) {
>>>+ if ((vma->vm_flags & VM_EXECUTABLE) &&
>>>+ vma->vm_file) {
>>>+ audit_log_d_path(audit_buf,
>>>+ " exe=",
>>>+ vma->vm_file->f_dentry,
>>>+ vma->vm_file->f_vfsmnt);
>>>+ break;
>>>+ }
>>>+ vma = vma->vm_next;
>>>+ }
>>>+ up_read(¤t->mm->mmap_sem);
>>>+ }
>>>+
>>
>>If this function was moved inside auditsc.c you could use a function there
>>that does this. But the question remains why all this data?
>>
>>
>>>+ if (secid != 0 &&
>>>+ security_secid_to_secctx(secid, &secctx, &secctx_len) == 0)
>>>+ audit_log_format(audit_buf, " subj=%s", secctx);
>>>+
>>>+ return audit_buf;
>>>+}
>>>+
>>>+/**
>>>+ * netlbl_audit_nomsg - Send an audit message without additional text
>>>+ * @type: audit message type
>>>+ * @secid: LSM context ID
>>>+ *
>>>+ * Description:
>>>+ * Send an audit message with only the common NetLabel audit fields.
>>>+ *
>>>+ */
>>>+void netlbl_audit_nomsg(int type, u32 secid)
>>>+{
>>>+ struct audit_buffer *audit_buf;
>>>+
>>>+ audit_buf = netlbl_audit_start_common(type, secid);
>>>+ audit_log_end(audit_buf);
>>
>>We normally record old and new value when changing flags like this.
>>
>>
>>>+}
>>>Index: net-2.6/net/netlabel/netlabel_user.h
>>>===================================================================
>>>--- net-2.6.orig/net/netlabel/netlabel_user.h
>>>+++ net-2.6/net/netlabel/netlabel_user.h
>>>@@ -34,6 +34,7 @@
>>> #include <linux/types.h>
>>> #include <linux/skbuff.h>
>>> #include <linux/capability.h>
>>>+#include <linux/audit.h>
>>> #include <net/netlink.h>
>>> #include <net/genetlink.h>
>>> #include <net/netlabel.h>
>>>@@ -75,4 +76,9 @@ static inline void *netlbl_netlink_hdr_p
>>>
>>> int netlbl_netlink_init(void);
>>>
>>>+/* NetLabel Audit Functions */
>>>+
>>>+struct audit_buffer *netlbl_audit_start_common(int type, u32 secid);
>>>+void netlbl_audit_nomsg(int type, u32 secid);
>>>+
>>> #endif
>>>
>>>--
>>>paul moore
>>>linux security @ hp
>>>
>>>--
>>>Linux-audit mailing list
>>>Linux-audit@redhat.com
>>>https://www.redhat.com/mailman/listinfo/linux-audit
>>
>>-
>>To unsubscribe from this list: send the line "unsubscribe netdev" in
>>the body of a message to majordomo@vger.kernel.org
>>More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
>
--
paul moore
linux security @ hp
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 1/1] NetLabel: add audit support for configuration changes
@ 2006-09-29 15:43 ` Paul Moore
0 siblings, 0 replies; 24+ messages in thread
From: Paul Moore @ 2006-09-29 15:43 UTC (permalink / raw)
To: David S. Miller; +Cc: James Morris, Steve Grubb, linux-audit, netdev, selinux
James Morris wrote:
> Dave,
>
> Looks like this patch needs to be reverted until these issues are
> resolved.
Yes, please revert this patch.
I posted an earlier version to the linux-audit list and waited for a day
to see if there any comments before submitting for inclusion, but
unfortunately it seems I didn't wait long enough for Steve. I'll deal
with Steve's comments once I handle the secid reconciliation patches as
I think they are more important right now.
> On Fri, 29 Sep 2006, Steve Grubb wrote:
>
>
>>On Thursday 28 September 2006 14:03, paul.moore@hp.com wrote:
>>
>>>This patch adds audit support to NetLabel, including six new audit message
>>>types shown below.
>>>
>>> #define AUDIT_MAC_UNLBL_ACCEPT 1406
>>> #define AUDIT_MAC_UNLBL_DENY 1407
>>> #define AUDIT_MAC_CIPSOV4_ADD 1408
>>> #define AUDIT_MAC_CIPSOV4_DEL 1409
>>> #define AUDIT_MAC_MAP_ADD 1410
>>> #define AUDIT_MAC_MAP_DEL 1411
>>>
>>>Please consider this for inclusion into 2.6.19.
>>>
>>>Signed-off-by: Paul Moore <paul.moore@hp.com>
>>>---
>>> include/linux/audit.h | 6 ++
>>> include/net/cipso_ipv4.h | 5 +-
>>> include/net/netlabel.h | 2
>>> net/ipv4/cipso_ipv4.c | 8 ++-
>>> net/netlabel/netlabel_cipso_v4.c | 43 +++++++++++++----
>>> net/netlabel/netlabel_domainhash.c | 54 +++++++++++++++++++--
>>> net/netlabel/netlabel_domainhash.h | 6 +-
>>> net/netlabel/netlabel_mgmt.c | 14 +++--
>>> net/netlabel/netlabel_unlabeled.c | 36 ++++++++++++--
>>> net/netlabel/netlabel_user.c | 91
>>>+++++++++++++++++++++++++++++++++++++ net/netlabel/netlabel_user.h |
>>> 6 ++
>>> 11 files changed, 235 insertions(+), 36 deletions(-)
>>>
>>>Index: net-2.6/include/linux/audit.h
>>>===================================================================
>>>--- net-2.6.orig/include/linux/audit.h
>>>+++ net-2.6/include/linux/audit.h
>>>@@ -95,6 +95,12 @@
>>> #define AUDIT_MAC_POLICY_LOAD 1403 /* Policy file load */
>>> #define AUDIT_MAC_STATUS 1404 /* Changed enforcing,permissive,off */
>>> #define AUDIT_MAC_CONFIG_CHANGE 1405 /* Changes to booleans */
>>>+#define AUDIT_MAC_UNLBL_ACCEPT 1406 /* NetLabel: allow unlabeled traffic */
>>>+#define AUDIT_MAC_UNLBL_DENY 1407 /* NetLabel: deny unlabeled traffic */
>>
>>Please drop the use of DENY per comments later down.
>>
>>
>>>+#define AUDIT_MAC_CIPSOV4_ADD 1408 /* NetLabel: add CIPSOv4 DOI entry */
>>>+#define AUDIT_MAC_CIPSOV4_DEL 1409 /* NetLabel: del CIPSOv4 DOI entry */
>>>+#define AUDIT_MAC_MAP_ADD 1410 /* NetLabel: add LSM domain mapping */
>>>+#define AUDIT_MAC_MAP_DEL 1411 /* NetLabel: del LSM domain mapping */
>>>
>>> #define AUDIT_FIRST_KERN_ANOM_MSG 1700
>>> #define AUDIT_LAST_KERN_ANOM_MSG 1799
>>>Index: net-2.6/include/net/cipso_ipv4.h
>>>===================================================================
>>>--- net-2.6.orig/include/net/cipso_ipv4.h
>>>+++ net-2.6/include/net/cipso_ipv4.h
>>>@@ -128,7 +128,9 @@ extern int cipso_v4_rbm_strictvalid;
>>>
>>> #ifdef CONFIG_NETLABEL
>>> int cipso_v4_doi_add(struct cipso_v4_doi *doi_def);
>>>-int cipso_v4_doi_remove(u32 doi, void (*callback) (struct rcu_head *
>>>head)); +int cipso_v4_doi_remove(u32 doi,
>>>+ u32 audit_secid,
>>>+ void (*callback) (struct rcu_head * head));
>>> struct cipso_v4_doi *cipso_v4_doi_getdef(u32 doi);
>>> int cipso_v4_doi_walk(u32 *skip_cnt,
>>> int (*callback) (struct cipso_v4_doi *doi_def, void *arg),
>>>@@ -143,6 +145,7 @@ static inline int cipso_v4_doi_add(struc
>>> }
>>>
>>> static inline int cipso_v4_doi_remove(u32 doi,
>>>+ u32 audit_secid,
>>> void (*callback) (struct rcu_head * head))
>>> {
>>> return 0;
>>>Index: net-2.6/include/net/netlabel.h
>>>===================================================================
>>>--- net-2.6.orig/include/net/netlabel.h
>>>+++ net-2.6/include/net/netlabel.h
>>>@@ -96,7 +96,7 @@
>>> struct netlbl_dom_map;
>>>
>>> /* Domain mapping operations */
>>>-int netlbl_domhsh_remove(const char *domain);
>>>+int netlbl_domhsh_remove(const char *domain, u32 audit_secid);
>>>
>>> /* LSM security attributes */
>>> struct netlbl_lsm_cache {
>>>Index: net-2.6/net/ipv4/cipso_ipv4.c
>>>===================================================================
>>>--- net-2.6.orig/net/ipv4/cipso_ipv4.c
>>>+++ net-2.6/net/ipv4/cipso_ipv4.c
>>>@@ -474,6 +474,7 @@ doi_add_failure_rlock:
>>> /**
>>> * cipso_v4_doi_remove - Remove an existing DOI from the CIPSO protocol
>>>engine * @doi: the DOI value
>>>+ * @audit_secid: the LSM secid to use in the audit message
>>> * @callback: the DOI cleanup/free callback
>>> *
>>> * Description:
>>>@@ -483,7 +484,9 @@ doi_add_failure_rlock:
>>> * success and negative values on failure.
>>> *
>>> */
>>>-int cipso_v4_doi_remove(u32 doi, void (*callback) (struct rcu_head *
>>>head)) +int cipso_v4_doi_remove(u32 doi,
>>>+ u32 audit_secid,
>>>+ void (*callback) (struct rcu_head * head))
>>> {
>>> struct cipso_v4_doi *doi_def;
>>> struct cipso_v4_domhsh_entry *dom_iter;
>>>@@ -502,7 +505,8 @@ int cipso_v4_doi_remove(u32 doi, void (*
>>> spin_unlock(&cipso_v4_doi_list_lock);
>>> list_for_each_entry_rcu(dom_iter, &doi_def->dom_list, list)
>>> if (dom_iter->valid)
>>>- netlbl_domhsh_remove(dom_iter->domain);
>>>+ netlbl_domhsh_remove(dom_iter->domain,
>>>+ audit_secid);
>>> cipso_v4_cache_invalidate();
>>> rcu_read_unlock();
>>>
>>>Index: net-2.6/net/netlabel/netlabel_cipso_v4.c
>>>===================================================================
>>>--- net-2.6.orig/net/netlabel/netlabel_cipso_v4.c
>>>+++ net-2.6/net/netlabel/netlabel_cipso_v4.c
>>>@@ -32,6 +32,7 @@
>>> #include <linux/socket.h>
>>> #include <linux/string.h>
>>> #include <linux/skbuff.h>
>>>+#include <linux/audit.h>
>>> #include <net/sock.h>
>>> #include <net/netlink.h>
>>> #include <net/genetlink.h>
>>>@@ -162,8 +163,7 @@ static int netlbl_cipsov4_add_std(struct
>>> int nla_a_rem;
>>> int nla_b_rem;
>>>
>>>- if (!info->attrs[NLBL_CIPSOV4_A_DOI] ||
>>>- !info->attrs[NLBL_CIPSOV4_A_TAGLST] ||
>>>+ if (!info->attrs[NLBL_CIPSOV4_A_TAGLST] ||
>>> !info->attrs[NLBL_CIPSOV4_A_MLSLVLLST])
>>> return -EINVAL;
>>>
>>>@@ -344,8 +344,7 @@ static int netlbl_cipsov4_add_pass(struc
>>> int ret_val;
>>> struct cipso_v4_doi *doi_def = NULL;
>>>
>>>- if (!info->attrs[NLBL_CIPSOV4_A_DOI] ||
>>>- !info->attrs[NLBL_CIPSOV4_A_TAGLST])
>>>+ if (!info->attrs[NLBL_CIPSOV4_A_TAGLST])
>>> return -EINVAL;
>>>
>>> doi_def = kmalloc(sizeof(*doi_def), GFP_KERNEL);
>>>@@ -381,21 +380,35 @@ static int netlbl_cipsov4_add(struct sk_
>>>
>>> {
>>> int ret_val = -EINVAL;
>>>- u32 map_type;
>>>+ u32 type;
>>>+ u32 doi;
>>>+ const char *type_str = "(unknown)";
>>>+ struct audit_buffer *audit_buf;
>>>
>>>- if (!info->attrs[NLBL_CIPSOV4_A_MTYPE])
>>>+ if (!info->attrs[NLBL_CIPSOV4_A_DOI] ||
>>>+ !info->attrs[NLBL_CIPSOV4_A_MTYPE])
>>> return -EINVAL;
>>>
>>>- map_type = nla_get_u32(info->attrs[NLBL_CIPSOV4_A_MTYPE]);
>>>- switch (map_type) {
>>>+ type = nla_get_u32(info->attrs[NLBL_CIPSOV4_A_MTYPE]);
>>>+ switch (type) {
>>> case CIPSO_V4_MAP_STD:
>>>+ type_str = "std";
>>> ret_val = netlbl_cipsov4_add_std(info);
>>> break;
>>> case CIPSO_V4_MAP_PASS:
>>>+ type_str = "pass";
>>> ret_val = netlbl_cipsov4_add_pass(info);
>>> break;
>>> }
>>>
>>>+ if (ret_val == 0) {
>>>+ doi = nla_get_u32(info->attrs[NLBL_CIPSOV4_A_DOI]);
>>>+ audit_buf = netlbl_audit_start_common(AUDIT_MAC_CIPSOV4_ADD,
>>>+ NETLINK_CB(skb).sid);
>>>+ audit_log_format(audit_buf, " doi=%u type=%s", doi, type_str);
>>
>>type field is already taken for another purpose, it needs to be renamed.
>>
>>
>>>+ audit_log_end(audit_buf);
>>>+ }
>>
>>Normally, we have a field res= that records whether or not the user was
>>successful in removing the rule. You would probably do something like
>>ret_val==0?1:0 but send a audit event regardless
>>
>>
>>>+
>>> return ret_val;
>>> }
>>>
>>>@@ -653,11 +666,21 @@ static int netlbl_cipsov4_listall(struct
>>> static int netlbl_cipsov4_remove(struct sk_buff *skb, struct genl_info
>>>*info) {
>>> int ret_val = -EINVAL;
>>>- u32 doi;
>>>+ u32 doi = 0;
>>>+ struct audit_buffer *audit_buf;
>>>
>>> if (info->attrs[NLBL_CIPSOV4_A_DOI]) {
>>> doi = nla_get_u32(info->attrs[NLBL_CIPSOV4_A_DOI]);
>>>- ret_val = cipso_v4_doi_remove(doi, netlbl_cipsov4_doi_free);
>>>+ ret_val = cipso_v4_doi_remove(doi,
>>>+ NETLINK_CB(skb).sid,
>>>+ netlbl_cipsov4_doi_free);
>>>+ }
>>>+
>>>+ if (ret_val == 0) {
>>>+ audit_buf = netlbl_audit_start_common(AUDIT_MAC_CIPSOV4_DEL,
>>>+ NETLINK_CB(skb).sid);
>>>+ audit_log_format(audit_buf, " doi=%u", doi);
>>>+ audit_log_end(audit_buf);
>>> }
>>
>>Same as above, global comment about res= being recorded no matter what.
>>
>>
>>> return ret_val;
>>>Index: net-2.6/net/netlabel/netlabel_domainhash.c
>>>===================================================================
>>>--- net-2.6.orig/net/netlabel/netlabel_domainhash.c
>>>+++ net-2.6/net/netlabel/netlabel_domainhash.c
>>>@@ -35,12 +35,14 @@
>>> #include <linux/skbuff.h>
>>> #include <linux/spinlock.h>
>>> #include <linux/string.h>
>>>+#include <linux/audit.h>
>>> #include <net/netlabel.h>
>>> #include <net/cipso_ipv4.h>
>>> #include <asm/bug.h>
>>>
>>> #include "netlabel_mgmt.h"
>>> #include "netlabel_domainhash.h"
>>>+#include "netlabel_user.h"
>>>
>>> struct netlbl_domhsh_tbl {
>>> struct list_head *tbl;
>>>@@ -186,6 +188,7 @@ int netlbl_domhsh_init(u32 size)
>>> /**
>>> * netlbl_domhsh_add - Adds a entry to the domain hash table
>>> * @entry: the entry to add
>>>+ * @audit_secid: the LSM secid to use in the audit message
>>> *
>>> * Description:
>>> * Adds a new entry to the domain hash table and handles any updates to
>>>the @@ -193,10 +196,12 @@ int netlbl_domhsh_init(u32 size)
>>> * negative on failure.
>>> *
>>> */
>>>-int netlbl_domhsh_add(struct netlbl_dom_map *entry)
>>>+int netlbl_domhsh_add(struct netlbl_dom_map *entry, u32 audit_secid)
>>> {
>>> int ret_val;
>>> u32 bkt;
>>>+ struct audit_buffer *audit_buf;
>>>+ char *audit_domain;
>>>
>>> switch (entry->type) {
>>> case NETLBL_NLTYPE_UNLABELED:
>>>@@ -236,6 +241,26 @@ int netlbl_domhsh_add(struct netlbl_dom_
>>> spin_unlock(&netlbl_domhsh_def_lock);
>>> } else
>>> ret_val = -EINVAL;
>>>+ if (ret_val == 0) {
>>>+ if (entry->domain != NULL)
>>>+ audit_domain = entry->domain;
>>>+ else
>>>+ audit_domain = "(default)";
>>>+ audit_buf = netlbl_audit_start_common(AUDIT_MAC_MAP_ADD,
>>>+ audit_secid);
>>>+ audit_log_format(audit_buf, " domain=%s", audit_domain);
>>>+ switch (entry->type) {
>>>+ case NETLBL_NLTYPE_UNLABELED:
>>>+ audit_log_format(audit_buf, " protocol=unlbl");
>>>+ break;
>>>+ case NETLBL_NLTYPE_CIPSOV4:
>>>+ audit_log_format(audit_buf,
>>>+ " protocol=cipsov4 doi=%u",
>>>+ entry->type_def.cipsov4->doi);
>>>+ break;
>>>+ }
>>>+ audit_log_end(audit_buf);
>>>+ }
>>> rcu_read_unlock();
>>>
>>> if (ret_val != 0) {
>>>@@ -254,6 +279,7 @@ int netlbl_domhsh_add(struct netlbl_dom_
>>> /**
>>> * netlbl_domhsh_add_default - Adds the default entry to the domain hash
>>>table * @entry: the entry to add
>>>+ * @audit_secid: the LSM secid to use in the audit message
>>> *
>>> * Description:
>>> * Adds a new default entry to the domain hash table and handles any
>>>updates @@ -261,14 +287,15 @@ int netlbl_domhsh_add(struct netlbl_dom_
>>> * negative on failure.
>>> *
>>> */
>>>-int netlbl_domhsh_add_default(struct netlbl_dom_map *entry)
>>>+int netlbl_domhsh_add_default(struct netlbl_dom_map *entry, u32
>>>audit_secid) {
>>>- return netlbl_domhsh_add(entry);
>>>+ return netlbl_domhsh_add(entry, audit_secid);
>>> }
>>>
>>> /**
>>> * netlbl_domhsh_remove - Removes an entry from the domain hash table
>>> * @domain: the domain to remove
>>>+ * @audit_secid: the LSM secid to use in the audit message
>>> *
>>> * Description:
>>> * Removes an entry from the domain hash table and handles any updates to
>>>the @@ -276,10 +303,12 @@ int netlbl_domhsh_add_default(struct net
>>> * negative on failure.
>>> *
>>> */
>>>-int netlbl_domhsh_remove(const char *domain)
>>>+int netlbl_domhsh_remove(const char *domain, u32 audit_secid)
>>> {
>>> int ret_val = -ENOENT;
>>> struct netlbl_dom_map *entry;
>>>+ struct audit_buffer *audit_buf;
>>>+ char *audit_domain;
>>>
>>> rcu_read_lock();
>>> if (domain != NULL)
>>>@@ -316,8 +345,18 @@ int netlbl_domhsh_remove(const char *dom
>>> ret_val = -ENOENT;
>>> spin_unlock(&netlbl_domhsh_def_lock);
>>> }
>>>- if (ret_val == 0)
>>>+ if (ret_val == 0) {
>>>+ if (entry->domain != NULL)
>>>+ audit_domain = entry->domain;
>>>+ else
>>>+ audit_domain = "(default)";
>>>+ audit_buf = netlbl_audit_start_common(AUDIT_MAC_MAP_DEL,
>>>+ audit_secid);
>>>+ audit_log_format(audit_buf, " domain=%s", audit_domain);
>>>+ audit_log_end(audit_buf);
>>>+
>>> call_rcu(&entry->rcu, netlbl_domhsh_free_entry);
>>>+ }
>>>
>>> remove_return:
>>> rcu_read_unlock();
>>>@@ -326,6 +365,7 @@ remove_return:
>>>
>>> /**
>>> * netlbl_domhsh_remove_default - Removes the default entry from the table
>>>+ * @audit_secid: the LSM secid to use in the audit message
>>> *
>>> * Description:
>>> * Removes/resets the default entry for the domain hash table and handles
>>>any @@ -333,9 +373,9 @@ remove_return:
>>> * success, non-zero on failure.
>>> *
>>> */
>>>-int netlbl_domhsh_remove_default(void)
>>>+int netlbl_domhsh_remove_default(u32 audit_secid)
>>> {
>>>- return netlbl_domhsh_remove(NULL);
>>>+ return netlbl_domhsh_remove(NULL, audit_secid);
>>> }
>>>
>>> /**
>>>Index: net-2.6/net/netlabel/netlabel_domainhash.h
>>>===================================================================
>>>--- net-2.6.orig/net/netlabel/netlabel_domainhash.h
>>>+++ net-2.6/net/netlabel/netlabel_domainhash.h
>>>@@ -57,9 +57,9 @@ struct netlbl_dom_map {
>>> int netlbl_domhsh_init(u32 size);
>>>
>>> /* Manipulate the domain hash table */
>>>-int netlbl_domhsh_add(struct netlbl_dom_map *entry);
>>>-int netlbl_domhsh_add_default(struct netlbl_dom_map *entry);
>>>-int netlbl_domhsh_remove_default(void);
>>>+int netlbl_domhsh_add(struct netlbl_dom_map *entry, u32 audit_secid);
>>>+int netlbl_domhsh_add_default(struct netlbl_dom_map *entry, u32
>>>audit_secid); +int netlbl_domhsh_remove_default(u32 audit_secid);
>>> struct netlbl_dom_map *netlbl_domhsh_getentry(const char *domain);
>>> int netlbl_domhsh_walk(u32 *skip_bkt,
>>> u32 *skip_chain,
>>>Index: net-2.6/net/netlabel/netlabel_mgmt.c
>>>===================================================================
>>>--- net-2.6.orig/net/netlabel/netlabel_mgmt.c
>>>+++ net-2.6/net/netlabel/netlabel_mgmt.c
>>>@@ -108,7 +108,7 @@ static int netlbl_mgmt_add(struct sk_buf
>>>
>>> switch (entry->type) {
>>> case NETLBL_NLTYPE_UNLABELED:
>>>- ret_val = netlbl_domhsh_add(entry);
>>>+ ret_val = netlbl_domhsh_add(entry, NETLINK_CB(skb).sid);
>>> break;
>>> case NETLBL_NLTYPE_CIPSOV4:
>>> if (!info->attrs[NLBL_MGMT_A_CV4DOI])
>>>@@ -125,7 +125,7 @@ static int netlbl_mgmt_add(struct sk_buf
>>> rcu_read_unlock();
>>> goto add_failure;
>>> }
>>>- ret_val = netlbl_domhsh_add(entry);
>>>+ ret_val = netlbl_domhsh_add(entry, NETLINK_CB(skb).sid);
>>> rcu_read_unlock();
>>> break;
>>> default:
>>>@@ -161,7 +161,7 @@ static int netlbl_mgmt_remove(struct sk_
>>> return -EINVAL;
>>>
>>> domain = nla_data(info->attrs[NLBL_MGMT_A_DOMAIN]);
>>>- return netlbl_domhsh_remove(domain);
>>>+ return netlbl_domhsh_remove(domain, NETLINK_CB(skb).sid);
>>> }
>>>
>>> /**
>>>@@ -277,7 +277,8 @@ static int netlbl_mgmt_adddef(struct sk_
>>>
>>> switch (entry->type) {
>>> case NETLBL_NLTYPE_UNLABELED:
>>>- ret_val = netlbl_domhsh_add_default(entry);
>>>+ ret_val = netlbl_domhsh_add_default(entry,
>>>+ NETLINK_CB(skb).sid);
>>> break;
>>> case NETLBL_NLTYPE_CIPSOV4:
>>> if (!info->attrs[NLBL_MGMT_A_CV4DOI])
>>>@@ -294,7 +295,8 @@ static int netlbl_mgmt_adddef(struct sk_
>>> rcu_read_unlock();
>>> goto adddef_failure;
>>> }
>>>- ret_val = netlbl_domhsh_add_default(entry);
>>>+ ret_val = netlbl_domhsh_add_default(entry,
>>>+ NETLINK_CB(skb).sid);
>>> rcu_read_unlock();
>>> break;
>>> default:
>>>@@ -322,7 +324,7 @@ adddef_failure:
>>> */
>>> static int netlbl_mgmt_removedef(struct sk_buff *skb, struct genl_info
>>>*info) {
>>>- return netlbl_domhsh_remove_default();
>>>+ return netlbl_domhsh_remove_default(NETLINK_CB(skb).sid);
>>> }
>>>
>>> /**
>>>Index: net-2.6/net/netlabel/netlabel_unlabeled.c
>>>===================================================================
>>>--- net-2.6.orig/net/netlabel/netlabel_unlabeled.c
>>>+++ net-2.6/net/netlabel/netlabel_unlabeled.c
>>>@@ -64,6 +64,27 @@ static struct nla_policy netlbl_unlabel_
>>> };
>>>
>>> /*
>>>+ * Helper Functions
>>>+ */
>>>+
>>>+/**
>>>+ * netlbl_unlabel_acceptflg_set - Set the unlabeled accept flag
>>>+ * @value: desired value
>>>+ * @audit_secid: the LSM secid to use in the audit message
>>>+ *
>>>+ * Description:
>>>+ * Set the value of the unlabeled accept flag to @value.
>>>+ *
>>>+ */
>>>+static void netlbl_unlabel_acceptflg_set(u8 value, u32 audit_secid)
>>>+{
>>>+ atomic_set(&netlabel_unlabel_accept_flg, value);
>>>+ netlbl_audit_nomsg((value ?
>>>+ AUDIT_MAC_UNLBL_ACCEPT : AUDIT_MAC_UNLBL_DENY),
>>>+ audit_secid);
>>
>>Looking at how this is being used, I think only 1 message type should be used.
>>There are places in the audit system where we set a flag to 1 or 0, but only
>>have 1 message type. We record the old and new value. So, you'd need to pass
>>that to the logger.
>>
>>
>>>+}
>>>+
>>>+/*
>>> * NetLabel Command Handlers
>>> */
>>>
>>>@@ -79,18 +100,18 @@ static struct nla_policy netlbl_unlabel_
>>> */
>>> static int netlbl_unlabel_accept(struct sk_buff *skb, struct genl_info
>>>*info) {
>>>- int ret_val = -EINVAL;
>>> u8 value;
>>>
>>> if (info->attrs[NLBL_UNLABEL_A_ACPTFLG]) {
>>> value = nla_get_u8(info->attrs[NLBL_UNLABEL_A_ACPTFLG]);
>>> if (value == 1 || value == 0) {
>>>- atomic_set(&netlabel_unlabel_accept_flg, value);
>>>- ret_val = 0;
>>>+ netlbl_unlabel_acceptflg_set(value,
>>>+ NETLINK_CB(skb).sid);
>>>+ return 0;
>>> }
>>> }
>>>
>>>- return ret_val;
>>>+ return -EINVAL;
>>> }
>>>
>>> /**
>>>@@ -229,16 +250,19 @@ int netlbl_unlabel_defconf(void)
>>> {
>>> int ret_val;
>>> struct netlbl_dom_map *entry;
>>>+ u32 secid;
>>>+
>>>+ security_task_getsecid(current, &secid);
>>>
>>> entry = kzalloc(sizeof(*entry), GFP_KERNEL);
>>> if (entry == NULL)
>>> return -ENOMEM;
>>> entry->type = NETLBL_NLTYPE_UNLABELED;
>>>- ret_val = netlbl_domhsh_add_default(entry);
>>>+ ret_val = netlbl_domhsh_add_default(entry, secid);
>>> if (ret_val != 0)
>>> return ret_val;
>>>
>>>- atomic_set(&netlabel_unlabel_accept_flg, 1);
>>>+ netlbl_unlabel_acceptflg_set(1, secid);
>>>
>>> return 0;
>>> }
>>>Index: net-2.6/net/netlabel/netlabel_user.c
>>>===================================================================
>>>--- net-2.6.orig/net/netlabel/netlabel_user.c
>>>+++ net-2.6/net/netlabel/netlabel_user.c
>>>@@ -32,6 +32,9 @@
>>> #include <linux/types.h>
>>> #include <linux/list.h>
>>> #include <linux/socket.h>
>>>+#include <linux/audit.h>
>>>+#include <linux/tty.h>
>>>+#include <linux/security.h>
>>> #include <net/sock.h>
>>> #include <net/netlink.h>
>>> #include <net/genetlink.h>
>>>@@ -74,3 +77,91 @@ int netlbl_netlink_init(void)
>>>
>>> return 0;
>>> }
>>>+
>>>+/*
>>>+ * NetLabel Audit Functions
>>>+ */
>>>+
>>>+/**
>>>+ * netlbl_audit_start_common - Start an audit message
>>>+ * @type: audit message type
>>>+ * @secid: LSM context ID
>>>+ *
>>>+ * Description:
>>>+ * Start an audit message using the type specified in @type and fill the
>>>audit + * message with some fields common to all NetLabel audit messages.
>>>Returns + * a pointer to the audit buffer on success, NULL on failure.
>>>+ *
>>>+ */
>>>+struct audit_buffer *netlbl_audit_start_common(int type, u32 secid)
>>>+{
>>
>>Generally, logging functions are moved into auditsc.c where the context and
>>other functions are defined.
>>
>>
>>>+ struct audit_context *audit_ctx = current->audit_context;
>>>+ struct audit_buffer *audit_buf;
>>>+ uid_t audit_loginuid;
>>>+ const char *audit_tty;
>>>+ char audit_comm[sizeof(current->comm)];
>>>+ struct vm_area_struct *vma;
>>>+ char *secctx;
>>>+ u32 secctx_len;
>>>+
>>>+ audit_buf = audit_log_start(audit_ctx, GFP_ATOMIC, type);
>>>+ if (audit_buf == NULL)
>>>+ return NULL;
>>>+
>>>+ audit_loginuid = audit_get_loginuid(audit_ctx);
>>
>>Netlink is async protocol, you should use the loginuid of the netlink packet
>>and pass it into this function.
>>
>>
>>>+ if (current->signal &&
>>>+ current->signal->tty &&
>>>+ current->signal->tty->name)
>>>+ audit_tty = current->signal->tty->name;
>>>+ else
>>>+ audit_tty = "(none)";
>>
>>Netlink is an async protocol. How do you know that the sender is current? This
>>is a global comment, please check use of current everywhere.
>>
>>
>>>+ get_task_comm(audit_comm, current);
>>>+
>>>+ audit_log_format(audit_buf,
>>>+ "netlabel: auid=%u uid=%u tty=%s pid=%d",
>>>+ audit_loginuid,
>>>+ current->uid,
>>>+ audit_tty,
>>>+ current->pid);
>>
>>Why are you logging all this? When we add audit rules, all that we log is the
>>auid, and subj. If we need to log all this, we should probably have a helper
>>function that gets called by other config change loggers.
>>
>>
>>>+ audit_log_format(audit_buf, " comm=");
>>>+ audit_log_untrustedstring(audit_buf, audit_comm);
>>>+ if (current->mm) {
>>>+ down_read(¤t->mm->mmap_sem);
>>>+ vma = current->mm->mmap;
>>>+ while (vma) {
>>>+ if ((vma->vm_flags & VM_EXECUTABLE) &&
>>>+ vma->vm_file) {
>>>+ audit_log_d_path(audit_buf,
>>>+ " exe=",
>>>+ vma->vm_file->f_dentry,
>>>+ vma->vm_file->f_vfsmnt);
>>>+ break;
>>>+ }
>>>+ vma = vma->vm_next;
>>>+ }
>>>+ up_read(¤t->mm->mmap_sem);
>>>+ }
>>>+
>>
>>If this function was moved inside auditsc.c you could use a function there
>>that does this. But the question remains why all this data?
>>
>>
>>>+ if (secid != 0 &&
>>>+ security_secid_to_secctx(secid, &secctx, &secctx_len) == 0)
>>>+ audit_log_format(audit_buf, " subj=%s", secctx);
>>>+
>>>+ return audit_buf;
>>>+}
>>>+
>>>+/**
>>>+ * netlbl_audit_nomsg - Send an audit message without additional text
>>>+ * @type: audit message type
>>>+ * @secid: LSM context ID
>>>+ *
>>>+ * Description:
>>>+ * Send an audit message with only the common NetLabel audit fields.
>>>+ *
>>>+ */
>>>+void netlbl_audit_nomsg(int type, u32 secid)
>>>+{
>>>+ struct audit_buffer *audit_buf;
>>>+
>>>+ audit_buf = netlbl_audit_start_common(type, secid);
>>>+ audit_log_end(audit_buf);
>>
>>We normally record old and new value when changing flags like this.
>>
>>
>>>+}
>>>Index: net-2.6/net/netlabel/netlabel_user.h
>>>===================================================================
>>>--- net-2.6.orig/net/netlabel/netlabel_user.h
>>>+++ net-2.6/net/netlabel/netlabel_user.h
>>>@@ -34,6 +34,7 @@
>>> #include <linux/types.h>
>>> #include <linux/skbuff.h>
>>> #include <linux/capability.h>
>>>+#include <linux/audit.h>
>>> #include <net/netlink.h>
>>> #include <net/genetlink.h>
>>> #include <net/netlabel.h>
>>>@@ -75,4 +76,9 @@ static inline void *netlbl_netlink_hdr_p
>>>
>>> int netlbl_netlink_init(void);
>>>
>>>+/* NetLabel Audit Functions */
>>>+
>>>+struct audit_buffer *netlbl_audit_start_common(int type, u32 secid);
>>>+void netlbl_audit_nomsg(int type, u32 secid);
>>>+
>>> #endif
>>>
>>>--
>>>paul moore
>>>linux security @ hp
>>>
>>>--
>>>Linux-audit mailing list
>>>Linux-audit@redhat.com
>>>https://www.redhat.com/mailman/listinfo/linux-audit
>>
>>-
>>To unsubscribe from this list: send the line "unsubscribe netdev" in
>>the body of a message to majordomo@vger.kernel.org
>>More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
>
--
paul moore
linux security @ hp
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/1] NetLabel: add audit support for configuration changes
2006-09-29 14:08 ` Steve Grubb
@ 2006-09-29 18:09 ` Paul Moore
-1 siblings, 0 replies; 24+ messages in thread
From: Paul Moore @ 2006-09-29 18:09 UTC (permalink / raw)
To: Steve Grubb; +Cc: netdev, linux-audit, selinux
Steve Grubb wrote:
> On Thursday 28 September 2006 14:03, paul.moore@hp.com wrote:
>>@@ -381,21 +380,35 @@ static int netlbl_cipsov4_add(struct sk_
>>
>> {
>> int ret_val = -EINVAL;
>>- u32 map_type;
>>+ u32 type;
>>+ u32 doi;
>>+ const char *type_str = "(unknown)";
>>+ struct audit_buffer *audit_buf;
>>
>>- if (!info->attrs[NLBL_CIPSOV4_A_MTYPE])
>>+ if (!info->attrs[NLBL_CIPSOV4_A_DOI] ||
>>+ !info->attrs[NLBL_CIPSOV4_A_MTYPE])
>> return -EINVAL;
>>
>>- map_type = nla_get_u32(info->attrs[NLBL_CIPSOV4_A_MTYPE]);
>>- switch (map_type) {
>>+ type = nla_get_u32(info->attrs[NLBL_CIPSOV4_A_MTYPE]);
>>+ switch (type) {
>> case CIPSO_V4_MAP_STD:
>>+ type_str = "std";
>> ret_val = netlbl_cipsov4_add_std(info);
>> break;
>> case CIPSO_V4_MAP_PASS:
>>+ type_str = "pass";
>> ret_val = netlbl_cipsov4_add_pass(info);
>> break;
>> }
>>
>>+ if (ret_val == 0) {
>>+ doi = nla_get_u32(info->attrs[NLBL_CIPSOV4_A_DOI]);
>>+ audit_buf = netlbl_audit_start_common(AUDIT_MAC_CIPSOV4_ADD,
>>+ NETLINK_CB(skb).sid);
>>+ audit_log_format(audit_buf, " doi=%u type=%s", doi, type_str);
>
>
> type field is already taken for another purpose, it needs to be renamed.
If we can't have duplicate field names I would propose prefixing both
these fields (and doing similar things with the other NetLabel specific
fields) with a "cipso_" making them "cipso_doi" and "cipso_type".
If this isn't acceptable please suggest names which you feel are
appropriate.
>>+/**
>>+ * netlbl_unlabel_acceptflg_set - Set the unlabeled accept flag
>>+ * @value: desired value
>>+ * @audit_secid: the LSM secid to use in the audit message
>>+ *
>>+ * Description:
>>+ * Set the value of the unlabeled accept flag to @value.
>>+ *
>>+ */
>>+static void netlbl_unlabel_acceptflg_set(u8 value, u32 audit_secid)
>>+{
>>+ atomic_set(&netlabel_unlabel_accept_flg, value);
>>+ netlbl_audit_nomsg((value ?
>>+ AUDIT_MAC_UNLBL_ACCEPT : AUDIT_MAC_UNLBL_DENY),
>>+ audit_secid);
>
> Looking at how this is being used, I think only 1 message type should be used.
> There are places in the audit system where we set a flag to 1 or 0, but only
> have 1 message type. We record the old and new value. So, you'd need to pass
> that to the logger.
With that in mind I would probably change the message type to
AUDIT_MAC_UNLBL_ALLOW and use a "unlbl_accept" field; is that okay? If
not please suggest something you would find acceptable.
>>+/**
>>+ * netlbl_audit_start_common - Start an audit message
>>+ * @type: audit message type
>>+ * @secid: LSM context ID
>>+ *
>>+ * Description:
>>+ * Start an audit message using the type specified in @type and fill the
>>audit + * message with some fields common to all NetLabel audit messages.
>>Returns + * a pointer to the audit buffer on success, NULL on failure.
>>+ *
>>+ */
>>+struct audit_buffer *netlbl_audit_start_common(int type, u32 secid)
>>+{
>
> Generally, logging functions are moved into auditsc.c where the context and
> other functions are defined.
How about leaving this for a future revision? I'd like this first
attempt to be relatively self contained. James Morris has made other
comments along the same lines.
>>+ audit_log_format(audit_buf,
>>+ "netlabel: auid=%u uid=%u tty=%s pid=%d",
>>+ audit_loginuid,
>>+ current->uid,
>>+ audit_tty,
>>+ current->pid);
>
>
> Why are you logging all this? When we add audit rules, all that we log is the
> auid, and subj. If we need to log all this, we should probably have a helper
> function that gets called by other config change loggers.
If I drop the uid, tty, and pid fields will this be acceptable?
>>+ audit_log_format(audit_buf, " comm=");
>>+ audit_log_untrustedstring(audit_buf, audit_comm);
>>+ if (current->mm) {
>>+ down_read(¤t->mm->mmap_sem);
>>+ vma = current->mm->mmap;
>>+ while (vma) {
>>+ if ((vma->vm_flags & VM_EXECUTABLE) &&
>>+ vma->vm_file) {
>>+ audit_log_d_path(audit_buf,
>>+ " exe=",
>>+ vma->vm_file->f_dentry,
>>+ vma->vm_file->f_vfsmnt);
>>+ break;
>>+ }
>>+ vma = vma->vm_next;
>>+ }
>>+ up_read(¤t->mm->mmap_sem);
>>+ }
>>+
>
>
> If this function was moved inside auditsc.c you could use a function there
> that does this. But the question remains why all this data?
In the ideal world would you prefer this to be removed?
--
paul moore
linux security @ hp
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 1/1] NetLabel: add audit support for configuration changes
@ 2006-09-29 18:09 ` Paul Moore
0 siblings, 0 replies; 24+ messages in thread
From: Paul Moore @ 2006-09-29 18:09 UTC (permalink / raw)
To: Steve Grubb; +Cc: linux-audit, netdev, selinux
Steve Grubb wrote:
> On Thursday 28 September 2006 14:03, paul.moore@hp.com wrote:
>>@@ -381,21 +380,35 @@ static int netlbl_cipsov4_add(struct sk_
>>
>> {
>> int ret_val = -EINVAL;
>>- u32 map_type;
>>+ u32 type;
>>+ u32 doi;
>>+ const char *type_str = "(unknown)";
>>+ struct audit_buffer *audit_buf;
>>
>>- if (!info->attrs[NLBL_CIPSOV4_A_MTYPE])
>>+ if (!info->attrs[NLBL_CIPSOV4_A_DOI] ||
>>+ !info->attrs[NLBL_CIPSOV4_A_MTYPE])
>> return -EINVAL;
>>
>>- map_type = nla_get_u32(info->attrs[NLBL_CIPSOV4_A_MTYPE]);
>>- switch (map_type) {
>>+ type = nla_get_u32(info->attrs[NLBL_CIPSOV4_A_MTYPE]);
>>+ switch (type) {
>> case CIPSO_V4_MAP_STD:
>>+ type_str = "std";
>> ret_val = netlbl_cipsov4_add_std(info);
>> break;
>> case CIPSO_V4_MAP_PASS:
>>+ type_str = "pass";
>> ret_val = netlbl_cipsov4_add_pass(info);
>> break;
>> }
>>
>>+ if (ret_val == 0) {
>>+ doi = nla_get_u32(info->attrs[NLBL_CIPSOV4_A_DOI]);
>>+ audit_buf = netlbl_audit_start_common(AUDIT_MAC_CIPSOV4_ADD,
>>+ NETLINK_CB(skb).sid);
>>+ audit_log_format(audit_buf, " doi=%u type=%s", doi, type_str);
>
>
> type field is already taken for another purpose, it needs to be renamed.
If we can't have duplicate field names I would propose prefixing both
these fields (and doing similar things with the other NetLabel specific
fields) with a "cipso_" making them "cipso_doi" and "cipso_type".
If this isn't acceptable please suggest names which you feel are
appropriate.
>>+/**
>>+ * netlbl_unlabel_acceptflg_set - Set the unlabeled accept flag
>>+ * @value: desired value
>>+ * @audit_secid: the LSM secid to use in the audit message
>>+ *
>>+ * Description:
>>+ * Set the value of the unlabeled accept flag to @value.
>>+ *
>>+ */
>>+static void netlbl_unlabel_acceptflg_set(u8 value, u32 audit_secid)
>>+{
>>+ atomic_set(&netlabel_unlabel_accept_flg, value);
>>+ netlbl_audit_nomsg((value ?
>>+ AUDIT_MAC_UNLBL_ACCEPT : AUDIT_MAC_UNLBL_DENY),
>>+ audit_secid);
>
> Looking at how this is being used, I think only 1 message type should be used.
> There are places in the audit system where we set a flag to 1 or 0, but only
> have 1 message type. We record the old and new value. So, you'd need to pass
> that to the logger.
With that in mind I would probably change the message type to
AUDIT_MAC_UNLBL_ALLOW and use a "unlbl_accept" field; is that okay? If
not please suggest something you would find acceptable.
>>+/**
>>+ * netlbl_audit_start_common - Start an audit message
>>+ * @type: audit message type
>>+ * @secid: LSM context ID
>>+ *
>>+ * Description:
>>+ * Start an audit message using the type specified in @type and fill the
>>audit + * message with some fields common to all NetLabel audit messages.
>>Returns + * a pointer to the audit buffer on success, NULL on failure.
>>+ *
>>+ */
>>+struct audit_buffer *netlbl_audit_start_common(int type, u32 secid)
>>+{
>
> Generally, logging functions are moved into auditsc.c where the context and
> other functions are defined.
How about leaving this for a future revision? I'd like this first
attempt to be relatively self contained. James Morris has made other
comments along the same lines.
>>+ audit_log_format(audit_buf,
>>+ "netlabel: auid=%u uid=%u tty=%s pid=%d",
>>+ audit_loginuid,
>>+ current->uid,
>>+ audit_tty,
>>+ current->pid);
>
>
> Why are you logging all this? When we add audit rules, all that we log is the
> auid, and subj. If we need to log all this, we should probably have a helper
> function that gets called by other config change loggers.
If I drop the uid, tty, and pid fields will this be acceptable?
>>+ audit_log_format(audit_buf, " comm=");
>>+ audit_log_untrustedstring(audit_buf, audit_comm);
>>+ if (current->mm) {
>>+ down_read(¤t->mm->mmap_sem);
>>+ vma = current->mm->mmap;
>>+ while (vma) {
>>+ if ((vma->vm_flags & VM_EXECUTABLE) &&
>>+ vma->vm_file) {
>>+ audit_log_d_path(audit_buf,
>>+ " exe=",
>>+ vma->vm_file->f_dentry,
>>+ vma->vm_file->f_vfsmnt);
>>+ break;
>>+ }
>>+ vma = vma->vm_next;
>>+ }
>>+ up_read(¤t->mm->mmap_sem);
>>+ }
>>+
>
>
> If this function was moved inside auditsc.c you could use a function there
> that does this. But the question remains why all this data?
In the ideal world would you prefer this to be removed?
--
paul moore
linux security @ hp
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 1/1] NetLabel: add audit support for configuration changes
2006-09-29 18:09 ` Paul Moore
@ 2006-09-29 18:35 ` Steve Grubb
-1 siblings, 0 replies; 24+ messages in thread
From: Steve Grubb @ 2006-09-29 18:35 UTC (permalink / raw)
To: Paul Moore; +Cc: netdev, linux-audit, selinux
On Friday 29 September 2006 14:09, Paul Moore wrote:
> > type field is already taken for another purpose, it needs to be renamed.
>
> If we can't have duplicate field names I would propose prefixing both
> these fields (and doing similar things with the other NetLabel specific
> fields) with a "cipso_" making them "cipso_doi" and "cipso_type".
That would be fine. This limits future field name collisions.
> >>+/**
> >>+ * netlbl_unlabel_acceptflg_set - Set the unlabeled accept flag
> >>+ * @value: desired value
> >>+ * @audit_secid: the LSM secid to use in the audit message
> >>+ *
> >>+ * Description:
> >>+ * Set the value of the unlabeled accept flag to @value.
> >>+ *
> >>+ */
> >>+static void netlbl_unlabel_acceptflg_set(u8 value, u32 audit_secid)
> >>+{
> >>+ atomic_set(&netlabel_unlabel_accept_flg, value);
> >>+ netlbl_audit_nomsg((value ?
> >>+ AUDIT_MAC_UNLBL_ACCEPT : AUDIT_MAC_UNLBL_DENY),
> >>+ audit_secid);
> >
> > Looking at how this is being used, I think only 1 message type should be
> > used. There are places in the audit system where we set a flag to 1 or 0,
> > but only have 1 message type. We record the old and new value. So, you'd
> > need to pass that to the logger.
>
> With that in mind I would probably change the message type to
> AUDIT_MAC_UNLBL_ALLOW and use a "unlbl_accept" field; is that okay?
That would be fine. Just a quick note...we have generally been "old " to
indicate the previous value. Example, "backlog=512 old=256".
> >>+/**
> >>+ * netlbl_audit_start_common - Start an audit message
> >>+ * @type: audit message type
> >>+ * @secid: LSM context ID
> >>+ *
> >>+ * Description:
> >>+ * Start an audit message using the type specified in @type and fill the
> >>audit + * message with some fields common to all NetLabel audit messages.
> >>Returns + * a pointer to the audit buffer on success, NULL on failure.
> >>+ *
> >>+ */
> >>+struct audit_buffer *netlbl_audit_start_common(int type, u32 secid)
> >>+{
> >
> > Generally, logging functions are moved into auditsc.c where the context
> > and other functions are defined.
>
> How about leaving this for a future revision?
Come to think of it, you don't need to move it. The reason to move it is to
access the context and use helper functions related to it. But I found that
you were using "current" which may not always be the sender. So if you cannot
use current, most of the stuff you are logging can't be, so the event being
logged becomes simpler and you don't need to move it.
I have not traced through all the code, but if you do any security checks
before taking the rules, be careful not to use current.
> >>+ audit_log_format(audit_buf,
> >>+ "netlabel: auid=%u uid=%u tty=%s pid=%d",
> >>+ audit_loginuid,
> >>+ current->uid,
> >>+ audit_tty,
> >>+ current->pid);
> >
> > Why are you logging all this? When we add audit rules, all that we log is
> > the auid, and subj. If we need to log all this, we should probably have a
> > helper function that gets called by other config change loggers.
>
> If I drop the uid, tty, and pid fields will this be acceptable?
and comm & exe, yes. Anything you were basing off of current has to go. The
audit rule logging was reduced to the credentials that are carried along in
the netlink packet since that's all you can trust. The sending process could
be gone by the time you get to this point in the code.
> >>+ audit_log_format(audit_buf, " comm=");
> >>+ audit_log_untrustedstring(audit_buf, audit_comm);
> >>+ if (current->mm) {
> >>+ down_read(¤t->mm->mmap_sem);
> >>+ vma = current->mm->mmap;
> >>+ while (vma) {
> >>+ if ((vma->vm_flags & VM_EXECUTABLE) &&
> >>+ vma->vm_file) {
> >>+ audit_log_d_path(audit_buf,
> >>+ " exe=",
> >>+ vma->vm_file->f_dentry,
> >>+ vma->vm_file->f_vfsmnt);
> >>+ break;
> >>+ }
> >>+ vma = vma->vm_next;
> >>+ }
> >>+ up_read(¤t->mm->mmap_sem);
> >>+ }
> >>+
> >
> > If this function was moved inside auditsc.c you could use a function
> > there that does this. But the question remains why all this data?
>
> In the ideal world would you prefer this to be removed?
Yes.
-Steve
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 1/1] NetLabel: add audit support for configuration changes
@ 2006-09-29 18:35 ` Steve Grubb
0 siblings, 0 replies; 24+ messages in thread
From: Steve Grubb @ 2006-09-29 18:35 UTC (permalink / raw)
To: Paul Moore; +Cc: linux-audit, netdev, selinux
On Friday 29 September 2006 14:09, Paul Moore wrote:
> > type field is already taken for another purpose, it needs to be renamed.
>
> If we can't have duplicate field names I would propose prefixing both
> these fields (and doing similar things with the other NetLabel specific
> fields) with a "cipso_" making them "cipso_doi" and "cipso_type".
That would be fine. This limits future field name collisions.
> >>+/**
> >>+ * netlbl_unlabel_acceptflg_set - Set the unlabeled accept flag
> >>+ * @value: desired value
> >>+ * @audit_secid: the LSM secid to use in the audit message
> >>+ *
> >>+ * Description:
> >>+ * Set the value of the unlabeled accept flag to @value.
> >>+ *
> >>+ */
> >>+static void netlbl_unlabel_acceptflg_set(u8 value, u32 audit_secid)
> >>+{
> >>+ atomic_set(&netlabel_unlabel_accept_flg, value);
> >>+ netlbl_audit_nomsg((value ?
> >>+ AUDIT_MAC_UNLBL_ACCEPT : AUDIT_MAC_UNLBL_DENY),
> >>+ audit_secid);
> >
> > Looking at how this is being used, I think only 1 message type should be
> > used. There are places in the audit system where we set a flag to 1 or 0,
> > but only have 1 message type. We record the old and new value. So, you'd
> > need to pass that to the logger.
>
> With that in mind I would probably change the message type to
> AUDIT_MAC_UNLBL_ALLOW and use a "unlbl_accept" field; is that okay?
That would be fine. Just a quick note...we have generally been "old " to
indicate the previous value. Example, "backlog=512 old=256".
> >>+/**
> >>+ * netlbl_audit_start_common - Start an audit message
> >>+ * @type: audit message type
> >>+ * @secid: LSM context ID
> >>+ *
> >>+ * Description:
> >>+ * Start an audit message using the type specified in @type and fill the
> >>audit + * message with some fields common to all NetLabel audit messages.
> >>Returns + * a pointer to the audit buffer on success, NULL on failure.
> >>+ *
> >>+ */
> >>+struct audit_buffer *netlbl_audit_start_common(int type, u32 secid)
> >>+{
> >
> > Generally, logging functions are moved into auditsc.c where the context
> > and other functions are defined.
>
> How about leaving this for a future revision?
Come to think of it, you don't need to move it. The reason to move it is to
access the context and use helper functions related to it. But I found that
you were using "current" which may not always be the sender. So if you cannot
use current, most of the stuff you are logging can't be, so the event being
logged becomes simpler and you don't need to move it.
I have not traced through all the code, but if you do any security checks
before taking the rules, be careful not to use current.
> >>+ audit_log_format(audit_buf,
> >>+ "netlabel: auid=%u uid=%u tty=%s pid=%d",
> >>+ audit_loginuid,
> >>+ current->uid,
> >>+ audit_tty,
> >>+ current->pid);
> >
> > Why are you logging all this? When we add audit rules, all that we log is
> > the auid, and subj. If we need to log all this, we should probably have a
> > helper function that gets called by other config change loggers.
>
> If I drop the uid, tty, and pid fields will this be acceptable?
and comm & exe, yes. Anything you were basing off of current has to go. The
audit rule logging was reduced to the credentials that are carried along in
the netlink packet since that's all you can trust. The sending process could
be gone by the time you get to this point in the code.
> >>+ audit_log_format(audit_buf, " comm=");
> >>+ audit_log_untrustedstring(audit_buf, audit_comm);
> >>+ if (current->mm) {
> >>+ down_read(¤t->mm->mmap_sem);
> >>+ vma = current->mm->mmap;
> >>+ while (vma) {
> >>+ if ((vma->vm_flags & VM_EXECUTABLE) &&
> >>+ vma->vm_file) {
> >>+ audit_log_d_path(audit_buf,
> >>+ " exe=",
> >>+ vma->vm_file->f_dentry,
> >>+ vma->vm_file->f_vfsmnt);
> >>+ break;
> >>+ }
> >>+ vma = vma->vm_next;
> >>+ }
> >>+ up_read(¤t->mm->mmap_sem);
> >>+ }
> >>+
> >
> > If this function was moved inside auditsc.c you could use a function
> > there that does this. But the question remains why all this data?
>
> In the ideal world would you prefer this to be removed?
Yes.
-Steve
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 1/1] NetLabel: add audit support for configuration changes
2006-09-29 18:35 ` Steve Grubb
@ 2006-09-29 20:28 ` Paul Moore
-1 siblings, 0 replies; 24+ messages in thread
From: Paul Moore @ 2006-09-29 20:28 UTC (permalink / raw)
To: David Miller; +Cc: netdev, James Morris, selinux, linux-audit
Dave,
I think Steve and I have agreed on a solution, I'll put together a patch
right now based on what is currently in net-2.6 (i.e. the existing
NetLabel audit patch) and submit it to the lists in a few hours.
Steve Grubb wrote:
> On Friday 29 September 2006 14:09, Paul Moore wrote:
>
>>>type field is already taken for another purpose, it needs to be renamed.
>>
>>If we can't have duplicate field names I would propose prefixing both
>>these fields (and doing similar things with the other NetLabel specific
>>fields) with a "cipso_" making them "cipso_doi" and "cipso_type".
>
>
> That would be fine. This limits future field name collisions.
>
>
>>>>+/**
>>>>+ * netlbl_unlabel_acceptflg_set - Set the unlabeled accept flag
>>>>+ * @value: desired value
>>>>+ * @audit_secid: the LSM secid to use in the audit message
>>>>+ *
>>>>+ * Description:
>>>>+ * Set the value of the unlabeled accept flag to @value.
>>>>+ *
>>>>+ */
>>>>+static void netlbl_unlabel_acceptflg_set(u8 value, u32 audit_secid)
>>>>+{
>>>>+ atomic_set(&netlabel_unlabel_accept_flg, value);
>>>>+ netlbl_audit_nomsg((value ?
>>>>+ AUDIT_MAC_UNLBL_ACCEPT : AUDIT_MAC_UNLBL_DENY),
>>>>+ audit_secid);
>>>
>>>Looking at how this is being used, I think only 1 message type should be
>>>used. There are places in the audit system where we set a flag to 1 or 0,
>>>but only have 1 message type. We record the old and new value. So, you'd
>>>need to pass that to the logger.
>>
>>With that in mind I would probably change the message type to
>>AUDIT_MAC_UNLBL_ALLOW and use a "unlbl_accept" field; is that okay?
>
>
> That would be fine. Just a quick note...we have generally been "old " to
> indicate the previous value. Example, "backlog=512 old=256".
>
>
>>>>+/**
>>>>+ * netlbl_audit_start_common - Start an audit message
>>>>+ * @type: audit message type
>>>>+ * @secid: LSM context ID
>>>>+ *
>>>>+ * Description:
>>>>+ * Start an audit message using the type specified in @type and fill the
>>>>audit + * message with some fields common to all NetLabel audit messages.
>>>>Returns + * a pointer to the audit buffer on success, NULL on failure.
>>>>+ *
>>>>+ */
>>>>+struct audit_buffer *netlbl_audit_start_common(int type, u32 secid)
>>>>+{
>>>
>>>Generally, logging functions are moved into auditsc.c where the context
>>>and other functions are defined.
>>
>>How about leaving this for a future revision?
>
>
> Come to think of it, you don't need to move it. The reason to move it is to
> access the context and use helper functions related to it. But I found that
> you were using "current" which may not always be the sender. So if you cannot
> use current, most of the stuff you are logging can't be, so the event being
> logged becomes simpler and you don't need to move it.
>
> I have not traced through all the code, but if you do any security checks
> before taking the rules, be careful not to use current.
>
>
>>>>+ audit_log_format(audit_buf,
>>>>+ "netlabel: auid=%u uid=%u tty=%s pid=%d",
>>>>+ audit_loginuid,
>>>>+ current->uid,
>>>>+ audit_tty,
>>>>+ current->pid);
>>>
>>>Why are you logging all this? When we add audit rules, all that we log is
>>>the auid, and subj. If we need to log all this, we should probably have a
>>>helper function that gets called by other config change loggers.
>>
>>If I drop the uid, tty, and pid fields will this be acceptable?
>
>
> and comm & exe, yes. Anything you were basing off of current has to go. The
> audit rule logging was reduced to the credentials that are carried along in
> the netlink packet since that's all you can trust. The sending process could
> be gone by the time you get to this point in the code.
>
>
>>>>+ audit_log_format(audit_buf, " comm=");
>>>>+ audit_log_untrustedstring(audit_buf, audit_comm);
>>>>+ if (current->mm) {
>>>>+ down_read(¤t->mm->mmap_sem);
>>>>+ vma = current->mm->mmap;
>>>>+ while (vma) {
>>>>+ if ((vma->vm_flags & VM_EXECUTABLE) &&
>>>>+ vma->vm_file) {
>>>>+ audit_log_d_path(audit_buf,
>>>>+ " exe=",
>>>>+ vma->vm_file->f_dentry,
>>>>+ vma->vm_file->f_vfsmnt);
>>>>+ break;
>>>>+ }
>>>>+ vma = vma->vm_next;
>>>>+ }
>>>>+ up_read(¤t->mm->mmap_sem);
>>>>+ }
>>>>+
>>>
>>>If this function was moved inside auditsc.c you could use a function
>>>there that does this. But the question remains why all this data?
>>
>>In the ideal world would you prefer this to be removed?
>
>
> Yes.
>
> -Steve
--
paul moore
linux security @ hp
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 1/1] NetLabel: add audit support for configuration changes
@ 2006-09-29 20:28 ` Paul Moore
0 siblings, 0 replies; 24+ messages in thread
From: Paul Moore @ 2006-09-29 20:28 UTC (permalink / raw)
To: David Miller; +Cc: Steve Grubb, linux-audit, netdev, selinux, James Morris
Dave,
I think Steve and I have agreed on a solution, I'll put together a patch
right now based on what is currently in net-2.6 (i.e. the existing
NetLabel audit patch) and submit it to the lists in a few hours.
Steve Grubb wrote:
> On Friday 29 September 2006 14:09, Paul Moore wrote:
>
>>>type field is already taken for another purpose, it needs to be renamed.
>>
>>If we can't have duplicate field names I would propose prefixing both
>>these fields (and doing similar things with the other NetLabel specific
>>fields) with a "cipso_" making them "cipso_doi" and "cipso_type".
>
>
> That would be fine. This limits future field name collisions.
>
>
>>>>+/**
>>>>+ * netlbl_unlabel_acceptflg_set - Set the unlabeled accept flag
>>>>+ * @value: desired value
>>>>+ * @audit_secid: the LSM secid to use in the audit message
>>>>+ *
>>>>+ * Description:
>>>>+ * Set the value of the unlabeled accept flag to @value.
>>>>+ *
>>>>+ */
>>>>+static void netlbl_unlabel_acceptflg_set(u8 value, u32 audit_secid)
>>>>+{
>>>>+ atomic_set(&netlabel_unlabel_accept_flg, value);
>>>>+ netlbl_audit_nomsg((value ?
>>>>+ AUDIT_MAC_UNLBL_ACCEPT : AUDIT_MAC_UNLBL_DENY),
>>>>+ audit_secid);
>>>
>>>Looking at how this is being used, I think only 1 message type should be
>>>used. There are places in the audit system where we set a flag to 1 or 0,
>>>but only have 1 message type. We record the old and new value. So, you'd
>>>need to pass that to the logger.
>>
>>With that in mind I would probably change the message type to
>>AUDIT_MAC_UNLBL_ALLOW and use a "unlbl_accept" field; is that okay?
>
>
> That would be fine. Just a quick note...we have generally been "old " to
> indicate the previous value. Example, "backlog=512 old=256".
>
>
>>>>+/**
>>>>+ * netlbl_audit_start_common - Start an audit message
>>>>+ * @type: audit message type
>>>>+ * @secid: LSM context ID
>>>>+ *
>>>>+ * Description:
>>>>+ * Start an audit message using the type specified in @type and fill the
>>>>audit + * message with some fields common to all NetLabel audit messages.
>>>>Returns + * a pointer to the audit buffer on success, NULL on failure.
>>>>+ *
>>>>+ */
>>>>+struct audit_buffer *netlbl_audit_start_common(int type, u32 secid)
>>>>+{
>>>
>>>Generally, logging functions are moved into auditsc.c where the context
>>>and other functions are defined.
>>
>>How about leaving this for a future revision?
>
>
> Come to think of it, you don't need to move it. The reason to move it is to
> access the context and use helper functions related to it. But I found that
> you were using "current" which may not always be the sender. So if you cannot
> use current, most of the stuff you are logging can't be, so the event being
> logged becomes simpler and you don't need to move it.
>
> I have not traced through all the code, but if you do any security checks
> before taking the rules, be careful not to use current.
>
>
>>>>+ audit_log_format(audit_buf,
>>>>+ "netlabel: auid=%u uid=%u tty=%s pid=%d",
>>>>+ audit_loginuid,
>>>>+ current->uid,
>>>>+ audit_tty,
>>>>+ current->pid);
>>>
>>>Why are you logging all this? When we add audit rules, all that we log is
>>>the auid, and subj. If we need to log all this, we should probably have a
>>>helper function that gets called by other config change loggers.
>>
>>If I drop the uid, tty, and pid fields will this be acceptable?
>
>
> and comm & exe, yes. Anything you were basing off of current has to go. The
> audit rule logging was reduced to the credentials that are carried along in
> the netlink packet since that's all you can trust. The sending process could
> be gone by the time you get to this point in the code.
>
>
>>>>+ audit_log_format(audit_buf, " comm=");
>>>>+ audit_log_untrustedstring(audit_buf, audit_comm);
>>>>+ if (current->mm) {
>>>>+ down_read(¤t->mm->mmap_sem);
>>>>+ vma = current->mm->mmap;
>>>>+ while (vma) {
>>>>+ if ((vma->vm_flags & VM_EXECUTABLE) &&
>>>>+ vma->vm_file) {
>>>>+ audit_log_d_path(audit_buf,
>>>>+ " exe=",
>>>>+ vma->vm_file->f_dentry,
>>>>+ vma->vm_file->f_vfsmnt);
>>>>+ break;
>>>>+ }
>>>>+ vma = vma->vm_next;
>>>>+ }
>>>>+ up_read(¤t->mm->mmap_sem);
>>>>+ }
>>>>+
>>>
>>>If this function was moved inside auditsc.c you could use a function
>>>there that does this. But the question remains why all this data?
>>
>>In the ideal world would you prefer this to be removed?
>
>
> Yes.
>
> -Steve
--
paul moore
linux security @ hp
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 1/1] NetLabel: add audit support for configuration changes
2006-09-29 20:28 ` Paul Moore
(?)
@ 2006-09-29 21:33 ` David Miller
-1 siblings, 0 replies; 24+ messages in thread
From: David Miller @ 2006-09-29 21:33 UTC (permalink / raw)
To: paul.moore; +Cc: netdev, jmorris, selinux, linux-audit
From: Paul Moore <paul.moore@hp.com>
Date: Fri, 29 Sep 2006 16:28:36 -0400
> I think Steve and I have agreed on a solution, I'll put together a patch
> right now based on what is currently in net-2.6 (i.e. the existing
> NetLabel audit patch) and submit it to the lists in a few hours.
Great, I'll just leave the tree alone until you send me that.
Thanks.
^ permalink raw reply [flat|nested] 24+ messages in thread