summaryrefslogtreecommitdiffstats
path: root/crypto
Commit message (Collapse)AuthorAgeFilesLines
* X.509: fix NULL dereference when restricting key with unsupported_sigEric Biggers2018-02-221-8/+13
| | | | | | | | | | | | | | | | | | | | | | | | The asymmetric key type allows an X.509 certificate to be added even if its signature's hash algorithm is not available in the crypto API. In that case 'payload.data[asym_auth]' will be NULL. But the key restriction code failed to check for this case before trying to use the signature, resulting in a NULL pointer dereference in key_or_keyring_common() or in restrict_link_by_signature(). Fix this by returning -ENOPKG when the signature is unsupported. Reproducer when all the CONFIG_CRYPTO_SHA512* options are disabled and keyctl has support for the 'restrict_keyring' command: keyctl new_session keyctl restrict_keyring @s asymmetric builtin_trusted openssl req -new -sha512 -x509 -batch -nodes -outform der \ | keyctl padd asymmetric desc @s Fixes: a511e1af8b12 ("KEYS: Move the point of trust determination to __key_link()") Cc: <stable@vger.kernel.org> # v4.7+ Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: David Howells <dhowells@redhat.com>
* X.509: fix BUG_ON() when hash algorithm is unsupportedEric Biggers2018-02-221-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | The X.509 parser mishandles the case where the certificate's signature's hash algorithm is not available in the crypto API. In this case, x509_get_sig_params() doesn't allocate the cert->sig->digest buffer; this part seems to be intentional. However, public_key_verify_signature() is still called via x509_check_for_self_signed(), which triggers the 'BUG_ON(!sig->digest)'. Fix this by making public_key_verify_signature() return -ENOPKG if the hash buffer has not been allocated. Reproducer when all the CONFIG_CRYPTO_SHA512* options are disabled: openssl req -new -sha512 -x509 -batch -nodes -outform der \ | keyctl padd asymmetric desc @s Fixes: 6c2dc5ae4ab7 ("X.509: Extract signature digest and make self-signed cert checks earlier") Reported-by: Paolo Valente <paolo.valente@linaro.org> Cc: Paolo Valente <paolo.valente@linaro.org> Cc: <stable@vger.kernel.org> # v4.7+ Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: David Howells <dhowells@redhat.com>
* PKCS#7: fix direct verification of SignerInfo signatureEric Biggers2018-02-221-0/+1
| | | | | | | | | | | | | | | | | | | | | If none of the certificates in a SignerInfo's certificate chain match a trusted key, nor is the last certificate signed by a trusted key, then pkcs7_validate_trust_one() tries to check whether the SignerInfo's signature was made directly by a trusted key. But, it actually fails to set the 'sig' variable correctly, so it actually verifies the last signature seen. That will only be the SignerInfo's signature if the certificate chain is empty; otherwise it will actually be the last certificate's signature. This is not by itself a security problem, since verifying any of the certificates in the chain should be sufficient to verify the SignerInfo. Still, it's not working as intended so it should be fixed. Fix it by setting 'sig' correctly for the direct verification case. Fixes: 757932e6da6d ("PKCS#7: Handle PKCS#7 messages that contain no X.509 certs") Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: David Howells <dhowells@redhat.com>
* PKCS#7: fix certificate blacklistingEric Biggers2018-02-221-4/+6
| | | | | | | | | | | | | | | | | | | If there is a blacklisted certificate in a SignerInfo's certificate chain, then pkcs7_verify_sig_chain() sets sinfo->blacklisted and returns 0. But, pkcs7_verify() fails to handle this case appropriately, as it actually continues on to the line 'actual_ret = 0;', indicating that the SignerInfo has passed verification. Consequently, PKCS#7 signature verification ignores the certificate blacklist. Fix this by not considering blacklisted SignerInfos to have passed verification. Also fix the function comment with regards to when 0 is returned. Fixes: 03bb79315ddc ("PKCS#7: Handle blacklisted certificates") Cc: <stable@vger.kernel.org> # v4.12+ Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: David Howells <dhowells@redhat.com>
* PKCS#7: fix certificate chain verificationEric Biggers2018-02-221-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When pkcs7_verify_sig_chain() is building the certificate chain for a SignerInfo using the certificates in the PKCS#7 message, it is passing the wrong arguments to public_key_verify_signature(). Consequently, when the next certificate is supposed to be used to verify the previous certificate, the next certificate is actually used to verify itself. An attacker can use this bug to create a bogus certificate chain that has no cryptographic relationship between the beginning and end. Fortunately I couldn't quite find a way to use this to bypass the overall signature verification, though it comes very close. Here's the reasoning: due to the bug, every certificate in the chain beyond the first actually has to be self-signed (where "self-signed" here refers to the actual key and signature; an attacker might still manipulate the certificate fields such that the self_signed flag doesn't actually get set, and thus the chain doesn't end immediately). But to pass trust validation (pkcs7_validate_trust()), either the SignerInfo or one of the certificates has to actually be signed by a trusted key. Since only self-signed certificates can be added to the chain, the only way for an attacker to introduce a trusted signature is to include a self-signed trusted certificate. But, when pkcs7_validate_trust_one() reaches that certificate, instead of trying to verify the signature on that certificate, it will actually look up the corresponding trusted key, which will succeed, and then try to verify the *previous* certificate, which will fail. Thus, disaster is narrowly averted (as far as I could tell). Fixes: 6c2dc5ae4ab7 ("X.509: Extract signature digest and make self-signed cert checks earlier") Cc: <stable@vger.kernel.org> # v4.7+ Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: David Howells <dhowells@redhat.com>
* Merge branch 'linus' of ↵Linus Torvalds2018-02-121-100/+118
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6 Pull crypto fixes from Herbert Xu: "This fixes the following issues: - oversize stack frames on mn10300 in sha3-generic - warning on old compilers in sha3-generic - API error in sun4i_ss_prng - potential dead-lock in sun4i_ss_prng - null-pointer dereference in sha512-mb - endless loop when DECO acquire fails in caam - kernel oops when hashing empty message in talitos" * 'linus' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6: crypto: sun4i_ss_prng - convert lock to _bh in sun4i_ss_prng_generate crypto: sun4i_ss_prng - fix return value of sun4i_ss_prng_generate crypto: caam - fix endless loop when DECO acquire fails crypto: sha3-generic - Use __optimize to support old compilers compiler-gcc.h: __nostackprotector needs gcc-4.4 and up compiler-gcc.h: Introduce __optimize function attribute crypto: sha3-generic - deal with oversize stack frames crypto: talitos - fix Kernel Oops on hashing an empty file crypto: sha512-mb - initialize pending lengths correctly
| * crypto: sha3-generic - Use __optimize to support old compilersGeert Uytterhoeven2018-02-081-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | With gcc-4.1.2: crypto/sha3_generic.c:39: warning: ‘__optimize__’ attribute directive ignored Use the newly introduced __optimize macro to fix this. Fixes: 83dee2ce1ae791c3 ("crypto: sha3-generic - rewrite KECCAK transform to help the compiler optimize") Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: sha3-generic - deal with oversize stack framesArd Biesheuvel2018-02-081-100/+118
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | As reported by kbuild test robot, the optimized SHA3 C implementation compiles to mn10300 code that uses a disproportionate amount of stack space, i.e., crypto/sha3_generic.c: In function 'keccakf': crypto/sha3_generic.c:147:1: warning: the frame size of 1232 bytes is larger than 1024 bytes [-Wframe-larger-than=] As kindly diagnosed by Arnd, this does not only occur when building for the mn10300 architecture (which is what the report was about) but also for h8300, and builds for other 32-bit architectures show an increase in stack space utilization as well. Given that SHA3 operates on 64-bit quantities, and keeps a state matrix of 25 64-bit words, it is not surprising that 32-bit architectures with few general purpose registers are impacted the most by this, and it is therefore reasonable to implement a workaround that distinguishes between 32-bit and 64-bit architectures. Arnd figured out that taking the round calculation out of the loop, and inlining it explicitly but only on 64-bit architectures preserves most of the performance gain achieved by the rewrite, and also gets rid of the excessive use of stack space. Reported-by: kbuild test robot <fengguang.wu@intel.com> Suggested-by: Arnd Bergmann <arnd@arndb.de> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* | vfs: do bulk POLL* -> EPOLL* replacementLinus Torvalds2018-02-111-8/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is the mindless scripted replacement of kernel use of POLL* variables as described by Al, done by this script: for V in IN OUT PRI ERR RDNORM RDBAND WRNORM WRBAND HUP RDHUP NVAL MSG; do L=`git grep -l -w POLL$V | grep -v '^t' | grep -v /um/ | grep -v '^sa' | grep -v '/poll.h$'|grep -v '^D'` for f in $L; do sed -i "-es/^\([^\"]*\)\(\<POLL$V\>\)/\\1E\\2/" $f; done done with de-mangling cleanups yet to come. NOTE! On almost all architectures, the EPOLL* constants have the same values as the POLL* constants do. But they keyword here is "almost". For various bad reasons they aren't the same, and epoll() doesn't actually work quite correctly in some cases due to this on Sparc et al. The next patch from Al will sort out the final differences, and we should be all done. Scripted-by: Al Viro <viro@zeniv.linux.org.uk> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
* | Merge branch 'linus' of ↵Linus Torvalds2018-01-3144-653/+2032
|\| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6 Pull crypto updates from Herbert Xu: "API: - Enforce the setting of keys for keyed aead/hash/skcipher algorithms. - Add multibuf speed tests in tcrypt. Algorithms: - Improve performance of sha3-generic. - Add native sha512 support on arm64. - Add v8.2 Crypto Extentions version of sha3/sm3 on arm64. - Avoid hmac nesting by requiring underlying algorithm to be unkeyed. - Add cryptd_max_cpu_qlen module parameter to cryptd. Drivers: - Add support for EIP97 engine in inside-secure. - Add inline IPsec support to chelsio. - Add RevB core support to crypto4xx. - Fix AEAD ICV check in crypto4xx. - Add stm32 crypto driver. - Add support for BCM63xx platforms in bcm2835 and remove bcm63xx. - Add Derived Key Protocol (DKP) support in caam. - Add Samsung Exynos True RNG driver. - Add support for Exynos5250+ SoCs in exynos PRNG driver" * 'linus' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6: (166 commits) crypto: picoxcell - Fix error handling in spacc_probe() crypto: arm64/sha512 - fix/improve new v8.2 Crypto Extensions code crypto: arm64/sm3 - new v8.2 Crypto Extensions implementation crypto: arm64/sha3 - new v8.2 Crypto Extensions implementation crypto: testmgr - add new testcases for sha3 crypto: sha3-generic - export init/update/final routines crypto: sha3-generic - simplify code crypto: sha3-generic - rewrite KECCAK transform to help the compiler optimize crypto: sha3-generic - fixes for alignment and big endian operation crypto: aesni - handle zero length dst buffer crypto: artpec6 - remove select on non-existing CRYPTO_SHA384 hwrng: bcm2835 - Remove redundant dev_err call in bcm2835_rng_probe() crypto: stm32 - remove redundant dev_err call in stm32_cryp_probe() crypto: axis - remove unnecessary platform_get_resource() error check crypto: testmgr - test misuse of result in ahash crypto: inside-secure - make function safexcel_try_push_requests static crypto: aes-generic - fix aes-generic regression on powerpc crypto: chelsio - Fix indentation warning crypto: arm64/sha1-ce - get rid of literal pool crypto: arm64/sha2-ce - move the round constant table to .rodata section ...
| * crypto: testmgr - add new testcases for sha3Ard Biesheuvel2018-01-261-0/+550
| | | | | | | | | | | | | | | | | | All current SHA3 test cases are smaller than the SHA3 block size, which means not all code paths are being exercised. So add a new test case to each variant, and make one of the existing test cases chunked. Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: sha3-generic - export init/update/final routinesArd Biesheuvel2018-01-261-15/+18
| | | | | | | | | | | | | | | | | | | | To allow accelerated implementations to fall back to the generic routines, e.g., in contexts where a SIMD based implementation is not allowed to run, expose the generic SHA3 init/update/final routines to other modules. Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: sha3-generic - simplify codeArd Biesheuvel2018-01-261-125/+59
| | | | | | | | | | | | | | | | | | | | In preparation of exposing the generic SHA3 implementation to other versions as a fallback, simplify the code, and remove an inconsistency in the output handling (endian swabbing rsizw words of state before writing the output does not make sense) Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: sha3-generic - rewrite KECCAK transform to help the compiler optimizeArd Biesheuvel2018-01-261-38/+96
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The way the KECCAK transform is currently coded involves many references into the state array using indexes that are calculated at runtime using simple but non-trivial arithmetic. This forces the compiler to treat the state matrix as an array in memory rather than keep it in registers, which results in poor performance. So instead, let's rephrase the algorithm using fixed array indexes only. This helps the compiler keep the state matrix in registers, resulting in the following speedup (SHA3-256 performance in cycles per byte): before after speedup Intel Core i7 @ 2.0 GHz (2.9 turbo) 100.6 35.7 2.8x Cortex-A57 @ 2.0 GHz (64-bit mode) 101.6 12.7 8.0x Cortex-A53 @ 1.0 GHz 224.4 15.8 14.2x Cortex-A57 @ 2.0 GHz (32-bit mode) 201.8 63.0 3.2x Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: sha3-generic - fixes for alignment and big endian operationArd Biesheuvel2018-01-261-2/+3
| | | | | | | | | | | | | | | | | | | | | | | | Ensure that the input is byte swabbed before injecting it into the SHA3 transform. Use the get_unaligned() accessor for this so that we don't perform unaligned access inadvertently on architectures that do not support that. Cc: <stable@vger.kernel.org> Fixes: 53964b9ee63b7075 ("crypto: sha3 - Add SHA-3 hash algorithm") Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: testmgr - test misuse of result in ahashKamil Konieczny2018-01-261-0/+39
| | | | | | | | | | | | | | | | Async hash operations can use result pointer in final/finup/digest, but not in init/update/export/import, so test it for misuse. Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: aes-generic - fix aes-generic regression on powerpcArnd Bergmann2018-01-201-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | My last bugfix added -Os on the command line, which unfortunately caused a build regression on powerpc in some configurations. I've done some more analysis of the original problem and found slightly different workaround that avoids this regression and also results in better performance on gcc-7.0: -fcode-hoisting is an optimization step that got added in gcc-7 and that for all gcc-7 versions causes worse performance. This disables -fcode-hoisting on all compilers that understand the option. For gcc-7.1 and 7.2 I found the same performance as my previous patch (using -Os), in gcc-7.0 it was even better. On gcc-8 I could see no change in performance from this patch. In theory, code hoisting should not be able make things better for the AES cipher, so leaving it disabled for gcc-8 only serves to simplify the Makefile change. Reported-by: kbuild test robot <fengguang.wu@intel.com> Link: https://www.mail-archive.com/linux-crypto@vger.kernel.org/msg30418.html Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83356 Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83651 Fixes: 148b974deea9 ("crypto: aes-generic - build with -Os on gcc-7+") Signed-off-by: Arnd Bergmann <arnd@arndb.de> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: x86/salsa20 - cleanup and convert to skcipher APIEric Biggers2018-01-121-0/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Convert salsa20-asm from the deprecated "blkcipher" API to the "skcipher" API, in the process fixing it up to use the generic helpers. This allows removing the salsa20_keysetup() and salsa20_ivsetup() assembly functions, which aren't performance critical; the C versions do just fine. This also fixes the same bug that salsa20-generic had, where the state array was being maintained directly in the transform context rather than on the stack or in the request context. Thus, if multiple threads used the same Salsa20 transform concurrently they produced the wrong results. Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: salsa20 - export generic helpersEric Biggers2018-01-121-13/+7
| | | | | | | | | | | | | | | | Export the Salsa20 constants, transform context, and initialization functions so that they can be reused by the x86 implementation. Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: salsa20-generic - cleanup and convert to skcipher APIEric Biggers2018-01-121-136/+104
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Convert salsa20-generic from the deprecated "blkcipher" API to the "skcipher" API, in the process fixing it up to be thread-safe (as the crypto API expects) by maintaining each request's state separately from the transform context. Also remove the unnecessary cra_alignmask and tighten validation of the key size by accepting only 16 or 32 bytes, not anything in between. These changes bring the code close to the way chacha20-generic does things, so hopefully it will be easier to maintain in the future. However, the way Salsa20 interprets the IV is still slightly different; that was not changed. Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: aes-generic - build with -Os on gcc-7+Arnd Bergmann2018-01-121-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | While testing other changes, I discovered that gcc-7.2.1 produces badly optimized code for aes_encrypt/aes_decrypt. This is especially true when CONFIG_UBSAN_SANITIZE_ALL is enabled, where it leads to extremely large stack usage that in turn might cause kernel stack overflows: crypto/aes_generic.c: In function 'aes_encrypt': crypto/aes_generic.c:1371:1: warning: the frame size of 4880 bytes is larger than 2048 bytes [-Wframe-larger-than=] crypto/aes_generic.c: In function 'aes_decrypt': crypto/aes_generic.c:1441:1: warning: the frame size of 4864 bytes is larger than 2048 bytes [-Wframe-larger-than=] I verified that this problem exists on all architectures that are supported by gcc-7.2, though arm64 in particular is less affected than the others. I also found that gcc-7.1 and gcc-8 do not show the extreme stack usage but still produce worse code than earlier versions for this file, apparently because of optimization passes that generally provide a substantial improvement in object code quality but understandably fail to find any shortcuts in the AES algorithm. Possible workarounds include a) disabling -ftree-pre and -ftree-sra optimizations, this was an earlier patch I tried, which reliably fixed the stack usage, but caused a serious performance regression in some versions, as later testing found. b) disabling UBSAN on this file or all ciphers, as suggested by Ard Biesheuvel. This would lead to massively better crypto performance in UBSAN-enabled kernels and avoid the stack usage, but there is a concern over whether we should exclude arbitrary files from UBSAN at all. c) Forcing the optimization level in a different way. Similar to a), but rather than deselecting specific optimization stages, this now uses "gcc -Os" for this file, regardless of the CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE/SIZE option. This is a reliable workaround for the stack consumption on all architecture, and I've retested the performance results now on x86, cycles/byte (lower is better) for cbc(aes-generic) with 256 bit keys: -O2 -Os gcc-6.3.1 14.9 15.1 gcc-7.0.1 14.7 15.3 gcc-7.1.1 15.3 14.7 gcc-7.2.1 16.8 15.9 gcc-8.0.0 15.5 15.6 This implements the option c) by enabling forcing -Os on all compiler versions starting with gcc-7.1. As a workaround for PR83356, it would only be needed for gcc-7.2+ with UBSAN enabled, but since it also shows better performance on gcc-7.1 without UBSAN, it seems appropriate to use the faster version here as well. Side note: during testing, I also played with the AES code in libressl, which had a similar performance regression from gcc-6 to gcc-7.2, but was three times slower overall. It might be interesting to investigate that further and possibly port the Linux implementation into that. Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83356 Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83651 Cc: Richard Biener <rguenther@suse.de> Cc: Jakub Jelinek <jakub@gcc.gnu.org> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Signed-off-by: Arnd Bergmann <arnd@arndb.de> Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: aead - prevent using AEADs without setting keyEric Biggers2018-01-122-10/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Similar to what was done for the hash API, update the AEAD API to track whether each transform has been keyed, and reject encryption/decryption if a key is needed but one hasn't been set. This isn't quite as important as the equivalent fix for the hash API because AEADs always require a key, so are unlikely to be used without one. Still, tracking the key will prevent accidental unkeyed use. algif_aead also had to track the key anyway, so the new flag replaces that and slightly simplifies the algif_aead implementation. Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: skcipher - prevent using skciphers without setting keyEric Biggers2018-01-122-50/+39
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Similar to what was done for the hash API, update the skcipher API to track whether each transform has been keyed, and reject encryption/decryption if a key is needed but one hasn't been set. This isn't as important as the equivalent fix for the hash API because symmetric ciphers almost always require a key (the "null cipher" is the only exception), so are unlikely to be used without one. Still, tracking the key will prevent accidental unkeyed use. algif_skcipher also had to track the key anyway, so the new flag replaces that and simplifies the algif_skcipher implementation. Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: ghash - remove checks for key being setEric Biggers2018-01-121-6/+0
| | | | | | | | | | | | | | | | Now that the crypto API prevents a keyed hash from being used without setting the key, there's no need for GHASH to do this check itself. Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: hash - prevent using keyed hashes without setting keyEric Biggers2018-01-123-50/+49
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently, almost none of the keyed hash algorithms check whether a key has been set before proceeding. Some algorithms are okay with this and will effectively just use a key of all 0's or some other bogus default. However, others will severely break, as demonstrated using "hmac(sha3-512-generic)", the unkeyed use of which causes a kernel crash via a (potentially exploitable) stack buffer overflow. A while ago, this problem was solved for AF_ALG by pairing each hash transform with a 'has_key' bool. However, there are still other places in the kernel where userspace can specify an arbitrary hash algorithm by name, and the kernel uses it as unkeyed hash without checking whether it is really unkeyed. Examples of this include: - KEYCTL_DH_COMPUTE, via the KDF extension - dm-verity - dm-crypt, via the ESSIV support - dm-integrity, via the "internal hash" mode with no key given - drbd (Distributed Replicated Block Device) This bug is especially bad for KEYCTL_DH_COMPUTE as that requires no privileges to call. Fix the bug for all users by adding a flag CRYPTO_TFM_NEED_KEY to the ->crt_flags of each hash transform that indicates whether the transform still needs to be keyed or not. Then, make the hash init, import, and digest functions return -ENOKEY if the key is still needed. The new flag also replaces the 'has_key' bool which algif_hash was previously using, thereby simplifying the algif_hash implementation. Reported-by: syzbot <syzkaller@googlegroups.com> Cc: stable@vger.kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: hash - annotate algorithms taking optional keyEric Biggers2018-01-124-8/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We need to consistently enforce that keyed hashes cannot be used without setting the key. To do this we need a reliable way to determine whether a given hash algorithm is keyed or not. AF_ALG currently does this by checking for the presence of a ->setkey() method. However, this is actually slightly broken because the CRC-32 algorithms implement ->setkey() but can also be used without a key. (The CRC-32 "key" is not actually a cryptographic key but rather represents the initial state. If not overridden, then a default initial state is used.) Prepare to fix this by introducing a flag CRYPTO_ALG_OPTIONAL_KEY which indicates that the algorithm has a ->setkey() method, but it is not required to be called. Then set it on all the CRC-32 algorithms. The same also applies to the Adler-32 implementation in Lustre. Also, the cryptd and mcryptd templates have to pass through the flag from their underlying algorithm. Cc: stable@vger.kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: poly1305 - remove ->setkey() methodEric Biggers2018-01-121-12/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since Poly1305 requires a nonce per invocation, the Linux kernel implementations of Poly1305 don't use the crypto API's keying mechanism and instead expect the key and nonce as the first 32 bytes of the data. But ->setkey() is still defined as a stub returning an error code. This prevents Poly1305 from being used through AF_ALG and will also break it completely once we start enforcing that all crypto API users (not just AF_ALG) call ->setkey() if present. Fix it by removing crypto_poly1305_setkey(), leaving ->setkey as NULL. Cc: stable@vger.kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: mcryptd - pass through absence of ->setkey()Eric Biggers2018-01-121-1/+2
| | | | | | | | | | | | | | | | | | | | | | When the mcryptd template is used to wrap an unkeyed hash algorithm, don't install a ->setkey() method to the mcryptd instance. This change is necessary for mcryptd to keep working with unkeyed hash algorithms once we start enforcing that ->setkey() is called when present. Cc: stable@vger.kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: cryptd - pass through absence of ->setkey()Eric Biggers2018-01-121-1/+2
| | | | | | | | | | | | | | | | | | | | | | When the cryptd template is used to wrap an unkeyed hash algorithm, don't install a ->setkey() method to the cryptd instance. This change is necessary for cryptd to keep working with unkeyed hash algorithms once we start enforcing that ->setkey() is called when present. Cc: stable@vger.kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: hash - introduce crypto_hash_alg_has_setkey()Eric Biggers2018-01-121-0/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | Templates that use an shash spawn can use crypto_shash_alg_has_setkey() to determine whether the underlying algorithm requires a key or not. But there was no corresponding function for ahash spawns. Add it. Note that the new function actually has to support both shash and ahash algorithms, since the ahash API can be used with either. Cc: stable@vger.kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: tcrypt - free xoutbuf instead of axbufColin Ian King2018-01-121-1/+1
| | | | | | | | | | | | | | | | | | | | | | There seems to be a cut-n-paste bug with the name of the buffer being free'd, xoutbuf should be used instead of axbuf. Detected by CoverityScan, CID#1463420 ("Copy-paste error") Fixes: 427988d981c4 ("crypto: tcrypt - add multibuf aead speed test") Signed-off-by: Colin Ian King <colin.king@canonical.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: tcrypt - fix spelling mistake: "bufufer"-> "buffer"Colin Ian King2018-01-121-2/+2
| | | | | | | | | | | | | | Trivial fix to spelling mistakes in pr_err error message text. Signed-off-by: Colin Ian King <colin.king@canonical.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: af_alg - whitelist mask and typeStephan Mueller2018-01-121-4/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | The user space interface allows specifying the type and mask field used to allocate the cipher. Only a subset of the possible flags are intended for user space. Therefore, white-list the allowed flags. In case the user space caller uses at least one non-allowed flag, EINVAL is returned. Reported-by: syzbot <syzkaller@googlegroups.com> Cc: <stable@vger.kernel.org> Signed-off-by: Stephan Mueller <smueller@chronox.de> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: testmgr - change `guard` to unsigned charJoey Pabalinas2018-01-121-1/+1
| | | | | | | | | | | | | | | | | | | | When char is signed, storing the values 0xba (186) and 0xad (173) in the `guard` array produces signed overflow. Change the type of `guard` to static unsigned char to correct undefined behavior and reduce function stack usage. Signed-off-by: Joey Pabalinas <joeypabalinas@gmail.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: poly1305 - remove cra_alignmaskEric Biggers2018-01-051-1/+0
| | | | | | | | | | | | | | | | | | Now that nothing in poly1305-generic assumes any special alignment, remove the cra_alignmask so that the crypto API does not have to unnecessarily align the buffers. Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: poly1305 - use unaligned access macros to output digestEric Biggers2018-01-051-5/+4
| | | | | | | | | | | | | | | | | | | | Currently the only part of poly1305-generic which is assuming special alignment is the part where the final digest is written. Switch this over to the unaligned access macros so that we'll be able to remove the cra_alignmask. Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: algapi - remove unused notificationsEric Biggers2018-01-052-8/+0
| | | | | | | | | | | | | | | | | | | | | | | | There is a message posted to the crypto notifier chain when an algorithm is unregistered, and when a template is registered or unregistered. But nothing is listening for those messages; currently there are only listeners for the algorithm request and registration messages. Get rid of these unused notifications for now. Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: algapi - convert cra_refcnt to refcount_tEric Biggers2018-01-055-10/+10
| | | | | | | | | | | | | | | | | | | | | | Reference counters should use refcount_t rather than atomic_t, since the refcount_t implementation can prevent overflows, reducing the exploitability of reference leak bugs. crypto_alg.cra_refcount is a reference counter with the usual semantics, so switch it over to refcount_t. Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: tcrypt - add multibuf aead speed testGilad Ben-Yossef2017-12-281-59/+378
| | | | | | | | | | | | | | | | | | | | | | | | The performance of some aead tfm providers is affected by the amount of parallelism possible with the processing. Introduce an async aead concurrent multiple buffer processing speed test to be able to test performance of such tfm providers. Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: tcrypt - add multibuf skcipher speed testGilad Ben-Yossef2017-12-281-0/+460
| | | | | | | | | | | | | | | | | | | | | | | | The performance of some skcipher tfm providers is affected by the amount of parallelism possible with the processing. Introduce an async skcipher concurrent multiple buffer processing speed test to be able to test performance of such tfm providers. Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: tcrypt - add multi buf ahash jiffies testGilad Ben-Yossef2017-12-281-30/+82
| | | | | | | | | | | | | | | | | | | | | | The multi buffer concurrent requests ahash speed test only supported the cycles mode. Add support for the so called jiffies mode that test performance of bytes/sec. We only add support for digest mode at the moment. Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: tcrypt - allow setting num of bufsGilad Ben-Yossef2017-12-281-12/+19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | For multiple buffers speed tests, the number of buffers, or requests, used actually sets the level of parallelism a tfm provider may utilize to hide latency. The existing number (of 8) is good for some software based providers but not enough for many HW providers with deep FIFOs. Add a module parameter that allows setting the number of multiple buffers/requests used, leaving the default at 8. Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: tcrypt - fix AEAD decryption speed testGilad Ben-Yossef2017-12-281-1/+29
| | | | | | | | | | | | | | | | | | | | | | | | | | The AEAD speed test pretended to support decryption, however that support was broken as decryption requires a valid auth field which the test did not provide. Fix this by running the encryption path once with inout/output sgls switched to calculate the auth field prior to performing decryption speed tests. Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * crypto: tcrypt - use multi buf for ahash mb testGilad Ben-Yossef2017-12-281-4/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The multi buffer ahash speed test was allocating multiple buffers for use with the multiple outstanding requests it was starting but never actually using them (except to free them), instead using a different single statically allocated buffer for all requests. Fix this by actually using the allocated buffers for the test. It is noted that it may seem tempting to instead remove the allocation and free of the multiple buffers and leave things as they are since this is a hash test where the input is read only. However, after consideration I believe that multiple buffers better reflect real life scenario with regard to data cache and TLB behaviours etc. Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * Merge git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6Herbert Xu2017-12-2213-92/+125
| |\ | | | | | | | | | Merge the crypto tree to pick up inside-secure fixes.
| * | crypto: seqiv - Remove unused alg/spawn variableCorentin Labbe2017-12-221-5/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | This patch remove two unused variable and some dead "code" using it. Fixes: 92932d03c2b3 ("crypto: seqiv - Remove AEAD compatibility code") Signed-off-by: Corentin Labbe <clabbe@baylibre.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * | crypto: echainiv - Remove unused alg/spawn variableCorentin Labbe2017-12-221-5/+0
| | | | | | | | | | | | | | | | | | | | | | | | This patch remove two unused variable and some dead "code" using it. Fixes: 66008d4230f6 ("crypto: echainiv - Remove AEAD compatibility code") Signed-off-by: Corentin Labbe <clabbe@baylibre.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * | crypto: gf128mul - remove incorrect commentEric Biggers2017-12-221-2/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | The comment in gf128mul_x8_ble() was copy-and-pasted from gf128mul.h and makes no sense in the new context. Remove it. Cc: Harsh Jain <harsh@chelsio.com> Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * | crypto: null - Get rid of crypto_{get,put}_default_null_skcipher2()Eric Biggers2017-12-225-11/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since commit 499a66e6b689 ("crypto: null - Remove default null blkcipher"), crypto_get_default_null_skcipher2() and crypto_put_default_null_skcipher2() are the same as their non-2 equivalents. So switch callers of the "2" versions over to the original versions and remove the "2" versions. Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
| * | crypto: api - Unexport crypto_larval_lookup()Eric Biggers2017-12-222-3/+2
| | | | | | | | | | | | | | | | | | | | | | | | crypto_larval_lookup() is not used outside of crypto/api.c, so unexport it and mark it 'static'. Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>