netdev - Re: [PATCH] tipc: fix size validations for the MSG

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: <3def567a-5f99-970d-f9e4-3b9426809b16@redhat.com>
Date:   Mon, 25 Oct 2021 20:53:15 -0400
From:   Jon Maloy <jmaloy@...hat.com>
To:     Greg KH <gregkh@...uxfoundation.org>, netdev@...r.kernel.org
Cc:     Jakub Kicinski <kuba@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Tuong Lien <tuong.t.lien@...tech.com.au>,
        Max VA <maxv@...tinelone.com>,
        Ying Xue <ying.xue@...driver.com>
Subject: Re: [PATCH] tipc: fix size validations for the MSG_CRYPTO type

Acked-by: Jon Maloy <jmaloy@...hat.com>

On 10/25/21 11:31, Greg KH wrote:
> From: Max VA <maxv@...tinelone.com>
>
> The function tipc_crypto_key_rcv is used to parse MSG_CRYPTO messages
> to receive keys from other nodes in the cluster in order to decrypt any
> further messages from them.
> This patch verifies that any supplied sizes in the message body are
> valid for the received message.
>
> Fixes: 1ef6f7c9390f ("tipc: add automatic session key exchange")
> Signed-off-by: Max VA <maxv@...tinelone.com>
> Acked-by: Ying Xue <ying.xue@...driver.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> ---
>   net/tipc/crypto.c | 32 +++++++++++++++++++++-----------
>   1 file changed, 21 insertions(+), 11 deletions(-)
>
> Max's email system doesn't seem to be able to send non-attachment
> patches out, so I'm forwarding this on for him.  It's already acked by
> one of the tipc maintainers.
>
> diff --git a/net/tipc/crypto.c b/net/tipc/crypto.c
> index c9391d38de85..dc60c32bb70d 100644
> --- a/net/tipc/crypto.c
> +++ b/net/tipc/crypto.c
> @@ -2285,43 +2285,53 @@ static bool tipc_crypto_key_rcv(struct tipc_crypto *rx, struct tipc_msg *hdr)
>   	u16 key_gen = msg_key_gen(hdr);
>   	u16 size = msg_data_sz(hdr);
>   	u8 *data = msg_data(hdr);
> +	unsigned int keylen;
> +
> +	/* Verify whether the size can exist in the packet */
> +	if (unlikely(size < sizeof(struct tipc_aead_key) + TIPC_AEAD_KEYLEN_MIN)) {
> +		pr_debug("%s: message data size is too small\n", rx->name);
> +		goto exit;
> +	}
> +
> +	keylen = ntohl(*((__be32 *)(data + TIPC_AEAD_ALG_NAME)));
> +
> +	/* Verify the supplied size values */
> +	if (unlikely(size != keylen + sizeof(struct tipc_aead_key) ||
> +		     keylen > TIPC_AEAD_KEY_SIZE_MAX)) {
> +		pr_debug("%s: invalid MSG_CRYPTO key size\n", rx->name);
> +		goto exit;
> +	}
>   
>   	spin_lock(&rx->lock);
>   	if (unlikely(rx->skey || (key_gen == rx->key_gen && rx->key.keys))) {
>   		pr_err("%s: key existed <%p>, gen %d vs %d\n", rx->name,
>   		       rx->skey, key_gen, rx->key_gen);
> -		goto exit;
> +		goto exit_unlock;
>   	}
>   
>   	/* Allocate memory for the key */
>   	skey = kmalloc(size, GFP_ATOMIC);
>   	if (unlikely(!skey)) {
>   		pr_err("%s: unable to allocate memory for skey\n", rx->name);
> -		goto exit;
> +		goto exit_unlock;
>   	}
>   
>   	/* Copy key from msg data */
> -	skey->keylen = ntohl(*((__be32 *)(data + TIPC_AEAD_ALG_NAME)));
> +	skey->keylen = keylen;
>   	memcpy(skey->alg_name, data, TIPC_AEAD_ALG_NAME);
>   	memcpy(skey->key, data + TIPC_AEAD_ALG_NAME + sizeof(__be32),
>   	       skey->keylen);
>   
> -	/* Sanity check */
> -	if (unlikely(size != tipc_aead_key_size(skey))) {
> -		kfree(skey);
> -		skey = NULL;
> -		goto exit;
> -	}
> -
>   	rx->key_gen = key_gen;
>   	rx->skey_mode = msg_key_mode(hdr);
>   	rx->skey = skey;
>   	rx->nokey = 0;
>   	mb(); /* for nokey flag */
>   
> -exit:
> +exit_unlock:
>   	spin_unlock(&rx->lock);
>   
> +exit:
>   	/* Schedule the key attaching on this crypto */
>   	if (likely(skey && queue_delayed_work(tx->wq, &rx->work, 0)))
>   		return true;