[RFC][PATCH] Smack<->Audit integration - Kernel

This is a discussion on [RFC][PATCH] Smack<->Audit integration - Kernel ; Hi all, This is the second step of integrating Audit with Smack. The AUDIT_SUBJ_USER and AUDIT_OBJ_USER SELinux flags are recycled for Smack to avoid `auditd' userspace modifications. Smack only needs auditing on subject/object bases, so those flags were enough. Patch ...

+ Reply to Thread
Results 1 to 14 of 14

Thread: [RFC][PATCH] Smack<->Audit integration

  1. [RFC][PATCH] Smack<->Audit integration

    Hi all,

    This is the second step of integrating Audit with Smack.

    The AUDIT_SUBJ_USER and AUDIT_OBJ_USER SELinux flags are recycled
    for Smack to avoid `auditd' userspace modifications. Smack only
    needs auditing on subject/object bases, so those flags were enough.

    Patch below setups the new Audit LSM hooks and add a secid field in
    smack inode and ipc security structures. The secid field was needed
    cause it's the main Audit way of distinguishing kernel objects to
    be audited from the remaining ones.

    Possibly the last steps would be to:
    1- report smack access grants and denials through Audit (like AVC)
    2- Letting Audit send an OBJ_USER event in case of a process that
    recieved a signal without affecting SELinux.

    Thank you for your reviews.

    Signed-off-by: Ahmed S. Darwish
    ---

    smack.h | 9 ++
    smack_lsm.c | 211 ++++++++++++++++++++++++++++++++++++++++++++++++++ +++-----

    2 files changed, 200 insertions(+), 17 deletions(-)

    Patch is generated over James security/next branch cause it's the
    only tree that currently has the Audit hooks merged:
    git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6.git#next

    diff --git a/security/smack/smack.h b/security/smack/smack.h
    index c444f48..2c8bb4c 100644
    --- a/security/smack/smack.h
    +++ b/security/smack/smack.h
    @@ -57,6 +57,15 @@ struct inode_smack {
    char *smk_inode; /* label of the fso */
    struct mutex smk_lock; /* initialization lock */
    int smk_flags; /* smack inode flags */
    + int secid; /* security identifier */
    +};
    +
    +/*
    + * IPC smack data
    + */
    +struct ipc_smack {
    + char *smk_ipc; /* ipc object label */
    + int secid; /* security identifier */
    };

    #define SMK_INODE_INSTANT 0x01 /* inode is instantiated */
    diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
    index afa7967..5b5e1fd 100644
    --- a/security/smack/smack_lsm.c
    +++ b/security/smack/smack_lsm.c
    @@ -6,6 +6,9 @@
    * Author:
    * Casey Schaufler
    *
    + * Audit integration by:
    + * Ahmed S. Darwish
    + *
    * Copyright (C) 2007 Casey Schaufler
    *
    * This program is free software; you can redistribute it and/or modify
    @@ -24,6 +27,7 @@
    #include
    #include
    #include
    +#include
    #include
    #include

    @@ -75,6 +79,7 @@ struct inode_smack *new_inode_smack(char *smack)

    isp->smk_inode = smack;
    isp->smk_flags = 0;
    + isp->secid = smack_to_secid(smack);
    mutex_init(&isp->smk_lock);

    return isp;
    @@ -638,6 +643,8 @@ static void smack_inode_post_setxattr(struct dentry *dentry, char *name,
    else
    isp->smk_inode = smack_known_invalid.smk_known;

    + isp->secid = smack_to_secid(isp->smk_inode);
    +
    return;
    }

    @@ -759,6 +766,17 @@ static int smack_inode_listsecurity(struct inode *inode, char *buffer,
    return -EINVAL;
    }

    +/**
    + * smack_inode_getsecid - Extract inode's security id
    + * @inode: inode to extract the info from
    + * @secid: where the result will be saved
    + */
    +static void smack_inode_getsecid(const struct inode *inode, u32 *secid)
    +{
    + struct inode_smack *isp = inode->i_security;
    + *secid = isp->secid;
    +}
    +
    /*
    * File Hooks
    */
    @@ -1344,6 +1362,7 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name,
    const void *value, size_t size, int flags)
    {
    char *sp;
    + struct smack_known *skp;
    struct inode_smack *nsp = inode->i_security;
    struct socket_smack *ssp;
    struct socket *sock;
    @@ -1352,12 +1371,14 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name,
    if (value == NULL || size > SMK_LABELLEN)
    return -EACCES;

    - sp = smk_import(value, size);
    - if (sp == NULL)
    + skp = smk_import_entry(value, size);
    + if (skp == NULL)
    return -EINVAL;

    + sp = skp->smk_known;
    if (strcmp(name, XATTR_SMACK_SUFFIX) == 0) {
    nsp->smk_inode = sp;
    + nsp->secid = skp->smk_secid;
    return 0;
    }
    /*
    @@ -1471,9 +1492,16 @@ static char *smack_of_shm(struct shmid_kernel *shp)
    */
    static int smack_shm_alloc_security(struct shmid_kernel *shp)
    {
    - struct kern_ipc_perm *isp = &shp->shm_perm;
    + struct ipc_smack *isp;
    + struct kern_ipc_perm *ipcp = &shp->shm_perm;

    - isp->security = current->security;
    + isp = kzalloc(sizeof(struct ipc_smack), GFP_KERNEL);
    + if (isp == NULL)
    + return -ENOMEM;
    +
    + isp->smk_ipc = current->security;
    + isp->secid = smack_to_secid(isp->smk_ipc);
    + ipcp->security = isp;
    return 0;
    }

    @@ -1485,9 +1513,9 @@ static int smack_shm_alloc_security(struct shmid_kernel *shp)
    */
    static void smack_shm_free_security(struct shmid_kernel *shp)
    {
    - struct kern_ipc_perm *isp = &shp->shm_perm;
    + struct kern_ipc_perm *ipcp = &shp->shm_perm;

    - isp->security = NULL;
    + kfree(ipcp->security);
    }

    /**
    @@ -1579,9 +1607,16 @@ static char *smack_of_sem(struct sem_array *sma)
    */
    static int smack_sem_alloc_security(struct sem_array *sma)
    {
    - struct kern_ipc_perm *isp = &sma->sem_perm;
    + struct ipc_smack *isp;
    + struct kern_ipc_perm *ipcp = &sma->sem_perm;
    +
    + isp = kzalloc(sizeof(struct ipc_smack), GFP_KERNEL);
    + if (isp == NULL)
    + return -ENOMEM;

    - isp->security = current->security;
    + isp->smk_ipc = current->security;
    + isp->secid = smack_to_secid(isp->smk_ipc);
    + ipcp->security = isp;
    return 0;
    }

    @@ -1595,7 +1630,7 @@ static void smack_sem_free_security(struct sem_array *sma)
    {
    struct kern_ipc_perm *isp = &sma->sem_perm;

    - isp->security = NULL;
    + kfree(isp->security);
    }

    /**
    @@ -1682,9 +1717,16 @@ static int smack_sem_semop(struct sem_array *sma, struct sembuf *sops,
    */
    static int smack_msg_queue_alloc_security(struct msg_queue *msq)
    {
    - struct kern_ipc_perm *kisp = &msq->q_perm;
    + struct ipc_smack *isp;
    + struct kern_ipc_perm *ipcp = &msq->q_perm;

    - kisp->security = current->security;
    + isp = kzalloc(sizeof(struct ipc_smack), GFP_KERNEL);
    + if (isp == NULL)
    + return -ENOMEM;
    +
    + isp->smk_ipc = current->security;
    + isp->secid = smack_to_secid(isp->smk_ipc);
    + ipcp->security = isp;
    return 0;
    }

    @@ -1696,9 +1738,9 @@ static int smack_msg_queue_alloc_security(struct msg_queue *msq)
    */
    static void smack_msg_queue_free_security(struct msg_queue *msq)
    {
    - struct kern_ipc_perm *kisp = &msq->q_perm;
    + struct kern_ipc_perm *ipcp = &msq->q_perm;

    - kisp->security = NULL;
    + kfree(ipcp->security);
    }

    /**
    @@ -1800,18 +1842,30 @@ static int smack_msg_queue_msgrcv(struct msg_queue *msq, struct msg_msg *msg,

    /**
    * smack_ipc_permission - Smack access for ipc_permission()
    - * @ipp: the object permissions
    + * @ipcp: the object permissions
    * @flag: access requested
    *
    * Returns 0 if current has read and write access, error code otherwise
    */
    -static int smack_ipc_permission(struct kern_ipc_perm *ipp, short flag)
    +static int smack_ipc_permission(struct kern_ipc_perm *ipcp, short flag)
    {
    - char *isp = ipp->security;
    int may;
    + struct ipc_smack *isp = ipcp->security;

    may = smack_flags_to_may(flag);
    - return smk_curacc(isp, may);
    + return smk_curacc(isp->smk_ipc, may);
    +}
    +
    +/**
    + * smack_ipc_getsecid - extract Smack security id
    + * @ipcp: the object permissions
    + * @secid: where result will be saved
    + */
    +static void smack_ipc_getsecid(struct kern_ipc_perm *ipcp, u32 *secid)
    +{
    + struct inode_smack *isp = ipcp->security;
    +
    + *secid = isp->secid;
    }

    /* module stacking operations */
    @@ -1967,6 +2021,7 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode)
    else
    isp->smk_inode = final;

    + isp->secid = smack_to_secid(isp->smk_inode);
    isp->smk_flags |= SMK_INODE_INSTANT;

    unlockandout:
    @@ -2391,6 +2446,116 @@ static int smack_key_permission(key_ref_t key_ref,
    #endif /* CONFIG_KEYS */

    /*
    + * Smack Audit hooks
    + *
    + * Audit requires an internal representation of each Smack specific
    + * rule which works as a glue between the audit hooks. Since smack_known
    + * repository entries are added but never deleted, we'll use the
    + * smack_known entry related to the given audit rule as the needed
    + * smack representation.
    + *
    + * This will also save us from searching the smack labels repository
    + * each time the audit_rule_match hook get called.
    + */
    +#ifdef CONFIG_AUDIT
    +
    +/**
    + * smack_audit_rule_init - Initialize a smack audit rule
    + * @field: Message flags given from user-space (audit.h)
    + * @op: Required operation (Only equality testing is allowed)
    + * @rulestr: Given user-space rule
    + * @vrule: Where we'll save our own audit rule representation
    + *
    + * The label to be audited (rulestr) is created if necessay
    + */
    +static int smack_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule)
    +{
    + struct smack_known **smk_rule = (struct smack_known **)vrule;
    + *smk_rule = NULL;
    +
    + if (field != AUDIT_SUBJ_USER && field != AUDIT_OBJ_USER)
    + return -EINVAL;
    +
    + if (op != AUDIT_EQUAL && op != AUDIT_NOT_EQUAL)
    + return -EINVAL;
    +
    + *smk_rule = smk_import_entry(rulestr, 0);
    +
    + return 0;
    +}
    +
    +/**
    + * smack_audit_rule_known - Distinguish smack audit rules from others
    + * @krule: rule of interest, in internal kernel representation format
    + *
    + * This is used to filter rules related to Smack from other saved
    + * Audit rules. If it's proved that this rule belongs to us, the
    + * audit_rule_match hook will be used to check a specific kernel
    + * object against its smack audit rule.
    + */
    +static int smack_audit_rule_known(struct audit_krule *krule)
    +{
    + struct audit_field *f;
    + int i;
    +
    + for (i = 0; i < krule->field_count; i++) {
    + f = &krule->fields[i];
    +
    + if (f->type == AUDIT_SUBJ_USER || f->type == AUDIT_OBJ_USER)
    + return 1;
    + }
    +
    + return 0;
    +}
    +
    +/**
    + * smack_audit_rule_match - Audit given secid identified object ?
    + * @secid: Security id to test
    + * @field: Message flags given from user-space
    + * @op: Required operation (only equality is allowed)
    + * @vrule: Smack audit rule that will be checked against the secid object
    + * @actx: audit context associated with the check (used for Audit logging)
    + *
    + * This is the core Audit hook. It's used to identify objects like
    + * syscalls and inodes requested from user-space to be audited from
    + * remaining kernel objects.
    + */
    +static int smack_audit_rule_match(u32 secid, u32 field, u32 op, void *vrule,
    + struct audit_context *actx)
    +{
    + struct smack_known *smk_rule = vrule;
    +
    + if (!smk_rule) {
    + audit_log(actx, GFP_KERNEL, AUDIT_SELINUX_ERR,
    + "Smack: missing rule\n");
    + return -ENOENT;
    + }
    +
    + if (field != AUDIT_SUBJ_USER && field != AUDIT_OBJ_USER)
    + return 0;
    +
    + if (op == AUDIT_EQUAL)
    + return (smk_rule->smk_secid == secid);
    + if (op == AUDIT_NOT_EQUAL)
    + return (smk_rule->smk_secid != secid);
    +
    + return 0;
    +}
    +
    +/**
    + * smack_audit_rule_free - free internal audit rule representation
    + * @vrule: rule to be freed.
    + *
    + * No memory was allocated in audit_rule_init.
    + */
    +static void smack_audit_rule_free(void *vrule)
    +{
    + /* No-op */
    +}
    +
    +#endif /* CONFIG_AUDIT */
    +
    +/*
    * smack_secid_to_secctx - return the smack label for a secid
    * @secid: incoming integer
    * @secdata: destination
    @@ -2476,6 +2641,7 @@ struct security_operations smack_ops = {
    .inode_getsecurity = smack_inode_getsecurity,
    .inode_setsecurity = smack_inode_setsecurity,
    .inode_listsecurity = smack_inode_listsecurity,
    + .inode_getsecid = smack_inode_getsecid,

    .file_permission = smack_file_permission,
    .file_alloc_security = smack_file_alloc_security,
    @@ -2506,6 +2672,7 @@ struct security_operations smack_ops = {
    .task_to_inode = smack_task_to_inode,

    .ipc_permission = smack_ipc_permission,
    + .ipc_getsecid = smack_ipc_getsecid,

    .msg_msg_alloc_security = smack_msg_msg_alloc_security,
    .msg_msg_free_security = smack_msg_msg_free_security,
    @@ -2550,12 +2717,22 @@ struct security_operations smack_ops = {
    .sk_free_security = smack_sk_free_security,
    .sock_graft = smack_sock_graft,
    .inet_conn_request = smack_inet_conn_request,
    +
    /* key management security hooks */
    #ifdef CONFIG_KEYS
    .key_alloc = smack_key_alloc,
    .key_free = smack_key_free,
    .key_permission = smack_key_permission,
    #endif /* CONFIG_KEYS */
    +
    + /* Audit hooks */
    +#ifdef CONFIG_AUDIT
    + .audit_rule_init = smack_audit_rule_init,
    + .audit_rule_known = smack_audit_rule_known,
    + .audit_rule_match = smack_audit_rule_match,
    + .audit_rule_free = smack_audit_rule_free,
    +#endif /* CONFIG_AUDIT */
    +
    .secid_to_secctx = smack_secid_to_secctx,
    .secctx_to_secid = smack_secctx_to_secid,
    .release_secctx = smack_release_secctx,

    Regards,

    --

    "Better to light a candle, than curse the darkness"

    Ahmed S. Darwish
    Homepage: http://darwish.07.googlepages.com
    Blog: http://darwish-07.blogspot.com

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [RFC][PATCH] Smack<->Audit integration


    --- "Ahmed S. Darwish" wrote:

    > Hi all,
    >
    > This is the second step of integrating Audit with Smack.
    >
    > The AUDIT_SUBJ_USER and AUDIT_OBJ_USER SELinux flags are recycled
    > for Smack to avoid `auditd' userspace modifications. Smack only
    > needs auditing on subject/object bases, so those flags were enough.
    >
    > Patch below setups the new Audit LSM hooks and add a secid field in
    > smack inode and ipc security structures. The secid field was needed
    > cause it's the main Audit way of distinguishing kernel objects to
    > be audited from the remaining ones.
    >
    > Possibly the last steps would be to:
    > 1- report smack access grants and denials through Audit (like AVC)
    > 2- Letting Audit send an OBJ_USER event in case of a process that
    > recieved a signal without affecting SELinux.
    >
    > Thank you for your reviews.
    >
    > Signed-off-by: Ahmed S. Darwish
    > ---
    >
    > smack.h | 9 ++
    > smack_lsm.c | 211
    > ++++++++++++++++++++++++++++++++++++++++++++++++++ +++-----
    >
    > 2 files changed, 200 insertions(+), 17 deletions(-)
    >
    > Patch is generated over James security/next branch cause it's the
    > only tree that currently has the Audit hooks merged:
    >

    git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6.git#next
    >
    > diff --git a/security/smack/smack.h b/security/smack/smack.h
    > index c444f48..2c8bb4c 100644
    > --- a/security/smack/smack.h
    > +++ b/security/smack/smack.h
    > @@ -57,6 +57,15 @@ struct inode_smack {
    > char *smk_inode; /* label of the fso */
    > struct mutex smk_lock; /* initialization lock */
    > int smk_flags; /* smack inode flags */
    > + int secid; /* security identifier */


    No.

    Secid's are horrid things and every effort should be made to
    expunge them from the known universe. Under no circumstances
    should thier use be expanded. The only reason Smack has them
    at all is because certain interfaces that in my mind should
    have known better use them. If you must deal with secids,
    and for this round of audit I think that's a given, use
    smack_to_secid(sp->smk_inode) where you need to. If there's a
    real performance issue apply intelligence to smack_to_secid
    instead of storing the secid. There ought to be a way to
    use container_of to do smack_to_secid, but I had trouble with
    that and moved along without figuring out what I had done
    wrong.

    > +};
    > +
    > +/*
    > + * IPC smack data
    > + */
    > +struct ipc_smack {
    > + char *smk_ipc; /* ipc object label */
    > + int secid; /* security identifier */


    As above.

    > };
    >
    > #define SMK_INODE_INSTANT 0x01 /* inode is instantiated */
    > diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
    > index afa7967..5b5e1fd 100644
    > --- a/security/smack/smack_lsm.c
    > +++ b/security/smack/smack_lsm.c
    > @@ -6,6 +6,9 @@
    > * Author:
    > * Casey Schaufler
    > *
    > + * Audit integration by:
    > + * Ahmed S. Darwish
    > + *
    > * Copyright (C) 2007 Casey Schaufler
    > *
    > * This program is free software; you can redistribute it and/or modify
    > @@ -24,6 +27,7 @@
    > #include
    > #include
    > #include
    > +#include
    > #include
    > #include
    >
    > @@ -75,6 +79,7 @@ struct inode_smack *new_inode_smack(char *smack)
    >
    > isp->smk_inode = smack;
    > isp->smk_flags = 0;
    > + isp->secid = smack_to_secid(smack);


    As above

    > mutex_init(&isp->smk_lock);
    >
    > return isp;
    > @@ -638,6 +643,8 @@ static void smack_inode_post_setxattr(struct dentry
    > *dentry, char *name,
    > else
    > isp->smk_inode = smack_known_invalid.smk_known;
    >
    > + isp->secid = smack_to_secid(isp->smk_inode);


    As above

    > +
    > return;
    > }
    >
    > @@ -759,6 +766,17 @@ static int smack_inode_listsecurity(struct inode *inode,
    > char *buffer,
    > return -EINVAL;
    > }
    >
    > +/**
    > + * smack_inode_getsecid - Extract inode's security id
    > + * @inode: inode to extract the info from
    > + * @secid: where the result will be saved
    > + */
    > +static void smack_inode_getsecid(const struct inode *inode, u32 *secid)
    > +{
    > + struct inode_smack *isp = inode->i_security;
    > + *secid = isp->secid;


    *secid = smack_to_secid(isp->smk_inode);

    > +}
    > +
    > /*
    > * File Hooks
    > */
    > @@ -1344,6 +1362,7 @@ static int smack_inode_setsecurity(struct inode *inode,
    > const char *name,
    > const void *value, size_t size, int flags)
    > {
    > char *sp;
    > + struct smack_known *skp;
    > struct inode_smack *nsp = inode->i_security;
    > struct socket_smack *ssp;
    > struct socket *sock;
    > @@ -1352,12 +1371,14 @@ static int smack_inode_setsecurity(struct inode
    > *inode, const char *name,
    > if (value == NULL || size > SMK_LABELLEN)
    > return -EACCES;
    >
    > - sp = smk_import(value, size);
    > - if (sp == NULL)
    > + skp = smk_import_entry(value, size);
    > + if (skp == NULL)
    > return -EINVAL;
    >
    > + sp = skp->smk_known;
    > if (strcmp(name, XATTR_SMACK_SUFFIX) == 0) {
    > nsp->smk_inode = sp;
    > + nsp->secid = skp->smk_secid;
    > return 0;
    > }


    No need to change this hook with no smk_secid.

    > /*
    > @@ -1471,9 +1492,16 @@ static char *smack_of_shm(struct shmid_kernel *shp)
    > */
    > static int smack_shm_alloc_security(struct shmid_kernel *shp)
    > {
    > - struct kern_ipc_perm *isp = &shp->shm_perm;
    > + struct ipc_smack *isp;
    > + struct kern_ipc_perm *ipcp = &shp->shm_perm;
    >
    > - isp->security = current->security;
    > + isp = kzalloc(sizeof(struct ipc_smack), GFP_KERNEL);
    > + if (isp == NULL)
    > + return -ENOMEM;
    > +
    > + isp->smk_ipc = current->security;
    > + isp->secid = smack_to_secid(isp->smk_ipc);
    > + ipcp->security = isp;
    > return 0;
    > }


    Same here.

    > @@ -1485,9 +1513,9 @@ static int smack_shm_alloc_security(struct shmid_kernel
    > *shp)
    > */
    > static void smack_shm_free_security(struct shmid_kernel *shp)
    > {
    > - struct kern_ipc_perm *isp = &shp->shm_perm;
    > + struct kern_ipc_perm *ipcp = &shp->shm_perm;
    >
    > - isp->security = NULL;
    > + kfree(ipcp->security);
    > }


    Same here, and all the more reason to leave the data as is.

    > /**
    > @@ -1579,9 +1607,16 @@ static char *smack_of_sem(struct sem_array *sma)
    > */
    > static int smack_sem_alloc_security(struct sem_array *sma)
    > {
    > - struct kern_ipc_perm *isp = &sma->sem_perm;
    > + struct ipc_smack *isp;
    > + struct kern_ipc_perm *ipcp = &sma->sem_perm;
    > +
    > + isp = kzalloc(sizeof(struct ipc_smack), GFP_KERNEL);
    > + if (isp == NULL)
    > + return -ENOMEM;
    >
    > - isp->security = current->security;
    > + isp->smk_ipc = current->security;
    > + isp->secid = smack_to_secid(isp->smk_ipc);
    > + ipcp->security = isp;
    > return 0;
    > }


    'nuff said?

    > @@ -1595,7 +1630,7 @@ static void smack_sem_free_security(struct sem_array
    > *sma)
    > {
    > struct kern_ipc_perm *isp = &sma->sem_perm;
    >
    > - isp->security = NULL;
    > + kfree(isp->security);
    > }


    And again.

    > /**
    > @@ -1682,9 +1717,16 @@ static int smack_sem_semop(struct sem_array *sma,
    > struct sembuf *sops,
    > */
    > static int smack_msg_queue_alloc_security(struct msg_queue *msq)
    > {
    > - struct kern_ipc_perm *kisp = &msq->q_perm;
    > + struct ipc_smack *isp;
    > + struct kern_ipc_perm *ipcp = &msq->q_perm;
    >
    > - kisp->security = current->security;
    > + isp = kzalloc(sizeof(struct ipc_smack), GFP_KERNEL);
    > + if (isp == NULL)
    > + return -ENOMEM;
    > +
    > + isp->smk_ipc = current->security;
    > + isp->secid = smack_to_secid(isp->smk_ipc);
    > + ipcp->security = isp;
    > return 0;
    > }


    And again

    > @@ -1696,9 +1738,9 @@ static int smack_msg_queue_alloc_security(struct
    > msg_queue *msq)
    > */
    > static void smack_msg_queue_free_security(struct msg_queue *msq)
    > {
    > - struct kern_ipc_perm *kisp = &msq->q_perm;
    > + struct kern_ipc_perm *ipcp = &msq->q_perm;
    >
    > - kisp->security = NULL;
    > + kfree(ipcp->security);
    > }


    Don't you just hate repetative reviewers?

    > /**
    > @@ -1800,18 +1842,30 @@ static int smack_msg_queue_msgrcv(struct msg_queue
    > *msq, struct msg_msg *msg,
    >
    > /**
    > * smack_ipc_permission - Smack access for ipc_permission()
    > - * @ipp: the object permissions
    > + * @ipcp: the object permissions
    > * @flag: access requested
    > *
    > * Returns 0 if current has read and write access, error code otherwise
    > */
    > -static int smack_ipc_permission(struct kern_ipc_perm *ipp, short flag)
    > +static int smack_ipc_permission(struct kern_ipc_perm *ipcp, short flag)
    > {
    > - char *isp = ipp->security;
    > int may;
    > + struct ipc_smack *isp = ipcp->security;
    >
    > may = smack_flags_to_may(flag);
    > - return smk_curacc(isp, may);
    > + return smk_curacc(isp->smk_ipc, may);
    > +}


    Last time, I think ...

    > +
    > +/**
    > + * smack_ipc_getsecid - extract Smack security id
    > + * @ipcp: the object permissions
    > + * @secid: where result will be saved
    > + */
    > +static void smack_ipc_getsecid(struct kern_ipc_perm *ipcp, u32 *secid)
    > +{
    > + struct inode_smack *isp = ipcp->security;
    > +
    > + *secid = isp->secid;


    *secid = smack_to_secid(ipcp->security);

    > }
    >
    > /* module stacking operations */
    > @@ -1967,6 +2021,7 @@ static void smack_d_instantiate(struct dentry
    > *opt_dentry, struct inode *inode)
    > else
    > isp->smk_inode = final;
    >
    > + isp->secid = smack_to_secid(isp->smk_inode);


    Out she goes.

    > isp->smk_flags |= SMK_INODE_INSTANT;
    >
    > unlockandout:
    > @@ -2391,6 +2446,116 @@ static int smack_key_permission(key_ref_t key_ref,
    > #endif /* CONFIG_KEYS */
    >
    > /*
    > + * Smack Audit hooks
    > + *
    > + * Audit requires an internal representation of each Smack specific
    > + * rule which works as a glue between the audit hooks. Since smack_known
    > + * repository entries are added but never deleted, we'll use the
    > + * smack_known entry related to the given audit rule as the needed
    > + * smack representation.
    > + *
    > + * This will also save us from searching the smack labels repository
    > + * each time the audit_rule_match hook get called.
    > + */
    > +#ifdef CONFIG_AUDIT
    > +
    > +/**
    > + * smack_audit_rule_init - Initialize a smack audit rule
    > + * @field: Message flags given from user-space (audit.h)
    > + * @op: Required operation (Only equality testing is allowed)
    > + * @rulestr: Given user-space rule
    > + * @vrule: Where we'll save our own audit rule representation
    > + *
    > + * The label to be audited (rulestr) is created if necessay
    > + */
    > +static int smack_audit_rule_init(u32 field, u32 op, char *rulestr, void
    > **vrule)
    > +{
    > + struct smack_known **smk_rule = (struct smack_known **)vrule;
    > + *smk_rule = NULL;
    > +
    > + if (field != AUDIT_SUBJ_USER && field != AUDIT_OBJ_USER)
    > + return -EINVAL;
    > +
    > + if (op != AUDIT_EQUAL && op != AUDIT_NOT_EQUAL)
    > + return -EINVAL;
    > +
    > + *smk_rule = smk_import_entry(rulestr, 0);
    > +
    > + return 0;
    > +}
    > +
    > +/**
    > + * smack_audit_rule_known - Distinguish smack audit rules from others
    > + * @krule: rule of interest, in internal kernel representation format
    > + *
    > + * This is used to filter rules related to Smack from other saved
    > + * Audit rules. If it's proved that this rule belongs to us, the
    > + * audit_rule_match hook will be used to check a specific kernel
    > + * object against its smack audit rule.
    > + */
    > +static int smack_audit_rule_known(struct audit_krule *krule)
    > +{
    > + struct audit_field *f;
    > + int i;
    > +
    > + for (i = 0; i < krule->field_count; i++) {
    > + f = &krule->fields[i];
    > +
    > + if (f->type == AUDIT_SUBJ_USER || f->type == AUDIT_OBJ_USER)
    > + return 1;
    > + }
    > +
    > + return 0;
    > +}
    > +
    > +/**
    > + * smack_audit_rule_match - Audit given secid identified object ?
    > + * @secid: Security id to test
    > + * @field: Message flags given from user-space
    > + * @op: Required operation (only equality is allowed)
    > + * @vrule: Smack audit rule that will be checked against the secid object
    > + * @actx: audit context associated with the check (used for Audit logging)
    > + *
    > + * This is the core Audit hook. It's used to identify objects like
    > + * syscalls and inodes requested from user-space to be audited from
    > + * remaining kernel objects.
    > + */
    > +static int smack_audit_rule_match(u32 secid, u32 field, u32 op, void *vrule,
    > + struct audit_context *actx)
    > +{
    > + struct smack_known *smk_rule = vrule;


    char *smack;

    > +
    > + if (!smk_rule) {
    > + audit_log(actx, GFP_KERNEL, AUDIT_SELINUX_ERR,
    > + "Smack: missing rule\n");
    > + return -ENOENT;
    > + }
    > +
    > + if (field != AUDIT_SUBJ_USER && field != AUDIT_OBJ_USER)
    > + return 0;
    > +


    smack = smack_from_secid(secid);

    > + if (op == AUDIT_EQUAL)
    > + return (smk_rule->smk_secid == secid);
    > + if (op == AUDIT_NOT_EQUAL)
    > + return (smk_rule->smk_secid != secid);


    if (op == AUDIT_EQUAL)
    return (smk_rule->smk_smack == smack);
    if (op == AUDIT_NOT_EQUAL)
    return (smk_rule->smk_smack == smack);

    It would be more Smack consistant to use Smack labels than
    secids here. Secids are tolerable for talking to other
    components, but I do not want them used internally. Since
    the Smack labels are stored in a list the pointer comparison
    will hold (unless it's in the inbound networking).

    > +
    > + return 0;
    > +}
    > +
    > +/**
    > + * smack_audit_rule_free - free internal audit rule representation
    > + * @vrule: rule to be freed.
    > + *
    > + * No memory was allocated in audit_rule_init.
    > + */
    > +static void smack_audit_rule_free(void *vrule)
    > +{
    > + /* No-op */
    > +}
    > +
    > +#endif /* CONFIG_AUDIT */
    > +
    > +/*
    > * smack_secid_to_secctx - return the smack label for a secid
    > * @secid: incoming integer
    > * @secdata: destination
    > @@ -2476,6 +2641,7 @@ struct security_operations smack_ops = {
    > .inode_getsecurity = smack_inode_getsecurity,
    > .inode_setsecurity = smack_inode_setsecurity,
    > .inode_listsecurity = smack_inode_listsecurity,
    > + .inode_getsecid = smack_inode_getsecid,
    >
    > .file_permission = smack_file_permission,
    > .file_alloc_security = smack_file_alloc_security,
    > @@ -2506,6 +2672,7 @@ struct security_operations smack_ops = {
    > .task_to_inode = smack_task_to_inode,
    >
    > .ipc_permission = smack_ipc_permission,
    > + .ipc_getsecid = smack_ipc_getsecid,
    >
    > .msg_msg_alloc_security = smack_msg_msg_alloc_security,
    > .msg_msg_free_security = smack_msg_msg_free_security,
    > @@ -2550,12 +2717,22 @@ struct security_operations smack_ops = {
    > .sk_free_security = smack_sk_free_security,
    > .sock_graft = smack_sock_graft,
    > .inet_conn_request = smack_inet_conn_request,
    > +
    > /* key management security hooks */
    > #ifdef CONFIG_KEYS
    > .key_alloc = smack_key_alloc,
    > .key_free = smack_key_free,
    > .key_permission = smack_key_permission,
    > #endif /* CONFIG_KEYS */
    > +
    > + /* Audit hooks */
    > +#ifdef CONFIG_AUDIT
    > + .audit_rule_init = smack_audit_rule_init,
    > + .audit_rule_known = smack_audit_rule_known,
    > + .audit_rule_match = smack_audit_rule_match,
    > + .audit_rule_free = smack_audit_rule_free,
    > +#endif /* CONFIG_AUDIT */
    > +
    > .secid_to_secctx = smack_secid_to_secctx,
    > .secctx_to_secid = smack_secctx_to_secid,
    > .release_secctx = smack_release_secctx,
    >
    > Regards,
    >
    > --
    >
    > "Better to light a candle, than curse the darkness"
    >
    > Ahmed S. Darwish
    > Homepage: http://darwish.07.googlepages.com
    > Blog: http://darwish-07.blogspot.com
    >
    >
    >



    Casey Schaufler
    casey@schaufler-ca.com
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [RFC][PATCH] Smack<->Audit integration

    On Mon, Mar 10, 2008 at 09:07:08AM -0700, Casey Schaufler wrote:
    >
    > --- "Ahmed S. Darwish" wrote:
    >

    ....
    > >
    > > diff --git a/security/smack/smack.h b/security/smack/smack.h
    > > index c444f48..2c8bb4c 100644
    > > --- a/security/smack/smack.h
    > > +++ b/security/smack/smack.h
    > > @@ -57,6 +57,15 @@ struct inode_smack {
    > > char *smk_inode; /* label of the fso */
    > > struct mutex smk_lock; /* initialization lock */
    > > int smk_flags; /* smack inode flags */
    > > + int secid; /* security identifier */

    >
    > No.
    >
    > Secid's are horrid things and every effort should be made to
    > expunge them from the known universe. Under no circumstances
    > should thier use be expanded. The only reason Smack has them
    > at all is because certain interfaces that in my mind should
    > have known better use them. If you must deal with secids,
    > and for this round of audit I think that's a given, use
    > smack_to_secid(sp->smk_inode) where you need to. If there's a
    > real performance issue apply intelligence to smack_to_secid
    > instead of storing the secid. There ought to be a way to
    > use container_of to do smack_to_secid, but I had trouble with
    > that and moved along without figuring out what I had done
    > wrong.
    >


    mm .. I should have remembered the un-official Smack motto:
    "Everything is a label, and whenever possible, this label is allocated
    once through system lifetime"

    About performance, yes there'll be issues searching labels espicially
    in audit_rule_match() which got called at the end of every system call.
    I'll try it using container_of (it should work at the end).

    ....
    >
    > > @@ -1696,9 +1738,9 @@ static int smack_msg_queue_alloc_security(struct
    > > msg_queue *msq)
    > > */
    > > static void smack_msg_queue_free_security(struct msg_queue *msq)
    > > {
    > > - struct kern_ipc_perm *kisp = &msq->q_perm;
    > > + struct kern_ipc_perm *ipcp = &msq->q_perm;
    > >
    > > - kisp->security = NULL;
    > > + kfree(ipcp->security);
    > > }

    >
    > Don't you just hate repetative reviewers?
    >


    Probably hating secids with passion ?

    Admittedly, after some thinking I felt now that they don't fit with
    the Smack model very well.

    ....
    > > +
    > > +/**
    > > + * smack_audit_rule_match - Audit given secid identified object ?
    > > + * @secid: Security id to test
    > > + * @field: Message flags given from user-space
    > > + * @op: Required operation (only equality is allowed)
    > > + * @vrule: Smack audit rule that will be checked against the secid object
    > > + * @actx: audit context associated with the check (used for Audit logging)
    > > + *
    > > + * This is the core Audit hook. It's used to identify objects like
    > > + * syscalls and inodes requested from user-space to be audited from
    > > + * remaining kernel objects.
    > > + */
    > > +static int smack_audit_rule_match(u32 secid, u32 field, u32 op, void *vrule,
    > > + struct audit_context *actx)
    > > +{
    > > + struct smack_known *smk_rule = vrule;

    >
    > char *smack;
    >


    More of "everything is a label".

    > > +
    > > + if (!smk_rule) {
    > > + audit_log(actx, GFP_KERNEL, AUDIT_SELINUX_ERR,
    > > + "Smack: missing rule\n");
    > > + return -ENOENT;
    > > + }
    > > +
    > > + if (field != AUDIT_SUBJ_USER && field != AUDIT_OBJ_USER)
    > > + return 0;
    > > +

    >
    > smack = smack_from_secid(secid);
    >
    > > + if (op == AUDIT_EQUAL)
    > > + return (smk_rule->smk_secid == secid);
    > > + if (op == AUDIT_NOT_EQUAL)
    > > + return (smk_rule->smk_secid != secid);

    >
    > if (op == AUDIT_EQUAL)
    > return (smk_rule->smk_smack == smack);
    > if (op == AUDIT_NOT_EQUAL)
    > return (smk_rule->smk_smack == smack);
    >


    You've meant using the short-circuit:
    smk_rule->smk_smack == smack || strnmp(smack, ..., ..)
    Right ?

    ....
    > > +
    > > +/**
    > > + * smack_audit_rule_free - free internal audit rule representation
    > > + * @vrule: rule to be freed.
    > > + *
    > > + * No memory was allocated in audit_rule_init.
    > > + */
    > > +static void smack_audit_rule_free(void *vrule)
    > > +{
    > > + /* No-op */
    > > +}


    This little no-op was the only thing that was agreed upon

    ....
    >
    > Casey Schaufler
    > casey@schaufler-ca.com


    Regards,

    --

    "Better to light a candle, than curse the darkness"

    Ahmed S. Darwish
    Homepage: http://darwish.07.googlepages.com
    Blog: http://darwish-07.blogspot.com

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [RFC][PATCH] Smack<->Audit integration


    --- "Ahmed S. Darwish" wrote:

    > ...
    > >

    >
    > You've meant using the short-circuit:
    > smk_rule->smk_smack == smack || strnmp(smack, ..., ..)
    > Right ?


    Yes, that's the safe approach.

    Thank you.


    Casey Schaufler
    casey@schaufler-ca.com
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. [RFC][PATCH -v2] Smack: Integrate with Audit

    Hi!,

    Setup the new Audit hooks for Smack. The AUDIT_SUBJ_USER and
    AUDIT_OBJ_USER SELinux flags are recycled to avoid `auditd'
    userspace modifications. Smack only needs auditing on
    a subject/object bases, so those flags were enough.

    Signed-off-by: Ahmed S. Darwish
    ---

    smack_lsm.c | 153 ++++++++++++++++++++++++++++++++++++++++++++
    1 file changed, 153 insertions(+)

    diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
    index afa7967..d471839 100644
    --- a/security/smack/smack_lsm.c
    +++ b/security/smack/smack_lsm.c
    @@ -26,6 +26,7 @@
    #include
    #include
    #include
    +#include

    #include "smack.h"

    @@ -759,6 +760,17 @@ static int smack_inode_listsecurity(struct inode *inode, char *buffer,
    return -EINVAL;
    }

    +/**
    + * smack_inode_getsecid - Extract inode's security id
    + * @inode: inode to extract the info from
    + * @secid: where result will be saved
    + */
    +static void smack_inode_getsecid(const struct inode *inode, u32 *secid)
    +{
    + struct inode_smack *isp = inode->i_security;
    + *secid = smack_to_secid(isp->smk_inode);
    +}
    +
    /*
    * File Hooks
    */
    @@ -1814,6 +1826,17 @@ static int smack_ipc_permission(struct kern_ipc_perm *ipp, short flag)
    return smk_curacc(isp, may);
    }

    +/**
    + * smack_ipc_getsecid - Extract ipc object security id
    + * @ipp: the object permissions
    + * @secid: where result will be saved
    + */
    +static void smack_ipc_getsecid(struct kern_ipc_perm *ipp, u32 *secid)
    +{
    + char *smack = ipp->security;
    + *secid = smack_to_secid(smack);
    +}
    +
    /* module stacking operations */

    /**
    @@ -2391,6 +2414,124 @@ static int smack_key_permission(key_ref_t key_ref,
    #endif /* CONFIG_KEYS */

    /*
    + * Smack Audit hooks
    + *
    + * Audit requires a unique representation of each Smack specific
    + * rule. This unique representation is used to distinguish the
    + * object to be audited from remaining kernel objects and also
    + * works as a glue between the audit hooks.
    + *
    + * Since repository entries are added but never deleted, we'll use
    + * the smack_known label address related to the given audit rule as
    + * the needed unique representation. This also better fits the smack
    + * model where nearly everything is a label.
    + */
    +#ifdef CONFIG_AUDIT
    +
    +/**
    + * smack_audit_rule_init - Initialize a smack audit rule
    + * @field: audit rule fields given from user-space (audit.h)
    + * @op: required testing operator (=, !=, >, <, ...)
    + * @rulestr: smack label to be audited
    + * @vrule: pointer to save our own audit rule representation
    + *
    + * Prepare to audit cases where (@field @op @rulestr) is true.
    + * The label to be audited is created if necessay.
    + */
    +static int smack_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule)
    +{
    + char **rule = (char **)vrule;
    + *rule = NULL;
    +
    + if (field != AUDIT_SUBJ_USER && field != AUDIT_OBJ_USER)
    + return -EINVAL;
    +
    + if (op != AUDIT_EQUAL && op != AUDIT_NOT_EQUAL)
    + return -EINVAL;
    +
    + *rule = smk_import(rulestr, 0);
    +
    + return 0;
    +}
    +
    +/**
    + * smack_audit_rule_known - Distinguish Smack audit rules
    + * @krule: rule of interest, in Audit kernel representation format
    + *
    + * This is used to filter Smack rules from remaining Audit ones.
    + * If it's proved that this rule belongs to us, the
    + * audit_rule_match hook will be called to do the final judgement.
    + */
    +static int smack_audit_rule_known(struct audit_krule *krule)
    +{
    + struct audit_field *f;
    + int i;
    +
    + for (i = 0; i < krule->field_count; i++) {
    + f = &krule->fields[i];
    +
    + if (f->type == AUDIT_SUBJ_USER || f->type == AUDIT_OBJ_USER)
    + return 1;
    + }
    +
    + return 0;
    +}
    +
    +/**
    + * smack_audit_rule_match - Audit given object ?
    + * @secid: security id for identifying the object to test
    + * @field: audit rule flags given from user-space
    + * @op: required testing operator
    + * @vrule: smack internal rule presentation
    + * @actx: audit context associated with the check
    + *
    + * The core Audit hook. It's used to take the decision of
    + * whether to audit or not to audit a given object.
    + */
    +static int smack_audit_rule_match(u32 secid, u32 field, u32 op, void *vrule,
    + struct audit_context *actx)
    +{
    + char *smack;
    + char *rule = vrule;
    +
    + if (!rule) {
    + audit_log(actx, GFP_KERNEL, AUDIT_SELINUX_ERR,
    + "Smack: missing rule\n");
    + return -ENOENT;
    + }
    +
    + if (field != AUDIT_SUBJ_USER && field != AUDIT_OBJ_USER)
    + return 0;
    +
    + smack = smack_from_secid(secid);
    +
    + /*
    + * No need to do string comparisons since we're sure
    + * that if a match occurs, both pointers will point
    + * to the same smack_konwn label.
    + */
    + if (op == AUDIT_EQUAL)
    + return (rule == smack);
    + if (op == AUDIT_NOT_EQUAL)
    + return (rule != smack);
    +
    + return 0;
    +}
    +
    +/**
    + * smack_audit_rule_free - free smack rule representation
    + * @vrule: rule to be freed.
    + *
    + * No memory was allocated.
    + */
    +static void smack_audit_rule_free(void *vrule)
    +{
    + /* No-op */
    +}
    +
    +#endif /* CONFIG_AUDIT */
    +
    +/*
    * smack_secid_to_secctx - return the smack label for a secid
    * @secid: incoming integer
    * @secdata: destination
    @@ -2476,6 +2617,7 @@ struct security_operations smack_ops = {
    .inode_getsecurity = smack_inode_getsecurity,
    .inode_setsecurity = smack_inode_setsecurity,
    .inode_listsecurity = smack_inode_listsecurity,
    + .inode_getsecid = smack_inode_getsecid,

    .file_permission = smack_file_permission,
    .file_alloc_security = smack_file_alloc_security,
    @@ -2506,6 +2648,7 @@ struct security_operations smack_ops = {
    .task_to_inode = smack_task_to_inode,

    .ipc_permission = smack_ipc_permission,
    + .ipc_getsecid = smack_ipc_getsecid,

    .msg_msg_alloc_security = smack_msg_msg_alloc_security,
    .msg_msg_free_security = smack_msg_msg_free_security,
    @@ -2550,12 +2693,22 @@ struct security_operations smack_ops = {
    .sk_free_security = smack_sk_free_security,
    .sock_graft = smack_sock_graft,
    .inet_conn_request = smack_inet_conn_request,
    +
    /* key management security hooks */
    #ifdef CONFIG_KEYS
    .key_alloc = smack_key_alloc,
    .key_free = smack_key_free,
    .key_permission = smack_key_permission,
    #endif /* CONFIG_KEYS */
    +
    + /* Audit hooks */
    +#ifdef CONFIG_AUDIT
    + .audit_rule_init = smack_audit_rule_init,
    + .audit_rule_known = smack_audit_rule_known,
    + .audit_rule_match = smack_audit_rule_match,
    + .audit_rule_free = smack_audit_rule_free,
    +#endif /* CONFIG_AUDIT */
    +
    .secid_to_secctx = smack_secid_to_secctx,
    .secctx_to_secid = smack_secctx_to_secid,
    .release_secctx = smack_release_secctx,

    --

    "Better to light a candle, than curse the darkness"

    Ahmed S. Darwish
    Homepage: http://darwish.07.googlepages.com
    Blog: http://darwish-07.blogspot.com

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: [RFC][PATCH -v2] Smack: Integrate with Audit


    --- "Ahmed S. Darwish" wrote:

    > Hi!,
    >
    > Setup the new Audit hooks for Smack. The AUDIT_SUBJ_USER and
    > AUDIT_OBJ_USER SELinux flags are recycled to avoid `auditd'
    > userspace modifications. Smack only needs auditing on
    > a subject/object bases, so those flags were enough.
    >
    > Signed-off-by: Ahmed S. Darwish
    > ---
    >
    > smack_lsm.c | 153 ++++++++++++++++++++++++++++++++++++++++++++
    > 1 file changed, 153 insertions(+)
    >
    > diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
    > index afa7967..d471839 100644
    > --- a/security/smack/smack_lsm.c
    > +++ b/security/smack/smack_lsm.c
    > @@ -26,6 +26,7 @@
    > #include
    > #include
    > #include
    > +#include
    >
    > #include "smack.h"
    >
    > @@ -759,6 +760,17 @@ static int smack_inode_listsecurity(struct inode *inode,
    > char *buffer,
    > return -EINVAL;
    > }
    >
    > +/**
    > + * smack_inode_getsecid - Extract inode's security id
    > + * @inode: inode to extract the info from
    > + * @secid: where result will be saved
    > + */
    > +static void smack_inode_getsecid(const struct inode *inode, u32 *secid)
    > +{
    > + struct inode_smack *isp = inode->i_security;


    How about a blank line between the declareations and the code?

    > + *secid = smack_to_secid(isp->smk_inode);
    > +}
    > +
    > /*
    > * File Hooks
    > */
    > @@ -1814,6 +1826,17 @@ static int smack_ipc_permission(struct kern_ipc_perm
    > *ipp, short flag)
    > return smk_curacc(isp, may);
    > }
    >
    > +/**
    > + * smack_ipc_getsecid - Extract ipc object security id
    > + * @ipp: the object permissions
    > + * @secid: where result will be saved
    > + */
    > +static void smack_ipc_getsecid(struct kern_ipc_perm *ipp, u32 *secid)
    > +{
    > + char *smack = ipp->security;


    Blank line

    > + *secid = smack_to_secid(smack);
    > +}
    > +
    > /* module stacking operations */
    >
    > /**
    > @@ -2391,6 +2414,124 @@ static int smack_key_permission(key_ref_t key_ref,
    > #endif /* CONFIG_KEYS */
    >
    > /*
    > + * Smack Audit hooks
    > + *
    > + * Audit requires a unique representation of each Smack specific
    > + * rule. This unique representation is used to distinguish the
    > + * object to be audited from remaining kernel objects and also
    > + * works as a glue between the audit hooks.
    > + *
    > + * Since repository entries are added but never deleted, we'll use
    > + * the smack_known label address related to the given audit rule as
    > + * the needed unique representation. This also better fits the smack
    > + * model where nearly everything is a label.
    > + */
    > +#ifdef CONFIG_AUDIT
    > +
    > +/**
    > + * smack_audit_rule_init - Initialize a smack audit rule
    > + * @field: audit rule fields given from user-space (audit.h)
    > + * @op: required testing operator (=, !=, >, <, ...)


    We could say that label1 > label2 if a subject with label1 can
    read an object with label2, and that label3 < label4 if a subject
    with label3 cannot read an object with label4. But that's pretty
    arbitrary. Let's leave it as you have it, at least for now.

    > + * @rulestr: smack label to be audited
    > + * @vrule: pointer to save our own audit rule representation
    > + *
    > + * Prepare to audit cases where (@field @op @rulestr) is true.
    > + * The label to be audited is created if necessay.
    > + */
    > +static int smack_audit_rule_init(u32 field, u32 op, char *rulestr, void
    > **vrule)
    > +{
    > + char **rule = (char **)vrule;
    > + *rule = NULL;
    > +
    > + if (field != AUDIT_SUBJ_USER && field != AUDIT_OBJ_USER)
    > + return -EINVAL;
    > +
    > + if (op != AUDIT_EQUAL && op != AUDIT_NOT_EQUAL)
    > + return -EINVAL;
    > +
    > + *rule = smk_import(rulestr, 0);
    > +
    > + return 0;
    > +}
    > +
    > +/**
    > + * smack_audit_rule_known - Distinguish Smack audit rules
    > + * @krule: rule of interest, in Audit kernel representation format
    > + *
    > + * This is used to filter Smack rules from remaining Audit ones.
    > + * If it's proved that this rule belongs to us, the
    > + * audit_rule_match hook will be called to do the final judgement.
    > + */
    > +static int smack_audit_rule_known(struct audit_krule *krule)
    > +{
    > + struct audit_field *f;
    > + int i;
    > +
    > + for (i = 0; i < krule->field_count; i++) {
    > + f = &krule->fields[i];
    > +
    > + if (f->type == AUDIT_SUBJ_USER || f->type == AUDIT_OBJ_USER)
    > + return 1;
    > + }
    > +
    > + return 0;
    > +}
    > +
    > +/**
    > + * smack_audit_rule_match - Audit given object ?
    > + * @secid: security id for identifying the object to test
    > + * @field: audit rule flags given from user-space
    > + * @op: required testing operator
    > + * @vrule: smack internal rule presentation
    > + * @actx: audit context associated with the check
    > + *
    > + * The core Audit hook. It's used to take the decision of
    > + * whether to audit or not to audit a given object.
    > + */
    > +static int smack_audit_rule_match(u32 secid, u32 field, u32 op, void *vrule,
    > + struct audit_context *actx)
    > +{
    > + char *smack;
    > + char *rule = vrule;
    > +
    > + if (!rule) {
    > + audit_log(actx, GFP_KERNEL, AUDIT_SELINUX_ERR,
    > + "Smack: missing rule\n");
    > + return -ENOENT;
    > + }
    > +
    > + if (field != AUDIT_SUBJ_USER && field != AUDIT_OBJ_USER)
    > + return 0;
    > +
    > + smack = smack_from_secid(secid);
    > +
    > + /*
    > + * No need to do string comparisons since we're sure
    > + * that if a match occurs, both pointers will point
    > + * to the same smack_konwn label.


    smack_known, not smack_konwn. Must be getting early there.

    > + */
    > + if (op == AUDIT_EQUAL)
    > + return (rule == smack);
    > + if (op == AUDIT_NOT_EQUAL)
    > + return (rule != smack);
    > +
    > + return 0;
    > +}
    > +
    > +/**
    > + * smack_audit_rule_free - free smack rule representation
    > + * @vrule: rule to be freed.
    > + *
    > + * No memory was allocated.
    > + */
    > +static void smack_audit_rule_free(void *vrule)
    > +{
    > + /* No-op */
    > +}
    > +
    > +#endif /* CONFIG_AUDIT */
    > +
    > +/*
    > * smack_secid_to_secctx - return the smack label for a secid
    > * @secid: incoming integer
    > * @secdata: destination
    > @@ -2476,6 +2617,7 @@ struct security_operations smack_ops = {
    > .inode_getsecurity = smack_inode_getsecurity,
    > .inode_setsecurity = smack_inode_setsecurity,
    > .inode_listsecurity = smack_inode_listsecurity,
    > + .inode_getsecid = smack_inode_getsecid,
    >
    > .file_permission = smack_file_permission,
    > .file_alloc_security = smack_file_alloc_security,
    > @@ -2506,6 +2648,7 @@ struct security_operations smack_ops = {
    > .task_to_inode = smack_task_to_inode,
    >
    > .ipc_permission = smack_ipc_permission,
    > + .ipc_getsecid = smack_ipc_getsecid,
    >
    > .msg_msg_alloc_security = smack_msg_msg_alloc_security,
    > .msg_msg_free_security = smack_msg_msg_free_security,
    > @@ -2550,12 +2693,22 @@ struct security_operations smack_ops = {
    > .sk_free_security = smack_sk_free_security,
    > .sock_graft = smack_sock_graft,
    > .inet_conn_request = smack_inet_conn_request,
    > +
    > /* key management security hooks */
    > #ifdef CONFIG_KEYS
    > .key_alloc = smack_key_alloc,
    > .key_free = smack_key_free,
    > .key_permission = smack_key_permission,
    > #endif /* CONFIG_KEYS */
    > +
    > + /* Audit hooks */
    > +#ifdef CONFIG_AUDIT
    > + .audit_rule_init = smack_audit_rule_init,
    > + .audit_rule_known = smack_audit_rule_known,
    > + .audit_rule_match = smack_audit_rule_match,
    > + .audit_rule_free = smack_audit_rule_free,
    > +#endif /* CONFIG_AUDIT */
    > +
    > .secid_to_secctx = smack_secid_to_secctx,
    > .secctx_to_secid = smack_secctx_to_secid,
    > .release_secctx = smack_release_secctx,
    >
    > --
    >
    > "Better to light a candle, than curse the darkness"
    >
    > Ahmed S. Darwish
    > Homepage: http://darwish.07.googlepages.com
    > Blog: http://darwish-07.blogspot.com



    Casey Schaufler
    casey@schaufler-ca.com
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  7. [PATCH -v2b] Smack: Integrate with Audit

    Hi!,

    [ Minor styling fixes ]

    -->

    Setup the new Audit hooks for Smack. SELinux Audit rule fields
    are recycled to avoid `auditd' userspace modifications.
    Currently only equality testing is supported on labels acting
    as a subject (AUDIT_SUBJ_USER) or as an object (AUDIT_OBJ_USER).

    Signed-off-by: Ahmed S. Darwish
    ---

    diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
    index afa7967..f999ab5 100644
    --- a/security/smack/smack_lsm.c
    +++ b/security/smack/smack_lsm.c
    @@ -26,6 +26,7 @@
    #include
    #include
    #include
    +#include

    #include "smack.h"

    @@ -759,6 +760,18 @@ static int smack_inode_listsecurity(struct inode *inode, char *buffer,
    return -EINVAL;
    }

    +/**
    + * smack_inode_getsecid - Extract inode's security id
    + * @inode: inode to extract the info from
    + * @secid: where result will be saved
    + */
    +static void smack_inode_getsecid(const struct inode *inode, u32 *secid)
    +{
    + struct inode_smack *isp = inode->i_security;
    +
    + *secid = smack_to_secid(isp->smk_inode);
    +}
    +
    /*
    * File Hooks
    */
    @@ -1814,6 +1827,18 @@ static int smack_ipc_permission(struct kern_ipc_perm *ipp, short flag)
    return smk_curacc(isp, may);
    }

    +/**
    + * smack_ipc_getsecid - Extract smack security id
    + * @ipcp: the object permissions
    + * @secid: where result will be saved
    + */
    +static void smack_ipc_getsecid(struct kern_ipc_perm *ipp, u32 *secid)
    +{
    + char *smack = ipp->security;
    +
    + *secid = smack_to_secid(smack);
    +}
    +
    /* module stacking operations */

    /**
    @@ -2391,6 +2416,124 @@ static int smack_key_permission(key_ref_t key_ref,
    #endif /* CONFIG_KEYS */

    /*
    + * Smack Audit hooks
    + *
    + * Audit requires a unique representation of each Smack specific
    + * rule. This unique representation is used to distinguish the
    + * object to be audited from remaining kernel objects and also
    + * works as a glue between the audit hooks.
    + *
    + * Since repository entries are added but never deleted, we'll use
    + * the smack_known label address related to the given audit rule as
    + * the needed unique representation. This also better fits the smack
    + * model where nearly everything is a label.
    + */
    +#ifdef CONFIG_AUDIT
    +
    +/**
    + * smack_audit_rule_init - Initialize a smack audit rule
    + * @field: audit rule fields given from user-space (audit.h)
    + * @op: required testing operator (=, !=, >, <, ...)
    + * @rulestr: smack label to be audited
    + * @vrule: pointer to save our own audit rule representation
    + *
    + * Prepare to audit cases where (@field @op @rulestr) is true.
    + * The label to be audited is created if necessay.
    + */
    +static int smack_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule)
    +{
    + char **rule = (char **)vrule;
    + *rule = NULL;
    +
    + if (field != AUDIT_SUBJ_USER && field != AUDIT_OBJ_USER)
    + return -EINVAL;
    +
    + if (op != AUDIT_EQUAL && op != AUDIT_NOT_EQUAL)
    + return -EINVAL;
    +
    + *rule = smk_import(rulestr, 0);
    +
    + return 0;
    +}
    +
    +/**
    + * smack_audit_rule_known - Distinguish Smack audit rules
    + * @krule: rule of interest, in Audit kernel representation format
    + *
    + * This is used to filter Smack rules from remaining Audit ones.
    + * If it's proved that this rule belongs to us, the
    + * audit_rule_match hook will be called to do the final judgement.
    + */
    +static int smack_audit_rule_known(struct audit_krule *krule)
    +{
    + struct audit_field *f;
    + int i;
    +
    + for (i = 0; i < krule->field_count; i++) {
    + f = &krule->fields[i];
    +
    + if (f->type == AUDIT_SUBJ_USER || f->type == AUDIT_OBJ_USER)
    + return 1;
    + }
    +
    + return 0;
    +}
    +
    +/**
    + * smack_audit_rule_match - Audit given object ?
    + * @secid: security id for identifying the object to test
    + * @field: audit rule flags given from user-space
    + * @op: required testing operator
    + * @vrule: smack internal rule presentation
    + * @actx: audit context associated with the check
    + *
    + * The core Audit hook. It's used to take the decision of
    + * whether to audit or not to audit a given object.
    + */
    +static int smack_audit_rule_match(u32 secid, u32 field, u32 op, void *vrule,
    + struct audit_context *actx)
    +{
    + char *smack;
    + char *rule = vrule;
    +
    + if (!rule) {
    + audit_log(actx, GFP_KERNEL, AUDIT_SELINUX_ERR,
    + "Smack: missing rule\n");
    + return -ENOENT;
    + }
    +
    + if (field != AUDIT_SUBJ_USER && field != AUDIT_OBJ_USER)
    + return 0;
    +
    + smack = smack_from_secid(secid);
    +
    + /*
    + * No need to do string comparisons. If a match occurs,
    + * both pointers will point to the same smack_known
    + * label.
    + */
    + if (op == AUDIT_EQUAL)
    + return (rule == smack);
    + if (op == AUDIT_NOT_EQUAL)
    + return (rule != smack);
    +
    + return 0;
    +}
    +
    +/**
    + * smack_audit_rule_free - free smack rule representation
    + * @vrule: rule to be freed.
    + *
    + * No memory was allocated.
    + */
    +static void smack_audit_rule_free(void *vrule)
    +{
    + /* No-op */
    +}
    +
    +#endif /* CONFIG_AUDIT */
    +
    +/*
    * smack_secid_to_secctx - return the smack label for a secid
    * @secid: incoming integer
    * @secdata: destination
    @@ -2476,6 +2619,7 @@ struct security_operations smack_ops = {
    .inode_getsecurity = smack_inode_getsecurity,
    .inode_setsecurity = smack_inode_setsecurity,
    .inode_listsecurity = smack_inode_listsecurity,
    + .inode_getsecid = smack_inode_getsecid,

    .file_permission = smack_file_permission,
    .file_alloc_security = smack_file_alloc_security,
    @@ -2506,6 +2650,7 @@ struct security_operations smack_ops = {
    .task_to_inode = smack_task_to_inode,

    .ipc_permission = smack_ipc_permission,
    + .ipc_getsecid = smack_ipc_getsecid,

    .msg_msg_alloc_security = smack_msg_msg_alloc_security,
    .msg_msg_free_security = smack_msg_msg_free_security,
    @@ -2550,12 +2695,22 @@ struct security_operations smack_ops = {
    .sk_free_security = smack_sk_free_security,
    .sock_graft = smack_sock_graft,
    .inet_conn_request = smack_inet_conn_request,
    +
    /* key management security hooks */
    #ifdef CONFIG_KEYS
    .key_alloc = smack_key_alloc,
    .key_free = smack_key_free,
    .key_permission = smack_key_permission,
    #endif /* CONFIG_KEYS */
    +
    + /* Audit hooks */
    +#ifdef CONFIG_AUDIT
    + .audit_rule_init = smack_audit_rule_init,
    + .audit_rule_known = smack_audit_rule_known,
    + .audit_rule_match = smack_audit_rule_match,
    + .audit_rule_free = smack_audit_rule_free,
    +#endif /* CONFIG_AUDIT */
    +
    .secid_to_secctx = smack_secid_to_secctx,
    .secctx_to_secid = smack_secctx_to_secid,
    .release_secctx = smack_release_secctx,

    --

    "Better to light a candle, than curse the darkness"

    Ahmed S. Darwish
    Homepage: http://darwish.07.googlepages.com
    Blog: http://darwish-07.blogspot.com

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  8. Re: [RFC][PATCH -v2] Smack: Integrate with Audit


    On Wed, 2008-03-12 at 04:44 +0200, Ahmed S. Darwish wrote:
    > Hi!,
    >
    > Setup the new Audit hooks for Smack. The AUDIT_SUBJ_USER and
    > AUDIT_OBJ_USER SELinux flags are recycled to avoid `auditd'
    > userspace modifications. Smack only needs auditing on
    > a subject/object bases, so those flags were enough.


    Only question I have is whether audit folks are ok with reuse of the
    flags in this manner, and whether the _USER flag is best suited for this
    purpose if you are going to reuse an existing flag (since Smack label
    seems more like a SELinux type than a SELinux user).

    Certainly will confuse matters if a user has audit filters on SELinux
    users in their /etc/audit/audit.rules and then boots a kernel with Smack
    enabled.

    >
    > Signed-off-by: Ahmed S. Darwish
    > ---
    >
    > smack_lsm.c | 153 ++++++++++++++++++++++++++++++++++++++++++++
    > 1 file changed, 153 insertions(+)
    >
    > diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
    > index afa7967..d471839 100644
    > --- a/security/smack/smack_lsm.c
    > +++ b/security/smack/smack_lsm.c
    > @@ -26,6 +26,7 @@
    > #include
    > #include
    > #include
    > +#include
    >
    > #include "smack.h"
    >
    > @@ -759,6 +760,17 @@ static int smack_inode_listsecurity(struct inode *inode, char *buffer,
    > return -EINVAL;
    > }
    >
    > +/**
    > + * smack_inode_getsecid - Extract inode's security id
    > + * @inode: inode to extract the info from
    > + * @secid: where result will be saved
    > + */
    > +static void smack_inode_getsecid(const struct inode *inode, u32 *secid)
    > +{
    > + struct inode_smack *isp = inode->i_security;
    > + *secid = smack_to_secid(isp->smk_inode);
    > +}
    > +
    > /*
    > * File Hooks
    > */
    > @@ -1814,6 +1826,17 @@ static int smack_ipc_permission(struct kern_ipc_perm *ipp, short flag)
    > return smk_curacc(isp, may);
    > }
    >
    > +/**
    > + * smack_ipc_getsecid - Extract ipc object security id
    > + * @ipp: the object permissions
    > + * @secid: where result will be saved
    > + */
    > +static void smack_ipc_getsecid(struct kern_ipc_perm *ipp, u32 *secid)
    > +{
    > + char *smack = ipp->security;
    > + *secid = smack_to_secid(smack);
    > +}
    > +
    > /* module stacking operations */
    >
    > /**
    > @@ -2391,6 +2414,124 @@ static int smack_key_permission(key_ref_t key_ref,
    > #endif /* CONFIG_KEYS */
    >
    > /*
    > + * Smack Audit hooks
    > + *
    > + * Audit requires a unique representation of each Smack specific
    > + * rule. This unique representation is used to distinguish the
    > + * object to be audited from remaining kernel objects and also
    > + * works as a glue between the audit hooks.
    > + *
    > + * Since repository entries are added but never deleted, we'll use
    > + * the smack_known label address related to the given audit rule as
    > + * the needed unique representation. This also better fits the smack
    > + * model where nearly everything is a label.
    > + */
    > +#ifdef CONFIG_AUDIT
    > +
    > +/**
    > + * smack_audit_rule_init - Initialize a smack audit rule
    > + * @field: audit rule fields given from user-space (audit.h)
    > + * @op: required testing operator (=, !=, >, <, ...)
    > + * @rulestr: smack label to be audited
    > + * @vrule: pointer to save our own audit rule representation
    > + *
    > + * Prepare to audit cases where (@field @op @rulestr) is true.
    > + * The label to be audited is created if necessay.
    > + */
    > +static int smack_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule)
    > +{
    > + char **rule = (char **)vrule;
    > + *rule = NULL;
    > +
    > + if (field != AUDIT_SUBJ_USER && field != AUDIT_OBJ_USER)
    > + return -EINVAL;
    > +
    > + if (op != AUDIT_EQUAL && op != AUDIT_NOT_EQUAL)
    > + return -EINVAL;
    > +
    > + *rule = smk_import(rulestr, 0);
    > +
    > + return 0;
    > +}
    > +
    > +/**
    > + * smack_audit_rule_known - Distinguish Smack audit rules
    > + * @krule: rule of interest, in Audit kernel representation format
    > + *
    > + * This is used to filter Smack rules from remaining Audit ones.
    > + * If it's proved that this rule belongs to us, the
    > + * audit_rule_match hook will be called to do the final judgement.
    > + */
    > +static int smack_audit_rule_known(struct audit_krule *krule)
    > +{
    > + struct audit_field *f;
    > + int i;
    > +
    > + for (i = 0; i < krule->field_count; i++) {
    > + f = &krule->fields[i];
    > +
    > + if (f->type == AUDIT_SUBJ_USER || f->type == AUDIT_OBJ_USER)
    > + return 1;
    > + }
    > +
    > + return 0;
    > +}
    > +
    > +/**
    > + * smack_audit_rule_match - Audit given object ?
    > + * @secid: security id for identifying the object to test
    > + * @field: audit rule flags given from user-space
    > + * @op: required testing operator
    > + * @vrule: smack internal rule presentation
    > + * @actx: audit context associated with the check
    > + *
    > + * The core Audit hook. It's used to take the decision of
    > + * whether to audit or not to audit a given object.
    > + */
    > +static int smack_audit_rule_match(u32 secid, u32 field, u32 op, void *vrule,
    > + struct audit_context *actx)
    > +{
    > + char *smack;
    > + char *rule = vrule;
    > +
    > + if (!rule) {
    > + audit_log(actx, GFP_KERNEL, AUDIT_SELINUX_ERR,
    > + "Smack: missing rule\n");
    > + return -ENOENT;
    > + }
    > +
    > + if (field != AUDIT_SUBJ_USER && field != AUDIT_OBJ_USER)
    > + return 0;
    > +
    > + smack = smack_from_secid(secid);
    > +
    > + /*
    > + * No need to do string comparisons since we're sure
    > + * that if a match occurs, both pointers will point
    > + * to the same smack_konwn label.
    > + */
    > + if (op == AUDIT_EQUAL)
    > + return (rule == smack);
    > + if (op == AUDIT_NOT_EQUAL)
    > + return (rule != smack);
    > +
    > + return 0;
    > +}
    > +
    > +/**
    > + * smack_audit_rule_free - free smack rule representation
    > + * @vrule: rule to be freed.
    > + *
    > + * No memory was allocated.
    > + */
    > +static void smack_audit_rule_free(void *vrule)
    > +{
    > + /* No-op */
    > +}
    > +
    > +#endif /* CONFIG_AUDIT */
    > +
    > +/*
    > * smack_secid_to_secctx - return the smack label for a secid
    > * @secid: incoming integer
    > * @secdata: destination
    > @@ -2476,6 +2617,7 @@ struct security_operations smack_ops = {
    > .inode_getsecurity = smack_inode_getsecurity,
    > .inode_setsecurity = smack_inode_setsecurity,
    > .inode_listsecurity = smack_inode_listsecurity,
    > + .inode_getsecid = smack_inode_getsecid,
    >
    > .file_permission = smack_file_permission,
    > .file_alloc_security = smack_file_alloc_security,
    > @@ -2506,6 +2648,7 @@ struct security_operations smack_ops = {
    > .task_to_inode = smack_task_to_inode,
    >
    > .ipc_permission = smack_ipc_permission,
    > + .ipc_getsecid = smack_ipc_getsecid,
    >
    > .msg_msg_alloc_security = smack_msg_msg_alloc_security,
    > .msg_msg_free_security = smack_msg_msg_free_security,
    > @@ -2550,12 +2693,22 @@ struct security_operations smack_ops = {
    > .sk_free_security = smack_sk_free_security,
    > .sock_graft = smack_sock_graft,
    > .inet_conn_request = smack_inet_conn_request,
    > +
    > /* key management security hooks */
    > #ifdef CONFIG_KEYS
    > .key_alloc = smack_key_alloc,
    > .key_free = smack_key_free,
    > .key_permission = smack_key_permission,
    > #endif /* CONFIG_KEYS */
    > +
    > + /* Audit hooks */
    > +#ifdef CONFIG_AUDIT
    > + .audit_rule_init = smack_audit_rule_init,
    > + .audit_rule_known = smack_audit_rule_known,
    > + .audit_rule_match = smack_audit_rule_match,
    > + .audit_rule_free = smack_audit_rule_free,
    > +#endif /* CONFIG_AUDIT */
    > +
    > .secid_to_secctx = smack_secid_to_secctx,
    > .secctx_to_secid = smack_secctx_to_secid,
    > .release_secctx = smack_release_secctx,
    >

    --
    Stephen Smalley
    National Security Agency

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  9. Re: [RFC][PATCH -v2] Smack: Integrate with Audit


    --- Stephen Smalley wrote:

    >
    > On Wed, 2008-03-12 at 04:44 +0200, Ahmed S. Darwish wrote:
    > > Hi!,
    > >
    > > Setup the new Audit hooks for Smack. The AUDIT_SUBJ_USER and
    > > AUDIT_OBJ_USER SELinux flags are recycled to avoid `auditd'
    > > userspace modifications. Smack only needs auditing on
    > > a subject/object bases, so those flags were enough.

    >
    > Only question I have is whether audit folks are ok with reuse of the
    > flags in this manner, and whether the _USER flag is best suited for this
    > purpose if you are going to reuse an existing flag (since Smack label
    > seems more like a SELinux type than a SELinux user).


    To-mate-o toe-maht-o.

    There really doesn't seem to be any real reason to create a new
    flag just because the granularity is different. The choice between
    _USER and _TYPE (and _ROLE for that matter) is arbitrary from a
    functional point of view. I say that since Smack has users, but
    not types or roles, _USER makes the most sense.

    > Certainly will confuse matters if a user has audit filters on SELinux
    > users in their /etc/audit/audit.rules and then boots a kernel with Smack
    > enabled.


    Somehow I doubt that will be their biggest concern.


    Casey Schaufler
    casey@schaufler-ca.com
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  10. Re: [RFC][PATCH -v2] Smack: Integrate with Audit


    On Wed, 2008-03-12 at 08:40 -0700, Casey Schaufler wrote:
    > --- Stephen Smalley wrote:
    >
    > >
    > > On Wed, 2008-03-12 at 04:44 +0200, Ahmed S. Darwish wrote:
    > > > Hi!,
    > > >
    > > > Setup the new Audit hooks for Smack. The AUDIT_SUBJ_USER and
    > > > AUDIT_OBJ_USER SELinux flags are recycled to avoid `auditd'
    > > > userspace modifications. Smack only needs auditing on
    > > > a subject/object bases, so those flags were enough.

    > >
    > > Only question I have is whether audit folks are ok with reuse of the
    > > flags in this manner, and whether the _USER flag is best suited for this
    > > purpose if you are going to reuse an existing flag (since Smack label
    > > seems more like a SELinux type than a SELinux user).

    >
    > To-mate-o toe-maht-o.
    >
    > There really doesn't seem to be any real reason to create a new
    > flag just because the granularity is different. The choice between
    > _USER and _TYPE (and _ROLE for that matter) is arbitrary from a
    > functional point of view. I say that since Smack has users, but
    > not types or roles, _USER makes the most sense.


    Perhaps I misunderstand, but Smack labels don't represent users (i.e.
    user identity) in any way, so it seemed like a mismatch to use the _USER
    flag there. Whereas types in SELinux bear some similarity to Smack
    labels - simple unstructured names whose meaning is only defined by the
    policy rules.

    Regardless, it seems like the audit maintainers ought to weigh in on the
    matter.

    > > Certainly will confuse matters if a user has audit filters on SELinux
    > > users in their /etc/audit/audit.rules and then boots a kernel with Smack
    > > enabled.

    >
    > Somehow I doubt that will be their biggest concern.


    --
    Stephen Smalley
    National Security Agency

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  11. Re: [RFC][PATCH -v2] Smack: Integrate with Audit

    Stephen Smalley wrote:
    > On Wed, 2008-03-12 at 08:40 -0700, Casey Schaufler wrote:
    >> --- Stephen Smalley wrote:
    >>
    >>> On Wed, 2008-03-12 at 04:44 +0200, Ahmed S. Darwish wrote:
    >>>> Hi!,
    >>>>
    >>>> Setup the new Audit hooks for Smack. The AUDIT_SUBJ_USER and
    >>>> AUDIT_OBJ_USER SELinux flags are recycled to avoid `auditd'
    >>>> userspace modifications. Smack only needs auditing on
    >>>> a subject/object bases, so those flags were enough.
    >>> Only question I have is whether audit folks are ok with reuse of the
    >>> flags in this manner, and whether the _USER flag is best suited for this
    >>> purpose if you are going to reuse an existing flag (since Smack label
    >>> seems more like a SELinux type than a SELinux user).

    >> To-mate-o toe-maht-o.
    >>
    >> There really doesn't seem to be any real reason to create a new
    >> flag just because the granularity is different. The choice between
    >> _USER and _TYPE (and _ROLE for that matter) is arbitrary from a
    >> functional point of view. I say that since Smack has users, but
    >> not types or roles, _USER makes the most sense.

    >
    > Perhaps I misunderstand, but Smack labels don't represent users (i.e.
    > user identity) in any way, so it seemed like a mismatch to use the _USER
    > flag there. Whereas types in SELinux bear some similarity to Smack
    > labels - simple unstructured names whose meaning is only defined by the
    > policy rules.
    >
    > Regardless, it seems like the audit maintainers ought to weigh in on the
    > matter.


    I don't count as an audit maintainer but I think as long as the
    man page is updated to say something other than:

    subj_user
    Program's SE Linux User

    then its fine for multiple LSMs to use the same rule flags and its
    better than inventing new ones for each LSM. I don't have an opinion
    on which flag that's currently specific to SELinux should be recycled
    but I think the manpage could be made more generic for all of them.

    >>> Certainly will confuse matters if a user has audit filters on SELinux
    >>> users in their /etc/audit/audit.rules and then boots a kernel with Smack
    >>> enabled.

    >> Somehow I doubt that will be their biggest concern.


    I agree.

    -- ljk
    >


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  12. Re: [RFC][PATCH -v2] Smack: Integrate with Audit

    On Wed, Mar 12, 2008 at 11:48:17AM -0400, Stephen Smalley wrote:
    >
    > On Wed, 2008-03-12 at 08:40 -0700, Casey Schaufler wrote:
    > > --- Stephen Smalley wrote:
    > >
    > > >
    > > > On Wed, 2008-03-12 at 04:44 +0200, Ahmed S. Darwish wrote:
    > > > > Hi!,
    > > > >
    > > > > Setup the new Audit hooks for Smack. The AUDIT_SUBJ_USER and
    > > > > AUDIT_OBJ_USER SELinux flags are recycled to avoid `auditd'
    > > > > userspace modifications. Smack only needs auditing on
    > > > > a subject/object bases, so those flags were enough.
    > > >
    > > > Only question I have is whether audit folks are ok with reuse of the
    > > > flags in this manner, and whether the _USER flag is best suited for this
    > > > purpose if you are going to reuse an existing flag (since Smack label
    > > > seems more like a SELinux type than a SELinux user).

    > >
    > > To-mate-o toe-maht-o.
    > >
    > > There really doesn't seem to be any real reason to create a new
    > > flag just because the granularity is different. The choice between
    > > _USER and _TYPE (and _ROLE for that matter) is arbitrary from a
    > > functional point of view. I say that since Smack has users, but
    > > not types or roles, _USER makes the most sense.

    >
    > Perhaps I misunderstand, but Smack labels don't represent users (i.e.
    > user identity) in any way, so it seemed like a mismatch to use the _USER
    > flag there. Whereas types in SELinux bear some similarity to Smack
    > labels - simple unstructured names whose meaning is only defined by the
    > policy rules.
    >


    I think Casey meant the common use of Smack where a login program
    (openssh, bin/login, ..) sets a label for each user that logs in, thus
    letting each label effectively representing a user.

    In a sense, smack labels share a bit of _USER and _TYPE.

    > Regardless, it seems like the audit maintainers ought to weigh in on the
    > matter.
    >


    Indeed.

    Regards,

    --

    "Better to light a candle, than curse the darkness"

    Ahmed S. Darwish
    Homepage: http://darwish.07.googlepages.com
    Blog: http://darwish-07.blogspot.com

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  13. Re: [RFC][PATCH -v2] Smack: Integrate with Audit


    --- "Ahmed S. Darwish" wrote:

    >
    > > Perhaps I misunderstand, but Smack labels don't represent users (i.e.
    > > user identity) in any way, so it seemed like a mismatch to use the _USER
    > > flag there. Whereas types in SELinux bear some similarity to Smack
    > > labels - simple unstructured names whose meaning is only defined by the
    > > policy rules.
    > >

    >
    > I think Casey meant the common use of Smack where a login program
    > (openssh, bin/login, ..) sets a label for each user that logs in, thus
    > letting each label effectively representing a user.


    No, I really just don't care which name gets used because none
    of them map properly but I don't see value in adding a new one.
    I say _USER is fine. I dislike _TYPE because it implies structure
    that isn't there and I dislike _ROLE because someone may want to
    implement roles on top of Smack (it wouldn't be hard) and don't
    want to start using that term for a specific meaning that might
    give 'em fits.

    >
    > In a sense, smack labels share a bit of _USER and _TYPE.


    And maybe _ROLE, if you look at it from the right angle.
    I don't think that it matters. Create a new _LATEFORDINNER
    if that makes y'all feel better. Best of all would be to
    stick with _USER and call it done.

    Thank you.


    Casey Schaufler
    casey@schaufler-ca.com
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  14. Re: [RFC][PATCH -v2] Smack: Integrate with Audit

    On Wednesday 12 March 2008 08:52:55 am Stephen Smalley wrote:
    > Only question I have is whether audit folks are ok with reuse of the
    > flags in this manner


    I am not going to have time to look at this until next week. I don't know how
    much help I can be since I don't actually have a system that would be running
    this new MAC framework. So, the less the audit system changes, the more
    likely it is to be OK. Lots of wild changes I can't see or test is bad.

    -Steve
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread