Re: [RFC PATCH 0/3] move WEP implementation to skcipher interface

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Eric,

On 06/07/2019 05:40 PM, Eric Biggers wrote:
On Fri, Jun 07, 2019 at 04:54:04PM -0500, Denis Kenzior wrote:
Hi Eric,

On 06/07/2019 04:41 PM, Eric Biggers wrote:
On Fri, Jun 07, 2019 at 04:28:59PM -0500, Denis Kenzior wrote:
Hi Eric,

On 06/07/2019 04:15 PM, Eric Biggers wrote:
On Fri, Jun 07, 2019 at 03:45:45PM -0500, Denis Kenzior wrote:
Hi Ard,


Ah ok, good to know. That does imply that the driver is not entirely
broken, which is good news I suppose.


Not entirely, but we did have to resort to using multiple sockets, otherwise
parallel encrypt/decrypt operations on the socket would result in invalid
behavior.  Probably due to the issue Eric already pointed out.

No such issue with any other ciphers that we use.

Regards,
-Denis

Okay, that sucks, so we do have to keep "ecb(arc4)" in the crypto API then.  And
we can't fix its name to be just "arc4".  It's odd that someone would choose to
use AF_ALG over writing a 20 line arc4_crypt() in userspace, but whatever.

Yes, "ecb(arc4)" isn't currently thread safe.  ARC4 uses a single key whereas
modern stream ciphers use a key + IV.  To comply with the crypto API it would
have to copy the key to a stack buffer for each encryption/decryption.  But it
doesn't; it just updates the key instead, making it non thread safe.  If users
are actually relying on that, we'll have to settle for adding a mutex instead.

Well the issue isn't even about being thread safe.  We run a single thread
in iwd.  The details are a bit fuzzy now due to time elapsed, but if I
recall correctly, even behavior like:

fd = socket();
bind(fd, ecb(arc4));
setsockopt(fd, ...key...);

sendmsg(fd, OP_ENCRYPT, ...);
sendmsg(fd, OP_DECRYPT, ...);
sendmsg(fd, OP_ENCRYPT, ...);

would produce different (incorrect) encrypted results compared to

sendmsg(fd, OP_ENCRYPT, ...)
sendmsg(fd, OP_ENCRYPT, ...)


That's because currently each operation uses the next bytes from the keystream,
and a new keystream is started only by setsockopt(..., ALG_SET_KEY, ...).
There's no difference between ARC4 encryption and decryption; both just XOR the
keystream with the data.  Are you saying you expected each encryption to be a
continuation of the previous encryption, but decryptions to be independent?


 From a userspace / api perspective, yes I would have expected the encrypt
and decrypt to work independently.  No biggie now, but I remember being
surprised when this bit me as no other cipher had this behavior.  E.g.
interleaving of operations seemed to only affect arc4 results.

Are the exact semantics spelled out somewhere?


For all other skcipher algorithms, every operation is independent and depends
only on the key which was set previously on the algorithm socket, plus the IV
provided for the operation.  There is no way to perform a single encryption or
decryption incrementally in multiple parts, unless the algorithm supports it
naturally by updating the IV (e.g. CBC mode).

Right, that is what I thought.


As I am attempting to explain, ecb(arc4) does not implement this API correctly
because it updates the *key* after each operation, not the IV.  I doubt this is
documented anywhere, but this can only be changed if people aren't relying on it
already.

It sounds to me like it was broken and should be fixed. So our vote / preference is to have ARC4 fixed to follow the proper semantics. We can deal with the kernel behavioral change on our end easily enough; the required workarounds are the worse evil.

Regards,
-Denis



[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]

  Powered by Linux