netdev - Re: [PATCH net 2/4] security: call security_sctp_assoc_request in sctp_sf_do_5_1D

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: <CAFqZXNu5PxBh8XFs=AFwJW0jpxKxPNQyXiyYekz_whHWGGXs5A@mail.gmail.com>
Date:   Mon, 25 Oct 2021 09:58:40 +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@...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 2/4] security: call security_sctp_assoc_request in sctp_sf_do_5_1D_ce

On Fri, Oct 22, 2021 at 8:36 AM Xin Long <lucien.xin@...il.com> wrote:
> The asoc created when receives the INIT chunk is a temporary one, it
> will be delete after INIT_ACK chunk is replied. So for the real asoc

Nit: s/delete/deleted/

> created in sctp_sf_do_5_1D_ce() when receives the COOKIE_ECHO chunk,

Nit: s/receives the COOKIE_ECHO chunk/the COOKIE_ECHO chunk is received/

> security_sctp_assoc_request() should also be called.
>
> 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 | 15 +++++++++------
>  net/sctp/sm_statefuns.c         |  5 +++++
>  2 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/security/SCTP.rst b/Documentation/security/SCTP.rst
> index 415b548d9ce0..9a38067762e5 100644
> --- a/Documentation/security/SCTP.rst
> +++ b/Documentation/security/SCTP.rst
> @@ -151,9 +151,9 @@ establishing an association.
>           INIT --------------------------------------------->
>                                                     sctp_sf_do_5_1B_init()
>                                                   Respond to an INIT chunk.
> -                                             SCTP peer endpoint "A" is
> -                                             asking for an association. Call
> -                                             security_sctp_assoc_request()
> +                                             SCTP peer endpoint "A" is asking
> +                                             for an temporary association.

Nit: s/an/a/

> +                                             Call security_sctp_assoc_request()
>                                               to set the peer label if first
>                                               association.
>                                               If not first association, check
> @@ -163,9 +163,12 @@ establishing an association.
>            |                                       discard the packet.
>            |
>      COOKIE ECHO ------------------------------------------>
> -                                                          |
> -                                                          |
> -                                                          |
> +                                                  sctp_sf_do_5_1D_ce()
> +                                             Respond to an COOKIE ECHO chunk.
> +                                             Confirm the cookie and create an

Nit: s/an/a/

> +                                             permanent association.
> +                                             Call security_sctp_assoc_request() to
> +                                             do the same as for INIT chunk Response.
>            <------------------------------------------- COOKIE ACK
>            |                                               |
>      sctp_sf_do_5_1E_ca                                    |
> diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> index 3206374209bc..b818532c3fc2 100644
> --- a/net/sctp/sm_statefuns.c
> +++ b/net/sctp/sm_statefuns.c
> @@ -781,6 +781,11 @@ enum sctp_disposition sctp_sf_do_5_1D_ce(struct net *net,
>                 }
>         }
>
> +       if (security_sctp_assoc_request(new_asoc, chunk->skb)) {
> +               sctp_association_free(new_asoc);
> +               return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
> +       }
> +
>         /* Delay state machine commands until later.
>          *
>          * Re-build the bind address for the association is done in
> --
> 2.27.0
>

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