From: Ondrej Mosnacek <omosnace@redhat.com>
To: Paul Moore <paul@paul-moore.com>
Cc: Xin Long <lucien.xin@gmail.com>,
Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>,
Jakub Kicinski <kuba@kernel.org>, netdev <netdev@vger.kernel.org>,
David Miller <davem@davemloft.net>,
SElinux list <selinux@vger.kernel.org>,
Richard Haines <richard_c_haines@btinternet.com>,
Vlad Yasevich <vyasevich@gmail.com>,
Neil Horman <nhorman@tuxdriver.com>,
"open list:SCTP PROTOCOL" <linux-sctp@vger.kernel.org>,
LSM List <linux-security-module@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
Prashanth Prahlad <pprahlad@redhat.com>
Subject: Re: [PATCH net v3 2/2] security: implement sctp_assoc_established hook in selinux
Date: Thu, 17 Feb 2022 14:41:30 +0100 [thread overview]
Message-ID: <CAFqZXNtPZkozkzpOeW31pOn1r+xcWg6MDt7p8onTMoaUZVqzEw@mail.gmail.com> (raw)
In-Reply-To: <CAHC9VhSHxk0MUR1krpmbot6iG-vqH48sRgKOnJQ0LsFTs6Jvqg@mail.gmail.com>
On Tue, Feb 15, 2022 at 9:03 PM Paul Moore <paul@paul-moore.com> wrote:
> On Mon, Feb 14, 2022 at 11:13 PM Xin Long <lucien.xin@gmail.com> wrote:
> > Looks okay to me.
> >
> > The difference from the old one is that: with
> > selinux_sctp_process_new_assoc() called in
> > selinux_sctp_assoc_established(), the client sksec->peer_sid is using
> > the first asoc's peer_secid, instead of the latest asoc's peer_secid.
> > And not sure if it will cause any problems when doing the extra check
> > sksec->peer_sid != asoc->peer_secid for the latest asoc and *returns
> > err*. But I don't know about selinux, I guess there must be a reason
> > from selinux side.
>
> Generally speaking we don't want to change any SELinux socket labels
> once it has been created. While the peer_sid is a bit different,
> changing it after userspace has access to the socket could be
> problematic. In the case where the peer_sid differs between the two
> we have a permission check which allows policy to control this
> behavior which seems like the best option at this point.
I think that maybe Xin was referring to the fact that on error return
from the hook the return code information is lost and the assoc is
just silently dropped (but I may have misunderstood). In case of a
denial (avc_has_perm() returning -EACCESS) this isn't much of a
problem, because the denial is logged in the audit log, so there is a
way to figure out why opening the association failed. In case of other
errors we could indeed do better and either log an SELINUX_ERR audit
event or at least pr_err() into the console, but there are likely
several other existing cases like this, so it would be best to do this
cleanup independently in another patch (if anyone feels up to the
task...).
--
Ondrej Mosnacek
Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.
next prev parent reply other threads:[~2022-02-17 13:42 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-12 17:59 [PATCH net v3 0/2] security: fixups for the security hooks in sctp Ondrej Mosnacek
2022-02-12 17:59 ` [PATCH net v3 1/2] security: add sctp_assoc_established hook Ondrej Mosnacek
2022-02-12 17:59 ` [PATCH net v3 2/2] security: implement sctp_assoc_established hook in selinux Ondrej Mosnacek
2022-02-14 22:14 ` Paul Moore
2022-02-15 0:54 ` Jakub Kicinski
[not found] ` <CAFSqH7zC-4Ti_mzK4ZrpCVtNVCxD8h729MezG2avJLGJ2JrMTg@mail.gmail.com>
2022-02-15 4:13 ` Xin Long
2022-02-15 20:02 ` Paul Moore
2022-02-17 13:41 ` Ondrej Mosnacek [this message]
2022-02-17 13:32 ` Ondrej Mosnacek
2022-02-12 21:58 ` [PATCH net v3 0/2] security: fixups for the security hooks in sctp Ondrej Mosnacek
2022-02-15 4:26 ` Xin Long
2022-02-15 9:41 ` Richard Haines
2022-02-15 20:08 ` Paul Moore
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=CAFqZXNtPZkozkzpOeW31pOn1r+xcWg6MDt7p8onTMoaUZVqzEw@mail.gmail.com \
--to=omosnace@redhat.com \
--cc=davem@davemloft.net \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sctp@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=lucien.xin@gmail.com \
--cc=marcelo.leitner@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=nhorman@tuxdriver.com \
--cc=paul@paul-moore.com \
--cc=pprahlad@redhat.com \
--cc=richard_c_haines@btinternet.com \
--cc=selinux@vger.kernel.org \
--cc=vyasevich@gmail.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).