[PATCH 1/2] crypto: DH - update test for public key verification
By adding a zero byte-length for the DH parameter Q value, the public
key verification test is disabled for the given test.
Reported-by: Eric Biggers
Signed-off-by: Stephan Mueller
---
crypto/testmgr.h | 4
1 file changed, 4 insertions(+)
diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index b6362169771a..759462d65f41 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -644,12 +644,14 @@ static const struct kpp_testvec dh_tv_template[] = {
"\x11\x02" /* len */
"\x00\x01\x00\x00" /* key_size */
"\x00\x01\x00\x00" /* p_size */
+ "\x00\x00\x00\x00" /* q_size */
"\x01\x00\x00\x00" /* g_size */
#else
"\x00\x01" /* type */
"\x02\x11" /* len */
"\x00\x00\x01\x00" /* key_size */
"\x00\x00\x01\x00" /* p_size */
+ "\x00\x00\x00\x00" /* q_size */
"\x00\x00\x00\x01" /* g_size */
#endif
/* xa */
@@ -751,12 +753,14 @@ static const struct kpp_testvec dh_tv_template[] = {
"\x11\x02" /* len */
"\x00\x01\x00\x00" /* key_size */
"\x00\x01\x00\x00" /* p_size */
+ "\x00\x00\x00\x00" /* q_size */
"\x01\x00\x00\x00" /* g_size */
#else
"\x00\x01" /* type */
"\x02\x11" /* len */
"\x00\x00\x01\x00" /* key_size */
"\x00\x00\x01\x00" /* p_size */
+ "\x00\x00\x00\x00" /* q_size */
"\x00\x00\x00\x01" /* g_size */
#endif
/* xa */
--
2.17.1
Re: [PATCH] crypto: dh - fix calculating encoded key size
On Wed, Jul 11, 2018 at 03:26:56PM +0800, Herbert Xu wrote:
> On Tue, Jul 10, 2018 at 08:59:05PM -0700, Eric Biggers wrote:
> > From: Eric Biggers
> >
> > It was forgotten to increase DH_KPP_SECRET_MIN_SIZE to include 'q_size',
> > causing an out-of-bounds write of 4 bytes in crypto_dh_encode_key(), and
> > an out-of-bounds read of 4 bytes in crypto_dh_decode_key(). Fix it.
> > Also add a BUG_ON() if crypto_dh_encode_key() doesn't exactly fill the
> > buffer, as that would have found this bug without resorting to KASAN.
> >
> > Reported-by: syzbot+6d38d558c25b53b8f...@syzkaller.appspotmail.com
> > Fixes: e3fe0ae12962 ("crypto: dh - add public key verification test")
> > Signed-off-by: Eric Biggers
>
> Is it possible to return an error and use WARN_ON instead of BUG_ON?
> Or do the callers not bother to check for errors?
>
The callers do check for errors, but at the point of the proposed BUG_ON() a
buffer overflow may have already occurred, so I think a BUG_ON() would be more
appropriate than a WARN_ON(). Of course, it would be better to prevent any
buffer overflow from occurring in the first place, but that's already the
purpose of the 'len != crypto_dh_key_len(params)' check; the issue was that
'crypto_dh_key_len()' calculated the wrong length.
- Eric
Re: [PATCH v2] crypto: DH - add public key verification test
Am Mittwoch, 11. Juli 2018, 06:10:59 CEST schrieb Eric Biggers: Hi Eric, > You forgot to update the self-tests in the kernel, so they're failing now, > as you *did* change the interface (the "key" is encoded differently now). I was actually looking for failing self tests. But accidentally I disabled the self tests in my test bed which I just saw now. :-( I will provide a fix shortly. Ciao Stephan
Re: [PATCH] crypto: dh - fix calculating encoded key size
Am Mittwoch, 11. Juli 2018, 05:59:05 CEST schrieb Eric Biggers:
Hi Eric,
> From: Eric Biggers
>
> It was forgotten to increase DH_KPP_SECRET_MIN_SIZE to include 'q_size',
> causing an out-of-bounds write of 4 bytes in crypto_dh_encode_key(), and
> an out-of-bounds read of 4 bytes in crypto_dh_decode_key(). Fix it.
> Also add a BUG_ON() if crypto_dh_encode_key() doesn't exactly fill the
> buffer, as that would have found this bug without resorting to KASAN.
>
> Reported-by: syzbot+6d38d558c25b53b8f...@syzkaller.appspotmail.com
> Fixes: e3fe0ae12962 ("crypto: dh - add public key verification test")
> Signed-off-by: Eric Biggers
Thanks.
Reviewed-by: Stephan Müller
Ciao
Stephan
[bug report] crypto: ccp - Add DOWNLOAD_FIRMWARE SEV command
Hello Janakarajan Natarajan,
The patch edd303ff0e9e: "crypto: ccp - Add DOWNLOAD_FIRMWARE SEV
command" from May 25, 2018, leads to the following static checker
warning:
drivers/crypto/ccp/psp-dev.c:397 sev_get_api_version()
error: uninitialized symbol 'error'.
drivers/crypto/ccp/psp-dev.c
389 static int sev_get_api_version(void)
390 {
391 struct sev_user_data_status *status;
392 int error, ret;
393
394 status = _master->status_cmd_buf;
395 ret = sev_platform_status(status, );
396 if (ret) {
397 dev_err(psp_master->dev,
398 "SEV: failed to get status. Error: %#x\n",
error);
sev_platform_status() could be defined as a no-op or there is an error
path where *error isn't set.
399 return 1;
400 }
401
402 psp_master->api_major = status->api_major;
403 psp_master->api_minor = status->api_minor;
404 psp_master->build = status->build;
405
406 return 0;
407 }
regards,
dan carpenter
Re: [PATCH] crypto: dh - fix calculating encoded key size
On Tue, Jul 10, 2018 at 08:59:05PM -0700, Eric Biggers wrote:
> From: Eric Biggers
>
> It was forgotten to increase DH_KPP_SECRET_MIN_SIZE to include 'q_size',
> causing an out-of-bounds write of 4 bytes in crypto_dh_encode_key(), and
> an out-of-bounds read of 4 bytes in crypto_dh_decode_key(). Fix it.
> Also add a BUG_ON() if crypto_dh_encode_key() doesn't exactly fill the
> buffer, as that would have found this bug without resorting to KASAN.
>
> Reported-by: syzbot+6d38d558c25b53b8f...@syzkaller.appspotmail.com
> Fixes: e3fe0ae12962 ("crypto: dh - add public key verification test")
> Signed-off-by: Eric Biggers
Is it possible to return an error and use WARN_ON instead of BUG_ON?
Or do the callers not bother to check for errors?
Thanks,
--
Email: Herbert Xu
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
