Date: Mon, 7 Oct 2013 17:23:14 -0700 From: John-Mark Gurney <jmg@funkthat.com> To: Gleb Kurtsou <gleb.kurtsou@gmail.com> Cc: Gleb Kurtsou <gleb@freebsd.org>, freebsd-current@freebsd.org, "delphij@freebsd.org" <delphij@freebsd.org>, Kris Moore <kris@pcbsd.org> Subject: Re: Committing PEFS to CURRENT Message-ID: <20131008002314.GA56872@funkthat.com> In-Reply-To: <CADDB7yFR-s2PExPCru2H9TM%2Bgfi6LoV0XygPGR=NH%2Bxir=bBOw@mail.gmail.com> References: <20131007163111.GB1590@reks.swifttest.com> <20131007231734.GY56872@funkthat.com> <CADDB7yFR-s2PExPCru2H9TM%2Bgfi6LoV0XygPGR=NH%2Bxir=bBOw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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@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... 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... But copy/pasting just because we don't want to do a bit more work isn't good justification... > > 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... > > 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."
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20131008002314.GA56872>