On Mon, Oct 7, 2013 at 5:23 PM, John-Mark Gurney <jmg_at_funkthat.com> wrote: > Gleb Kurtsou wrote this message on Mon, Oct 07, 2013 at 16:47 -0700: >> On Mon, Oct 7, 2013 at 4:17 PM, John-Mark Gurney <jmg_at_funkthat.com> wrote: >> > Gleb Kurtsou wrote this message on Mon, Oct 07, 2013 at 09:31 -0700: >> >> Patch is available here: >> >> https://github.com/glk/freebsd-head/commit/b4d2c4a5f42f88fdd07cb75feba3467e4d4c043c.patch >> > >> > Is there a reason you are writing your own AES-NI implementation instead >> > of using the OpenCrypto framework? >> >> It reuses the same AES-NI implementation used by opencrypto, >> but code doesn't use opencrypto directly. Main limitation in opencrypto is >> that is incomplete implementation AES-XTS -- it doesn't implement ciphertext >> stealing. opencrypto contexts seemed to be too much overhead list time I > > I remember noticing that when I was working on it.. but as there are > different modes of packing/padding, I decided not to do anything with > that... > > I also don't like your lack of comments arround xts_lastblock and about > why it is accessing dst - 1... To me, you should pass in the previous > block as an arg to xts_lastblock instead of doing dst[-1]... You did > comment what you're doing (m - 1), but not why it is safe to do that... > There is no comment that you're implementing ciphertext stealing w/ the > function which makes it even harder to understand that you'd going it > properly... The code comes from University of Tsukuba. The function is internal to the module and is safe to use that way if you look at pefs_xts_block_encrypt and pefs_xts_block_decrypt. > It wouldn't be hard to add ciphertext stealing to the opencrypto > implementation if that is really all that is missing... but.. > >> looked at them especially in the case of multiple keys per file system in PEFS. >> AES-NI interface is not designed to be used outside of opencrypto thus >> some minor copy-past. > > We have discovered that by the "minor" copy/paste we now have an > inferior implementation of AES-XTS... If it performed similar to the > one before it, it is over 10x slower than the one that I committed.. > >> > I updated the kernel's AES-NI implementation to have a very fast >> > AES-XTS... Upon looking at your implementation, you have a very >> > slow implementation as you do not pipeline AES-XTS at all... Please >> > switch to using the opencrypto version.. You'll then be able to make >> > use of any accelerators that other platforms may have... >> >> I was looking into incorporating AES_XTS pipeline changes in PEFS. >> I'd like to avoid switching to opencrypto at this point. But pipelining is >> doable without opencrypto and copying code around. > > I really don't like the idea of adding yet anothe AES-XTS implementation > to our tree (especially considering how bad both the previous one and > this one is)... Even if you do bring over the pipeline changes... > It'll be yet another copy of code to maintain and port performance > improvements too... > > We could always refactor the AES-NI code to make it more usable outside > the opencrypto framework as a stepping stone to possibly using pjd's > improved opencrypto framework... Refactoring AES-NI would be ideal, it would also be great to extract AES-XTS implementation and make it usable outside of opencrypto adding ciphertext stealing. > But copy/pasting just because we don't want to do a bit more work isn't > good justification... It's not that I "don't want to do a bit more work" I never said that, it's rather about avoiding changes after KBI freeze. >> > Are there plans to add authentication to this scheme? See that as a >> > todo, but w/o authentication, you can't store anything reliably on it.. >> > And w/ XTS, the attacker can take pot shots at your file in 16 byte >> > chuncks... >> >> I have data authentication prototype. It's too far from being complete, >> and I keep working on it as time permits. There are a few more ideas >> I'd like to work on while redesigning the file system. >> >> Patch also includes hmac and pkcs5v2 implementations which in fact >> were generic versions to go under sys/crypto until yesterday. >> Considering we are late in code freeze already I've merged them >> into PEFS not to touch geli code with the patch. > > Can't you keep them named the same under sys/crypto and just link w/ them > as necessary to prevent repo churn when you finally integrate them into > sys/crypto? That seems better than moving them arround, though I guess > w/ svn, it isn't as big of a deal... Someone w/ a repo hat on should > chime in here... It won't be possible at least because of pkcs5v2_genkey() (name collision) being defined internally in geli and pkcs5v2 using hmac from geli. Moving them to sys/crypto in 11-CURRENT is a minor issue IMHO. I was pushing those changes to HEAD years ago, but they got stuck somewhere in review process. >> > The only reason I'm running zfs on geli w/o authentication is that I'm >> > using a 256bit checksum, so the chances of someone modifing two blocks >> > to fool zfs into decrypting the correct new checksum value for their >> > modified block is very small... In short, I'm trusting zfs to do the >> > authentication for me... > > -- > John-Mark Gurney Voice: +1 415 225 5579 > > "All that I will do, has been done, All that I have, has not."Received on Mon Oct 07 2013 - 22:41:11 UTC
This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:40:42 UTC