[PATCH] smack: tcp: ipv4, fix incorrect labeling and unauthorized writes

[Casey Schaufler]

On 6/2/2024 5:06 AM, Konstantin Andreev wrote:
> Currently, Smack mirrors the label of incoming tcp/ipv4 connections:
> when a label 'foo' connects to a label 'bar' with tcp/ipv4,
> 'foo' always gets 'foo' in returned ipv4 packets. So,
> 1) returned packets are incorrectly labeled ('foo' instead of 'bar')
> 2) 'bar' can write to 'foo' without being authorized to write.
>
> Here is a scenario how to see this:
>
> * Take two machines, let's call them C and S,
>   with active Smack in the default state
>   (no settings, no rules, no labeled hosts, only builtin labels)
>
> * At S, add Smack rule 'foo bar w'
>   (labels 'foo' and 'bar' are instantiated at S at this moment)
>
> * At S, at label 'bar', launch a program
>   that listens for incoming tcp/ipv4 connections
>
> * From C, at label 'foo', connect to the listener at S.
>   (label 'foo' is instantiated at C at this moment)
>   Connection succeedes and works.
>
> * Send some data in both directions.
> * Collect network traffic of this connection.
>
> All packets in both directions are labeled with the CIPSO
> of the label 'foo'. Hence, label 'bar' writes to 'foo' without
> being authorized, and even without ever being known at C.
>
> If anybody cares: exactly the same happens with DCCP.
>
> This behavior 1st manifested in release 2.6.29.4 (see Fixes below)
> and it looks unintentional. At least, no explanation was provided.
>
> I changed returned packes label into the 'bar',
> to bring it into line with the Smack documentation claims.
>
> Fixes: 6c3823bc3abf ("smack: Set the proper NetLabel security
> attributes for connection requests")
> Signed-off-by: Konstantin Andreev <andreev at swemel.ru>

I've verified the issue and your fix. Responding with the outbound
label from the socket is the correct thing to do. It introduces an
inconvenient situation, where the connection hangs and times out on
the client end. The connection isn't refused, but the reply never
gets to the client socket because access is denied. There needs to
be a change on the receiving end to address this part of the handshake.

Nonetheless, I'm taking this in smack-next. Thank you.

> ---
> The patch is against branch smack-for-6.9 at
> https://github.com/cschaufler/smack-next
> The hash 6c3823bc3abf is against
> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
> The commit 6c3823bc3abf is based on 07feee8f812f, which is available
> at `smack-next' repo,
> but 6c3823bc3abf has much narrower scope, so I point at it, not
> 07feee8f812f.
> This fix passed `Smack kernel test suite'
> https://github.com/smack-team/smack-testsuite.git
>
>  security/smack/smack_lsm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index b18b50232947..d836ca61e081 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -4428,7 +4428,7 @@ static int smack_inet_conn_request(const struct
> sock *sk, struct sk_buff *skb,
>      rcu_read_unlock();
>
>      if (hskp == NULL)
> -        rc = netlbl_req_setattr(req, &skp->smk_netlabel);
> +        rc = netlbl_req_setattr(req, &ssp->smk_out->smk_netlabel);
>      else
>          netlbl_req_delattr(req);
>
> -- 
> 2.40.1