From patchwork Wed Nov 29 02:01:38 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Biggers X-Patchwork-Id: 10081345 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id ABC6860311 for ; Wed, 29 Nov 2017 02:03:05 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9EAE129807 for ; Wed, 29 Nov 2017 02:03:05 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 931C429809; Wed, 29 Nov 2017 02:03:05 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1B7FA29807 for ; Wed, 29 Nov 2017 02:03:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751953AbdK2CCu (ORCPT ); Tue, 28 Nov 2017 21:02:50 -0500 Received: from mail-pl0-f67.google.com ([209.85.160.67]:45614 "EHLO mail-pl0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751846AbdK2CCr (ORCPT ); Tue, 28 Nov 2017 21:02:47 -0500 Received: by mail-pl0-f67.google.com with SMTP id f6so1122187pln.12; Tue, 28 Nov 2017 18:02:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=rPW6PFFxTu1hHgXG1030Jbb48Vr3lYNP+dG1QBZancI=; b=PSbWOjuMR4QnYO3UFktgMnmMrBomyJd9Ls/fZO/1CWFrkaOpOCuhGAI+Mdy5LFjqHJ 7MPUfWoKatn/u24YqeEhfJqt1HMQtlUgfSQYL9MPTzgb10bkSDaq/qcEc2NkTI/rGwBR 8gnHCSL0q1t4tWYqBHmCGFmj9BxEbFh8O8TJ5DWzwaMSegJCOVHpmN644aTuB5ehQozH H7JrYKbnFcO/h+o2FUOEpChxlMpcQ2U2G03PxKXB7c4hK6nXM44KQr0mqk4FCc8jrwGf UjX4bCN8llBG5kjQ2pwSwmfbshnpoB5JFho23g009tuEuE7Pu1Y3jn9K4+tFHCcc7QIl oWXA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=rPW6PFFxTu1hHgXG1030Jbb48Vr3lYNP+dG1QBZancI=; b=TyjwFYy0LbiUcWQCNHAi0yqhw3p2XFfW2IovR2q2XHrQW5f/fjh4My71IEs5NZuaB4 Fsxd2phNLwYnCK40QYl/o4S82Dg2u3F+eRnNnNDsBEboruBUccPatSgTu7Jt5D2hepU9 pHApuK1oo/i3aW3QgPgP3d3cXZTqnPpHJB2HX5GAHy6dZsc90alhLJ6V7F17CiPAkvJH otr7Jl4HRf4Z4jotEyv03dfj3weLsMhpP0OHKQfqD/DPGby1vnqXumRRQcoc5unjqQY0 15Nnb/O4pN/OD6CVgeSncgvAzdcC5kMxpl3wO/xCIkh5cGHWqmgC2EJSsLwvfK4RLzmu GbBA== X-Gm-Message-State: AJaThX5sNWkh75gFYT4yZc9di+oLDepv/hm6EuCi2gJl7/VDfIbrfM32 uWIWbq+wAeCAJ5N+RyPVds+lvz0p X-Google-Smtp-Source: AGs4zMYtK0rOLAc+y6/Ojq2ijemoz5AiCtrYyFPv+pSzWF5MZaSEYN1a0LrXIN0zkLfvUK4PPSPr6w== X-Received: by 10.84.225.146 with SMTP id u18mr1215393plj.69.1511920966389; Tue, 28 Nov 2017 18:02:46 -0800 (PST) Received: from zzz.localdomain (c-67-185-97-198.hsd1.wa.comcast.net. [67.185.97.198]) by smtp.gmail.com with ESMTPSA id g127sm429457pgc.29.2017.11.28.18.02.45 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 28 Nov 2017 18:02:45 -0800 (PST) From: Eric Biggers To: linux-crypto@vger.kernel.org, Herbert Xu Cc: "David S . Miller" , linux-kernel@vger.kernel.org, Eric Biggers , stable@vger.kernel.org Subject: [PATCH v2] crypto: hmac - require that the underlying hash algorithm is unkeyed Date: Tue, 28 Nov 2017 18:01:38 -0800 Message-Id: <20171129020138.4436-1-ebiggers3@gmail.com> X-Mailer: git-send-email 2.15.0 Sender: linux-crypto-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-crypto@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Eric Biggers Because the HMAC template didn't check that its underlying hash algorithm is unkeyed, trying to use "hmac(hmac(sha3-512-generic))" through AF_ALG or through KEYCTL_DH_COMPUTE resulted in the inner HMAC being used without having been keyed, resulting in sha3_update() being called without sha3_init(), causing a stack buffer overflow. This is a very old bug, but it seems to have only started causing real problems when SHA-3 support was added (requires CONFIG_CRYPTO_SHA3) because the innermost hash's state is ->import()ed from a zeroed buffer, and it just so happens that other hash algorithms are fine with that, but SHA-3 is not. However, there could be arch or hardware-dependent hash algorithms also affected; I couldn't test everything. Fix the bug by introducing a function crypto_shash_alg_has_setkey() which tests whether a shash algorithm is keyed. Then update the HMAC template to require that its underlying hash algorithm is unkeyed. Here is a reproducer: #include #include int main() { int algfd; struct sockaddr_alg addr = { .salg_type = "hash", .salg_name = "hmac(hmac(sha3-512-generic))", }; char key[4096] = { 0 }; algfd = socket(AF_ALG, SOCK_SEQPACKET, 0); bind(algfd, (const struct sockaddr *)&addr, sizeof(addr)); setsockopt(algfd, SOL_ALG, ALG_SET_KEY, key, sizeof(key)); } Here was the KASAN report from syzbot: BUG: KASAN: stack-out-of-bounds in memcpy include/linux/string.h:341 [inline] BUG: KASAN: stack-out-of-bounds in sha3_update+0xdf/0x2e0 crypto/sha3_generic.c:161 Write of size 4096 at addr ffff8801cca07c40 by task syzkaller076574/3044 CPU: 1 PID: 3044 Comm: syzkaller076574 Not tainted 4.14.0-mm1+ #25 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:17 [inline] dump_stack+0x194/0x257 lib/dump_stack.c:53 print_address_description+0x73/0x250 mm/kasan/report.c:252 kasan_report_error mm/kasan/report.c:351 [inline] kasan_report+0x25b/0x340 mm/kasan/report.c:409 check_memory_region_inline mm/kasan/kasan.c:260 [inline] check_memory_region+0x137/0x190 mm/kasan/kasan.c:267 memcpy+0x37/0x50 mm/kasan/kasan.c:303 memcpy include/linux/string.h:341 [inline] sha3_update+0xdf/0x2e0 crypto/sha3_generic.c:161 crypto_shash_update+0xcb/0x220 crypto/shash.c:109 shash_finup_unaligned+0x2a/0x60 crypto/shash.c:151 crypto_shash_finup+0xc4/0x120 crypto/shash.c:165 hmac_finup+0x182/0x330 crypto/hmac.c:152 crypto_shash_finup+0xc4/0x120 crypto/shash.c:165 shash_digest_unaligned+0x9e/0xd0 crypto/shash.c:172 crypto_shash_digest+0xc4/0x120 crypto/shash.c:186 hmac_setkey+0x36a/0x690 crypto/hmac.c:66 crypto_shash_setkey+0xad/0x190 crypto/shash.c:64 shash_async_setkey+0x47/0x60 crypto/shash.c:207 crypto_ahash_setkey+0xaf/0x180 crypto/ahash.c:200 hash_setkey+0x40/0x90 crypto/algif_hash.c:446 alg_setkey crypto/af_alg.c:221 [inline] alg_setsockopt+0x2a1/0x350 crypto/af_alg.c:254 SYSC_setsockopt net/socket.c:1851 [inline] SyS_setsockopt+0x189/0x360 net/socket.c:1830 entry_SYSCALL_64_fastpath+0x1f/0x96 Reported-by: syzbot Cc: Signed-off-by: Eric Biggers --- crypto/hmac.c | 6 +++++- crypto/shash.c | 5 +++-- include/crypto/internal/hash.h | 8 ++++++++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/crypto/hmac.c b/crypto/hmac.c index 92871dc2a63e..e74730224f0a 100644 --- a/crypto/hmac.c +++ b/crypto/hmac.c @@ -195,11 +195,15 @@ static int hmac_create(struct crypto_template *tmpl, struct rtattr **tb) salg = shash_attr_alg(tb[1], 0, 0); if (IS_ERR(salg)) return PTR_ERR(salg); + alg = &salg->base; + /* The underlying hash algorithm must be unkeyed */ err = -EINVAL; + if (crypto_shash_alg_has_setkey(salg)) + goto out_put_alg; + ds = salg->digestsize; ss = salg->statesize; - alg = &salg->base; if (ds > alg->cra_blocksize || ss < alg->cra_blocksize) goto out_put_alg; diff --git a/crypto/shash.c b/crypto/shash.c index 325a14da5827..e849d3ee2e27 100644 --- a/crypto/shash.c +++ b/crypto/shash.c @@ -25,11 +25,12 @@ static const struct crypto_type crypto_shash_type; -static int shash_no_setkey(struct crypto_shash *tfm, const u8 *key, - unsigned int keylen) +int shash_no_setkey(struct crypto_shash *tfm, const u8 *key, + unsigned int keylen) { return -ENOSYS; } +EXPORT_SYMBOL_GPL(shash_no_setkey); static int shash_setkey_unaligned(struct crypto_shash *tfm, const u8 *key, unsigned int keylen) diff --git a/include/crypto/internal/hash.h b/include/crypto/internal/hash.h index f0b44c16e88f..c2bae8da642c 100644 --- a/include/crypto/internal/hash.h +++ b/include/crypto/internal/hash.h @@ -82,6 +82,14 @@ int ahash_register_instance(struct crypto_template *tmpl, struct ahash_instance *inst); void ahash_free_instance(struct crypto_instance *inst); +int shash_no_setkey(struct crypto_shash *tfm, const u8 *key, + unsigned int keylen); + +static inline bool crypto_shash_alg_has_setkey(struct shash_alg *alg) +{ + return alg->setkey != shash_no_setkey; +} + int crypto_init_ahash_spawn(struct crypto_ahash_spawn *spawn, struct hash_alg_common *alg, struct crypto_instance *inst);