--- Paul Moore <[EMAIL PROTECTED]> wrote:

> On Friday 15 February 2008 12:38:49 am Casey Schaufler wrote:
> > From: Casey Schaufler <[EMAIL PROTECTED]>
> >
> > Smack uses CIPSO labeling, but allows for unlabeled packets
> > by specifying an "ambient" label that is applied to incoming
> > unlabeled packets. Because the other end of the connection
> > may dislike IP options, and ssh is one know application that
> > behaves thus, it is prudent to respond in kind. This patch
> > changes the network labeling behavior such that an outgoing
> > packet that would be given a CIPSO label that matches the
> > ambient label is left unlabeled. An "unlbl" domain is added
> > and the netlabel defaulting mechanism invoked rather than
> > assuming that everything is CIPSO. Locking has been added
> > around changes to the ambient label as the mechanisms used
> > to do so are more involved.
> >
> > Cleaned up some issues noted in review.
> > Make smk_cipso_doi() static.
> > Create a hook for the new security_secctx_to_secid()
> > using existing underlying code.
> > Fill in audit data for netlbl domain calls.
> > Collapse unnecessary multiple assignments.
> >
> > Signed-off-by: Casey Schaufler <[EMAIL PROTECTED]>
> 
> Hi Casey,
> 
> Thanks for the update, it's much improved.  I'd ack it except for one 
> last thing which popped up in this revision ... (and don't worry, it's 
> kinda my fault - not yours) ...
> 
> > @@ -1282,15 +1281,21 @@ static int smack_netlabel(struct sock *s
> >  {
> >     struct socket_smack *ssp;
> >     struct netlbl_lsm_secattr secattr;
> > -   int rc = 0;
> > +   int rc;
> >
> >     ssp = sk->sk_security;
> >     netlbl_secattr_init(&secattr);
> >     smack_to_secattr(ssp->smk_out, &secattr);
> > -   if (secattr.flags != NETLBL_SECATTR_NONE)
> > -           rc = netlbl_sock_setattr(sk, &secattr);
> > -
> > +   rc = netlbl_sock_setattr(sk, &secattr);
> >     netlbl_secattr_destroy(&secattr);
> > +
> > +   /*
> > +    * A return of -ENOENT from netlbl_sock_setattr
> > +    * indicates that the "domain" was not found, but that's
> > +    * not an issue because of the defaulting behavior.
> > +    */
> > +   if (rc == -ENOENT)
> > +           rc = 0;
> >     return rc;
> >  }
> 
> ... you shouldn't fix-up the return value from netlbl_sock_setattr().  
> It only returns an error when there really is an error, if there are no 
> matching domain mappings and the default catches the "domain" then the 
> function will return 0 (assuming no other failures).
> 
> The fact that you ran into this problem isn't your fault, it's mine, but 
> thankfully for both of us Pavel Emelyanov found this bug and fixed 
> it[1].  It hasn't hit Linus' tree yet but it's in the net-2.6 tree.  If 
> you can't wait for it to hit Linus' tree you can always apply the fix 
> by hand, it's pretty minor.
> 
> Sorry about that.
> 
>
[1]http://git.kernel.org/?p=linux/kernel/git/davem/net-2.6.git;a=commit;h=4c3a0a254e5d706d3fe01bf42261534858d05586

Yerk. I can put that fix into my tree, but my patch without
the "correction" makes sockets behave very badly. I can't have
people using it without Pavel's fix. Any notion on the plans to
get that in?

Thank you.


Casey Schaufler
[EMAIL PROTECTED]
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to