[PATCH] pkcs7: return correct error code if pkcs7_check_authattrs() fails

2017-11-26 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> If pkcs7_check_authattrs() returns an error code, we should pass that error code on, rather than using ENOMEM. Fixes: 99db44350672 ("PKCS#7: Appropriately restrict authenticated attributes and content type") Signed-off-by: Eri

[PATCH] X.509: fix printing uninitialized stack memory when OID is empty

2017-11-26 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> Callers of sprint_oid() do not check its return value before printing the result. In the case where the OID is zero-length, -EBADMSG was being returned without anything being written to the buffer, resulting in uninitialized stack memory being p

[PATCH] X.509: fix buffer overflow detection in sprint_oid()

2017-11-26 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> In sprint_oid(), if the input buffer were to be more than 1 byte too small for the first snprintf(), 'bufsize' would underflow, causing a buffer overflow when printing the remainder of the OID. Fortunately this cannot actually happen currently, b

[PATCH] X.509: fix comparisons of ->pkey_algo

2017-11-26 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> ->pkey_algo used to be an enum, but was changed to a string by commit 4e8ae72a75aa ("X.509: Make algo identifiers text instead of enum"). But two comparisons were not updated. Fix them to use strcmp(). This bug broke signature veri

[PATCH] X.509: reject invalid BIT STRING for subjectPublicKey

2017-11-26 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> Adding a specially crafted X.509 certificate whose subjectPublicKey ASN.1 value is zero-length caused x509_extract_key_data() to set the public key size to SIZE_MAX, as it subtracted the nonexistent BIT STRING metadata byte. Then, x509_cert_parse()

[PATCH] ASN.1: check for error from ASN1_OP_END__ACT actions

2017-11-26 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> asn1_ber_decoder() was ignoring errors from actions associated with the opcodes ASN1_OP_END_SEQ_ACT, ASN1_OP_END_SET_ACT, ASN1_OP_END_SEQ_OF_ACT, and ASN1_OP_END_SET_OF_ACT. In practice, this meant the pkcs7_note_signed_info() action

[PATCH] ASN.1: fix out-of-bounds read when parsing indefinite length item

2017-11-26 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> In asn1_ber_decoder(), indefinitely-sized ASN.1 items were being passed to the action functions before their lengths had been computed, using the bogus length of 0x80 (ASN1_INDEFINITE_LENGTH). This resulted in reading data past the end of the input

Re: [PATCH 5/5] crypto: chacha20 - Fix keystream alignment for chacha20_block()

2017-11-22 Thread Eric Biggers
On Wed, Nov 22, 2017 at 08:51:57PM +, Ard Biesheuvel wrote: > On 22 November 2017 at 19:51, Eric Biggers <ebigge...@gmail.com> wrote: > > From: Eric Biggers <ebigg...@google.com> > > > > When chacha20_block() outputs the keystream block, it uses 'u32' stores &g

[PATCH 3/5] crypto: chacha20 - Remove cra_alignmask

2017-11-22 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> Now that crypto_chacha20_setkey() and crypto_chacha20_init() use the unaligned access macros and crypto_xor() also accepts unaligned buffers, there is no need to have a cra_alignmask set for chacha20-generic. Signed-off-by: Eric Biggers

[PATCH 2/5] crypto: chacha20 - Use unaligned access macros when loading key and IV

2017-11-22 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> The generic ChaCha20 implementation has a cra_alignmask of 3, which ensures that the key passed into crypto_chacha20_setkey() and the IV passed into crypto_chacha20_init() are 4-byte aligned. However, these functions are also called from the ARM and

[PATCH 1/5] crypto: chacha20 - Fix unaligned access when loading constants

2017-11-22 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> The four 32-bit constants for the initial state of ChaCha20 were loaded from a char array which is not guaranteed to have the needed alignment. Fix it by just assigning the constants directly instead. Signed-off-by: Eric Biggers <ebigg...@g

[PATCH 5/5] crypto: chacha20 - Fix keystream alignment for chacha20_block()

2017-11-22 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> When chacha20_block() outputs the keystream block, it uses 'u32' stores directly. However, the callers (crypto/chacha20_generic.c and drivers/char/random.c) declare the keystream buffer as a 'u8' array, which is not guaranteed to have the needed ali

[PATCH 0/5] crypto: chacha20 - Alignment fixes

2017-11-22 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> This series fixes potentially unaligned memory accesses when loading the initial state, key, and IV for ChaCha20, and when outputting each keystream block. It also removes the cra_alignmask from the generic and x86 ChaCha20 implementations

[PATCH 4/5] crypto: x86/chacha20 - Remove cra_alignmask

2017-11-22 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> Now that the generic ChaCha20 implementation no longer needs a cra_alignmask, the x86 one doesn't either -- given that the x86 implementation doesn't need the alignment itself. Signed-off-by: Eric Biggers <ebigg...@google.com> --- arc

Re: x509 parsing bug + fuzzing crypto in the userspace

2017-11-21 Thread Eric Biggers
On Tue, Nov 21, 2017 at 09:00:26AM +0100, Dmitry Vyukov wrote: > > > > Note that separate from asymmetric_keys (which you can think of as being > > in-between the keyrings subsystem and the crypto subsystem) there is also > > the > > userspace interface to cryptographic algorithms, AF_ALG. It

Re: x509 parsing bug + fuzzing crypto in the userspace

2017-11-20 Thread Eric Biggers
+Cc keyri...@vger.kernel.org (for asymmetric_keys) First of all, thanks for working on this! A lot of this code really needs to be better tested. On Mon, Nov 20, 2017 at 03:10:55PM +0100, Alexander Potapenko wrote: > Hi all, > > TL;DR userspace fuzzing may be very effective for finding bugs in

[PATCH -stable] arm: crypto: reduce priority of bit-sliced AES cipher

2017-11-17 Thread Eric Biggers
250. This was fixed upstream by commit cc477bf64573 ("crypto: arm/aes - replace bit-sliced OpenSSL NEON code"), but it was just a small part of a complete rewrite. This patch just fixes the priority bug for older kernels. Signed-off-by: Eric Biggers <ebigg...@google.com> --- arch/arm/cryp

Re: general protection fault in asn1_ber_decoder

2017-11-08 Thread Eric Biggers
On Mon, Nov 06, 2017 at 10:36:00AM -0800, syzbot wrote: > > syzbot will keep track of this bug report. > Once a fix for this bug is committed, please reply to this email with: > #syz fix: exact-commit-title > To mark this as a duplicate of another syzbot report, please reply with: > #syz dup:

Re: [PATCH v2] lib/mpi: call cond_resched() from mpi_powm() loop

2017-11-08 Thread Eric Biggers
On Wed, Nov 08, 2017 at 08:17:12PM +, David Howells wrote: > Eric Biggers <ebigge...@gmail.com> wrote: > > > This probably should be grouped with my series "crypto: dh - input > > validation > > fixes", as this is also a fix for Diffie-Hellman.

[PATCH v2] lib/mpi: call cond_resched() from mpi_powm() loop

2017-11-07 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> On a non-preemptible kernel, if KEYCTL_DH_COMPUTE is called with the largest permitted inputs (16384 bits), the kernel spends 10+ seconds doing modular exponentiation in mpi_powm() without rescheduling. If all threads do it, it locks up the

Re: [PATCH] lib/mpi: call cond_resched() from mpi_powm() loop

2017-11-07 Thread Eric Biggers
On Tue, Nov 07, 2017 at 10:38:30AM -0800, Mat Martineau wrote: > > Eric, > > On Mon, 6 Nov 2017, Eric Biggers wrote: > > >From: Eric Biggers <ebigg...@google.com> > > > >On a non-preemptible kernel, if KEYCTL_DH_COMPUTE is called with the > >larg

[PATCH] lib/mpi: call cond_resched() from mpi_powm() loop

2017-11-06 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> On a non-preemptible kernel, if KEYCTL_DH_COMPUTE is called with the largest permitted inputs (16384 bits), the kernel spends 10+ seconds doing modular exponentiation in mpi_powm() without rescheduling. If all threads do it, it locks up the

Re: general protection fault in asn1_ber_decoder

2017-11-06 Thread Eric Biggers
later in the function. How about this: commit 8bbe85872c660c891eb978a037f590198319e3b2 Author: Eric Biggers <ebigg...@google.com> Date: Mon Nov 6 10:06:32 2017 -0800 KEYS: fix NULL pointer dereference during ASN.1 parsing syzkaller reported a NULL pointer dereference in asn1_ber_decoder

Re: general protection fault in asn1_ber_decoder

2017-11-06 Thread Eric Biggers
On Mon, Nov 06, 2017 at 10:36:00AM -0800, syzbot wrote: > kasan: GPF could be caused by NULL-ptr deref or user memory access > general protection fault: [#1] SMP KASAN > Dumping ftrace buffer: >(ftrace buffer empty) > Modules linked in: > CPU: 3 PID: 2984 Comm: syzkaller229187 Not tainted

[PATCH v2 2/5] crypto: dh - Don't permit 'p' to be 0

2017-11-05 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> 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_val

[PATCH v2 1/5] crypto: dh - Fix double free of ctx->p

2017-11-05 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> When setting the secret with the software Diffie-Hellman implementation, if allocating 'g' failed (e.g. if it was longer than MAX_EXTERN_MPI_BITS), then 'p' was freed twice: once immediately, and once later when the crypto_kpp tfm was destroyed

[PATCH v2 4/5] crypto: qat - Clean up error handling in qat_dh_set_secret()

2017-11-05 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> Update the error handling in qat_dh_set_secret() to mirror dh_set_secret(). The new version is less error-prone because freeing memory and setting the pointers to NULL is now only done in one place. Signed-off-by: Eric Biggers <ebigg...@g

[PATCH v2 3/5] crypto: dh - Don't permit 'key' or 'g' size longer than 'p'

2017-11-05 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> 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_

[PATCH v2 5/5] crypto: dh - Remove pointless checks for NULL 'p' and 'g'

2017-11-05 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> Neither 'p' nor 'g' can be NULL, as they were unpacked using crypto_dh_decode_key(). And it makes no sense for them to be optional. So remove the NULL checks that were copy-and-pasted into both modules. Signed-off-by: Eric Biggers <ebigg...@g

[PATCH v2 0/5] crypto: dh - input validation fixes

2017-11-05 Thread Eric Biggers
. With the QAT DH implementation, setting 'key' or 'g' larger than 'p' caused a buffer underflow. Note that in kernels configured with CONFIG_KEY_DH_OPERATIONS=y, these bugs are reachable by unprivileged users via KEYCTL_DH_COMPUTE. Patches 4 and 5 are cleanup only. Eric Biggers (5): crypto: dh - Fix

Re: [PATCH 2/4] crypto: dh - don't permit 'p' to be 0

2017-11-02 Thread Eric Biggers
On Thu, Nov 02, 2017 at 01:40:51PM +0200, Tudor Ambarus wrote: > Hi, Eric, > > On 11/02/2017 12:25 AM, Eric Biggers wrote: > >If 'p' is 0 for the software Diffie-Hellman implementation, then > >dh_max_size() returns 0. > > dh_set_secret() returns -

Re: [PATCH 1/4] crypto: dh - fix double free of ctx->p

2017-11-02 Thread Eric Biggers
Hi Tudor, On Thu, Nov 02, 2017 at 12:55:56PM +0200, Tudor Ambarus wrote: > Hi, Eric, > > On 11/02/2017 12:25 AM, Eric Biggers wrote: > >When setting the secret with the software Diffie-Hellman implementation, > >if allocating 'g' failed (e.g. if it was longer than &

Re: invalid opcode: 0000 [#1] SMP [aesni_intel]

2017-11-01 Thread Eric Biggers
On Mon, Oct 23, 2017 at 07:01:32PM +0300, SviMik wrote: > Hi! > > Got the following kernel panic: > > invalid opcode: [#1] SMP > CPU: 0 PID: 1449 Comm: openvpn Not tainted 4.8.13-1.el6.elrepo.x86_64 #1 > cut > Call Trace: > > [] ? enqueue_entity+0x45e/0x6f0 > [] ?

[PATCH 1/4] crypto: dh - fix double free of ctx->p

2017-11-01 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> When setting the secret with the software Diffie-Hellman implementation, if allocating 'g' failed (e.g. if it was longer than MAX_EXTERN_MPI_BITS), then 'p' was freed twice: once immediately, and once later when the crypto_kpp tfm was destroyed

[PATCH 2/4] crypto: dh - don't permit 'p' to be 0

2017-11-01 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> 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_POINTER to be passed to sg_init_one(), which with CONFIG_DEBUG_SG=y triggers the 'BUG_ON(!virt_addr_val

[PATCH 0/4] crypto: dh - input validation fixes

2017-11-01 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> This series fixes several corner cases in the Diffie-Hellman key exchange implementations: - With CONFIG_DEBUG_SG=y and the software DH implementation, setting 'p' to 0 caused a BUG_ON(). - Both the software and QAT DH implementations had a doubl

[PATCH 3/4] crypto: qat - fix double free of ctx->p

2017-11-01 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> When setting the secret with the "qat-dh" Diffie-Hellman implementation, if allocating 'g' failed, then 'p' was freed twice: once immediately, and once later when the crypto_kpp tfm was destroyed. Fix it by using qat_dh_clear_ctx() in

[PATCH 4/4] crypto: dh - don't permit 'key' or 'g' size longer than 'p'

2017-11-01 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> 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 allowed users to cause a buffer underflow via KEYCTL_

Re: [GIT PULL] KEYS: Fixes and crypto fixes

2017-09-27 Thread Eric Biggers
On Thu, Sep 28, 2017 at 09:14:58AM +1000, James Morris wrote: > On Wed, 27 Sep 2017, David Howells wrote: > > > (2) Fixing big_key to use safe crypto from Jason A. Donenfeld. > > > > I'm concerned about the lack of crypto review mentioned by Jason -- I > wonder if we can get this rewrite any

Re: [PATCH 00/12] x86/crypto: Fix RBP usage in several crypto .S files

2017-09-14 Thread Eric Biggers
Hi Josh, On Wed, Sep 13, 2017 at 05:33:03PM -0500, Josh Poimboeuf wrote: > And here's v2 of the sha512-avx2 patch. It should hopefully gain back > most of the performance lost by v1. > > From: Josh Poimboeuf > Subject: [PATCH] x86/crypto: Fix RBP usage in sha512-avx2-asm.S

Re: [PATCH 00/12] x86/crypto: Fix RBP usage in several crypto .S files

2017-09-08 Thread Eric Biggers
On Thu, Sep 07, 2017 at 11:26:47PM +0200, Ingo Molnar wrote: > > * Eric Biggers <ebigge...@gmail.com> wrote: > > > On Thu, Sep 07, 2017 at 09:15:34AM +0200, Ingo Molnar wrote: > > > > > > * Eric Biggers <ebigge...@gmail.com> wrote: > > >

Re: [PATCH 00/12] x86/crypto: Fix RBP usage in several crypto .S files

2017-09-07 Thread Eric Biggers
On Thu, Sep 07, 2017 at 09:15:34AM +0200, Ingo Molnar wrote: > > * Eric Biggers <ebigge...@gmail.com> wrote: > > > Thanks for fixing these! I don't have time to review these in detail, but > > I ran > > the crypto self-tests on the affected algor

Re: [PATCH 00/12] x86/crypto: Fix RBP usage in several crypto .S files

2017-09-01 Thread Eric Biggers
Hi Josh, On Tue, Aug 29, 2017 at 01:05:33PM -0500, Josh Poimboeuf wrote: > Many of the x86 crypto functions use RBP as a temporary register. This > breaks frame pointer convention, and breaks stack traces when unwinding > from an interrupt in the crypto code. > > Convert most* of them to leave

Re: [PATCH v5 2/5] lib: Add zstd modules

2017-08-10 Thread Eric Biggers
On Thu, Aug 10, 2017 at 01:41:21PM -0400, Chris Mason wrote: > On 08/10/2017 04:30 AM, Eric Biggers wrote: > >On Wed, Aug 09, 2017 at 07:35:53PM -0700, Nick Terrell wrote: > > >>The memory reported is the amount of memory the compressor requests. > >> > >>|

Re: [PATCH v5 2/5] lib: Add zstd modules

2017-08-10 Thread Eric Biggers
On Thu, Aug 10, 2017 at 10:57:01AM -0400, Austin S. Hemmelgarn wrote: > Also didn't think to mention this, but I could see the max level > being very popular for use with SquashFS root filesystems used in > LiveCD's. Currently, they have to decide between read performance > and image size, while

Re: [PATCH v5 2/5] lib: Add zstd modules

2017-08-10 Thread Eric Biggers
On Thu, Aug 10, 2017 at 07:32:18AM -0400, Austin S. Hemmelgarn wrote: > On 2017-08-10 04:30, Eric Biggers wrote: > >On Wed, Aug 09, 2017 at 07:35:53PM -0700, Nick Terrell wrote: > >> > >>It can compress at speeds approaching lz4, and quality approaching lzma. &g

Re: [PATCH v5 2/5] lib: Add zstd modules

2017-08-10 Thread Eric Biggers
On Wed, Aug 09, 2017 at 07:35:53PM -0700, Nick Terrell wrote: > > It can compress at speeds approaching lz4, and quality approaching lzma. Well, for a very loose definition of "approaching", and certainly not at the same time. I doubt there's a use case for using the highest compression levels

[PATCH v2 3/7] fscrypt: use HKDF-SHA512 to derive the per-inode encryption keys

2017-07-26 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> By design, the keys which userspace provides in the keyring are not used to encrypt data directly. Instead, a KDF (Key Derivation Function) is used to derive a unique encryption key for each inode, given a "master" key and a nonce.

[PATCH v2 6/7] fscrypt: cache the HMAC transform for each master key

2017-07-26 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> Now that we have a key_hash field which securely identifies a master key payload, introduce a cache of the HMAC transforms for the master keys currently in use for inodes using v2+ encryption policies. The entries in this cache are called '

[PATCH v2 7/7] fscrypt: for v2 policies, support "fscrypt:" key prefix only

2017-07-26 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> Since v2 encryption policies are opt-in, take the opportunity to also drop support for the legacy filesystem-specific key description prefixes "ext4:", "f2fs:", and "ubifs:", instead requiring the generic prefix "fs

[PATCH v2 5/7] fscrypt: verify that the correct master key was supplied

2017-07-26 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> Currently, while a fscrypt master key is required to have a certain description in the keyring, its payload is never verified to be correct. While sufficient for well-behaved userspace, this is insecure in a multi-user system where a user has been

[PATCH v2 1/7] fscrypt: add v2 encryption context and policy

2017-07-26 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> Currently, the fscrypt_context (i.e. the encryption xattr) does not contain a cryptographically secure identifier for the master key's payload. Therefore it's not possible to verify that the correct key was supplied, which is problematic in mult

[PATCH v2 0/7] fscrypt: key verification and KDF improvement

2017-07-26 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> This patch series solves two major problems which filesystem-level encryption has currently. First, the user-supplied master keys are not verified, which means a malicious user can provide the wrong key for another user's file and cause a DOS

Re: [PATCH v4 0/8] crypto: aes - retire table based generic AES

2017-07-24 Thread Eric Biggers
On Mon, Jul 24, 2017 at 07:59:43AM +0100, Ard Biesheuvel wrote: > On 18 July 2017 at 13:06, Ard Biesheuvel wrote: > > The generic AES driver uses 16 lookup tables of 1 KB each, and has > > encryption and decryption routines that are fully unrolled. Given how > > the

Re: [PATCH 5/6] fscrypt: cache the HMAC transform for each master key

2017-07-19 Thread Eric Biggers
On Mon, Jul 17, 2017 at 10:45:51AM -0700, Michael Halcrow wrote: > > +/* > > + * Get the fscrypt_master_key identified by the specified v2+ encryption > > + * context, or create it if not found. > > + * > > + * Returns the fscrypt_master_key with a reference taken, or an ERR_PTR(). > > + */ > >

Re: [PATCH 3/6] fscrypt: use HKDF-SHA512 to derive the per-inode encryption keys

2017-07-19 Thread Eric Biggers
On Fri, Jul 14, 2017 at 09:24:40AM -0700, Michael Halcrow wrote: > > +static int hkdf_expand(struct crypto_shash *hmac, u8 context, > > + const u8 *info, unsigned int infolen, > > + u8 *okm, unsigned int okmlen) > > +{ > > + SHASH_DESC_ON_STACK(desc, hmac); > >

Re: [PATCH 1/6] fscrypt: add v2 encryption context and policy

2017-07-13 Thread Eric Biggers
Hi Michael, On Thu, Jul 13, 2017 at 03:29:44PM -0700, Michael Halcrow wrote: > On Wed, Jul 12, 2017 at 02:00:30PM -0700, Eric Biggers wrote: > > From: Eric Biggers <ebigg...@google.com> > > > > Currently, the fscrypt_context (i.e. the encryption xattr) does not >

Re: [PATCH 3/6] fscrypt: use HKDF-SHA512 to derive the per-inode encryption keys

2017-07-13 Thread Eric Biggers
Hi Stephan, On Thu, Jul 13, 2017 at 04:54:55PM +0200, Stephan Müller wrote: > Am Mittwoch, 12. Juli 2017, 23:00:32 CEST schrieb Eric Biggers: > > Hi Herbert, > > This patch adds a second KDF to the kernel -- the first is found in the keys > subsystem. > > Th

[PATCH 5/6] fscrypt: cache the HMAC transform for each master key

2017-07-12 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> Now that we have a key_hash field which securely identifies a master key payload, introduce a cache of the HMAC transforms for the master keys currently in use for inodes using v2+ encryption policies. The entries in this cache are called '

[PATCH 2/6] fscrypt: rename ->ci_master_key to ->ci_master_key_descriptor

2017-07-12 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> In struct fscrypt_info, ->ci_master_key is the master key descriptor, not the master key itself. In preparation for introducing a struct fscrypt_master_key and making ->ci_master_key point to it, rename the existing -&g

[PATCH 3/6] fscrypt: use HKDF-SHA512 to derive the per-inode encryption keys

2017-07-12 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> By design, the keys which userspace provides in the keyring are not used to encrypt data directly. Instead, a KDF (Key Derivation Function) is used to derive a unique encryption key for each inode, given a "master" key and a nonce.

[PATCH 4/6] fscrypt: verify that the correct master key was supplied

2017-07-12 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> Currently, while a fscrypt master key is required to have a certain description in the keyring, its payload is never verified to be correct. While sufficient for well-behaved userspace, this is insecure in a multi-user system where a user has been

[PATCH 6/6] fscrypt: for v2 policies, support "fscrypt:" key prefix only

2017-07-12 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> Since v2 encryption policies are opt-in, take the opportunity to also drop support for the legacy filesystem-specific key description prefixes "ext4:", "f2fs:", and "ubifs:", instead requiring the generic prefix "fs

[PATCH 1/6] fscrypt: add v2 encryption context and policy

2017-07-12 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> Currently, the fscrypt_context (i.e. the encryption xattr) does not contain a cryptographically secure identifier for the master key's payload. Therefore it's not possible to verify that the correct key was supplied, which is problematic in mult

[PATCH 0/6] fscrypt: key verification and KDF improvement

2017-07-12 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> This patch series solves two major problems which filesystem-level encryption has currently. First, the user-supplied master keys are not verified, which means a malicious user can provide the wrong key for another user's file and cause a DOS

Re: [PATCH v3 net-next 3/4] tls: kernel TLS support

2017-07-11 Thread Eric Biggers
On Tue, Jul 11, 2017 at 11:53:11AM -0700, Dave Watson wrote: > On 07/11/17 08:29 AM, Steffen Klassert wrote: > > Sorry for replying to old mail... > > > +int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx) > > > +{ > > > > ... > > > > > + > > > + if (!sw_ctx->aead_send) { > > > +

Re: [PATCH v6 0/2] IV Generation algorithms for dm-crypt

2017-06-23 Thread Eric Biggers
On Fri, Jun 23, 2017 at 04:13:41PM +0800, Herbert Xu wrote: > Binoy Jayan wrote: > > === > > dm-crypt optimization for larger block sizes > >

Re: [PATCH v2 0/6] crypto: aes - allow generic AES to be omitted

2017-06-18 Thread Eric Biggers
Hi Ard, On Fri, Jun 16, 2017 at 01:17:43PM +0200, Ard Biesheuvel wrote: > The generic AES driver uses 16 lookup tables of 1 KB each, and has > encryption and decryption routines that are fully unrolled. Given how > the dependencies between this code and other drivers are declared in > Kconfig

Re: [kernel-hardening] Re: [PATCH v3 04/13] crypto/rng: ensure that the RNG is ready before using

2017-06-06 Thread Eric Biggers
Hi Jason, On Tue, Jun 06, 2017 at 05:23:04PM +0200, Jason A. Donenfeld wrote: > Hey again Eric, > > One thing led to another and I wound up just rewriting all the crypto > in big_keys.c. I'll include this for v4: > >

Re: [kernel-hardening] Re: [PATCH v3 04/13] crypto/rng: ensure that the RNG is ready before using

2017-06-05 Thread Eric Biggers
On Tue, Jun 06, 2017 at 05:56:20AM +0200, Jason A. Donenfeld wrote: > Hey Ted, > > On Tue, Jun 6, 2017 at 5:00 AM, Theodore Ts'o wrote: > > Note that crypto_rng_reset() is called by big_key_init() in > > security/keys/big_key.c as a late_initcall(). So if we are on a > > system

Re: [PATCH] crypto: x86/aes - Don't use %rbp as temporary register

2017-05-18 Thread Eric Biggers
On Thu, May 18, 2017 at 08:56:32PM -0500, Josh Poimboeuf wrote: > > > > Hmm, it looks like a number of other algorithms in arch/x86/crypto/ use > > %rbp (or > > %ebp), e.g. blowfish, camellia, cast5, and aes-i586. Presumably they have > > the > > same problem. I'm a little confused: do these

Re: [PATCH] crypto: x86/aes - Don't use %rbp as temporary register

2017-05-17 Thread Eric Biggers
On Wed, May 17, 2017 at 03:44:27PM -0500, Josh Poimboeuf wrote: > On Tue, May 16, 2017 at 09:03:08PM -0700, Eric Biggers wrote: > > From: Eric Biggers <ebigg...@google.com> > > > > When using the "aes-asm" implementation of AES (*not* the AES-NI > > imp

[PATCH] crypto: x86/aes - Don't use %rbp as temporary register

2017-05-16 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> When using the "aes-asm" implementation of AES (*not* the AES-NI implementation) on an x86_64, v4.12-rc1 kernel with lockdep enabled, the following warning was reported, along with a long unwinder dump: WARNING: k

Re: [PATCH 4/4] crypto: Documentation: fix none signal safe sample

2017-05-16 Thread Eric Biggers
ill DMA from/into the buffers. > > Resolve this by using wait_for_completion() instead. > > Reported-by: Eric Biggers <ebigge...@gmail.com> > Signed-off-by: Gilad Ben-Yossef <gi...@benyossef.com> > --- > Documentation/crypto/api-samples.rst | 2 +- > 1 file change

Re: [PATCH 2/4] crypto: drbg wait for crypto op not signal safe

2017-05-16 Thread Eric Biggers
; corrupting the output buffer. > > Resolve this by using wait_for_completion() instead. > > Reported-by: Eric Biggers <ebigge...@gmail.com> > Signed-off-by: Gilad Ben-Yossef <gi...@benyossef.com> > CC: sta...@vger.kernel.org > --- > crypto/drbg.c | 3 +-- > 1 fil

Re: [RFC 01/10] crypto: factor async completion for general use

2017-05-11 Thread Eric Biggers
On Thu, May 11, 2017 at 10:29:47AM +0300, Gilad Ben-Yossef wrote: > > With regards to the wait being uninterruptible, I agree that this should be > > the > > default behavior, because I think users waiting for specific crypto > > requests are > > generally not prepared to handle the wait

Re: [RFC 07/10] fscrypt: move to generic async completion

2017-05-10 Thread Eric Biggers
Hi Gilad, On Sat, May 06, 2017 at 03:59:56PM +0300, Gilad Ben-Yossef wrote: > int fscrypt_do_page_crypto(const struct inode *inode, fscrypt_direction_t rw, > u64 lblk_num, struct page *src_page, > struct page *dest_page, unsigned int len, > @@

Re: [RFC 01/10] crypto: factor async completion for general use

2017-05-10 Thread Eric Biggers
Hi Gilad, On Sat, May 06, 2017 at 03:59:50PM +0300, Gilad Ben-Yossef wrote: > Invoking a possibly async. crypto op and waiting for completion > while correctly handling backlog processing is a common task > in the crypto API implementation and outside users of it. > > This patch re-factors one

[PATCH] crypto: aes_ti - fix comment for MixColumns step

2017-05-09 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> mix_columns() contains a comment which shows the matrix used by the MixColumns step of AES, but the last entry in this matrix was incorrect --- and did not match the code, which is correct. Fix the comment. Signed-off-by: Eric Biggers

Re: [PATCH 3/5] KEYS: DH: don't feed uninitialized result memory into KDF

2017-04-20 Thread Eric Biggers
Hi Stephan, On Thu, Apr 20, 2017 at 08:38:30PM +0200, Stephan Müller wrote: > > > > By the way: do we really need this in the kernel at all, given that it's > > just doing some math on data which userspace has access to? > > It is the question about how we want the keys subsystem to operate.

Re: [PATCH 3/5] KEYS: DH: don't feed uninitialized result memory into KDF

2017-04-20 Thread Eric Biggers
Hi Stephan, On Thu, Apr 20, 2017 at 03:27:17PM +0200, Stephan Müller wrote: > Am Donnerstag, 20. April 2017, 07:46:31 CEST schrieb Eric Biggers: > > Hi Eric, > > > From: Eric Biggers <ebigg...@google.com> > > > > The result of the Diffie-Hellman computati

[PATCH 5/5] KEYS: DH: add __user annotations to keyctl_kdf_params

2017-04-19 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> Signed-off-by: Eric Biggers <ebigg...@google.com> --- include/uapi/linux/keyctl.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/uapi/linux/keyctl.h b/include/uapi/linux/keyctl.h index 201c6644b237..ef16df0

[PATCH 2/5] KEYS: DH: don't feed uninitialized "otherinfo" into KDF

2017-04-19 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> If userspace called KEYCTL_DH_COMPUTE with kdf_params containing NULL otherinfo but nonzero otherinfolen, the kernel would allocate a buffer for the otherinfo, then feed it into the KDF without initializing it. Fix this by always doing the cop

[PATCH 1/5] KEYS: DH: forbid using digest_null as the KDF hash

2017-04-19 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> Requesting "digest_null" in the keyctl_kdf_params caused an infinite loop in kdf_ctr() because the "null" hash has a digest size of 0. Fix it by rejecting hash algorithms with a digest size of 0. Signed-off-by: Eric Bi

[PATCH 4/5] KEYS: DH: ensure the KDF counter is properly aligned

2017-04-19 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> Accessing a 'u8[4]' through a '__be32 *' violates alignment rules. Just make the counter a __be32 instead. Signed-off-by: Eric Biggers <ebigg...@google.com> --- security/keys/dh.c | 16 +++- 1 file changed, 3 insertions(+),

[PATCH 0/5] KEYS: fixes for new keyctl_dh_compute() KDF extension

2017-04-19 Thread Eric Biggers
This patch series fixes several bugs in the KDF extension to keyctl_dh_compute() currently sitting in keys-next: a way userspace could cause an infinite loop, two ways userspace could cause the use of uninitialized memory, a misalignment, and missing __user annotations. Eric Biggers (5): KEYS

[PATCH 3/5] KEYS: DH: don't feed uninitialized result memory into KDF

2017-04-19 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> The result of the Diffie-Hellman computation may be shorter than the input prime number. Only calculate the KDF over the actual result; don't include additional uninitialized memory. Signed-off-by: Eric Biggers <ebigg...@google.com> --- s

Re: [RFC PATCH v1 1/1] crypto: algif_compression - User-space interface for compression

2017-04-16 Thread Eric Biggers
On Fri, Apr 14, 2017 at 12:04:54AM +0530, Abed Kamaluddin wrote: > crypto: algif_compression - User-space interface for compression > > This patch adds af_alg plugin for compression algorithms of type scomp/acomp > registered to the kernel crypto layer. > > The user needs to set operation

Re: [PATCH v5 0/4] gf128mul refactoring

2017-04-04 Thread Eric Biggers
tch gf128mul_x_ble to le128 > crypto: glue_helper - remove the le128_gf128mul_x_ble function > crypto: xts - drop gf128mul dependency These all look good to me, and you can add Reviewed-by: Eric Biggers <ebigg...@google.com> to the patches. I think the change to le128 is an improve

Re: [PATCH v3] crypto: gf128mul - define gf128mul_x_* in gf128mul.h

2017-03-31 Thread Eric Biggers
. > > Signed-off-by: Ondrej Mosnacek <omosna...@gmail.com> > Cc: Eric Biggers <ebigg...@google.com> Reviewed-by: Eric Biggers <ebigg...@google.com> Also, I realized that for gf128mul_x_lle() now that we aren't using the table we don't need to shift '_tt' but rather

Re: [PATCH v2] crypto: gf128mul - define gf128mul_x_* in gf128mul.h

2017-03-30 Thread Eric Biggers
e, the speed of the generic 'xts(aes)' implementation > increased from ~225 MiB/s to ~235 MiB/s (measured using 'cryptsetup > benchmark -c aes-xts-plain64' on an Intel system with CRYPTO_AES_X86_64 > and CRYPTO_AES_NI_INTEL disabled). > Reviewed-by: Eric Biggers <ebigg...@google.com> Thanks, - Eric

Re: [PATCH] crypto: gf128mul - define gf128mul_x_ble in gf128mul.h

2017-03-30 Thread Eric Biggers
Hi Ondrej, On Thu, Mar 30, 2017 at 09:25:35PM +0200, Ondrej Mosnacek wrote: > The gf128mul_x_ble function is currently defined in gf128mul.c, because > it depends on the gf128mul_table_be multiplication table. > > However, since the function is very small and only uses two values from > the

Re: [PATCH 0/7] crypto: aes - allow generic AES to be omitted

2017-03-28 Thread Eric Biggers
On Tue, Mar 28, 2017 at 09:51:54AM +0100, Ard Biesheuvel wrote: > On 28 March 2017 at 06:43, Eric Biggers <ebigge...@gmail.com> wrote: > > > > Just a thought: how about renaming CRYPTO_AES to CRYPTO_AES_GENERIC, then > > renaming what you called CRYPTO_NEED_A

Re: [PATCH 0/7] crypto: aes - allow generic AES to be omitted

2017-03-27 Thread Eric Biggers
Hi Ard, On Sun, Mar 26, 2017 at 07:49:01PM +0100, Ard Biesheuvel wrote: > The generic AES driver uses 16 lookup tables of 1 KB each, and has > encryption and decryption routines that are fully unrolled. Given how > the dependencies between this code and other drivers are declared in > Kconfig

[PATCH] crypto: xts,lrw - fix out-of-bounds write after kmalloc failure

2017-03-23 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> In the generic XTS and LRW algorithms, for input data > 128 bytes, a temporary buffer is allocated to hold the values to be XOR'ed with the data before and after encryption or decryption. If the allocation fails, the fixed-size buffer

Re: crypto: out-of-bounds write in pre_crypt

2017-03-23 Thread Eric Biggers
Hi Dmitry, On Thu, Mar 23, 2017 at 11:51:30AM +0100, Dmitry Vyukov wrote: > Hello, > > I've got the following report while running syzkaller fuzzer. > init_crypt ignores kmalloc failure, which later leads to out-of-bounds > writes in ptr_crypt. On commit >

Re: [PATCH] md5: remove from lib and only live in crypto

2017-03-23 Thread Eric Biggers
e patch itself looks good to me, and you can add Reviewed-by: Eric Biggers <ebigg...@google.com> There is a small issue, though, which is that currently cryptodev/master (where this patch would be applied to) still has md5_transform() in drivers/char/random.c, because cryptodev/master is based

[PATCH 1/2] crypto: kpp - constify buffer passed to crypto_kpp_set_secret()

2017-02-24 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> Constify the buffer passed to crypto_kpp_set_secret() and kpp_alg.set_secret, since it is never modified. Signed-off-by: Eric Biggers <ebigg...@google.com> --- crypto/dh.c | 3 ++- c

[PATCH 2/2] crypto: testmgr - constify all test vectors

2017-02-24 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> Cryptographic test vectors should never be modified, so constify them to enforce this at both compile-time and run-time. This moves a significant amount of data from .data to .rodata when the crypto tests are enabled. Signed-off-by: Eric Biggers

[PATCH 0/2] crypto: constify test vectors

2017-02-24 Thread Eric Biggers
From: Eric Biggers <ebigg...@google.com> These two patches mark all the cryptographic test vectors as 'const'. This has several potential advantages and moves a large amount of data from .data to .rodata when the tests are enabled. The second patch does the real work; the first just pr

<    1   2   3   4   5   >