linux-f2fs-devel.lists.sourceforge.net archive mirror
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers@kernel.org>
To: "Theodore Y. Ts'o" <tytso@mit.edu>
Cc: Satya Tangirala <satyat@google.com>,
	linux-api@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net,
	linux-fscrypt@vger.kernel.org, keyrings@vger.kernel.org,
	linux-mtd@lists.infradead.org, linux-crypto@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org,
	Paul Crowley <paulcrowley@google.com>
Subject: Re: [f2fs-dev] [PATCH v7 16/16] fscrypt: document the new ioctls and policy version
Date: Mon, 29 Jul 2019 14:36:43 -0700	[thread overview]
Message-ID: <20190729213642.GI169027@gmail.com> (raw)
In-Reply-To: <20190729020009.GA3863@mit.edu>

On Sun, Jul 28, 2019 at 10:00:09PM -0400, Theodore Y. Ts'o wrote:
> On Fri, Jul 26, 2019 at 03:41:41PM -0700, Eric Biggers wrote:
> > +- The kernel cannot magically wipe copies of the master key(s) that
> > +  userspace might have as well.  Therefore, userspace must wipe all
> > +  copies of the master key(s) it makes as well.  Naturally, the same
> > +  also applies to all higher levels in the key hierarchy.  Userspace
> > +  should also follow other security precautions such as mlock()ing
> > +  memory containing keys to prevent it from being swapped out.
> 
> Normally, shouldn't userspace have wiped all copies of the master key
> after they have called ADD_KEY?  Why should they be left hanging
> around?  Waiting until REMOVE_KEY to remove other copies of the master
> key seems.... late.

Correct, normally userspace should wipe its copy of the key immediately after
adding it to the kernel.  I'll clarify that here.

> 
> > +- In general, decrypted contents and filenames in the kernel VFS
> > +  caches are freed but not wiped.  Therefore, portions thereof may be
> > +  recoverable from freed memory, even after the corresponding key(s)
> > +  were wiped.  To partially solve this, you can set
> > +  CONFIG_PAGE_POISONING=y in your kernel config and add page_poison=1
> > +  to your kernel command line.  However, this has a performance cost.
> 
> ... and even this won't help if you have swap configured....

Yes, but that's a larger issue.  Unencrypted data can be written to swap and
then be recovered from disk offline.  This has nothing to do with whether the
key is ever removed on-line or not.  So swap really could use its own mention
somewhere else, maybe in the "Offline attacks" section.

> 
> > +v1 encryption policies have some weaknesses with respect to online
> > +attacks:
> > +
> > +- There is no verification that the provided master key is correct.
> > +  Consequently, malicious users can associate the wrong key with
> > +  encrypted files, even files to which they have only read-only
> > +  access.
> 
> Yes, but they won't be able to trick other users into using that
> incorrect key.  With the old interface, it gets written into the
> user's session keyring, which won't get used by another user.  And
> with the newer interface, only root is allowed to set v1 key.
> 

As mentioned in a previous reply, they *can* trick other users into using that
incorrect key, by opening files using that incorrect key.  The incorrect key is
then cached for everyone.  (This assumes the other users have at least read
access to the file.  If it's mode 0700, this won't work.)

> > +Master keys should be pseudorandom, i.e. indistinguishable from random
> > +bytestrings of the same length.  This implies that users **must not**
> > +directly use a password as a master key, zero-pad a shorter key, or
> > +repeat a shorter key.
> 
> These paragraphs starts a bit funny, since we first say "should" in
> the first sentence, and then it's followed up by "**must not**" in the
> second sentence.  Basically, they *could* do this, but it would just
> weaken the security of the system significantly.
> 
> At the very least, we should explain the basis of the recommendation.

I think we should go with "must" instead of "should".

Basically the point of this paragraph is to explain that the API takes a real
cryptographic key of the full given length.

Otherwise the security guarantees for the algorithms the master key may be used
in (AES-128-ECB KDF, HKDF-SHA512, or Adiantum) aren't guaranteed to hold.

One can argue about how much of a problem this actually is, like how unsalted
HKDF on a key with unevenly distributed entropy is *probably* fine in practice
(and much better than the AES-128-ECB KDF).  But the security proof for unsalted
HKDF actually still assumes a pseudorandom key.  It's only randomly salted HKDF
that doesn't.

I'd strongly prefer to go with *must* for things that are necessary for the
security proofs or cryptanalysis to apply, even if they *might* still be "good
enough" in practice.

I'll try to find a better way to word this paragraph.

> 
> > +The KDF used for a particular master key differs depending on whether
> > +the key is used for v1 encryption policies or for v2 encryption
> > +policies.  Users **must not** use the same key for both v1 and v2
> > +encryption policies.
> 
> "Must not" seems a bit strong.  If they do, and a v1 per-file key and
> nonce leaks out, then the encryption key will be compromised.  So the
> strength of the key will be limited by the weaknesses of the v1
> scheme.  But it's not like using a that was originally meant for v1,
> and then using it for v2, causes any additional weakness.  Right?
> 

Probably, but we don't know for sure.  It's theoretically possible that
cryptanalysis of two cryptographic primitives A and B, where they are each given
the same key, could be much easier than attacking A or B individually.

So again, I'd prefer to go with *must not* for things where there is no theory
of cryptography that says it is okay, even if *probably* someone could get away
with doing it in practice.

- Eric


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

      reply	other threads:[~2019-07-29 21:36 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-26 22:41 [f2fs-dev] [PATCH v7 00/16] fscrypt: key management improvements Eric Biggers
2019-07-26 22:41 ` [f2fs-dev] [PATCH v7 01/16] fs, fscrypt: move uapi definitions to new header <linux/fscrypt.h> Eric Biggers
2019-07-28 15:08   ` Theodore Y. Ts'o
2019-07-26 22:41 ` [f2fs-dev] [PATCH v7 02/16] fscrypt: use FSCRYPT_ prefix for uapi constants Eric Biggers
2019-07-26 22:41 ` [f2fs-dev] [PATCH v7 03/16] fscrypt: use FSCRYPT_* definitions, not FS_* Eric Biggers
2019-07-26 22:41 ` [f2fs-dev] [PATCH v7 04/16] fscrypt: add ->ci_inode to fscrypt_info Eric Biggers
2019-07-28 15:09   ` Theodore Y. Ts'o
2019-07-26 22:41 ` [f2fs-dev] [PATCH v7 05/16] fscrypt: refactor v1 policy key setup into keysetup_legacy.c Eric Biggers
2019-07-28 15:40   ` Theodore Y. Ts'o
2019-07-29 19:37     ` Eric Biggers
2019-07-26 22:41 ` [f2fs-dev] [PATCH v7 06/16] fscrypt: add FS_IOC_ADD_ENCRYPTION_KEY ioctl Eric Biggers
2019-07-28 18:50   ` Theodore Y. Ts'o
2019-07-29 19:46     ` Eric Biggers
2019-07-29 20:14       ` Theodore Y. Ts'o
2019-07-26 22:41 ` [f2fs-dev] [PATCH v7 07/16] fscrypt: add FS_IOC_REMOVE_ENCRYPTION_KEY ioctl Eric Biggers
2019-07-28 19:24   ` Theodore Y. Ts'o
2019-07-29 19:58     ` Eric Biggers
2019-07-31 18:38       ` Eric Biggers
2019-07-31 23:38         ` Theodore Y. Ts'o
2019-08-01  1:11           ` Eric Biggers
2019-08-01  5:31             ` Theodore Y. Ts'o
2019-08-01 18:35               ` Eric Biggers
2019-08-01 18:46                 ` Eric Biggers
2019-08-01 22:04               ` Eric Biggers
2019-08-02  4:38                 ` Eric Biggers
2019-08-12 14:16                   ` Theodore Y. Ts'o
2019-07-26 22:41 ` [f2fs-dev] [PATCH v7 08/16] fscrypt: add FS_IOC_GET_ENCRYPTION_KEY_STATUS ioctl Eric Biggers
2019-07-28 19:30   ` Theodore Y. Ts'o
2019-07-26 22:41 ` [f2fs-dev] [PATCH v7 09/16] fscrypt: add an HKDF-SHA512 implementation Eric Biggers
2019-07-28 19:39   ` Theodore Y. Ts'o
2019-07-29 20:29     ` Eric Biggers
2019-07-29 21:42       ` James Bottomley
2019-07-26 22:41 ` [f2fs-dev] [PATCH v7 10/16] fscrypt: v2 encryption policy support Eric Biggers
2019-07-28 21:17   ` Theodore Y. Ts'o
2019-07-29 20:46     ` Eric Biggers
2019-07-26 22:41 ` [f2fs-dev] [PATCH v7 11/16] fscrypt: allow unprivileged users to add/remove keys for v2 policies Eric Biggers
2019-07-28 21:22   ` Theodore Y. Ts'o
2019-07-26 22:41 ` [f2fs-dev] [PATCH v7 12/16] fscrypt: require that key be added when setting a v2 encryption policy Eric Biggers
2019-07-28 21:24   ` Theodore Y. Ts'o
2019-07-26 22:41 ` [f2fs-dev] [PATCH v7 13/16] ext4: wire up new fscrypt ioctls Eric Biggers
2019-07-28 21:24   ` Theodore Y. Ts'o
2019-07-26 22:41 ` [f2fs-dev] [PATCH v7 14/16] f2fs: " Eric Biggers
2019-07-30  0:36   ` Jaegeuk Kim
2019-08-02  8:10   ` Chao Yu
2019-08-02 17:31     ` Eric Biggers
2019-08-04  9:42       ` Chao Yu
2019-07-26 22:41 ` [f2fs-dev] [PATCH v7 15/16] ubifs: " Eric Biggers
2019-07-30  0:39   ` Theodore Y. Ts'o
2019-07-26 22:41 ` [f2fs-dev] [PATCH v7 16/16] fscrypt: document the new ioctls and policy version Eric Biggers
2019-07-29  2:00   ` Theodore Y. Ts'o
2019-07-29 21:36     ` Eric Biggers [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190729213642.GI169027@gmail.com \
    --to=ebiggers@kernel.org \
    --cc=keyrings@vger.kernel.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-fscrypt@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=paulcrowley@google.com \
    --cc=satyat@google.com \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).