On Mon, May 22, 2017 at 07:32:07PM +0200, Mike Belopuhov wrote:
> >
> > It makes it to confirm the problem is related to the switch to
> > constant-time-aes in the context of full-disk-encryption.
> >
>
> Thanks for verifying this. I've looked through the sr_hibernate_io
> (that's hib->io_func) but couldn't find anything wrong with it. The
> only thing that springs to mind is that AES_CTX and therefore the
> XTS context (aes_xts_ctx) is larger and requires more stack space.
> Though I can't see what might be affected by that.
>
While looking at code source and `struct aes_xts_ctx` definition (in
order to add the padding stuff), I see the following:
1. `struct aes_xts_ctx` and aes_xts_* funtions are defined in
crypto/xform.c and used by regular kernel code.
with the constant-time AES (so the faulting one, before reverting), the
code was using AES_CTX and AES_*() functions.
crypto/xform.c:
123 struct aes_xts_ctx {
124 AES_CTX key1;
125 AES_CTX key2;
126 u_int8_t tweak[AES_XTS_BLOCKSIZE];
127 };
and later:
480 void
481 aes_xts_reinit(caddr_t key, u_int8_t *iv)
482 {
...
496 /* Last 64 bits of IV are always zero */
497 bzero(ctx->tweak + AES_XTS_IVSIZE, AES_XTS_IVSIZE);
498
499 AES_Encrypt(&ctx->key2, ctx->tweak, ctx->tweak);
500 }
2. `struct aes_xts_ctx` and functions are *also* defined in
lib/libsa/aes_xts.h and used by bootloader code. They are using
rijndael code.
Please note that the switch to AES_CTX does *not* touch them. There were
still use rijndael code and struct, while the kernel was using AES_CTX
version.
lib/libsa/aes_xts.h:
27 struct aes_xts_ctx {
28 rijndael_ctx key1;
29 rijndael_ctx key2;
30 u_int8_t tweak[AES_XTS_BLOCKSIZE];
31 };
lib/libsa/aes_xts.c:
25 void
26 aes_xts_reinit(struct aes_xts_ctx *ctx, u_int8_t *iv)
27 {
...
40 /* Last 64 bits of IV are always zero */
41 bzero(ctx->tweak + AES_XTS_IVSIZE, AES_XTS_IVSIZE);
42
43 rijndael_encrypt(&ctx->key2, ctx->tweak, ctx->tweak);
44 }
3. in dev/softraid.c, the hibernate code sr_hibernate_io() seems to use
to bootloader definition of `struct aes_xts_ctx`:
dev/softraid.c:
53 #ifdef HIBERNATE
54 #include <lib/libsa/aes_xts.h>
55 #include <sys/hibernate.h>
56 #include <scsi/sdvar.h>
57 #endif /* HIBERNATE */
but as it is a regular kernel code and not bootloader code, I expect the
linkage of aes_xts_*() functions to be done using kernel code (so
AES_* version).
The structure used was rijndael_ctx based (sizeof=992), and functions
used was AES_Encrypt() based. As AES_* functions operated on larger
struct (sizeof=1464), it was resulting some garbage and stack corruption
inside sr_hibernate_io().
As we move back to rijndael code for AES_XTS, bootloader code and kernel
code come back in sync, and sr_hibernate_io() uses right code now.
I see several possibles things to do to avoid future mistakes:
- in dev/softraid.c : doesn't use libsa code but standard kernel code.
But I dunno if it would be compatible or not. It depends if we share
some elements with bootloader or not.
- in crypto/xform.c and lib/libsa/aes_xts.h : add a comment to recall
that the struct should be keep in sync together.
Does this analyze makes sens ?
--
Sebastien Marie
