On Wed, 2016-12-21 at 14:09 -0200, Marcelo Ricardo Leitner wrote:
> On Wed, Dec 14, 2016 at 01:39:59PM +0000, Richard Haines wrote:
> > +SCTP Socket Option Permissions
> > +===============================
> > +The permissions consist of: "bindx_add" "bindx_rem" "connectx"
> > "set_addr" and
> > +"set_params" that are validated on setsockopt(2) calls, and
> > "peeloff" that is
> > +validated on getsockopt(2) calls.
> > +
> > +SCTP_SOCKOPT_BINDX_ADD - Allows additional bind addresses to be
> > + associated after (optionally) calling
> > bind(2)
> > + if given the "bind_add" permission.
> > +
> > +SCTP_SOCKOPT_CONNECTX - Allows the allocation of multiple
> > + addresses for reaching a multi-homed peer
> > + if given the "connectx" permission.
> > +
> > + Together they are used to form SCTP associations with
> > information being
> > + passed over the link to inform the peer of any changes. As these
> > two options
> > + can support multiple addresses, each address is checked via
> > + selinux_socket_bind() or selinux_socket_connect() to determine
> > whether they
> > + have the correct permissions:
> > + bindx_add: bind, name_bind, node_bind + node SID + port SID
> > via the
> > + (portcon sctp port ctx) policy statement.
> > + connectx: connect, name_connect + port SID via the
> > + (portcon sctp port ctx) policy statement.
> > +
> > +SCTP_SOCKOPT_BINDX_REM - Allows additional bind addresses to be
> > removed
> > + if given the "bind_rem" permission.
> > +
> > +SCTP_PEER_ADDR_PARAMS - Alter heartbeats and address max
> > retransmissions.
> > +SCTP_PEER_ADDR_THLDS - Alter the thresholds.
> > +SCTP_ASSOCINFO - Alter association and endpoint parameters.
> > + These require the "set_params" permission.
> > +
> > +SCTP_PRIMARY_ADDR - Set local primary address.
> > +SCTP_SET_PEER_PRIMARY_ADDR - Request peer sets address as
> > association primary.
> > + These require the "set_addr" permission.
> > +
> > +SCTP_SOCKOPT_PEELOFF - Branch off an association into a new socket
> > that
> > +will be a one-to-one style socket. As SELinux already handles the
> > creation
> > +of new sockets, only the "peeloff" permission is checked.
>
> ...
>
> > diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> > index 7b0e059..ff4f1a8 100644
> > --- a/net/sctp/socket.c
> > +++ b/net/sctp/socket.c
> > @@ -1009,6 +1009,12 @@ static int sctp_setsockopt_bindx(struct sock
> > *sk,
> > /* Do the work. */
> > switch (op) {
> > case SCTP_BINDX_ADD_ADDR:
> > + /* Allow security module to validate bindx
> > addresses. */
> > + err = security_sk_setsockopt(sk, SOL_SCTP,
> > + SCTP_SOCKOPT_BINDX_AD
> > D,
> > + (char *)kaddrs,
> > addrs_size);
>
> Here, kaddrs is about the addresses that we are going to bind to.
>
> > + if (err)
> > + goto out;
> > err = sctp_bindx_add(sk, kaddrs, addrcnt);
> > if (err)
> > goto out;
> > @@ -1329,9 +1335,17 @@ static int __sctp_setsockopt_connectx(struct
> > sock *sk,
> > if (__copy_from_user(kaddrs, addrs, addrs_size)) {
> > err = -EFAULT;
> > } else {
> > + /* Allow security module to validate connectx
> > addresses. */
> > + err = security_sk_setsockopt(sk, SOL_SCTP,
> > + SCTP_SOCKOPT_CONNECTX
> > ,
> > + (char *)kaddrs,
> > addrs_size);
>
Sorry for the delay but I now think I've resolved all but one of the
SCTP issues with tests to check them. The only area I'm having trouble
with is labeling TCP-style child sockets but hope to resolve.
> Here, kaddrs is about the remote addresses that we are connecting to.
> Not sure how feasible this is for SELinux, to maintain a list of
> allowed
> peers.
SELinux does not maintain lists, however it can check whether the
addresses/ports are allowed or not (which is what I do for binds,
connects etc.).
> But this being right, I think we are missing the hooks at ASCONF
> handling side.
> One SCTP peer can start/stop binding to another IP in runtime using
> ASCONF chunks. So considering that peer A here validated that it can
> associate to be peer B, if B is using ASCONF to inform A that it's
> now
> also binding on address X, A should validate so before ACKing it.
>
> This validation would be around sctp_process_asconf_param. Not sure
> you
> can hook it on selinux_sctp_setsockopt too as it would be similar to
> the
> validation done for CONNECT.
I now have this working after hooking into sctp_process_asconf_param
and checking permissions on address/port as required. Also have tests
as part of the selinux-testsuite.
>
> Richard, the other point we talked offline, was for validating that
> peer
> A can actually request to add address X, that would be ok, yes.
>
This was regarding handling ASCONF requests on receiver the side. Yes
this does seem to be resolved.
I hope to send out an updated patch in a few weeks so hopefully these
can be verified.
> Thanks,
> Marcelo
>
> > + if (err)
> > + goto out_free;
> > +
> > err = __sctp_connect(sk, kaddrs, addrs_size,
> > assoc_id);
> > }
> >
> > +out_free:
> > kfree(kaddrs);
> >
> > return err;
>
>
>
> > +int selinux_sctp_setsockopt(struct sock *sk, int level, int
> > optname,
> > + char *optval, int
> > optlen)
> > +{
> > + int err, addrlen;
> > + void *addr_buf;
> > + struct sockaddr *address;
> > + struct socket *sock;
> > + int walk_size = 0;
> > +
> > + if (level != SOL_SCTP || level != IPPROTO_SCTP)
> > + return 0;
> > +
> > + switch (optname) {
> > + case SCTP_SOCKOPT_BINDX_ADD:
> > + case SCTP_SOCKOPT_CONNECTX:
> > + /* Note that for SCTP_SOCKOPT_BINDX_ADD and
> > + * SCTP_SOCKOPT_CONNECTX the sctp kernel code has
> > already
> > + * copied the optval to kernel space. See
> > net/sctp/socket.c
> > + * security_sk_setsockopt() calls.
> > + */
> > + err = sock_has_perm(current, sk,
> > + (optname == SCTP_SOCKOPT_BINDX_ADD ?
> > + SCTP_SOCKET__BINDX_ADD :
> > + SCTP_SOCKET__CONNECTX));
> > + if (err)
> > + return err;
> > +
> > + sock = sk->sk_socket;
> > + addr_buf = optval;
> > + /* Process list - may contain IPv4 or IPv6 addr's
> > */
> > + while (walk_size < optlen) {
> > + address = addr_buf;
> > +
> > + switch (address->sa_family) {
> > + case PF_INET:
> > + addrlen = sizeof(struct
> > sockaddr_in);
> > + break;
> > + case PF_INET6:
> > + addrlen = sizeof(struct
> > sockaddr_in6);
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + err = -EINVAL;
> > + if (optname == SCTP_SOCKOPT_BINDX_ADD) {
> > + err = selinux_socket_bind(sock,
> > + address, addrlen);
> > + } else if (optname ==
> > SCTP_SOCKOPT_CONNECTX) {
> > + err = selinux_socket_connect(sock,
> > + address, addrlen);
> > + }
> > + if (err)
> > + return err;
> > +
> > + addr_buf += addrlen;
> > + walk_size += addrlen;
> > + }
> > + break;
> > +
> > + case SCTP_SOCKOPT_BINDX_REM:
> > + /* The addresses have been checked as they were
> > + * added, so just see if allowed to be removed.
> > + */
> > + err = sock_has_perm(current, sk,
> > SCTP_SOCKET__BINDX_REM);
> > + if (err)
> > + return err;
> > + break;
> > +
> > + /* Set heartbeats and address max retransmissions. */
> > + case SCTP_PEER_ADDR_PARAMS:
> > + /* Set thresholds. */
> > + case SCTP_PEER_ADDR_THLDS:
> > + /* Set association and endpoint parameters */
> > + case SCTP_ASSOCINFO:
> > + err = sock_has_perm(current, sk,
> > SCTP_SOCKET__SET_PARAMS);
> > + if (err)
> > + return err;
> > + break;
> > +
> > + /* Set local primary address. */
> > + case SCTP_PRIMARY_ADDR:
> > + /* Request peer sets address as association primary. */
> > + case SCTP_SET_PEER_PRIMARY_ADDR:
> > + err = sock_has_perm(current, sk,
> > SCTP_SOCKET__SET_ADDR);
> > + if (err)
> > + return err;
> > + break;
> > + }
> > +
> > + return 0;
> > +}
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
