On Mon, Apr 24, 2017 at 04:36 +0200, Mike Belopuhov wrote: > Hi, > > This is the first diff in series to replace our table-driven AES > implementation in the crypto framework with a constant time one > authored by Thomas Pornin. I've been on the lookout for a complete > constant time implementation for several years and thanks to Thomas, > we've got a very nice one now. > > I've started fitting this code into our framework and exploring the > rijndael.c dependencies around last Christmas, but wasn't able to > get back to it until now. In my github branch newaes2[1] I've got a > complete set of diffs for those who want to go look ahead, but I > believe it's time to start getting it slowly into the tree. > > I've got a few positive test reports, however until today, the IPsec > was broken because of a somewhat tricky change[2]. Now it should be > fixed. > > This change introduces the 32-bit constant time AES implementation > from BearSSL[3] merging aes_ct.c, aes_ct_dec.c and aes_ct_enc.c. > Thomas was kind enough to review my wrapping code and has optimized > and largely rewritten it exposing additional API for NIST-compatible > subkey expansion required (in OpenBSD) by the VIA padlock driver: > AES_KeySetup_Encrypt and AES_KeySetup_Decrypt. > > Initially, I've considered using both ct and ct64 implementations but, > as Thomas has rightfully pointed out, ct64 is faster when multiple > blocks are processed at the same time which is never the case in our > framework at the moment and direct calls to AES usually implement some > kind of a cipher mode and thus are not able to take advantage of it as > well. Therefore aes_ct64.c was dropped and aes_ct.c was included > directly into the aes.c. > > OK? >
I've received OKs for all but this diff :-) > [2] > https://github.com/mbelop/src/commit/79c082e9fbe2decda63f782e4fd77fd4a722831d This commit (or it's newer reincarnation*) deserves a bit of an explanation. In the beginning I had two subkeys in the AES_CTX: one for encryption (ek) and the other one for decryption (dk). They're expanded to ek_exp and dk_exp making AES_CTX so huge that the build would fail because of increased stack usage. To mitigate that I've prepared a set of diffs to move contexts off the stack. What I didn't notice until yesterday was that we only need one subkey so I made a few changes (e.g. AES_Setkey lost its fourth argument specifying whether we're only interested in the encryption key) and suddenly AES_CTX and GMAC_CTX encompassing it stopped blowing up the stack usage. This put me on the fence regarding all these diffs moving stuff around, including this one. Now, of course there might still be an architecture where this new AES_CTX (or some other context including it) is too much so I'm keeping an eye on this. *) https://github.com/mbelop/src/commit/58b5952c43f39fa6f52f44552a0e558ac8bffcd5
