On Mon, Nov 13, 2017 at 5:05 PM, Richard Haines
[off-list ref] wrote:

On Mon, 2017-11-06 at 19:09 -0500, Paul Moore wrote:

quoted

On Tue, Oct 17, 2017 at 9:59 AM, Richard Haines
[off-list ref] wrote:

quoted

The SELinux SCTP implementation is explained in:
Documentation/security/SELinux-sctp.txt

Signed-off-by: Richard Haines <redacted>
---
 Documentation/security/SELinux-sctp.txt | 108 +++++++++++++
 security/selinux/hooks.c                | 268
++++++++++++++++++++++++++++++--
 security/selinux/include/classmap.h     |   3 +-
 security/selinux/include/netlabel.h     |   9 +-
 security/selinux/include/objsec.h       |   5 +
 security/selinux/netlabel.c             |  52 ++++++-
 6 files changed, 427 insertions(+), 18 deletions(-)
 create mode 100644 Documentation/security/SELinux-sctp.txt

...

quoted

quoted

+Policy Statements
+==================
+The following class and permissions to support SCTP are available
within the
+kernel:
+    class sctp_socket inherits socket { node_bind }
+
+whenever the following policy capability is enabled:
+    policycap extended_socket_class;
+
+The SELinux SCTP support adds the additional permissions that are
explained
+in the sections below:
+    association bindx connectx

Is the distinction between bind and bindx significant?  The same
question applies to connect/connectx.  I think we can probably just
reuse bind and connect in these cases.

This has been discussed before with Marcelo and keeping bindx/connectx
is a useful distinction.

My apologies, I must have forgotten/missed that discussion.  Do you
have an archive pointer?

quoted

quoted

+SCTP Peer Labeling
+===================
+An SCTP socket will only have one peer label assigned to it. This
will be
+assigned during the establishment of the first association. Once
the peer
+label has been assigned, any new associations will have the
"association"
+permission validated by checking the socket peer sid against the
received
+packets peer sid to determine whether the association should be
allowed or
+denied.
+
+NOTES:
+   1) If peer labeling is not enabled, then the peer context will
always be
+      SECINITSID_UNLABELED (unlabeled_t in Reference Policy).
+
+   2) As SCTP supports multiple endpoints with multi-homing on a
single socket
+      it is recommended that peer labels are consistent.

My apologies if I'm confused, but I thought it was multiple
associations per-endpoint, with the associations providing the
multi-homing functionality, no?

I've reworded to:
As SCTP can support more than one transport address per endpoint
(multi-homing) on a single socket, it is possible to configure policy
and NetLabel to provide different peer labels for each of these. As the
socket peer label is determined by the first associations transport
address, it is recommended that all peer labels are consistent.

I'm still not sure this makes complete sense to me, but since I'm
still not 100% confident in my understanding of SCTP I'm willing to
punt on this for the moment.

quoted

quoted

+   3) getpeercon(3) may be used by userspace to retrieve the
sockets peer
+       context.
+
+   4) If using NetLabel be aware that if a label is assigned to a
specific
+      interface, and that interface 'goes down', then the NetLabel
service
+      will remove the entry. Therefore ensure that the network
startup scripts
+      call netlabelctl(8) to set the required label (see netlabel-
config(8)
+      helper script for details).

Maybe this will be made clear as I work my way through this patch,
but
how is point #4 SCTP specific?

It's not, I added this as a useful hint as I keep forgetting about it,
I'll reword to: While not SCTP specific, be aware .....

Okay.  Better.

quoted

quoted

+/* Called whenever SCTP receives an INIT or INIT_ACK chunk */
+static int selinux_sctp_assoc_request(struct sctp_endpoint *ep,
+                                     struct sk_buff *skb,
+                                     int sctp_cid)
+{
+       struct sk_security_struct *sksec = ep->base.sk-

quoted

sk_security;

+       struct common_audit_data ad;
+       struct lsm_network_audit net = {0,};
+       u8 peerlbl_active;
+       u32 peer_sid = SECINITSID_UNLABELED;
+       u32 conn_sid;
+       int err;
+
+       if (!selinux_policycap_extsockclass)
+               return 0;

We *may* need to protect a lot of the new SCTP code with a new policy
capability, I think reusing extsockclass here could be problematic.

I hope not. I will need some direction here as I've not had problems
(yet).

It's actually not that bad, take a look at the NNP/nosuid patch from
Stephen, af63f4193f9f ("selinux: Generalize support for NNP/nosuid
SELinux domain transitions"), pay attention to the
"selinux_policycap_nnp_nosuid_transition" variable.

quoted

quoted

+               if (err)
+                       return err;
+
+               if (peer_sid == SECSID_NULL)
+                       peer_sid = SECINITSID_UNLABELED;
+       }
+
+       if (sksec->sctp_assoc_state == SCTP_ASSOC_UNSET) {
+               sksec->sctp_assoc_state = SCTP_ASSOC_SET;
+
+               /* Here as first association on socket. As the peer
SID
+                * was allowed by peer recv (and the netif/node
checks),
+                * then it is approved by policy and used as the
primary
+                * peer SID for getpeercon(3).
+                */
+               sksec->peer_sid = peer_sid;
+       } else if  (sksec->peer_sid != peer_sid) {
+               /* Other association peer SIDs are checked to
enforce
+                * consistency among the peer SIDs.
+                */
+               ad.type = LSM_AUDIT_DATA_NET;
+               ad.u.net = &net;
+               ad.u.net->sk = ep->base.sk;
+               err = avc_has_perm(sksec->peer_sid, peer_sid,
sksec->sclass,
+                                  SCTP_SOCKET__ASSOCIATION, &ad);

Can anyone think of any reason why we would ever want to allow an
association that doesn't have the same label as the existing
associations?  Maybe I'm thinking about this wrong, but I can't
imagine this being a good idea ...

This has been discussed a number of times and evolved to this ...

Yes, I think my comment was the result of faulty SCTP understanding on my part.

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