On Monday, July 16 2007 10:59:41 pm Casey Schaufler wrote:
> --- Paul Moore <[EMAIL PROTECTED]> wrote:
> > On Saturday, July 14 2007 5:47:38 pm Casey Schaufler wrote:
> > +#include "../../net/netlabel/netlabel_domainhash.h"
> > +#include <net/cipso_ipv4.h>
> > +
> > + {snip}
> > +
> > +static void smk_cipso_doi(void)
> > +{
> > +   int rc;
> > +   struct cipso_v4_doi *doip;
> > +   struct netlbl_dom_map *ndmp;
> > +   struct netlbl_audit audit_info;
> > +
> > +   if (smk_cipso_doied != 0)
> > +           return;
> > +   smk_cipso_doied = 1;
> > +
> > +   doip = kmalloc(sizeof(struct cipso_v4_doi), GFP_KERNEL);
> > +   if (doip == NULL)
> > +           panic("smack:  Failed to initialize cipso DOI.\n");
> > +   doip->map.std = NULL;
> > +
> > +   ndmp = kmalloc(sizeof(struct netlbl_dom_map), GFP_KERNEL);
> > +   if (ndmp == NULL)
> > +           panic("smack:  Failed to initialize cipso ndmp.\n");
> > +
> > +   doip->doi = SMACK_CIPSO_DOI;
> > +   doip->type = CIPSO_V4_MAP_PASS;
> > +   doip->tags[0] = CIPSO_V4_TAG_RBITMAP;
> > +   for (rc = 1; rc < CIPSO_V4_TAG_MAXCNT; rc++)
> > +           doip->tags[rc] = CIPSO_V4_TAG_INVALID;
> > +
> > +   rc = cipso_v4_doi_add(doip);
> > +   printk("%s:%d add doi rc = %d\n", __FUNCTION__, __LINE__, rc);
> > +
> > +   /*
> > +   rc = cipso_v4_doi_domhsh_add(doip, SMACK_CIPSO_DOMAIN_NAME);
> > +   printk("%s:%d hash doi rc = %d\n", __FUNCTION__, __LINE__, rc);
> > +    */
> > +   ndmp->domain = SMACK_CIPSO_DOMAIN_NAME;
> > +   ndmp->type = NETLBL_NLTYPE_CIPSOV4;
> > +   ndmp->type_def.cipsov4 = doip;
> > +   rc = netlbl_domhsh_add(ndmp, &audit_info);
> > +   printk("%s:%d hash doi rc = %d\n", __FUNCTION__, __LINE__, rc);
> > +}
> >
> > If SMACK gets an OK for upstream we need to find a better solution than
> > the above.  The smk_cipso_doi() function pokes into too many NetLabel
> > internals.
> >
> > The two obvious options are to rely on userspace (netlabel_tools) to
> > configure NetLabel for SMACK, like SELinux, or to create a new NetLabel
> > API for allowing LSMs to manipulate the NetLabel domain mapping.
>
> A userspace scheme is a non-starter, it goes against the project
> goals of minimal dependence on helper applications. I would be happy
> to work with you to create an appropriate API.

All right, that's fair.  For the time being just throw a comment in the code 
and we'll work on something.

> > Also, any reason why you don't just use the NetLabel default domain
> > mapping?
>
> Uh, only that I couldn't figure out how to go about doing so. If it
> simplifies (there's that word again) things I'm all for it. I would
> be happy to have my ignorance dispelled.

Two things:

 1. change "ndmp->domain = SMACK_CIPSO_DOMAIN_NAME" to "ndmp->domain = NULL"
 2. change "netlbl_domhsh_add()" to "netlbl_domhsh_add_default()"

If you want to get really nitpicky the second step is optional, but I'd prefer 
you use it in case we ever need to do something radically different for the 
default NetLabel domain mapping (it's really easy as they take the same 
arguments in the same order, just change the function name).

> > +/*
> > + * CIPSO domain information.
> > + * Casey says that these may wind up being configurable
> > + * at some point.
> > + */
> > +#define SMACK_CIPSO_DOMAIN_NAME    "SmackCIPSO"
> > +#define SMACK_CIPSO_DOI            3
> > +#define SMACK_CIPSO_DIRECT 250     /* Arbitrary and bad */
> >
> > Yes, SMACK_CIPSO_DIRECT is bad.
> > There has to be a cleaner solution than this,
> > think harder ;)
>
> The cleanest solution is already in the code, where there's a 1:1
> mapping of Smack labels to Cipso level/catset maintained via
> /smack/cipso, and set administratively. A purist can always set
> this up to be complete.
>
> On the other hand, it has already been acknowledged that mapping one
> set of CIPSO tags to another is necessary in some cases, for example
> when one machine's bit 6 represents another's bits 2 and 3.

My main complaint is the "magic number" of 250.  Constants that are visible 
outside a subsystem make me very nervous, especially when they are "Arbitrary 
and bad".  I understand the desire to have a direct mapping, that isn't my 
concern, my worry is how you specify to the other end that it is a direct 
mapping.

> > +static smack_t smack_from_secattr(struct netlbl_lsm_secattr *sap)
> > +{
> > +   struct smk_cipso_entry *scp;
> > +   smack_t smack = 0LL;
> > +   int pcat;
> > +
> > +   if ((sap->flags & NETLBL_SECATTR_MLS_LVL) == 0) {
> > +           /*
> > +            * If there are flags but no level netlabel isn't
> > +            * behaving the way we expect it to.
> > +            *
> > +            * Without guidance regarding the smack value
> > +            * for the packet fall back on the network
> > +            * ambient value.
> > +            */
> > +           return smack_net_ambient;
> > +   }
> > +   /*
> > +    * Get the categories, if any
> > +    */
> > +   if ((sap->flags & NETLBL_SECATTR_MLS_CAT) != 0)
> > +           for (pcat = -1;;) {
> > +                   pcat = netlbl_secattr_catmap_walk(sap->mls_cat, pcat+1);
> > +                   if (pcat < 0)
> > +                           break;
> > +                   smack_catset_bit(pcat, &smack);
> > +           }
> > +   /*
> > +    * If it is CIPSO using smack direct mapping
> > +    * we are already done. WeeHee.
> > +    */
> > +   if (sap->mls_lvl == SMACK_CIPSO_DIRECT)
> > +           return smack;
> > +
> > +   /*
> > +    * Look it up in the supplied table if it is not a direct mapping.
> > +    */
> > +   for (scp = smack_cipso; scp != NULL; scp = scp->smk_next)
> > +           if (scp->smk_level == sap->mls_lvl && scp->smk_catset == smack)
> > +                   return scp->smk_smack;
> >
> > The NetLabel caching mechanism can help make this a lot less painful,
> > i.e. direct return of the correct "smack" instead of traversing an
> > unordered linked list each time.
>
> I got the impression that that might be the case, but as with the
> default domain, the details of how I might do so did not seem obvious
> to me. Again, I would welcome enlightenment.

Look at security/selinux/ss/services.c, down near the bottom at the NetLabel 
glue code left there.  Making use of the NetLabel cache involves the 
following things:

 * Define a cache data structure, in the case of SELinux this is
   "struct selinux_netlbl_cache", I imagine in your case it might just be
   a smack pointer
 * Define a callback function to free the cache structure, in SELinux this is
   "security_netlbl_cache_free()"
 * When you resolve a netlbl_lsm_secattr struct to a smack value you populate
   your cache data structure with whatever you want returned to you later and
   assign it to a new netlbl_lsm_secattr struct which you use in a call to
   netlbl_cache_add(), look at security_netlbl_cache_add() for the SELinux
   example[1].
 * When you get a new netlbl_lsm_secattr struct from NetLabel check to see if
   the cache flag is set, if so use the cached value directly, see
   security_netlbl_secattr_to_sid() for the SELinux example.

[1] I just realized that security_netlbl_cache_add() does everything but the 
final call to netlbl_cache_add(), oops.  I think it was mistakenly dropped in 
the patchset that split most of the NetLabel code out of ss/services.c, my 
mistake.

-- 
paul moore
linux security @ hp
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to