Commit Graph

9 Commits

Author SHA1 Message Date
Eric Biggers d6e43798b3 crypto: dh - make crypto_dh_encode_key() make robust
Make it return -EINVAL if crypto_dh_key_len() is incorrect rather than
overflowing the buffer.

Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
2018-08-03 18:06:06 +08:00
Eric Biggers 35f7d5225f crypto: dh - fix calculating encoded key size
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, and
fix the lengths of the test vectors to match this.

Reported-by: syzbot+6d38d558c25b53b8f4ed@syzkaller.appspotmail.com
Fixes: e3fe0ae129 ("crypto: dh - add public key verification test")
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
2018-08-03 18:06:06 +08:00
Stephan Mueller e3fe0ae129 crypto: dh - add public key verification test
According to SP800-56A section 5.6.2.1, the public key to be processed
for the DH operation shall be checked for appropriateness. The check
shall covers the full verification test in case the domain parameter Q
is provided as defined in SP800-56A section 5.6.2.3.1. If Q is not
provided, the partial check according to SP800-56A section 5.6.2.3.2 is
performed.

The full verification test requires the presence of the domain parameter
Q. Thus, the patch adds the support to handle Q. It is permissible to
not provide the Q value as part of the domain parameters. This implies
that the interface is still backwards-compatible where so far only P and
G are to be provided. However, if Q is provided, it is imported.

Without the test, the NIST ACVP testing fails. After adding this check,
the NIST ACVP testing passes. Testing without providing the Q domain
parameter has been performed to verify the interface has not changed.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
2018-07-09 00:26:22 +08:00
Eric Biggers ccd9888f14 crypto: dh - Don't permit 'key' or 'g' size longer than 'p'
The "qat-dh" DH implementation assumes that 'key' and 'g' can be copied
into a buffer with size 'p_size'.  However it was never checked that
that was actually the case, which most likely allowed users to cause a
buffer underflow via KEYCTL_DH_COMPUTE.

Fix this by updating crypto_dh_decode_key() to verify this precondition
for all DH implementations.

Fixes: c9839143eb ("crypto: qat - Add DH support")
Cc: <stable@vger.kernel.org> # v4.8+
Signed-off-by: Eric Biggers <ebiggers@google.com>
Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
2017-11-10 19:20:17 +08:00
Eric Biggers 199512b123 crypto: dh - Don't permit 'p' to be 0
If 'p' is 0 for the software Diffie-Hellman implementation, then
dh_max_size() returns 0.  In the case of KEYCTL_DH_COMPUTE, this causes
ZERO_SIZE_PTR to be passed to sg_init_one(), which with
CONFIG_DEBUG_SG=y triggers the 'BUG_ON(!virt_addr_valid(buf));' in
sg_set_buf().

Fix this by making crypto_dh_decode_key() reject 0 for 'p'.  p=0 makes
no sense for any DH implementation because 'p' is supposed to be a prime
number.  Moreover, 'mod 0' is not mathematically defined.

Bug report:

    kernel BUG at ./include/linux/scatterlist.h:140!
    invalid opcode: 0000 [#1] SMP KASAN
    CPU: 0 PID: 27112 Comm: syz-executor2 Not tainted 4.14.0-rc7-00010-gf5dbb5d0ce32-dirty #7
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.3-20171021_125229-anatol 04/01/2014
    task: ffff88006caac0c0 task.stack: ffff88006c7c8000
    RIP: 0010:sg_set_buf include/linux/scatterlist.h:140 [inline]
    RIP: 0010:sg_init_one+0x1b3/0x240 lib/scatterlist.c:156
    RSP: 0018:ffff88006c7cfb08 EFLAGS: 00010216
    RAX: 0000000000010000 RBX: ffff88006c7cfe30 RCX: 00000000000064ee
    RDX: ffffffff81cf64c3 RSI: ffffc90000d72000 RDI: ffffffff92e937e0
    RBP: ffff88006c7cfb30 R08: ffffed000d8f9fab R09: ffff88006c7cfd30
    R10: 0000000000000005 R11: ffffed000d8f9faa R12: ffff88006c7cfd30
    R13: 0000000000000000 R14: 0000000000000010 R15: ffff88006c7cfc50
    FS:  00007fce190fa700(0000) GS:ffff88003ea00000(0000) knlGS:0000000000000000
    CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    CR2: 00007fffc6b33db8 CR3: 000000003cf64000 CR4: 00000000000006f0
    Call Trace:
     __keyctl_dh_compute+0xa95/0x19b0 security/keys/dh.c:360
     keyctl_dh_compute+0xac/0x100 security/keys/dh.c:434
     SYSC_keyctl security/keys/keyctl.c:1745 [inline]
     SyS_keyctl+0x72/0x2c0 security/keys/keyctl.c:1641
     entry_SYSCALL_64_fastpath+0x1f/0xbe
    RIP: 0033:0x4585c9
    RSP: 002b:00007fce190f9bd8 EFLAGS: 00000216 ORIG_RAX: 00000000000000fa
    RAX: ffffffffffffffda RBX: 0000000000738020 RCX: 00000000004585c9
    RDX: 000000002000d000 RSI: 0000000020000ff4 RDI: 0000000000000017
    RBP: 0000000000000046 R08: 0000000020008000 R09: 0000000000000000
    R10: 0000000000000000 R11: 0000000000000216 R12: 00007fff6e610cde
    R13: 00007fff6e610cdf R14: 00007fce190fa700 R15: 0000000000000000
    Code: 03 0f b6 14 02 48 89 f8 83 e0 07 83 c0 03 38 d0 7c 04 84 d2 75 33 5b 45 89 6c 24 14 41 5c 41 5d 41 5e 41 5f 5d c3 e8 fd 8f 68 ff <0f> 0b e8 f6 8f 68 ff 0f 0b e8 ef 8f 68 ff 0f 0b e8 e8 8f 68 ff 20
    RIP: sg_set_buf include/linux/scatterlist.h:140 [inline] RSP: ffff88006c7cfb08
    RIP: sg_init_one+0x1b3/0x240 lib/scatterlist.c:156 RSP: ffff88006c7cfb08

Fixes: 802c7f1c84 ("crypto: dh - Add DH software implementation")
Cc: <stable@vger.kernel.org> # v4.8+
Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
2017-11-10 19:20:17 +08:00
Tudor-Dan Ambarus 5b3f3a8bed crypto: dh - return unsigned value for crypto_dh_key_len()
DH_KPP_SECRET_MIN_SIZE and dh_data_size() are both returning
unsigned values.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
2017-10-12 22:55:05 +08:00
Tudor-Dan Ambarus cb195b3625 crypto: dh - return unsigned int for dh_data_size()
p->key_size, p->p_size, p->g_size are all of unsigned int type.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
2017-10-12 22:55:01 +08:00
Tudor-Dan Ambarus c0ca1215dc crypto: kpp, (ec)dh - fix typos
While here, add missing argument description (ndigits).

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
2017-06-10 12:04:25 +08:00
Salvatore Benedetto 802c7f1c84 crypto: dh - Add DH software implementation
* Implement MPI based Diffie-Hellman under kpp API
 * Test provided uses data generad by OpenSSL

Signed-off-by: Salvatore Benedetto <salvatore.benedetto@intel.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
2016-06-23 18:29:56 +08:00