Re: [RFC PATCH 5/5] selinux: Add SCTP support

2017-11-21 Thread Richard Haines 
On Mon, 2017-11-20 at 16:55 -0500, Paul Moore wrote:
> On Tue, Nov 14, 2017 at 4:52 PM, Richard Haines
>  wrote:
> > On Mon, 2017-11-13 at 17:40 -0500, Paul Moore wrote:
> > > On Mon, Nov 13, 2017 at 5:05 PM, Richard Haines
> > >  wrote:
> > > > On Mon, 2017-11-06 at 19:09 -0500, Paul Moore wrote:
> > > > > On Tue, Oct 17, 2017 at 9:59 AM, Richard Haines
> > > > >  wrote:
> > > > > > The SELinux SCTP implementation is explained in:
> > > > > > Documentation/security/SELinux-sctp.txt
> > > > > > 
> > > > > > Signed-off-by: Richard Haines  > > > > > 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
> > > 
> > > ...
> > > 
> > > > > > +Policy Statements
> > > > > > +==
> > > > > > +The following class and permissions to support SCTP are
> > > > > > available
> > > > > > within the
> > > > > > +kernel:
> > > > > > +class sctp_socket inherits socket { node_bind }
> > > > > > +
> > > > > > +whenever the following policy capability is enabled:
> > > > > > +policycap extended_socket_class;
> > > > > > +
> > > > > > +The SELinux SCTP support adds the additional permissions
> > > > > > that
> > > > > > are
> > > > > > explained
> > > > > > +in the sections below:
> > > > > > +association bindx connectx
> > > > > 
> > > > > Is the distinction between bind and bindx significant?  The
> > > > > same
> > > > > question applies to connect/connectx.  I think we can
> > > > > probably
> > > > > just
> > > > > reuse bind and connect in these cases.
> > > > 
> > > > This has been discussed before with Marcelo and keeping
> > > > bindx/connectx
> > > > is a useful distinction.
> > > 
> > > My apologies, I must have forgotten/missed that discussion.  Do
> > > you
> > > have an archive pointer?
> > 
> > No this was off list, however I've copied the relevant bits:
> > 
> > > SCTP Socket Option Permissions
> > > ===
> > > Permissions that are validated on setsockopt(2) calls (note that
> > > the
> > > sctp_socket SETOPT permission must be allowed):
> > > 
> > > This option requires the BINDX_ADDR permission:
> > > SCTP_SOCKOPT_BINDX_REM - Remove additional bind address.
> > 
> > Can't see an usage for this one.
> > 
> > > 
> > > These options require the SET_PARAMS permission:
> > > 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.
> > 
> > > 
> > > 
> > > SCTP Bind, Connect and ASCONF Chunk Parameter Permission Checks
> > > ==
> > > The hook security_sctp_addr_list() is called by SCTP when
> > > processing
> > > various options (@optname) to check permissions required for the
> > > list
> > > of ipv4/ipv6 addresses (@address) as follows:
> > > ---
> > > -
> > > >sctp_socket BIND type permission
> > > > checks  |
> > > >(The socket must also have the BIND
> > > > permission)  |
> > > >  @optname|
> > > > Permission  |  @address  |
> > > > --|-|
> > > > -|
> > > > 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
> > 
> > > > 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.
> > 
> > > ---
> > > -
> > > ---
> > > -
> > > >sctp_socket CONNECT type permission
> > > > checks|
> > > >(The socket must also have the CONNECT
> > > > permission)|
> > > >  @optname|
> > > > Permission  |  @address  |
> > > >

Re: [RFC PATCH 5/5] selinux: Add SCTP support

2017-11-20 Thread Paul Moore 
On Tue, Nov 14, 2017 at 4:52 PM, Richard Haines
 wrote:
> On Mon, 2017-11-13 at 17:40 -0500, Paul Moore wrote:
>> On Mon, Nov 13, 2017 at 5:05 PM, Richard Haines
>>  wrote:
>> > On Mon, 2017-11-06 at 19:09 -0500, Paul Moore wrote:
>> > > On Tue, Oct 17, 2017 at 9:59 AM, Richard Haines
>> > >  wrote:
>> > > > The SELinux SCTP implementation is explained in:
>> > > > Documentation/security/SELinux-sctp.txt
>> > > >
>> > > > Signed-off-by: Richard Haines 
>> > > > ---
>> > > >  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
>>
>> ...
>>
>> > > > +Policy Statements
>> > > > +==
>> > > > +The following class and permissions to support SCTP are
>> > > > available
>> > > > within the
>> > > > +kernel:
>> > > > +class sctp_socket inherits socket { node_bind }
>> > > > +
>> > > > +whenever the following policy capability is enabled:
>> > > > +policycap extended_socket_class;
>> > > > +
>> > > > +The SELinux SCTP support adds the additional permissions that
>> > > > are
>> > > > explained
>> > > > +in the sections below:
>> > > > +association bindx connectx
>> > >
>> > > Is the distinction between bind and bindx significant?  The same
>> > > question applies to connect/connectx.  I think we can probably
>> > > just
>> > > reuse bind and connect in these cases.
>> >
>> > This has been discussed before with Marcelo and keeping
>> > bindx/connectx
>> > is a useful distinction.
>>
>> My apologies, I must have forgotten/missed that discussion.  Do you
>> have an archive pointer?
>
> No this was off list, however I've copied the relevant bits:
>
>> SCTP Socket Option Permissions
>> ===
>> Permissions that are validated on setsockopt(2) calls (note that the
>> sctp_socket SETOPT permission must be allowed):
>>
>> This option requires the BINDX_ADDR permission:
>> SCTP_SOCKOPT_BINDX_REM - Remove additional bind address.
>
> Can't see an usage for this one.
>
>>
>> These options require the SET_PARAMS permission:
>> 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.
>
>>
>>
>> SCTP Bind, Connect and ASCONF Chunk Parameter Permission Checks
>> ==
>> The hook security_sctp_addr_list() is called by SCTP when processing
>> various options (@optname) to check permissions required for the list
>> of ipv4/ipv6 addresses (@address) as follows:
>> 
>> |sctp_socket BIND type permission checks  |
>> |(The socket must also have the BIND permission)  |
>> |  @optname| Permission  |  @address  |
>> |--|-|-|
>> |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
>
>> |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.
>
>> 
>> 
>> |sctp_socket CONNECT type permission checks|
>> |(The socket must also have the CONNECT permission)|
>> |  @optname| Permission  |  @address  |
>> |--|-|-|
>> |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.
>
>> |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..
>
>> 
>>
>> SCTP_SOCKOPT_BINDX_ADD - Allows additional bind addresses to be
>> associated after (optionally) calling
>> bind(3).
>>

Re: [RFC PATCH 5/5] selinux: Add SCTP support

2017-11-14 Thread Richard Haines 
On Mon, 2017-11-13 at 17:40 -0500, Paul Moore wrote:
> On Mon, Nov 13, 2017 at 5:05 PM, Richard Haines
>  wrote:
> > On Mon, 2017-11-06 at 19:09 -0500, Paul Moore wrote:
> > > On Tue, Oct 17, 2017 at 9:59 AM, Richard Haines
> > >  wrote:
> > > > The SELinux SCTP implementation is explained in:
> > > > Documentation/security/SELinux-sctp.txt
> > > > 
> > > > Signed-off-by: Richard Haines 
> > > > ---
> > > >  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
> 
> ...
> 
> > > > +Policy Statements
> > > > +==
> > > > +The following class and permissions to support SCTP are
> > > > available
> > > > within the
> > > > +kernel:
> > > > +class sctp_socket inherits socket { node_bind }
> > > > +
> > > > +whenever the following policy capability is enabled:
> > > > +policycap extended_socket_class;
> > > > +
> > > > +The SELinux SCTP support adds the additional permissions that
> > > > are
> > > > explained
> > > > +in the sections below:
> > > > +association bindx connectx
> > > 
> > > Is the distinction between bind and bindx significant?  The same
> > > question applies to connect/connectx.  I think we can probably
> > > just
> > > reuse bind and connect in these cases.
> > 
> > This has been discussed before with Marcelo and keeping
> > bindx/connectx
> > is a useful distinction.
> 
> My apologies, I must have forgotten/missed that discussion.  Do you
> have an archive pointer?

No this was off list, however I've copied the relevant bits:

> SCTP Socket Option Permissions
> ===
> Permissions that are validated on setsockopt(2) calls (note that the
> sctp_socket SETOPT permission must be allowed):
>
> This option requires the BINDX_ADDR permission:
> SCTP_SOCKOPT_BINDX_REM - Remove additional bind address.

Can't see an usage for this one.

>
> These options require the SET_PARAMS permission:
> 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.

>
>
> SCTP Bind, Connect and ASCONF Chunk Parameter Permission Checks
> ==
> The hook security_sctp_addr_list() is called by SCTP when processing
> various options (@optname) to check permissions required for the list
> of ipv4/ipv6 addresses (@address) as follows:
> 
> |sctp_socket BIND type permission checks  |
> |(The socket must also have the BIND permission)  |
> |  @optname| Permission  |  @address  |
> |--|-|-|
> |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

> |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.

> 
> 
> |sctp_socket CONNECT type permission checks|
> |(The socket must also have the CONNECT permission)|
> |  @optname| Permission  |  @address  |
> |--|-|-|
> |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.

> |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..

> 
>
> 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

Re: [RFC PATCH 5/5] selinux: Add SCTP support

2017-11-14 Thread Stephen Smalley 
On Mon, 2017-11-13 at 17:40 -0500, Paul Moore wrote:
> On Mon, Nov 13, 2017 at 5:05 PM, Richard Haines
>  wrote:
> > On Mon, 2017-11-06 at 19:09 -0500, Paul Moore wrote:
> > > On Tue, Oct 17, 2017 at 9:59 AM, Richard Haines
> > >  wrote:
> > > > The SELinux SCTP implementation is explained in:
> > > > Documentation/security/SELinux-sctp.txt
> > > > 
> > > > Signed-off-by: Richard Haines 
> > > > ---
> > > >  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
> 
> ...
> 
> > > > +Policy Statements
> > > > +==
> > > > +The following class and permissions to support SCTP are
> > > > available
> > > > within the
> > > > +kernel:
> > > > +class sctp_socket inherits socket { node_bind }
> > > > +
> > > > +whenever the following policy capability is enabled:
> > > > +policycap extended_socket_class;
> > > > +
> > > > +The SELinux SCTP support adds the additional permissions that
> > > > are
> > > > explained
> > > > +in the sections below:
> > > > +association bindx connectx
> > > 
> > > Is the distinction between bind and bindx significant?  The same
> > > question applies to connect/connectx.  I think we can probably
> > > just
> > > reuse bind and connect in these cases.
> > 
> > This has been discussed before with Marcelo and keeping
> > bindx/connectx
> > is a useful distinction.
> 
> My apologies, I must have forgotten/missed that discussion.  Do you
> have an archive pointer?
> 
> > > > +SCTP Peer Labeling
> > > > +===
> > > > +An SCTP socket will only have one peer label assigned to it.
> > > > This
> > > > will be
> > > > +assigned during the establishment of the first association.
> > > > Once
> > > > the peer
> > > > +label has been assigned, any new associations will have the
> > > > "association"
> > > > +permission validated by checking the socket peer sid against
> > > > the
> > > > received
> > > > +packets peer sid to determine whether the association should
> > > > be
> > > > allowed or
> > > > +denied.
> > > > +
> > > > +NOTES:
> > > > +   1) If peer labeling is not enabled, then the peer context
> > > > will
> > > > always be
> > > > +  SECINITSID_UNLABELED (unlabeled_t in Reference Policy).
> > > > +
> > > > +   2) As SCTP supports multiple endpoints with multi-homing on
> > > > a
> > > > single socket
> > > > +  it is recommended that peer labels are consistent.
> > > 
> > > My apologies if I'm confused, but I thought it was multiple
> > > associations per-endpoint, with the associations providing the
> > > multi-homing functionality, no?
> > 
> > I've reworded to:
> > As SCTP can support more than one transport address per endpoint
> > (multi-homing) on a single socket, it is possible to configure
> > policy
> > and NetLabel to provide different peer labels for each of these. As
> > the
> > socket peer label is determined by the first associations transport
> > address, it is recommended that all peer labels are consistent.
> 
> I'm still not sure this makes complete sense to me, but since I'm
> still not 100% confident in my understanding of SCTP I'm willing to
> punt on this for the moment.
> 
> > > > +   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.
> 
> > > > +/* 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;
> > > > +

Re: [RFC PATCH 5/5] selinux: Add SCTP support

2017-11-13 Thread Paul Moore 
On Mon, Nov 13, 2017 at 5:05 PM, Richard Haines
 wrote:
> On Mon, 2017-11-06 at 19:09 -0500, Paul Moore wrote:
>> On Tue, Oct 17, 2017 at 9:59 AM, Richard Haines
>>  wrote:
>> > The SELinux SCTP implementation is explained in:
>> > Documentation/security/SELinux-sctp.txt
>> >
>> > Signed-off-by: Richard Haines 
>> > ---
>> >  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

...

>> > +Policy Statements
>> > +==
>> > +The following class and permissions to support SCTP are available
>> > within the
>> > +kernel:
>> > +class sctp_socket inherits socket { node_bind }
>> > +
>> > +whenever the following policy capability is enabled:
>> > +policycap extended_socket_class;
>> > +
>> > +The SELinux SCTP support adds the additional permissions that are
>> > explained
>> > +in the sections below:
>> > +association bindx connectx
>>
>> Is the distinction between bind and bindx significant?  The same
>> question applies to connect/connectx.  I think we can probably just
>> reuse bind and connect in these cases.
>
> This has been discussed before with Marcelo and keeping bindx/connectx
> is a useful distinction.

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

>> > +SCTP Peer Labeling
>> > +===
>> > +An SCTP socket will only have one peer label assigned to it. This
>> > will be
>> > +assigned during the establishment of the first association. Once
>> > the peer
>> > +label has been assigned, any new associations will have the
>> > "association"
>> > +permission validated by checking the socket peer sid against the
>> > received
>> > +packets peer sid to determine whether the association should be
>> > allowed or
>> > +denied.
>> > +
>> > +NOTES:
>> > +   1) If peer labeling is not enabled, then the peer context will
>> > always be
>> > +  SECINITSID_UNLABELED (unlabeled_t in Reference Policy).
>> > +
>> > +   2) As SCTP supports multiple endpoints with multi-homing on a
>> > single socket
>> > +  it is recommended that peer labels are consistent.
>>
>> My apologies if I'm confused, but I thought it was multiple
>> associations per-endpoint, with the associations providing the
>> multi-homing functionality, no?
>
> I've reworded to:
> As SCTP can support more than one transport address per endpoint
> (multi-homing) on a single socket, it is possible to configure policy
> and NetLabel to provide different peer labels for each of these. As the
> socket peer label is determined by the first associations transport
> address, it is recommended that all peer labels are consistent.

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

>> > +   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.

>> > +/* 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.
>
> I hope not. I will need some direction here as I've not had problems
> (yet).

It's actually not that

Re: [RFC PATCH 5/5] selinux: Add SCTP support

2017-11-13 Thread Richard Haines 
On Mon, 2017-11-06 at 19:09 -0500, Paul Moore wrote:
> On Tue, Oct 17, 2017 at 9:59 AM, Richard Haines
>  wrote:
> > The SELinux SCTP implementation is explained in:
> > Documentation/security/SELinux-sctp.txt
> > 
> > Signed-off-by: Richard Haines 
> > ---
> >  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 000..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
> 
> > +   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
> 
> 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.
> 
> See my question on sctp_socket:association below.
> 
> > +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   |
> > +

Re: [RFC PATCH 5/5] selinux: Add SCTP support

2017-11-06 Thread Paul Moore 
On Tue, Oct 17, 2017 at 9:59 AM, Richard Haines
 wrote:
> The SELinux SCTP implementation is explained in:
> Documentation/security/SELinux-sctp.txt
>
> Signed-off-by: Richard Haines 
> ---
>  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 000..32e0255
> --- /dev/null
> +++ b/Documentation/security/SELinux-sctp.txt
> @@ -0,0 +1,108 @@

See my previous comments about moving to reStructuredText for these docs.

> +   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

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.

> +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

Re: [RFC PATCH 5/5] selinux: Add SCTP support

2017-11-01 Thread Richard Haines 
On Tue, 2017-10-31 at 15:16 -0200, Marcelo Ricardo Leitner wrote:
> On Tue, Oct 17, 2017 at 02:59:53PM +0100, Richard Haines wrote:
> > The SELinux SCTP implementation is explained in:
> > Documentation/security/SELinux-sctp.txt
> > 
> > Signed-off-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
> 
> ...
> > @@ -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) {

> 
>   Marcelo
> --
> To unsubscribe from this list: send the line "unsubscribe linux-
> security-module" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 5/5] selinux: Add SCTP support

2017-10-31 Thread Marcelo Ricardo Leitner 
On Tue, Oct 17, 2017 at 02:59:53PM +0100, Richard Haines wrote:
> The SELinux SCTP implementation is explained in:
> Documentation/security/SELinux-sctp.txt
> 
> Signed-off-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
...
> @@ -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

Re: [RFC PATCH 5/5] selinux: Add SCTP support

2017-10-24 Thread Richard Haines 
On Fri, 2017-10-20 at 15:00 -0400, Stephen Smalley wrote:
> On Tue, 2017-10-17 at 14:59 +0100, Richard Haines wrote:
> > The SELinux SCTP implementation is explained in:
> > Documentation/security/SELinux-sctp.txt
> > 
> > Signed-off-by: Richard Haines 
> > ---
> >  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 000..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
> >

Re: [RFC PATCH 5/5] selinux: Add SCTP support

2017-10-20 Thread Stephen Smalley 
On Tue, 2017-10-17 at 14:59 +0100, Richard Haines wrote:
> The SELinux SCTP implementation is explained in:
> Documentation/security/SELinux-sctp.txt
> 
> Signed-off-by: Richard Haines 
> ---
>  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 000..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

[RFC PATCH 5/5] selinux: Add SCTP support

2017-10-17 Thread Richard Haines 
The SELinux SCTP implementation is explained in:
Documentation/security/SELinux-sctp.txt

Signed-off-by: Richard Haines 
---
 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 000..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.