Taylor R Campbell wrote:
> It's not clear to me that this makes a meaningful improvement in
> security. The AES code is still full of secret-dependent array
> indexing, which is the basic problem -- maybe one or two particular
> arrays are a little smaller and thus have slightly reduced cache
> footprint, but that's all.
No, this change doesn't eliminate side channel attacks but it
unifies the code with OpenBSD's sys/crypto and C implementation in
OpenSSL. The latter has also two assembly versions on amd64: one
is a variation of the C code and the other is based on AES-NI.
> There are better ways to thwart the attack this change attempts to
> mitigate. Here are two examples:
>
> 1. Implement AES-NI in the kernel and teach cgd(4) to use it.
> Requires saving and restoring the SSE state in kernel threads --
> I'm not sure offhand what the status of that on x86 is.
It's a very well known implementation but it's x86 only. Other platforms
are still vulnerable to timing attacks.
> 2. Add support for a block cipher that is not designed to leak secrets
> through timing like AES is, such as Threefish. Here's some
> portable C code for Threefish:
>
> http://mumble.net/~campbell/tmp/threefish.c
> http://mumble.net/~campbell/tmp/threefish.h
It looks nice. Is there anything that stops you from porting it to
NetBSD?
> That said, it isn't obvious to me that this change is harmful either.
It's been in OpenSSL and OpenBSD for a couple of years already.
> If you want to make it, could you also add an AES self-test with a few
> known-answer test vectors, like we have for ChaCha20 in
> sys/crypto/cprng_fast/cprng_fast.c?
Your case is simpler because you test only one algorithm on startup.
It's not clear when to test cgd algorithms. For instance, it could be
done when cgd is attached or when a key/params are received from
userspace.
While we're here, two const arrays can be made static in
crypto_core_selftest():
Index: cprng_fast.c
===================================================================
RCS file: /cvsroot/src/sys/crypto/cprng_fast/cprng_fast.c,v
retrieving revision 1.13
diff -p -u -u -r1.13 cprng_fast.c
--- cprng_fast.c 13 Apr 2015 22:43:41 -0000 1.13
+++ cprng_fast.c 1 Oct 2016 15:29:06 -0000
@@ -138,8 +138,8 @@ static const uint32_t crypto_core_consta
static void
crypto_core_selftest(void)
{
- const uint32_t zero32[8] = {0};
- const uint8_t sigma[] = "expand 32-byte k";
+ static const uint32_t zero32[8] = {0};
+ static const uint8_t sigma[] = "expand 32-byte k";
uint32_t block[16];
unsigned i;
--
Alex
Attachment:
signature.asc
Description: PGP signature