Re: [PATCH] fscrypt: add support for ChaCha20 contents encryption

2017-12-08 Thread Jeffrey Walton
> Still, a stream cipher is sufficient to protect data confidentiality in
> the event of a single point-in-time permanent offline compromise of the
> disk, which currently is the primary threat model for fscrypt.  Thus,
> when the alternative is quite literally *no encryption*, we might as
> well use a stream cipher.

The "single point in time" requirement is kind of interesting. I
believe you are saying the scheme lacks semantic security.

Forgive my ignorance... Does that mean this cipher should not be used
when backups are in effect; or sync'ing to  happens?

Jeff

On Thu, Dec 7, 2017 at 8:38 PM, Eric Biggers  wrote:
> From: Eric Biggers 
>
> fscrypt currently only supports AES encryption.  However, many low-end
> mobile devices still use older CPUs such as ARMv7, which do not support
> the AES instructions (the ARMv8 Cryptography Extensions).  This results
> in very poor AES performance, even if the NEON bit-sliced implementation
> is used.  Roughly 20-40 MB/s is a typical number, in comparison to
> 300-800 MB/s on CPUs that support the AES instructions.  Switching from
> AES-256 to AES-128 only helps by about 30%.
>
> The result is that vendors don't enable encryption on these devices,
> leaving users unprotected.
>
> A performance difference of similar magnitude can also be observed on
> x86, between CPUs with and without the AES-NI instruction set.
>
> This patch provides an alternative to AES by updating fscrypt to support
> the ChaCha20 stream cipher (RFC7539) for contents encryption.  ChaCha20
> was designed to have a large security margin, to be efficient on
> general-purpose CPUs without dedicated instructions, and to be
> vectorizable.  It is already supported by the Linux crypto API,
> including a vectorized implementation for ARM using NEON instructions,
> and vectorized implementations for x86 using SSSE3 or AVX2 instructions.
>
> On 32-bit ARM processors with NEON support, ChaCha20 is about 3.2 times
> faster than AES-128-XTS (chacha20-neon vs. xts-aes-neonbs).  Without
> NEON support, ChaCha20 is about 1.5 times as fast (chacha20-generic vs.
> xts(aes-asm)).  The improvement over AES-256-XTS is even greater.
>
> Note that stream ciphers are not an ideal choice for disk encryption,
> since each data block has to be encrypted with the same IV each time it
> is overwritten.  Consequently, an adversary who observes the ciphertext
> both before and after a write can trivially recover the keystream if
> they can guess one of the plaintexts.  Moreover, an adversary who can
> write to the ciphertext can flip arbitrary bits in the plaintext, merely
> by flipping the corresponding bits in the ciphertext.  A block cipher
> operating in the XTS or CBC-ESSIV mode provides some protection against
> these types of attacks -- albeit not full protection, which would at
> minimum require the use an authenticated encryption mode with nonces.
>
> Unfortunately, we are unaware of any block cipher which performs as well
> as ChaCha20, has a similar or greater security margin, and has been
> subject to as much public security analysis.  We do not consider Speck
> to be a viable alternative at this time.
>
> Still, a stream cipher is sufficient to protect data confidentiality in
> the event of a single point-in-time permanent offline compromise of the
> disk, which currently is the primary threat model for fscrypt.  Thus,
> when the alternative is quite literally *no encryption*, we might as
> well use a stream cipher.
>
> We offer ChaCha20 rather than the reduced-round variants ChaCha8 or
> ChaCha12 because ChaCha20 has a much higher security margin, and we are
> primarily targeting CPUs where ChaCha20 is fast enough, in particular
> CPUs that have vector instructions such as NEON or SSSE3.  Also, the
> crypto API currently only supports ChaCha20.  Still, if ChaCha8 and/or
> ChaCha12 support were to be added to the crypto API, it would be
> straightforward to support them in fscrypt too.
>
> Currently, stream ciphers cannot be used for filenames encryption with
> fscrypt because all filenames in a directory have to be encrypted with
> the same IV.  Therefore, we offer ChaCha20 for contents encryption only.
> Filenames encryption still must use AES-256-CTS-CBC.  This is acceptable
> because filenames encryption is not as performance-critical as contents
> encryption.
>
> ...


Re: [RFC PATCH] crypto: chacha20 - add implementation using 96-bit nonce

2017-12-08 Thread Ard Biesheuvel
On 8 December 2017 at 23:11, Eric Biggers  wrote:
> On Fri, Dec 08, 2017 at 10:54:24PM +, Ard Biesheuvel wrote:
>> >> Note that there are two conflicting conventions for what inputs ChaCha 
>> >> takes.
>> >> The original paper by Daniel Bernstein
>> >> (https://cr.yp.to/chacha/chacha-20080128.pdf) says that the block counter 
>> >> is
>> >> 64-bit and the nonce is 64-bit, thereby expanding the key into 2^64 
>> >> randomly
>> >> accessible streams, each containing 2^64 randomly accessible 64-byte 
>> >> blocks.
>> >>
>> >> The RFC 7539 convention is equivalent to seeking to a large offset 
>> >> (determined
>> >> by the first 32 bits of the 96-bit nonce) in the keystream defined by the 
>> >> djb
>> >> convention, but only if the 32-bit portion of the block counter never 
>> >> overflows.
>> >>
>> >> Maybe it is only RFC 7539 that matters because that is what is being
>> >> standardized by the IETF; I don't know.  But it confused me.
>> >>
>> >
>> > The distinction only matters if you start the counter at zero (or
>> > one), because you 'lose' 32 bits of IV that will never be != 0 in
>> > practice if you use a 64-bit counter.
>> >
>> > So that argues for not exposing the block counter as part of the API,
>> > given that it should start at zero anyway, and that you should take
>> > care not to put colliding values in it.
>> >
>> >> Anyway, I actually thought it was intentional that the ChaCha 
>> >> implementations in
>> >> the Linux kernel allowed specifying the block counter, and therefore 
>> >> allowed
>> >> seeking to any point in the keystream, exposing the full functionality of 
>> >> the
>> >> cipher.  It's true that it's easily misused though, so there may 
>> >> nevertheless be
>> >> value in providing a nonce-only variant.
>> >>
>> >
>> > Currently, the skcipher API does not allow such random access, so
>> > while I can see how that could be a useful feature, we can't really
>> > make use of it today. But more importantly, it still does not mean the
>> > block counter should be exposed to the /users/ of the skcipher API
>> > which typically encrypt/decrypt blocks that are much larger than 64
>> > bytes.
>>
>> ... but now that I think of it, how is this any different from, say,
>> AES in CTR mode? The counter is big endian, but apart from that, using
>> IVs derived from a counter will result in the exact same issue, only
>> with a shift of 16 bytes.
>>
>> That means using file block numbers as IV is simply inappropriate, and
>> you should encrypt them first like is done for AES-CBC
>
> The problem with using a stream cipher --- whether that is ChaCha20, AES-CTR, 
> or
> something else --- for disk/file encryption is that by necessity of file/disk
> encryption, each time the "same" block is written to, the IV is the same, 
> which
> is really bad for stream ciphers (but not as bad for AES-XTS, AES-CBC, etc.).
> It's irrelevant whether you do ESSIV or otherwise encrypt the IVs.  ESSIV does
> make the IV for each offset unpredictable by an attacker, which is desirable 
> for
> AES-CBC, but it doesn't stop the IV from being repeated for each overwrite.
>

I'm not suggesting using an encrypted IV to fix the stream cipher
issue, I'm well aware that that is impossible. What I am saying is
that the counter collision can be mitigated by encrypting the IV.

> And just to clarify, you definitely *can* seek to any position in the ChaCha20
> stream using the existing ChaCha20 implementations and the existing skcipher
> API, simply by providing the appropriate IV.  Maybe it was unintentional, but 
> it
> does work.  chacha20poly1305.c even uses it to start at block 1 instead of 
> block
> 0.  I don't know whether there are other users, though.
>

Well, I understand that that's how ChaCha20 works, and that you can
manipulate the IV directly to start at another point in the keystream.
AES-CTR can do exactly the same, for the same reason. What I am saying
is that the skcipher API does not allow you to decrypt an arbitrary
part of a block, which could benefit from the ability of not having to
generate the entire key stream.

So the more we discuss this, the more I think there is actually no
difference with AES-CTR (apart from the block size), and there a
similar enhancement in RFC3686 where the IV does not cover the AES
block level counter, making it safe to use another counter to generate
the IVs.

Of course, this is essentially what you did for the fscrypt code, I
just don't like seeing that kind of reasoning being implement in the
crypto API client.


Re: [RFC PATCH] crypto: chacha20 - add implementation using 96-bit nonce

2017-12-08 Thread Eric Biggers
On Fri, Dec 08, 2017 at 10:54:24PM +, Ard Biesheuvel wrote:
> >> Note that there are two conflicting conventions for what inputs ChaCha 
> >> takes.
> >> The original paper by Daniel Bernstein
> >> (https://cr.yp.to/chacha/chacha-20080128.pdf) says that the block counter 
> >> is
> >> 64-bit and the nonce is 64-bit, thereby expanding the key into 2^64 
> >> randomly
> >> accessible streams, each containing 2^64 randomly accessible 64-byte 
> >> blocks.
> >>
> >> The RFC 7539 convention is equivalent to seeking to a large offset 
> >> (determined
> >> by the first 32 bits of the 96-bit nonce) in the keystream defined by the 
> >> djb
> >> convention, but only if the 32-bit portion of the block counter never 
> >> overflows.
> >>
> >> Maybe it is only RFC 7539 that matters because that is what is being
> >> standardized by the IETF; I don't know.  But it confused me.
> >>
> >
> > The distinction only matters if you start the counter at zero (or
> > one), because you 'lose' 32 bits of IV that will never be != 0 in
> > practice if you use a 64-bit counter.
> >
> > So that argues for not exposing the block counter as part of the API,
> > given that it should start at zero anyway, and that you should take
> > care not to put colliding values in it.
> >
> >> Anyway, I actually thought it was intentional that the ChaCha 
> >> implementations in
> >> the Linux kernel allowed specifying the block counter, and therefore 
> >> allowed
> >> seeking to any point in the keystream, exposing the full functionality of 
> >> the
> >> cipher.  It's true that it's easily misused though, so there may 
> >> nevertheless be
> >> value in providing a nonce-only variant.
> >>
> >
> > Currently, the skcipher API does not allow such random access, so
> > while I can see how that could be a useful feature, we can't really
> > make use of it today. But more importantly, it still does not mean the
> > block counter should be exposed to the /users/ of the skcipher API
> > which typically encrypt/decrypt blocks that are much larger than 64
> > bytes.
> 
> ... but now that I think of it, how is this any different from, say,
> AES in CTR mode? The counter is big endian, but apart from that, using
> IVs derived from a counter will result in the exact same issue, only
> with a shift of 16 bytes.
> 
> That means using file block numbers as IV is simply inappropriate, and
> you should encrypt them first like is done for AES-CBC

The problem with using a stream cipher --- whether that is ChaCha20, AES-CTR, or
something else --- for disk/file encryption is that by necessity of file/disk
encryption, each time the "same" block is written to, the IV is the same, which
is really bad for stream ciphers (but not as bad for AES-XTS, AES-CBC, etc.).
It's irrelevant whether you do ESSIV or otherwise encrypt the IVs.  ESSIV does
make the IV for each offset unpredictable by an attacker, which is desirable for
AES-CBC, but it doesn't stop the IV from being repeated for each overwrite.

And just to clarify, you definitely *can* seek to any position in the ChaCha20
stream using the existing ChaCha20 implementations and the existing skcipher
API, simply by providing the appropriate IV.  Maybe it was unintentional, but it
does work.  chacha20poly1305.c even uses it to start at block 1 instead of block
0.  I don't know whether there are other users, though.

Eric


Re: [RFC PATCH] crypto: chacha20 - add implementation using 96-bit nonce

2017-12-08 Thread Ard Biesheuvel
On 8 December 2017 at 22:42, Ard Biesheuvel  wrote:
> On 8 December 2017 at 22:17, Eric Biggers  wrote:
>> On Fri, Dec 08, 2017 at 11:55:02AM +, Ard Biesheuvel wrote:
>>> As pointed out by Eric [0], the way RFC7539 was interpreted when creating
>>> our implementation of ChaCha20 creates a risk of IV reuse when using a
>>> little endian counter as the IV generator. The reason is that the low end
>>> bits of the counter get mapped onto the ChaCha20 block counter, which
>>> advances every 64 bytes. This means that the counter value that gets
>>> selected as IV for the next input block will collide with the ChaCha20
>>> block counter of the previous block, basically recreating the same
>>> keystream but shifted by 64 bytes.
>>>
>>> RFC7539 describes the inputs of the algorithm as follows:
>>>
>>>   The inputs to ChaCha20 are:
>>>
>>>  o  A 256-bit key
>>>
>>>  o  A 32-bit initial counter.  This can be set to any number, but will
>>> usually be zero or one.  It makes sense to use one if we use the
>>> zero block for something else, such as generating a one-time
>>> authenticator key as part of an AEAD algorithm.
>>>
>>>  o  A 96-bit nonce.  In some protocols, this is known as the
>>> Initialization Vector.
>>>
>>>  o  An arbitrary-length plaintext
>>>
>>> The solution is to use a fixed value of 0 for the initial counter, and
>>> only expose a 96-bit IV to the upper layers of the crypto API.
>>>
>>> So introduce a new ChaCha20 flavor called chacha20-iv96, which takes the
>>> above into account, and should become the preferred ChaCha20
>>> implementation going forward for general use.
>>
>> Note that there are two conflicting conventions for what inputs ChaCha takes.
>> The original paper by Daniel Bernstein
>> (https://cr.yp.to/chacha/chacha-20080128.pdf) says that the block counter is
>> 64-bit and the nonce is 64-bit, thereby expanding the key into 2^64 randomly
>> accessible streams, each containing 2^64 randomly accessible 64-byte blocks.
>>
>> The RFC 7539 convention is equivalent to seeking to a large offset 
>> (determined
>> by the first 32 bits of the 96-bit nonce) in the keystream defined by the djb
>> convention, but only if the 32-bit portion of the block counter never 
>> overflows.
>>
>> Maybe it is only RFC 7539 that matters because that is what is being
>> standardized by the IETF; I don't know.  But it confused me.
>>
>
> The distinction only matters if you start the counter at zero (or
> one), because you 'lose' 32 bits of IV that will never be != 0 in
> practice if you use a 64-bit counter.
>
> So that argues for not exposing the block counter as part of the API,
> given that it should start at zero anyway, and that you should take
> care not to put colliding values in it.
>
>> Anyway, I actually thought it was intentional that the ChaCha 
>> implementations in
>> the Linux kernel allowed specifying the block counter, and therefore allowed
>> seeking to any point in the keystream, exposing the full functionality of the
>> cipher.  It's true that it's easily misused though, so there may 
>> nevertheless be
>> value in providing a nonce-only variant.
>>
>
> Currently, the skcipher API does not allow such random access, so
> while I can see how that could be a useful feature, we can't really
> make use of it today. But more importantly, it still does not mean the
> block counter should be exposed to the /users/ of the skcipher API
> which typically encrypt/decrypt blocks that are much larger than 64
> bytes.

... but now that I think of it, how is this any different from, say,
AES in CTR mode? The counter is big endian, but apart from that, using
IVs derived from a counter will result in the exact same issue, only
with a shift of 16 bytes.

That means using file block numbers as IV is simply inappropriate, and
you should encrypt them first like is done for AES-CBC


Re: [RFC PATCH] crypto: chacha20 - add implementation using 96-bit nonce

2017-12-08 Thread Ard Biesheuvel
On 8 December 2017 at 22:17, Eric Biggers  wrote:
> On Fri, Dec 08, 2017 at 11:55:02AM +, Ard Biesheuvel wrote:
>> As pointed out by Eric [0], the way RFC7539 was interpreted when creating
>> our implementation of ChaCha20 creates a risk of IV reuse when using a
>> little endian counter as the IV generator. The reason is that the low end
>> bits of the counter get mapped onto the ChaCha20 block counter, which
>> advances every 64 bytes. This means that the counter value that gets
>> selected as IV for the next input block will collide with the ChaCha20
>> block counter of the previous block, basically recreating the same
>> keystream but shifted by 64 bytes.
>>
>> RFC7539 describes the inputs of the algorithm as follows:
>>
>>   The inputs to ChaCha20 are:
>>
>>  o  A 256-bit key
>>
>>  o  A 32-bit initial counter.  This can be set to any number, but will
>> usually be zero or one.  It makes sense to use one if we use the
>> zero block for something else, such as generating a one-time
>> authenticator key as part of an AEAD algorithm.
>>
>>  o  A 96-bit nonce.  In some protocols, this is known as the
>> Initialization Vector.
>>
>>  o  An arbitrary-length plaintext
>>
>> The solution is to use a fixed value of 0 for the initial counter, and
>> only expose a 96-bit IV to the upper layers of the crypto API.
>>
>> So introduce a new ChaCha20 flavor called chacha20-iv96, which takes the
>> above into account, and should become the preferred ChaCha20
>> implementation going forward for general use.
>
> Note that there are two conflicting conventions for what inputs ChaCha takes.
> The original paper by Daniel Bernstein
> (https://cr.yp.to/chacha/chacha-20080128.pdf) says that the block counter is
> 64-bit and the nonce is 64-bit, thereby expanding the key into 2^64 randomly
> accessible streams, each containing 2^64 randomly accessible 64-byte blocks.
>
> The RFC 7539 convention is equivalent to seeking to a large offset (determined
> by the first 32 bits of the 96-bit nonce) in the keystream defined by the djb
> convention, but only if the 32-bit portion of the block counter never 
> overflows.
>
> Maybe it is only RFC 7539 that matters because that is what is being
> standardized by the IETF; I don't know.  But it confused me.
>

The distinction only matters if you start the counter at zero (or
one), because you 'lose' 32 bits of IV that will never be != 0 in
practice if you use a 64-bit counter.

So that argues for not exposing the block counter as part of the API,
given that it should start at zero anyway, and that you should take
care not to put colliding values in it.

> Anyway, I actually thought it was intentional that the ChaCha implementations 
> in
> the Linux kernel allowed specifying the block counter, and therefore allowed
> seeking to any point in the keystream, exposing the full functionality of the
> cipher.  It's true that it's easily misused though, so there may nevertheless 
> be
> value in providing a nonce-only variant.
>

Currently, the skcipher API does not allow such random access, so
while I can see how that could be a useful feature, we can't really
make use of it today. But more importantly, it still does not mean the
block counter should be exposed to the /users/ of the skcipher API
which typically encrypt/decrypt blocks that are much larger than 64
bytes.


Re: [RFC PATCH] crypto: chacha20 - add implementation using 96-bit nonce

2017-12-08 Thread Eric Biggers
On Fri, Dec 08, 2017 at 11:55:02AM +, Ard Biesheuvel wrote:
> As pointed out by Eric [0], the way RFC7539 was interpreted when creating
> our implementation of ChaCha20 creates a risk of IV reuse when using a
> little endian counter as the IV generator. The reason is that the low end
> bits of the counter get mapped onto the ChaCha20 block counter, which
> advances every 64 bytes. This means that the counter value that gets
> selected as IV for the next input block will collide with the ChaCha20
> block counter of the previous block, basically recreating the same
> keystream but shifted by 64 bytes.
> 
> RFC7539 describes the inputs of the algorithm as follows:
> 
>   The inputs to ChaCha20 are:
> 
>  o  A 256-bit key
> 
>  o  A 32-bit initial counter.  This can be set to any number, but will
> usually be zero or one.  It makes sense to use one if we use the
> zero block for something else, such as generating a one-time
> authenticator key as part of an AEAD algorithm.
> 
>  o  A 96-bit nonce.  In some protocols, this is known as the
> Initialization Vector.
> 
>  o  An arbitrary-length plaintext
> 
> The solution is to use a fixed value of 0 for the initial counter, and
> only expose a 96-bit IV to the upper layers of the crypto API.
> 
> So introduce a new ChaCha20 flavor called chacha20-iv96, which takes the
> above into account, and should become the preferred ChaCha20
> implementation going forward for general use.

Note that there are two conflicting conventions for what inputs ChaCha takes.
The original paper by Daniel Bernstein
(https://cr.yp.to/chacha/chacha-20080128.pdf) says that the block counter is
64-bit and the nonce is 64-bit, thereby expanding the key into 2^64 randomly
accessible streams, each containing 2^64 randomly accessible 64-byte blocks.

The RFC 7539 convention is equivalent to seeking to a large offset (determined
by the first 32 bits of the 96-bit nonce) in the keystream defined by the djb
convention, but only if the 32-bit portion of the block counter never overflows.

Maybe it is only RFC 7539 that matters because that is what is being
standardized by the IETF; I don't know.  But it confused me.

Anyway, I actually thought it was intentional that the ChaCha implementations in
the Linux kernel allowed specifying the block counter, and therefore allowed
seeking to any point in the keystream, exposing the full functionality of the
cipher.  It's true that it's easily misused though, so there may nevertheless be
value in providing a nonce-only variant.

Eric


Re: [PATCH] fscrypt: add support for ChaCha20 contents encryption

2017-12-08 Thread Eric Biggers
On Fri, Dec 08, 2017 at 07:20:43AM +, Ard Biesheuvel wrote:
> On 8 December 2017 at 02:51, Jason A. Donenfeld  wrote:
> > Hi Eric,
> >
> > Nice to see more use of ChaCha20. However...
> >
> > Can we skip over the "sort of worse than XTS, but not having _real_
> > authentication sucks anyway in either case, so whatever" and move
> > directly to, "linux finally supports authenticated encryption for disk
> > encryption!"?
> 
> Ehm, it doesn't? This is plain ChaCha20, not any AEAD variant.
> 
> > This would be a big deal and would actually be a
> > noticeable security improvement, instead of a potentially dubious step
> > sidewaysbackish.
> >
> 
> It is actually dubious, given the large scale reuse of IVs with a
> stream cipher. I do suppose though that using an AEAD variant would at
> least catch any attacks involving flipping ciphertext bits resulting
> in plaintext bits being flipped at the same offset (but file updates
> would still be visible in the clear)
> 

It *is* dubious, but it would be a replacement for No Encryption, not a
replacement for AES.  AES would continue to be required on devices that can do
AES fast enough.  This would only be for devices that do not meet the AES
performance bar, so their status quo is No Encryption.  I know, it is fun to
poke holes in bad crypto, but much less interesting to poke holes in "no crypto"
:-)

We can't use authenticated encryption for the same reason we can't use random or
sequential nonces: there is nowhere to store the additional metadata
(authentication tag and nonce) per filesystem block *and* have it updated
atomically with respect to the contents of said block.  Copy-on-write
filesystems such as btrfs or bcachefs can do it.  Traditional filesystems
cannot.  F2FS comes closer than EXT4 since F2FS is based on a log-structured
filesystem, but only partially; for one, it still updates data in-place
sometimes.  This is not a new problem; this is also the reason why we haven't
been able to add AES-GCM support yet.

Authentication aside, the greater problem here is the IV reuse.  Unfortunately
it actually will likely be even worse than we thought originally, because this
would be used on flash storage that does wear leveling, and likely also with
F2FS which often doesn't overwrite data in-place.  So even under the "single
point-in-time permanent offline compromise" threat model it may not be good
enough.  We are going to spend some more time investigating alternatives, but
unfortunately there are not many.

Eric


Re: [PATCH] KEYS: reject NULL restriction string when type is specified

2017-12-08 Thread Mat Martineau

On Fri, 8 Dec 2017, David Howells wrote:


Mat Martineau  wrote:


Since this fixes the bug for the asymmetric key type and ensures that other
key types won't make the same mistake, I agree this is the way to fix it. I
did not find any issues in the patch.


Can I put that down as a Reviewed-by?


Yes. Looks like I missed the window for your pull request, though - I'll 
be sure to add Reviewed-by in future reviews.


--
Mat Martineau
Intel OTC


Re: [PATCH 00/18] crypto: talitos - fixes and performance improvement

2017-12-08 Thread Horia Geantă
On 10/12/2017 6:20 PM, Herbert Xu wrote:
> On Fri, Oct 06, 2017 at 03:04:31PM +0200, Christophe Leroy wrote:
>> This serie fixes and improves the talitos crypto driver.
>>
>> First 6 patchs are fixes of failures reported by the new tests in the
>> kernel crypto test manager.
>>
Looks like these fixes are required also on older 4.9+ -stable kernels.
(I haven't seen them on latest 4.9.68-stable mail from Greg, even though
they are in main tree.)

In case you agree, what would be the recommended way to add the patches
to -stable?

Thanks,
Horia

>> The 8 following patches are cleanups and simplifications.
>>
>> The last 4 ones are performance improvement. The main improvement is
>> in the one before the last, it divides by 2 the time needed for a md5
>> hash on the SEC1.
>>
>> Christophe Leroy (18):
>>   crypto: talitos - fix AEAD test failures
>>   crypto: talitos - fix memory corruption on SEC2
>>   crypto: talitos - fix setkey to check key weakness
>>   crypto: talitos - fix AEAD for sha224 on non sha224 capable chips
>>   crypto: talitos - fix use of sg_link_tbl_len
>>   crypto: talitos - fix ctr-aes-talitos
>>   crypto: talitos - zeroize the descriptor with memset()
>>   crypto: talitos - declare local functions static
>>   crypto: talitos - use devm_kmalloc()
>>   crypto: talitos - use of_property_read_u32()
>>   crypto: talitos - use devm_ioremap()
>>   crypto: talitos - don't check the number of channels at each interrupt
>>   crypto: talitos - remove to_talitos_ptr_len()
>>   crypto: talitos - simplify tests in ipsec_esp()
>>   crypto: talitos - DMA map key in setkey()
>>   crypto: talitos - do hw_context DMA mapping outside the requests
>>   crypto: talitos - chain in buffered data for ahash on SEC1
>>   crypto: talitos - avoid useless copy
> 
> All applied.  Thanks.
> 


Re: [PATCH] KEYS: reject NULL restriction string when type is specified

2017-12-08 Thread David Howells
Mat Martineau  wrote:

> Since this fixes the bug for the asymmetric key type and ensures that other
> key types won't make the same mistake, I agree this is the way to fix it. I
> did not find any issues in the patch.

Can I put that down as a Reviewed-by?

David


AF_ALG: skb limits

2017-12-08 Thread Stephan Mueller
Am Freitag, 8. Dezember 2017, 12:39:06 CET schrieb Jonathan Cameron:

Hi Jonathan,

> 
> As a heads up, the other nasties we've found so far are around hitting
> limits on the various socket buffers.  When you run into those you can end
> up with parts of the data to be encrypted going through without it being
> obvious.
> 

The entire code uses sock_alloc to prevent user space to chew up kernel 
memory. I am aware that if you have a too low skb buffer limit, parts of the 
cipher operation will not succeed as a malloc will fail.

But that is returned with an error to user space. If you observe such an 
error, the entire message you wanted to read with recvmsg must be considered 
tainted (i.e. you do not know which part of the message has been encrypted or 
not). Thus, the recvmsg must be invoked again on the same buffer sent to the 
kernel if you want to ensure you encrypted the data.

Could you please help me understand where that functionality fails?

PS: If you want to give more memory to your sockets, you have to tweak /proc/
sys/net/core/optmem_max.

Ciao
Stephan


[RFC PATCH] crypto: chacha20 - add implementation using 96-bit nonce

2017-12-08 Thread Ard Biesheuvel
As pointed out by Eric [0], the way RFC7539 was interpreted when creating
our implementation of ChaCha20 creates a risk of IV reuse when using a
little endian counter as the IV generator. The reason is that the low end
bits of the counter get mapped onto the ChaCha20 block counter, which
advances every 64 bytes. This means that the counter value that gets
selected as IV for the next input block will collide with the ChaCha20
block counter of the previous block, basically recreating the same
keystream but shifted by 64 bytes.

RFC7539 describes the inputs of the algorithm as follows:

  The inputs to ChaCha20 are:

 o  A 256-bit key

 o  A 32-bit initial counter.  This can be set to any number, but will
usually be zero or one.  It makes sense to use one if we use the
zero block for something else, such as generating a one-time
authenticator key as part of an AEAD algorithm.

 o  A 96-bit nonce.  In some protocols, this is known as the
Initialization Vector.

 o  An arbitrary-length plaintext

The solution is to use a fixed value of 0 for the initial counter, and
only expose a 96-bit IV to the upper layers of the crypto API.

So introduce a new ChaCha20 flavor called chacha20-iv96, which takes the
above into account, and should become the preferred ChaCha20
implementation going forward for general use.

[0] https://marc.info/?l=linux-crypto-vger&m=151269722430848&w=2

Cc: Eric Biggers 
Cc: linux-fscr...@vger.kernel.org
Cc: Theodore Ts'o 
Cc: linux-e...@vger.kernel.org
Cc: linux-f2fs-de...@lists.sourceforge.net
Cc: linux-...@lists.infradead.org
Cc: linux-fsde...@vger.kernel.org
Cc: linux-crypto@vger.kernel.org
Cc: Jaegeuk Kim 
Cc: Michael Halcrow 
Cc: Paul Crowley 
Cc: Martin Willi 
Cc: David Gstir 
Cc: "Jason A . Donenfeld" 
Cc: Stephan Mueller 
Signed-off-by: Ard Biesheuvel 
---
 arch/arm64/crypto/chacha20-neon-glue.c | 27 +++---
 arch/x86/crypto/chacha20_glue.c| 19 +-
 crypto/chacha20_generic.c  | 38 ++--
 crypto/testmgr.c   |  9 +
 crypto/testmgr.h   |  2 +-
 include/crypto/chacha20.h  |  4 ++-
 6 files changed, 81 insertions(+), 18 deletions(-)

diff --git a/arch/arm64/crypto/chacha20-neon-glue.c 
b/arch/arm64/crypto/chacha20-neon-glue.c
index cbdb75d15cd0..76a570058cc8 100644
--- a/arch/arm64/crypto/chacha20-neon-glue.c
+++ b/arch/arm64/crypto/chacha20-neon-glue.c
@@ -70,7 +70,7 @@ static int chacha20_neon(struct skcipher_request *req)
 
err = skcipher_walk_virt(&walk, req, true);
 
-   crypto_chacha20_init(state, ctx, walk.iv);
+   crypto_chacha20_init(state, ctx, walk.iv, crypto_skcipher_ivsize(tfm));
 
kernel_neon_begin();
while (walk.nbytes > 0) {
@@ -88,7 +88,7 @@ static int chacha20_neon(struct skcipher_request *req)
return err;
 }
 
-static struct skcipher_alg alg = {
+static struct skcipher_alg alg[] = {{
.base.cra_name  = "chacha20",
.base.cra_driver_name   = "chacha20-neon",
.base.cra_priority  = 300,
@@ -104,19 +104,35 @@ static struct skcipher_alg alg = {
.setkey = crypto_chacha20_setkey,
.encrypt= chacha20_neon,
.decrypt= chacha20_neon,
-};
+}, {
+   .base.cra_name  = "chacha20-iv96",
+   .base.cra_driver_name   = "chacha20-neon",
+   .base.cra_priority  = 300,
+   .base.cra_blocksize = 1,
+   .base.cra_ctxsize   = sizeof(struct chacha20_ctx),
+   .base.cra_module= THIS_MODULE,
+
+   .min_keysize= CHACHA20_KEY_SIZE,
+   .max_keysize= CHACHA20_KEY_SIZE,
+   .ivsize = CHACHA20_NONCE_SIZE,
+   .chunksize  = CHACHA20_BLOCK_SIZE,
+   .walksize   = 4 * CHACHA20_BLOCK_SIZE,
+   .setkey = crypto_chacha20_setkey,
+   .encrypt= chacha20_neon,
+   .decrypt= chacha20_neon,
+}};
 
 static int __init chacha20_simd_mod_init(void)
 {
if (!(elf_hwcap & HWCAP_ASIMD))
return -ENODEV;
 
-   return crypto_register_skcipher(&alg);
+   return crypto_register_skciphers(alg, ARRAY_SIZE(alg));
 }
 
 static void __exit chacha20_simd_mod_fini(void)
 {
-   crypto_unregister_skcipher(&alg);
+   crypto_unregister_skciphers(alg, ARRAY_SIZE(alg));
 }
 
 module_init(chacha20_simd_mod_init);
@@ -125,3 +141,4 @@ module_exit(chacha20_simd_mod_fini);
 MODULE_AUTHOR("Ard Biesheuvel ");
 MODULE_LICENSE("GPL v2");
 MODULE_ALIAS_CRYPTO("chacha20");
+MODULE_ALIAS_CRYPTO("chacha20-iv96");
diff --git a/arch/x86/crypto/chacha20_glue.c b/arch/x86/crypto/chacha20_glue.c
index dce7c5d39c2f..44c7fe376a1d 100644
--- a/arch/x86/crypto/chacha20_glue.c
+++ b/arch/x86/crypto/chacha20_glue.c
@@ -79,7 +79,7 @@ static int chacha20_simd(struct skcipher_request *req)
 
err = skcipher_walk_virt(&walk, req, true);

Re: [PATCH] crypto: AF_ALG - fix race accessing cipher request

2017-12-08 Thread Jonathan Cameron
On Fri, 8 Dec 2017 11:50:37 +0100
Stephan Müller  wrote:

> Hi Herbert,
> 
> This patch would go on top of 7d2c3f54e6f646887d019faa45f35d6fe9fe82ce
> "crypto: af_alg - remove locking in async callback" found in Linus' tree
> which is not yet in the cryptodev-2.6 tree.
> 
> In addition, this patch is already on top of the other patches discussed
> on this list fixing similar issues. I.e. depending in which order you apply
> the patches, there may be a hunk. In case you want me to rebase the patch,
> please let me know.
> 
> ---8<---
> When invoking an asynchronous cipher operation, the invocation of the
> callback may be performed before the subsequent operations in the
> initial code path are invoked. The callback deletes the cipher request
> data structure which implies that after the invocation of the
> asynchronous cipher operation, this data structure must not be accessed
> any more.
> 
> The setting of the return code size with the request data structure must
> therefore be moved before the invocation of the asynchronous cipher
> operation.
> 
> Fixes: e870456d8e7c ("crypto: algif_skcipher - overhaul memory management")
> Fixes: d887c52d6ae4 ("crypto: algif_aead - overhaul memory management")
> Reported-by: syzbot 
> Cc:  # v4.14+
> Signed-off-by: Stephan Mueller 
Acked-by: Jonathan Cameron 

Have an identical patch in my queue having observed this happening on our
hardware - though only had the skcipher case.

As a heads up, the other nasties we've found so far are around hitting limits
on the various socket buffers.  When you run into those you can end up with
parts of the data to be encrypted going through without it being obvious.

Will update on that properly once I've chased down a local problem that
is limiting the length of my test runs to a few million messages.

Jonathan

> ---
>  crypto/algif_aead.c | 10 +-
>  crypto/algif_skcipher.c | 10 +-
>  2 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
> index 15e561dc47b5..d7ec2f4ebaf9 100644
> --- a/crypto/algif_aead.c
> +++ b/crypto/algif_aead.c
> @@ -291,6 +291,10 @@ static int _aead_recvmsg(struct socket *sock, struct 
> msghdr *msg,
>   /* AIO operation */
>   sock_hold(sk);
>   areq->iocb = msg->msg_iocb;
> +
> + /* Remember output size that will be generated. */
> + areq->outlen = outlen;
> +
>   aead_request_set_callback(&areq->cra_u.aead_req,
> CRYPTO_TFM_REQ_MAY_BACKLOG,
> af_alg_async_cb, areq);
> @@ -298,12 +302,8 @@ static int _aead_recvmsg(struct socket *sock, struct 
> msghdr *msg,
>crypto_aead_decrypt(&areq->cra_u.aead_req);
>  
>   /* AIO operation in progress */
> - if (err == -EINPROGRESS || err == -EBUSY) {
> - /* Remember output size that will be generated. */
> - areq->outlen = outlen;
> -
> + if (err == -EINPROGRESS || err == -EBUSY)
>   return -EIOCBQUEUED;
> - }
>  
>   sock_put(sk);
>   } else {
> diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
> index 6fb595cd63ac..baef9bfccdda 100644
> --- a/crypto/algif_skcipher.c
> +++ b/crypto/algif_skcipher.c
> @@ -125,6 +125,10 @@ static int _skcipher_recvmsg(struct socket *sock, struct 
> msghdr *msg,
>   /* AIO operation */
>   sock_hold(sk);
>   areq->iocb = msg->msg_iocb;
> +
> + /* Remember output size that will be generated. */
> + areq->outlen = len;
> +
>   skcipher_request_set_callback(&areq->cra_u.skcipher_req,
> CRYPTO_TFM_REQ_MAY_SLEEP,
> af_alg_async_cb, areq);
> @@ -133,12 +137,8 @@ static int _skcipher_recvmsg(struct socket *sock, struct 
> msghdr *msg,
>   crypto_skcipher_decrypt(&areq->cra_u.skcipher_req);
>  
>   /* AIO operation in progress */
> - if (err == -EINPROGRESS || err == -EBUSY) {
> - /* Remember output size that will be generated. */
> - areq->outlen = len;
> -
> + if (err == -EINPROGRESS || err == -EBUSY)
>   return -EIOCBQUEUED;
> - }
>  
>   sock_put(sk);
>   } else {



[PATCH] crypto: AF_ALG - fix race accessing cipher request

2017-12-08 Thread Stephan Müller
Hi Herbert,

This patch would go on top of 7d2c3f54e6f646887d019faa45f35d6fe9fe82ce
"crypto: af_alg - remove locking in async callback" found in Linus' tree
which is not yet in the cryptodev-2.6 tree.

In addition, this patch is already on top of the other patches discussed
on this list fixing similar issues. I.e. depending in which order you apply
the patches, there may be a hunk. In case you want me to rebase the patch,
please let me know.

---8<---
When invoking an asynchronous cipher operation, the invocation of the
callback may be performed before the subsequent operations in the
initial code path are invoked. The callback deletes the cipher request
data structure which implies that after the invocation of the
asynchronous cipher operation, this data structure must not be accessed
any more.

The setting of the return code size with the request data structure must
therefore be moved before the invocation of the asynchronous cipher
operation.

Fixes: e870456d8e7c ("crypto: algif_skcipher - overhaul memory management")
Fixes: d887c52d6ae4 ("crypto: algif_aead - overhaul memory management")
Reported-by: syzbot 
Cc:  # v4.14+
Signed-off-by: Stephan Mueller 
---
 crypto/algif_aead.c | 10 +-
 crypto/algif_skcipher.c | 10 +-
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
index 15e561dc47b5..d7ec2f4ebaf9 100644
--- a/crypto/algif_aead.c
+++ b/crypto/algif_aead.c
@@ -291,6 +291,10 @@ static int _aead_recvmsg(struct socket *sock, struct 
msghdr *msg,
/* AIO operation */
sock_hold(sk);
areq->iocb = msg->msg_iocb;
+
+   /* Remember output size that will be generated. */
+   areq->outlen = outlen;
+
aead_request_set_callback(&areq->cra_u.aead_req,
  CRYPTO_TFM_REQ_MAY_BACKLOG,
  af_alg_async_cb, areq);
@@ -298,12 +302,8 @@ static int _aead_recvmsg(struct socket *sock, struct 
msghdr *msg,
 crypto_aead_decrypt(&areq->cra_u.aead_req);
 
/* AIO operation in progress */
-   if (err == -EINPROGRESS || err == -EBUSY) {
-   /* Remember output size that will be generated. */
-   areq->outlen = outlen;
-
+   if (err == -EINPROGRESS || err == -EBUSY)
return -EIOCBQUEUED;
-   }
 
sock_put(sk);
} else {
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 6fb595cd63ac..baef9bfccdda 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -125,6 +125,10 @@ static int _skcipher_recvmsg(struct socket *sock, struct 
msghdr *msg,
/* AIO operation */
sock_hold(sk);
areq->iocb = msg->msg_iocb;
+
+   /* Remember output size that will be generated. */
+   areq->outlen = len;
+
skcipher_request_set_callback(&areq->cra_u.skcipher_req,
  CRYPTO_TFM_REQ_MAY_SLEEP,
  af_alg_async_cb, areq);
@@ -133,12 +137,8 @@ static int _skcipher_recvmsg(struct socket *sock, struct 
msghdr *msg,
crypto_skcipher_decrypt(&areq->cra_u.skcipher_req);
 
/* AIO operation in progress */
-   if (err == -EINPROGRESS || err == -EBUSY) {
-   /* Remember output size that will be generated. */
-   areq->outlen = len;
-
+   if (err == -EINPROGRESS || err == -EBUSY)
return -EIOCBQUEUED;
-   }
 
sock_put(sk);
} else {
-- 
2.14.3




Re: [PATCH] fscrypt: add support for ChaCha20 contents encryption

2017-12-08 Thread Ard Biesheuvel
On 8 December 2017 at 10:14, Stephan Mueller  wrote:
> Am Freitag, 8. Dezember 2017, 11:06:31 CET schrieb Ard Biesheuvel:
>
> Hi Ard,
>
>>
>> Given how it is not uncommon for counters to be used as IV, this is a
>> fundamental flaw that could rear its head in other places as well, so
>> I propose we fix this one way (fix the current code) or the other
>> (deprecate the current code and create a new chacha20-rfc7539
>> blockcipher that uses a 96-bit IV and sets the counter to 0)
>
> Instead of having a complete new implementation of the ChaCha20 cipher, what
> about using a specific IV generator for which the kernel crypto API has
> already support (see crypto/seqiv.c for example)?
>
> I.e. we have the current ChaCha20 cipher, but use some "rfc7539iv(chacha20)"
> cipher mode where that rfc7539iv is the mentioned IV generator that turns the
> given IV (sector number?) into the proper IV for ChaCha20.
>

To be honest, I don't fully understand how the IV generators work.
seqiv is implemented as an AEAD not as a skcipher, and we'd need to
wrap chacha20 in something that is usable as a skcipher.

In any case, it does make sense to address this by wrapping chacha20
in something generic, rather than having to extend all
implementations.


Re: [PATCH] fscrypt: add support for ChaCha20 contents encryption

2017-12-08 Thread Stephan Mueller
Am Freitag, 8. Dezember 2017, 11:06:31 CET schrieb Ard Biesheuvel:

Hi Ard,

> 
> Given how it is not uncommon for counters to be used as IV, this is a
> fundamental flaw that could rear its head in other places as well, so
> I propose we fix this one way (fix the current code) or the other
> (deprecate the current code and create a new chacha20-rfc7539
> blockcipher that uses a 96-bit IV and sets the counter to 0)

Instead of having a complete new implementation of the ChaCha20 cipher, what 
about using a specific IV generator for which the kernel crypto API has 
already support (see crypto/seqiv.c for example)?

I.e. we have the current ChaCha20 cipher, but use some "rfc7539iv(chacha20)" 
cipher mode where that rfc7539iv is the mentioned IV generator that turns the 
given IV (sector number?) into the proper IV for ChaCha20.

Ciao
Stephan


Re: [PATCH] fscrypt: add support for ChaCha20 contents encryption

2017-12-08 Thread Ard Biesheuvel
On 8 December 2017 at 09:11, Ard Biesheuvel  wrote:
> On 8 December 2017 at 09:11, Ard Biesheuvel  wrote:
>> Hi Eric,
>>
>> On 8 December 2017 at 01:38, Eric Biggers  wrote:
>>> From: Eric Biggers 
>>>
>>> fscrypt currently only supports AES encryption.  However, many low-end
>>> mobile devices still use older CPUs such as ARMv7, which do not support
>>> the AES instructions (the ARMv8 Cryptography Extensions).  This results
>>> in very poor AES performance, even if the NEON bit-sliced implementation
>>> is used.  Roughly 20-40 MB/s is a typical number, in comparison to
>>> 300-800 MB/s on CPUs that support the AES instructions.  Switching from
>>> AES-256 to AES-128 only helps by about 30%.
>>>
>>> The result is that vendors don't enable encryption on these devices,
>>> leaving users unprotected.
>>>
>>> A performance difference of similar magnitude can also be observed on
>>> x86, between CPUs with and without the AES-NI instruction set.
>>>
>>> This patch provides an alternative to AES by updating fscrypt to support
>>> the ChaCha20 stream cipher (RFC7539) for contents encryption.  ChaCha20
>>> was designed to have a large security margin, to be efficient on
>>> general-purpose CPUs without dedicated instructions, and to be
>>> vectorizable.  It is already supported by the Linux crypto API,
>>> including a vectorized implementation for ARM using NEON instructions,
>>> and vectorized implementations for x86 using SSSE3 or AVX2 instructions.
>>>
>>> On 32-bit ARM processors with NEON support, ChaCha20 is about 3.2 times
>>> faster than AES-128-XTS (chacha20-neon vs. xts-aes-neonbs).  Without
>>> NEON support, ChaCha20 is about 1.5 times as fast (chacha20-generic vs.
>>> xts(aes-asm)).  The improvement over AES-256-XTS is even greater.
>>>
>>> Note that stream ciphers are not an ideal choice for disk encryption,
>>> since each data block has to be encrypted with the same IV each time it
>>> is overwritten.  Consequently, an adversary who observes the ciphertext
>>> both before and after a write can trivially recover the keystream if
>>> they can guess one of the plaintexts.  Moreover, an adversary who can
>>> write to the ciphertext can flip arbitrary bits in the plaintext, merely
>>> by flipping the corresponding bits in the ciphertext.  A block cipher
>>> operating in the XTS or CBC-ESSIV mode provides some protection against
>>> these types of attacks -- albeit not full protection, which would at
>>> minimum require the use an authenticated encryption mode with nonces.
>>>
>>> Unfortunately, we are unaware of any block cipher which performs as well
>>> as ChaCha20, has a similar or greater security margin, and has been
>>> subject to as much public security analysis.  We do not consider Speck
>>> to be a viable alternative at this time.
>>>
>>> Still, a stream cipher is sufficient to protect data confidentiality in
>>> the event of a single point-in-time permanent offline compromise of the
>>> disk, which currently is the primary threat model for fscrypt.  Thus,
>>> when the alternative is quite literally *no encryption*, we might as
>>> well use a stream cipher.
>>>
>>> We offer ChaCha20 rather than the reduced-round variants ChaCha8 or
>>> ChaCha12 because ChaCha20 has a much higher security margin, and we are
>>> primarily targeting CPUs where ChaCha20 is fast enough, in particular
>>> CPUs that have vector instructions such as NEON or SSSE3.  Also, the
>>> crypto API currently only supports ChaCha20.  Still, if ChaCha8 and/or
>>> ChaCha12 support were to be added to the crypto API, it would be
>>> straightforward to support them in fscrypt too.
>>>
>>> Currently, stream ciphers cannot be used for filenames encryption with
>>> fscrypt because all filenames in a directory have to be encrypted with
>>> the same IV.  Therefore, we offer ChaCha20 for contents encryption only.
>>> Filenames encryption still must use AES-256-CTS-CBC.  This is acceptable
>>> because filenames encryption is not as performance-critical as contents
>>> encryption.
>>>
>>> Reviewed-by: Michael Halcrow 
>>> Signed-off-by: Eric Biggers 
>>> ---
>>>  Documentation/filesystems/fscrypt.rst | 43 +++---
>>>  fs/crypto/Kconfig |  1 +
>>>  fs/crypto/crypto.c| 69 
>>> ---
>>>  fs/crypto/keyinfo.c   |  2 +
>>>  include/linux/fscrypt.h   |  6 ++-
>>>  include/uapi/linux/fs.h   |  1 +
>>>  6 files changed, 102 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/Documentation/filesystems/fscrypt.rst 
>>> b/Documentation/filesystems/fscrypt.rst
>>> index 776ddc655f79..927d3c88816b 100644
>>> --- a/Documentation/filesystems/fscrypt.rst
>>> +++ b/Documentation/filesystems/fscrypt.rst
>>> @@ -184,6 +184,9 @@ replaced with HKDF or another more standard KDF in the 
>>> future.
>>>  Encryption modes and usage
>>>  ==
>>>
>>> +Available modes
>>> +---
>>> +
>>>  fscrypt allows one encrypt

Re: [PATCH] fscrypt: add support for ChaCha20 contents encryption

2017-12-08 Thread Ard Biesheuvel
On 8 December 2017 at 09:11, Ard Biesheuvel  wrote:
> Hi Eric,
>
> On 8 December 2017 at 01:38, Eric Biggers  wrote:
>> From: Eric Biggers 
>>
>> fscrypt currently only supports AES encryption.  However, many low-end
>> mobile devices still use older CPUs such as ARMv7, which do not support
>> the AES instructions (the ARMv8 Cryptography Extensions).  This results
>> in very poor AES performance, even if the NEON bit-sliced implementation
>> is used.  Roughly 20-40 MB/s is a typical number, in comparison to
>> 300-800 MB/s on CPUs that support the AES instructions.  Switching from
>> AES-256 to AES-128 only helps by about 30%.
>>
>> The result is that vendors don't enable encryption on these devices,
>> leaving users unprotected.
>>
>> A performance difference of similar magnitude can also be observed on
>> x86, between CPUs with and without the AES-NI instruction set.
>>
>> This patch provides an alternative to AES by updating fscrypt to support
>> the ChaCha20 stream cipher (RFC7539) for contents encryption.  ChaCha20
>> was designed to have a large security margin, to be efficient on
>> general-purpose CPUs without dedicated instructions, and to be
>> vectorizable.  It is already supported by the Linux crypto API,
>> including a vectorized implementation for ARM using NEON instructions,
>> and vectorized implementations for x86 using SSSE3 or AVX2 instructions.
>>
>> On 32-bit ARM processors with NEON support, ChaCha20 is about 3.2 times
>> faster than AES-128-XTS (chacha20-neon vs. xts-aes-neonbs).  Without
>> NEON support, ChaCha20 is about 1.5 times as fast (chacha20-generic vs.
>> xts(aes-asm)).  The improvement over AES-256-XTS is even greater.
>>
>> Note that stream ciphers are not an ideal choice for disk encryption,
>> since each data block has to be encrypted with the same IV each time it
>> is overwritten.  Consequently, an adversary who observes the ciphertext
>> both before and after a write can trivially recover the keystream if
>> they can guess one of the plaintexts.  Moreover, an adversary who can
>> write to the ciphertext can flip arbitrary bits in the plaintext, merely
>> by flipping the corresponding bits in the ciphertext.  A block cipher
>> operating in the XTS or CBC-ESSIV mode provides some protection against
>> these types of attacks -- albeit not full protection, which would at
>> minimum require the use an authenticated encryption mode with nonces.
>>
>> Unfortunately, we are unaware of any block cipher which performs as well
>> as ChaCha20, has a similar or greater security margin, and has been
>> subject to as much public security analysis.  We do not consider Speck
>> to be a viable alternative at this time.
>>
>> Still, a stream cipher is sufficient to protect data confidentiality in
>> the event of a single point-in-time permanent offline compromise of the
>> disk, which currently is the primary threat model for fscrypt.  Thus,
>> when the alternative is quite literally *no encryption*, we might as
>> well use a stream cipher.
>>
>> We offer ChaCha20 rather than the reduced-round variants ChaCha8 or
>> ChaCha12 because ChaCha20 has a much higher security margin, and we are
>> primarily targeting CPUs where ChaCha20 is fast enough, in particular
>> CPUs that have vector instructions such as NEON or SSSE3.  Also, the
>> crypto API currently only supports ChaCha20.  Still, if ChaCha8 and/or
>> ChaCha12 support were to be added to the crypto API, it would be
>> straightforward to support them in fscrypt too.
>>
>> Currently, stream ciphers cannot be used for filenames encryption with
>> fscrypt because all filenames in a directory have to be encrypted with
>> the same IV.  Therefore, we offer ChaCha20 for contents encryption only.
>> Filenames encryption still must use AES-256-CTS-CBC.  This is acceptable
>> because filenames encryption is not as performance-critical as contents
>> encryption.
>>
>> Reviewed-by: Michael Halcrow 
>> Signed-off-by: Eric Biggers 
>> ---
>>  Documentation/filesystems/fscrypt.rst | 43 +++---
>>  fs/crypto/Kconfig |  1 +
>>  fs/crypto/crypto.c| 69 
>> ---
>>  fs/crypto/keyinfo.c   |  2 +
>>  include/linux/fscrypt.h   |  6 ++-
>>  include/uapi/linux/fs.h   |  1 +
>>  6 files changed, 102 insertions(+), 20 deletions(-)
>>
>> diff --git a/Documentation/filesystems/fscrypt.rst 
>> b/Documentation/filesystems/fscrypt.rst
>> index 776ddc655f79..927d3c88816b 100644
>> --- a/Documentation/filesystems/fscrypt.rst
>> +++ b/Documentation/filesystems/fscrypt.rst
>> @@ -184,6 +184,9 @@ replaced with HKDF or another more standard KDF in the 
>> future.
>>  Encryption modes and usage
>>  ==
>>
>> +Available modes
>> +---
>> +
>>  fscrypt allows one encryption mode to be specified for file contents
>>  and one encryption mode to be specified for filenames.  Different
>>  directory trees are permitted to

Re: [PATCH] fscrypt: add support for ChaCha20 contents encryption

2017-12-08 Thread Ard Biesheuvel
Hi Eric,

On 8 December 2017 at 01:38, Eric Biggers  wrote:
> From: Eric Biggers 
>
> fscrypt currently only supports AES encryption.  However, many low-end
> mobile devices still use older CPUs such as ARMv7, which do not support
> the AES instructions (the ARMv8 Cryptography Extensions).  This results
> in very poor AES performance, even if the NEON bit-sliced implementation
> is used.  Roughly 20-40 MB/s is a typical number, in comparison to
> 300-800 MB/s on CPUs that support the AES instructions.  Switching from
> AES-256 to AES-128 only helps by about 30%.
>
> The result is that vendors don't enable encryption on these devices,
> leaving users unprotected.
>
> A performance difference of similar magnitude can also be observed on
> x86, between CPUs with and without the AES-NI instruction set.
>
> This patch provides an alternative to AES by updating fscrypt to support
> the ChaCha20 stream cipher (RFC7539) for contents encryption.  ChaCha20
> was designed to have a large security margin, to be efficient on
> general-purpose CPUs without dedicated instructions, and to be
> vectorizable.  It is already supported by the Linux crypto API,
> including a vectorized implementation for ARM using NEON instructions,
> and vectorized implementations for x86 using SSSE3 or AVX2 instructions.
>
> On 32-bit ARM processors with NEON support, ChaCha20 is about 3.2 times
> faster than AES-128-XTS (chacha20-neon vs. xts-aes-neonbs).  Without
> NEON support, ChaCha20 is about 1.5 times as fast (chacha20-generic vs.
> xts(aes-asm)).  The improvement over AES-256-XTS is even greater.
>
> Note that stream ciphers are not an ideal choice for disk encryption,
> since each data block has to be encrypted with the same IV each time it
> is overwritten.  Consequently, an adversary who observes the ciphertext
> both before and after a write can trivially recover the keystream if
> they can guess one of the plaintexts.  Moreover, an adversary who can
> write to the ciphertext can flip arbitrary bits in the plaintext, merely
> by flipping the corresponding bits in the ciphertext.  A block cipher
> operating in the XTS or CBC-ESSIV mode provides some protection against
> these types of attacks -- albeit not full protection, which would at
> minimum require the use an authenticated encryption mode with nonces.
>
> Unfortunately, we are unaware of any block cipher which performs as well
> as ChaCha20, has a similar or greater security margin, and has been
> subject to as much public security analysis.  We do not consider Speck
> to be a viable alternative at this time.
>
> Still, a stream cipher is sufficient to protect data confidentiality in
> the event of a single point-in-time permanent offline compromise of the
> disk, which currently is the primary threat model for fscrypt.  Thus,
> when the alternative is quite literally *no encryption*, we might as
> well use a stream cipher.
>
> We offer ChaCha20 rather than the reduced-round variants ChaCha8 or
> ChaCha12 because ChaCha20 has a much higher security margin, and we are
> primarily targeting CPUs where ChaCha20 is fast enough, in particular
> CPUs that have vector instructions such as NEON or SSSE3.  Also, the
> crypto API currently only supports ChaCha20.  Still, if ChaCha8 and/or
> ChaCha12 support were to be added to the crypto API, it would be
> straightforward to support them in fscrypt too.
>
> Currently, stream ciphers cannot be used for filenames encryption with
> fscrypt because all filenames in a directory have to be encrypted with
> the same IV.  Therefore, we offer ChaCha20 for contents encryption only.
> Filenames encryption still must use AES-256-CTS-CBC.  This is acceptable
> because filenames encryption is not as performance-critical as contents
> encryption.
>
> Reviewed-by: Michael Halcrow 
> Signed-off-by: Eric Biggers 
> ---
>  Documentation/filesystems/fscrypt.rst | 43 +++---
>  fs/crypto/Kconfig |  1 +
>  fs/crypto/crypto.c| 69 
> ---
>  fs/crypto/keyinfo.c   |  2 +
>  include/linux/fscrypt.h   |  6 ++-
>  include/uapi/linux/fs.h   |  1 +
>  6 files changed, 102 insertions(+), 20 deletions(-)
>
> diff --git a/Documentation/filesystems/fscrypt.rst 
> b/Documentation/filesystems/fscrypt.rst
> index 776ddc655f79..927d3c88816b 100644
> --- a/Documentation/filesystems/fscrypt.rst
> +++ b/Documentation/filesystems/fscrypt.rst
> @@ -184,6 +184,9 @@ replaced with HKDF or another more standard KDF in the 
> future.
>  Encryption modes and usage
>  ==
>
> +Available modes
> +---
> +
>  fscrypt allows one encryption mode to be specified for file contents
>  and one encryption mode to be specified for filenames.  Different
>  directory trees are permitted to use different encryption modes.
> @@ -191,24 +194,52 @@ Currently, the following pairs of encryption modes are 
> supported:
>
>  - AES-256-XTS for co