[PATCH 4/5] crypto: testmgr - add extra kw(aes) encryption test vector

[Eric Biggers]

From: Eric Biggers 

One "kw(aes)" decryption test vector doesn't exactly match an encryption
test vector with input and result swapped.  In preparation for removing
the decryption test vectors, add this test vector to the encryption test
vectors, so we don't lose any test coverage.

Signed-off-by: Eric Biggers 
---
 crypto/testmgr.h | 13 +
 1 file changed, 13 insertions(+)

diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index 745a0ed0a73a..75ddbf790a99 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -33695,6 +33695,19 @@ static const struct cipher_testvec 
aes_kw_enc_tv_template[] = {
  "\xf5\x6f\xab\xea\x25\x48\xf5\xfb",
.rlen   = 16,
.iv_out = "\x03\x1f\x6b\xd7\xe6\x1e\x64\x3d",
+   }, {
+   .key= "\x80\xaa\x99\x73\x27\xa4\x80\x6b"
+ "\x6a\x7a\x41\xa5\x2b\x86\xc3\x71"
+ "\x03\x86\xf9\x32\x78\x6e\xf7\x96"
+ "\x76\xfa\xfb\x90\xb8\x26\x3c\x5f",
+   .klen   = 32,
+   .input  = "\x0a\x25\x6b\xa7\x5c\xfa\x03\xaa"
+ "\xa0\x2b\xa9\x42\x03\xf1\x5b\xaa",
+   .ilen   = 16,
+   .result = "\xd3\x3d\x3d\x97\x7b\xf0\xa9\x15"
+ "\x59\xf9\x9c\x8a\xcd\x29\x3d\x43",
+   .rlen   = 16,
+   .iv_out = "\x42\x3c\x96\x0d\x8a\x2a\xc4\xc1",
},
 };
 
-- 
2.17.0

[PATCH 3/5] crypto: testmgr - add extra ecb(tnepres) encryption test vectors

[Eric Biggers]

From: Eric Biggers 

None of the four "ecb(tnepres)" decryption test vectors exactly match an
encryption test vector with input and result swapped.  In preparation
for removing the decryption test vectors, add these to the encryption
test vectors, so we don't lose any test coverage.

Signed-off-by: Eric Biggers 
---
 crypto/testmgr.h | 40 +++-
 1 file changed, 39 insertions(+), 1 deletion(-)

diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index bdc67c058d5c..745a0ed0a73a 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -12047,6 +12047,14 @@ static const struct cipher_testvec 
serpent_enc_tv_template[] = {
 };
 
 static const struct cipher_testvec tnepres_enc_tv_template[] = {
+   { /* KeySize=0 */
+   .input  = "\x00\x01\x02\x03\x04\x05\x06\x07"
+ "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
+   .ilen   = 16,
+   .result = "\x41\xcc\x6b\x31\x59\x31\x45\x97"
+ "\x6d\x6f\xbb\x38\x4b\x37\x21\x28",
+   .rlen   = 16,
+   },
{ /* KeySize=128, PT=0, I=1 */
.input  = "\x00\x00\x00\x00\x00\x00\x00\x00"
  "\x00\x00\x00\x00\x00\x00\x00\x00",
@@ -12057,6 +12065,24 @@ static const struct cipher_testvec 
tnepres_enc_tv_template[] = {
.result = "\x49\xaf\xbf\xad\x9d\x5a\x34\x05"
  "\x2c\xd8\xff\xa5\x98\x6b\xd2\xdd",
.rlen   = 16,
+   }, { /* KeySize=128 */
+   .key= "\x00\x01\x02\x03\x04\x05\x06\x07"
+ "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
+   .klen   = 16,
+   .input  = "\x00\x01\x02\x03\x04\x05\x06\x07"
+ "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
+   .ilen   = 16,
+   .result = "\xea\xf4\xd7\xfc\xd8\x01\x34\x47"
+ "\x81\x45\x0b\xfa\x0c\xd6\xad\x6e",
+   .rlen   = 16,
+   }, { /* KeySize=128, I=121 */
+   .key= 
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x80",
+   .klen   = 16,
+   .input  = zeroed_string,
+   .ilen   = 16,
+   .result = "\x3d\xda\xbf\xc0\x06\xda\xab\x06"
+ "\x46\x2a\xf4\xef\x81\x54\x4e\x26",
+   .rlen   = 16,
}, { /* KeySize=192, PT=0, I=1 */
.key= "\x80\x00\x00\x00\x00\x00\x00\x00"
  "\x00\x00\x00\x00\x00\x00\x00\x00"
@@ -12092,7 +12118,19 @@ static const struct cipher_testvec 
tnepres_enc_tv_template[] = {
.result = "\x5c\xe7\x1c\x70\xd2\x88\x2e\x5b"
  "\xb8\x32\xe4\x33\xf8\x9f\x26\xde",
.rlen   = 16,
-   },
+   }, { /* KeySize=256 */
+   .key= "\x00\x01\x02\x03\x04\x05\x06\x07"
+ "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f"
+ "\x10\x11\x12\x13\x14\x15\x16\x17"
+ "\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f",
+   .klen   = 32,
+   .input  = "\x00\x01\x02\x03\x04\x05\x06\x07"
+ "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
+   .ilen   = 16,
+   .result = "\x64\xa9\x1a\x37\xed\x9f\xe7\x49"
+ "\xa8\x4e\x76\xd6\xf5\x0d\x78\xee",
+   .rlen   = 16,
+   }
 };
 
 
-- 
2.17.0

[PATCH 1/5] crypto: testmgr - add extra ecb(des) encryption test vectors

[Eric Biggers]

From: Eric Biggers 

Two "ecb(des)" decryption test vectors don't exactly match any of the
encryption test vectors with input and result swapped.  In preparation
for removing the decryption test vectors, add these to the encryption
test vectors, so we don't lose any test coverage.

Signed-off-by: Eric Biggers 
---
 crypto/testmgr.h | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index 552d8f00d85b..5ab36fb8dd31 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -5595,6 +5595,28 @@ static const struct cipher_testvec des_enc_tv_template[] 
= {
.rlen   = 16,
.np = 2,
.tap= { 8, 8 }
+   }, {
+   .key= "\x01\x23\x45\x67\x89\xab\xcd\xef",
+   .klen   = 8,
+   .input  = "\x01\x23\x45\x67\x89\xab\xcd\xe7"
+ "\xa3\x99\x7b\xca\xaf\x69\xa0\xf5",
+   .ilen   = 16,
+   .result = "\xc9\x57\x44\x25\x6a\x5e\xd3\x1d"
+ "\x69\x0f\x5b\x0d\x9a\x26\x93\x9b",
+   .rlen   = 16,
+   .np = 2,
+   .tap= { 8, 8 }
+   }, {
+   .key= "\x01\x23\x45\x67\x89\xab\xcd\xef",
+   .klen   = 8,
+   .input  = "\x01\x23\x45\x67\x89\xab\xcd\xe7"
+ "\xa3\x99\x7b\xca\xaf\x69\xa0\xf5",
+   .ilen   = 16,
+   .result = "\xc9\x57\x44\x25\x6a\x5e\xd3\x1d"
+ "\x69\x0f\x5b\x0d\x9a\x26\x93\x9b",
+   .rlen   = 16,
+   .np = 3,
+   .tap= { 3, 12, 1 }
}, { /* Four blocks -- for testing encryption with chunking */
.key= "\x01\x23\x45\x67\x89\xab\xcd\xef",
.klen   = 8,
-- 
2.17.0

[PATCH 2/5] crypto: testmgr - make an cbc(des) encryption test vector chunked

[Eric Biggers]

From: Eric Biggers 

One "cbc(des)" decryption test vector doesn't exactly match an
encryption test vector with input and result swapped.  It's *almost* the
same as one, but the decryption version is "chunked" while the
encryption version is "unchunked".  In preparation for removing the
decryption test vectors, make the encryption one both chunked and
unchunked, so we don't lose any test coverage.

Signed-off-by: Eric Biggers 
---
 crypto/testmgr.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index 5ab36fb8dd31..bdc67c058d5c 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -5885,6 +5885,9 @@ static const struct cipher_testvec 
des_cbc_enc_tv_template[] = {
.ilen   = 8,
.result = "\x68\x37\x88\x49\x9a\x7c\x05\xf6",
.rlen   = 8,
+   .np = 2,
+   .tap= { 4, 4 },
+   .also_non_np = 1,
}, { /* Copy of openssl vector for chunk testing */
 /* From OpenSSL */
.key= "\x01\x23\x45\x67\x89\xab\xcd\xef",
-- 
2.17.0

[PATCH 0/5] crypto: eliminate redundant decryption test vectors

[Eric Biggers]

Hello,

When adding the Speck cipher support I was annoyed by having to add both
encryption and decryption test vectors, since they are redundant: the
decryption ones are just the encryption ones with the input and result
flipped.

It turns out that's nearly always the case for all the other
ciphers/skciphers too.  A few have slight differences, but they seem to
be accidental except for "kw(aes)", and we can still handle "kw(aes)"
nearly as easily with just one copy of the test vectors.

Therefore, this series removes all the decryption cipher_testvecs and
updates testmgr to test both encryption and decryption using what used
to be the encryption test vectors.  I did not change any of the AEAD
test vectors, though a similar change could be made for them too.

Patches 1-4 add some encryption test vectors, just so no test coverage
is lost.  Patch 5 is the real patch.  Due to the 1+ lines deleted
from testmgr.h, the patch file is 615 KB so it may be too large for the
mailing list.  You can also grab the series from git:
https://github.com/ebiggers/linux, branch "test_vector_redundancy_v1"
(HEAD is a09e48518f957bb61bb278227917eaad64cf13be).  Most of the patch
is scripted, but there are also some manual changes, mostly to
testmgr.c.  For review purposes, in case the full 615 KB patch doesn't
reach the mailing list, I'm also pasting an abbreviated version of the
patch below that excludes the scripted changes to testmgr.h, i.e. it
only includes my manual changes on top of the scripted changes.

Eric Biggers (5):
  crypto: testmgr - add extra ecb(des) encryption test vectors
  crypto: testmgr - make an cbc(des) encryption test vector chunked
  crypto: testmgr - add extra ecb(tnepres) encryption test vectors
  crypto: testmgr - add extra kw(aes) encryption test vector
  crypto: testmgr - eliminate redundant decryption test vectors

 crypto/testmgr.c |   409 +-
 crypto/testmgr.h | 12227 -
 2 files changed, 954 insertions(+), 11682 deletions(-)

(Abbreviated patch for review purposes only begins here, in case full
 patch is too large for the list; also see git link above)

[PATCH 5/5] crypto: testmgr - eliminate redundant decryption test vectors

Currently testmgr has separate encryption and decryption test vectors
for symmetric ciphers.  That's massively redundant, since with few
exceptions (mostly mistakes, apparently), all decryption tests are
identical to the encryption tests, just with the input/result flipped.

Therefore, eliminate the redundancy by removing the decryption test
vectors and updating testmgr to test both encryption and decryption
using what used to be the encryption test vectors.  Naming is adjusted
accordingly: each cipher_testvec now has a 'ptext' (plaintext), 'ctext'
(ciphertext), and 'len' instead of an 'input', 'result', 'ilen', and
'rlen'.  Note that it was always the case that 'ilen == rlen'.

AES keywrap ("kw(aes)") is special because its IV is generated by the
encryption.  Previously this was handled by specifying 'iv_out' for
encryption and 'iv' for decryption.  To make it work cleanly with only
one set of test vectors, put the IV in 'iv', remove 'iv_out', and add a
boolean that indicates that the IV is generated by the encryption.

In total, this removes over 1 lines from testmgr.h, with no
reduction in test coverage since prior patches already copied the few
unique decryption test vectors into the encryption test vectors.

This covers all algorithms that used 'struct cipher_testvec', e.g. any
block cipher in the ECB, CBC, CTR, XTS, LRW, CTS-CBC, PCBC, OFB, or
keywrap modes, and Salsa20 and ChaCha20.  No change is made to AEAD
tests, though we probably can eliminate a similar redundancy there too.

The testmgr.h portion of this patch was automatically generated using
the following awk script, with some slight manual fixups on top (updated
'struct cipher_testvec' definition, updated a few comments, and fixed up
the AES keywrap test vectors):

BEGIN { OTHER = 0; ENCVEC = 1; DECVEC = 2; DECVEC_TAIL = 3; mode = OTHER }

/^static const struct cipher_testvec.*_enc_/ { sub("_enc", ""); mode = 
ENCVEC }
/^static const struct cipher_testvec.*_dec_/ { mode = DECVEC }
mode == ENCVEC && !/\.ilen[[:space:]]*=/ {
sub(/\.input[[:space:]]*=$/,".ptext =")
sub(/\.input[[:space:]]*=/, ".ptext\t=")
sub(/\.result[[:space:]]*=$/,   ".ctext =")
sub(/\.result[[:space:]]*=/,".ctext\t=")
sub(/\.rlen[[:space:]]*=/,  ".len\t=")
print
}
mode == DECVEC_TAIL && /[^[:space:]]/ { mode = OTHER }
mode == OTHER { print }
mode == ENCVEC && /^};/   { mode = OTHER }
mode == DECVEC && /^};/   { mode = DECVEC_TAIL }

Note that git's default diff algorithm gets confused by the testmgr.h
portion of this patch, and reports too many lines added and removed.
It's better viewed with 'git diff --minimal' (or 'git show --minimal'),

Re: [PATCH v2] fscrypt: log the crypto algorithm implementations

[Theodore Y. Ts'o]

On Fri, May 18, 2018 at 10:58:14AM -0700, Eric Biggers wrote:
> Log the crypto algorithm driver name for each fscrypt encryption mode on
> its first use, also showing a friendly name for the mode.
> 
> This will help people determine whether the expected implementations are
> being used.  In some cases we've seen people do benchmarks and reject
> using encryption for performance reasons, when in fact they used a much
> slower implementation of AES-XTS than was possible on the hardware.  It
> can make an enormous difference; e.g., AES-XTS on ARM is about 10x
> faster with the crypto extensions (AES instructions) than without.
> 
> This also makes it more obvious which modes are being used, now that
> fscrypt supports multiple combinations of modes.
> 
> Example messages (with default modes, on x86_64):
> 
> [   35.492057] fscrypt: AES-256-CTS-CBC using implementation 
> "cts(cbc-aes-aesni)"
> [   35.492171] fscrypt: AES-256-XTS using implementation "xts-aes-aesni"
> 
> Note: algorithms can be dynamically added to the crypto API, which can
> result in different implementations being used at different times.  But
> this is rare; for most users, showing the first will be good enough.
> 
> Signed-off-by: Eric Biggers 

Applied, thanks.

- Ted

Re: [PATCH v2] fscrypt: add Speck128/256 support

[Theodore Y. Ts'o]

On Mon, May 07, 2018 at 05:22:08PM -0700, Eric Biggers wrote:
> fscrypt currently only supports AES encryption.  However, many low-end
> mobile devices have older CPUs that don't have AES instructions, e.g.
> the ARMv8 Cryptography Extensions.  Currently, user data on such devices
> is not encrypted at rest because AES is too slow, even when the NEON
> bit-sliced implementation of AES is used.  Unfortunately, it is
> infeasible to encrypt these devices at all when AES is the only option.
> 
> Therefore, this patch updates fscrypt to support the Speck block cipher,
> which was recently added to the crypto API.  The C implementation of
> Speck is not especially fast, but Speck can be implemented very
> efficiently with general-purpose vector instructions, e.g. ARM NEON.
> For example, on an ARMv7 processor, we measured the NEON-accelerated
> Speck128/256-XTS at 69 MB/s for both encryption and decryption, while
> AES-256-XTS with the NEON bit-sliced implementation was only 22 MB/s
> encryption and 19 MB/s decryption.
> 
> There are multiple variants of Speck.  This patch only adds support for
> Speck128/256, which is the variant with a 128-bit block size and 256-bit
> key size -- the same as AES-256.  This is believed to be the most secure
> variant of Speck, and it's only about 6% slower than Speck128/128.
> Speck64/128 would be at least 20% faster because it has 20% rounds, and
> it can be even faster on CPUs that can't efficiently do the 64-bit
> operations needed for Speck128.  However, Speck64's 64-bit block size is
> not preferred security-wise.  ARM NEON also supports the needed 64-bit
> operations even on 32-bit CPUs, resulting in Speck128 being fast enough
> for our targeted use cases so far.
> 
> The chosen modes of operation are XTS for contents and CTS-CBC for
> filenames.  These are the same modes of operation that fscrypt defaults
> to for AES.  Note that as with the other fscrypt modes, Speck will not
> be used unless userspace chooses to use it.  Nor are any of the existing
> modes (which are all AES-based) being removed, of course.
> 
> We intentionally don't make CONFIG_FS_ENCRYPTION select
> CONFIG_CRYPTO_SPECK, so people will have to enable Speck support
> themselves if they need it.  This is because we shouldn't bloat the
> FS_ENCRYPTION dependencies with every new cipher, especially ones that
> aren't recommended for most users.  Moreover, CRYPTO_SPECK is just the
> generic implementation, which won't be fast enough for many users; in
> practice, they'll need to enable CRYPTO_SPECK_NEON to get acceptable
> performance.
> 
> More details about our choice of Speck can be found in our patches that
> added Speck to the crypto API, and the follow-on discussion threads.
> We're planning a publication that explains the choice in more detail.
> But briefly, we can't use ChaCha20 as we previously proposed, since it
> would be insecure to use a stream cipher in this context, with potential
> IV reuse during writes on f2fs and/or on wear-leveling flash storage.
> 
> We also evaluated many other lightweight and/or ARX-based block ciphers
> such as Chaskey-LTS, RC5, LEA, CHAM, Threefish, RC6, NOEKEON, SPARX, and
> XTEA.  However, all had disadvantages vs. Speck, such as insufficient
> performance with NEON, much less published cryptanalysis, or an
> insufficient security level.  Various design choices in Speck make it
> perform better with NEON than competing ciphers while still having a
> security margin similar to AES, and in the case of Speck128 also the
> same available security levels.  Unfortunately, Speck does have some
> political baggage attached -- it's an NSA designed cipher, and was
> rejected from an ISO standard (though for context, as far as I know none
> of the above-mentioned alternatives are ISO standards either).
> Nevertheless, we believe it is a good solution to the problem from a
> technical perspective.
> 
> Certain algorithms constructed from ChaCha or the ChaCha permutation,
> such as MEM (Masked Even-Mansour) or HPolyC, may also meet our
> performance requirements.  However, these are new constructions that
> need more time to receive the cryptographic review and acceptance needed
> to be confident in their security.  HPolyC hasn't been published yet,
> and we are concerned that MEM makes stronger assumptions about the
> underlying permutation than the ChaCha stream cipher does.  In contrast,
> the XTS mode of operation is relatively well accepted, and Speck has
> over 70 cryptanalysis papers.  Of course, these ChaCha-based algorithms
> can still be added later if they become ready.
> 
> The best known attack on Speck128/256 is a differential cryptanalysis
> attack on 25 of 34 rounds with 2^253 time complexity and 2^125 chosen
> plaintexts, i.e. only marginally faster than brute force.  There is no
> known attack on the full 34 rounds.
> 
> Signed-off-by: Eric Biggers 

Applied, thanks.

- Ted

Re: cryptomgr_test / drbg_ctr: BUG: sleeping function called from invalid context

[Stephan Müller]

Am Freitag, 18. Mai 2018, 10:36:04 CEST schrieb Geert Uytterhoeven:

Hi Geert,
> 
> I tried following the code path, but couldn't find where it went wrong.
> 
> mutex_lock(>drbg_mutex) is called from drbg_instantiate(), which is
> inlined by the compiler into drbg_kcapi_seed().
> 
> Do you have a clue?

It is the first time I hear from such an issue. Yes, the DRBG should not be 
called in atomic context. But I do not see where we have an atomic context 
(either a spin_lock or in an interrupt handler) when we are executing the test 
manager.

I will keep looking.

Ciao
Stephan

4.16: /dev/random - a new approach

[Stephan Müller]

Hi,

The patch set available at [1] provides a different approach to /dev/random 
which I call Linux Random Number Generator (LRNG) to collect entropy within 
the Linux kernel. The main improvements compared to the legacy /dev/random is 
to provide sufficient entropy during boot time as well as in virtual 
environments and when using SSDs. A secondary design goal is to limit the 
impact of the entropy collection on massive parallel systems and also allow 
the use accelerated cryptographic primitives. Also, all steps of the entropic 
data processing are testable.

The design and implementation is driven by a set of goals described in [1]
that the LRNG completely implements. Furthermore, [1] includes a
comparison with RNG design suggestions such as SP800-90B, SP800-90C, and
AIS20/31.

The LRNG provides a complete separation of the noise source maintenance
and the collection of entropy into an entropy pool from the post-processing
using a pseudo-random number generator. Different PRNGs are supported,
including:

* Built-in ChaCha20 PRNG which has no dependency to other kernel
  frameworks.

* SP800-90A DRBG using the kernel crypto API including its accelerated
  raw cipher implementations.

* Arbitrary PRNGs registered with the kernel crypto API

Booting the patch with the kernel command line option
"dyndbg=file drivers/char/lrng* +p" generates logs indicating the operation
of the LRNG. Each log is pre-pended with "lrng:".

The LRNG has a flexible design by allowing an easy replacement of the
deterministic random number generator component.

[1] http://www.chronox.de/lrng.html

Changes (compared to the previous patch set for 4.15):

 * Addition of SPOX copyright identifier
 * Use the updated poll infrastructure
 * Add the kernel crypto API PRNG support
-- 
2.14.3

[PATCH] crypto: x86/aegis256 - Fix wrong key buffer size

[Ondrej Mosnáček]

From: Ondrej Mosnacek 

AEGIS-256 key is two blocks, not one.

Fixes: 1d373d4e8e15 ("crypto: x86 - Add optimized AEGIS implementations")
Reported-by: Eric Biggers 
Signed-off-by: Ondrej Mosnacek 
---
 arch/x86/crypto/aegis256-aesni-glue.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/crypto/aegis256-aesni-glue.c 
b/arch/x86/crypto/aegis256-aesni-glue.c
index 3181655dd862..2b5dd3af8f4d 100644
--- a/arch/x86/crypto/aegis256-aesni-glue.c
+++ b/arch/x86/crypto/aegis256-aesni-glue.c
@@ -57,7 +57,7 @@ struct aegis_state {
 };
 
 struct aegis_ctx {
-   struct aegis_block key;
+   struct aegis_block key[AEGIS256_KEY_SIZE / AEGIS256_BLOCK_SIZE];
 };
 
 struct aegis_crypt_ops {
@@ -164,7 +164,7 @@ static int crypto_aegis256_aesni_setkey(struct crypto_aead 
*aead, const u8 *key,
return -EINVAL;
}
 
-   memcpy(ctx->key.bytes, key, AEGIS256_KEY_SIZE);
+   memcpy(ctx->key, key, AEGIS256_KEY_SIZE);
 
return 0;
 }
@@ -190,7 +190,7 @@ static void crypto_aegis256_aesni_crypt(struct aead_request 
*req,
 
kernel_fpu_begin();
 
-   crypto_aegis256_aesni_init(, ctx->key.bytes, req->iv);
+   crypto_aegis256_aesni_init(, ctx->key, req->iv);
crypto_aegis256_aesni_process_ad(, req->src, req->assoclen);
crypto_aegis256_aesni_process_crypt(, req, ops);
crypto_aegis256_aesni_final(, tag_xor, req->assoclen, cryptlen);
-- 
2.17.0

Re: [PATCH 3/3] crypto: x86 - Add optimized AEGIS implementations

[Ondrej Mosnáček]

2018-05-20 4:41 GMT+02:00 Eric Biggers :
> Hi Ondrej,
>
> On Fri, May 11, 2018 at 02:12:51PM +0200, Ondrej Mosnáček wrote:
>> From: Ondrej Mosnacek 
>>
>> This patch adds optimized implementations of AEGIS-128, AEGIS-128L,
>> and AEGIS-256, utilizing the AES-NI and SSE2 x86 extensions.
>>
>> Signed-off-by: Ondrej Mosnacek 
> [...]
>> +static int crypto_aegis256_aesni_setkey(struct crypto_aead *aead, const u8 
>> *key,
>> + unsigned int keylen)
>> +{
>> + struct aegis_ctx *ctx = crypto_aegis256_aesni_ctx(aead);
>> +
>> + if (keylen != AEGIS256_KEY_SIZE) {
>> + crypto_aead_set_flags(aead, CRYPTO_TFM_RES_BAD_KEY_LEN);
>> + return -EINVAL;
>> + }
>> +
>> + memcpy(ctx->key.bytes, key, AEGIS256_KEY_SIZE);
>> +
>> + return 0;
>> +}
>
> This code is copying 32 bytes into a 16-byte buffer.

Indeed, I must have overlooked that while copy-pasting and editing the
boilerplate...

I will send a follow-up patch soon.

Thanks for the report!

>
> ==
> BUG: KASAN: slab-out-of-bounds in memcpy include/linux/string.h:345 [inline]
> BUG: KASAN: slab-out-of-bounds in crypto_aegis256_aesni_setkey+0x23/0x60 
> arch/x86/crypto/aegis256-aesni-glue.c:167
> Write of size 32 at addr 88006c16b650 by task cryptomgr_test/120
> CPU: 2 PID: 120 Comm: cryptomgr_test Not tainted 
> 4.17.0-rc1-00069-g6ecc9d9ff91f #31
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
> 1.11.0-20171110_100015-anatol 04/01/2014
> Call Trace:
>  __dump_stack lib/dump_stack.c:77 [inline]
>  dump_stack+0x86/0xca lib/dump_stack.c:113
>  print_address_description+0x65/0x204 mm/kasan/report.c:256
>  kasan_report_error mm/kasan/report.c:354 [inline]
>  kasan_report.cold.6+0x242/0x304 mm/kasan/report.c:412
>  check_memory_region_inline mm/kasan/kasan.c:260 [inline]
>  check_memory_region+0x13c/0x1b0 mm/kasan/kasan.c:267
>  memcpy+0x37/0x50 mm/kasan/kasan.c:303
>  memcpy include/linux/string.h:345 [inline]
>  crypto_aegis256_aesni_setkey+0x23/0x60 
> arch/x86/crypto/aegis256-aesni-glue.c:167
>  crypto_aead_setkey+0xa4/0x1e0 crypto/aead.c:62
>  cryptd_aead_setkey+0x30/0x50 crypto/cryptd.c:938
>  crypto_aead_setkey+0xa4/0x1e0 crypto/aead.c:62
>  cryptd_aegis256_aesni_setkey+0x30/0x50 
> arch/x86/crypto/aegis256-aesni-glue.c:259
>  crypto_aead_setkey+0xa4/0x1e0 crypto/aead.c:62
>  __test_aead+0x8bf/0x3770 crypto/testmgr.c:675
>  test_aead+0x28/0x110 crypto/testmgr.c:957
>  alg_test_aead+0x8b/0x140 crypto/testmgr.c:1690
>  alg_test.part.5+0x1bb/0x4d0 crypto/testmgr.c:3845
>  alg_test+0x23/0x25 crypto/testmgr.c:3865
>  cryptomgr_test+0x56/0x80 crypto/algboss.c:223
>  kthread+0x329/0x3f0 kernel/kthread.c:238
>  ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:412
> Allocated by task 120:
>  save_stack mm/kasan/kasan.c:448 [inline]
>  set_track mm/kasan/kasan.c:460 [inline]
>  kasan_kmalloc.part.1+0x5f/0xf0 mm/kasan/kasan.c:553
>  kasan_kmalloc+0xaf/0xc0 mm/kasan/kasan.c:538
>  __do_kmalloc mm/slab.c:3718 [inline]
>  __kmalloc+0x114/0x1d0 mm/slab.c:3727
>  kmalloc include/linux/slab.h:517 [inline]
>  kzalloc include/linux/slab.h:701 [inline]
>  crypto_create_tfm+0x80/0x2c0 crypto/api.c:464
>  crypto_spawn_tfm2+0x57/0x90 crypto/algapi.c:717
>  crypto_spawn_aead include/crypto/internal/aead.h:112 [inline]
>  cryptd_aead_init_tfm+0x3d/0x110 crypto/cryptd.c:1033
>  crypto_aead_init_tfm+0x130/0x190 crypto/aead.c:111
>  crypto_create_tfm+0xda/0x2c0 crypto/api.c:471
>  crypto_alloc_tfm+0xcf/0x1d0 crypto/api.c:543
>  crypto_alloc_aead+0x14/0x20 crypto/aead.c:351
>  cryptd_alloc_aead+0xeb/0x1c0 crypto/cryptd.c:1334
>  cryptd_aegis256_aesni_init_tfm+0x24/0xf0 
> arch/x86/crypto/aegis256-aesni-glue.c:308
>  crypto_aead_init_tfm+0x130/0x190 crypto/aead.c:111
>  crypto_create_tfm+0xda/0x2c0 crypto/api.c:471
>  crypto_alloc_tfm+0xcf/0x1d0 crypto/api.c:543
>  crypto_alloc_aead+0x14/0x20 crypto/aead.c:351
>  alg_test_aead+0x1f/0x140 crypto/testmgr.c:1682
>  alg_test.part.5+0x1bb/0x4d0 crypto/testmgr.c:3845
>  alg_test+0x23/0x25 crypto/testmgr.c:3865
>  cryptomgr_test+0x56/0x80 crypto/algboss.c:223
>  kthread+0x329/0x3f0 kernel/kthread.c:238
>  ret_from_[   16.453502] serial8250: too much work for irq4
> Freed by task 0:
> (stack is not available)
> The buggy address belongs to the object at 88006c16b600
> The buggy address is located 80 bytes inside of
> The buggy address belongs to the page:
> page:ea00017a4f68 count:1 mapcount:0 mapping:88006c16b000 index:0x0
> flags: 0x1000100(slab)
> raw: 01000100 88006c16b000  00010015
> raw: ea00017a2470 88006d401548 88006d400400
> page dumped because: kasan: bad access detected
> Memory state around the buggy address:
>  88006c16b500: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>  88006c16b580: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc