mbox series

[net,0/4] security: fixups for the security hooks in sctp

Message ID cover.1634884487.git.lucien.xin@gmail.com (mailing list archive)
Headers show
Series security: fixups for the security hooks in sctp | expand

Message

Xin Long Oct. 22, 2021, 6:36 a.m. UTC 
There are a couple of problems in the currect security hooks in sctp:

1. The hooks incorrectly treat sctp_endpoint in SCTP as request_sock in
   TCP, while it's in fact no more than an extension of the sock, and
   represents the local host. It is created when sock is created, not
   when a conn request comes. sctp_association is actually the correct
   one to represent the connection, and created when a conn request
   arrives.

2. security_sctp_assoc_request() hook should also be called in processing
   COOKIE ECHO, as that's the place where the real assoc is created and
   used in the future.

The problems above may cause accept sk, peeloff sk or client sk having
the incorrect security labels.

So this patchset is to change some hooks and pass asoc into them and save
these secids into asoc, as well as add the missing sctp_assoc_request
hook into the COOKIE ECHO processing.

Xin Long (4):
  security: pass asoc to sctp_assoc_request and sctp_sk_clone
  security: call security_sctp_assoc_request in sctp_sf_do_5_1D_ce
  security: add sctp_assoc_established hook
  security: implement sctp_assoc_established hook in selinux

 Documentation/security/SCTP.rst     | 65 +++++++++++++++--------------
 include/linux/lsm_hook_defs.h       |  6 ++-
 include/linux/lsm_hooks.h           | 13 ++++--
 include/linux/security.h            | 18 +++++---
 include/net/sctp/structs.h          | 20 ++++-----
 net/sctp/sm_statefuns.c             | 31 ++++++++------
 net/sctp/socket.c                   |  5 +--
 security/security.c                 | 15 +++++--
 security/selinux/hooks.c            | 36 +++++++++++-----
 security/selinux/include/netlabel.h |  4 +-
 security/selinux/netlabel.c         | 14 +++----
 11 files changed, 135 insertions(+), 92 deletions(-)

Comments

Richard Haines Oct. 24, 2021, 1:42 p.m. UTC | #1 
On Fri, 2021-10-22 at 02:36 -0400, Xin Long wrote:
> There are a couple of problems in the currect security hooks in sctp:
> 
> 1. The hooks incorrectly treat sctp_endpoint in SCTP as request_sock in
>    TCP, while it's in fact no more than an extension of the sock, and
>    represents the local host. It is created when sock is created, not
>    when a conn request comes. sctp_association is actually the correct
>    one to represent the connection, and created when a conn request
>    arrives.
> 
> 2. security_sctp_assoc_request() hook should also be called in
> processing
>    COOKIE ECHO, as that's the place where the real assoc is created and
>    used in the future.
> 
> The problems above may cause accept sk, peeloff sk or client sk having
> the incorrect security labels.
> 
> So this patchset is to change some hooks and pass asoc into them and
> save
> these secids into asoc, as well as add the missing sctp_assoc_request
> hook into the COOKIE ECHO processing.

I've built this patchset on kernel 5.15-rc5 with no problems.
I tested this using the SELinux testsuite with Ondrej's "[PATCH
testsuite] tests/sctp: add client peeloff tests" [1] added. All SCTP
tests ran with no errors. Also ran the sctp-tests from [2] with no
errors.

[1]
https://lore.kernel.org/selinux/20211021144543.740762-1-omosnace@redhat.com/
[2] https://github.com/sctp/sctp-tests.git

Reviewed-by: Richard Haines <richard_c_haines@btinternet.com>
Tested-by: Richard Haines <richard_c_haines@btinternet.com>

> 
> Xin Long (4):
>   security: pass asoc to sctp_assoc_request and sctp_sk_clone
>   security: call security_sctp_assoc_request in sctp_sf_do_5_1D_ce
>   security: add sctp_assoc_established hook
>   security: implement sctp_assoc_established hook in selinux
> 
>  Documentation/security/SCTP.rst     | 65 +++++++++++++++--------------
>  include/linux/lsm_hook_defs.h       |  6 ++-
>  include/linux/lsm_hooks.h           | 13 ++++--
>  include/linux/security.h            | 18 +++++---
>  include/net/sctp/structs.h          | 20 ++++-----
>  net/sctp/sm_statefuns.c             | 31 ++++++++------
>  net/sctp/socket.c                   |  5 +--
>  security/security.c                 | 15 +++++--
>  security/selinux/hooks.c            | 36 +++++++++++-----
>  security/selinux/include/netlabel.h |  4 +-
>  security/selinux/netlabel.c         | 14 +++----
>  11 files changed, 135 insertions(+), 92 deletions(-)
>