Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 7 Oct 2013 17:41:10 -0700
From:      Gleb Kurtsou <gleb.kurtsou@gmail.com>
To:        Gleb Kurtsou <gleb.kurtsou@gmail.com>, Gleb Kurtsou <gleb@freebsd.org>,  "freebsd-current@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:  <CADDB7yFsCfBpVvV4ew866ANr_=CpjENmjen2Oakm5wbmwZ0cQQ@mail.gmail.com>
In-Reply-To: <20131008002314.GA56872@funkthat.com>
References:  <20131007163111.GB1590@reks.swifttest.com> <20131007231734.GY56872@funkthat.com> <CADDB7yFR-s2PExPCru2H9TM%2Bgfi6LoV0XygPGR=NH%2Bxir=bBOw@mail.gmail.com> <20131008002314.GA56872@funkthat.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Oct 7, 2013 at 5:23 PM, John-Mark Gurney <jmg@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@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."



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CADDB7yFsCfBpVvV4ew866ANr_=CpjENmjen2Oakm5wbmwZ0cQQ>