LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Paul Moore <paul.moore@hp.com>
To: casey@schaufler-ca.com
Cc: akpm@linux-foundation.org, torvalds@linux-foundation.org,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH] (02/14/08 Linus git) Smack unlabeled outgoing ambient packets - v3
Date: Fri, 15 Feb 2008 16:37:14 -0500	[thread overview]
Message-ID: <200802151637.14835.paul.moore@hp.com> (raw)
In-Reply-To: <291070.74513.qm@web36606.mail.mud.yahoo.com>

On Friday 15 February 2008 4:00:26 pm Casey Schaufler wrote:
> --- Paul Moore <paul.moore@hp.com> wrote:
> > On Friday 15 February 2008 12:38:49 am Casey Schaufler wrote:
> > > From: Casey Schaufler <casey@schaufler-ca.com>
> > >
> > > 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 <casey@schaufler-ca.com>
> >
> > 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=comm
>it;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?

FYI, it looks like Linus just tagged -rc2 and it does have the fix you 
need.

-- 
paul moore
linux security @ hp

  parent reply	other threads:[~2008-02-15 21:37 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-15  5:38 Casey Schaufler
2008-02-15 18:59 ` Paul Moore
2008-02-15 21:00   ` Casey Schaufler
2008-02-15 21:08     ` Paul Moore
2008-02-15 21:37     ` Paul Moore [this message]
2008-02-15 22:39       ` Casey Schaufler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200802151637.14835.paul.moore@hp.com \
    --to=paul.moore@hp.com \
    --cc=akpm@linux-foundation.org \
    --cc=casey@schaufler-ca.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    --subject='Re: [PATCH] (02/14/08 Linus git) Smack unlabeled outgoing ambient packets - v3' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).