Skip to content

Commit

Permalink
Merge branch 'sctp-=security-hook-fixes'
Browse files Browse the repository at this point in the history 
Xin Long says:

====================
security: fixups for the security hooks in sctp

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.

v1->v2:
  - See each patch, and thanks the help from Ondrej, Paul and Richard.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David S. Miller committed Nov 3, 2021
2 parents 843c3cb + e7310c9 commit 2bd080b
Show file tree
Hide file tree
Showing 11 changed files with 133 additions and 95 deletions.
65 changes: 33 additions & 32 deletions Documentation/security/SCTP.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,19 @@ For security module support, three SCTP specific hooks have been implemented::
security_sctp_assoc_request()
security_sctp_bind_connect()
security_sctp_sk_clone()

Also the following security hook has been utilised::

security_inet_conn_established()
security_sctp_assoc_established()

The usage of these hooks are described below with the SELinux implementation
described in the `SCTP SELinux Support`_ chapter.


security_sctp_assoc_request()
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Passes the ``@ep`` and ``@chunk->skb`` of the association INIT packet to the
Passes the ``@asoc`` and ``@chunk->skb`` of the association INIT packet to the
security module. Returns 0 on success, error on failure.
::

@ep - pointer to sctp endpoint structure.
@asoc - pointer to sctp association structure.
@skb - pointer to skbuff of association packet.


Expand Down Expand Up @@ -117,24 +114,25 @@ Called whenever a new socket is created by **accept**\(2)
calls **sctp_peeloff**\(3).
::

@ep - pointer to current sctp endpoint structure.
@asoc - pointer to current sctp association structure.
@sk - pointer to current sock structure.
@sk - pointer to new sock structure.
@newsk - pointer to new sock structure.


security_inet_conn_established()
security_sctp_assoc_established()
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Called when a COOKIE ACK is received::
Called when a COOKIE ACK is received, and the peer secid will be
saved into ``@asoc->peer_secid`` for client::

@sk - pointer to sock structure.
@asoc - pointer to sctp association structure.
@skb - pointer to skbuff of the COOKIE ACK packet.


Security Hooks used for Association Establishment
-------------------------------------------------

The following diagram shows the use of ``security_sctp_bind_connect()``,
``security_sctp_assoc_request()``, ``security_inet_conn_established()`` when
``security_sctp_assoc_request()``, ``security_sctp_assoc_established()`` when
establishing an association.
::

Expand All @@ -151,9 +149,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 a temporary association.
Call security_sctp_assoc_request()
to set the peer label if first
association.
If not first association, check
Expand All @@ -163,13 +161,16 @@ 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 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 |
Call security_inet_conn_established() |
Call security_sctp_assoc_established() |
to set the peer label. |
| |
| If SCTP_SOCKET_TCP or peeled off
Expand All @@ -195,27 +196,27 @@ hooks with the SELinux specifics expanded below::
security_sctp_assoc_request()
security_sctp_bind_connect()
security_sctp_sk_clone()
security_inet_conn_established()
security_sctp_assoc_established()


security_sctp_assoc_request()
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Passes the ``@ep`` and ``@chunk->skb`` of the association INIT packet to the
Passes the ``@asoc`` and ``@chunk->skb`` of the association INIT packet to the
security module. Returns 0 on success, error on failure.
::

@ep - pointer to sctp endpoint structure.
@asoc - pointer to sctp association structure.
@skb - pointer to skbuff of association packet.

The security module performs the following operations:
IF this is the first association on ``@ep->base.sk``, then set the peer
IF this is the first association on ``@asoc->base.sk``, then set the peer
sid to that in ``@skb``. This will ensure there is only one peer sid
assigned to ``@ep->base.sk`` that may support multiple associations.
assigned to ``@asoc->base.sk`` that may support multiple associations.

ELSE validate the ``@ep->base.sk peer_sid`` against the ``@skb peer sid``
ELSE validate the ``@asoc->base.sk peer_sid`` against the ``@skb peer sid``
to determine whether the association should be allowed or denied.

Set the sctp ``@ep sid`` to socket's sid (from ``ep->base.sk``) with
Set the sctp ``@asoc sid`` to socket's sid (from ``asoc->base.sk``) with
MLS portion taken from ``@skb peer sid``. This will be used by SCTP
TCP style sockets and peeled off connections as they cause a new socket
to be generated.
Expand Down Expand Up @@ -259,21 +260,21 @@ security_sctp_sk_clone()
Called whenever a new socket is created by **accept**\(2) (i.e. a TCP style
socket) or when a socket is 'peeled off' e.g userspace calls
**sctp_peeloff**\(3). ``security_sctp_sk_clone()`` will set the new
sockets sid and peer sid to that contained in the ``@ep sid`` and
``@ep peer sid`` respectively.
sockets sid and peer sid to that contained in the ``@asoc sid`` and
``@asoc peer sid`` respectively.
::

@ep - pointer to current sctp endpoint structure.
@asoc - pointer to current sctp association structure.
@sk - pointer to current sock structure.
@sk - pointer to new sock structure.
@newsk - pointer to new sock structure.


security_inet_conn_established()
security_sctp_assoc_established()
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Called when a COOKIE ACK is received where it sets the connection's peer sid
to that in ``@skb``::

@sk - pointer to sock structure.
@asoc - pointer to sctp association structure.
@skb - pointer to skbuff of the COOKIE ACK packet.


Expand Down
6 changes: 4 additions & 2 deletions include/linux/lsm_hook_defs.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -329,12 +329,14 @@ LSM_HOOK(int, 0, tun_dev_create, void)
LSM_HOOK(int, 0, tun_dev_attach_queue, void *security)
LSM_HOOK(int, 0, tun_dev_attach, struct sock *sk, void *security)
LSM_HOOK(int, 0, tun_dev_open, void *security)
LSM_HOOK(int, 0, sctp_assoc_request, struct sctp_endpoint *ep,
LSM_HOOK(int, 0, sctp_assoc_request, struct sctp_association *asoc,
struct sk_buff *skb)
LSM_HOOK(int, 0, sctp_bind_connect, struct sock *sk, int optname,
struct sockaddr *address, int addrlen)
LSM_HOOK(void, LSM_RET_VOID, sctp_sk_clone, struct sctp_endpoint *ep,
LSM_HOOK(void, LSM_RET_VOID, sctp_sk_clone, struct sctp_association *asoc,
struct sock *sk, struct sock *newsk)
LSM_HOOK(void, LSM_RET_VOID, sctp_assoc_established, struct sctp_association *asoc,
struct sk_buff *skb)
#endif /* CONFIG_SECURITY_NETWORK */

#ifdef CONFIG_SECURITY_INFINIBAND
Expand Down
13 changes: 9 additions & 4 deletions include/linux/lsm_hooks.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1027,9 +1027,9 @@
* Security hooks for SCTP
*
* @sctp_assoc_request:
* Passes the @ep and @chunk->skb of the association INIT packet to
* Passes the @asoc and @chunk->skb of the association INIT packet to
* the security module.
* @ep pointer to sctp endpoint structure.
* @asoc pointer to sctp association structure.
* @skb pointer to skbuff of association packet.
* Return 0 on success, error on failure.
* @sctp_bind_connect:
Expand All @@ -1047,9 +1047,14 @@
* Called whenever a new socket is created by accept(2) (i.e. a TCP
* style socket) or when a socket is 'peeled off' e.g userspace
* calls sctp_peeloff(3).
* @ep pointer to current sctp endpoint structure.
* @asoc pointer to current sctp association structure.
* @sk pointer to current sock structure.
* @sk pointer to new sock structure.
* @newsk pointer to new sock structure.
* @sctp_assoc_established:
* Passes the @asoc and @chunk->skb of the association COOKIE_ACK packet
* to the security module.
* @asoc pointer to sctp association structure.
* @skb pointer to skbuff of association packet.
*
* Security hooks for Infiniband
*
Expand Down
17 changes: 12 additions & 5 deletions include/linux/security.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ struct xfrm_policy;
struct xfrm_state;
struct xfrm_user_sec_ctx;
struct seq_file;
struct sctp_endpoint;
struct sctp_association;

#ifdef CONFIG_MMU
extern unsigned long mmap_min_addr;
Expand Down Expand Up @@ -1425,11 +1425,13 @@ int security_tun_dev_create(void);
int security_tun_dev_attach_queue(void *security);
int security_tun_dev_attach(struct sock *sk, void *security);
int security_tun_dev_open(void *security);
int security_sctp_assoc_request(struct sctp_endpoint *ep, struct sk_buff *skb);
int security_sctp_assoc_request(struct sctp_association *asoc, struct sk_buff *skb);
int security_sctp_bind_connect(struct sock *sk, int optname,
struct sockaddr *address, int addrlen);
void security_sctp_sk_clone(struct sctp_endpoint *ep, struct sock *sk,
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,
Expand Down Expand Up @@ -1631,7 +1633,7 @@ static inline int security_tun_dev_open(void *security)
return 0;
}

static inline int security_sctp_assoc_request(struct sctp_endpoint *ep,
static inline int security_sctp_assoc_request(struct sctp_association *asoc,
struct sk_buff *skb)
{
return 0;
Expand All @@ -1644,11 +1646,16 @@ static inline int security_sctp_bind_connect(struct sock *sk, int optname,
return 0;
}

static inline void security_sctp_sk_clone(struct sctp_endpoint *ep,
static inline void security_sctp_sk_clone(struct sctp_association *asoc,
struct sock *sk,
struct sock *newsk)
{
}

static inline void security_sctp_assoc_established(struct sctp_association *asoc,
struct sk_buff *skb)
{
}
#endif /* CONFIG_SECURITY_NETWORK */

#ifdef CONFIG_SECURITY_INFINIBAND
Expand Down
20 changes: 10 additions & 10 deletions include/net/sctp/structs.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1355,16 +1355,6 @@ struct sctp_endpoint {
reconf_enable:1;

__u8 strreset_enable;

/* Security identifiers from incoming (INIT). These are set by
* security_sctp_assoc_request(). These will only be used by
* SCTP TCP type sockets and peeled off connections as they
* cause a new socket to be generated. security_sctp_sk_clone()
* will then plug these into the new socket.
*/

u32 secid;
u32 peer_secid;
};

/* Recover the outter endpoint structure. */
Expand Down Expand Up @@ -2104,6 +2094,16 @@ struct sctp_association {
__u64 abandoned_unsent[SCTP_PR_INDEX(MAX) + 1];
__u64 abandoned_sent[SCTP_PR_INDEX(MAX) + 1];

/* Security identifiers from incoming (INIT). These are set by
* security_sctp_assoc_request(). These will only be used by
* SCTP TCP type sockets and peeled off connections as they
* cause a new socket to be generated. security_sctp_sk_clone()
* will then plug these into the new socket.
*/

u32 secid;
u32 peer_secid;

struct rcu_head rcu;
};

Expand Down
31 changes: 18 additions & 13 deletions net/sctp/sm_statefuns.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -326,11 +326,6 @@ enum sctp_disposition sctp_sf_do_5_1B_init(struct net *net,
struct sctp_packet *packet;
int len;

/* Update socket peer label if first association. */
if (security_sctp_assoc_request((struct sctp_endpoint *)ep,
chunk->skb))
return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);

/* 6.10 Bundling
* An endpoint MUST NOT bundle INIT, INIT ACK or
* SHUTDOWN COMPLETE with any other chunks.
Expand Down Expand Up @@ -415,6 +410,12 @@ enum sctp_disposition sctp_sf_do_5_1B_init(struct net *net,
if (!new_asoc)
goto nomem;

/* Update socket peer label if first association. */
if (security_sctp_assoc_request(new_asoc, chunk->skb)) {
sctp_association_free(new_asoc);
return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
}

if (sctp_assoc_set_bind_addr_from_ep(new_asoc,
sctp_scope(sctp_source(chunk)),
GFP_ATOMIC) < 0)
Expand Down Expand Up @@ -780,6 +781,10 @@ 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.
*
Expand Down Expand Up @@ -941,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
*
Expand Down Expand Up @@ -1517,11 +1522,6 @@ static enum sctp_disposition sctp_sf_do_unexpected_init(
struct sctp_packet *packet;
int len;

/* Update socket peer label if first association. */
if (security_sctp_assoc_request((struct sctp_endpoint *)ep,
chunk->skb))
return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);

/* 6.10 Bundling
* An endpoint MUST NOT bundle INIT, INIT ACK or
* SHUTDOWN COMPLETE with any other chunks.
Expand Down Expand Up @@ -1594,6 +1594,12 @@ static enum sctp_disposition sctp_sf_do_unexpected_init(
if (!new_asoc)
goto nomem;

/* Update socket peer label if first association. */
if (security_sctp_assoc_request(new_asoc, chunk->skb)) {
sctp_association_free(new_asoc);
return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
}

if (sctp_assoc_set_bind_addr_from_ep(new_asoc,
sctp_scope(sctp_source(chunk)), GFP_ATOMIC) < 0)
goto nomem;
Expand Down Expand Up @@ -2255,8 +2261,7 @@ enum sctp_disposition sctp_sf_do_5_2_4_dupcook(
}

/* Update socket peer label if first association. */
if (security_sctp_assoc_request((struct sctp_endpoint *)ep,
chunk->skb)) {
if (security_sctp_assoc_request(new_asoc, chunk->skb)) {
sctp_association_free(new_asoc);
return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
}
Expand Down
5 changes: 2 additions & 3 deletions net/sctp/socket.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9412,7 +9412,6 @@ void sctp_copy_sock(struct sock *newsk, struct sock *sk,
struct inet_sock *inet = inet_sk(sk);
struct inet_sock *newinet;
struct sctp_sock *sp = sctp_sk(sk);
struct sctp_endpoint *ep = sp->ep;

newsk->sk_type = sk->sk_type;
newsk->sk_bound_dev_if = sk->sk_bound_dev_if;
Expand Down Expand Up @@ -9457,9 +9456,9 @@ void sctp_copy_sock(struct sock *newsk, struct sock *sk,
net_enable_timestamp();

/* Set newsk security attributes from original sk and connection
* security attribute from ep.
* security attribute from asoc.
*/
security_sctp_sk_clone(ep, sk, newsk);
security_sctp_sk_clone(asoc, sk, newsk);
}

static inline void sctp_copy_descendant(struct sock *sk_to,
Expand Down
Loading

0 comments on commit 2bd080b

Please sign in to comment.