Skip to content

Commit 0a90729

Browse files
committed
Merge tag 'selinux-pr-20211112' of git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux
Pull selinux fixes from Paul Moore: "Unfortunately I need to request a revert for two LSM/SELinux patches that came in via the network tree. The two patches in question add a new SCTP/LSM hook as well as an SELinux implementation of that LSM hook. The short version of "why?" is in the commit description of the revert patch, but I'll copy-n-paste the important bits below to save some time for the curious: ... Unfortunately these two patches were merged without proper review (the Reviewed-by and Tested-by tags from Richard Haines were for previous revisions of these patches that were significantly different) and there are outstanding objections from the SELinux maintainers regarding these patches. Work is currently ongoing to correct the problems identified in the reverted patches, as well as others that have come up during review, but it is unclear at this point in time when that work will be ready for inclusion in the mainline kernel. In the interest of not keeping objectionable code in the kernel for multiple weeks, and potentially a kernel release, we are reverting the two problematic patches. As usual with these things there is plenty of context to go with this and I'll try to do my best to provide that now. This effort started with a report of SCTP client side peel-offs not working correctly with SELinux, Ondrej Mosnacek put forth a patch which he believed properly addressed the problem but upon review by the netdev folks Xin Long described some additional issues and submitted an improved patchset for review. The SELinux folks reviewed Xin Long's initial patchset and suggested some changes which resulted in a second patchset (v2) from Xin Long; this is the patchset that is currently in your tree. Unfortunately this v2 patchset from Xin Long was merged before it had spent even just 24 hours on the mailing lists during the early days of the merge window, a time when many of us were busy doing verification of the newly released v5.15 kernel as well final review and testing of our v5.16 pull requests. Making matters worse, upon reviewing the v2 patchset there were both changes which were found objectionable by SELinux standards as well as additional outstanding SCTP/SELinux interaction problems. At this point we did two things: resumed working on a better fix for the SCTP/SELinux issue(s) - thank you Ondrej - and we asked the networking folks to revert the v2 patchset. The revert request was obviously rejected, but at the time I believed it was just going to be an issue for linux-next; I wasn't expecting something this significant that was merged into the networking tree during the merge window to make it into your tree in the same window, yet as of last night that is exactly what happened. While we continue to try and resolve the SCTP/SELinux problem I am asking once again to revert the v2 patches and not ship the current security_sctp_assoc_established() hook in a v5.16-rcX kernel. If I was confident that we could solve these issues in a week, maybe two, I would refrain from asking for the revert but our current estimate is for a minimum of two weeks for the next patch revision. With the likelihood of additional delays due to normal patch review follow-up and/or holidays it seems to me that the safest course of action is to revert the patch both to try and keep some objectionable code out of a release kernel and limit the chances of any new breakages from such a change. While the SCTP/SELinux code in v5.15 and earlier has problems, they are known problems, and I'd like to try and avoid creating new and different problems while we work to fix things properly. One final thing to mention: Xin Long's v2 patchset consisted of four patches, yet this revert is for only the last two. We see the first two patches as good, reasonable, and not likely to cause an issue. In an attempt to create a cleaner revert patch we suggest leaving the first two patches in the tree as they are currently" * tag 'selinux-pr-20211112' of git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux: net,lsm,selinux: revert the security_sctp_assoc_established() hook
2 parents 7c3737c + 32a370a commit 0a90729

File tree

7 files changed

+14
-45
lines changed

7 files changed

+14
-45
lines changed

Documentation/security/SCTP.rst

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,10 @@ For security module support, three SCTP specific hooks have been implemented::
1515
security_sctp_assoc_request()
1616
security_sctp_bind_connect()
1717
security_sctp_sk_clone()
18-
security_sctp_assoc_established()
18+
19+
Also the following security hook has been utilised::
20+
21+
security_inet_conn_established()
1922

2023
The usage of these hooks are described below with the SELinux implementation
2124
described in the `SCTP SELinux Support`_ chapter.
@@ -119,20 +122,19 @@ calls **sctp_peeloff**\(3).
119122
@newsk - pointer to new sock structure.
120123

121124

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

127-
@asoc - pointer to sctp association structure.
129+
@sk - pointer to sock structure.
128130
@skb - pointer to skbuff of the COOKIE ACK packet.
129131

130132

131133
Security Hooks used for Association Establishment
132134
-------------------------------------------------
133135

134136
The following diagram shows the use of ``security_sctp_bind_connect()``,
135-
``security_sctp_assoc_request()``, ``security_sctp_assoc_established()`` when
137+
``security_sctp_assoc_request()``, ``security_inet_conn_established()`` when
136138
establishing an association.
137139
::
138140

@@ -170,7 +172,7 @@ establishing an association.
170172
<------------------------------------------- COOKIE ACK
171173
| |
172174
sctp_sf_do_5_1E_ca |
173-
Call security_sctp_assoc_established() |
175+
Call security_inet_conn_established() |
174176
to set the peer label. |
175177
| |
176178
| If SCTP_SOCKET_TCP or peeled off
@@ -196,7 +198,7 @@ hooks with the SELinux specifics expanded below::
196198
security_sctp_assoc_request()
197199
security_sctp_bind_connect()
198200
security_sctp_sk_clone()
199-
security_sctp_assoc_established()
201+
security_inet_conn_established()
200202

201203

202204
security_sctp_assoc_request()
@@ -269,12 +271,12 @@ sockets sid and peer sid to that contained in the ``@asoc sid`` and
269271
@newsk - pointer to new sock structure.
270272

271273

272-
security_sctp_assoc_established()
274+
security_inet_conn_established()
273275
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
274276
Called when a COOKIE ACK is received where it sets the connection's peer sid
275277
to that in ``@skb``::
276278

277-
@asoc - pointer to sctp association structure.
279+
@sk - pointer to sock structure.
278280
@skb - pointer to skbuff of the COOKIE ACK packet.
279281

280282

include/linux/lsm_hook_defs.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -335,8 +335,6 @@ LSM_HOOK(int, 0, sctp_bind_connect, struct sock *sk, int optname,
335335
struct sockaddr *address, int addrlen)
336336
LSM_HOOK(void, LSM_RET_VOID, sctp_sk_clone, struct sctp_association *asoc,
337337
struct sock *sk, struct sock *newsk)
338-
LSM_HOOK(void, LSM_RET_VOID, sctp_assoc_established, struct sctp_association *asoc,
339-
struct sk_buff *skb)
340338
#endif /* CONFIG_SECURITY_NETWORK */
341339

342340
#ifdef CONFIG_SECURITY_INFINIBAND

include/linux/lsm_hooks.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1050,11 +1050,6 @@
10501050
* @asoc pointer to current sctp association structure.
10511051
* @sk pointer to current sock structure.
10521052
* @newsk pointer to new sock structure.
1053-
* @sctp_assoc_established:
1054-
* Passes the @asoc and @chunk->skb of the association COOKIE_ACK packet
1055-
* to the security module.
1056-
* @asoc pointer to sctp association structure.
1057-
* @skb pointer to skbuff of association packet.
10581053
*
10591054
* Security hooks for Infiniband
10601055
*

include/linux/security.h

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1430,8 +1430,6 @@ int security_sctp_bind_connect(struct sock *sk, int optname,
14301430
struct sockaddr *address, int addrlen);
14311431
void security_sctp_sk_clone(struct sctp_association *asoc, struct sock *sk,
14321432
struct sock *newsk);
1433-
void security_sctp_assoc_established(struct sctp_association *asoc,
1434-
struct sk_buff *skb);
14351433

14361434
#else /* CONFIG_SECURITY_NETWORK */
14371435
static inline int security_unix_stream_connect(struct sock *sock,
@@ -1651,11 +1649,6 @@ static inline void security_sctp_sk_clone(struct sctp_association *asoc,
16511649
struct sock *newsk)
16521650
{
16531651
}
1654-
1655-
static inline void security_sctp_assoc_established(struct sctp_association *asoc,
1656-
struct sk_buff *skb)
1657-
{
1658-
}
16591652
#endif /* CONFIG_SECURITY_NETWORK */
16601653

16611654
#ifdef CONFIG_SECURITY_INFINIBAND

net/sctp/sm_statefuns.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -946,7 +946,7 @@ enum sctp_disposition sctp_sf_do_5_1E_ca(struct net *net,
946946
sctp_add_cmd_sf(commands, SCTP_CMD_INIT_COUNTER_RESET, SCTP_NULL());
947947

948948
/* Set peer label for connection. */
949-
security_sctp_assoc_established((struct sctp_association *)asoc, chunk->skb);
949+
security_inet_conn_established(ep->base.sk, chunk->skb);
950950

951951
/* RFC 2960 5.1 Normal Establishment of an Association
952952
*

security/security.c

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2388,13 +2388,6 @@ void security_sctp_sk_clone(struct sctp_association *asoc, struct sock *sk,
23882388
}
23892389
EXPORT_SYMBOL(security_sctp_sk_clone);
23902390

2391-
void security_sctp_assoc_established(struct sctp_association *asoc,
2392-
struct sk_buff *skb)
2393-
{
2394-
call_void_hook(sctp_assoc_established, asoc, skb);
2395-
}
2396-
EXPORT_SYMBOL(security_sctp_assoc_established);
2397-
23982391
#endif /* CONFIG_SECURITY_NETWORK */
23992392

24002393
#ifdef CONFIG_SECURITY_INFINIBAND

security/selinux/hooks.c

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5502,8 +5502,7 @@ static void selinux_sctp_sk_clone(struct sctp_association *asoc, struct sock *sk
55025502
if (!selinux_policycap_extsockclass())
55035503
return selinux_sk_clone_security(sk, newsk);
55045504

5505-
if (asoc->secid != SECSID_WILD)
5506-
newsksec->sid = asoc->secid;
5505+
newsksec->sid = asoc->secid;
55075506
newsksec->peer_sid = asoc->peer_secid;
55085507
newsksec->sclass = sksec->sclass;
55095508
selinux_netlbl_sctp_sk_clone(sk, newsk);
@@ -5559,16 +5558,6 @@ static void selinux_inet_conn_established(struct sock *sk, struct sk_buff *skb)
55595558
selinux_skb_peerlbl_sid(skb, family, &sksec->peer_sid);
55605559
}
55615560

5562-
static void selinux_sctp_assoc_established(struct sctp_association *asoc,
5563-
struct sk_buff *skb)
5564-
{
5565-
struct sk_security_struct *sksec = asoc->base.sk->sk_security;
5566-
5567-
selinux_inet_conn_established(asoc->base.sk, skb);
5568-
asoc->peer_secid = sksec->peer_sid;
5569-
asoc->secid = SECSID_WILD;
5570-
}
5571-
55725561
static int selinux_secmark_relabel_packet(u32 sid)
55735562
{
55745563
const struct task_security_struct *__tsec;
@@ -7239,7 +7228,6 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
72397228
LSM_HOOK_INIT(sctp_assoc_request, selinux_sctp_assoc_request),
72407229
LSM_HOOK_INIT(sctp_sk_clone, selinux_sctp_sk_clone),
72417230
LSM_HOOK_INIT(sctp_bind_connect, selinux_sctp_bind_connect),
7242-
LSM_HOOK_INIT(sctp_assoc_established, selinux_sctp_assoc_established),
72437231
LSM_HOOK_INIT(inet_conn_request, selinux_inet_conn_request),
72447232
LSM_HOOK_INIT(inet_csk_clone, selinux_inet_csk_clone),
72457233
LSM_HOOK_INIT(inet_conn_established, selinux_inet_conn_established),

0 commit comments

Comments
 (0)