diff mbox

[3/3] encrypted-keys: document new fscrypt key format

Message ID 20180110124418.24385-3-git@andred.net (mailing list archive)
State New, archived
Headers show

Commit Message

André Draszik Jan. 10, 2018, 12:44 p.m. UTC 
Signed-off-by: André Draszik <git@andred.net>
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: David Howells <dhowells@redhat.com>
Cc: James Morris <james.l.morris@oracle.com>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: "Theodore Y. Ts'o" <tytso@mit.edu>
Cc: Jaegeuk Kim <jaegeuk@kernel.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Kees Cook <keescook@chromium.org>
Cc: linux-integrity@vger.kernel.org
Cc: keyrings@vger.kernel.org
Cc: linux-security-module@vger.kernel.org
Cc: linux-fscrypt@vger.kernel.org
Cc: linux-doc@vger.kernel.org
---
 Documentation/security/keys/fscrypt.rst           | 67 +++++++++++++++++++++++
 Documentation/security/keys/trusted-encrypted.rst | 12 ++--
 2 files changed, 74 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/security/keys/fscrypt.rst

Comments

Eric Biggers Jan. 11, 2018, 4:48 a.m. UTC | #1 
Hi André,

On Wed, Jan 10, 2018 at 12:44:18PM +0000, André Draszik wrote:
> diff --git a/Documentation/security/keys/fscrypt.rst b/Documentation/security/keys/fscrypt.rst
> new file mode 100644
> index 000000000000..e4a29592513e
> --- /dev/null
> +++ b/Documentation/security/keys/fscrypt.rst
> @@ -0,0 +1,67 @@
> +========================================
> +Encrypted keys for the fscrypt subsystem
> +========================================

There is now documentation for fscrypt in Documentation/filesystems/fscrypt.rst;
see in particular the "Adding keys" section.  The documentation for any new ways
to add keys should go in there.

> +
> +fscrypt allows file systems to implement transparent encryption and decryption
> +of files, similar to eCryptfs, using keys derived from a master key descriptor.

Note that the master key *descriptor* refers to the hex string used in the
keyring key description.  It is not the same as the master key itself, which is
stored in the payload.  The cryptography is done with the master key, not with
the master key *descriptor*.

> +In order to avoid known-plaintext attacks, the datablob obtained through
> +commands 'keyctl print' or 'keyctl pipe' does not contain the overall
> +fscrypt_key, the contents of which is well known, but only the master key
> +descriptor itself in encrypted form.
> +
> +The fscrypt subsystem may really benefit from using encrypted keys in that the
> +required key can be securely generated by an Administrator and provided at boot
> +time after the unsealing of a 'trusted' key in order to perform the mount in a
> +controlled environment.  Another advantage is that the key is not exposed to
> +threats of malicious software, because it is available in clear form only at
> +kernel level.

Please be very clear about exactly what security properties are achieved by
using encrypted-keys.  Note that such keys are present in the clear in kernel
memory, so they will be exposed by any exploit that compromises the kernel, or
even just finds a way to read its memory.  (And if you've been paying attention
in the last week, you may be aware that certain CPU vendors have "helpfully"
made reading kernel memory quite easy.)  So, it's definitely *not* categorically
true that "the key is not exposed to threats of malicious software".

Also note that fscrypt is already using the "logon" key type which cannot be
read by userspace (without exploits).  This is different from eCryptfs which
uses the "user" key type.

> +Usage::
> +
> +   keyctl add encrypted fscrypt:policy "new fscrypt key-type:master-key-name keylen" ring
> +   keyctl add encrypted fscrypt:policy "load hex_blob" ring
> +   keyctl update keyid "update key-type:master-key-name"
> +
> +Where::
> +
> +	policy:= '<16 hexadecimal characters>'
> +	key-type:= 'trusted' | 'user'
> +	keylen:= 16 | 32 | 64
> +
> +
> +Example of encrypted key usage with the fscrypt subsystem:
> +
> +Create an encrypted key "1234567890123456" of length 64 bytes with format
> +'fscrypt' and save it using a previously loaded user key "test"::
> +
> +    $ keyctl add encrypted fscrypt:1234567890123456 "new fscrypt user:test 64" @u
> +    1023935199
> +
> +    $ keyctl print 1023935199
> +    fscrypt user:test 64 e5606689fdc25d78a787249f4069fb3b007e992f4b21d0eda60
> +    c97986fc2e3326b5542e2b32216fc5007d9fd19cd3cb6668fa9850e954d2ba25e1b8a331
> +    1b0c1f20666c
> +
> +    $ keyctl pipe 1023935199 > fscrypt.blob

What is the point of having the kernel wrap a key with the "user" key type?  It
seems pointless; userspace could just do it instead.

Note that if you use keyctl_read() to read from an encrypted-key, you can
actually request that the key be encrypted using an arbitrary key of the type
with which the key is supposed to be wrapped.  This can be done by adding a key
to your thread/process/session keyring whose type and description matches the
intended wrapping key.  Thus, any "encrypted" key wrapped with a "user" key can
effectively be retrieved in the clear by calling keyctl_read(), then decrypting
the ciphertext in userspace.

Perhaps that's actually a bug; I don't know.  But using "user" wrapping keys
seems pretty pointless anyway.

I think it's really only "trusted" wrapping keys where this new feature would
have any useful security properties.  So the documentation needs to explain
that, and use that in the examples.

Eric
André Draszik Jan. 17, 2018, 2:38 p.m. UTC | #2 
Hi Eric,

On Wed, 2018-01-10 at 20:48 -0800, Eric Biggers wrote:
> Hi André,
> 
> On Wed, Jan 10, 2018 at 12:44:18PM +0000, André Draszik wrote:
> > diff --git a/Documentation/security/keys/fscrypt.rst
> > b/Documentation/security/keys/fscrypt.rst
> > new file mode 100644
> > index 000000000000..e4a29592513e
> > --- /dev/null
> > +++ b/Documentation/security/keys/fscrypt.rst
> > @@ -0,0 +1,67 @@
> > +========================================
> > +Encrypted keys for the fscrypt subsystem
> > +========================================
> 
> There is now documentation for fscrypt in
> Documentation/filesystems/fscrypt.rst;
> see in particular the "Adding keys" section.  The documentation for any
> new ways
> to add keys should go in there.

Done.

> 
> > +
> > +fscrypt allows file systems to implement transparent encryption and
> > decryption
> > +of files, similar to eCryptfs, using keys derived from a master key
> > descriptor.
> 
> Note that the master key *descriptor* refers to the hex string used in the
> keyring key description.  It is not the same as the master key itself,
> which is
> stored in the payload.  The cryptography is done with the master key, not
> with
> the master key *descriptor*.

Ups, thanks.

> > [...]
> 
> Please be very clear about exactly what security properties are achieved
> by
> using encrypted-keys.

I've left out all of this in the updated documentation, as any such
information should probably be in Documentation/security/keys/trusted-
encrypted.rst in the first place.

> 
[...]
> > +
> > +Example of encrypted key usage with the fscrypt subsystem:
> > +
> > +Create an encrypted key "1234567890123456" of length 64 bytes with
> > format
> > +'fscrypt' and save it using a previously loaded user key "test"::
> > +
> > +    $ keyctl add encrypted fscrypt:1234567890123456 "new fscrypt
> > user:test 64" @u
> > +    1023935199
> > +
> > +    $ keyctl print 1023935199
> > +    fscrypt user:test 64
> > e5606689fdc25d78a787249f4069fb3b007e992f4b21d0eda60
> > +    c97986fc2e3326b5542e2b32216fc5007d9fd19cd3cb6668fa9850e954d2ba25e1b
> > 8a331
> > +    1b0c1f20666c
> > +
> > +    $ keyctl pipe 1023935199 > fscrypt.blob
> 
> What is the point of having the kernel wrap a key with the "user" key
> type?  It
> seems pointless; userspace could just do it instead.

Yes... My real reasoning is being able to use an encrypted key, backed by a
trusted TPM key.

I've updated the examples.

> 
[...]
> I think it's really only "trusted" wrapping keys where this new feature
> would
> have any useful security properties.  So the documentation needs to
> explain
> that, and use that in the examples.

You're right. Done.


Cheers,
André
Theodore Ts'o Jan. 17, 2018, 6:05 p.m. UTC | #3 
On Wed, Jan 17, 2018 at 02:38:59PM +0000, André Draszik wrote:
> > > [...]
> > 
> > Please be very clear about exactly what security properties are achieved
> > by
> > using encrypted-keys.
> 
> I've left out all of this in the updated documentation, as any such
> information should probably be in Documentation/security/keys/trusted-
> encrypted.rst in the first place.

Where is this document going to be found / when will it be written?
It seems really odd to be requesting a do code review when the
specifications aren't available and/or haven't been written yet.  I
prefer to review the *design* first, as opposed to trying to both
review the code and try to guess at the design and review my guess of
the design at the same time....

					- Ted
André Draszik Jan. 19, 2018, 9:16 a.m. UTC | #4 
Thank you Ted,

On Wed, 2018-01-17 at 13:05 -0500, Theodore Ts'o wrote:
> On Wed, Jan 17, 2018 at 02:38:59PM +0000, André Draszik wrote:
> > > > [...]
> > > 
> > > Please be very clear about exactly what security properties are
> > > achieved
> > > by
> > > using encrypted-keys.
> > 
> > I've left out all of this in the updated documentation, as any such
> > information should probably be in Documentation/security/keys/trusted-
> > encrypted.rst in the first place.
> 
> Where is this document going to be found / when will it be written?
> It seems really odd to be requesting a do code review when the
> specifications aren't available and/or haven't been written yet.  I
> prefer to review the *design* first, as opposed to trying to both
> review the code and try to guess at the design and review my guess of
> the design at the same time....

Does v3's commit message https://patchwork.kernel.org/patch/10173189/ serve
as a good enough design document?

Cheers,
Andre'
diff mbox

Patch

diff --git a/Documentation/security/keys/fscrypt.rst b/Documentation/security/keys/fscrypt.rst
new file mode 100644
index 000000000000..e4a29592513e
--- /dev/null
+++ b/Documentation/security/keys/fscrypt.rst
@@ -0,0 +1,67 @@ 
+========================================
+Encrypted keys for the fscrypt subsystem
+========================================
+
+fscrypt allows file systems to implement transparent encryption and decryption
+of files, similar to eCryptfs, using keys derived from a master key descriptor.
+
+The data structure defined by fscrypt to contain information required for the
+master key descriptor is the fscrypt_key and, currently, can be stored in a
+kernel key of the 'user' type, inserted in the user's session specific keyring
+by the userspace utilities 'keyctl', 'fscryptctl', or 'e4crypt'.
+
+The 'encrypted' key type has been extended with the introduction of the new
+format 'fscrypt' in order to be used in conjunction with the fscrypt
+subsystem.  Encrypted keys of the newly introduced format store an
+fscrypt_key in its payload with a master key descriptor randomly generated by
+the kernel and protected by the parent master key.
+
+In order to avoid known-plaintext attacks, the datablob obtained through
+commands 'keyctl print' or 'keyctl pipe' does not contain the overall
+fscrypt_key, the contents of which is well known, but only the master key
+descriptor itself in encrypted form.
+
+The fscrypt subsystem may really benefit from using encrypted keys in that the
+required key can be securely generated by an Administrator and provided at boot
+time after the unsealing of a 'trusted' key in order to perform the mount in a
+controlled environment.  Another advantage is that the key is not exposed to
+threats of malicious software, because it is available in clear form only at
+kernel level.
+
+Usage::
+
+   keyctl add encrypted fscrypt:policy "new fscrypt key-type:master-key-name keylen" ring
+   keyctl add encrypted fscrypt:policy "load hex_blob" ring
+   keyctl update keyid "update key-type:master-key-name"
+
+Where::
+
+	policy:= '<16 hexadecimal characters>'
+	key-type:= 'trusted' | 'user'
+	keylen:= 16 | 32 | 64
+
+
+Example of encrypted key usage with the fscrypt subsystem:
+
+Create an encrypted key "1234567890123456" of length 64 bytes with format
+'fscrypt' and save it using a previously loaded user key "test"::
+
+    $ keyctl add encrypted fscrypt:1234567890123456 "new fscrypt user:test 64" @u
+    1023935199
+
+    $ keyctl print 1023935199
+    fscrypt user:test 64 e5606689fdc25d78a787249f4069fb3b007e992f4b21d0eda60
+    c97986fc2e3326b5542e2b32216fc5007d9fd19cd3cb6668fa9850e954d2ba25e1b8a331
+    1b0c1f20666c
+
+    $ keyctl pipe 1023935199 > fscrypt.blob
+
+Set fscrypt policy on an (empty) encrypted directory /encrypted::
+
+    $ fscryptctl set_policy 1234567890123456 /encrypted
+
+The directory policy will remain across reboots, so after a reboot the key
+generated earlier will simply have to be loaded into the kernel keyring
+again::
+
+    $ keyctl add encrypted fscrypt:1234567890123456 "load $(cat fscrypt.blob)" @u
diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst
index 3bb24e09a332..d0250112bb4d 100644
--- a/Documentation/security/keys/trusted-encrypted.rst
+++ b/Documentation/security/keys/trusted-encrypted.rst
@@ -76,7 +76,7 @@  Usage::
 
 Where::
 
-	format:= 'default | ecryptfs'
+	format:= 'default | ecryptfs | fscrypt'
 	key-type:= 'trusted' | 'user'
 
 
@@ -169,7 +169,9 @@  Load an encrypted key "evm" from saved blob::
     24717c64 5972dcb82ab2dde83376d82b2e3c09ffc
 
 Other uses for trusted and encrypted keys, such as for disk and file encryption
-are anticipated.  In particular the new format 'ecryptfs' has been defined in
-in order to use encrypted keys to mount an eCryptfs filesystem.  More details
-about the usage can be found in the file
-``Documentation/security/keys/ecryptfs.rst``.
+are anticipated.  In particular the new formats 'ecryptfs' and 'fscrypt' have
+been defined in order to use encrypted keys to mount an eCryptfs or fscrypt
+filesystem, respectively. More details about the usage can be found in the
+files
+``Documentation/security/keys/ecryptfs.rst`` and
+``Documentation/security/keys/fscrypt.rst``.