Committing PEFS to CURRENT
Gleb Kurtsou
gleb.kurtsou at gmail.com
Tue Oct 8 00:41:11 UTC 2013
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."
More information about the freebsd-current
mailing list