Discussion:
[RFC PATCH 5/5] selinux: Add SCTP support
Richard Haines
2017-10-17 13:59:53 UTC
Permalink
The SELinux SCTP implementation is explained in:
Documentation/security/SELinux-sctp.txt

Signed-off-by: Richard Haines <***@btinternet.com>
---
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

diff --git a/Documentation/security/SELinux-sctp.txt b/Documentation/security/SELinux-sctp.txt
new file mode 100644
index 0000000..32e0255
--- /dev/null
+++ b/Documentation/security/SELinux-sctp.txt
@@ -0,0 +1,108 @@
+ SCTP SELinux Support
+ ======================
+
+Security Hooks
+===============
+
+The Documentation/security/LSM-sctp.txt document describes how the following
+sctp security hooks are utilised:
+ security_sctp_assoc_request()
+ security_sctp_bind_connect()
+ security_sctp_sk_clone()
+
+ security_inet_conn_established()
+
+
+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
+
+If userspace tools have been updated, SCTP will support the portcon
+statement as shown in the following example:
+ portcon sctp 1024-1036 system_u:object_r:sctp_ports_t:s0
+
+
+SCTP Bind, Connect and ASCONF Chunk Parameter Permission Checks
+================================================================
+The hook security_sctp_bind_connect() is called by SCTP to check permissions
+required for ipv4/ipv6 addresses based on the @optname as follows:
+
+ ------------------------------------------------------------------
+ | BINDX Permission Check |
+ | @optname | @address contains |
+ |----------------------------|-----------------------------------|
+ | SCTP_SOCKOPT_BINDX_ADD | One or more ipv4 / ipv6 addresses |
+ ------------------------------------------------------------------
+
+ ------------------------------------------------------------------
+ | BIND Permission Checks |
+ | @optname | @address contains |
+ |----------------------------|-----------------------------------|
+ | SCTP_PRIMARY_ADDR | Single ipv4 or ipv6 address |
+ | SCTP_SET_PEER_PRIMARY_ADDR | Single ipv4 or ipv6 address |
+ ------------------------------------------------------------------
+
+ ------------------------------------------------------------------
+ | CONNECTX Permission Check |
+ | @optname | @address contains |
+ |----------------------------|-----------------------------------|
+ | SCTP_SOCKOPT_CONNECTX | One or more ipv4 / ipv6 addresses |
+ ------------------------------------------------------------------
+
+ ------------------------------------------------------------------
+ | CONNECT Permission Checks |
+ | @optname | @address contains |
+ |----------------------------|-----------------------------------|
+ | SCTP_SENDMSG_CONNECT | Single ipv4 or ipv6 address |
+ | SCTP_PARAM_ADD_IP | One or more ipv4 / ipv6 addresses |
+ | SCTP_PARAM_SET_PRIMARY | Single ipv4 or ipv6 address |
+ ------------------------------------------------------------------
+
+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.
+
+ 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).
+
+ 5) The NetLabel SCTP peer labeling rules apply as discussed in the following
+ set of posts tagged "netlabel" at: http://www.paul-moore.com/blog/t.
+
+ 6) CIPSO is only supported for IPv4 addressing: socket(AF_INET, ...)
+ CALIPSO is only supported for IPv6 addressing: socket(AF_INET6, ...)
+
+ Note the following when testing CIPSO/CALIPSO:
+ a) CIPSO will send an ICMP packet if an SCTP packet cannot be
+ delivered because of an invalid label.
+ b) CALIPSO does not send an ICMP packet, just silently discards it.
+
+ 7) IPSEC is not supported as rfc3554 - sctp/ipsec support has not been
+ implemented in userspace (racoon(8) or ipsec_pluto(8)), although the
+ kernel supports SCTP/IPSEC.
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 33fd061..c3e9600 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -67,6 +67,8 @@
#include <linux/tcp.h>
#include <linux/udp.h>
#include <linux/dccp.h>
+#include <linux/sctp.h>
+#include <net/sctp/structs.h>
#include <linux/quota.h>
#include <linux/un.h> /* for Unix socket types */
#include <net/af_unix.h> /* for Unix socket types */
@@ -4119,6 +4121,23 @@ static int selinux_parse_skb_ipv4(struct sk_buff *skb,
break;
}

+#if IS_ENABLED(CONFIG_IP_SCTP)
+ case IPPROTO_SCTP: {
+ struct sctphdr _sctph, *sh;
+
+ if (ntohs(ih->frag_off) & IP_OFFSET)
+ break;
+
+ offset += ihlen;
+ sh = skb_header_pointer(skb, offset, sizeof(_sctph), &_sctph);
+ if (sh == NULL)
+ break;
+
+ ad->u.net->sport = sh->source;
+ ad->u.net->dport = sh->dest;
+ break;
+ }
+#endif
default:
break;
}
@@ -4192,6 +4211,19 @@ static int selinux_parse_skb_ipv6(struct sk_buff *skb,
break;
}

+#if IS_ENABLED(CONFIG_IP_SCTP)
+ case IPPROTO_SCTP: {
+ struct sctphdr _sctph, *sh;
+
+ sh = skb_header_pointer(skb, offset, sizeof(_sctph), &_sctph);
+ if (sh == NULL)
+ break;
+
+ ad->u.net->sport = sh->source;
+ ad->u.net->dport = sh->dest;
+ break;
+ }
+#endif
/* includes fragments */
default:
break;
@@ -4381,6 +4413,10 @@ static int selinux_socket_post_create(struct socket *sock, int family,
sksec = sock->sk->sk_security;
sksec->sclass = sclass;
sksec->sid = sid;
+ /* Allows detection of the first association on this socket */
+ if (sksec->sclass == SECCLASS_SCTP_SOCKET)
+ sksec->sctp_assoc_state = SCTP_ASSOC_UNSET;
+
err = selinux_netlbl_socket_post_create(sock->sk, family);
}

@@ -4401,11 +4437,7 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in
if (err)
goto out;

- /*
- * If PF_INET or PF_INET6, check name_bind permission for the port.
- * Multiple address binding for SCTP is not supported yet: we just
- * check the first address now.
- */
+ /* If PF_INET or PF_INET6, check name_bind permission for the port. */
family = sk->sk_family;
if (family == PF_INET || family == PF_INET6) {
char *addrp;
@@ -4417,7 +4449,13 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in
unsigned short snum;
u32 sid, node_perm;

- if (family == PF_INET) {
+ /*
+ * sctp_bindx(3) calls via selinux_sctp_bind_connect()
+ * that validates multiple binding addresses. Because of this
+ * need to check address->sa_family as it is possible to have
+ * sk->sk_family = PF_INET6 with addr->sa_family = AF_INET.
+ */
+ if (family == PF_INET || address->sa_family == AF_INET) {
if (addrlen < sizeof(struct sockaddr_in)) {
err = -EINVAL;
goto out;
@@ -4471,6 +4509,10 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in
node_perm = DCCP_SOCKET__NODE_BIND;
break;

+ case SECCLASS_SCTP_SOCKET:
+ node_perm = SCTP_SOCKET__NODE_BIND;
+ break;
+
default:
node_perm = RAWIP_SOCKET__NODE_BIND;
break;
@@ -4485,7 +4527,7 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in
ad.u.net->sport = htons(snum);
ad.u.net->family = family;

- if (family == PF_INET)
+ if (family == PF_INET || address->sa_family == AF_INET)
ad.u.net->v4info.saddr = addr4->sin_addr.s_addr;
else
ad.u.net->v6info.saddr = addr6->sin6_addr;
@@ -4510,10 +4552,12 @@ static int selinux_socket_connect(struct socket *sock, struct sockaddr *address,
return err;

/*
- * If a TCP or DCCP socket, check name_connect permission for the port.
+ * If a TCP, DCCP or SCTP socket, check name_connect permission
+ * for the port.
*/
if (sksec->sclass == SECCLASS_TCP_SOCKET ||
- sksec->sclass == SECCLASS_DCCP_SOCKET) {
+ sksec->sclass == SECCLASS_DCCP_SOCKET ||
+ sksec->sclass == SECCLASS_SCTP_SOCKET) {
struct common_audit_data ad;
struct lsm_network_audit net = {0,};
struct sockaddr_in *addr4 = NULL;
@@ -4521,7 +4565,14 @@ static int selinux_socket_connect(struct socket *sock, struct sockaddr *address,
unsigned short snum;
u32 sid, perm;

- if (sk->sk_family == PF_INET) {
+ /* sctp_connectx(3) calls via
+ *selinux_sctp_bind_connect() that validates multiple
+ * connect addresses. Because of this need to check
+ * address->sa_family as it is possible to have
+ * sk->sk_family = PF_INET6 with addr->sa_family = AF_INET.
+ */
+ if (sk->sk_family == PF_INET ||
+ address->sa_family == AF_INET) {
addr4 = (struct sockaddr_in *)address;
if (addrlen < sizeof(struct sockaddr_in))
return -EINVAL;
@@ -4534,11 +4585,21 @@ static int selinux_socket_connect(struct socket *sock, struct sockaddr *address,
}

err = sel_netport_sid(sk->sk_protocol, snum, &sid);
+
if (err)
goto out;

- perm = (sksec->sclass == SECCLASS_TCP_SOCKET) ?
- TCP_SOCKET__NAME_CONNECT : DCCP_SOCKET__NAME_CONNECT;
+ switch (sksec->sclass) {
+ case SECCLASS_TCP_SOCKET:
+ perm = TCP_SOCKET__NAME_CONNECT;
+ break;
+ case SECCLASS_DCCP_SOCKET:
+ perm = DCCP_SOCKET__NAME_CONNECT;
+ break;
+ case SECCLASS_SCTP_SOCKET:
+ perm = SCTP_SOCKET__NAME_CONNECT;
+ break;
+ }

ad.type = LSM_AUDIT_DATA_NET;
ad.u.net = &net;
@@ -4815,7 +4876,8 @@ static int selinux_socket_getpeersec_stream(struct socket *sock, char __user *op
u32 peer_sid = SECSID_NULL;

if (sksec->sclass == SECCLASS_UNIX_STREAM_SOCKET ||
- sksec->sclass == SECCLASS_TCP_SOCKET)
+ sksec->sclass == SECCLASS_TCP_SOCKET ||
+ sksec->sclass == SECCLASS_SCTP_SOCKET)
peer_sid = sksec->peer_sid;
if (peer_sid == SECSID_NULL)
return -ENOPROTOOPT;
@@ -4928,6 +4990,183 @@ static void selinux_sock_graft(struct sock *sk, struct socket *parent)
sksec->sclass = isec->sclass;
}

+/* 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->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;
+
+ peerlbl_active = selinux_peerlbl_enabled();
+
+ if (peerlbl_active) {
+ /* This will return peer_sid = SECSID_NULL if there are
+ * no peer labels, see security_net_peersid_resolve().
+ */
+ err = selinux_skb_peerlbl_sid(skb, ep->base.sk->sk_family,
+ &peer_sid);
+
+ 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);
+ if (err)
+ return err;
+ }
+
+ if (sctp_cid == SCTP_CID_INIT) {
+ /* Have INIT when incoming connect(2), sctp_connectx(3)
+ * or sctp_sendmsg(3) (with no association already present),
+ * so compute the MLS component for the connection and store
+ * the information in ep. This will be used by SCTP TCP type
+ * sockets and peeled off connections as they cause a new
+ * socket to be generated. selinux_sctp_sk_clone() will then
+ * plug this into the new socket.
+ */
+ err = selinux_conn_sid(sksec->sid, peer_sid, &conn_sid);
+ if (err)
+ return err;
+
+ ep->secid = conn_sid;
+ ep->peer_secid = peer_sid;
+
+ /* Set any NetLabel labels including CIPSO/CALIPSO options. */
+ return selinux_netlbl_sctp_assoc_request(ep, skb);
+ }
+
+ return 0;
+}
+
+/*
+ * Check if sctp IPv4/IPv6 addresses are valid for binding or connecting
+ * based on their @optname.
+ */
+static int selinux_sctp_bind_connect(struct sock *sk, int optname,
+ struct sockaddr *address,
+ int addrlen)
+{
+ int len, err = 0, walk_size = 0;
+ void *addr_buf;
+ struct sockaddr *addr;
+ struct socket *sock;
+
+ if (!selinux_policycap_extsockclass)
+ return 0;
+
+ switch (optname) {
+ case SCTP_SOCKOPT_BINDX_ADD:
+ err = sock_has_perm(sk, SCTP_SOCKET__BINDX);
+ break;
+ case SCTP_SOCKOPT_CONNECTX:
+ err = sock_has_perm(sk, SCTP_SOCKET__CONNECTX);
+ break;
+ /* These need SOCKET__BIND or SOCKET__CONNECT permissions that will
+ * be checked later.
+ */
+ case SCTP_PRIMARY_ADDR:
+ case SCTP_SET_PEER_PRIMARY_ADDR:
+ case SCTP_PARAM_SET_PRIMARY:
+ case SCTP_PARAM_ADD_IP:
+ case SCTP_SENDMSG_CONNECT:
+ break;
+ default:
+ err = -EINVAL;
+ }
+ if (err)
+ return err;
+
+ /* Process one or more addresses that may be IPv4 or IPv6 */
+ sock = sk->sk_socket;
+ addr_buf = address;
+
+ while (walk_size < addrlen) {
+ addr = addr_buf;
+ switch (addr->sa_family) {
+ case AF_INET:
+ len = sizeof(struct sockaddr_in);
+ break;
+ case AF_INET6:
+ len = sizeof(struct sockaddr_in6);
+ break;
+ default:
+ return -EAFNOSUPPORT;
+ }
+
+ err = -EINVAL;
+ switch (optname) {
+ /* Bind checks */
+ case SCTP_PRIMARY_ADDR:
+ case SCTP_SET_PEER_PRIMARY_ADDR:
+ case SCTP_SOCKOPT_BINDX_ADD:
+ err = selinux_socket_bind(sock, addr, len);
+ break;
+ /* Connect checks */
+ case SCTP_SOCKOPT_CONNECTX:
+ case SCTP_PARAM_SET_PRIMARY:
+ case SCTP_PARAM_ADD_IP:
+ case SCTP_SENDMSG_CONNECT:
+ err = selinux_socket_connect(sock, addr, len);
+ break;
+ }
+
+ if (err)
+ return err;
+
+ addr_buf += len;
+ walk_size += len;
+ }
+ return 0;
+}
+
+/* Called whenever a new socket is created by accept(2) or sctp_peeloff(3). */
+static void selinux_sctp_sk_clone(struct sctp_endpoint *ep, struct sock *sk,
+ struct sock *newsk)
+{
+ struct sk_security_struct *sksec = sk->sk_security;
+ struct sk_security_struct *newsksec = newsk->sk_security;
+
+ /* If policy does not support SECCLASS_SCTP_SOCKET then call
+ * the non-sctp clone version.
+ */
+ if (!selinux_policycap_extsockclass)
+ return selinux_sk_clone_security(sk, newsk);
+
+ newsksec->sid = ep->secid;
+ newsksec->peer_sid = ep->peer_secid;
+ newsksec->sclass = sksec->sclass;
+ newsksec->nlbl_state = sksec->nlbl_state;
+}
+
static int selinux_inet_conn_request(struct sock *sk, struct sk_buff *skb,
struct request_sock *req)
{
@@ -6416,6 +6655,9 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(sk_clone_security, selinux_sk_clone_security),
LSM_HOOK_INIT(sk_getsecid, selinux_sk_getsecid),
LSM_HOOK_INIT(sock_graft, selinux_sock_graft),
+ LSM_HOOK_INIT(sctp_assoc_request, selinux_sctp_assoc_request),
+ LSM_HOOK_INIT(sctp_sk_clone, selinux_sctp_sk_clone),
+ LSM_HOOK_INIT(sctp_bind_connect, selinux_sctp_bind_connect),
LSM_HOOK_INIT(inet_conn_request, selinux_inet_conn_request),
LSM_HOOK_INIT(inet_csk_clone, selinux_inet_csk_clone),
LSM_HOOK_INIT(inet_conn_established, selinux_inet_conn_established),
diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
index b9fe343..b4b10da 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -173,7 +173,8 @@ struct security_class_mapping secclass_map[] = {
{ COMMON_CAP2_PERMS, NULL } },
{ "sctp_socket",
{ COMMON_SOCK_PERMS,
- "node_bind", NULL } },
+ "node_bind", "name_connect", "association", "bindx",
+ "connectx", NULL } },
{ "icmp_socket",
{ COMMON_SOCK_PERMS,
"node_bind", NULL } },
diff --git a/security/selinux/include/netlabel.h b/security/selinux/include/netlabel.h
index 75686d5..835a0d6 100644
--- a/security/selinux/include/netlabel.h
+++ b/security/selinux/include/netlabel.h
@@ -33,6 +33,7 @@
#include <linux/skbuff.h>
#include <net/sock.h>
#include <net/request_sock.h>
+#include <net/sctp/structs.h>

#include "avc.h"
#include "objsec.h"
@@ -53,7 +54,8 @@ int selinux_netlbl_skbuff_getsid(struct sk_buff *skb,
int selinux_netlbl_skbuff_setsid(struct sk_buff *skb,
u16 family,
u32 sid);
-
+int selinux_netlbl_sctp_assoc_request(struct sctp_endpoint *ep,
+ struct sk_buff *skb);
int selinux_netlbl_inet_conn_request(struct request_sock *req, u16 family);
void selinux_netlbl_inet_csk_clone(struct sock *sk, u16 family);
int selinux_netlbl_socket_post_create(struct sock *sk, u16 family);
@@ -114,6 +116,11 @@ static inline int selinux_netlbl_conn_setsid(struct sock *sk,
return 0;
}

+static inline int selinux_netlbl_sctp_assoc_request(struct sctp_endpoint *ep,
+ struct sk_buff *skb)
+{
+ return 0;
+}
static inline int selinux_netlbl_inet_conn_request(struct request_sock *req,
u16 family)
{
diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
index 6ebc61e..660f270 100644
--- a/security/selinux/include/objsec.h
+++ b/security/selinux/include/objsec.h
@@ -130,6 +130,11 @@ struct sk_security_struct {
u32 sid; /* SID of this object */
u32 peer_sid; /* SID of peer */
u16 sclass; /* sock security class */
+
+ enum { /* SCTP association state */
+ SCTP_ASSOC_UNSET = 0,
+ SCTP_ASSOC_SET,
+ } sctp_assoc_state;
};

struct tun_security_struct {
diff --git a/security/selinux/netlabel.c b/security/selinux/netlabel.c
index aaba667..7d5aa15 100644
--- a/security/selinux/netlabel.c
+++ b/security/selinux/netlabel.c
@@ -250,6 +250,7 @@ int selinux_netlbl_skbuff_setsid(struct sk_buff *skb,
sk = skb_to_full_sk(skb);
if (sk != NULL) {
struct sk_security_struct *sksec = sk->sk_security;
+
if (sksec->nlbl_state != NLBL_REQSKB)
return 0;
secattr = selinux_netlbl_sock_getattr(sk, sid);
@@ -271,6 +272,41 @@ int selinux_netlbl_skbuff_setsid(struct sk_buff *skb,
}

/**
+ * selinux_netlbl_sctp_assoc_request - Label an incoming sctp association.
+ * @ep: incoming association endpoint.
+ * @skb: the packet.
+ *
+ * Description:
+ * A new incoming connection is represented by @ep, ......
+ * Returns zero on success, negative values on failure.
+ *
+ */
+int selinux_netlbl_sctp_assoc_request(struct sctp_endpoint *ep,
+ struct sk_buff *skb)
+{
+ int rc;
+ struct netlbl_lsm_secattr secattr;
+ struct sk_security_struct *sksec = ep->base.sk->sk_security;
+
+ if (ep->base.sk->sk_family != PF_INET &&
+ ep->base.sk->sk_family != PF_INET6)
+ return 0;
+
+ netlbl_secattr_init(&secattr);
+ rc = security_netlbl_sid_to_secattr(ep->secid, &secattr);
+ if (rc != 0)
+ goto assoc_request_return;
+
+ rc = netlbl_sctp_setattr(ep->base.sk, skb, &secattr);
+ if (rc == 0)
+ sksec->nlbl_state = NLBL_LABELED;
+
+assoc_request_return:
+ netlbl_secattr_destroy(&secattr);
+ return rc;
+}
+
+/**
* selinux_netlbl_inet_conn_request - Label an incoming stream connection
* @req: incoming connection request socket
*
@@ -481,7 +517,7 @@ int selinux_netlbl_socket_setsockopt(struct socket *sock,
*/
int selinux_netlbl_socket_connect(struct sock *sk, struct sockaddr *addr)
{
- int rc;
+ int rc, already_owned_by_user = 0;
struct sk_security_struct *sksec = sk->sk_security;
struct netlbl_lsm_secattr *secattr;

@@ -489,7 +525,16 @@ int selinux_netlbl_socket_connect(struct sock *sk, struct sockaddr *addr)
sksec->nlbl_state != NLBL_CONNLABELED)
return 0;

- lock_sock(sk);
+ /* Note: When called via connect(2) this happens before the socket
+ * protocol layer connect operation and @sk is not locked, HOWEVER,
+ * when called by the SCTP protocol layer via sctp_connectx(3),
+ * sctp_sendmsg(3) or sendmsg(2), @sk is locked. Therefore check if
+ * @sk owned already.
+ */
+ if (sock_owned_by_user(sk) && sksec->sclass == SECCLASS_SCTP_SOCKET)
+ already_owned_by_user = 1;
+ else
+ lock_sock(sk);

/* connected sockets are allowed to disconnect when the address family
* is set to AF_UNSPEC, if that is what is happening we want to reset
@@ -510,6 +555,7 @@ int selinux_netlbl_socket_connect(struct sock *sk, struct sockaddr *addr)
sksec->nlbl_state = NLBL_CONNLABELED;

socket_connect_return:
- release_sock(sk);
+ if (!already_owned_by_user)
+ release_sock(sk);
return rc;
}
--
2.13.6
Stephen Smalley
2017-10-20 19:00:28 UTC
Permalink
Post by Richard Haines
Documentation/security/SELinux-sctp.txt
---
 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
diff --git a/Documentation/security/SELinux-sctp.txt
b/Documentation/security/SELinux-sctp.txt
new file mode 100644
index 0000000..32e0255
--- /dev/null
+++ b/Documentation/security/SELinux-sctp.txt
@@ -0,0 +1,108 @@
+                               SCTP SELinux Support
+                              ======================
+
+Security Hooks
+===============
+
+The Documentation/security/LSM-sctp.txt document describes how the following
+    security_sctp_assoc_request()
+    security_sctp_bind_connect()
+    security_sctp_sk_clone()
+
+    security_inet_conn_established()
+
+
+Policy Statements
+==================
+The following class and permissions to support SCTP are available within the
+    class sctp_socket inherits socket { node_bind }
+
+    policycap extended_socket_class;
+
+The SELinux SCTP support adds the additional permissions that are explained
+    association bindx connectx
+
+If userspace tools have been updated, SCTP will support the portcon
+    portcon sctp 1024-1036 system_u:object_r:sctp_ports_t:s0
+
+
+SCTP Bind, Connect and ASCONF Chunk Parameter Permission Checks
+================================================================
+The hook security_sctp_bind_connect() is called by SCTP to check permissions
+
+  ------------------------------------------------------------------
+  |                      BINDX Permission Check                    |
+  |----------------------------|-----------------------------------|
+  | SCTP_SOCKOPT_BINDX_ADD     | One or more ipv4 / ipv6 addresses |
+  ------------------------------------------------------------------
+
+  ------------------------------------------------------------------
+  |                  BIND Permission Checks                        |
+  |----------------------------|-----------------------------------|
+  | SCTP_PRIMARY_ADDR          | Single ipv4 or ipv6 address       |
+  | SCTP_SET_PEER_PRIMARY_ADDR | Single ipv4 or ipv6 address       |
+  ------------------------------------------------------------------
+
+  ------------------------------------------------------------------
+  |                 CONNECTX Permission Check                      |
+  |----------------------------|-----------------------------------|
+  | SCTP_SOCKOPT_CONNECTX      | One or more ipv4 / ipv6 addresses |
+  ------------------------------------------------------------------
+
+  ------------------------------------------------------------------
+  |                 CONNECT Permission Checks                      |
+  |----------------------------|-----------------------------------|
+  | SCTP_SENDMSG_CONNECT       | Single ipv4 or ipv6 address       |
+  | SCTP_PARAM_ADD_IP          | One or more ipv4 / ipv6 addresses |
+  | SCTP_PARAM_SET_PRIMARY     | Single ipv4 or ipv6 address       |
+  ------------------------------------------------------------------
+
+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.
+
+   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.
+
+   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).
+
+   5) The NetLabel SCTP peer labeling rules apply as discussed in
the following
+      set of posts tagged "netlabel" at: http://www.paul-moore.com/b
log/t.
+
+   6) CIPSO is only supported for IPv4 addressing: socket(AF_INET,
...)
socket(AF_INET6, ...)
+
+         a) CIPSO will send an ICMP packet if an SCTP packet cannot
be
+            delivered because of an invalid label.
+         b) CALIPSO does not send an ICMP packet, just silently
discards it.
+
+   7) IPSEC is not supported as rfc3554 - sctp/ipsec support has not
been
+      implemented in userspace (racoon(8) or ipsec_pluto(8)),
although the
+      kernel supports SCTP/IPSEC.
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 33fd061..c3e9600 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -67,6 +67,8 @@
 #include <linux/tcp.h>
 #include <linux/udp.h>
 #include <linux/dccp.h>
+#include <linux/sctp.h>
+#include <net/sctp/structs.h>
 #include <linux/quota.h>
 #include <linux/un.h> /* for Unix socket types */
 #include <net/af_unix.h> /* for Unix socket types */
@@ -4119,6 +4121,23 @@ static int selinux_parse_skb_ipv4(struct sk_buff *skb,
  break;
  }
 
+#if IS_ENABLED(CONFIG_IP_SCTP)
+ case IPPROTO_SCTP: {
+ struct sctphdr _sctph, *sh;
+
+ if (ntohs(ih->frag_off) & IP_OFFSET)
+ break;
+
+ offset += ihlen;
+ sh = skb_header_pointer(skb, offset, sizeof(_sctph),
&_sctph);
+ if (sh == NULL)
+ break;
+
+ ad->u.net->sport = sh->source;
+ ad->u.net->dport = sh->dest;
+ break;
+ }
+#endif
  break;
  }
@@ -4192,6 +4211,19 @@ static int selinux_parse_skb_ipv6(struct sk_buff *skb,
  break;
  }
 
+#if IS_ENABLED(CONFIG_IP_SCTP)
+ case IPPROTO_SCTP: {
+ struct sctphdr _sctph, *sh;
+
+ sh = skb_header_pointer(skb, offset, sizeof(_sctph),
&_sctph);
+ if (sh == NULL)
+ break;
+
+ ad->u.net->sport = sh->source;
+ ad->u.net->dport = sh->dest;
+ break;
+ }
+#endif
  /* includes fragments */
  break;
@@ -4381,6 +4413,10 @@ static int selinux_socket_post_create(struct
socket *sock, int family,
  sksec = sock->sk->sk_security;
  sksec->sclass = sclass;
  sksec->sid = sid;
+ /* Allows detection of the first association on this
socket */
+ if (sksec->sclass == SECCLASS_SCTP_SOCKET)
+ sksec->sctp_assoc_state = SCTP_ASSOC_UNSET;
+
What prevents this from interleaving with selinux_sctp_assoc_request()
accesses to sctp_assoc_state?
Post by Richard Haines
  err = selinux_netlbl_socket_post_create(sock->sk,
family);
  }
 
@@ -4401,11 +4437,7 @@ static int selinux_socket_bind(struct socket
*sock, struct sockaddr *address, in
  if (err)
  goto out;
 
- /*
-  * If PF_INET or PF_INET6, check name_bind permission for
the port.
we just
-  * check the first address now.
-  */
+ /* If PF_INET or PF_INET6, check name_bind permission for
the port. */
  family = sk->sk_family;
  if (family == PF_INET || family == PF_INET6) {
  char *addrp;
@@ -4417,7 +4449,13 @@ static int selinux_socket_bind(struct socket
*sock, struct sockaddr *address, in
  unsigned short snum;
  u32 sid, node_perm;
 
- if (family == PF_INET) {
+ /*
+  * sctp_bindx(3) calls via
selinux_sctp_bind_connect()
+  * that validates multiple binding addresses.
Because of this
+  * need to check address->sa_family as it is
possible to have
+  * sk->sk_family = PF_INET6 with addr->sa_family =
AF_INET.
+  */
+ if (family == PF_INET || address->sa_family ==
AF_INET) {
  if (addrlen < sizeof(struct sockaddr_in)) {
  err = -EINVAL;
  goto out;
@@ -4471,6 +4509,10 @@ static int selinux_socket_bind(struct socket
*sock, struct sockaddr *address, in
  node_perm = DCCP_SOCKET__NODE_BIND;
  break;
 
+ node_perm = SCTP_SOCKET__NODE_BIND;
+ break;
+
  node_perm = RAWIP_SOCKET__NODE_BIND;
  break;
@@ -4485,7 +4527,7 @@ static int selinux_socket_bind(struct socket
*sock, struct sockaddr *address, in
  ad.u.net->sport = htons(snum);
  ad.u.net->family = family;
 
- if (family == PF_INET)
+ if (family == PF_INET || address->sa_family ==
AF_INET)
  ad.u.net->v4info.saddr = addr4-
Post by Richard Haines
sin_addr.s_addr;
  else
  ad.u.net->v6info.saddr = addr6->sin6_addr;
@@ -4510,10 +4552,12 @@ static int selinux_socket_connect(struct
socket *sock, struct sockaddr *address,
  return err;
 
  /*
-  * If a TCP or DCCP socket, check name_connect permission
for the port.
+  * If a TCP, DCCP or SCTP socket, check name_connect
permission
+  * for the port.
   */
  if (sksec->sclass == SECCLASS_TCP_SOCKET ||
-     sksec->sclass == SECCLASS_DCCP_SOCKET) {
+     sksec->sclass == SECCLASS_DCCP_SOCKET ||
+     sksec->sclass == SECCLASS_SCTP_SOCKET) {
  struct common_audit_data ad;
  struct lsm_network_audit net = {0,};
  struct sockaddr_in *addr4 = NULL;
@@ -4521,7 +4565,14 @@ static int selinux_socket_connect(struct
socket *sock, struct sockaddr *address,
  unsigned short snum;
  u32 sid, perm;
 
- if (sk->sk_family == PF_INET) {
+ /* sctp_connectx(3) calls via
+  *selinux_sctp_bind_connect() that validates
multiple
+  * connect addresses. Because of this need to check
+  * address->sa_family as it is possible to have
+  * sk->sk_family = PF_INET6 with addr->sa_family =
AF_INET.
+  */
+ if (sk->sk_family == PF_INET ||
+ address->sa_family ==
AF_INET) {
  addr4 = (struct sockaddr_in *)address;
  if (addrlen < sizeof(struct sockaddr_in))
  return -EINVAL;
@@ -4534,11 +4585,21 @@ static int selinux_socket_connect(struct
socket *sock, struct sockaddr *address,
  }
 
  err = sel_netport_sid(sk->sk_protocol, snum, &sid);
+
  if (err)
  goto out;
 
- perm = (sksec->sclass == SECCLASS_TCP_SOCKET) ?
DCCP_SOCKET__NAME_CONNECT;
+ switch (sksec->sclass) {
+ perm = TCP_SOCKET__NAME_CONNECT;
+ break;
+ perm = DCCP_SOCKET__NAME_CONNECT;
+ break;
+ perm = SCTP_SOCKET__NAME_CONNECT;
+ break;
+ }
 
  ad.type = LSM_AUDIT_DATA_NET;
  ad.u.net = &net;
@@ -4815,7 +4876,8 @@ static int
selinux_socket_getpeersec_stream(struct socket *sock, char __user *op
  u32 peer_sid = SECSID_NULL;
 
  if (sksec->sclass == SECCLASS_UNIX_STREAM_SOCKET ||
-     sksec->sclass == SECCLASS_TCP_SOCKET)
+     sksec->sclass == SECCLASS_TCP_SOCKET ||
+     sksec->sclass == SECCLASS_SCTP_SOCKET)
  peer_sid = sksec->peer_sid;
  if (peer_sid == SECSID_NULL)
  return -ENOPROTOOPT;
@@ -4928,6 +4990,183 @@ static void selinux_sock_graft(struct sock
*sk, struct socket *parent)
  sksec->sclass = isec->sclass;
 }
 
+/* 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->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;
+
+ peerlbl_active = selinux_peerlbl_enabled();
+
+ if (peerlbl_active) {
+ /* This will return peer_sid = SECSID_NULL if there
are
+  * no peer labels, see
security_net_peersid_resolve().
+  */
+ err = selinux_skb_peerlbl_sid(skb, ep->base.sk-
Post by Richard Haines
sk_family,
+       &peer_sid);
+
+ 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-
Post by Richard Haines
sclass,
+    SCTP_SOCKET__ASSOCIATION, &ad);
+ if (err)
+ return err;
+ }
+
+ if (sctp_cid == SCTP_CID_INIT) {
+ /* Have INIT when incoming connect(2),
sctp_connectx(3)
+  * or sctp_sendmsg(3) (with no association already
present),
+  * so compute the MLS component for the connection
and store
+  * the information in ep. This will be used by SCTP
TCP type
+  * sockets and peeled off connections as they cause
a new
+  * socket to be generated. selinux_sctp_sk_clone()
will then
+  * plug this into the new socket.
+  */
+ err = selinux_conn_sid(sksec->sid, peer_sid,
&conn_sid);
+ if (err)
+ return err;
+
+ ep->secid = conn_sid;
+ ep->peer_secid = peer_sid;
+
+ /* Set any NetLabel labels including CIPSO/CALIPSO
options. */
+ return selinux_netlbl_sctp_assoc_request(ep, skb);
+ }
+
+ return 0;
+}
+
+/*
+ * Check if sctp IPv4/IPv6 addresses are valid for binding or
connecting
+ */
+static int selinux_sctp_bind_connect(struct sock *sk, int optname,
+      struct sockaddr *address,
+      int addrlen)
+{
+ int len, err = 0, walk_size = 0;
+ void *addr_buf;
+ struct sockaddr *addr;
+ struct socket *sock;
+
+ if (!selinux_policycap_extsockclass)
+ return 0;
+
+ switch (optname) {
+ err = sock_has_perm(sk, SCTP_SOCKET__BINDX);
+ break;
+ err = sock_has_perm(sk, SCTP_SOCKET__CONNECTX);
+ break;
+ /* These need SOCKET__BIND or SOCKET__CONNECT permissions
that will
+  * be checked later.
+  */
+ break;
+ err = -EINVAL;
+ }
+ if (err)
+ return err;
+
+ /* Process one or more addresses that may be IPv4 or IPv6 */
+ sock = sk->sk_socket;
+ addr_buf = address;
+
+ while (walk_size < addrlen) {
+ addr = addr_buf;
+ switch (addr->sa_family) {
+ len = sizeof(struct sockaddr_in);
+ break;
+ len = sizeof(struct sockaddr_in6);
+ break;
+ return -EAFNOSUPPORT;
+ }
+
+ err = -EINVAL;
+ switch (optname) {
+ /* Bind checks */
+ err = selinux_socket_bind(sock, addr, len);
+ break;
+ /* Connect checks */
+ err = selinux_socket_connect(sock, addr,
len);
+ break;
+ }
+
+ if (err)
+ return err;
+
+ addr_buf += len;
+ walk_size += len;
+ }
+ return 0;
+}
+
+/* Called whenever a new socket is created by accept(2) or
sctp_peeloff(3). */
+static void selinux_sctp_sk_clone(struct sctp_endpoint *ep, struct sock *sk,
+   struct sock *newsk)
+{
+ struct sk_security_struct *sksec = sk->sk_security;
+ struct sk_security_struct *newsksec = newsk->sk_security;
+
+ /* If policy does not support SECCLASS_SCTP_SOCKET then call
+  * the non-sctp clone version.
+  */
+ if (!selinux_policycap_extsockclass)
+ return selinux_sk_clone_security(sk, newsk);
+
+ newsksec->sid = ep->secid;
+ newsksec->peer_sid = ep->peer_secid;
+ newsksec->sclass = sksec->sclass;
+ newsksec->nlbl_state = sksec->nlbl_state;
+}
+
 static int selinux_inet_conn_request(struct sock *sk, struct sk_buff
*skb,
       struct request_sock *req)
 {
@@ -6416,6 +6655,9 @@ static struct security_hook_list
selinux_hooks[] __lsm_ro_after_init = {
  LSM_HOOK_INIT(sk_clone_security, selinux_sk_clone_security),
  LSM_HOOK_INIT(sk_getsecid, selinux_sk_getsecid),
  LSM_HOOK_INIT(sock_graft, selinux_sock_graft),
+ LSM_HOOK_INIT(sctp_assoc_request,
selinux_sctp_assoc_request),
+ LSM_HOOK_INIT(sctp_sk_clone, selinux_sctp_sk_clone),
+ LSM_HOOK_INIT(sctp_bind_connect, selinux_sctp_bind_connect),
  LSM_HOOK_INIT(inet_conn_request, selinux_inet_conn_request),
  LSM_HOOK_INIT(inet_csk_clone, selinux_inet_csk_clone),
  LSM_HOOK_INIT(inet_conn_established,
selinux_inet_conn_established),
diff --git a/security/selinux/include/classmap.h
b/security/selinux/include/classmap.h
index b9fe343..b4b10da 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -173,7 +173,8 @@ struct security_class_mapping secclass_map[] = {
    { COMMON_CAP2_PERMS, NULL } },
  { "sctp_socket",
    { COMMON_SOCK_PERMS,
-     "node_bind", NULL } },
+     "node_bind", "name_connect", "association", "bindx",
+     "connectx", NULL } },
  { "icmp_socket",
    { COMMON_SOCK_PERMS,
      "node_bind", NULL } },
diff --git a/security/selinux/include/netlabel.h
b/security/selinux/include/netlabel.h
index 75686d5..835a0d6 100644
--- a/security/selinux/include/netlabel.h
+++ b/security/selinux/include/netlabel.h
@@ -33,6 +33,7 @@
 #include <linux/skbuff.h>
 #include <net/sock.h>
 #include <net/request_sock.h>
+#include <net/sctp/structs.h>
 
 #include "avc.h"
 #include "objsec.h"
@@ -53,7 +54,8 @@ int selinux_netlbl_skbuff_getsid(struct sk_buff *skb,
 int selinux_netlbl_skbuff_setsid(struct sk_buff *skb,
   u16 family,
   u32 sid);
-
+int selinux_netlbl_sctp_assoc_request(struct sctp_endpoint *ep,
+      struct sk_buff *skb);
 int selinux_netlbl_inet_conn_request(struct request_sock *req, u16
family);
 void selinux_netlbl_inet_csk_clone(struct sock *sk, u16 family);
 int selinux_netlbl_socket_post_create(struct sock *sk, u16 family);
@@ -114,6 +116,11 @@ static inline int
selinux_netlbl_conn_setsid(struct sock *sk,
  return 0;
 }
 
+static inline int selinux_netlbl_sctp_assoc_request(struct
sctp_endpoint *ep,
+     struct sk_buff
*skb)
+{
+ return 0;
+}
 static inline int selinux_netlbl_inet_conn_request(struct
request_sock *req,
     u16 family)
 {
diff --git a/security/selinux/include/objsec.h
b/security/selinux/include/objsec.h
index 6ebc61e..660f270 100644
--- a/security/selinux/include/objsec.h
+++ b/security/selinux/include/objsec.h
@@ -130,6 +130,11 @@ struct sk_security_struct {
  u32 sid; /* SID of this object */
  u32 peer_sid; /* SID of peer */
  u16 sclass; /* sock security class */
+
+ enum { /* SCTP association
state */
+ SCTP_ASSOC_UNSET = 0,
+ SCTP_ASSOC_SET,
+ } sctp_assoc_state;
 };
 
 struct tun_security_struct {
diff --git a/security/selinux/netlabel.c
b/security/selinux/netlabel.c
index aaba667..7d5aa15 100644
--- a/security/selinux/netlabel.c
+++ b/security/selinux/netlabel.c
@@ -250,6 +250,7 @@ int selinux_netlbl_skbuff_setsid(struct sk_buff *skb,
  sk = skb_to_full_sk(skb);
  if (sk != NULL) {
  struct sk_security_struct *sksec = sk->sk_security;
+
  if (sksec->nlbl_state != NLBL_REQSKB)
  return 0;
  secattr = selinux_netlbl_sock_getattr(sk, sid);
@@ -271,6 +272,41 @@ int selinux_netlbl_skbuff_setsid(struct sk_buff *skb,
 }
 
 /**
+ * selinux_netlbl_sctp_assoc_request - Label an incoming sctp
association.
+ *
+ * Returns zero on success, negative values on failure.
+ *
+ */
+int selinux_netlbl_sctp_assoc_request(struct sctp_endpoint *ep,
+      struct sk_buff *skb)
+{
+ int rc;
+ struct netlbl_lsm_secattr secattr;
+ struct sk_security_struct *sksec = ep->base.sk->sk_security;
+
+ if (ep->base.sk->sk_family != PF_INET &&
+ ep->base.sk->sk_family != PF_INET6)
+ return 0;
+
+ netlbl_secattr_init(&secattr);
+ rc = security_netlbl_sid_to_secattr(ep->secid, &secattr);
+ if (rc != 0)
+ goto assoc_request_return;
+
+ rc = netlbl_sctp_setattr(ep->base.sk, skb, &secattr);
+ if (rc == 0)
+ sksec->nlbl_state = NLBL_LABELED;
+
+ netlbl_secattr_destroy(&secattr);
+ return rc;
+}
+
+/**
  * selinux_netlbl_inet_conn_request - Label an incoming stream
connection
  *
@@ -481,7 +517,7 @@ int selinux_netlbl_socket_setsockopt(struct socket *sock,
  */
 int selinux_netlbl_socket_connect(struct sock *sk, struct sockaddr
*addr)
 {
- int rc;
+ int rc, already_owned_by_user = 0;
  struct sk_security_struct *sksec = sk->sk_security;
  struct netlbl_lsm_secattr *secattr;
 
@@ -489,7 +525,16 @@ int selinux_netlbl_socket_connect(struct sock
*sk, struct sockaddr *addr)
      sksec->nlbl_state != NLBL_CONNLABELED)
  return 0;
 
- lock_sock(sk);
+ /* Note: When called via connect(2) this happens before the
socket
HOWEVER,
+  * when called by the SCTP protocol layer via
sctp_connectx(3),
check if
+  */
+ if (sock_owned_by_user(sk) && sksec->sclass ==
SECCLASS_SCTP_SOCKET)
+ already_owned_by_user = 1;
+ else
+ lock_sock(sk);
Conditional locking is generally considered harmful. I'd split the
cases for the different callers, and use a common helper for both.
Post by Richard Haines
 
  /* connected sockets are allowed to disconnect when the
address family
   * is set to AF_UNSPEC, if that is what is happening we want
to reset
@@ -510,6 +555,7 @@ int selinux_netlbl_socket_connect(struct sock
*sk, struct sockaddr *addr)
  sksec->nlbl_state = NLBL_CONNLABELED;
 
- release_sock(sk);
+ if (!already_owned_by_user)
+ release_sock(sk);
  return rc;
 }
Richard Haines
2017-10-24 15:50:14 UTC
Permalink
Post by Stephen Smalley
Post by Richard Haines
Documentation/security/SELinux-sctp.txt
---
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
diff --git a/Documentation/security/SELinux-sctp.txt
b/Documentation/security/SELinux-sctp.txt
new file mode 100644
index 0000000..32e0255
--- /dev/null
+++ b/Documentation/security/SELinux-sctp.txt
@@ -0,0 +1,108 @@
+ SCTP SELinux Support
+ ======================
+
+Security Hooks
+===============
+
+The Documentation/security/LSM-sctp.txt document describes how the following
+ security_sctp_assoc_request()
+ security_sctp_bind_connect()
+ security_sctp_sk_clone()
+
+ security_inet_conn_established()
+
+
+Policy Statements
+==================
+The following class and permissions to support SCTP are available within the
+ class sctp_socket inherits socket { node_bind }
+
+ policycap extended_socket_class;
+
+The SELinux SCTP support adds the additional permissions that are explained
+ association bindx connectx
+
+If userspace tools have been updated, SCTP will support the
portcon
+ portcon sctp 1024-1036 system_u:object_r:sctp_ports_t:s0
+
+
+SCTP Bind, Connect and ASCONF Chunk Parameter Permission Checks
+================================================================
+The hook security_sctp_bind_connect() is called by SCTP to check permissions
+
+ --------------------------------------------------------------
----
+ | BINDX Permission
Check |
contains |
+ |----------------------------|--------------------------------
---|
+ | SCTP_SOCKOPT_BINDX_ADD | One or more ipv4 / ipv6 addresses |
+ --------------------------------------------------------------
----
+
+ --------------------------------------------------------------
----
+ | BIND Permission
Checks |
contains |
+ |----------------------------|--------------------------------
---|
+ | SCTP_PRIMARY_ADDR | Single ipv4 or ipv6
address |
+ | SCTP_SET_PEER_PRIMARY_ADDR | Single ipv4 or ipv6
address |
+ --------------------------------------------------------------
----
+
+ --------------------------------------------------------------
----
+ | CONNECTX Permission
Check |
contains |
+ |----------------------------|--------------------------------
---|
+ | SCTP_SOCKOPT_CONNECTX | One or more ipv4 / ipv6 addresses |
+ --------------------------------------------------------------
----
+
+ --------------------------------------------------------------
----
+ | CONNECT Permission
Checks |
contains |
+ |----------------------------|--------------------------------
---|
+ | SCTP_SENDMSG_CONNECT | Single ipv4 or ipv6
address |
+ | SCTP_PARAM_ADD_IP | One or more ipv4 / ipv6 addresses |
+ | SCTP_PARAM_SET_PRIMARY | Single ipv4 or ipv6
address |
+ --------------------------------------------------------------
----
+
+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.
+
+ 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.
+
+ 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).
+
+ 5) The NetLabel SCTP peer labeling rules apply as discussed in the following
+ set of posts tagged "netlabel" at: http://www.paul-moore.com
/b
log/t.
+
+ 6) CIPSO is only supported for IPv4 addressing: socket(AF_INET, ...)
socket(AF_INET6, ...)
+
+ a) CIPSO will send an ICMP packet if an SCTP packet
cannot
be
+ delivered because of an invalid label.
+ b) CALIPSO does not send an ICMP packet, just silently discards it.
+
+ 7) IPSEC is not supported as rfc3554 - sctp/ipsec support has
not
been
+ implemented in userspace (racoon(8) or ipsec_pluto(8)), although the
+ kernel supports SCTP/IPSEC.
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 33fd061..c3e9600 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -67,6 +67,8 @@
#include <linux/tcp.h>
#include <linux/udp.h>
#include <linux/dccp.h>
+#include <linux/sctp.h>
+#include <net/sctp/structs.h>
#include <linux/quota.h>
#include <linux/un.h> /* for Unix socket types */
#include <net/af_unix.h> /* for Unix socket types */
@@ -4119,6 +4121,23 @@ static int selinux_parse_skb_ipv4(struct sk_buff *skb,
break;
}
+#if IS_ENABLED(CONFIG_IP_SCTP)
+ case IPPROTO_SCTP: {
+ struct sctphdr _sctph, *sh;
+
+ if (ntohs(ih->frag_off) & IP_OFFSET)
+ break;
+
+ offset += ihlen;
+ sh = skb_header_pointer(skb, offset,
sizeof(_sctph),
&_sctph);
+ if (sh == NULL)
+ break;
+
+ ad->u.net->sport = sh->source;
+ ad->u.net->dport = sh->dest;
+ break;
+ }
+#endif
break;
}
@@ -4192,6 +4211,19 @@ static int selinux_parse_skb_ipv6(struct sk_buff *skb,
break;
}
+#if IS_ENABLED(CONFIG_IP_SCTP)
+ case IPPROTO_SCTP: {
+ struct sctphdr _sctph, *sh;
+
+ sh = skb_header_pointer(skb, offset,
sizeof(_sctph),
&_sctph);
+ if (sh == NULL)
+ break;
+
+ ad->u.net->sport = sh->source;
+ ad->u.net->dport = sh->dest;
+ break;
+ }
+#endif
/* includes fragments */
break;
@@ -4381,6 +4413,10 @@ static int selinux_socket_post_create(struct
socket *sock, int family,
sksec = sock->sk->sk_security;
sksec->sclass = sclass;
sksec->sid = sid;
+ /* Allows detection of the first association on
this
socket */
+ if (sksec->sclass == SECCLASS_SCTP_SOCKET)
+ sksec->sctp_assoc_state =
SCTP_ASSOC_UNSET;
+
What prevents this from interleaving with
selinux_sctp_assoc_request()
accesses to sctp_assoc_state?
I've added a spinlock in selinux_sctp_assoc_request()
Post by Stephen Smalley
Post by Richard Haines
err = selinux_netlbl_socket_post_create(sock->sk,
family);
}
@@ -4401,11 +4437,7 @@ static int selinux_socket_bind(struct socket
*sock, struct sockaddr *address, in
if (err)
goto out;
- /*
- * If PF_INET or PF_INET6, check name_bind permission for
the port.
we just
- * check the first address now.
- */
+ /* If PF_INET or PF_INET6, check name_bind permission for
the port. */
family = sk->sk_family;
if (family == PF_INET || family == PF_INET6) {
char *addrp;
@@ -4417,7 +4449,13 @@ static int selinux_socket_bind(struct socket
*sock, struct sockaddr *address, in
unsigned short snum;
u32 sid, node_perm;
- if (family == PF_INET) {
+ /*
+ * sctp_bindx(3) calls via
selinux_sctp_bind_connect()
+ * that validates multiple binding addresses.
Because of this
+ * need to check address->sa_family as it is
possible to have
+ * sk->sk_family = PF_INET6 with addr->sa_family =
AF_INET.
+ */
+ if (family == PF_INET || address->sa_family ==
AF_INET) {
if (addrlen < sizeof(struct sockaddr_in))
{
err = -EINVAL;
goto out;
@@ -4471,6 +4509,10 @@ static int selinux_socket_bind(struct socket
*sock, struct sockaddr *address, in
node_perm = DCCP_SOCKET__NODE_BIND;
break;
+ node_perm = SCTP_SOCKET__NODE_BIND;
+ break;
+
node_perm = RAWIP_SOCKET__NODE_BIND;
break;
@@ -4485,7 +4527,7 @@ static int selinux_socket_bind(struct socket
*sock, struct sockaddr *address, in
ad.u.net->sport = htons(snum);
ad.u.net->family = family;
- if (family == PF_INET)
+ if (family == PF_INET || address->sa_family ==
AF_INET)
ad.u.net->v4info.saddr = addr4-
Post by Richard Haines
sin_addr.s_addr;
else
ad.u.net->v6info.saddr = addr6->sin6_addr;
@@ -4510,10 +4552,12 @@ static int selinux_socket_connect(struct
socket *sock, struct sockaddr *address,
return err;
/*
- * If a TCP or DCCP socket, check name_connect permission
for the port.
+ * If a TCP, DCCP or SCTP socket, check name_connect
permission
+ * for the port.
*/
if (sksec->sclass == SECCLASS_TCP_SOCKET ||
- sksec->sclass == SECCLASS_DCCP_SOCKET) {
+ sksec->sclass == SECCLASS_DCCP_SOCKET ||
+ sksec->sclass == SECCLASS_SCTP_SOCKET) {
struct common_audit_data ad;
struct lsm_network_audit net = {0,};
struct sockaddr_in *addr4 = NULL;
@@ -4521,7 +4565,14 @@ static int selinux_socket_connect(struct
socket *sock, struct sockaddr *address,
unsigned short snum;
u32 sid, perm;
- if (sk->sk_family == PF_INET) {
+ /* sctp_connectx(3) calls via
+ *selinux_sctp_bind_connect() that validates
multiple
+ * connect addresses. Because of this need to
check
+ * address->sa_family as it is possible to have
+ * sk->sk_family = PF_INET6 with addr->sa_family =
AF_INET.
+ */
+ if (sk->sk_family == PF_INET ||
+ address->sa_family ==
AF_INET) {
addr4 = (struct sockaddr_in *)address;
if (addrlen < sizeof(struct sockaddr_in))
return -EINVAL;
@@ -4534,11 +4585,21 @@ static int selinux_socket_connect(struct
socket *sock, struct sockaddr *address,
}
err = sel_netport_sid(sk->sk_protocol, snum,
&sid);
+
if (err)
goto out;
- perm = (sksec->sclass == SECCLASS_TCP_SOCKET) ?
DCCP_SOCKET__NAME_CONNECT;
+ switch (sksec->sclass) {
+ perm = TCP_SOCKET__NAME_CONNECT;
+ break;
+ perm = DCCP_SOCKET__NAME_CONNECT;
+ break;
+ perm = SCTP_SOCKET__NAME_CONNECT;
+ break;
+ }
ad.type = LSM_AUDIT_DATA_NET;
ad.u.net = &net;
@@ -4815,7 +4876,8 @@ static int
selinux_socket_getpeersec_stream(struct socket *sock, char __user *op
u32 peer_sid = SECSID_NULL;
if (sksec->sclass == SECCLASS_UNIX_STREAM_SOCKET ||
- sksec->sclass == SECCLASS_TCP_SOCKET)
+ sksec->sclass == SECCLASS_TCP_SOCKET ||
+ sksec->sclass == SECCLASS_SCTP_SOCKET)
peer_sid = sksec->peer_sid;
if (peer_sid == SECSID_NULL)
return -ENOPROTOOPT;
@@ -4928,6 +4990,183 @@ static void selinux_sock_graft(struct sock
*sk, struct socket *parent)
sksec->sclass = isec->sclass;
}
+/* 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-
Post by Richard Haines
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;
+
+ peerlbl_active = selinux_peerlbl_enabled();
+
+ if (peerlbl_active) {
+ /* This will return peer_sid = SECSID_NULL if
there
are
+ * no peer labels, see
security_net_peersid_resolve().
+ */
+ err = selinux_skb_peerlbl_sid(skb, ep->base.sk-
Post by Richard Haines
sk_family,
+ &peer_sid);
+
+ 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-
Post by Richard Haines
sclass,
+ SCTP_SOCKET__ASSOCIATION, &ad);
+ if (err)
+ return err;
+ }
+
+ if (sctp_cid == SCTP_CID_INIT) {
+ /* Have INIT when incoming connect(2),
sctp_connectx(3)
+ * or sctp_sendmsg(3) (with no association already
present),
+ * so compute the MLS component for the connection
and store
+ * the information in ep. This will be used by
SCTP
TCP type
+ * sockets and peeled off connections as they
cause
a new
+ * socket to be generated. selinux_sctp_sk_clone()
will then
+ * plug this into the new socket.
+ */
+ err = selinux_conn_sid(sksec->sid, peer_sid,
&conn_sid);
+ if (err)
+ return err;
+
+ ep->secid = conn_sid;
+ ep->peer_secid = peer_sid;
+
+ /* Set any NetLabel labels including CIPSO/CALIPSO
options. */
+ return selinux_netlbl_sctp_assoc_request(ep, skb);
+ }
+
+ return 0;
+}
+
+/*
+ * Check if sctp IPv4/IPv6 addresses are valid for binding or connecting
+ */
+static int selinux_sctp_bind_connect(struct sock *sk, int optname,
+ struct sockaddr *address,
+ int addrlen)
+{
+ int len, err = 0, walk_size = 0;
+ void *addr_buf;
+ struct sockaddr *addr;
+ struct socket *sock;
+
+ if (!selinux_policycap_extsockclass)
+ return 0;
+
+ switch (optname) {
+ err = sock_has_perm(sk, SCTP_SOCKET__BINDX);
+ break;
+ err = sock_has_perm(sk, SCTP_SOCKET__CONNECTX);
+ break;
+ /* These need SOCKET__BIND or SOCKET__CONNECT permissions
that will
+ * be checked later.
+ */
+ break;
+ err = -EINVAL;
+ }
+ if (err)
+ return err;
+
+ /* Process one or more addresses that may be IPv4 or IPv6
*/
+ sock = sk->sk_socket;
+ addr_buf = address;
+
+ while (walk_size < addrlen) {
+ addr = addr_buf;
+ switch (addr->sa_family) {
+ len = sizeof(struct sockaddr_in);
+ break;
+ len = sizeof(struct sockaddr_in6);
+ break;
+ return -EAFNOSUPPORT;
+ }
+
+ err = -EINVAL;
+ switch (optname) {
+ /* Bind checks */
+ err = selinux_socket_bind(sock, addr,
len);
+ break;
+ /* Connect checks */
+ err = selinux_socket_connect(sock, addr,
len);
+ break;
+ }
+
+ if (err)
+ return err;
+
+ addr_buf += len;
+ walk_size += len;
+ }
+ return 0;
+}
+
+/* Called whenever a new socket is created by accept(2) or
sctp_peeloff(3). */
+static void selinux_sctp_sk_clone(struct sctp_endpoint *ep, struct sock *sk,
+ struct sock *newsk)
+{
+ struct sk_security_struct *sksec = sk->sk_security;
+ struct sk_security_struct *newsksec = newsk->sk_security;
+
+ /* If policy does not support SECCLASS_SCTP_SOCKET then
call
+ * the non-sctp clone version.
+ */
+ if (!selinux_policycap_extsockclass)
+ return selinux_sk_clone_security(sk, newsk);
+
+ newsksec->sid = ep->secid;
+ newsksec->peer_sid = ep->peer_secid;
+ newsksec->sclass = sksec->sclass;
+ newsksec->nlbl_state = sksec->nlbl_state;
+}
+
static int selinux_inet_conn_request(struct sock *sk, struct
sk_buff
*skb,
struct request_sock *req)
{
@@ -6416,6 +6655,9 @@ static struct security_hook_list
selinux_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(sk_clone_security,
selinux_sk_clone_security),
LSM_HOOK_INIT(sk_getsecid, selinux_sk_getsecid),
LSM_HOOK_INIT(sock_graft, selinux_sock_graft),
+ LSM_HOOK_INIT(sctp_assoc_request,
selinux_sctp_assoc_request),
+ LSM_HOOK_INIT(sctp_sk_clone, selinux_sctp_sk_clone),
+ LSM_HOOK_INIT(sctp_bind_connect,
selinux_sctp_bind_connect),
LSM_HOOK_INIT(inet_conn_request,
selinux_inet_conn_request),
LSM_HOOK_INIT(inet_csk_clone, selinux_inet_csk_clone),
LSM_HOOK_INIT(inet_conn_established,
selinux_inet_conn_established),
diff --git a/security/selinux/include/classmap.h
b/security/selinux/include/classmap.h
index b9fe343..b4b10da 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -173,7 +173,8 @@ struct security_class_mapping secclass_map[] = {
{ COMMON_CAP2_PERMS, NULL } },
{ "sctp_socket",
{ COMMON_SOCK_PERMS,
- "node_bind", NULL } },
+ "node_bind", "name_connect", "association", "bindx",
+ "connectx", NULL } },
{ "icmp_socket",
{ COMMON_SOCK_PERMS,
"node_bind", NULL } },
diff --git a/security/selinux/include/netlabel.h
b/security/selinux/include/netlabel.h
index 75686d5..835a0d6 100644
--- a/security/selinux/include/netlabel.h
+++ b/security/selinux/include/netlabel.h
@@ -33,6 +33,7 @@
#include <linux/skbuff.h>
#include <net/sock.h>
#include <net/request_sock.h>
+#include <net/sctp/structs.h>
#include "avc.h"
#include "objsec.h"
@@ -53,7 +54,8 @@ int selinux_netlbl_skbuff_getsid(struct sk_buff *skb,
int selinux_netlbl_skbuff_setsid(struct sk_buff *skb,
u16 family,
u32 sid);
-
+int selinux_netlbl_sctp_assoc_request(struct sctp_endpoint *ep,
+ struct sk_buff *skb);
int selinux_netlbl_inet_conn_request(struct request_sock *req, u16 family);
void selinux_netlbl_inet_csk_clone(struct sock *sk, u16 family);
int selinux_netlbl_socket_post_create(struct sock *sk, u16
family);
@@ -114,6 +116,11 @@ static inline int
selinux_netlbl_conn_setsid(struct sock *sk,
return 0;
}
+static inline int selinux_netlbl_sctp_assoc_request(struct
sctp_endpoint *ep,
+ struct sk_buff
*skb)
+{
+ return 0;
+}
static inline int selinux_netlbl_inet_conn_request(struct
request_sock *req,
u16 family)
{
diff --git a/security/selinux/include/objsec.h
b/security/selinux/include/objsec.h
index 6ebc61e..660f270 100644
--- a/security/selinux/include/objsec.h
+++ b/security/selinux/include/objsec.h
@@ -130,6 +130,11 @@ struct sk_security_struct {
u32 sid; /* SID of this object */
u32 peer_sid; /* SID of peer */
u16 sclass; /* sock security class
*/
+
+ enum { /* SCTP association
state */
+ SCTP_ASSOC_UNSET = 0,
+ SCTP_ASSOC_SET,
+ } sctp_assoc_state;
};
struct tun_security_struct {
diff --git a/security/selinux/netlabel.c
b/security/selinux/netlabel.c
index aaba667..7d5aa15 100644
--- a/security/selinux/netlabel.c
+++ b/security/selinux/netlabel.c
@@ -250,6 +250,7 @@ int selinux_netlbl_skbuff_setsid(struct sk_buff *skb,
sk = skb_to_full_sk(skb);
if (sk != NULL) {
struct sk_security_struct *sksec = sk-
Post by Richard Haines
sk_security;
+
if (sksec->nlbl_state != NLBL_REQSKB)
return 0;
secattr = selinux_netlbl_sock_getattr(sk, sid);
@@ -271,6 +272,41 @@ int selinux_netlbl_skbuff_setsid(struct
sk_buff
*skb,
}
/**
+ * selinux_netlbl_sctp_assoc_request - Label an incoming sctp association.
+ *
+ * Returns zero on success, negative values on failure.
+ *
+ */
+int selinux_netlbl_sctp_assoc_request(struct sctp_endpoint *ep,
+ struct sk_buff *skb)
+{
+ int rc;
+ struct netlbl_lsm_secattr secattr;
+ struct sk_security_struct *sksec = ep->base.sk-
Post by Richard Haines
sk_security;
+
+ if (ep->base.sk->sk_family != PF_INET &&
+ ep->base.sk->sk_family !=
PF_INET6)
+ return 0;
+
+ netlbl_secattr_init(&secattr);
+ rc = security_netlbl_sid_to_secattr(ep->secid, &secattr);
+ if (rc != 0)
+ goto assoc_request_return;
+
+ rc = netlbl_sctp_setattr(ep->base.sk, skb, &secattr);
+ if (rc == 0)
+ sksec->nlbl_state = NLBL_LABELED;
+
+ netlbl_secattr_destroy(&secattr);
+ return rc;
+}
+
+/**
* selinux_netlbl_inet_conn_request - Label an incoming stream connection
*
@@ -481,7 +517,7 @@ int selinux_netlbl_socket_setsockopt(struct socket *sock,
*/
int selinux_netlbl_socket_connect(struct sock *sk, struct sockaddr *addr)
{
- int rc;
+ int rc, already_owned_by_user = 0;
struct sk_security_struct *sksec = sk->sk_security;
struct netlbl_lsm_secattr *secattr;
@@ -489,7 +525,16 @@ int selinux_netlbl_socket_connect(struct sock
*sk, struct sockaddr *addr)
sksec->nlbl_state != NLBL_CONNLABELED)
return 0;
- lock_sock(sk);
+ /* Note: When called via connect(2) this happens before
the
socket
HOWEVER,
+ * when called by the SCTP protocol layer via
sctp_connectx(3),
check if
+ */
+ if (sock_owned_by_user(sk) && sksec->sclass ==
SECCLASS_SCTP_SOCKET)
+ already_owned_by_user = 1;
+ else
+ lock_sock(sk);
Conditional locking is generally considered harmful. I'd split the
cases for the different callers, and use a common helper for both.
I've now split this as suggested.
Post by Stephen Smalley
Post by Richard Haines
/* connected sockets are allowed to disconnect when the
address family
* is set to AF_UNSPEC, if that is what is happening we
want
to reset
@@ -510,6 +555,7 @@ int selinux_netlbl_socket_connect(struct sock
*sk, struct sockaddr *addr)
sksec->nlbl_state = NLBL_CONNLABELED;
- release_sock(sk);
+ if (!already_owned_by_user)
+ release_sock(sk);
return rc;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-sctp"
in
More majordomo info at http://vger.kernel.org/majordomo-info.html
Marcelo Ricardo Leitner
2017-10-31 17:16:15 UTC
Permalink
Post by Richard Haines
Documentation/security/SELinux-sctp.txt
---
...
Post by Richard Haines
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 33fd061..c3e9600 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
...
Post by Richard Haines
@@ -4521,7 +4565,14 @@ static int selinux_socket_connect(struct socket *sock, struct sockaddr *address,
unsigned short snum;
u32 sid, perm;
- if (sk->sk_family == PF_INET) {
+ /* sctp_connectx(3) calls via
+ *selinux_sctp_bind_connect() that validates multiple
+ * connect addresses. Because of this need to check
+ * address->sa_family as it is possible to have
+ * sk->sk_family = PF_INET6 with addr->sa_family = AF_INET.
+ */
+ if (sk->sk_family == PF_INET ||
+ address->sa_family == AF_INET) {
Not sure which code style applies on this file but the if () above
looks odd. At least, checkpatch.pl complained about it.

Marcelo
Richard Haines
2017-11-01 21:34:09 UTC
Permalink
Post by Marcelo Ricardo Leitner
Post by Richard Haines
Documentation/security/SELinux-sctp.txt
---
...
Post by Richard Haines
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 33fd061..c3e9600 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
...
Post by Richard Haines
@@ -4521,7 +4565,14 @@ static int selinux_socket_connect(struct
socket *sock, struct sockaddr *address,
unsigned short snum;
u32 sid, perm;
- if (sk->sk_family == PF_INET) {
+ /* sctp_connectx(3) calls via
+ *selinux_sctp_bind_connect() that validates
multiple
+ * connect addresses. Because of this need to
check
+ * address->sa_family as it is possible to have
+ * sk->sk_family = PF_INET6 with addr->sa_family =
AF_INET.
+ */
+ if (sk->sk_family == PF_INET ||
+ address->sa_family ==
AF_INET) {
Not sure which code style applies on this file but the if () above
looks odd. At least, checkpatch.pl complained about it.
Changed to read:
if (sk->sk_family == PF_INET ||
address->sa_family == AF_INET) {
Post by Marcelo Ricardo Leitner
Marcelo
--
To unsubscribe from this list: send the line "unsubscribe linux-
security-module" in
More majordomo info at http://vger.kernel.org/majordomo-info.html
Paul Moore
2017-11-07 00:09:08 UTC
Permalink
On Tue, Oct 17, 2017 at 9:59 AM, Richard Haines
Post by Richard Haines
Documentation/security/SELinux-sctp.txt
---
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
diff --git a/Documentation/security/SELinux-sctp.txt b/Documentation/security/SELinux-sctp.txt
new file mode 100644
index 0000000..32e0255
--- /dev/null
+++ b/Documentation/security/SELinux-sctp.txt
@@ -0,0 +1,108 @@
See my previous comments about moving to reStructuredText for these docs.
Post by Richard Haines
+ SCTP SELinux Support
+ ======================
+
+Security Hooks
+===============
+
+The Documentation/security/LSM-sctp.txt document describes how the following
+ security_sctp_assoc_request()
+ security_sctp_bind_connect()
+ security_sctp_sk_clone()
+
+ security_inet_conn_established()
+
+
+Policy Statements
+==================
+The following class and permissions to support SCTP are available within the
+ class sctp_socket inherits socket { node_bind }
+
+ policycap extended_socket_class;
+
+The SELinux SCTP support adds the additional permissions that are explained
+ 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.

See my question on sctp_socket:association below.
Post by Richard Haines
+If userspace tools have been updated, SCTP will support the portcon
+ portcon sctp 1024-1036 system_u:object_r:sctp_ports_t:s0
+
+
+SCTP Bind, Connect and ASCONF Chunk Parameter Permission Checks
+================================================================
+The hook security_sctp_bind_connect() is called by SCTP to check permissions
+
+ ------------------------------------------------------------------
+ | BINDX Permission Check |
+ |----------------------------|-----------------------------------|
+ | SCTP_SOCKOPT_BINDX_ADD | One or more ipv4 / ipv6 addresses |
+ ------------------------------------------------------------------
+
+ ------------------------------------------------------------------
+ | BIND Permission Checks |
+ |----------------------------|-----------------------------------|
+ | SCTP_PRIMARY_ADDR | Single ipv4 or ipv6 address |
+ | SCTP_SET_PEER_PRIMARY_ADDR | Single ipv4 or ipv6 address |
+ ------------------------------------------------------------------
+
+ ------------------------------------------------------------------
+ | CONNECTX Permission Check |
+ |----------------------------|-----------------------------------|
+ | SCTP_SOCKOPT_CONNECTX | One or more ipv4 / ipv6 addresses |
+ ------------------------------------------------------------------
+
+ ------------------------------------------------------------------
+ | CONNECT Permission Checks |
+ |----------------------------|-----------------------------------|
+ | SCTP_SENDMSG_CONNECT | Single ipv4 or ipv6 address |
+ | SCTP_PARAM_ADD_IP | One or more ipv4 / ipv6 addresses |
+ | SCTP_PARAM_SET_PRIMARY | Single ipv4 or ipv6 address |
+ ------------------------------------------------------------------
+
+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.
+
+ 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?
Post by Richard Haines
+ 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?
Post by Richard Haines
+ 5) The NetLabel SCTP peer labeling rules apply as discussed in the following
+ set of posts tagged "netlabel" at: http://www.paul-moore.com/blog/t.
+
+ 6) CIPSO is only supported for IPv4 addressing: socket(AF_INET, ...)
+ CALIPSO is only supported for IPv6 addressing: socket(AF_INET6, ...)
+
+ a) CIPSO will send an ICMP packet if an SCTP packet cannot be
+ delivered because of an invalid label.
+ b) CALIPSO does not send an ICMP packet, just silently discards it.
Okay, this answers one of my questions from earlier in the patchset.
Post by Richard Haines
+ 7) IPSEC is not supported as rfc3554 - sctp/ipsec support has not been
+ implemented in userspace (racoon(8) or ipsec_pluto(8)), although the
+ kernel supports SCTP/IPSEC.
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 33fd061..c3e9600 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -67,6 +67,8 @@
#include <linux/tcp.h>
#include <linux/udp.h>
#include <linux/dccp.h>
+#include <linux/sctp.h>
+#include <net/sctp/structs.h>
#include <linux/quota.h>
#include <linux/un.h> /* for Unix socket types */
#include <net/af_unix.h> /* for Unix socket types */
@@ -4119,6 +4121,23 @@ static int selinux_parse_skb_ipv4(struct sk_buff *skb,
break;
}
+#if IS_ENABLED(CONFIG_IP_SCTP)
+ case IPPROTO_SCTP: {
+ struct sctphdr _sctph, *sh;
+
+ if (ntohs(ih->frag_off) & IP_OFFSET)
+ break;
+
+ offset += ihlen;
+ sh = skb_header_pointer(skb, offset, sizeof(_sctph), &_sctph);
+ if (sh == NULL)
+ break;
+
+ ad->u.net->sport = sh->source;
+ ad->u.net->dport = sh->dest;
+ break;
+ }
+#endif
break;
}
@@ -4192,6 +4211,19 @@ static int selinux_parse_skb_ipv6(struct sk_buff *skb,
break;
}
+#if IS_ENABLED(CONFIG_IP_SCTP)
+ case IPPROTO_SCTP: {
+ struct sctphdr _sctph, *sh;
+
+ sh = skb_header_pointer(skb, offset, sizeof(_sctph), &_sctph);
+ if (sh == NULL)
+ break;
+
+ ad->u.net->sport = sh->source;
+ ad->u.net->dport = sh->dest;
+ break;
+ }
+#endif
/* includes fragments */
break;
@@ -4381,6 +4413,10 @@ static int selinux_socket_post_create(struct socket *sock, int family,
sksec = sock->sk->sk_security;
sksec->sclass = sclass;
sksec->sid = sid;
+ /* Allows detection of the first association on this socket */
+ if (sksec->sclass == SECCLASS_SCTP_SOCKET)
+ sksec->sctp_assoc_state = SCTP_ASSOC_UNSET;
+
err = selinux_netlbl_socket_post_create(sock->sk, family);
}
@@ -4401,11 +4437,7 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in
if (err)
goto out;
- /*
- * If PF_INET or PF_INET6, check name_bind permission for the port.
- * Multiple address binding for SCTP is not supported yet: we just
- * check the first address now.
- */
+ /* If PF_INET or PF_INET6, check name_bind permission for the port. */
family = sk->sk_family;
Instead of checking both family and address->sa_family multiple times
in this function, let's just set the correct family value once and
store it in the family variable as we do today; for example:

family = sk->sk_family;
if (family == PF_INET6 && address->sa_family == PF_INET)
family = PF_INET;

... or I wonder if it is just safe to ignore the address family in the
socket, and simply use the value provided in address? Perhaps that is
the better option?
Post by Richard Haines
if (family == PF_INET || family == PF_INET6) {
char *addrp;
@@ -4417,7 +4449,13 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in
unsigned short snum;
u32 sid, node_perm;
- if (family == PF_INET) {
+ /*
+ * sctp_bindx(3) calls via selinux_sctp_bind_connect()
+ * that validates multiple binding addresses. Because of this
+ * need to check address->sa_family as it is possible to have
+ * sk->sk_family = PF_INET6 with addr->sa_family = AF_INET.
+ */
+ if (family == PF_INET || address->sa_family == AF_INET) {
if (addrlen < sizeof(struct sockaddr_in)) {
err = -EINVAL;
goto out;
@@ -4471,6 +4509,10 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in
node_perm = DCCP_SOCKET__NODE_BIND;
break;
+ node_perm = SCTP_SOCKET__NODE_BIND;
+ break;
+
node_perm = RAWIP_SOCKET__NODE_BIND;
break;
@@ -4485,7 +4527,7 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in
ad.u.net->sport = htons(snum);
ad.u.net->family = family;
- if (family == PF_INET)
+ if (family == PF_INET || address->sa_family == AF_INET)
ad.u.net->v4info.saddr = addr4->sin_addr.s_addr;
else
ad.u.net->v6info.saddr = addr6->sin6_addr;
@@ -4510,10 +4552,12 @@ static int selinux_socket_connect(struct socket *sock, struct sockaddr *address,
return err;
/*
- * If a TCP or DCCP socket, check name_connect permission for the port.
+ * If a TCP, DCCP or SCTP socket, check name_connect permission
+ * for the port.
*/
if (sksec->sclass == SECCLASS_TCP_SOCKET ||
- sksec->sclass == SECCLASS_DCCP_SOCKET) {
+ sksec->sclass == SECCLASS_DCCP_SOCKET ||
+ sksec->sclass == SECCLASS_SCTP_SOCKET) {
struct common_audit_data ad;
struct lsm_network_audit net = {0,};
struct sockaddr_in *addr4 = NULL;
@@ -4521,7 +4565,14 @@ static int selinux_socket_connect(struct socket *sock, struct sockaddr *address,
unsigned short snum;
u32 sid, perm;
- if (sk->sk_family == PF_INET) {
+ /* sctp_connectx(3) calls via
+ *selinux_sctp_bind_connect() that validates multiple
Typo. A space is needed after the "*" in the comment line above.
Post by Richard Haines
+ * connect addresses. Because of this need to check
+ * address->sa_family as it is possible to have
+ * sk->sk_family = PF_INET6 with addr->sa_family = AF_INET.
+ */
+ if (sk->sk_family == PF_INET ||
+ address->sa_family == AF_INET) {
Once again, I wonder if we just check sa_family and skip the sk_family?
Post by Richard Haines
addr4 = (struct sockaddr_in *)address;
if (addrlen < sizeof(struct sockaddr_in))
return -EINVAL;
@@ -4534,11 +4585,21 @@ static int selinux_socket_connect(struct socket *sock, struct sockaddr *address,
}
err = sel_netport_sid(sk->sk_protocol, snum, &sid);
+
No added vertical whitespace here please.
Post by Richard Haines
if (err)
goto out;
- perm = (sksec->sclass == SECCLASS_TCP_SOCKET) ?
- TCP_SOCKET__NAME_CONNECT : DCCP_SOCKET__NAME_CONNECT;
+ switch (sksec->sclass) {
+ perm = TCP_SOCKET__NAME_CONNECT;
+ break;
+ perm = DCCP_SOCKET__NAME_CONNECT;
+ break;
+ perm = SCTP_SOCKET__NAME_CONNECT;
+ break;
+ }
ad.type = LSM_AUDIT_DATA_NET;
ad.u.net = &net;
@@ -4815,7 +4876,8 @@ static int selinux_socket_getpeersec_stream(struct socket *sock, char __user *op
u32 peer_sid = SECSID_NULL;
if (sksec->sclass == SECCLASS_UNIX_STREAM_SOCKET ||
- sksec->sclass == SECCLASS_TCP_SOCKET)
+ sksec->sclass == SECCLASS_TCP_SOCKET ||
+ sksec->sclass == SECCLASS_SCTP_SOCKET)
peer_sid = sksec->peer_sid;
if (peer_sid == SECSID_NULL)
return -ENOPROTOOPT;
@@ -4928,6 +4990,183 @@ static void selinux_sock_graft(struct sock *sk, struct socket *parent)
sksec->sclass = isec->sclass;
}
+/* 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->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.
Post by Richard Haines
+ peerlbl_active = selinux_peerlbl_enabled();
+
+ if (peerlbl_active) {
+ /* This will return peer_sid = SECSID_NULL if there are
+ * no peer labels, see security_net_peersid_resolve().
+ */
+ err = selinux_skb_peerlbl_sid(skb, ep->base.sk->sk_family,
+ &peer_sid);
+
Drop the extra vertical whitespace between the function call and the
return code check please.
Post by Richard Haines
+ 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 ...
Post by Richard Haines
+ if (err)
+ return err;
+ }
+
+ if (sctp_cid == SCTP_CID_INIT) {
+ /* Have INIT when incoming connect(2), sctp_connectx(3)
+ * or sctp_sendmsg(3) (with no association already present),
+ * so compute the MLS component for the connection and store
+ * the information in ep. This will be used by SCTP TCP type
+ * sockets and peeled off connections as they cause a new
+ * socket to be generated. selinux_sctp_sk_clone() will then
+ * plug this into the new socket.
+ */
+ err = selinux_conn_sid(sksec->sid, peer_sid, &conn_sid);
+ if (err)
+ return err;
+
+ ep->secid = conn_sid;
+ ep->peer_secid = peer_sid;
+
+ /* Set any NetLabel labels including CIPSO/CALIPSO options. */
+ return selinux_netlbl_sctp_assoc_request(ep, skb);
+ }
+
+ return 0;
+}
+
+/*
+ * Check if sctp IPv4/IPv6 addresses are valid for binding or connecting
+ */
+static int selinux_sctp_bind_connect(struct sock *sk, int optname,
+ struct sockaddr *address,
+ int addrlen)
+{
+ int len, err = 0, walk_size = 0;
+ void *addr_buf;
+ struct sockaddr *addr;
+ struct socket *sock;
+
+ if (!selinux_policycap_extsockclass)
+ return 0;
+
+ switch (optname) {
+ err = sock_has_perm(sk, SCTP_SOCKET__BINDX);
+ break;
+ err = sock_has_perm(sk, SCTP_SOCKET__CONNECTX);
+ break;
+ /* These need SOCKET__BIND or SOCKET__CONNECT permissions that will
+ * be checked later.
+ */
+ break;
+ err = -EINVAL;
+ }
+ if (err)
+ return err;
+
+ /* Process one or more addresses that may be IPv4 or IPv6 */
+ sock = sk->sk_socket;
+ addr_buf = address;
+
+ while (walk_size < addrlen) {
+ addr = addr_buf;
+ switch (addr->sa_family) {
+ len = sizeof(struct sockaddr_in);
+ break;
+ len = sizeof(struct sockaddr_in6);
+ break;
+ return -EAFNOSUPPORT;
+ }
+
+ err = -EINVAL;
+ switch (optname) {
+ /* Bind checks */
+ err = selinux_socket_bind(sock, addr, len);
+ break;
+ /* Connect checks */
+ err = selinux_socket_connect(sock, addr, len);
+ break;
+ }
+
+ if (err)
+ return err;
+
+ addr_buf += len;
+ walk_size += len;
+ }
+ return 0;
+}
+
+/* Called whenever a new socket is created by accept(2) or sctp_peeloff(3). */
+static void selinux_sctp_sk_clone(struct sctp_endpoint *ep, struct sock *sk,
+ struct sock *newsk)
+{
+ struct sk_security_struct *sksec = sk->sk_security;
+ struct sk_security_struct *newsksec = newsk->sk_security;
+
+ /* If policy does not support SECCLASS_SCTP_SOCKET then call
+ * the non-sctp clone version.
+ */
+ if (!selinux_policycap_extsockclass)
+ return selinux_sk_clone_security(sk, newsk);
+
+ newsksec->sid = ep->secid;
+ newsksec->peer_sid = ep->peer_secid;
+ newsksec->sclass = sksec->sclass;
+ newsksec->nlbl_state = sksec->nlbl_state;
+}
+
static int selinux_inet_conn_request(struct sock *sk, struct sk_buff *skb,
struct request_sock *req)
{
@@ -6416,6 +6655,9 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(sk_clone_security, selinux_sk_clone_security),
LSM_HOOK_INIT(sk_getsecid, selinux_sk_getsecid),
LSM_HOOK_INIT(sock_graft, selinux_sock_graft),
+ LSM_HOOK_INIT(sctp_assoc_request, selinux_sctp_assoc_request),
+ LSM_HOOK_INIT(sctp_sk_clone, selinux_sctp_sk_clone),
+ LSM_HOOK_INIT(sctp_bind_connect, selinux_sctp_bind_connect),
LSM_HOOK_INIT(inet_conn_request, selinux_inet_conn_request),
LSM_HOOK_INIT(inet_csk_clone, selinux_inet_csk_clone),
LSM_HOOK_INIT(inet_conn_established, selinux_inet_conn_established),
diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
index b9fe343..b4b10da 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -173,7 +173,8 @@ struct security_class_mapping secclass_map[] = {
{ COMMON_CAP2_PERMS, NULL } },
{ "sctp_socket",
{ COMMON_SOCK_PERMS,
- "node_bind", NULL } },
+ "node_bind", "name_connect", "association", "bindx",
+ "connectx", NULL } },
{ "icmp_socket",
{ COMMON_SOCK_PERMS,
"node_bind", NULL } },
diff --git a/security/selinux/include/netlabel.h b/security/selinux/include/netlabel.h
index 75686d5..835a0d6 100644
--- a/security/selinux/include/netlabel.h
+++ b/security/selinux/include/netlabel.h
@@ -33,6 +33,7 @@
#include <linux/skbuff.h>
#include <net/sock.h>
#include <net/request_sock.h>
+#include <net/sctp/structs.h>
#include "avc.h"
#include "objsec.h"
@@ -53,7 +54,8 @@ int selinux_netlbl_skbuff_getsid(struct sk_buff *skb,
int selinux_netlbl_skbuff_setsid(struct sk_buff *skb,
u16 family,
u32 sid);
-
+int selinux_netlbl_sctp_assoc_request(struct sctp_endpoint *ep,
+ struct sk_buff *skb);
int selinux_netlbl_inet_conn_request(struct request_sock *req, u16 family);
void selinux_netlbl_inet_csk_clone(struct sock *sk, u16 family);
int selinux_netlbl_socket_post_create(struct sock *sk, u16 family);
@@ -114,6 +116,11 @@ static inline int selinux_netlbl_conn_setsid(struct sock *sk,
return 0;
}
+static inline int selinux_netlbl_sctp_assoc_request(struct sctp_endpoint *ep,
+ struct sk_buff *skb)
+{
+ return 0;
+}
static inline int selinux_netlbl_inet_conn_request(struct request_sock *req,
u16 family)
{
diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
index 6ebc61e..660f270 100644
--- a/security/selinux/include/objsec.h
+++ b/security/selinux/include/objsec.h
@@ -130,6 +130,11 @@ struct sk_security_struct {
u32 sid; /* SID of this object */
u32 peer_sid; /* SID of peer */
u16 sclass; /* sock security class */
+
Extra vertical whitespace is not needed.
Post by Richard Haines
+ enum { /* SCTP association state */
+ SCTP_ASSOC_UNSET = 0,
+ SCTP_ASSOC_SET,
+ } sctp_assoc_state;
};
struct tun_security_struct {
diff --git a/security/selinux/netlabel.c b/security/selinux/netlabel.c
index aaba667..7d5aa15 100644
--- a/security/selinux/netlabel.c
+++ b/security/selinux/netlabel.c
@@ -250,6 +250,7 @@ int selinux_netlbl_skbuff_setsid(struct sk_buff *skb,
sk = skb_to_full_sk(skb);
if (sk != NULL) {
struct sk_security_struct *sksec = sk->sk_security;
+
if (sksec->nlbl_state != NLBL_REQSKB)
return 0;
secattr = selinux_netlbl_sock_getattr(sk, sid);
@@ -271,6 +272,41 @@ int selinux_netlbl_skbuff_setsid(struct sk_buff *skb,
}
/**
+ * selinux_netlbl_sctp_assoc_request - Label an incoming sctp association.
+ *
+ * Returns zero on success, negative values on failure.
+ *
+ */
+int selinux_netlbl_sctp_assoc_request(struct sctp_endpoint *ep,
+ struct sk_buff *skb)
+{
+ int rc;
+ struct netlbl_lsm_secattr secattr;
+ struct sk_security_struct *sksec = ep->base.sk->sk_security;
+
+ if (ep->base.sk->sk_family != PF_INET &&
+ ep->base.sk->sk_family != PF_INET6)
+ return 0;
+
+ netlbl_secattr_init(&secattr);
+ rc = security_netlbl_sid_to_secattr(ep->secid, &secattr);
+ if (rc != 0)
+ goto assoc_request_return;
+
+ rc = netlbl_sctp_setattr(ep->base.sk, skb, &secattr);
+ if (rc == 0)
+ sksec->nlbl_state = NLBL_LABELED;
+
+ netlbl_secattr_destroy(&secattr);
+ return rc;
+}
+
+/**
* selinux_netlbl_inet_conn_request - Label an incoming stream connection
*
@@ -481,7 +517,7 @@ int selinux_netlbl_socket_setsockopt(struct socket *sock,
*/
int selinux_netlbl_socket_connect(struct sock *sk, struct sockaddr *addr)
{
- int rc;
+ int rc, already_owned_by_user = 0;
struct sk_security_struct *sksec = sk->sk_security;
struct netlbl_lsm_secattr *secattr;
@@ -489,7 +525,16 @@ int selinux_netlbl_socket_connect(struct sock *sk, struct sockaddr *addr)
sksec->nlbl_state != NLBL_CONNLABELED)
return 0;
- lock_sock(sk);
+ /* Note: When called via connect(2) this happens before the socket
+ * when called by the SCTP protocol layer via sctp_connectx(3),
+ */
+ if (sock_owned_by_user(sk) && sksec->sclass == SECCLASS_SCTP_SOCKET)
+ already_owned_by_user = 1;
+ else
+ lock_sock(sk);
/* connected sockets are allowed to disconnect when the address family
* is set to AF_UNSPEC, if that is what is happening we want to reset
@@ -510,6 +555,7 @@ int selinux_netlbl_socket_connect(struct sock *sk, struct sockaddr *addr)
sksec->nlbl_state = NLBL_CONNLABELED;
- release_sock(sk);
+ if (!already_owned_by_user)
+ release_sock(sk);
return rc;
}
--
2.13.6
--
paul moore
www.paul-moore.com
Richard Haines
2017-11-13 22:05:21 UTC
Permalink
Post by Paul Moore
On Tue, Oct 17, 2017 at 9:59 AM, Richard Haines
Post by Richard Haines
Documentation/security/SELinux-sctp.txt
---
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
diff --git a/Documentation/security/SELinux-sctp.txt
b/Documentation/security/SELinux-sctp.txt
new file mode 100644
index 0000000..32e0255
--- /dev/null
+++ b/Documentation/security/SELinux-sctp.txt
@@ -0,0 +1,108 @@
See my previous comments about moving to reStructuredText for these docs.
Done
Post by Paul Moore
Post by Richard Haines
+ SCTP SELinux Support
+ ======================
+
+Security Hooks
+===============
+
+The Documentation/security/LSM-sctp.txt document describes how the following
+ security_sctp_assoc_request()
+ security_sctp_bind_connect()
+ security_sctp_sk_clone()
+
+ security_inet_conn_established()
+
+
+Policy Statements
+==================
+The following class and permissions to support SCTP are available within the
+ class sctp_socket inherits socket { node_bind }
+
+ policycap extended_socket_class;
+
+The SELinux SCTP support adds the additional permissions that are explained
+ 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.
Post by Paul Moore
See my question on sctp_socket:association below.
Post by Richard Haines
+If userspace tools have been updated, SCTP will support the
portcon
+ portcon sctp 1024-1036 system_u:object_r:sctp_ports_t:s0
+
+
+SCTP Bind, Connect and ASCONF Chunk Parameter Permission Checks
+================================================================
+The hook security_sctp_bind_connect() is called by SCTP to check permissions
+
+ --------------------------------------------------------------
----
+ | BINDX Permission
Check |
contains |
+ |----------------------------|--------------------------------
---|
+ | SCTP_SOCKOPT_BINDX_ADD | One or more ipv4 / ipv6 addresses |
+ --------------------------------------------------------------
----
+
+ --------------------------------------------------------------
----
+ | BIND Permission
Checks |
contains |
+ |----------------------------|--------------------------------
---|
+ | SCTP_PRIMARY_ADDR | Single ipv4 or ipv6
address |
+ | SCTP_SET_PEER_PRIMARY_ADDR | Single ipv4 or ipv6
address |
+ --------------------------------------------------------------
----
+
+ --------------------------------------------------------------
----
+ | CONNECTX Permission
Check |
contains |
+ |----------------------------|--------------------------------
---|
+ | SCTP_SOCKOPT_CONNECTX | One or more ipv4 / ipv6 addresses |
+ --------------------------------------------------------------
----
+
+ --------------------------------------------------------------
----
+ | CONNECT Permission
Checks |
contains |
+ |----------------------------|--------------------------------
---|
+ | SCTP_SENDMSG_CONNECT | Single ipv4 or ipv6
address |
+ | SCTP_PARAM_ADD_IP | One or more ipv4 / ipv6 addresses |
+ | SCTP_PARAM_SET_PRIMARY | Single ipv4 or ipv6
address |
+ --------------------------------------------------------------
----
+
+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.
+
+ 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.
Post by Paul Moore
Post by Richard Haines
+ 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 .....
Post by Paul Moore
Post by Richard Haines
+ 5) The NetLabel SCTP peer labeling rules apply as discussed in the following
+ set of posts tagged "netlabel" at: http://www.paul-moore.com
/blog/t.
+
+ 6) CIPSO is only supported for IPv4 addressing: socket(AF_INET, ...)
socket(AF_INET6, ...)
+
+ a) CIPSO will send an ICMP packet if an SCTP packet cannot be
+ delivered because of an invalid label.
+ b) CALIPSO does not send an ICMP packet, just silently discards it.
Okay, this answers one of my questions from earlier in the patchset.
Post by Richard Haines
+ 7) IPSEC is not supported as rfc3554 - sctp/ipsec support has not been
+ implemented in userspace (racoon(8) or ipsec_pluto(8)), although the
+ kernel supports SCTP/IPSEC.
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 33fd061..c3e9600 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -67,6 +67,8 @@
#include <linux/tcp.h>
#include <linux/udp.h>
#include <linux/dccp.h>
+#include <linux/sctp.h>
+#include <net/sctp/structs.h>
#include <linux/quota.h>
#include <linux/un.h> /* for Unix socket types */
#include <net/af_unix.h> /* for Unix socket types */
@@ -4119,6 +4121,23 @@ static int selinux_parse_skb_ipv4(struct sk_buff *skb,
break;
}
+#if IS_ENABLED(CONFIG_IP_SCTP)
+ case IPPROTO_SCTP: {
+ struct sctphdr _sctph, *sh;
+
+ if (ntohs(ih->frag_off) & IP_OFFSET)
+ break;
+
+ offset += ihlen;
+ sh = skb_header_pointer(skb, offset,
sizeof(_sctph), &_sctph);
+ if (sh == NULL)
+ break;
+
+ ad->u.net->sport = sh->source;
+ ad->u.net->dport = sh->dest;
+ break;
+ }
+#endif
break;
}
@@ -4192,6 +4211,19 @@ static int selinux_parse_skb_ipv6(struct sk_buff *skb,
break;
}
+#if IS_ENABLED(CONFIG_IP_SCTP)
+ case IPPROTO_SCTP: {
+ struct sctphdr _sctph, *sh;
+
+ sh = skb_header_pointer(skb, offset,
sizeof(_sctph), &_sctph);
+ if (sh == NULL)
+ break;
+
+ ad->u.net->sport = sh->source;
+ ad->u.net->dport = sh->dest;
+ break;
+ }
+#endif
/* includes fragments */
break;
@@ -4381,6 +4413,10 @@ static int selinux_socket_post_create(struct
socket *sock, int family,
sksec = sock->sk->sk_security;
sksec->sclass = sclass;
sksec->sid = sid;
+ /* Allows detection of the first association on this socket */
+ if (sksec->sclass == SECCLASS_SCTP_SOCKET)
+ sksec->sctp_assoc_state = SCTP_ASSOC_UNSET;
+
err = selinux_netlbl_socket_post_create(sock->sk, family);
}
@@ -4401,11 +4437,7 @@ static int selinux_socket_bind(struct socket
*sock, struct sockaddr *address, in
if (err)
goto out;
- /*
- * If PF_INET or PF_INET6, check name_bind permission for the port.
- * Multiple address binding for SCTP is not supported yet: we just
- * check the first address now.
- */
+ /* If PF_INET or PF_INET6, check name_bind permission for the port. */
family = sk->sk_family;
Instead of checking both family and address->sa_family multiple times
in this function, let's just set the correct family value once and
family = sk->sk_family;
if (family == PF_INET6 && address->sa_family == PF_INET)
family = PF_INET;
... or I wonder if it is just safe to ignore the address family in the
socket, and simply use the value provided in address? Perhaps that is
the better option?
Done - Implemented the "ignore the socket address family" option
Post by Paul Moore
Post by Richard Haines
if (family == PF_INET || family == PF_INET6) {
char *addrp;
@@ -4417,7 +4449,13 @@ static int selinux_socket_bind(struct socket
*sock, struct sockaddr *address, in
unsigned short snum;
u32 sid, node_perm;
- if (family == PF_INET) {
+ /*
+ * sctp_bindx(3) calls via
selinux_sctp_bind_connect()
+ * that validates multiple binding addresses. Because of this
+ * need to check address->sa_family as it is
possible to have
+ * sk->sk_family = PF_INET6 with addr->sa_family = AF_INET.
+ */
+ if (family == PF_INET || address->sa_family == AF_INET) {
if (addrlen < sizeof(struct sockaddr_in)) {
err = -EINVAL;
goto out;
@@ -4471,6 +4509,10 @@ static int selinux_socket_bind(struct socket
*sock, struct sockaddr *address, in
node_perm = DCCP_SOCKET__NODE_BIND;
break;
+ node_perm = SCTP_SOCKET__NODE_BIND;
+ break;
+
node_perm = RAWIP_SOCKET__NODE_BIND;
break;
@@ -4485,7 +4527,7 @@ static int selinux_socket_bind(struct socket
*sock, struct sockaddr *address, in
ad.u.net->sport = htons(snum);
ad.u.net->family = family;
- if (family == PF_INET)
+ if (family == PF_INET || address->sa_family == AF_INET)
ad.u.net->v4info.saddr = addr4-
Post by Richard Haines
sin_addr.s_addr;
else
ad.u.net->v6info.saddr = addr6->sin6_addr;
@@ -4510,10 +4552,12 @@ static int selinux_socket_connect(struct
socket *sock, struct sockaddr *address,
return err;
/*
- * If a TCP or DCCP socket, check name_connect permission for the port.
+ * If a TCP, DCCP or SCTP socket, check name_connect
permission
+ * for the port.
*/
if (sksec->sclass == SECCLASS_TCP_SOCKET ||
- sksec->sclass == SECCLASS_DCCP_SOCKET) {
+ sksec->sclass == SECCLASS_DCCP_SOCKET ||
+ sksec->sclass == SECCLASS_SCTP_SOCKET) {
struct common_audit_data ad;
struct lsm_network_audit net = {0,};
struct sockaddr_in *addr4 = NULL;
@@ -4521,7 +4565,14 @@ static int selinux_socket_connect(struct
socket *sock, struct sockaddr *address,
unsigned short snum;
u32 sid, perm;
- if (sk->sk_family == PF_INET) {
+ /* sctp_connectx(3) calls via
+ *selinux_sctp_bind_connect() that validates
multiple
Typo. A space is needed after the "*" in the comment line above.
Done
Post by Paul Moore
Post by Richard Haines
+ * connect addresses. Because of this need to check
+ * address->sa_family as it is possible to have
+ * sk->sk_family = PF_INET6 with addr->sa_family = AF_INET.
+ */
+ if (sk->sk_family == PF_INET ||
+ address->sa_family == AF_INET) {
Once again, I wonder if we just check sa_family and skip the
sk_family?
Done
Post by Paul Moore
Post by Richard Haines
addr4 = (struct sockaddr_in *)address;
if (addrlen < sizeof(struct sockaddr_in))
return -EINVAL;
@@ -4534,11 +4585,21 @@ static int selinux_socket_connect(struct
socket *sock, struct sockaddr *address,
}
err = sel_netport_sid(sk->sk_protocol, snum, &sid);
+
No added vertical whitespace here please.
Done
Post by Paul Moore
Post by Richard Haines
if (err)
goto out;
- perm = (sksec->sclass == SECCLASS_TCP_SOCKET) ?
DCCP_SOCKET__NAME_CONNECT;
+ switch (sksec->sclass) {
+ perm = TCP_SOCKET__NAME_CONNECT;
+ break;
+ perm = DCCP_SOCKET__NAME_CONNECT;
+ break;
+ perm = SCTP_SOCKET__NAME_CONNECT;
+ break;
+ }
ad.type = LSM_AUDIT_DATA_NET;
ad.u.net = &net;
@@ -4815,7 +4876,8 @@ static int
selinux_socket_getpeersec_stream(struct socket *sock, char __user
*op
u32 peer_sid = SECSID_NULL;
if (sksec->sclass == SECCLASS_UNIX_STREAM_SOCKET ||
- sksec->sclass == SECCLASS_TCP_SOCKET)
+ sksec->sclass == SECCLASS_TCP_SOCKET ||
+ sksec->sclass == SECCLASS_SCTP_SOCKET)
peer_sid = sksec->peer_sid;
if (peer_sid == SECSID_NULL)
return -ENOPROTOOPT;
@@ -4928,6 +4990,183 @@ static void selinux_sock_graft(struct sock
*sk, struct socket *parent)
sksec->sclass = isec->sclass;
}
+/* 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-
Post by Richard Haines
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).
Post by Paul Moore
Post by Richard Haines
+ peerlbl_active = selinux_peerlbl_enabled();
+
+ if (peerlbl_active) {
+ /* This will return peer_sid = SECSID_NULL if there are
+ * no peer labels, see
security_net_peersid_resolve().
+ */
+ err = selinux_skb_peerlbl_sid(skb, ep->base.sk-
Post by Richard Haines
sk_family,
+ &peer_sid);
+
Drop the extra vertical whitespace between the function call and the
return code check please.
Done
Post by Paul Moore
Post by Richard Haines
+ 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. Maybe
the reworded bullet 2) in SELinux-sctp.txt clarifies this:

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.
Post by Paul Moore
Post by Richard Haines
+ if (err)
+ return err;
+ }
+
+ if (sctp_cid == SCTP_CID_INIT) {
+ /* Have INIT when incoming connect(2),
sctp_connectx(3)
+ * or sctp_sendmsg(3) (with no association already present),
+ * so compute the MLS component for the connection and store
+ * the information in ep. This will be used by SCTP TCP type
+ * sockets and peeled off connections as they cause a new
+ * socket to be generated. selinux_sctp_sk_clone() will then
+ * plug this into the new socket.
+ */
+ err = selinux_conn_sid(sksec->sid, peer_sid,
&conn_sid);
+ if (err)
+ return err;
+
+ ep->secid = conn_sid;
+ ep->peer_secid = peer_sid;
+
+ /* Set any NetLabel labels including CIPSO/CALIPSO options. */
+ return selinux_netlbl_sctp_assoc_request(ep, skb);
+ }
+
+ return 0;
+}
+
+/*
+ * Check if sctp IPv4/IPv6 addresses are valid for binding or connecting
+ */
+static int selinux_sctp_bind_connect(struct sock *sk, int optname,
+ struct sockaddr *address,
+ int addrlen)
+{
+ int len, err = 0, walk_size = 0;
+ void *addr_buf;
+ struct sockaddr *addr;
+ struct socket *sock;
+
+ if (!selinux_policycap_extsockclass)
+ return 0;
+
+ switch (optname) {
+ err = sock_has_perm(sk, SCTP_SOCKET__BINDX);
+ break;
+ err = sock_has_perm(sk, SCTP_SOCKET__CONNECTX);
+ break;
+ /* These need SOCKET__BIND or SOCKET__CONNECT permissions that will
+ * be checked later.
+ */
+ break;
+ err = -EINVAL;
+ }
+ if (err)
+ return err;
+
+ /* Process one or more addresses that may be IPv4 or IPv6 */
+ sock = sk->sk_socket;
+ addr_buf = address;
+
+ while (walk_size < addrlen) {
+ addr = addr_buf;
+ switch (addr->sa_family) {
+ len = sizeof(struct sockaddr_in);
+ break;
+ len = sizeof(struct sockaddr_in6);
+ break;
+ return -EAFNOSUPPORT;
+ }
+
+ err = -EINVAL;
+ switch (optname) {
+ /* Bind checks */
+ err = selinux_socket_bind(sock, addr, len);
+ break;
+ /* Connect checks */
+ err = selinux_socket_connect(sock, addr, len);
+ break;
+ }
+
+ if (err)
+ return err;
+
+ addr_buf += len;
+ walk_size += len;
+ }
+ return 0;
+}
+
+/* Called whenever a new socket is created by accept(2) or
sctp_peeloff(3). */
+static void selinux_sctp_sk_clone(struct sctp_endpoint *ep, struct sock *sk,
+ struct sock *newsk)
+{
+ struct sk_security_struct *sksec = sk->sk_security;
+ struct sk_security_struct *newsksec = newsk->sk_security;
+
+ /* If policy does not support SECCLASS_SCTP_SOCKET then call
+ * the non-sctp clone version.
+ */
+ if (!selinux_policycap_extsockclass)
+ return selinux_sk_clone_security(sk, newsk);
+
+ newsksec->sid = ep->secid;
+ newsksec->peer_sid = ep->peer_secid;
+ newsksec->sclass = sksec->sclass;
+ newsksec->nlbl_state = sksec->nlbl_state;
+}
+
static int selinux_inet_conn_request(struct sock *sk, struct sk_buff *skb,
struct request_sock *req)
{
@@ -6416,6 +6655,9 @@ static struct security_hook_list
selinux_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(sk_clone_security,
selinux_sk_clone_security),
LSM_HOOK_INIT(sk_getsecid, selinux_sk_getsecid),
LSM_HOOK_INIT(sock_graft, selinux_sock_graft),
+ LSM_HOOK_INIT(sctp_assoc_request,
selinux_sctp_assoc_request),
+ LSM_HOOK_INIT(sctp_sk_clone, selinux_sctp_sk_clone),
+ LSM_HOOK_INIT(sctp_bind_connect,
selinux_sctp_bind_connect),
LSM_HOOK_INIT(inet_conn_request,
selinux_inet_conn_request),
LSM_HOOK_INIT(inet_csk_clone, selinux_inet_csk_clone),
LSM_HOOK_INIT(inet_conn_established,
selinux_inet_conn_established),
diff --git a/security/selinux/include/classmap.h
b/security/selinux/include/classmap.h
index b9fe343..b4b10da 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -173,7 +173,8 @@ struct security_class_mapping secclass_map[] = {
{ COMMON_CAP2_PERMS, NULL } },
{ "sctp_socket",
{ COMMON_SOCK_PERMS,
- "node_bind", NULL } },
+ "node_bind", "name_connect", "association", "bindx",
+ "connectx", NULL } },
{ "icmp_socket",
{ COMMON_SOCK_PERMS,
"node_bind", NULL } },
diff --git a/security/selinux/include/netlabel.h
b/security/selinux/include/netlabel.h
index 75686d5..835a0d6 100644
--- a/security/selinux/include/netlabel.h
+++ b/security/selinux/include/netlabel.h
@@ -33,6 +33,7 @@
#include <linux/skbuff.h>
#include <net/sock.h>
#include <net/request_sock.h>
+#include <net/sctp/structs.h>
#include "avc.h"
#include "objsec.h"
@@ -53,7 +54,8 @@ int selinux_netlbl_skbuff_getsid(struct sk_buff *skb,
int selinux_netlbl_skbuff_setsid(struct sk_buff *skb,
u16 family,
u32 sid);
-
+int selinux_netlbl_sctp_assoc_request(struct sctp_endpoint *ep,
+ struct sk_buff *skb);
int selinux_netlbl_inet_conn_request(struct request_sock *req, u16 family);
void selinux_netlbl_inet_csk_clone(struct sock *sk, u16 family);
int selinux_netlbl_socket_post_create(struct sock *sk, u16
family);
@@ -114,6 +116,11 @@ static inline int
selinux_netlbl_conn_setsid(struct sock *sk,
return 0;
}
+static inline int selinux_netlbl_sctp_assoc_request(struct
sctp_endpoint *ep,
+ struct sk_buff *skb)
+{
+ return 0;
+}
static inline int selinux_netlbl_inet_conn_request(struct
request_sock *req,
u16 family)
{
diff --git a/security/selinux/include/objsec.h
b/security/selinux/include/objsec.h
index 6ebc61e..660f270 100644
--- a/security/selinux/include/objsec.h
+++ b/security/selinux/include/objsec.h
@@ -130,6 +130,11 @@ struct sk_security_struct {
u32 sid; /* SID of this object */
u32 peer_sid; /* SID of peer */
u16 sclass; /* sock security class */
+
Extra vertical whitespace is not needed.
Done
Post by Paul Moore
Post by Richard Haines
+ enum { /* SCTP association state */
+ SCTP_ASSOC_UNSET = 0,
+ SCTP_ASSOC_SET,
+ } sctp_assoc_state;
};
struct tun_security_struct {
diff --git a/security/selinux/netlabel.c
b/security/selinux/netlabel.c
index aaba667..7d5aa15 100644
--- a/security/selinux/netlabel.c
+++ b/security/selinux/netlabel.c
@@ -250,6 +250,7 @@ int selinux_netlbl_skbuff_setsid(struct sk_buff *skb,
sk = skb_to_full_sk(skb);
if (sk != NULL) {
struct sk_security_struct *sksec = sk->sk_security;
+
if (sksec->nlbl_state != NLBL_REQSKB)
return 0;
secattr = selinux_netlbl_sock_getattr(sk, sid);
@@ -271,6 +272,41 @@ int selinux_netlbl_skbuff_setsid(struct
sk_buff *skb,
}
/**
+ * selinux_netlbl_sctp_assoc_request - Label an incoming sctp association.
+ *
+ * Returns zero on success, negative values on failure.
+ *
+ */
+int selinux_netlbl_sctp_assoc_request(struct sctp_endpoint *ep,
+ struct sk_buff *skb)
+{
+ int rc;
+ struct netlbl_lsm_secattr secattr;
+ struct sk_security_struct *sksec = ep->base.sk-
Post by Richard Haines
sk_security;
+
+ if (ep->base.sk->sk_family != PF_INET &&
+ ep->base.sk->sk_family != PF_INET6)
+ return 0;
+
+ netlbl_secattr_init(&secattr);
+ rc = security_netlbl_sid_to_secattr(ep->secid, &secattr);
+ if (rc != 0)
+ goto assoc_request_return;
+
+ rc = netlbl_sctp_setattr(ep->base.sk, skb, &secattr);
+ if (rc == 0)
+ sksec->nlbl_state = NLBL_LABELED;
+
+ netlbl_secattr_destroy(&secattr);
+ return rc;
+}
+
+/**
* selinux_netlbl_inet_conn_request - Label an incoming stream connection
*
@@ -481,7 +517,7 @@ int selinux_netlbl_socket_setsockopt(struct socket *sock,
*/
int selinux_netlbl_socket_connect(struct sock *sk, struct sockaddr *addr)
{
- int rc;
+ int rc, already_owned_by_user = 0;
struct sk_security_struct *sksec = sk->sk_security;
struct netlbl_lsm_secattr *secattr;
@@ -489,7 +525,16 @@ int selinux_netlbl_socket_connect(struct sock
*sk, struct sockaddr *addr)
sksec->nlbl_state != NLBL_CONNLABELED)
return 0;
- lock_sock(sk);
+ /* Note: When called via connect(2) this happens before the socket
+ * when called by the SCTP protocol layer via
sctp_connectx(3),
+ */
+ if (sock_owned_by_user(sk) && sksec->sclass ==
SECCLASS_SCTP_SOCKET)
+ already_owned_by_user = 1;
+ else
+ lock_sock(sk);
/* connected sockets are allowed to disconnect when the address family
* is set to AF_UNSPEC, if that is what is happening we want to reset
@@ -510,6 +555,7 @@ int selinux_netlbl_socket_connect(struct sock
*sk, struct sockaddr *addr)
sksec->nlbl_state = NLBL_CONNLABELED;
- release_sock(sk);
+ if (!already_owned_by_user)
+ release_sock(sk);
return rc;
}
--
2.13.6
Paul Moore
2017-11-13 22:40:57 UTC
Permalink
On Mon, Nov 13, 2017 at 5:05 PM, Richard Haines
Post by Richard Haines
Post by Paul Moore
On Tue, Oct 17, 2017 at 9:59 AM, Richard Haines
Post by Richard Haines
Documentation/security/SELinux-sctp.txt
---
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
...
Post by Richard Haines
Post by Paul Moore
Post by Richard Haines
+Policy Statements
+==================
+The following class and permissions to support SCTP are available within the
+ class sctp_socket inherits socket { node_bind }
+
+ policycap extended_socket_class;
+
+The SELinux SCTP support adds the additional permissions that are explained
+ 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?
Post by Richard Haines
Post by Paul Moore
Post by Richard Haines
+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.
+
+ 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?
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.
Post by Richard Haines
Post by Paul Moore
Post by Richard Haines
+ 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.
Post by Richard Haines
Post by Paul Moore
Post by Richard Haines
+/* 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-
Post by Richard Haines
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.
Post by Richard Haines
Post by Paul Moore
Post by Richard Haines
+ 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
Stephen Smalley
2017-11-14 13:41:54 UTC
Permalink
Post by Paul Moore
On Mon, Nov 13, 2017 at 5:05 PM, Richard Haines
Post by Richard Haines
Post by Paul Moore
On Tue, Oct 17, 2017 at 9:59 AM, Richard Haines
Post by Richard Haines
Documentation/security/SELinux-sctp.txt
---
 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
...
Post by Richard Haines
Post by Paul Moore
Post by Richard Haines
+Policy Statements
+==================
+The following class and permissions to support SCTP are
available
within the
+    class sctp_socket inherits socket { node_bind }
+
+    policycap extended_socket_class;
+
+The SELinux SCTP support adds the additional permissions that
are
explained
+    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?
Post by Richard Haines
Post by Paul Moore
Post by Richard Haines
+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.
+
+   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?
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.
Post by Richard Haines
Post by Paul Moore
Post by Richard Haines
+   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.
Post by Richard Haines
Post by Paul Moore
Post by Richard Haines
+/* 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-
Post by Richard Haines
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.
AFAICT, Fedora has not yet enabled extended_socket_class policy
capability in their policy, not even in rawhide, so no breakage should
occur in Fedora. Upstream refpolicy did enable it in the 20170805
release, so it could possibly be enabled in other distributions, but
only if using the latest refpolicy + libsepol. Android enabled it in
the Android 8.0 policy, although few Android kernels are likely to
include the support yet (Android common kernel appears to be <= 4.9),
so it should presently be a no-op.
Post by Paul Moore
Post by Richard Haines
Post by Paul Moore
Post by Richard Haines
+               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.
Richard Haines
2017-11-14 21:52:57 UTC
Permalink
Post by Paul Moore
On Mon, Nov 13, 2017 at 5:05 PM, Richard Haines
Post by Richard Haines
Post by Paul Moore
On Tue, Oct 17, 2017 at 9:59 AM, Richard Haines
Post by Richard Haines
Documentation/security/SELinux-sctp.txt
---
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
...
Post by Richard Haines
Post by Paul Moore
Post by Richard Haines
+Policy Statements
+==================
+The following class and permissions to support SCTP are
available
within the
+ class sctp_socket inherits socket { node_bind }
+
+ policycap extended_socket_class;
+
+The SELinux SCTP support adds the additional permissions that
are
explained
+ 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?
SCTP Socket Option Permissions
===============================
Permissions that are validated on setsockopt(2) calls (note that the
SCTP_SOCKOPT_BINDX_REM - Remove additional bind address.
Can't see an usage for this one.
Post by Paul Moore
SCTP_PEER_ADDR_PARAMS - Set heartbeats and address max
retransmissions.
SCTP_PEER_ADDR_THLDS - Set thresholds.
SCTP_ASSOCINFO - Set association / endpoint parameters.
Also for these, considering we are not willing to go as deep as to only
allow these if within a given threshold. But still even then, sounds
like too much.
Post by Paul Moore
SCTP Bind, Connect and ASCONF Chunk Parameter Permission Checks
==============================================================
The hook security_sctp_addr_list() is called by SCTP when processing
--------------------------------------------------------------------
| sctp_socket BIND type permission checks |
| (The socket must also have the BIND permission) |
|--------------------------|-------------|-------------------------|
|SCTP_SOCKOPT_BINDX_ADD |BINDX_ADDRS |One or more ipv4/ipv6 adr|
This one can be useful, for that privilege-dropping case.

Paul note: I later changed BINDX_ADDRS to just BINDX
Post by Paul Moore
|SCTP_PRIMARY_ADDR |SET_PRI_ADDR |Single ipv4 or ipv6 adr |
|SCTP_SET_PEER_PRIMARY_ADDR|SET_PEER_ADDR|Single ipv4 or ipv6 adr |
But these, can't use an use-case.
Post by Paul Moore
--------------------------------------------------------------------
--------------------------------------------------------------------
| sctp_socket CONNECT type permission checks |
| (The socket must also have the CONNECT permission) |
|--------------------------|-------------|-------------------------|
|SCTP_SOCKOPT_CONNECTX |CONNECTX |One or more ipv4/ipv6 adr|
|SCTP_PARAM_ADD_IP |BINDX_ADDRS |One or more ipv4/ipv6 adr|
The 2 above, can be useful.
Post by Paul Moore
|SCTP_PARAM_DEL_IP |BINDX_ADDRS |One or more ipv4/ipv6 adr|
|SCTP_PARAM_SET_PRIMARY |SET_PRI_ADDR |Single ipv4 or ipv6 adr |
But not these two..
Post by Paul Moore
--------------------------------------------------------------------
SCTP_SOCKOPT_BINDX_ADD - Allows additional bind addresses to be
associated after (optionally) calling
bind(3).
sctp_bindx(3) adds a set of bind
addresses on a socket.
SCTP_SOCKOPT_CONNECTX - Allows the allocation of multiple
addresses for reaching a peer
(multi-homed).
sctp_connectx(3) initiates a connection
on an SCTP socket using multiple
destination addresses.
SCTP_PRIMARY_ADDR - Set local primary address.
SCTP_SET_PEER_PRIMARY_ADDR - Request peer sets address as
association primary.
No and no for the 2 above.
Post by Paul Moore
Post by Richard Haines
Post by Paul Moore
Post by Richard Haines
+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.
+
+ 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?
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.
I would like you to be happy with this so I've added what I think is
some clarification.

The code that handles this is the selinux_sctp_assoc_request() in
hooks.c. If not the first association, then the other association peer
SIDs are validated to enforce consistency among the peer SIDs. However
what I recommend is that all peer labels should be the same. For
example:

Don't configure this (although valid):
netlabelctl unlbl add interface:lo address:127.0.0.0/8 \
label:system_u:object_r:netlabel_peer_lo_t:s0
netlabelctl unlbl add interface:wlp12s0 address:192.168.1.0/24 \
label:system_u:object_r:netlabel_peer_wlan_t:s0
netlabelctl unlbl add interface:enp9s0 address:193.168.1.0/24 \
label:system_u:object_r:netlabel_peer_eth_t:s0

As this is recommended:
netlabelctl unlbl add interface:lo address:127.0.0.0/8 \
label:system_u:object_r:netlabel_peer_t:s0
netlabelctl unlbl add interface:wlp12s0 address:192.168.1.0/24 \
label:system_u:object_r:netlabel_peer_t:s0
netlabelctl unlbl add interface:enp9s0 address:193.168.1.0/24 \
label:system_u:object_r:netlabel_peer_t:s0

Would you prefer me to delete this section ?
Post by Paul Moore
Post by Richard Haines
Post by Paul Moore
Post by Richard Haines
+ 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.
Post by Richard Haines
Post by Paul Moore
Post by Richard Haines
+/* 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-
Post by Richard Haines
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.
Post by Richard Haines
Post by Paul Moore
Post by Richard Haines
+ 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
2017-11-20 21:55:33 UTC
Permalink
On Tue, Nov 14, 2017 at 4:52 PM, Richard Haines
Post by Richard Haines
Post by Paul Moore
On Mon, Nov 13, 2017 at 5:05 PM, Richard Haines
Post by Richard Haines
Post by Paul Moore
On Tue, Oct 17, 2017 at 9:59 AM, Richard Haines
Post by Richard Haines
Documentation/security/SELinux-sctp.txt
---
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
...
Post by Richard Haines
Post by Paul Moore
Post by Richard Haines
+Policy Statements
+==================
+The following class and permissions to support SCTP are
available
within the
+ class sctp_socket inherits socket { node_bind }
+
+ policycap extended_socket_class;
+
+The SELinux SCTP support adds the additional permissions that
are
explained
+ 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?
SCTP Socket Option Permissions
===============================
Permissions that are validated on setsockopt(2) calls (note that the
SCTP_SOCKOPT_BINDX_REM - Remove additional bind address.
Can't see an usage for this one.
Post by Paul Moore
SCTP_PEER_ADDR_PARAMS - Set heartbeats and address max
retransmissions.
SCTP_PEER_ADDR_THLDS - Set thresholds.
SCTP_ASSOCINFO - Set association / endpoint parameters.
Also for these, considering we are not willing to go as deep as to only
allow these if within a given threshold. But still even then, sounds
like too much.
Post by Paul Moore
SCTP Bind, Connect and ASCONF Chunk Parameter Permission Checks
==============================================================
The hook security_sctp_addr_list() is called by SCTP when processing
--------------------------------------------------------------------
| sctp_socket BIND type permission checks |
| (The socket must also have the BIND permission) |
|--------------------------|-------------|-------------------------|
|SCTP_SOCKOPT_BINDX_ADD |BINDX_ADDRS |One or more ipv4/ipv6 adr|
This one can be useful, for that privilege-dropping case.
Paul note: I later changed BINDX_ADDRS to just BINDX
Post by Paul Moore
|SCTP_PRIMARY_ADDR |SET_PRI_ADDR |Single ipv4 or ipv6 adr |
|SCTP_SET_PEER_PRIMARY_ADDR|SET_PEER_ADDR|Single ipv4 or ipv6 adr |
But these, can't use an use-case.
Post by Paul Moore
--------------------------------------------------------------------
--------------------------------------------------------------------
| sctp_socket CONNECT type permission checks |
| (The socket must also have the CONNECT permission) |
|--------------------------|-------------|-------------------------|
|SCTP_SOCKOPT_CONNECTX |CONNECTX |One or more ipv4/ipv6 adr|
|SCTP_PARAM_ADD_IP |BINDX_ADDRS |One or more ipv4/ipv6 adr|
The 2 above, can be useful.
Post by Paul Moore
|SCTP_PARAM_DEL_IP |BINDX_ADDRS |One or more ipv4/ipv6 adr|
|SCTP_PARAM_SET_PRIMARY |SET_PRI_ADDR |Single ipv4 or ipv6 adr |
But not these two..
Post by Paul Moore
--------------------------------------------------------------------
SCTP_SOCKOPT_BINDX_ADD - Allows additional bind addresses to be
associated after (optionally) calling
bind(3).
sctp_bindx(3) adds a set of bind
addresses on a socket.
SCTP_SOCKOPT_CONNECTX - Allows the allocation of multiple
addresses for reaching a peer
(multi-homed).
sctp_connectx(3) initiates a connection
on an SCTP socket using multiple
destination addresses.
SCTP_PRIMARY_ADDR - Set local primary address.
SCTP_SET_PEER_PRIMARY_ADDR - Request peer sets address as
association primary.
No and no for the 2 above.
I still don't undetstand how, from a security perspective,
sctp_socket:bindx is different from sctp_socket:bind. I understand
the distinction is important from a SCTP perspective, but from a
SELinux perspective isn't sctp_socket:bindx simply sctp_socket:bind?
How is bindx different from bind?

The same applies for connect/connectx.
Post by Richard Haines
Post by Paul Moore
Post by Richard Haines
Post by Paul Moore
Post by Richard Haines
+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.
+
+ 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?
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.
I would like you to be happy with this so I've added what I think is
some clarification.
The code that handles this is the selinux_sctp_assoc_request() in
hooks.c. If not the first association, then the other association peer
SIDs are validated to enforce consistency among the peer SIDs. However
what I recommend is that all peer labels should be the same. For
netlabelctl unlbl add interface:lo address:127.0.0.0/8 \
label:system_u:object_r:netlabel_peer_lo_t:s0
netlabelctl unlbl add interface:wlp12s0 address:192.168.1.0/24 \
label:system_u:object_r:netlabel_peer_wlan_t:s0
netlabelctl unlbl add interface:enp9s0 address:193.168.1.0/24 \
label:system_u:object_r:netlabel_peer_eth_t:s0
netlabelctl unlbl add interface:lo address:127.0.0.0/8 \
label:system_u:object_r:netlabel_peer_t:s0
netlabelctl unlbl add interface:wlp12s0 address:192.168.1.0/24 \
label:system_u:object_r:netlabel_peer_t:s0
netlabelctl unlbl add interface:enp9s0 address:193.168.1.0/24 \
label:system_u:object_r:netlabel_peer_t:s0
Would you prefer me to delete this section ?
Okay, I think I'm getting a better idea of this ... let me walk
through some of this, slowly.

On the send/egress side of things we don't really need to worry about
the remote peer label as we only do access control checks using the
sender's label (the local process/socket). There is a slight
exception with respect to forwarded traffic, but I think we handle
that correctly for SCTP by substituting the traffic's label for the
local process/socket label (current behavior).

On the receive/ingress side we could receive traffic from any of the
associations, with the peer:recv permission checking the remote peer
label (aka the sending association's label) against the local
receiving socket. This seems right to me. As far as userspace is
concerned, *_getpeersec_dgram() should work correctly, but
*_getpeersec_stream() will only return the peer label we stored during
the initial association setup, yes? This seems ... not the best.
Although I'm not sure how to solve this.

Is this why the requirement exists for each association to have an
equivalent label?
--
paul moore
www.paul-moore.com
Richard Haines
2017-11-21 17:37:31 UTC
Permalink
Post by Paul Moore
On Tue, Nov 14, 2017 at 4:52 PM, Richard Haines
Post by Richard Haines
Post by Paul Moore
On Mon, Nov 13, 2017 at 5:05 PM, Richard Haines
Post by Richard Haines
Post by Paul Moore
On Tue, Oct 17, 2017 at 9:59 AM, Richard Haines
Post by Richard Haines
Documentation/security/SELinux-sctp.txt
com>
---
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
...
Post by Richard Haines
Post by Paul Moore
Post by Richard Haines
+Policy Statements
+==================
+The following class and permissions to support SCTP are
available
within the
+ class sctp_socket inherits socket { node_bind }
+
+ policycap extended_socket_class;
+
+The SELinux SCTP support adds the additional permissions that
are
explained
+ 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?
SCTP Socket Option Permissions
===============================
Permissions that are validated on setsockopt(2) calls (note that the
SCTP_SOCKOPT_BINDX_REM - Remove additional bind address.
Can't see an usage for this one.
Post by Paul Moore
SCTP_PEER_ADDR_PARAMS - Set heartbeats and address max
retransmissions.
SCTP_PEER_ADDR_THLDS - Set thresholds.
SCTP_ASSOCINFO - Set association / endpoint parameters.
Also for these, considering we are not willing to go as deep as to only
allow these if within a given threshold. But still even then, sounds
like too much.
Post by Paul Moore
SCTP Bind, Connect and ASCONF Chunk Parameter Permission Checks
==============================================================
The hook security_sctp_addr_list() is called by SCTP when
processing
---------------------------------------------------------------
-----
Post by Richard Haines
sctp_socket BIND type permission
checks |
(The socket must also have the BIND
permission) |
@optname |
--------------------------|-------------|--------------------
-----|
SCTP_SOCKOPT_BINDX_ADD |BINDX_ADDRS |One or more ipv4/ipv6
adr|
This one can be useful, for that privilege-dropping case.
Paul note: I later changed BINDX_ADDRS to just BINDX
Post by Paul Moore
Post by Richard Haines
SCTP_PRIMARY_ADDR |SET_PRI_ADDR |Single ipv4 or ipv6
adr |
SCTP_SET_PEER_PRIMARY_ADDR|SET_PEER_ADDR|Single ipv4 or ipv6
adr |
But these, can't use an use-case.
Post by Paul Moore
---------------------------------------------------------------
-----
---------------------------------------------------------------
-----
Post by Richard Haines
sctp_socket CONNECT type permission
checks |
(The socket must also have the CONNECT
permission) |
@optname |
--------------------------|-------------|--------------------
-----|
SCTP_SOCKOPT_CONNECTX |CONNECTX |One or more ipv4/ipv6
adr|
SCTP_PARAM_ADD_IP |BINDX_ADDRS |One or more ipv4/ipv6
adr|
The 2 above, can be useful.
Post by Paul Moore
Post by Richard Haines
SCTP_PARAM_DEL_IP |BINDX_ADDRS |One or more ipv4/ipv6
adr|
SCTP_PARAM_SET_PRIMARY |SET_PRI_ADDR |Single ipv4 or ipv6
adr |
But not these two..
Post by Paul Moore
---------------------------------------------------------------
-----
SCTP_SOCKOPT_BINDX_ADD - Allows additional bind addresses to be
associated after (optionally) calling
bind(3).
sctp_bindx(3) adds a set of bind
addresses on a socket.
SCTP_SOCKOPT_CONNECTX - Allows the allocation of multiple
addresses for reaching a peer
(multi-homed).
sctp_connectx(3) initiates a connection
on an SCTP socket using multiple
destination addresses.
SCTP_PRIMARY_ADDR - Set local primary address.
SCTP_SET_PEER_PRIMARY_ADDR - Request peer sets address as
association primary.
No and no for the 2 above.
I still don't undetstand how, from a security perspective,
sctp_socket:bindx is different from sctp_socket:bind. I understand
the distinction is important from a SCTP perspective, but from a
SELinux perspective isn't sctp_socket:bindx simply sctp_socket:bind?
How is bindx different from bind?
The same applies for connect/connectx.
Okay I'll remove the bindx and connectx permissions.
Post by Paul Moore
Post by Richard Haines
Post by Paul Moore
Post by Richard Haines
Post by Paul Moore
Post by Richard Haines
+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.
+
+ 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?
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.
I would like you to be happy with this so I've added what I think is
some clarification.
The code that handles this is the selinux_sctp_assoc_request() in
hooks.c. If not the first association, then the other association peer
SIDs are validated to enforce consistency among the peer SIDs. However
what I recommend is that all peer labels should be the same. For
netlabelctl unlbl add interface:lo address:127.0.0.0/8 \
label:system_u:object_r:netlabel_peer_lo_t:s0
netlabelctl unlbl add interface:wlp12s0 address:192.168.1.0/24 \
label:system_u:object_r:netlabel_peer_wlan_t:s0
netlabelctl unlbl add interface:enp9s0 address:193.168.1.0/24 \
label:system_u:object_r:netlabel_peer_eth_t:s0
netlabelctl unlbl add interface:lo address:127.0.0.0/8 \
label:system_u:object_r:netlabel_peer_t:s0
netlabelctl unlbl add interface:wlp12s0 address:192.168.1.0/24 \
label:system_u:object_r:netlabel_peer_t:s0
netlabelctl unlbl add interface:enp9s0 address:193.168.1.0/24 \
label:system_u:object_r:netlabel_peer_t:s0
Would you prefer me to delete this section ?
Okay, I think I'm getting a better idea of this ... let me walk
through some of this, slowly.
On the send/egress side of things we don't really need to worry about
the remote peer label as we only do access control checks using the
sender's label (the local process/socket). There is a slight
exception with respect to forwarded traffic, but I think we handle
that correctly for SCTP by substituting the traffic's label for the
local process/socket label (current behavior).
On the receive/ingress side we could receive traffic from any of the
associations, with the peer:recv permission checking the remote peer
label (aka the sending association's label) against the local
receiving socket. This seems right to me. As far as userspace is
concerned, *_getpeersec_dgram() should work correctly,
I did get this working at one point as that's how I discovered IPv6 was
not working, hence https://github.com/SELinuxProject/selinux-kernel/iss
ues/24
Post by Paul Moore
but
*_getpeersec_stream() will only return the peer label we stored during
the initial association setup, yes?
Correct - Using this seemed reasonable as SCTP is 'connection
orientated'
Post by Paul Moore
This seems ... not the best.
Although I'm not sure how to solve this.
I had thought of taking the initial associations peer label TE
component (as now) and then setting the MLS component to that of the
process as this reflects what the service can handle when using
MLS/CIPSO/CALIPSO. Using the SCTP selinux-testsuite for example:
Server process with CIPSO enabled using SOCK_SEQPACKET:
unconfined_u:unconfined_r:test_sctp_server_t:s0:c714,c769,c782,c788,c80
3,c842,c864

First client process MLS component: s0:c769,c788,c803 requests an
association that sets the servers peer context as:
system_u:object_r:netlabel_peer_t:s0:c769,c788,c803

However as packets are accepted by the server as per peer:recv
permission checking, then client 2 with an MLS component of
s0:c782,c714,c842,c864 would also be able to request an
association/send data. Therefore in these situations having the MLS
component as part of the peer contexts seems best. RFC 5570 (CALIPSO -
7.3.3. SCTP-Related Issues) has words of wisdom that I think I follow
regarding this type of scenario !!!
Post by Paul Moore
Is this why the requirement exists for each association to have an
equivalent label?
Yes I thought it was best to make this statement as it makes life
easier. I could also force this by accepting the first association (and
maybe set the MLS component as above), then do something like this in
selinux_sctp_assoc_request():

} else if (sksec->peer_sid != peer_sid) {
/* All associations MUST have the same peer label. */
char *ctx1 = NULL, *ctx2 = NULL;
u32 len;

security_sid_to_context(peer_sid, &ctx1, &len);
security_sid_to_context(sksec->peer_sid, &ctx2, &len);

pr_err("%s: Invalid association peer context=%s
current=%s\n",
__func__, ctx1, ctx2);
kfree(ctx1);
kfree(ctx2);
err = -EINVAL;
}

Loading...