diff mbox series

[v2] smack: unix sockets: fix accept()ed socket label

Message ID 20240616224441.3379-1-andreev@swemel.ru (mailing list archive)
State Handled Elsewhere
Headers show
Series [v2] smack: unix sockets: fix accept()ed socket label | expand

Commit Message

Konstantin Andreev June 16, 2024, 10:44 p.m. UTC
When a process accept()s connection from a unix socket
(either stream or seqpacket)
it gets the socket with the label of the connecting process.

For example, if a connecting process has a label 'foo',
the accept()ed socket will also have 'in' and 'out' labels 'foo',
regardless of the label of the listener process.

This is because kernel creates unix child sockets
in the context of the connecting process.

I do not see any obvious way for the listener to abuse
alien labels coming with the new socket, but,
to be on the safe side, it's better fix new socket labels.

Signed-off-by: Konstantin Andreev <andreev@swemel.ru>
---
v2: fixed comment style
The patch is against `next' branch at https://github.com/cschaufler/smack-next
The patch does not hurt `Smack kernel test suite' https://github.com/smack-team/smack-testsuite.git

 security/smack/smack_lsm.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Comments

Casey Schaufler June 20, 2024, 5:42 p.m. UTC | #1
On 6/16/2024 3:44 PM, Konstantin Andreev wrote:
> When a process accept()s connection from a unix socket
> (either stream or seqpacket)
> it gets the socket with the label of the connecting process.
>
> For example, if a connecting process has a label 'foo',
> the accept()ed socket will also have 'in' and 'out' labels 'foo',
> regardless of the label of the listener process.
>
> This is because kernel creates unix child sockets
> in the context of the connecting process.
>
> I do not see any obvious way for the listener to abuse
> alien labels coming with the new socket, but,
> to be on the safe side, it's better fix new socket labels.
>
> Signed-off-by: Konstantin Andreev <andreev@swemel.ru>

Thanks. I have taken this in Smack next.

> ---
> v2: fixed comment style
> The patch is against `next' branch at https://github.com/cschaufler/smack-next
> The patch does not hurt `Smack kernel test suite' https://github.com/smack-team/smack-testsuite.git
>
>  security/smack/smack_lsm.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 56e02cc5c44d..d0d484c1599a 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -3846,12 +3846,18 @@ static int smack_unix_stream_connect(struct sock *sock,
>  		}
>  	}
>  
> -	/*
> -	 * Cross reference the peer labels for SO_PEERSEC.
> -	 */
>  	if (rc == 0) {
> +		/*
> +		 * Cross reference the peer labels for SO_PEERSEC.
> +		 */
>  		nsp->smk_packet = ssp->smk_out;
>  		ssp->smk_packet = osp->smk_out;
> +
> +		/*
> +		 * new/child/established socket must inherit listening socket labels
> +		 */
> +		nsp->smk_out = osp->smk_out;
> +		nsp->smk_in  = osp->smk_in;
>  	}
>  
>  	return rc;
diff mbox series

Patch

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 56e02cc5c44d..d0d484c1599a 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3846,12 +3846,18 @@  static int smack_unix_stream_connect(struct sock *sock,
 		}
 	}
 
-	/*
-	 * Cross reference the peer labels for SO_PEERSEC.
-	 */
 	if (rc == 0) {
+		/*
+		 * Cross reference the peer labels for SO_PEERSEC.
+		 */
 		nsp->smk_packet = ssp->smk_out;
 		ssp->smk_packet = osp->smk_out;
+
+		/*
+		 * new/child/established socket must inherit listening socket labels
+		 */
+		nsp->smk_out = osp->smk_out;
+		nsp->smk_in  = osp->smk_in;
 	}
 
 	return rc;