On 09/15/2016 07:47 PM, Alex Elsayed wrote:
On Thu, 15 Sep 2016 19:33:48 +0800, Anand Jain wrote:

Thanks for commenting. pls see inline below.

On 09/15/2016 12:53 PM, Alex Elsayed wrote:
On Tue, 13 Sep 2016 21:39:46 +0800, Anand Jain wrote:

This patchset adds btrfs encryption support.

The main objective of this series is to have bugs fixed and stability.
I have verified with fstests to confirm that there is no regression.

A design write-up is coming next, however here below is the quick
example on the cli usage. Please try out, let me know if I have missed
something.

Also would like to mention that a review from the security experts is
due,
which is important and I believe those review comments can be
accommodated without major changes from here.

Also yes, thanks for the emails, I hear, per file encryption and
inline with vfs layer is also important, which is wip among other
things in the list.

As of now these patch set supports encryption on per subvolume, as
managing properties on per subvolume is a kind of core to btrfs, which
is easier for data center solution-ing, seamlessly persistent and easy
to manage.


Steps:
-----

Make sure following kernel TFMs are compiled in.
# cat /proc/crypto | egrep 'cbc\(aes\)|ctr\(aes\)'
name         : ctr(aes)
name         : cbc(aes)

First problem: These are purely encryption algorithms, rather than AE
(Authenticated Encryption) or AEAD (Authenticated Encryption with
Associated Data). As a result, they are necessarily vulnerable to
adaptive chosen-ciphertext attacks, and CBC has historically had other
issues. I highly recommend using a well-reviewed AE or AEAD mode, such
as AES-GCM (as ecryptfs does), as long as the code can handle the
ciphertext being longer than the plaintext.

If it _cannot_ handle the ciphertext being longer than the plaintext,
please consider that a very serious red flag: It means that you cannot
provide better security than block-level encryption, which greatly
reduces the benefit of filesystem-integrated encryption. Being at the
extent level _should_ permit using AEAD - if it does not, something is
wrong.

If at all possible, I'd suggest _only_ permitting AEAD cipher modes to
be used.

Anyway, even for block-level encryption, CTR and CBC have been
considered obsolete and potentially dangerous to use in disk encryption
for quite a while - current recommendations for block-level encryption
are to use either a narrow-block tweakable cipher mode (such as XTS),
or a wide- block one (such as EME or CMC), with the latter providing
slightly better security, but worse performance.

   Yes. CTR should be changed, so I have kept it as a cli option. And
   with the current internal design, hope we can plugin more algorithms
   as suggested/if-its-outdated and yes code can handle (or with a
   little tweak) bigger ciphertext (than plaintext) as well.

   encryption + keyhash (as below) + Btrfs-data-checksum provides
   similar to AE,  right ?

No, it does not provide anything remotely similar to AE. AE requires
_cryptographic_ authentication of the data. Not only is a CRC (as Btrfs
uses for the data checksum) not enough, a _cryptographic hash_ (such as
SHA256) isn't even enough. A MAC (message authentication code) is
necessary.

Moreover, combining an encryption algorithm and a MAC is very easy to get
wrong, in ways that absolutely ruin security - as an example, see the
Vaudenay/Lucky13 padding oracle attacks on TLS.

In order for this to be secure, you need to use a secure encryption
system that also authenticates the data in a cryptographically secure
manner. Certain schemes are well-studied and believed to be secure - AES-
GCM and ChaCha20-Poly1305 are common and well-regarded, and there's a
generic security reduction for Encrypt-then-MAC constructions (using CTR
together with HMAC in such a construction is generally acceptable).

The Btrfs data checksum is wholly inadequate, and the keyhash is a non-
sequitur - it prevents accidentally opening the subvolume with the wrong
key, but neither it (nor the btrfs data checksum, which is a CRC rather
than a cryptographic MAC) protect adequately against malicious corruption
of the ciphertext.

I'd suggest pulling in Herbert Xu, as he'd likely be able to tell you
what of the Crypto API is actually sane to use for this.


 As mentioned 'inline with vfs layer' I mean to say to use
 fs/crypto KPIs. Which I haven't seen what parts of the code
 was made as generic KPIs from ext4. If that's solving the
 problem, then it would here as well.


Create encrypted subvolume.
# btrfs su create -e 'ctr(aes)' /btrfs/e1 Create subvolume '/btrfs/e1'
Passphrase:
Again passphrase:

I presume the command first creates a key, then creates a subvolume
referencing that key? If so, that seems sensible.

  Hmm I didn't get the why part, any help ? (this doesn't encrypt
  metadata part).

Basically, if your tool merely sets up an entry in the kernel keyring,
then calls the subvolume creation interface (passing in the key ID), then
it can be composed with more advanced tooling that generates the key in a
different manner.

If, instead, you call the subvolume creation API with a flag saying
"please also create a key", then it does not compose and is inflexible.

That then becomes an obstacle to later extensions, such as trusted &
encrypted keys.

  Yes key creation and subvol create are separate and independent.

Thanks, Anand


A key is created and its hash is updated into the subvolume item,
and then added to the system keyctl.
# btrfs su show /btrfs/e1 | egrep -i encrypt
        Encryption:             ctr(aes)@btrfs:75197c8e (594790215)

# keyctl show 594790215 Keyring
 594790215 --alsw-v      0     0  logon: btrfs:75197c8e

That's entirely reasonable, though you may want to support "trusted and
encrypted keys" (Documentation/security/keys-trusted-encrypted.txt)

   Yes. that's in the list.


Okay, good to hear!

<snip>

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to