From: Chuck Lever <[email protected]>
Anna says:
> KASAN reports [...] a slab-out-of-bounds in gss_krb5_checksum(),
> and it can cause my client to panic when running cthon basic
> tests with krb5p.
> Running faddr2line gives me:
>
> gss_krb5_checksum+0x4b6/0x630:
> ahash_request_free at
> /home/anna/Programs/linux-nfs.git/./include/crypto/hash.h:619
> (inlined by) gss_krb5_checksum at
> /home/anna/Programs/linux-nfs.git/net/sunrpc/auth_gss/gss_krb5_crypto.c:358
My diagnosis is that the memcpy() at the end of gss_krb5_checksum()
reads past the end of the buffer containing the checksum data
because the callers have ignored gss_krb5_checksum()'s API contract:
* Caller provides the truncation length of the output token (h) in
* cksumout.len.
Instead they provide the fixed length of the hmac buffer. This
length happens to be larger than the value returned by
crypto_ahash_digestsize().
Change these errant callers to work like krb5_etm_{en,de}crypt().
As a defensive measure, bound the length of the byte copy at the
end of gss_krb5_checksum().
Kunit sez:
Testing complete. Ran 68 tests: passed: 68
Elapsed time: 81.680s total, 5.875s configuring, 75.610s building, 0.103s running
Reported-by: Anna Schumaker <[email protected]>
Fixes: 8270dbfcebea ("SUNRPC: Obscure Kerberos integrity keys")
Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/auth_gss/gss_krb5_crypto.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/net/sunrpc/auth_gss/gss_krb5_crypto.c b/net/sunrpc/auth_gss/gss_krb5_crypto.c
index 6c7c52eeed4f..212c5d57465a 100644
--- a/net/sunrpc/auth_gss/gss_krb5_crypto.c
+++ b/net/sunrpc/auth_gss/gss_krb5_crypto.c
@@ -353,7 +353,9 @@ gss_krb5_checksum(struct crypto_ahash *tfm, char *header, int hdrlen,
err = crypto_ahash_final(req);
if (err)
goto out_free_ahash;
- memcpy(cksumout->data, checksumdata, cksumout->len);
+
+ memcpy(cksumout->data, checksumdata,
+ min_t(int, cksumout->len, crypto_ahash_digestsize(tfm)));
out_free_ahash:
ahash_request_free(req);
@@ -809,8 +811,7 @@ gss_krb5_aes_encrypt(struct krb5_ctx *kctx, u32 offset,
buf->tail[0].iov_len += GSS_KRB5_TOK_HDR_LEN;
buf->len += GSS_KRB5_TOK_HDR_LEN;
- /* Do the HMAC */
- hmac.len = GSS_KRB5_MAX_CKSUM_LEN;
+ hmac.len = kctx->gk5e->cksumlength;
hmac.data = buf->tail[0].iov_base + buf->tail[0].iov_len;
/*
@@ -873,8 +874,7 @@ gss_krb5_aes_decrypt(struct krb5_ctx *kctx, u32 offset, u32 len,
if (ret)
goto out_err;
- /* Calculate our hmac over the plaintext data */
- our_hmac_obj.len = sizeof(our_hmac);
+ our_hmac_obj.len = kctx->gk5e->cksumlength;
our_hmac_obj.data = our_hmac;
ret = gss_krb5_checksum(ahash, NULL, 0, &subbuf, 0, &our_hmac_obj);
if (ret)
On Wed, Mar 22, 2023 at 3:17 PM Chuck Lever <[email protected]> wrote:
>
> From: Chuck Lever <[email protected]>
>
> Anna says:
> > KASAN reports [...] a slab-out-of-bounds in gss_krb5_checksum(),
> > and it can cause my client to panic when running cthon basic
> > tests with krb5p.
>
> > Running faddr2line gives me:
> >
> > gss_krb5_checksum+0x4b6/0x630:
> > ahash_request_free at
> > /home/anna/Programs/linux-nfs.git/./include/crypto/hash.h:619
> > (inlined by) gss_krb5_checksum at
> > /home/anna/Programs/linux-nfs.git/net/sunrpc/auth_gss/gss_krb5_crypto.c:358
>
> My diagnosis is that the memcpy() at the end of gss_krb5_checksum()
> reads past the end of the buffer containing the checksum data
> because the callers have ignored gss_krb5_checksum()'s API contract:
>
> * Caller provides the truncation length of the output token (h) in
> * cksumout.len.
>
> Instead they provide the fixed length of the hmac buffer. This
> length happens to be larger than the value returned by
> crypto_ahash_digestsize().
>
> Change these errant callers to work like krb5_etm_{en,de}crypt().
> As a defensive measure, bound the length of the byte copy at the
> end of gss_krb5_checksum().
>
> Kunit sez:
> Testing complete. Ran 68 tests: passed: 68
> Elapsed time: 81.680s total, 5.875s configuring, 75.610s building, 0.103s running
>
> Reported-by: Anna Schumaker <[email protected]>
> Fixes: 8270dbfcebea ("SUNRPC: Obscure Kerberos integrity keys")
> Signed-off-by: Chuck Lever <[email protected]>
This patch fixed the issue for me, thanks! Are you going to submit it
with a future 6.3-rc pull request, or should I?
Anna
> ---
> net/sunrpc/auth_gss/gss_krb5_crypto.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/net/sunrpc/auth_gss/gss_krb5_crypto.c b/net/sunrpc/auth_gss/gss_krb5_crypto.c
> index 6c7c52eeed4f..212c5d57465a 100644
> --- a/net/sunrpc/auth_gss/gss_krb5_crypto.c
> +++ b/net/sunrpc/auth_gss/gss_krb5_crypto.c
> @@ -353,7 +353,9 @@ gss_krb5_checksum(struct crypto_ahash *tfm, char *header, int hdrlen,
> err = crypto_ahash_final(req);
> if (err)
> goto out_free_ahash;
> - memcpy(cksumout->data, checksumdata, cksumout->len);
> +
> + memcpy(cksumout->data, checksumdata,
> + min_t(int, cksumout->len, crypto_ahash_digestsize(tfm)));
>
> out_free_ahash:
> ahash_request_free(req);
> @@ -809,8 +811,7 @@ gss_krb5_aes_encrypt(struct krb5_ctx *kctx, u32 offset,
> buf->tail[0].iov_len += GSS_KRB5_TOK_HDR_LEN;
> buf->len += GSS_KRB5_TOK_HDR_LEN;
>
> - /* Do the HMAC */
> - hmac.len = GSS_KRB5_MAX_CKSUM_LEN;
> + hmac.len = kctx->gk5e->cksumlength;
> hmac.data = buf->tail[0].iov_base + buf->tail[0].iov_len;
>
> /*
> @@ -873,8 +874,7 @@ gss_krb5_aes_decrypt(struct krb5_ctx *kctx, u32 offset, u32 len,
> if (ret)
> goto out_err;
>
> - /* Calculate our hmac over the plaintext data */
> - our_hmac_obj.len = sizeof(our_hmac);
> + our_hmac_obj.len = kctx->gk5e->cksumlength;
> our_hmac_obj.data = our_hmac;
> ret = gss_krb5_checksum(ahash, NULL, 0, &subbuf, 0, &our_hmac_obj);
> if (ret)
>
>
> On Mar 22, 2023, at 4:55 PM, Anna Schumaker <[email protected]> wrote:
>
> On Wed, Mar 22, 2023 at 3:17 PM Chuck Lever <[email protected]> wrote:
>>
>> From: Chuck Lever <[email protected]>
>>
>> Anna says:
>>> KASAN reports [...] a slab-out-of-bounds in gss_krb5_checksum(),
>>> and it can cause my client to panic when running cthon basic
>>> tests with krb5p.
>>
>>> Running faddr2line gives me:
>>>
>>> gss_krb5_checksum+0x4b6/0x630:
>>> ahash_request_free at
>>> /home/anna/Programs/linux-nfs.git/./include/crypto/hash.h:619
>>> (inlined by) gss_krb5_checksum at
>>> /home/anna/Programs/linux-nfs.git/net/sunrpc/auth_gss/gss_krb5_crypto.c:358
>>
>> My diagnosis is that the memcpy() at the end of gss_krb5_checksum()
>> reads past the end of the buffer containing the checksum data
>> because the callers have ignored gss_krb5_checksum()'s API contract:
>>
>> * Caller provides the truncation length of the output token (h) in
>> * cksumout.len.
>>
>> Instead they provide the fixed length of the hmac buffer. This
>> length happens to be larger than the value returned by
>> crypto_ahash_digestsize().
>>
>> Change these errant callers to work like krb5_etm_{en,de}crypt().
>> As a defensive measure, bound the length of the byte copy at the
>> end of gss_krb5_checksum().
>>
>> Kunit sez:
>> Testing complete. Ran 68 tests: passed: 68
>> Elapsed time: 81.680s total, 5.875s configuring, 75.610s building, 0.103s running
>>
>> Reported-by: Anna Schumaker <[email protected]>
>> Fixes: 8270dbfcebea ("SUNRPC: Obscure Kerberos integrity keys")
>> Signed-off-by: Chuck Lever <[email protected]>
>
> This patch fixed the issue for me, thanks! Are you going to submit it
> with a future 6.3-rc pull request, or should I?
I'll queue it in nfsd-fixes.
> Anna
>
>> ---
>> net/sunrpc/auth_gss/gss_krb5_crypto.c | 10 +++++-----
>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/net/sunrpc/auth_gss/gss_krb5_crypto.c b/net/sunrpc/auth_gss/gss_krb5_crypto.c
>> index 6c7c52eeed4f..212c5d57465a 100644
>> --- a/net/sunrpc/auth_gss/gss_krb5_crypto.c
>> +++ b/net/sunrpc/auth_gss/gss_krb5_crypto.c
>> @@ -353,7 +353,9 @@ gss_krb5_checksum(struct crypto_ahash *tfm, char *header, int hdrlen,
>> err = crypto_ahash_final(req);
>> if (err)
>> goto out_free_ahash;
>> - memcpy(cksumout->data, checksumdata, cksumout->len);
>> +
>> + memcpy(cksumout->data, checksumdata,
>> + min_t(int, cksumout->len, crypto_ahash_digestsize(tfm)));
>>
>> out_free_ahash:
>> ahash_request_free(req);
>> @@ -809,8 +811,7 @@ gss_krb5_aes_encrypt(struct krb5_ctx *kctx, u32 offset,
>> buf->tail[0].iov_len += GSS_KRB5_TOK_HDR_LEN;
>> buf->len += GSS_KRB5_TOK_HDR_LEN;
>>
>> - /* Do the HMAC */
>> - hmac.len = GSS_KRB5_MAX_CKSUM_LEN;
>> + hmac.len = kctx->gk5e->cksumlength;
>> hmac.data = buf->tail[0].iov_base + buf->tail[0].iov_len;
>>
>> /*
>> @@ -873,8 +874,7 @@ gss_krb5_aes_decrypt(struct krb5_ctx *kctx, u32 offset, u32 len,
>> if (ret)
>> goto out_err;
>>
>> - /* Calculate our hmac over the plaintext data */
>> - our_hmac_obj.len = sizeof(our_hmac);
>> + our_hmac_obj.len = kctx->gk5e->cksumlength;
>> our_hmac_obj.data = our_hmac;
>> ret = gss_krb5_checksum(ahash, NULL, 0, &subbuf, 0, &our_hmac_obj);
>> if (ret)
--
Chuck Lever
On Wed, Mar 22, 2023 at 4:59 PM Chuck Lever III <[email protected]> wrote:
>
>
>
> > On Mar 22, 2023, at 4:55 PM, Anna Schumaker <[email protected]> wrote:
> >
> > On Wed, Mar 22, 2023 at 3:17 PM Chuck Lever <[email protected]> wrote:
> >>
> >> From: Chuck Lever <[email protected]>
> >>
> >> Anna says:
> >>> KASAN reports [...] a slab-out-of-bounds in gss_krb5_checksum(),
> >>> and it can cause my client to panic when running cthon basic
> >>> tests with krb5p.
> >>
> >>> Running faddr2line gives me:
> >>>
> >>> gss_krb5_checksum+0x4b6/0x630:
> >>> ahash_request_free at
> >>> /home/anna/Programs/linux-nfs.git/./include/crypto/hash.h:619
> >>> (inlined by) gss_krb5_checksum at
> >>> /home/anna/Programs/linux-nfs.git/net/sunrpc/auth_gss/gss_krb5_crypto.c:358
> >>
> >> My diagnosis is that the memcpy() at the end of gss_krb5_checksum()
> >> reads past the end of the buffer containing the checksum data
> >> because the callers have ignored gss_krb5_checksum()'s API contract:
> >>
> >> * Caller provides the truncation length of the output token (h) in
> >> * cksumout.len.
> >>
> >> Instead they provide the fixed length of the hmac buffer. This
> >> length happens to be larger than the value returned by
> >> crypto_ahash_digestsize().
> >>
> >> Change these errant callers to work like krb5_etm_{en,de}crypt().
> >> As a defensive measure, bound the length of the byte copy at the
> >> end of gss_krb5_checksum().
> >>
> >> Kunit sez:
> >> Testing complete. Ran 68 tests: passed: 68
> >> Elapsed time: 81.680s total, 5.875s configuring, 75.610s building, 0.103s running
> >>
> >> Reported-by: Anna Schumaker <[email protected]>
> >> Fixes: 8270dbfcebea ("SUNRPC: Obscure Kerberos integrity keys")
> >> Signed-off-by: Chuck Lever <[email protected]>
> >
> > This patch fixed the issue for me, thanks! Are you going to submit it
> > with a future 6.3-rc pull request, or should I?
>
> I'll queue it in nfsd-fixes.
Sounds good. Thanks!
Anna
>
>
> > Anna
> >
> >> ---
> >> net/sunrpc/auth_gss/gss_krb5_crypto.c | 10 +++++-----
> >> 1 file changed, 5 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/net/sunrpc/auth_gss/gss_krb5_crypto.c b/net/sunrpc/auth_gss/gss_krb5_crypto.c
> >> index 6c7c52eeed4f..212c5d57465a 100644
> >> --- a/net/sunrpc/auth_gss/gss_krb5_crypto.c
> >> +++ b/net/sunrpc/auth_gss/gss_krb5_crypto.c
> >> @@ -353,7 +353,9 @@ gss_krb5_checksum(struct crypto_ahash *tfm, char *header, int hdrlen,
> >> err = crypto_ahash_final(req);
> >> if (err)
> >> goto out_free_ahash;
> >> - memcpy(cksumout->data, checksumdata, cksumout->len);
> >> +
> >> + memcpy(cksumout->data, checksumdata,
> >> + min_t(int, cksumout->len, crypto_ahash_digestsize(tfm)));
> >>
> >> out_free_ahash:
> >> ahash_request_free(req);
> >> @@ -809,8 +811,7 @@ gss_krb5_aes_encrypt(struct krb5_ctx *kctx, u32 offset,
> >> buf->tail[0].iov_len += GSS_KRB5_TOK_HDR_LEN;
> >> buf->len += GSS_KRB5_TOK_HDR_LEN;
> >>
> >> - /* Do the HMAC */
> >> - hmac.len = GSS_KRB5_MAX_CKSUM_LEN;
> >> + hmac.len = kctx->gk5e->cksumlength;
> >> hmac.data = buf->tail[0].iov_base + buf->tail[0].iov_len;
> >>
> >> /*
> >> @@ -873,8 +874,7 @@ gss_krb5_aes_decrypt(struct krb5_ctx *kctx, u32 offset, u32 len,
> >> if (ret)
> >> goto out_err;
> >>
> >> - /* Calculate our hmac over the plaintext data */
> >> - our_hmac_obj.len = sizeof(our_hmac);
> >> + our_hmac_obj.len = kctx->gk5e->cksumlength;
> >> our_hmac_obj.data = our_hmac;
> >> ret = gss_krb5_checksum(ahash, NULL, 0, &subbuf, 0, &our_hmac_obj);
> >> if (ret)
>
> --
> Chuck Lever
>
>