netdev - Re: [PATCH net 3/4] security: add sctp_assoc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAFqZXNvsXW7-4iD+ErKWX9c9YJ42WJHwcjQJMupVO+tbyeod1g@mail.gmail.com>
Date:   Mon, 25 Oct 2021 10:01:39 +0200
From:   Ondrej Mosnacek <omosnace@...hat.com>
To:     Xin Long <lucien.xin@...il.com>
Cc:     network dev <netdev@...r.kernel.org>,
        SElinux list <selinux@...r.kernel.org>,
        Linux Security Module list 
        <linux-security-module@...r.kernel.org>,
        "linux-sctp @ vger . kernel . org" <linux-sctp@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Marcelo Ricardo Leitner <marcelo.leitner@...il.com>,
        James Morris <jmorris@...ei.org>,
        Paul Moore <paul@...l-moore.com>,
        Richard Haines <richard_c_haines@...nternet.com>
Subject: Re: [PATCH net 3/4] security: add sctp_assoc_established hook

On Fri, Oct 22, 2021 at 8:36 AM Xin Long <lucien.xin@...il.com> wrote:
>
> security_sctp_assoc_established() is added to replace
> security_inet_conn_established() called in
> sctp_sf_do_5_1E_ca(), so that asoc can be accessed in security
> subsystem and save the peer secid to asoc->peer_secid.
>
> Fixes: 72e89f50084c ("security: Add support for SCTP security hooks")
> Reported-by: Prashanth Prahlad <pprahlad@...hat.com>
> Signed-off-by: Xin Long <lucien.xin@...il.com>
> ---
>  Documentation/security/SCTP.rst | 22 ++++++++++------------
>  include/linux/lsm_hook_defs.h   |  2 ++
>  include/linux/lsm_hooks.h       |  5 +++++
>  include/linux/security.h        |  8 ++++++++
>  net/sctp/sm_statefuns.c         |  2 +-
>  security/security.c             |  7 +++++++
>  6 files changed, 33 insertions(+), 13 deletions(-)
[...]
> diff --git a/include/linux/security.h b/include/linux/security.h
> index a16407444871..11cdddf9685c 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -1423,6 +1423,8 @@ int security_sctp_bind_connect(struct sock *sk, int optname,
>                                struct sockaddr *address, int addrlen);
>  void security_sctp_sk_clone(struct sctp_association *asoc, struct sock *sk,
>                             struct sock *newsk);
> +void security_sctp_assoc_established(struct sctp_association *asoc,
> +                                    struct sk_buff *skb);
>
>  #else  /* CONFIG_SECURITY_NETWORK */
>  static inline int security_unix_stream_connect(struct sock *sock,
> @@ -1642,6 +1644,12 @@ static inline void security_sctp_sk_clone(struct sctp_association *asoc,
>                                           struct sock *newsk)
>  {
>  }
> +
> +static inline void security_sctp_assoc_established(struct sctp_association *asoc,
> +                                                  struct sk_buff *skb)
> +{
> +       return 0;

It has now been pointed out by the kernel robot as well, but you are
returning a value from a function with return type void here.

> +}
>  #endif /* CONFIG_SECURITY_NETWORK */
>
>  #ifdef CONFIG_SECURITY_INFINIBAND
> diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> index b818532c3fc2..5fabaa54b77d 100644
> --- a/net/sctp/sm_statefuns.c
> +++ b/net/sctp/sm_statefuns.c
> @@ -946,7 +946,7 @@ enum sctp_disposition sctp_sf_do_5_1E_ca(struct net *net,
>         sctp_add_cmd_sf(commands, SCTP_CMD_INIT_COUNTER_RESET, SCTP_NULL());
>
>         /* Set peer label for connection. */
> -       security_inet_conn_established(ep->base.sk, chunk->skb);
> +       security_sctp_assoc_established((struct sctp_association *)asoc, chunk->skb);
>
>         /* RFC 2960 5.1 Normal Establishment of an Association
>          *
> diff --git a/security/security.c b/security/security.c
> index b0f1c007aa3b..4b2b4b5beb27 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -2387,6 +2387,13 @@ void security_sctp_sk_clone(struct sctp_association *asoc, struct sock *sk,
>  }
>  EXPORT_SYMBOL(security_sctp_sk_clone);
>
> +void security_sctp_assoc_established(struct sctp_association *asoc,
> +                                    struct sk_buff *skb)
> +{
> +       call_void_hook(sctp_assoc_established, asoc, skb);
> +}
> +EXPORT_SYMBOL(security_sctp_assoc_established);
> +
>  #endif /* CONFIG_SECURITY_NETWORK */
>
>  #ifdef CONFIG_SECURITY_INFINIBAND
> --
> 2.27.0
>

--
Ondrej Mosnacek
Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.