[PATCH 1/3] encrypted-keys: add fscrypt format support

[PATCH 1/3] encrypted-keys: add fscrypt format support

[André Draszik]

This is heavily based on commit 79a73d188726
("encrypted-keys: add ecryptfs format support").

The 'encrypted' key type defines its own payload format which contains a
symmetric key randomly generated that cannot be used directly by the
fscrypt subsystem, because it instead expects an fscrypt_key structure.

This patch introduces the new format 'fscrypt' that allows to store an
fscrypt_key structure inside the encrypted key payload containing
a randomly generated symmetric key, as the same for the format 'default'
and 'ecryptfs'.

More details about the usage of encrypted keys with the fscrypt
subsystem can be found in the file 'Documentation/security/keys/fscrypt.rst'.

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: 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
---
 security/keys/encrypted-keys/Makefile         |  2 +-
 security/keys/encrypted-keys/encrypted.c      | 19 +++++++-
 security/keys/encrypted-keys/fscrypt_format.c | 70 +++++++++++++++++++++++++++
 security/keys/encrypted-keys/fscrypt_format.h | 20 ++++++++
 4 files changed, 108 insertions(+), 3 deletions(-)
 create mode 100644 security/keys/encrypted-keys/fscrypt_format.c
 create mode 100644 security/keys/encrypted-keys/fscrypt_format.h

diff --git a/security/keys/encrypted-keys/Makefile b/security/keys/encrypted-keys/Makefile
index 7a44dce6f69d..586702ce9622 100644
--- a/security/keys/encrypted-keys/Makefile
+++ b/security/keys/encrypted-keys/Makefile
@@ -5,7 +5,7 @@
 
 obj-$(CONFIG_ENCRYPTED_KEYS) += encrypted-keys.o
 
-encrypted-keys-y := encrypted.o ecryptfs_format.o
+encrypted-keys-y := encrypted.o ecryptfs_format.o fscrypt_format.o
 masterkey-$(CONFIG_TRUSTED_KEYS) := masterkey_trusted.o
 masterkey-$(CONFIG_TRUSTED_KEYS)-$(CONFIG_ENCRYPTED_KEYS) := masterkey_trusted.o
 encrypted-keys-y += $(masterkey-y) $(masterkey-m-m)
diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index d92cbf9687c3..b570a930583a 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -37,6 +37,7 @@
 
 #include "encrypted.h"
 #include "ecryptfs_format.h"
+#include "fscrypt_format.h"
 
 static const char KEY_TRUSTED_PREFIX[] = "trusted:";
 static const char KEY_USER_PREFIX[] = "user:";
@@ -45,6 +46,7 @@ static const char hmac_alg[] = "hmac(sha256)";
 static const char blkcipher_alg[] = "cbc(aes)";
 static const char key_format_default[] = "default";
 static const char key_format_ecryptfs[] = "ecryptfs";
+static const char key_format_fscrypt[] = "fscrypt";
 static unsigned int ivsize;
 static int blksize;
 
@@ -62,12 +64,13 @@ enum {
 };
 
 enum {
-	Opt_error = -1, Opt_default, Opt_ecryptfs
+	Opt_error = -1, Opt_default, Opt_ecryptfs, Opt_fscrypt
 };
 
 static const match_table_t key_format_tokens = {
 	{Opt_default, "default"},
 	{Opt_ecryptfs, "ecryptfs"},
+	{Opt_fscrypt, "fscrypt"},
 	{Opt_error, NULL}
 };
 
@@ -185,7 +188,7 @@ static int datablob_parse(char *datablob, const char **format,
 	}
 	key_cmd = match_token(keyword, key_tokens, args);
 
-	/* Get optional format: default | ecryptfs */
+	/* Get optional format: default | ecryptfs | fscrypt */
 	p = strsep(&datablob, " \t");
 	if (!p) {
 		pr_err("encrypted_key: insufficient parameters specified\n");
@@ -194,6 +197,7 @@ static int datablob_parse(char *datablob, const char **format,
 
 	key_format = match_token(p, key_format_tokens, args);
 	switch (key_format) {
+	case Opt_fscrypt:
 	case Opt_ecryptfs:
 	case Opt_default:
 		*format = p;
@@ -634,6 +638,11 @@ static struct encrypted_key_payload *encrypted_key_alloc(struct key *key,
 		}
 		decrypted_datalen = ECRYPTFS_MAX_KEY_BYTES;
 		payload_datalen = sizeof(struct ecryptfs_auth_tok);
+	} else if (format && !strcmp(format, key_format_fscrypt)) {
+		ret = fscrypt_encrypted_key_reserve_payload(decrypted_datalen,
+							    &payload_datalen);
+		if (ret < 0)
+			return ERR_PTR(ret);
 	}
 
 	encrypted_datalen = roundup(decrypted_datalen, blksize);
@@ -734,6 +743,8 @@ static void __ekey_init(struct encrypted_key_payload *epayload,
 		if (!strcmp(format, key_format_ecryptfs))
 			epayload->decrypted_data =
 				ecryptfs_get_auth_tok_key((struct ecryptfs_auth_tok *)epayload->payload_data);
+		else if (!strcmp(format, key_format_fscrypt))
+			fscrypt__ekey_init(epayload);
 
 		memcpy(epayload->format, format, format_len);
 	}
@@ -762,6 +773,10 @@ static int encrypted_init(struct encrypted_key_payload *epayload,
 
 		ecryptfs_fill_auth_tok((struct ecryptfs_auth_tok *)epayload->payload_data,
 				       key_desc);
+	} else if (format && !strcmp(format, key_format_fscrypt)) {
+		ret = fscrypt_valid_desc(key_desc);
+		if (ret < 0)
+			return ret;
 	}
 
 	__ekey_init(epayload, format, master_desc, datalen);
diff --git a/security/keys/encrypted-keys/fscrypt_format.c b/security/keys/encrypted-keys/fscrypt_format.c
new file mode 100644
index 000000000000..7620c0fa3ff9
--- /dev/null
+++ b/security/keys/encrypted-keys/fscrypt_format.c
@@ -0,0 +1,70 @@
+/*
+ * fscrypt_format.c: helper functions for the encrypted key type
+ *
+ * Copyright (C) 2006 International Business Machines Corp.
+ * Copyright (C) 2010 Politecnico di Torino, Italy
+ *                    TORSEC group -- http://security.polito.it
+ *
+ * Authors:
+ * André Draszik <git@andred.net>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 2 of the License.
+ */
+
+#include <linux/ctype.h>
+#define __FS_HAS_ENCRYPTION 0
+#include <linux/fscrypt.h>
+#include <keys/encrypted-type.h>
+#include <crypto/aes.h>
+#include "fscrypt_format.h"
+
+
+#define FS_KEY_DESCRIPTOR_HEX_SIZE (FS_KEY_DESCRIPTOR_SIZE*2)
+
+int fscrypt_encrypted_key_reserve_payload(unsigned short decrypted_datalen,
+					  unsigned short *payload_datalen)
+{
+	if (decrypted_datalen < AES_BLOCK_SIZE /* FS_AES_128_CBC_KEY_SIZE */
+	    || decrypted_datalen > FS_MAX_KEY_SIZE
+	    || decrypted_datalen % AES_BLOCK_SIZE != 0) {
+		pr_err("encrypted_key: fscrypt keylen must be a multiple of %d up to %d bytes\n",
+		       AES_BLOCK_SIZE, FS_MAX_KEY_SIZE);
+		return -EINVAL;
+	}
+	*payload_datalen = sizeof(struct fscrypt_key);
+	return 0;
+}
+
+void fscrypt__ekey_init(struct encrypted_key_payload *epayload)
+{
+	struct fscrypt_key *fk = (struct fscrypt_key *)epayload->payload_data;
+
+	epayload->decrypted_data = fk->raw;
+
+	fk->mode = 0;
+	fk->size = epayload->decrypted_datalen;
+}
+
+int fscrypt_valid_desc(const char *desc)
+{
+	int i;
+
+	if (strlen(desc) != (FS_KEY_DESC_PREFIX_SIZE
+			     + FS_KEY_DESCRIPTOR_HEX_SIZE))
+		goto error;
+	if (memcmp(desc, FS_KEY_DESC_PREFIX, FS_KEY_DESC_PREFIX_SIZE))
+		goto error;
+	desc += FS_KEY_DESC_PREFIX_SIZE;
+	for (i = 0; i < FS_KEY_DESCRIPTOR_HEX_SIZE; i++)
+		if (!isxdigit(desc[i]))
+			goto error;
+
+	return 0;
+
+error:
+	pr_err("encrypted_key: key description must be 'fscrypt:<policy>'\n");
+	return -EINVAL;
+}
+
diff --git a/security/keys/encrypted-keys/fscrypt_format.h b/security/keys/encrypted-keys/fscrypt_format.h
new file mode 100644
index 000000000000..c6d7da1a2113
--- /dev/null
+++ b/security/keys/encrypted-keys/fscrypt_format.h
@@ -0,0 +1,20 @@
+/*
+ * fscrypt_format.h: helper functions for the encrypted key type
+ *
+ * Copyright (C) 2006 International Business Machines Corp.
+ * Copyright (C) 2010 Politecnico di Torino, Italy
+ *                    TORSEC group -- http://security.polito.it
+ *
+ * Authors:
+ * André Draszik <git@andred.net>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 2 of the License.
+ */
+#pragma once
+
+int fscrypt_encrypted_key_reserve_payload(unsigned short decrypted_datalen,
+					  unsigned short *payload_datalen);
+void fscrypt__ekey_init(struct encrypted_key_payload *epayload);
+int fscrypt_valid_desc(const char *desc);
-- 
2.15.1

[PATCH 1/3] encrypted-keys: add fscrypt format support

[André Draszik]

This is heavily based on commit 79a73d188726
("encrypted-keys: add ecryptfs format support").

The 'encrypted' key type defines its own payload format which contains a
symmetric key randomly generated that cannot be used directly by the
fscrypt subsystem, because it instead expects an fscrypt_key structure.

This patch introduces the new format 'fscrypt' that allows to store an
fscrypt_key structure inside the encrypted key payload containing
a randomly generated symmetric key, as the same for the format 'default'
and 'ecryptfs'.

More details about the usage of encrypted keys with the fscrypt
subsystem can be found in the file 'Documentation/security/keys/fscrypt.rst'.

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: 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
---
 security/keys/encrypted-keys/Makefile         |  2 +-
 security/keys/encrypted-keys/encrypted.c      | 19 +++++++-
 security/keys/encrypted-keys/fscrypt_format.c | 70 +++++++++++++++++++++++++++
 security/keys/encrypted-keys/fscrypt_format.h | 20 ++++++++
 4 files changed, 108 insertions(+), 3 deletions(-)
 create mode 100644 security/keys/encrypted-keys/fscrypt_format.c
 create mode 100644 security/keys/encrypted-keys/fscrypt_format.h

diff --git a/security/keys/encrypted-keys/Makefile b/security/keys/encrypted-keys/Makefile
index 7a44dce6f69d..586702ce9622 100644
--- a/security/keys/encrypted-keys/Makefile
+++ b/security/keys/encrypted-keys/Makefile
@@ -5,7 +5,7 @@
 
 obj-$(CONFIG_ENCRYPTED_KEYS) += encrypted-keys.o
 
-encrypted-keys-y := encrypted.o ecryptfs_format.o
+encrypted-keys-y := encrypted.o ecryptfs_format.o fscrypt_format.o
 masterkey-$(CONFIG_TRUSTED_KEYS) := masterkey_trusted.o
 masterkey-$(CONFIG_TRUSTED_KEYS)-$(CONFIG_ENCRYPTED_KEYS) := masterkey_trusted.o
 encrypted-keys-y += $(masterkey-y) $(masterkey-m-m)
diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index d92cbf9687c3..b570a930583a 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -37,6 +37,7 @@
 
 #include "encrypted.h"
 #include "ecryptfs_format.h"
+#include "fscrypt_format.h"
 
 static const char KEY_TRUSTED_PREFIX[] = "trusted:";
 static const char KEY_USER_PREFIX[] = "user:";
@@ -45,6 +46,7 @@ static const char hmac_alg[] = "hmac(sha256)";
 static const char blkcipher_alg[] = "cbc(aes)";
 static const char key_format_default[] = "default";
 static const char key_format_ecryptfs[] = "ecryptfs";
+static const char key_format_fscrypt[] = "fscrypt";
 static unsigned int ivsize;
 static int blksize;
 
@@ -62,12 +64,13 @@ enum {
 };
 
 enum {
-	Opt_error = -1, Opt_default, Opt_ecryptfs
+	Opt_error = -1, Opt_default, Opt_ecryptfs, Opt_fscrypt
 };
 
 static const match_table_t key_format_tokens = {
 	{Opt_default, "default"},
 	{Opt_ecryptfs, "ecryptfs"},
+	{Opt_fscrypt, "fscrypt"},
 	{Opt_error, NULL}
 };
 
@@ -185,7 +188,7 @@ static int datablob_parse(char *datablob, const char **format,
 	}
 	key_cmd = match_token(keyword, key_tokens, args);
 
-	/* Get optional format: default | ecryptfs */
+	/* Get optional format: default | ecryptfs | fscrypt */
 	p = strsep(&datablob, " \t");
 	if (!p) {
 		pr_err("encrypted_key: insufficient parameters specified\n");
@@ -194,6 +197,7 @@ static int datablob_parse(char *datablob, const char **format,
 
 	key_format = match_token(p, key_format_tokens, args);
 	switch (key_format) {
+	case Opt_fscrypt:
 	case Opt_ecryptfs:
 	case Opt_default:
 		*format = p;
@@ -634,6 +638,11 @@ static struct encrypted_key_payload *encrypted_key_alloc(struct key *key,
 		}
 		decrypted_datalen = ECRYPTFS_MAX_KEY_BYTES;
 		payload_datalen = sizeof(struct ecryptfs_auth_tok);
+	} else if (format && !strcmp(format, key_format_fscrypt)) {
+		ret = fscrypt_encrypted_key_reserve_payload(decrypted_datalen,
+							    &payload_datalen);
+		if (ret < 0)
+			return ERR_PTR(ret);
 	}
 
 	encrypted_datalen = roundup(decrypted_datalen, blksize);
@@ -734,6 +743,8 @@ static void __ekey_init(struct encrypted_key_payload *epayload,
 		if (!strcmp(format, key_format_ecryptfs))
 			epayload->decrypted_data  				ecryptfs_get_auth_tok_key((struct ecryptfs_auth_tok *)epayload->payload_data);
+		else if (!strcmp(format, key_format_fscrypt))
+			fscrypt__ekey_init(epayload);
 
 		memcpy(epayload->format, format, format_len);
 	}
@@ -762,6 +773,10 @@ static int encrypted_init(struct encrypted_key_payload *epayload,
 
 		ecryptfs_fill_auth_tok((struct ecryptfs_auth_tok *)epayload->payload_data,
 				       key_desc);
+	} else if (format && !strcmp(format, key_format_fscrypt)) {
+		ret = fscrypt_valid_desc(key_desc);
+		if (ret < 0)
+			return ret;
 	}
 
 	__ekey_init(epayload, format, master_desc, datalen);
diff --git a/security/keys/encrypted-keys/fscrypt_format.c b/security/keys/encrypted-keys/fscrypt_format.c
new file mode 100644
index 000000000000..7620c0fa3ff9
--- /dev/null
+++ b/security/keys/encrypted-keys/fscrypt_format.c
@@ -0,0 +1,70 @@
+/*
+ * fscrypt_format.c: helper functions for the encrypted key type
+ *
+ * Copyright (C) 2006 International Business Machines Corp.
+ * Copyright (C) 2010 Politecnico di Torino, Italy
+ *                    TORSEC group -- http://security.polito.it
+ *
+ * Authors:
+ * André Draszik <git@andred.net>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 2 of the License.
+ */
+
+#include <linux/ctype.h>
+#define __FS_HAS_ENCRYPTION 0
+#include <linux/fscrypt.h>
+#include <keys/encrypted-type.h>
+#include <crypto/aes.h>
+#include "fscrypt_format.h"
+
+
+#define FS_KEY_DESCRIPTOR_HEX_SIZE (FS_KEY_DESCRIPTOR_SIZE*2)
+
+int fscrypt_encrypted_key_reserve_payload(unsigned short decrypted_datalen,
+					  unsigned short *payload_datalen)
+{
+	if (decrypted_datalen < AES_BLOCK_SIZE /* FS_AES_128_CBC_KEY_SIZE */
+	    || decrypted_datalen > FS_MAX_KEY_SIZE
+	    || decrypted_datalen % AES_BLOCK_SIZE != 0) {
+		pr_err("encrypted_key: fscrypt keylen must be a multiple of %d up to %d bytes\n",
+		       AES_BLOCK_SIZE, FS_MAX_KEY_SIZE);
+		return -EINVAL;
+	}
+	*payload_datalen = sizeof(struct fscrypt_key);
+	return 0;
+}
+
+void fscrypt__ekey_init(struct encrypted_key_payload *epayload)
+{
+	struct fscrypt_key *fk = (struct fscrypt_key *)epayload->payload_data;
+
+	epayload->decrypted_data = fk->raw;
+
+	fk->mode = 0;
+	fk->size = epayload->decrypted_datalen;
+}
+
+int fscrypt_valid_desc(const char *desc)
+{
+	int i;
+
+	if (strlen(desc) != (FS_KEY_DESC_PREFIX_SIZE
+			     + FS_KEY_DESCRIPTOR_HEX_SIZE))
+		goto error;
+	if (memcmp(desc, FS_KEY_DESC_PREFIX, FS_KEY_DESC_PREFIX_SIZE))
+		goto error;
+	desc += FS_KEY_DESC_PREFIX_SIZE;
+	for (i = 0; i < FS_KEY_DESCRIPTOR_HEX_SIZE; i++)
+		if (!isxdigit(desc[i]))
+			goto error;
+
+	return 0;
+
+error:
+	pr_err("encrypted_key: key description must be 'fscrypt:<policy>'\n");
+	return -EINVAL;
+}
+
diff --git a/security/keys/encrypted-keys/fscrypt_format.h b/security/keys/encrypted-keys/fscrypt_format.h
new file mode 100644
index 000000000000..c6d7da1a2113
--- /dev/null
+++ b/security/keys/encrypted-keys/fscrypt_format.h
@@ -0,0 +1,20 @@
+/*
+ * fscrypt_format.h: helper functions for the encrypted key type
+ *
+ * Copyright (C) 2006 International Business Machines Corp.
+ * Copyright (C) 2010 Politecnico di Torino, Italy
+ *                    TORSEC group -- http://security.polito.it
+ *
+ * Authors:
+ * André Draszik <git@andred.net>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 2 of the License.
+ */
+#pragma once
+
+int fscrypt_encrypted_key_reserve_payload(unsigned short decrypted_datalen,
+					  unsigned short *payload_datalen);
+void fscrypt__ekey_init(struct encrypted_key_payload *epayload);
+int fscrypt_valid_desc(const char *desc);
-- 
2.15.1

[PATCH 1/3] encrypted-keys: add fscrypt format support

[André Draszik]

This is heavily based on commit 79a73d188726
("encrypted-keys: add ecryptfs format support").

The 'encrypted' key type defines its own payload format which contains a
symmetric key randomly generated that cannot be used directly by the
fscrypt subsystem, because it instead expects an fscrypt_key structure.

This patch introduces the new format 'fscrypt' that allows to store an
fscrypt_key structure inside the encrypted key payload containing
a randomly generated symmetric key, as the same for the format 'default'
and 'ecryptfs'.

More details about the usage of encrypted keys with the fscrypt
subsystem can be found in the file 'Documentation/security/keys/fscrypt.rst'.

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: Kees Cook <keescook@chromium.org>
Cc: linux-integrity at vger.kernel.org
Cc: keyrings at vger.kernel.org
Cc: linux-security-module at vger.kernel.org
Cc: linux-fscrypt at vger.kernel.org
---
 security/keys/encrypted-keys/Makefile         |  2 +-
 security/keys/encrypted-keys/encrypted.c      | 19 +++++++-
 security/keys/encrypted-keys/fscrypt_format.c | 70 +++++++++++++++++++++++++++
 security/keys/encrypted-keys/fscrypt_format.h | 20 ++++++++
 4 files changed, 108 insertions(+), 3 deletions(-)
 create mode 100644 security/keys/encrypted-keys/fscrypt_format.c
 create mode 100644 security/keys/encrypted-keys/fscrypt_format.h

diff --git a/security/keys/encrypted-keys/Makefile b/security/keys/encrypted-keys/Makefile
index 7a44dce6f69d..586702ce9622 100644
--- a/security/keys/encrypted-keys/Makefile
+++ b/security/keys/encrypted-keys/Makefile
@@ -5,7 +5,7 @@
 
 obj-$(CONFIG_ENCRYPTED_KEYS) += encrypted-keys.o
 
-encrypted-keys-y := encrypted.o ecryptfs_format.o
+encrypted-keys-y := encrypted.o ecryptfs_format.o fscrypt_format.o
 masterkey-$(CONFIG_TRUSTED_KEYS) := masterkey_trusted.o
 masterkey-$(CONFIG_TRUSTED_KEYS)-$(CONFIG_ENCRYPTED_KEYS) := masterkey_trusted.o
 encrypted-keys-y += $(masterkey-y) $(masterkey-m-m)
diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index d92cbf9687c3..b570a930583a 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -37,6 +37,7 @@
 
 #include "encrypted.h"
 #include "ecryptfs_format.h"
+#include "fscrypt_format.h"
 
 static const char KEY_TRUSTED_PREFIX[] = "trusted:";
 static const char KEY_USER_PREFIX[] = "user:";
@@ -45,6 +46,7 @@ static const char hmac_alg[] = "hmac(sha256)";
 static const char blkcipher_alg[] = "cbc(aes)";
 static const char key_format_default[] = "default";
 static const char key_format_ecryptfs[] = "ecryptfs";
+static const char key_format_fscrypt[] = "fscrypt";
 static unsigned int ivsize;
 static int blksize;
 
@@ -62,12 +64,13 @@ enum {
 };
 
 enum {
-	Opt_error = -1, Opt_default, Opt_ecryptfs
+	Opt_error = -1, Opt_default, Opt_ecryptfs, Opt_fscrypt
 };
 
 static const match_table_t key_format_tokens = {
 	{Opt_default, "default"},
 	{Opt_ecryptfs, "ecryptfs"},
+	{Opt_fscrypt, "fscrypt"},
 	{Opt_error, NULL}
 };
 
@@ -185,7 +188,7 @@ static int datablob_parse(char *datablob, const char **format,
 	}
 	key_cmd = match_token(keyword, key_tokens, args);
 
-	/* Get optional format: default | ecryptfs */
+	/* Get optional format: default | ecryptfs | fscrypt */
 	p = strsep(&datablob, " \t");
 	if (!p) {
 		pr_err("encrypted_key: insufficient parameters specified\n");
@@ -194,6 +197,7 @@ static int datablob_parse(char *datablob, const char **format,
 
 	key_format = match_token(p, key_format_tokens, args);
 	switch (key_format) {
+	case Opt_fscrypt:
 	case Opt_ecryptfs:
 	case Opt_default:
 		*format = p;
@@ -634,6 +638,11 @@ static struct encrypted_key_payload *encrypted_key_alloc(struct key *key,
 		}
 		decrypted_datalen = ECRYPTFS_MAX_KEY_BYTES;
 		payload_datalen = sizeof(struct ecryptfs_auth_tok);
+	} else if (format && !strcmp(format, key_format_fscrypt)) {
+		ret = fscrypt_encrypted_key_reserve_payload(decrypted_datalen,
+							    &payload_datalen);
+		if (ret < 0)
+			return ERR_PTR(ret);
 	}
 
 	encrypted_datalen = roundup(decrypted_datalen, blksize);
@@ -734,6 +743,8 @@ static void __ekey_init(struct encrypted_key_payload *epayload,
 		if (!strcmp(format, key_format_ecryptfs))
 			epayload->decrypted_data =
 				ecryptfs_get_auth_tok_key((struct ecryptfs_auth_tok *)epayload->payload_data);
+		else if (!strcmp(format, key_format_fscrypt))
+			fscrypt__ekey_init(epayload);
 
 		memcpy(epayload->format, format, format_len);
 	}
@@ -762,6 +773,10 @@ static int encrypted_init(struct encrypted_key_payload *epayload,
 
 		ecryptfs_fill_auth_tok((struct ecryptfs_auth_tok *)epayload->payload_data,
 				       key_desc);
+	} else if (format && !strcmp(format, key_format_fscrypt)) {
+		ret = fscrypt_valid_desc(key_desc);
+		if (ret < 0)
+			return ret;
 	}
 
 	__ekey_init(epayload, format, master_desc, datalen);
diff --git a/security/keys/encrypted-keys/fscrypt_format.c b/security/keys/encrypted-keys/fscrypt_format.c
new file mode 100644
index 000000000000..7620c0fa3ff9
--- /dev/null
+++ b/security/keys/encrypted-keys/fscrypt_format.c
@@ -0,0 +1,70 @@
+/*
+ * fscrypt_format.c: helper functions for the encrypted key type
+ *
+ * Copyright (C) 2006 International Business Machines Corp.
+ * Copyright (C) 2010 Politecnico di Torino, Italy
+ *                    TORSEC group -- http://security.polito.it
+ *
+ * Authors:
+ * Andr? Draszik <git@andred.net>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 2 of the License.
+ */
+
+#include <linux/ctype.h>
+#define __FS_HAS_ENCRYPTION 0
+#include <linux/fscrypt.h>
+#include <keys/encrypted-type.h>
+#include <crypto/aes.h>
+#include "fscrypt_format.h"
+
+
+#define FS_KEY_DESCRIPTOR_HEX_SIZE (FS_KEY_DESCRIPTOR_SIZE*2)
+
+int fscrypt_encrypted_key_reserve_payload(unsigned short decrypted_datalen,
+					  unsigned short *payload_datalen)
+{
+	if (decrypted_datalen < AES_BLOCK_SIZE /* FS_AES_128_CBC_KEY_SIZE */
+	    || decrypted_datalen > FS_MAX_KEY_SIZE
+	    || decrypted_datalen % AES_BLOCK_SIZE != 0) {
+		pr_err("encrypted_key: fscrypt keylen must be a multiple of %d up to %d bytes\n",
+		       AES_BLOCK_SIZE, FS_MAX_KEY_SIZE);
+		return -EINVAL;
+	}
+	*payload_datalen = sizeof(struct fscrypt_key);
+	return 0;
+}
+
+void fscrypt__ekey_init(struct encrypted_key_payload *epayload)
+{
+	struct fscrypt_key *fk = (struct fscrypt_key *)epayload->payload_data;
+
+	epayload->decrypted_data = fk->raw;
+
+	fk->mode = 0;
+	fk->size = epayload->decrypted_datalen;
+}
+
+int fscrypt_valid_desc(const char *desc)
+{
+	int i;
+
+	if (strlen(desc) != (FS_KEY_DESC_PREFIX_SIZE
+			     + FS_KEY_DESCRIPTOR_HEX_SIZE))
+		goto error;
+	if (memcmp(desc, FS_KEY_DESC_PREFIX, FS_KEY_DESC_PREFIX_SIZE))
+		goto error;
+	desc += FS_KEY_DESC_PREFIX_SIZE;
+	for (i = 0; i < FS_KEY_DESCRIPTOR_HEX_SIZE; i++)
+		if (!isxdigit(desc[i]))
+			goto error;
+
+	return 0;
+
+error:
+	pr_err("encrypted_key: key description must be 'fscrypt:<policy>'\n");
+	return -EINVAL;
+}
+
diff --git a/security/keys/encrypted-keys/fscrypt_format.h b/security/keys/encrypted-keys/fscrypt_format.h
new file mode 100644
index 000000000000..c6d7da1a2113
--- /dev/null
+++ b/security/keys/encrypted-keys/fscrypt_format.h
@@ -0,0 +1,20 @@
+/*
+ * fscrypt_format.h: helper functions for the encrypted key type
+ *
+ * Copyright (C) 2006 International Business Machines Corp.
+ * Copyright (C) 2010 Politecnico di Torino, Italy
+ *                    TORSEC group -- http://security.polito.it
+ *
+ * Authors:
+ * Andr? Draszik <git@andred.net>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 2 of the License.
+ */
+#pragma once
+
+int fscrypt_encrypted_key_reserve_payload(unsigned short decrypted_datalen,
+					  unsigned short *payload_datalen);
+void fscrypt__ekey_init(struct encrypted_key_payload *epayload);
+int fscrypt_valid_desc(const char *desc);
-- 
2.15.1

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

[PATCH 1/3] encrypted-keys: add fscrypt format support

[André Draszik]

This is heavily based on commit 79a73d188726
("encrypted-keys: add ecryptfs format support").

The 'encrypted' key type defines its own payload format which contains a
symmetric key randomly generated that cannot be used directly by the
fscrypt subsystem, because it instead expects an fscrypt_key structure.

This patch introduces the new format 'fscrypt' that allows to store an
fscrypt_key structure inside the encrypted key payload containing
a randomly generated symmetric key, as the same for the format 'default'
and 'ecryptfs'.

More details about the usage of encrypted keys with the fscrypt
subsystem can be found in the file 'Documentation/security/keys/fscrypt.rst'.

Signed-off-by: Andre 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: 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
---
 security/keys/encrypted-keys/Makefile         |  2 +-
 security/keys/encrypted-keys/encrypted.c      | 19 +++++++-
 security/keys/encrypted-keys/fscrypt_format.c | 70 +++++++++++++++++++++++++++
 security/keys/encrypted-keys/fscrypt_format.h | 20 ++++++++
 4 files changed, 108 insertions(+), 3 deletions(-)
 create mode 100644 security/keys/encrypted-keys/fscrypt_format.c
 create mode 100644 security/keys/encrypted-keys/fscrypt_format.h

diff --git a/security/keys/encrypted-keys/Makefile b/security/keys/encrypted-keys/Makefile
index 7a44dce6f69d..586702ce9622 100644
--- a/security/keys/encrypted-keys/Makefile
+++ b/security/keys/encrypted-keys/Makefile
@@ -5,7 +5,7 @@
 
 obj-$(CONFIG_ENCRYPTED_KEYS) += encrypted-keys.o
 
-encrypted-keys-y := encrypted.o ecryptfs_format.o
+encrypted-keys-y := encrypted.o ecryptfs_format.o fscrypt_format.o
 masterkey-$(CONFIG_TRUSTED_KEYS) := masterkey_trusted.o
 masterkey-$(CONFIG_TRUSTED_KEYS)-$(CONFIG_ENCRYPTED_KEYS) := masterkey_trusted.o
 encrypted-keys-y += $(masterkey-y) $(masterkey-m-m)
diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index d92cbf9687c3..b570a930583a 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -37,6 +37,7 @@
 
 #include "encrypted.h"
 #include "ecryptfs_format.h"
+#include "fscrypt_format.h"
 
 static const char KEY_TRUSTED_PREFIX[] = "trusted:";
 static const char KEY_USER_PREFIX[] = "user:";
@@ -45,6 +46,7 @@ static const char hmac_alg[] = "hmac(sha256)";
 static const char blkcipher_alg[] = "cbc(aes)";
 static const char key_format_default[] = "default";
 static const char key_format_ecryptfs[] = "ecryptfs";
+static const char key_format_fscrypt[] = "fscrypt";
 static unsigned int ivsize;
 static int blksize;
 
@@ -62,12 +64,13 @@ enum {
 };
 
 enum {
-	Opt_error = -1, Opt_default, Opt_ecryptfs
+	Opt_error = -1, Opt_default, Opt_ecryptfs, Opt_fscrypt
 };
 
 static const match_table_t key_format_tokens = {
 	{Opt_default, "default"},
 	{Opt_ecryptfs, "ecryptfs"},
+	{Opt_fscrypt, "fscrypt"},
 	{Opt_error, NULL}
 };
 
@@ -185,7 +188,7 @@ static int datablob_parse(char *datablob, const char **format,
 	}
 	key_cmd = match_token(keyword, key_tokens, args);
 
-	/* Get optional format: default | ecryptfs */
+	/* Get optional format: default | ecryptfs | fscrypt */
 	p = strsep(&datablob, " \t");
 	if (!p) {
 		pr_err("encrypted_key: insufficient parameters specified\n");
@@ -194,6 +197,7 @@ static int datablob_parse(char *datablob, const char **format,
 
 	key_format = match_token(p, key_format_tokens, args);
 	switch (key_format) {
+	case Opt_fscrypt:
 	case Opt_ecryptfs:
 	case Opt_default:
 		*format = p;
@@ -634,6 +638,11 @@ static struct encrypted_key_payload *encrypted_key_alloc(struct key *key,
 		}
 		decrypted_datalen = ECRYPTFS_MAX_KEY_BYTES;
 		payload_datalen = sizeof(struct ecryptfs_auth_tok);
+	} else if (format && !strcmp(format, key_format_fscrypt)) {
+		ret = fscrypt_encrypted_key_reserve_payload(decrypted_datalen,
+							    &payload_datalen);
+		if (ret < 0)
+			return ERR_PTR(ret);
 	}
 
 	encrypted_datalen = roundup(decrypted_datalen, blksize);
@@ -734,6 +743,8 @@ static void __ekey_init(struct encrypted_key_payload *epayload,
 		if (!strcmp(format, key_format_ecryptfs))
 			epayload->decrypted_data =
 				ecryptfs_get_auth_tok_key((struct ecryptfs_auth_tok *)epayload->payload_data);
+		else if (!strcmp(format, key_format_fscrypt))
+			fscrypt__ekey_init(epayload);
 
 		memcpy(epayload->format, format, format_len);
 	}
@@ -762,6 +773,10 @@ static int encrypted_init(struct encrypted_key_payload *epayload,
 
 		ecryptfs_fill_auth_tok((struct ecryptfs_auth_tok *)epayload->payload_data,
 				       key_desc);
+	} else if (format && !strcmp(format, key_format_fscrypt)) {
+		ret = fscrypt_valid_desc(key_desc);
+		if (ret < 0)
+			return ret;
 	}
 
 	__ekey_init(epayload, format, master_desc, datalen);
diff --git a/security/keys/encrypted-keys/fscrypt_format.c b/security/keys/encrypted-keys/fscrypt_format.c
new file mode 100644
index 000000000000..7620c0fa3ff9
--- /dev/null
+++ b/security/keys/encrypted-keys/fscrypt_format.c
@@ -0,0 +1,70 @@
+/*
+ * fscrypt_format.c: helper functions for the encrypted key type
+ *
+ * Copyright (C) 2006 International Business Machines Corp.
+ * Copyright (C) 2010 Politecnico di Torino, Italy
+ *                    TORSEC group -- http://security.polito.it
+ *
+ * Authors:
+ * Andre Draszik <git@andred.net>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 2 of the License.
+ */
+
+#include <linux/ctype.h>
+#define __FS_HAS_ENCRYPTION 0
+#include <linux/fscrypt.h>
+#include <keys/encrypted-type.h>
+#include <crypto/aes.h>
+#include "fscrypt_format.h"
+
+
+#define FS_KEY_DESCRIPTOR_HEX_SIZE (FS_KEY_DESCRIPTOR_SIZE*2)
+
+int fscrypt_encrypted_key_reserve_payload(unsigned short decrypted_datalen,
+					  unsigned short *payload_datalen)
+{
+	if (decrypted_datalen < AES_BLOCK_SIZE /* FS_AES_128_CBC_KEY_SIZE */
+	    || decrypted_datalen > FS_MAX_KEY_SIZE
+	    || decrypted_datalen % AES_BLOCK_SIZE != 0) {
+		pr_err("encrypted_key: fscrypt keylen must be a multiple of %d up to %d bytes\n",
+		       AES_BLOCK_SIZE, FS_MAX_KEY_SIZE);
+		return -EINVAL;
+	}
+	*payload_datalen = sizeof(struct fscrypt_key);
+	return 0;
+}
+
+void fscrypt__ekey_init(struct encrypted_key_payload *epayload)
+{
+	struct fscrypt_key *fk = (struct fscrypt_key *)epayload->payload_data;
+
+	epayload->decrypted_data = fk->raw;
+
+	fk->mode = 0;
+	fk->size = epayload->decrypted_datalen;
+}
+
+int fscrypt_valid_desc(const char *desc)
+{
+	int i;
+
+	if (strlen(desc) != (FS_KEY_DESC_PREFIX_SIZE
+			     + FS_KEY_DESCRIPTOR_HEX_SIZE))
+		goto error;
+	if (memcmp(desc, FS_KEY_DESC_PREFIX, FS_KEY_DESC_PREFIX_SIZE))
+		goto error;
+	desc += FS_KEY_DESC_PREFIX_SIZE;
+	for (i = 0; i < FS_KEY_DESCRIPTOR_HEX_SIZE; i++)
+		if (!isxdigit(desc[i]))
+			goto error;
+
+	return 0;
+
+error:
+	pr_err("encrypted_key: key description must be 'fscrypt:<policy>'\n");
+	return -EINVAL;
+}
+
diff --git a/security/keys/encrypted-keys/fscrypt_format.h b/security/keys/encrypted-keys/fscrypt_format.h
new file mode 100644
index 000000000000..c6d7da1a2113
--- /dev/null
+++ b/security/keys/encrypted-keys/fscrypt_format.h
@@ -0,0 +1,20 @@
+/*
+ * fscrypt_format.h: helper functions for the encrypted key type
+ *
+ * Copyright (C) 2006 International Business Machines Corp.
+ * Copyright (C) 2010 Politecnico di Torino, Italy
+ *                    TORSEC group -- http://security.polito.it
+ *
+ * Authors:
+ * Andre Draszik <git@andred.net>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 2 of the License.
+ */
+#pragma once
+
+int fscrypt_encrypted_key_reserve_payload(unsigned short decrypted_datalen,
+					  unsigned short *payload_datalen);
+void fscrypt__ekey_init(struct encrypted_key_payload *epayload);
+int fscrypt_valid_desc(const char *desc);
-- 
2.15.1

[PATCH 2/3] fscrypt: add support for the encrypted key type

[André Draszik]

We now try to acquire the key according to the
encryption policy from both key types, 'logon'
as well as 'encrypted'.

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: 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
---
 fs/crypto/keyinfo.c | 58 +++++++++++++++++++++++++++++++++++------------------
 1 file changed, 38 insertions(+), 20 deletions(-)

diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 5e6e846f5a24..023fa19fec48 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -10,6 +10,7 @@
  */
 
 #include <keys/user-type.h>
+#include <keys/encrypted-type.h>
 #include <linux/scatterlist.h>
 #include <linux/ratelimit.h>
 #include <crypto/aes.h>
@@ -66,14 +67,20 @@ static int derive_key_aes(u8 deriving_key[FS_AES_128_ECB_KEY_SIZE],
 	return res;
 }
 
-static int validate_user_key(struct fscrypt_info *crypt_info,
+static inline struct key *fscrypt_get_encrypted_key(const char *sig)
+{
+	if (IS_ENABLED(CONFIG_ENCRYPTED_KEYS))
+		return request_key(&key_type_encrypted, sig, NULL);
+	return ERR_PTR(-ENOKEY);
+}
+
+static int validate_keyring_key(struct fscrypt_info *crypt_info,
 			struct fscrypt_context *ctx, u8 *raw_key,
 			const char *prefix, int min_keysize)
 {
 	char *description;
 	struct key *keyring_key;
 	struct fscrypt_key *master_key;
-	const struct user_key_payload *ukp;
 	int res;
 
 	description = kasprintf(GFP_NOFS, "%s%*phN", prefix,
@@ -83,28 +90,39 @@ static int validate_user_key(struct fscrypt_info *crypt_info,
 		return -ENOMEM;
 
 	keyring_key = request_key(&key_type_logon, description, NULL);
+	if (IS_ERR(keyring_key))
+		keyring_key = fscrypt_get_encrypted_key(description);
 	kfree(description);
 	if (IS_ERR(keyring_key))
 		return PTR_ERR(keyring_key);
 	down_read(&keyring_key->sem);
 
-	if (keyring_key->type != &key_type_logon) {
+	if (keyring_key->type == &key_type_logon) {
+		const struct user_key_payload *ukp;
+
+		ukp = user_key_payload_locked(keyring_key);
+		if (!ukp) {
+			/* key was revoked before we acquired its semaphore */
+			res = -EKEYREVOKED;
+			goto out;
+		}
+		if (ukp->datalen != sizeof(struct fscrypt_key)) {
+			res = -EINVAL;
+			goto out;
+		}
+		master_key = (struct fscrypt_key *)ukp->data;
+	} else if (keyring_key->type == &key_type_encrypted) {
+		const struct encrypted_key_payload *ekp;
+
+		ekp = keyring_key->payload.data[0];
+		master_key = (struct fscrypt_key *)ekp->payload_data;
+	} else {
 		printk_once(KERN_WARNING
-				"%s: key type must be logon\n", __func__);
+				"%s: key type must be logon or encrypted\n",
+				__func__);
 		res = -ENOKEY;
 		goto out;
 	}
-	ukp = user_key_payload_locked(keyring_key);
-	if (!ukp) {
-		/* key was revoked before we acquired its semaphore */
-		res = -EKEYREVOKED;
-		goto out;
-	}
-	if (ukp->datalen != sizeof(struct fscrypt_key)) {
-		res = -EINVAL;
-		goto out;
-	}
-	master_key = (struct fscrypt_key *)ukp->data;
 	BUILD_BUG_ON(FS_AES_128_ECB_KEY_SIZE != FS_KEY_DERIVATION_NONCE_SIZE);
 
 	if (master_key->size < min_keysize || master_key->size > FS_MAX_KEY_SIZE
@@ -302,12 +320,12 @@ int fscrypt_get_encryption_info(struct inode *inode)
 	if (!raw_key)
 		goto out;
 
-	res = validate_user_key(crypt_info, &ctx, raw_key, FS_KEY_DESC_PREFIX,
-				keysize);
+	res = validate_keyring_key(crypt_info, &ctx, raw_key,
+				   FS_KEY_DESC_PREFIX, keysize);
 	if (res && inode->i_sb->s_cop->key_prefix) {
-		int res2 = validate_user_key(crypt_info, &ctx, raw_key,
-					     inode->i_sb->s_cop->key_prefix,
-					     keysize);
+		int res2 = validate_keyring_key(crypt_info, &ctx, raw_key,
+						inode->i_sb->s_cop->key_prefix,
+						keysize);
 		if (res2) {
 			if (res2 == -ENOKEY)
 				res = -ENOKEY;
-- 
2.15.1

[PATCH 2/3] fscrypt: add support for the encrypted key type

[André Draszik]

We now try to acquire the key according to the
encryption policy from both key types, 'logon'
as well as 'encrypted'.

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: 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
---
 fs/crypto/keyinfo.c | 58 +++++++++++++++++++++++++++++++++++------------------
 1 file changed, 38 insertions(+), 20 deletions(-)

diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 5e6e846f5a24..023fa19fec48 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -10,6 +10,7 @@
  */
 
 #include <keys/user-type.h>
+#include <keys/encrypted-type.h>
 #include <linux/scatterlist.h>
 #include <linux/ratelimit.h>
 #include <crypto/aes.h>
@@ -66,14 +67,20 @@ static int derive_key_aes(u8 deriving_key[FS_AES_128_ECB_KEY_SIZE],
 	return res;
 }
 
-static int validate_user_key(struct fscrypt_info *crypt_info,
+static inline struct key *fscrypt_get_encrypted_key(const char *sig)
+{
+	if (IS_ENABLED(CONFIG_ENCRYPTED_KEYS))
+		return request_key(&key_type_encrypted, sig, NULL);
+	return ERR_PTR(-ENOKEY);
+}
+
+static int validate_keyring_key(struct fscrypt_info *crypt_info,
 			struct fscrypt_context *ctx, u8 *raw_key,
 			const char *prefix, int min_keysize)
 {
 	char *description;
 	struct key *keyring_key;
 	struct fscrypt_key *master_key;
-	const struct user_key_payload *ukp;
 	int res;
 
 	description = kasprintf(GFP_NOFS, "%s%*phN", prefix,
@@ -83,28 +90,39 @@ static int validate_user_key(struct fscrypt_info *crypt_info,
 		return -ENOMEM;
 
 	keyring_key = request_key(&key_type_logon, description, NULL);
+	if (IS_ERR(keyring_key))
+		keyring_key = fscrypt_get_encrypted_key(description);
 	kfree(description);
 	if (IS_ERR(keyring_key))
 		return PTR_ERR(keyring_key);
 	down_read(&keyring_key->sem);
 
-	if (keyring_key->type != &key_type_logon) {
+	if (keyring_key->type = &key_type_logon) {
+		const struct user_key_payload *ukp;
+
+		ukp = user_key_payload_locked(keyring_key);
+		if (!ukp) {
+			/* key was revoked before we acquired its semaphore */
+			res = -EKEYREVOKED;
+			goto out;
+		}
+		if (ukp->datalen != sizeof(struct fscrypt_key)) {
+			res = -EINVAL;
+			goto out;
+		}
+		master_key = (struct fscrypt_key *)ukp->data;
+	} else if (keyring_key->type = &key_type_encrypted) {
+		const struct encrypted_key_payload *ekp;
+
+		ekp = keyring_key->payload.data[0];
+		master_key = (struct fscrypt_key *)ekp->payload_data;
+	} else {
 		printk_once(KERN_WARNING
-				"%s: key type must be logon\n", __func__);
+				"%s: key type must be logon or encrypted\n",
+				__func__);
 		res = -ENOKEY;
 		goto out;
 	}
-	ukp = user_key_payload_locked(keyring_key);
-	if (!ukp) {
-		/* key was revoked before we acquired its semaphore */
-		res = -EKEYREVOKED;
-		goto out;
-	}
-	if (ukp->datalen != sizeof(struct fscrypt_key)) {
-		res = -EINVAL;
-		goto out;
-	}
-	master_key = (struct fscrypt_key *)ukp->data;
 	BUILD_BUG_ON(FS_AES_128_ECB_KEY_SIZE != FS_KEY_DERIVATION_NONCE_SIZE);
 
 	if (master_key->size < min_keysize || master_key->size > FS_MAX_KEY_SIZE
@@ -302,12 +320,12 @@ int fscrypt_get_encryption_info(struct inode *inode)
 	if (!raw_key)
 		goto out;
 
-	res = validate_user_key(crypt_info, &ctx, raw_key, FS_KEY_DESC_PREFIX,
-				keysize);
+	res = validate_keyring_key(crypt_info, &ctx, raw_key,
+				   FS_KEY_DESC_PREFIX, keysize);
 	if (res && inode->i_sb->s_cop->key_prefix) {
-		int res2 = validate_user_key(crypt_info, &ctx, raw_key,
-					     inode->i_sb->s_cop->key_prefix,
-					     keysize);
+		int res2 = validate_keyring_key(crypt_info, &ctx, raw_key,
+						inode->i_sb->s_cop->key_prefix,
+						keysize);
 		if (res2) {
 			if (res2 = -ENOKEY)
 				res = -ENOKEY;
-- 
2.15.1

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

[PATCH 2/3] fscrypt: add support for the encrypted key type

[André Draszik]

We now try to acquire the key according to the
encryption policy from both key types, 'logon'
as well as 'encrypted'.

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: Kees Cook <keescook@chromium.org>
Cc: linux-integrity at vger.kernel.org
Cc: keyrings at vger.kernel.org
Cc: linux-security-module at vger.kernel.org
Cc: linux-fscrypt at vger.kernel.org
---
 fs/crypto/keyinfo.c | 58 +++++++++++++++++++++++++++++++++++------------------
 1 file changed, 38 insertions(+), 20 deletions(-)

diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 5e6e846f5a24..023fa19fec48 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -10,6 +10,7 @@
  */
 
 #include <keys/user-type.h>
+#include <keys/encrypted-type.h>
 #include <linux/scatterlist.h>
 #include <linux/ratelimit.h>
 #include <crypto/aes.h>
@@ -66,14 +67,20 @@ static int derive_key_aes(u8 deriving_key[FS_AES_128_ECB_KEY_SIZE],
 	return res;
 }
 
-static int validate_user_key(struct fscrypt_info *crypt_info,
+static inline struct key *fscrypt_get_encrypted_key(const char *sig)
+{
+	if (IS_ENABLED(CONFIG_ENCRYPTED_KEYS))
+		return request_key(&key_type_encrypted, sig, NULL);
+	return ERR_PTR(-ENOKEY);
+}
+
+static int validate_keyring_key(struct fscrypt_info *crypt_info,
 			struct fscrypt_context *ctx, u8 *raw_key,
 			const char *prefix, int min_keysize)
 {
 	char *description;
 	struct key *keyring_key;
 	struct fscrypt_key *master_key;
-	const struct user_key_payload *ukp;
 	int res;
 
 	description = kasprintf(GFP_NOFS, "%s%*phN", prefix,
@@ -83,28 +90,39 @@ static int validate_user_key(struct fscrypt_info *crypt_info,
 		return -ENOMEM;
 
 	keyring_key = request_key(&key_type_logon, description, NULL);
+	if (IS_ERR(keyring_key))
+		keyring_key = fscrypt_get_encrypted_key(description);
 	kfree(description);
 	if (IS_ERR(keyring_key))
 		return PTR_ERR(keyring_key);
 	down_read(&keyring_key->sem);
 
-	if (keyring_key->type != &key_type_logon) {
+	if (keyring_key->type == &key_type_logon) {
+		const struct user_key_payload *ukp;
+
+		ukp = user_key_payload_locked(keyring_key);
+		if (!ukp) {
+			/* key was revoked before we acquired its semaphore */
+			res = -EKEYREVOKED;
+			goto out;
+		}
+		if (ukp->datalen != sizeof(struct fscrypt_key)) {
+			res = -EINVAL;
+			goto out;
+		}
+		master_key = (struct fscrypt_key *)ukp->data;
+	} else if (keyring_key->type == &key_type_encrypted) {
+		const struct encrypted_key_payload *ekp;
+
+		ekp = keyring_key->payload.data[0];
+		master_key = (struct fscrypt_key *)ekp->payload_data;
+	} else {
 		printk_once(KERN_WARNING
-				"%s: key type must be logon\n", __func__);
+				"%s: key type must be logon or encrypted\n",
+				__func__);
 		res = -ENOKEY;
 		goto out;
 	}
-	ukp = user_key_payload_locked(keyring_key);
-	if (!ukp) {
-		/* key was revoked before we acquired its semaphore */
-		res = -EKEYREVOKED;
-		goto out;
-	}
-	if (ukp->datalen != sizeof(struct fscrypt_key)) {
-		res = -EINVAL;
-		goto out;
-	}
-	master_key = (struct fscrypt_key *)ukp->data;
 	BUILD_BUG_ON(FS_AES_128_ECB_KEY_SIZE != FS_KEY_DERIVATION_NONCE_SIZE);
 
 	if (master_key->size < min_keysize || master_key->size > FS_MAX_KEY_SIZE
@@ -302,12 +320,12 @@ int fscrypt_get_encryption_info(struct inode *inode)
 	if (!raw_key)
 		goto out;
 
-	res = validate_user_key(crypt_info, &ctx, raw_key, FS_KEY_DESC_PREFIX,
-				keysize);
+	res = validate_keyring_key(crypt_info, &ctx, raw_key,
+				   FS_KEY_DESC_PREFIX, keysize);
 	if (res && inode->i_sb->s_cop->key_prefix) {
-		int res2 = validate_user_key(crypt_info, &ctx, raw_key,
-					     inode->i_sb->s_cop->key_prefix,
-					     keysize);
+		int res2 = validate_keyring_key(crypt_info, &ctx, raw_key,
+						inode->i_sb->s_cop->key_prefix,
+						keysize);
 		if (res2) {
 			if (res2 == -ENOKEY)
 				res = -ENOKEY;
-- 
2.15.1

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

[PATCH 2/3] fscrypt: add support for the encrypted key type

[André Draszik]

We now try to acquire the key according to the
encryption policy from both key types, 'logon'
as well as 'encrypted'.

Signed-off-by: Andre 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: 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
---
 fs/crypto/keyinfo.c | 58 +++++++++++++++++++++++++++++++++++------------------
 1 file changed, 38 insertions(+), 20 deletions(-)

diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 5e6e846f5a24..023fa19fec48 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -10,6 +10,7 @@
  */
 
 #include <keys/user-type.h>
+#include <keys/encrypted-type.h>
 #include <linux/scatterlist.h>
 #include <linux/ratelimit.h>
 #include <crypto/aes.h>
@@ -66,14 +67,20 @@ static int derive_key_aes(u8 deriving_key[FS_AES_128_ECB_KEY_SIZE],
 	return res;
 }
 
-static int validate_user_key(struct fscrypt_info *crypt_info,
+static inline struct key *fscrypt_get_encrypted_key(const char *sig)
+{
+	if (IS_ENABLED(CONFIG_ENCRYPTED_KEYS))
+		return request_key(&key_type_encrypted, sig, NULL);
+	return ERR_PTR(-ENOKEY);
+}
+
+static int validate_keyring_key(struct fscrypt_info *crypt_info,
 			struct fscrypt_context *ctx, u8 *raw_key,
 			const char *prefix, int min_keysize)
 {
 	char *description;
 	struct key *keyring_key;
 	struct fscrypt_key *master_key;
-	const struct user_key_payload *ukp;
 	int res;
 
 	description = kasprintf(GFP_NOFS, "%s%*phN", prefix,
@@ -83,28 +90,39 @@ static int validate_user_key(struct fscrypt_info *crypt_info,
 		return -ENOMEM;
 
 	keyring_key = request_key(&key_type_logon, description, NULL);
+	if (IS_ERR(keyring_key))
+		keyring_key = fscrypt_get_encrypted_key(description);
 	kfree(description);
 	if (IS_ERR(keyring_key))
 		return PTR_ERR(keyring_key);
 	down_read(&keyring_key->sem);
 
-	if (keyring_key->type != &key_type_logon) {
+	if (keyring_key->type == &key_type_logon) {
+		const struct user_key_payload *ukp;
+
+		ukp = user_key_payload_locked(keyring_key);
+		if (!ukp) {
+			/* key was revoked before we acquired its semaphore */
+			res = -EKEYREVOKED;
+			goto out;
+		}
+		if (ukp->datalen != sizeof(struct fscrypt_key)) {
+			res = -EINVAL;
+			goto out;
+		}
+		master_key = (struct fscrypt_key *)ukp->data;
+	} else if (keyring_key->type == &key_type_encrypted) {
+		const struct encrypted_key_payload *ekp;
+
+		ekp = keyring_key->payload.data[0];
+		master_key = (struct fscrypt_key *)ekp->payload_data;
+	} else {
 		printk_once(KERN_WARNING
-				"%s: key type must be logon\n", __func__);
+				"%s: key type must be logon or encrypted\n",
+				__func__);
 		res = -ENOKEY;
 		goto out;
 	}
-	ukp = user_key_payload_locked(keyring_key);
-	if (!ukp) {
-		/* key was revoked before we acquired its semaphore */
-		res = -EKEYREVOKED;
-		goto out;
-	}
-	if (ukp->datalen != sizeof(struct fscrypt_key)) {
-		res = -EINVAL;
-		goto out;
-	}
-	master_key = (struct fscrypt_key *)ukp->data;
 	BUILD_BUG_ON(FS_AES_128_ECB_KEY_SIZE != FS_KEY_DERIVATION_NONCE_SIZE);
 
 	if (master_key->size < min_keysize || master_key->size > FS_MAX_KEY_SIZE
@@ -302,12 +320,12 @@ int fscrypt_get_encryption_info(struct inode *inode)
 	if (!raw_key)
 		goto out;
 
-	res = validate_user_key(crypt_info, &ctx, raw_key, FS_KEY_DESC_PREFIX,
-				keysize);
+	res = validate_keyring_key(crypt_info, &ctx, raw_key,
+				   FS_KEY_DESC_PREFIX, keysize);
 	if (res && inode->i_sb->s_cop->key_prefix) {
-		int res2 = validate_user_key(crypt_info, &ctx, raw_key,
-					     inode->i_sb->s_cop->key_prefix,
-					     keysize);
+		int res2 = validate_keyring_key(crypt_info, &ctx, raw_key,
+						inode->i_sb->s_cop->key_prefix,
+						keysize);
 		if (res2) {
 			if (res2 == -ENOKEY)
 				res = -ENOKEY;
-- 
2.15.1

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

[André Draszik]

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

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``.
-- 
2.15.1

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

[André Draszik]

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

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``.
-- 
2.15.1

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

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

[André Draszik]

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 at vger.kernel.org
Cc: keyrings at vger.kernel.org
Cc: linux-security-module at vger.kernel.org
Cc: linux-fscrypt at vger.kernel.org
Cc: linux-doc at 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

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``.
-- 
2.15.1

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

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

[André Draszik]

Signed-off-by: Andre 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

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``.
-- 
2.15.1

Re: [PATCH 1/3] encrypted-keys: add fscrypt format support

[Eric Biggers]

Hi Andr�,

On Wed, Jan 10, 2018 at 12:44:16PM +0000, Andr� Draszik wrote:
> This is heavily based on commit 79a73d188726
> ("encrypted-keys: add ecryptfs format support").
> 
> The 'encrypted' key type defines its own payload format which contains a
> symmetric key randomly generated that cannot be used directly by the
> fscrypt subsystem, because it instead expects an fscrypt_key structure.
> 
> This patch introduces the new format 'fscrypt' that allows to store an
> fscrypt_key structure inside the encrypted key payload containing
> a randomly generated symmetric key, as the same for the format 'default'
> and 'ecryptfs'.
> 
> More details about the usage of encrypted keys with the fscrypt
> subsystem can be found in the file 'Documentation/security/keys/fscrypt.rst'.
> 

I don't think a new encrypted-key format is needed.  fscrypt really only needs
the raw key.  The fact that fscrypt uses 'struct fscrypt_key' for the key
payloads is a mistake, given that a raw byte array would work just as well.  In
particular, the 'size' field is redundant, since a 'struct key' knows the size
of its payload; and the 'mode' field is meaningless and therefore is ignored.
Also since there are no reserved fields the only way we would ever be able to
add anything new to 'struct fscrypt_key' is by doing a hack where we put an
invalid value in the 'size' field, which would be ugly.

Also I have proposed an fscrypt ioctl to add keys to a filesystem-level keyring,
and it doesn't use 'struct fscrypt_key' at all:
https://marc.info/?l=linux-fsdevel&m=150879505206393

So I think you should just use the "default" encrypted-key format, where the
payload is just the raw key.  fscrypt can very easily be updated to work with
such keys.

Eric

Re: [PATCH 1/3] encrypted-keys: add fscrypt format support

[Eric Biggers]

Hi André,

On Wed, Jan 10, 2018 at 12:44:16PM +0000, André Draszik wrote:
> This is heavily based on commit 79a73d188726
> ("encrypted-keys: add ecryptfs format support").
> 
> The 'encrypted' key type defines its own payload format which contains a
> symmetric key randomly generated that cannot be used directly by the
> fscrypt subsystem, because it instead expects an fscrypt_key structure.
> 
> This patch introduces the new format 'fscrypt' that allows to store an
> fscrypt_key structure inside the encrypted key payload containing
> a randomly generated symmetric key, as the same for the format 'default'
> and 'ecryptfs'.
> 
> More details about the usage of encrypted keys with the fscrypt
> subsystem can be found in the file 'Documentation/security/keys/fscrypt.rst'.
> 

I don't think a new encrypted-key format is needed.  fscrypt really only needs
the raw key.  The fact that fscrypt uses 'struct fscrypt_key' for the key
payloads is a mistake, given that a raw byte array would work just as well.  In
particular, the 'size' field is redundant, since a 'struct key' knows the size
of its payload; and the 'mode' field is meaningless and therefore is ignored.
Also since there are no reserved fields the only way we would ever be able to
add anything new to 'struct fscrypt_key' is by doing a hack where we put an
invalid value in the 'size' field, which would be ugly.

Also I have proposed an fscrypt ioctl to add keys to a filesystem-level keyring,
and it doesn't use 'struct fscrypt_key' at all:
https://marc.info/?l=linux-fsdevel&m\x150879505206393

So I think you should just use the "default" encrypted-key format, where the
payload is just the raw key.  fscrypt can very easily be updated to work with
such keys.

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

Re: [PATCH 1/3] encrypted-keys: add fscrypt format support

[Eric Biggers]

Hi André,

On Wed, Jan 10, 2018 at 12:44:16PM +0000, André Draszik wrote:
> This is heavily based on commit 79a73d188726
> ("encrypted-keys: add ecryptfs format support").
> 
> The 'encrypted' key type defines its own payload format which contains a
> symmetric key randomly generated that cannot be used directly by the
> fscrypt subsystem, because it instead expects an fscrypt_key structure.
> 
> This patch introduces the new format 'fscrypt' that allows to store an
> fscrypt_key structure inside the encrypted key payload containing
> a randomly generated symmetric key, as the same for the format 'default'
> and 'ecryptfs'.
> 
> More details about the usage of encrypted keys with the fscrypt
> subsystem can be found in the file 'Documentation/security/keys/fscrypt.rst'.
> 

I don't think a new encrypted-key format is needed.  fscrypt really only needs
the raw key.  The fact that fscrypt uses 'struct fscrypt_key' for the key
payloads is a mistake, given that a raw byte array would work just as well.  In
particular, the 'size' field is redundant, since a 'struct key' knows the size
of its payload; and the 'mode' field is meaningless and therefore is ignored.
Also since there are no reserved fields the only way we would ever be able to
add anything new to 'struct fscrypt_key' is by doing a hack where we put an
invalid value in the 'size' field, which would be ugly.

Also I have proposed an fscrypt ioctl to add keys to a filesystem-level keyring,
and it doesn't use 'struct fscrypt_key' at all:
https://marc.info/?l=linux-fsdevel&m=150879505206393

So I think you should just use the "default" encrypted-key format, where the
payload is just the raw key.  fscrypt can very easily be updated to work with
such keys.

Eric

[PATCH 1/3] encrypted-keys: add fscrypt format support

[Eric Biggers]

Hi Andr?,

On Wed, Jan 10, 2018 at 12:44:16PM +0000, Andr? Draszik wrote:
> This is heavily based on commit 79a73d188726
> ("encrypted-keys: add ecryptfs format support").
> 
> The 'encrypted' key type defines its own payload format which contains a
> symmetric key randomly generated that cannot be used directly by the
> fscrypt subsystem, because it instead expects an fscrypt_key structure.
> 
> This patch introduces the new format 'fscrypt' that allows to store an
> fscrypt_key structure inside the encrypted key payload containing
> a randomly generated symmetric key, as the same for the format 'default'
> and 'ecryptfs'.
> 
> More details about the usage of encrypted keys with the fscrypt
> subsystem can be found in the file 'Documentation/security/keys/fscrypt.rst'.
> 

I don't think a new encrypted-key format is needed.  fscrypt really only needs
the raw key.  The fact that fscrypt uses 'struct fscrypt_key' for the key
payloads is a mistake, given that a raw byte array would work just as well.  In
particular, the 'size' field is redundant, since a 'struct key' knows the size
of its payload; and the 'mode' field is meaningless and therefore is ignored.
Also since there are no reserved fields the only way we would ever be able to
add anything new to 'struct fscrypt_key' is by doing a hack where we put an
invalid value in the 'size' field, which would be ugly.

Also I have proposed an fscrypt ioctl to add keys to a filesystem-level keyring,
and it doesn't use 'struct fscrypt_key' at all:
https://marc.info/?l=linux-fsdevel&m=150879505206393

So I think you should just use the "default" encrypted-key format, where the
payload is just the raw key.  fscrypt can very easily be updated to work with
such keys.

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

Re: [PATCH 1/3] encrypted-keys: add fscrypt format support

[Eric Biggers]

Hi Andre,

On Wed, Jan 10, 2018 at 12:44:16PM +0000, Andre Draszik wrote:
> This is heavily based on commit 79a73d188726
> ("encrypted-keys: add ecryptfs format support").
> 
> The 'encrypted' key type defines its own payload format which contains a
> symmetric key randomly generated that cannot be used directly by the
> fscrypt subsystem, because it instead expects an fscrypt_key structure.
> 
> This patch introduces the new format 'fscrypt' that allows to store an
> fscrypt_key structure inside the encrypted key payload containing
> a randomly generated symmetric key, as the same for the format 'default'
> and 'ecryptfs'.
> 
> More details about the usage of encrypted keys with the fscrypt
> subsystem can be found in the file 'Documentation/security/keys/fscrypt.rst'.
> 

I don't think a new encrypted-key format is needed.  fscrypt really only needs
the raw key.  The fact that fscrypt uses 'struct fscrypt_key' for the key
payloads is a mistake, given that a raw byte array would work just as well.  In
particular, the 'size' field is redundant, since a 'struct key' knows the size
of its payload; and the 'mode' field is meaningless and therefore is ignored.
Also since there are no reserved fields the only way we would ever be able to
add anything new to 'struct fscrypt_key' is by doing a hack where we put an
invalid value in the 'size' field, which would be ugly.

Also I have proposed an fscrypt ioctl to add keys to a filesystem-level keyring,
and it doesn't use 'struct fscrypt_key' at all:
https://marc.info/?l=linux-fsdevel&m=150879505206393

So I think you should just use the "default" encrypted-key format, where the
payload is just the raw key.  fscrypt can very easily be updated to work with
such keys.

Eric

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

[Eric Biggers]

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

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

[Eric Biggers]

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
--
To unsubscribe from this list: send the line "unsubscribe keyrings" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Eric Biggers]

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

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

[Eric Biggers]

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
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Eric Biggers]

Hi Andre,

On Wed, Jan 10, 2018 at 12:44:18PM +0000, Andre 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

[PATCH v2 1/2] fscrypt: add support for the encrypted key type

[André Draszik]

We now try to acquire the key according to the
encryption policy from both key types, 'logon'
as well as 'encrypted'.

Signed-off-by: André Draszik <git@andred.net>
Cc: "Theodore Y. Ts'o" <tytso@mit.edu>
Cc: Jaegeuk Kim <jaegeuk@kernel.org>
Cc: linux-fscrypt@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Eric Biggers <ebiggers@google.com>

---
changes in v2:
* dropped the previously added 'fscrypt' encrypted-key,
  and just use the 'default' format
---
 fs/crypto/keyinfo.c | 72 +++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 50 insertions(+), 22 deletions(-)

diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 5e6e846f5a24..925af599f954 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -10,6 +10,7 @@
  */
 
 #include <keys/user-type.h>
+#include <keys/encrypted-type.h>
 #include <linux/scatterlist.h>
 #include <linux/ratelimit.h>
 #include <crypto/aes.h>
@@ -66,14 +67,20 @@ static int derive_key_aes(u8 deriving_key[FS_AES_128_ECB_KEY_SIZE],
 	return res;
 }
 
-static int validate_user_key(struct fscrypt_info *crypt_info,
+static inline struct key *fscrypt_get_encrypted_key(const char *sig)
+{
+	if (IS_ENABLED(CONFIG_ENCRYPTED_KEYS))
+		return request_key(&key_type_encrypted, sig, NULL);
+	return ERR_PTR(-ENOKEY);
+}
+
+static int validate_keyring_key(struct fscrypt_info *crypt_info,
 			struct fscrypt_context *ctx, u8 *raw_key,
 			const char *prefix, int min_keysize)
 {
 	char *description;
 	struct key *keyring_key;
-	struct fscrypt_key *master_key;
-	const struct user_key_payload *ukp;
+	struct fscrypt_key *master_key, master_key_;
 	int res;
 
 	description = kasprintf(GFP_NOFS, "%s%*phN", prefix,
@@ -83,28 +90,45 @@ static int validate_user_key(struct fscrypt_info *crypt_info,
 		return -ENOMEM;
 
 	keyring_key = request_key(&key_type_logon, description, NULL);
+	if (IS_ERR(keyring_key))
+		keyring_key = fscrypt_get_encrypted_key(description);
 	kfree(description);
 	if (IS_ERR(keyring_key))
 		return PTR_ERR(keyring_key);
 	down_read(&keyring_key->sem);
 
-	if (keyring_key->type != &key_type_logon) {
+	if (keyring_key->type == &key_type_logon) {
+		const struct user_key_payload *ukp;
+
+		ukp = user_key_payload_locked(keyring_key);
+		if (!ukp) {
+			/* key was revoked before we acquired its semaphore */
+			res = -EKEYREVOKED;
+			goto out;
+		}
+		if (ukp->datalen != sizeof(struct fscrypt_key)) {
+			res = -EINVAL;
+			goto out;
+		}
+		master_key = (struct fscrypt_key *)ukp->data;
+	} else if (keyring_key->type == &key_type_encrypted) {
+		const struct encrypted_key_payload *ekp;
+
+		ekp = keyring_key->payload.data[0];
+		master_key = &master_key_;
+
+		master_key->mode = 0;
+		memcpy (master_key->raw, ekp->decrypted_data,
+			min (sizeof (master_key->raw),
+			     (size_t) ekp->decrypted_datalen));
+		master_key->size = ekp->decrypted_datalen;
+	} else {
 		printk_once(KERN_WARNING
-				"%s: key type must be logon\n", __func__);
+				"%s: key type must be logon or encrypted\n",
+				__func__);
 		res = -ENOKEY;
 		goto out;
 	}
-	ukp = user_key_payload_locked(keyring_key);
-	if (!ukp) {
-		/* key was revoked before we acquired its semaphore */
-		res = -EKEYREVOKED;
-		goto out;
-	}
-	if (ukp->datalen != sizeof(struct fscrypt_key)) {
-		res = -EINVAL;
-		goto out;
-	}
-	master_key = (struct fscrypt_key *)ukp->data;
 	BUILD_BUG_ON(FS_AES_128_ECB_KEY_SIZE != FS_KEY_DERIVATION_NONCE_SIZE);
 
 	if (master_key->size < min_keysize || master_key->size > FS_MAX_KEY_SIZE
@@ -113,9 +137,13 @@ static int validate_user_key(struct fscrypt_info *crypt_info,
 				"%s: key size incorrect: %d\n",
 				__func__, master_key->size);
 		res = -ENOKEY;
-		goto out;
+		goto out_clear_key;
 	}
 	res = derive_key_aes(ctx->nonce, master_key, raw_key);
+
+out_clear_key:
+	if (master_key == &master_key_)
+		memzero_explicit(master_key->raw, sizeof (master_key->raw));
 out:
 	up_read(&keyring_key->sem);
 	key_put(keyring_key);
@@ -302,12 +330,12 @@ int fscrypt_get_encryption_info(struct inode *inode)
 	if (!raw_key)
 		goto out;
 
-	res = validate_user_key(crypt_info, &ctx, raw_key, FS_KEY_DESC_PREFIX,
-				keysize);
+	res = validate_keyring_key(crypt_info, &ctx, raw_key,
+				   FS_KEY_DESC_PREFIX, keysize);
 	if (res && inode->i_sb->s_cop->key_prefix) {
-		int res2 = validate_user_key(crypt_info, &ctx, raw_key,
-					     inode->i_sb->s_cop->key_prefix,
-					     keysize);
+		int res2 = validate_keyring_key(crypt_info, &ctx, raw_key,
+						inode->i_sb->s_cop->key_prefix,
+						keysize);
 		if (res2) {
 			if (res2 == -ENOKEY)
 				res = -ENOKEY;
-- 
2.15.1

[PATCH v2 2/2] fscrypt: update documentation for encrypted key support

[André Draszik]

Signed-off-by: André Draszik <git@andred.net>
Cc: "Theodore Y. Ts'o" <tytso@mit.edu>
Cc: Jaegeuk Kim <jaegeuk@kernel.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: linux-fscrypt@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Eric Biggers <ebiggers@google.com>
Cc: linux-doc@vger.kernel.org
---
 Documentation/filesystems/fscrypt.rst | 56 +++++++++++++++++++++++++++++++----
 1 file changed, 51 insertions(+), 5 deletions(-)

diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst
index 776ddc655f79..852ac2900b66 100644
--- a/Documentation/filesystems/fscrypt.rst
+++ b/Documentation/filesystems/fscrypt.rst
@@ -368,11 +368,19 @@ Adding keys
 To provide a master key, userspace must add it to an appropriate
 keyring using the add_key() system call (see:
 ``Documentation/security/keys/core.rst``).  The key type must be
-"logon"; keys of this type are kept in kernel memory and cannot be
-read back by userspace.  The key description must be "fscrypt:"
-followed by the 16-character lower case hex representation of the
-``master_key_descriptor`` that was set in the encryption policy.  The
-key payload must conform to the following structure::
+either "logon" or "encrypted"; "logon" keys are kept in kernel
+memory and cannot be read back by userspace while "encrypted"
+keys can be rooted in a "trusted" key and thus are protected by
+a TPM and cannot be read by userspace in unencrypted form. Note
+that while an "encrypted" key can also be rooted in a "user" key,
+any "encrypted" key rooted in a "user" key can effectively be
+retrieved in the clear, hence only rooting the key in a "trusted"
+key has any useful security properties!
+
+The key description must be "fscrypt:" followed by the 16-character
+lower case hex representation of the ``master_key_descriptor`` that
+was set in the encryption policy.  For a "logon" key, key payload
+must conform to the following structure::
 
     #define FS_MAX_KEY_SIZE 64
 
@@ -386,6 +394,17 @@ key payload must conform to the following structure::
 ``raw`` with ``size`` indicating its size in bytes.  That is, the
 bytes ``raw[0..size-1]`` (inclusive) are the actual key.
 
+When using an "encrypted" key, only the actual ``raw`` key from above
+fscrypt_key structure is needed::
+
+    keyctl add encrypted "fscrypt:``master_key_descriptor``" "new default trusted:``master-key-name`` ``size``" ``ring``
+    keyctl add encrypted "fscrypt:``master_key_descriptor``" "load ``hex_blob``" ``ring``
+
+Where::
+
+    master-key-name:= name of the trusted key this fscrypt master key
+                      shall be rooted in
+
 The key description prefix "fscrypt:" may alternatively be replaced
 with a filesystem-specific prefix such as "ext4:".  However, the
 filesystem-specific prefixes are deprecated and should not be used in
@@ -412,6 +431,33 @@ evicted.  In the future there probably should be a way to provide keys
 directly to the filesystem instead, which would make the intended
 semantics clearer.
 
+Complete Examples
+------------------
+
+Set fscrypt policy on an (empty) encrypted directory, /encrypted::
+
+    $ fscryptctl set_policy 1234567890123456 /encrypted
+
+Create an encrypted key "1234567890123456" of length 64 bytes with format
+'fscrypt' and root it in a previously loaded trusted "kmk"::
+
+    $ keyctl add encrypted "fscrypt:1234567890123456" "new default trusted:kmk 64" @u
+    839715473
+
+    $ keyctl print 839715473
+    default trusted:kmk 64 e98a49dc11eb9312f46530879aac869300ee734035100f4ee
+    5441279369a4c9d83d6e59b8158d0a3de01790c0bb99af82e9603cb6977c7d1229338cda
+    80375aaf034678405a00c19806d6fb12490e39b1d7ca603c491b58a962345160e344ae51
+    83483e066692d05f5ab3d8b9ea39cab0e
+
+    $ keyctl pipe 839715473 > fscrypt.blob
+
+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
+
 Access semantics
 ================
 
-- 
2.15.1

Re: [PATCH 1/3] encrypted-keys: add fscrypt format support

[André Draszik]

Thanks Eric for the review!

On Wed, 2018-01-10 at 20:00 -0800, Eric Biggers wrote:
> Hi André,
> 
> On Wed, Jan 10, 2018 at 12:44:16PM +0000, André Draszik wrote:
> > This is heavily based on commit 79a73d188726
> > ("encrypted-keys: add ecryptfs format support").
> > 
> > The 'encrypted' key type defines its own payload format which contains a
> > symmetric key randomly generated that cannot be used directly by the
> > fscrypt subsystem, because it instead expects an fscrypt_key structure.
> > 
> > This patch introduces the new format 'fscrypt' that allows to store an
> > fscrypt_key structure inside the encrypted key payload containing
> > a randomly generated symmetric key, as the same for the format 'default'
> > and 'ecryptfs'.
> > 
> > More details about the usage of encrypted keys with the fscrypt
> > subsystem can be found in the file
> > 'Documentation/security/keys/fscrypt.rst'.
> > 
> 
> I don't think a new encrypted-key format is needed.  fscrypt really only
> needs
> the raw key.

This was actually my original approach, but I thought it might potentially
be useful to have a new encrypted-key type to be able to do verification of
parameters (e.g. key size) early.
Additionally, because the type is then encoded in the blob stored in the
file system (keyctl pipe), it'd be easy to spot incompatibilities in case
fscrypt internal data structures change, whereas without one can only rely
on the name of the key (key description), should such a change ever happen.

> Also I have proposed an fscrypt ioctl to add keys to a filesystem-level
> keyring,
> and it doesn't use 'struct fscrypt_key' at all:
> https://marc.info/?l=linux-fsdevel&m=150879505206393
> 
> So I think you should just use the "default" encrypted-key format, where
> the
> payload is just the raw key.  fscrypt can very easily be updated to work
> with
> such keys.

I've done that in v2 - I am generating the fscrypt_key temporarily on the
fly but haven't gotten rid of fscrypt_key altogether... Is that what you had
in mind?

I also haven't based it on your work mentioned above, would that be
preferred?

Cheers,
Andre'

Re: [PATCH 1/3] encrypted-keys: add fscrypt format support

[André Draszik]

Thanks Eric for the review!

On Wed, 2018-01-10 at 20:00 -0800, Eric Biggers wrote:
> Hi André,
> 
> On Wed, Jan 10, 2018 at 12:44:16PM +0000, André Draszik wrote:
> > This is heavily based on commit 79a73d188726
> > ("encrypted-keys: add ecryptfs format support").
> > 
> > The 'encrypted' key type defines its own payload format which contains a
> > symmetric key randomly generated that cannot be used directly by the
> > fscrypt subsystem, because it instead expects an fscrypt_key structure.
> > 
> > This patch introduces the new format 'fscrypt' that allows to store an
> > fscrypt_key structure inside the encrypted key payload containing
> > a randomly generated symmetric key, as the same for the format 'default'
> > and 'ecryptfs'.
> > 
> > More details about the usage of encrypted keys with the fscrypt
> > subsystem can be found in the file
> > 'Documentation/security/keys/fscrypt.rst'.
> > 
> 
> I don't think a new encrypted-key format is needed.  fscrypt really only
> needs
> the raw key.

This was actually my original approach, but I thought it might potentially
be useful to have a new encrypted-key type to be able to do verification of
parameters (e.g. key size) early.
Additionally, because the type is then encoded in the blob stored in the
file system (keyctl pipe), it'd be easy to spot incompatibilities in case
fscrypt internal data structures change, whereas without one can only rely
on the name of the key (key description), should such a change ever happen.

> Also I have proposed an fscrypt ioctl to add keys to a filesystem-level
> keyring,
> and it doesn't use 'struct fscrypt_key' at all:
> https://marc.info/?l=linux-fsdevel&m\x150879505206393
> 
> So I think you should just use the "default" encrypted-key format, where
> the
> payload is just the raw key.  fscrypt can very easily be updated to work
> with
> such keys.

I've done that in v2 - I am generating the fscrypt_key temporarily on the
fly but haven't gotten rid of fscrypt_key altogether... Is that what you had
in mind?

I also haven't based it on your work mentioned above, would that be
preferred?

Cheers,
Andre'

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

[PATCH 1/3] encrypted-keys: add fscrypt format support

[André Draszik]

Thanks Eric for the review!

On Wed, 2018-01-10 at 20:00 -0800, Eric Biggers wrote:
> Hi Andr?,
> 
> On Wed, Jan 10, 2018 at 12:44:16PM +0000, Andr? Draszik wrote:
> > This is heavily based on commit 79a73d188726
> > ("encrypted-keys: add ecryptfs format support").
> > 
> > The 'encrypted' key type defines its own payload format which contains a
> > symmetric key randomly generated that cannot be used directly by the
> > fscrypt subsystem, because it instead expects an fscrypt_key structure.
> > 
> > This patch introduces the new format 'fscrypt' that allows to store an
> > fscrypt_key structure inside the encrypted key payload containing
> > a randomly generated symmetric key, as the same for the format 'default'
> > and 'ecryptfs'.
> > 
> > More details about the usage of encrypted keys with the fscrypt
> > subsystem can be found in the file
> > 'Documentation/security/keys/fscrypt.rst'.
> > 
> 
> I don't think a new encrypted-key format is needed.  fscrypt really only
> needs
> the raw key.

This was actually my original approach, but I thought it might potentially
be useful to have a new encrypted-key type to be able to do verification of
parameters (e.g. key size) early.
Additionally, because the type is then encoded in the blob stored in the
file system (keyctl pipe), it'd be easy to spot incompatibilities in case
fscrypt internal data structures change, whereas without one can only rely
on the name of the key (key description), should such a change ever happen.

> Also I have proposed an fscrypt ioctl to add keys to a filesystem-level
> keyring,
> and it doesn't use 'struct fscrypt_key' at all:
> https://marc.info/?l=linux-fsdevel&m=150879505206393
> 
> So I think you should just use the "default" encrypted-key format, where
> the
> payload is just the raw key.  fscrypt can very easily be updated to work
> with
> such keys.

I've done that in v2 - I am generating the fscrypt_key temporarily on the
fly but haven't gotten rid of fscrypt_key altogether... Is that what you had
in mind?

I also haven't based it on your work mentioned above, would that be
preferred?

Cheers,
Andre'

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

Re: [PATCH 1/3] encrypted-keys: add fscrypt format support

[André Draszik]

Thanks Eric for the review!

On Wed, 2018-01-10 at 20:00 -0800, Eric Biggers wrote:
> Hi Andre,
> 
> On Wed, Jan 10, 2018 at 12:44:16PM +0000, Andre Draszik wrote:
> > This is heavily based on commit 79a73d188726
> > ("encrypted-keys: add ecryptfs format support").
> > 
> > The 'encrypted' key type defines its own payload format which contains a
> > symmetric key randomly generated that cannot be used directly by the
> > fscrypt subsystem, because it instead expects an fscrypt_key structure.
> > 
> > This patch introduces the new format 'fscrypt' that allows to store an
> > fscrypt_key structure inside the encrypted key payload containing
> > a randomly generated symmetric key, as the same for the format 'default'
> > and 'ecryptfs'.
> > 
> > More details about the usage of encrypted keys with the fscrypt
> > subsystem can be found in the file
> > 'Documentation/security/keys/fscrypt.rst'.
> > 
> 
> I don't think a new encrypted-key format is needed.  fscrypt really only
> needs
> the raw key.

This was actually my original approach, but I thought it might potentially
be useful to have a new encrypted-key type to be able to do verification of
parameters (e.g. key size) early.
Additionally, because the type is then encoded in the blob stored in the
file system (keyctl pipe), it'd be easy to spot incompatibilities in case
fscrypt internal data structures change, whereas without one can only rely
on the name of the key (key description), should such a change ever happen.

> Also I have proposed an fscrypt ioctl to add keys to a filesystem-level
> keyring,
> and it doesn't use 'struct fscrypt_key' at all:
> https://marc.info/?l=linux-fsdevel&m=150879505206393
> 
> So I think you should just use the "default" encrypted-key format, where
> the
> payload is just the raw key.  fscrypt can very easily be updated to work
> with
> such keys.

I've done that in v2 - I am generating the fscrypt_key temporarily on the
fly but haven't gotten rid of fscrypt_key altogether... Is that what you had
in mind?

I also haven't based it on your work mentioned above, would that be
preferred?

Cheers,
Andre'

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

[André Draszik]

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é

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

[André Draszik]

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é

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

[André Draszik]

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?

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

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

[André Draszik]

Hi Eric,

On Wed, 2018-01-10 at 20:48 -0800, Eric Biggers wrote:
> Hi Andre,
> 
> On Wed, Jan 10, 2018 at 12:44:18PM +0000, Andre 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,
Andre

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

[Theodore Ts'o]

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
				

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

[Theodore Ts'o]

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
				
--
To unsubscribe from this list: send the line "unsubscribe keyrings" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Theodore Ts'o]

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
				

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

[Theodore Ts'o]

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
				
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Theodore Ts'o]

On Wed, Jan 17, 2018 at 02:38:59PM +0000, Andre 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
				

Re: [PATCH 1/3] encrypted-keys: add fscrypt format support

[Eric Biggers]

Hi Andr�,

On Wed, Jan 17, 2018 at 02:29:29PM +0000, Andr� Draszik wrote:
> Thanks Eric for the review!
> 
> On Wed, 2018-01-10 at 20:00 -0800, Eric Biggers wrote:
> > Hi Andr�,
> > 
> > On Wed, Jan 10, 2018 at 12:44:16PM +0000, Andr� Draszik wrote:
> > > This is heavily based on commit 79a73d188726
> > > ("encrypted-keys: add ecryptfs format support").
> > > 
> > > The 'encrypted' key type defines its own payload format which contains a
> > > symmetric key randomly generated that cannot be used directly by the
> > > fscrypt subsystem, because it instead expects an fscrypt_key structure.
> > > 
> > > This patch introduces the new format 'fscrypt' that allows to store an
> > > fscrypt_key structure inside the encrypted key payload containing
> > > a randomly generated symmetric key, as the same for the format 'default'
> > > and 'ecryptfs'.
> > > 
> > > More details about the usage of encrypted keys with the fscrypt
> > > subsystem can be found in the file
> > > 'Documentation/security/keys/fscrypt.rst'.
> > > 
> > 
> > I don't think a new encrypted-key format is needed.  fscrypt really only
> > needs
> > the raw key.
> 
> This was actually my original approach, but I thought it might potentially
> be useful to have a new encrypted-key type to be able to do verification of
> parameters (e.g. key size) early.
> Additionally, because the type is then encoded in the blob stored in the
> file system (keyctl pipe), it'd be easy to spot incompatibilities in case
> fscrypt internal data structures change, whereas without one can only rely
> on the name of the key (key description), should such a change ever happen.

'struct fscrypt_key' isn't really useful because it doesn't have any reserved
fields.  So if we wanted to change the format we'd have to change the key
description anyway, by setting some flag in the fscrypt_policy and the
fscrypt_context that indicates a different key description should be used.

And fscrypt does verify the key's size before it uses it already.  Sure, it
might to nice to verify it at add_key() time, but I don't think it's necessary.

> 
> > Also I have proposed an fscrypt ioctl to add keys to a filesystem-level
> > keyring,
> > and it doesn't use 'struct fscrypt_key' at all:
> > https://marc.info/?l=linux-fsdevel&m=150879505206393
> > 
> > So I think you should just use the "default" encrypted-key format, where
> > the
> > payload is just the raw key.  fscrypt can very easily be updated to work
> > with
> > such keys.
> 
> I've done that in v2 - I am generating the fscrypt_key temporarily on the
> fly but haven't gotten rid of fscrypt_key altogether... Is that what you had
> in mind?

That would work, but we don't actually need the fscrypt_key structure at all.
Just declare 'const u8 *raw' and 'u32 size' local variables and set them
appropriately, then pass those into derive_key_aes() (changing its prototype)
rather than the fscrypt_key.

> 
> I also haven't based it on your work mentioned above, would that be
> preferred?

Not really, that patch series ("fscrypt: filesystem-level keyring and v2 policy
support") is still in RFC status, so we don't know which would be merged first.
I'm still looking for people with the interest and expertise to review it :-)

That being said, we could choose to support the "encrypted" key type using only
FS_IOC_ADD_ENCRYPTION_KEY, rather than the current mechanism of
process-subscribed keyrings.  (With FS_IOC_ADD_ENCRYPTION_KEY, we'd need to use
a flag in 'struct fscrypt_add_key_args' to indicate that the key should be found
by searching the current process's keyrings for an "encrypted" key with the
given description, rather than taking the key directly from the ioctl argument.)
But it wouldn't be a huge deal to support it both ways.

Eric

Re: [PATCH 1/3] encrypted-keys: add fscrypt format support

[Eric Biggers]

Hi André,

On Wed, Jan 17, 2018 at 02:29:29PM +0000, André Draszik wrote:
> Thanks Eric for the review!
> 
> On Wed, 2018-01-10 at 20:00 -0800, Eric Biggers wrote:
> > Hi André,
> > 
> > On Wed, Jan 10, 2018 at 12:44:16PM +0000, André Draszik wrote:
> > > This is heavily based on commit 79a73d188726
> > > ("encrypted-keys: add ecryptfs format support").
> > > 
> > > The 'encrypted' key type defines its own payload format which contains a
> > > symmetric key randomly generated that cannot be used directly by the
> > > fscrypt subsystem, because it instead expects an fscrypt_key structure.
> > > 
> > > This patch introduces the new format 'fscrypt' that allows to store an
> > > fscrypt_key structure inside the encrypted key payload containing
> > > a randomly generated symmetric key, as the same for the format 'default'
> > > and 'ecryptfs'.
> > > 
> > > More details about the usage of encrypted keys with the fscrypt
> > > subsystem can be found in the file
> > > 'Documentation/security/keys/fscrypt.rst'.
> > > 
> > 
> > I don't think a new encrypted-key format is needed.  fscrypt really only
> > needs
> > the raw key.
> 
> This was actually my original approach, but I thought it might potentially
> be useful to have a new encrypted-key type to be able to do verification of
> parameters (e.g. key size) early.
> Additionally, because the type is then encoded in the blob stored in the
> file system (keyctl pipe), it'd be easy to spot incompatibilities in case
> fscrypt internal data structures change, whereas without one can only rely
> on the name of the key (key description), should such a change ever happen.

'struct fscrypt_key' isn't really useful because it doesn't have any reserved
fields.  So if we wanted to change the format we'd have to change the key
description anyway, by setting some flag in the fscrypt_policy and the
fscrypt_context that indicates a different key description should be used.

And fscrypt does verify the key's size before it uses it already.  Sure, it
might to nice to verify it at add_key() time, but I don't think it's necessary.

> 
> > Also I have proposed an fscrypt ioctl to add keys to a filesystem-level
> > keyring,
> > and it doesn't use 'struct fscrypt_key' at all:
> > https://marc.info/?l=linux-fsdevel&m\x150879505206393
> > 
> > So I think you should just use the "default" encrypted-key format, where
> > the
> > payload is just the raw key.  fscrypt can very easily be updated to work
> > with
> > such keys.
> 
> I've done that in v2 - I am generating the fscrypt_key temporarily on the
> fly but haven't gotten rid of fscrypt_key altogether... Is that what you had
> in mind?

That would work, but we don't actually need the fscrypt_key structure at all.
Just declare 'const u8 *raw' and 'u32 size' local variables and set them
appropriately, then pass those into derive_key_aes() (changing its prototype)
rather than the fscrypt_key.

> 
> I also haven't based it on your work mentioned above, would that be
> preferred?

Not really, that patch series ("fscrypt: filesystem-level keyring and v2 policy
support") is still in RFC status, so we don't know which would be merged first.
I'm still looking for people with the interest and expertise to review it :-)

That being said, we could choose to support the "encrypted" key type using only
FS_IOC_ADD_ENCRYPTION_KEY, rather than the current mechanism of
process-subscribed keyrings.  (With FS_IOC_ADD_ENCRYPTION_KEY, we'd need to use
a flag in 'struct fscrypt_add_key_args' to indicate that the key should be found
by searching the current process's keyrings for an "encrypted" key with the
given description, rather than taking the key directly from the ioctl argument.)
But it wouldn't be a huge deal to support it both ways.

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

Re: [PATCH 1/3] encrypted-keys: add fscrypt format support

[Eric Biggers]

Hi André,

On Wed, Jan 17, 2018 at 02:29:29PM +0000, André Draszik wrote:
> Thanks Eric for the review!
> 
> On Wed, 2018-01-10 at 20:00 -0800, Eric Biggers wrote:
> > Hi André,
> > 
> > On Wed, Jan 10, 2018 at 12:44:16PM +0000, André Draszik wrote:
> > > This is heavily based on commit 79a73d188726
> > > ("encrypted-keys: add ecryptfs format support").
> > > 
> > > The 'encrypted' key type defines its own payload format which contains a
> > > symmetric key randomly generated that cannot be used directly by the
> > > fscrypt subsystem, because it instead expects an fscrypt_key structure.
> > > 
> > > This patch introduces the new format 'fscrypt' that allows to store an
> > > fscrypt_key structure inside the encrypted key payload containing
> > > a randomly generated symmetric key, as the same for the format 'default'
> > > and 'ecryptfs'.
> > > 
> > > More details about the usage of encrypted keys with the fscrypt
> > > subsystem can be found in the file
> > > 'Documentation/security/keys/fscrypt.rst'.
> > > 
> > 
> > I don't think a new encrypted-key format is needed.  fscrypt really only
> > needs
> > the raw key.
> 
> This was actually my original approach, but I thought it might potentially
> be useful to have a new encrypted-key type to be able to do verification of
> parameters (e.g. key size) early.
> Additionally, because the type is then encoded in the blob stored in the
> file system (keyctl pipe), it'd be easy to spot incompatibilities in case
> fscrypt internal data structures change, whereas without one can only rely
> on the name of the key (key description), should such a change ever happen.

'struct fscrypt_key' isn't really useful because it doesn't have any reserved
fields.  So if we wanted to change the format we'd have to change the key
description anyway, by setting some flag in the fscrypt_policy and the
fscrypt_context that indicates a different key description should be used.

And fscrypt does verify the key's size before it uses it already.  Sure, it
might to nice to verify it at add_key() time, but I don't think it's necessary.

> 
> > Also I have proposed an fscrypt ioctl to add keys to a filesystem-level
> > keyring,
> > and it doesn't use 'struct fscrypt_key' at all:
> > https://marc.info/?l=linux-fsdevel&m=150879505206393
> > 
> > So I think you should just use the "default" encrypted-key format, where
> > the
> > payload is just the raw key.  fscrypt can very easily be updated to work
> > with
> > such keys.
> 
> I've done that in v2 - I am generating the fscrypt_key temporarily on the
> fly but haven't gotten rid of fscrypt_key altogether... Is that what you had
> in mind?

That would work, but we don't actually need the fscrypt_key structure at all.
Just declare 'const u8 *raw' and 'u32 size' local variables and set them
appropriately, then pass those into derive_key_aes() (changing its prototype)
rather than the fscrypt_key.

> 
> I also haven't based it on your work mentioned above, would that be
> preferred?

Not really, that patch series ("fscrypt: filesystem-level keyring and v2 policy
support") is still in RFC status, so we don't know which would be merged first.
I'm still looking for people with the interest and expertise to review it :-)

That being said, we could choose to support the "encrypted" key type using only
FS_IOC_ADD_ENCRYPTION_KEY, rather than the current mechanism of
process-subscribed keyrings.  (With FS_IOC_ADD_ENCRYPTION_KEY, we'd need to use
a flag in 'struct fscrypt_add_key_args' to indicate that the key should be found
by searching the current process's keyrings for an "encrypted" key with the
given description, rather than taking the key directly from the ioctl argument.)
But it wouldn't be a huge deal to support it both ways.

Eric

[PATCH 1/3] encrypted-keys: add fscrypt format support

[Eric Biggers]

Hi Andr?,

On Wed, Jan 17, 2018 at 02:29:29PM +0000, Andr? Draszik wrote:
> Thanks Eric for the review!
> 
> On Wed, 2018-01-10 at 20:00 -0800, Eric Biggers wrote:
> > Hi Andr?,
> > 
> > On Wed, Jan 10, 2018 at 12:44:16PM +0000, Andr? Draszik wrote:
> > > This is heavily based on commit 79a73d188726
> > > ("encrypted-keys: add ecryptfs format support").
> > > 
> > > The 'encrypted' key type defines its own payload format which contains a
> > > symmetric key randomly generated that cannot be used directly by the
> > > fscrypt subsystem, because it instead expects an fscrypt_key structure.
> > > 
> > > This patch introduces the new format 'fscrypt' that allows to store an
> > > fscrypt_key structure inside the encrypted key payload containing
> > > a randomly generated symmetric key, as the same for the format 'default'
> > > and 'ecryptfs'.
> > > 
> > > More details about the usage of encrypted keys with the fscrypt
> > > subsystem can be found in the file
> > > 'Documentation/security/keys/fscrypt.rst'.
> > > 
> > 
> > I don't think a new encrypted-key format is needed.  fscrypt really only
> > needs
> > the raw key.
> 
> This was actually my original approach, but I thought it might potentially
> be useful to have a new encrypted-key type to be able to do verification of
> parameters (e.g. key size) early.
> Additionally, because the type is then encoded in the blob stored in the
> file system (keyctl pipe), it'd be easy to spot incompatibilities in case
> fscrypt internal data structures change, whereas without one can only rely
> on the name of the key (key description), should such a change ever happen.

'struct fscrypt_key' isn't really useful because it doesn't have any reserved
fields.  So if we wanted to change the format we'd have to change the key
description anyway, by setting some flag in the fscrypt_policy and the
fscrypt_context that indicates a different key description should be used.

And fscrypt does verify the key's size before it uses it already.  Sure, it
might to nice to verify it at add_key() time, but I don't think it's necessary.

> 
> > Also I have proposed an fscrypt ioctl to add keys to a filesystem-level
> > keyring,
> > and it doesn't use 'struct fscrypt_key' at all:
> > https://marc.info/?l=linux-fsdevel&m=150879505206393
> > 
> > So I think you should just use the "default" encrypted-key format, where
> > the
> > payload is just the raw key.  fscrypt can very easily be updated to work
> > with
> > such keys.
> 
> I've done that in v2 - I am generating the fscrypt_key temporarily on the
> fly but haven't gotten rid of fscrypt_key altogether... Is that what you had
> in mind?

That would work, but we don't actually need the fscrypt_key structure at all.
Just declare 'const u8 *raw' and 'u32 size' local variables and set them
appropriately, then pass those into derive_key_aes() (changing its prototype)
rather than the fscrypt_key.

> 
> I also haven't based it on your work mentioned above, would that be
> preferred?

Not really, that patch series ("fscrypt: filesystem-level keyring and v2 policy
support") is still in RFC status, so we don't know which would be merged first.
I'm still looking for people with the interest and expertise to review it :-)

That being said, we could choose to support the "encrypted" key type using only
FS_IOC_ADD_ENCRYPTION_KEY, rather than the current mechanism of
process-subscribed keyrings.  (With FS_IOC_ADD_ENCRYPTION_KEY, we'd need to use
a flag in 'struct fscrypt_add_key_args' to indicate that the key should be found
by searching the current process's keyrings for an "encrypted" key with the
given description, rather than taking the key directly from the ioctl argument.)
But it wouldn't be a huge deal to support it both ways.

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

Re: [PATCH 1/3] encrypted-keys: add fscrypt format support

[Eric Biggers]

Hi Andre,

On Wed, Jan 17, 2018 at 02:29:29PM +0000, Andre Draszik wrote:
> Thanks Eric for the review!
> 
> On Wed, 2018-01-10 at 20:00 -0800, Eric Biggers wrote:
> > Hi Andre,
> > 
> > On Wed, Jan 10, 2018 at 12:44:16PM +0000, Andre Draszik wrote:
> > > This is heavily based on commit 79a73d188726
> > > ("encrypted-keys: add ecryptfs format support").
> > > 
> > > The 'encrypted' key type defines its own payload format which contains a
> > > symmetric key randomly generated that cannot be used directly by the
> > > fscrypt subsystem, because it instead expects an fscrypt_key structure.
> > > 
> > > This patch introduces the new format 'fscrypt' that allows to store an
> > > fscrypt_key structure inside the encrypted key payload containing
> > > a randomly generated symmetric key, as the same for the format 'default'
> > > and 'ecryptfs'.
> > > 
> > > More details about the usage of encrypted keys with the fscrypt
> > > subsystem can be found in the file
> > > 'Documentation/security/keys/fscrypt.rst'.
> > > 
> > 
> > I don't think a new encrypted-key format is needed.  fscrypt really only
> > needs
> > the raw key.
> 
> This was actually my original approach, but I thought it might potentially
> be useful to have a new encrypted-key type to be able to do verification of
> parameters (e.g. key size) early.
> Additionally, because the type is then encoded in the blob stored in the
> file system (keyctl pipe), it'd be easy to spot incompatibilities in case
> fscrypt internal data structures change, whereas without one can only rely
> on the name of the key (key description), should such a change ever happen.

'struct fscrypt_key' isn't really useful because it doesn't have any reserved
fields.  So if we wanted to change the format we'd have to change the key
description anyway, by setting some flag in the fscrypt_policy and the
fscrypt_context that indicates a different key description should be used.

And fscrypt does verify the key's size before it uses it already.  Sure, it
might to nice to verify it at add_key() time, but I don't think it's necessary.

> 
> > Also I have proposed an fscrypt ioctl to add keys to a filesystem-level
> > keyring,
> > and it doesn't use 'struct fscrypt_key' at all:
> > https://marc.info/?l=linux-fsdevel&m=150879505206393
> > 
> > So I think you should just use the "default" encrypted-key format, where
> > the
> > payload is just the raw key.  fscrypt can very easily be updated to work
> > with
> > such keys.
> 
> I've done that in v2 - I am generating the fscrypt_key temporarily on the
> fly but haven't gotten rid of fscrypt_key altogether... Is that what you had
> in mind?

That would work, but we don't actually need the fscrypt_key structure at all.
Just declare 'const u8 *raw' and 'u32 size' local variables and set them
appropriately, then pass those into derive_key_aes() (changing its prototype)
rather than the fscrypt_key.

> 
> I also haven't based it on your work mentioned above, would that be
> preferred?

Not really, that patch series ("fscrypt: filesystem-level keyring and v2 policy
support") is still in RFC status, so we don't know which would be merged first.
I'm still looking for people with the interest and expertise to review it :-)

That being said, we could choose to support the "encrypted" key type using only
FS_IOC_ADD_ENCRYPTION_KEY, rather than the current mechanism of
process-subscribed keyrings.  (With FS_IOC_ADD_ENCRYPTION_KEY, we'd need to use
a flag in 'struct fscrypt_add_key_args' to indicate that the key should be found
by searching the current process's keyrings for an "encrypted" key with the
given description, rather than taking the key directly from the ioctl argument.)
But it wouldn't be a huge deal to support it both ways.

Eric

Re: [PATCH v2 1/2] fscrypt: add support for the encrypted key type

[Eric Biggers]

Hi Andr�,

On Wed, Jan 17, 2018 at 02:13:18PM +0000, Andr� Draszik wrote:
> We now try to acquire the key according to the
> encryption policy from both key types, 'logon'
> as well as 'encrypted'.
> 
> Signed-off-by: Andr� Draszik <git@andred.net>
> Cc: "Theodore Y. Ts'o" <tytso@mit.edu>
> Cc: Jaegeuk Kim <jaegeuk@kernel.org>
> Cc: linux-fscrypt@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: Eric Biggers <ebiggers@google.com>
> 
> ---
> changes in v2:
> * dropped the previously added 'fscrypt' encrypted-key,
>   and just use the 'default' format

The commit message needs to explain the purpose of the change, not just what the
change is.  It might also be helpful to combine this with the documentation
patch, as some of the explanation can be in the documentation.

Can you please also keep the keyrings mailing list and encrypted-keys
maintainers Cc'ed on this series?  I know it doesn't touch the keyrings code
directly anymore, but people may still be interested.  Historically there have
also been a lot of bugs in code using the keyrings API, e.g. not using correct
locking.

> ---
>  fs/crypto/keyinfo.c | 72 +++++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 50 insertions(+), 22 deletions(-)
> 
> diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
> index 5e6e846f5a24..925af599f954 100644
> --- a/fs/crypto/keyinfo.c
> +++ b/fs/crypto/keyinfo.c
> @@ -10,6 +10,7 @@
>   */
>  
>  #include <keys/user-type.h>
> +#include <keys/encrypted-type.h>
>  #include <linux/scatterlist.h>
>  #include <linux/ratelimit.h>
>  #include <crypto/aes.h>
> @@ -66,14 +67,20 @@ static int derive_key_aes(u8 deriving_key[FS_AES_128_ECB_KEY_SIZE],
>  	return res;
>  }
>  
> -static int validate_user_key(struct fscrypt_info *crypt_info,
> +static inline struct key *fscrypt_get_encrypted_key(const char *sig)
> +{

Call this 'description', not 'sig'.  I think you copied the 'sig' naming from
eCryptfs, but it doesn't make sense for fscrypt.


> +	if (IS_ENABLED(CONFIG_ENCRYPTED_KEYS))
> +		return request_key(&key_type_encrypted, sig, NULL);
> +	return ERR_PTR(-ENOKEY);
> +}
> +
> +static int validate_keyring_key(struct fscrypt_info *crypt_info,
>  			struct fscrypt_context *ctx, u8 *raw_key,
>  			const char *prefix, int min_keysize)
>  {
>  	char *description;
>  	struct key *keyring_key;
> -	struct fscrypt_key *master_key;
> -	const struct user_key_payload *ukp;
> +	struct fscrypt_key *master_key, master_key_;
>  	int res;
>  
>  	description = kasprintf(GFP_NOFS, "%s%*phN", prefix,
> @@ -83,28 +90,45 @@ static int validate_user_key(struct fscrypt_info *crypt_info,
>  		return -ENOMEM;
>  
>  	keyring_key = request_key(&key_type_logon, description, NULL);
> +	if (IS_ERR(keyring_key))
> +		keyring_key = fscrypt_get_encrypted_key(description);

I think the fallback to the "encrypted" key type should only happen if
request_key() returns ERR_PTR(-ENOKEY), indicating that the "logon" key isn't
present.  Otherwise it will fall back for other types of errors as well which
doesn't really make sense.

>  	kfree(description);
>  	if (IS_ERR(keyring_key))
>  		return PTR_ERR(keyring_key);
>  	down_read(&keyring_key->sem);
>  
> -	if (keyring_key->type != &key_type_logon) {
> +	if (keyring_key->type == &key_type_logon) {
> +		const struct user_key_payload *ukp;
> +
> +		ukp = user_key_payload_locked(keyring_key);
> +		if (!ukp) {
> +			/* key was revoked before we acquired its semaphore */
> +			res = -EKEYREVOKED;
> +			goto out;
> +		}
> +		if (ukp->datalen != sizeof(struct fscrypt_key)) {
> +			res = -EINVAL;
> +			goto out;
> +		}
> +		master_key = (struct fscrypt_key *)ukp->data;
> +	} else if (keyring_key->type == &key_type_encrypted) {
> +		const struct encrypted_key_payload *ekp;
> +
> +		ekp = keyring_key->payload.data[0];
> +		master_key = &master_key_;
> +
> +		master_key->mode = 0;
> +		memcpy (master_key->raw, ekp->decrypted_data,
> +			min (sizeof (master_key->raw),
> +			     (size_t) ekp->decrypted_datalen));
> +		master_key->size = ekp->decrypted_datalen;
> +	} else {
>  		printk_once(KERN_WARNING
> -				"%s: key type must be logon\n", __func__);
> +				"%s: key type must be logon or encrypted\n",
> +				__func__);
>  		res = -ENOKEY;
>  		goto out;
>  	}
> -	ukp = user_key_payload_locked(keyring_key);
> -	if (!ukp) {
> -		/* key was revoked before we acquired its semaphore */
> -		res = -EKEYREVOKED;
> -		goto out;
> -	}
> -	if (ukp->datalen != sizeof(struct fscrypt_key)) {
> -		res = -EINVAL;
> -		goto out;
> -	}
> -	master_key = (struct fscrypt_key *)ukp->data;
>  	BUILD_BUG_ON(FS_AES_128_ECB_KEY_SIZE != FS_KEY_DERIVATION_NONCE_SIZE);
>  
>  	if (master_key->size < min_keysize || master_key->size > FS_MAX_KEY_SIZE
> @@ -113,9 +137,13 @@ static int validate_user_key(struct fscrypt_info *crypt_info,
>  				"%s: key size incorrect: %d\n",
>  				__func__, master_key->size);
>  		res = -ENOKEY;
> -		goto out;
> +		goto out_clear_key;
>  	}
>  	res = derive_key_aes(ctx->nonce, master_key, raw_key);
> +
> +out_clear_key:
> +	if (master_key == &master_key_)
> +		memzero_explicit(master_key->raw, sizeof (master_key->raw));
>  out:
>  	up_read(&keyring_key->sem);
>  	key_put(keyring_key);
> @@ -302,12 +330,12 @@ int fscrypt_get_encryption_info(struct inode *inode)
>  	if (!raw_key)
>  		goto out;

No need for the temporary 'struct fscrypt_key', just use ekp->decrypted_data and
ekp->decrypted_datalen directly.

Eric

Re: [PATCH v2 1/2] fscrypt: add support for the encrypted key type

[Eric Biggers]

Hi André,

On Wed, Jan 17, 2018 at 02:13:18PM +0000, André Draszik wrote:
> We now try to acquire the key according to the
> encryption policy from both key types, 'logon'
> as well as 'encrypted'.
> 
> Signed-off-by: André Draszik <git@andred.net>
> Cc: "Theodore Y. Ts'o" <tytso@mit.edu>
> Cc: Jaegeuk Kim <jaegeuk@kernel.org>
> Cc: linux-fscrypt@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: Eric Biggers <ebiggers@google.com>
> 
> ---
> changes in v2:
> * dropped the previously added 'fscrypt' encrypted-key,
>   and just use the 'default' format

The commit message needs to explain the purpose of the change, not just what the
change is.  It might also be helpful to combine this with the documentation
patch, as some of the explanation can be in the documentation.

Can you please also keep the keyrings mailing list and encrypted-keys
maintainers Cc'ed on this series?  I know it doesn't touch the keyrings code
directly anymore, but people may still be interested.  Historically there have
also been a lot of bugs in code using the keyrings API, e.g. not using correct
locking.

> ---
>  fs/crypto/keyinfo.c | 72 +++++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 50 insertions(+), 22 deletions(-)
> 
> diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
> index 5e6e846f5a24..925af599f954 100644
> --- a/fs/crypto/keyinfo.c
> +++ b/fs/crypto/keyinfo.c
> @@ -10,6 +10,7 @@
>   */
>  
>  #include <keys/user-type.h>
> +#include <keys/encrypted-type.h>
>  #include <linux/scatterlist.h>
>  #include <linux/ratelimit.h>
>  #include <crypto/aes.h>
> @@ -66,14 +67,20 @@ static int derive_key_aes(u8 deriving_key[FS_AES_128_ECB_KEY_SIZE],
>  	return res;
>  }
>  
> -static int validate_user_key(struct fscrypt_info *crypt_info,
> +static inline struct key *fscrypt_get_encrypted_key(const char *sig)
> +{

Call this 'description', not 'sig'.  I think you copied the 'sig' naming from
eCryptfs, but it doesn't make sense for fscrypt.


> +	if (IS_ENABLED(CONFIG_ENCRYPTED_KEYS))
> +		return request_key(&key_type_encrypted, sig, NULL);
> +	return ERR_PTR(-ENOKEY);
> +}
> +
> +static int validate_keyring_key(struct fscrypt_info *crypt_info,
>  			struct fscrypt_context *ctx, u8 *raw_key,
>  			const char *prefix, int min_keysize)
>  {
>  	char *description;
>  	struct key *keyring_key;
> -	struct fscrypt_key *master_key;
> -	const struct user_key_payload *ukp;
> +	struct fscrypt_key *master_key, master_key_;
>  	int res;
>  
>  	description = kasprintf(GFP_NOFS, "%s%*phN", prefix,
> @@ -83,28 +90,45 @@ static int validate_user_key(struct fscrypt_info *crypt_info,
>  		return -ENOMEM;
>  
>  	keyring_key = request_key(&key_type_logon, description, NULL);
> +	if (IS_ERR(keyring_key))
> +		keyring_key = fscrypt_get_encrypted_key(description);

I think the fallback to the "encrypted" key type should only happen if
request_key() returns ERR_PTR(-ENOKEY), indicating that the "logon" key isn't
present.  Otherwise it will fall back for other types of errors as well which
doesn't really make sense.

>  	kfree(description);
>  	if (IS_ERR(keyring_key))
>  		return PTR_ERR(keyring_key);
>  	down_read(&keyring_key->sem);
>  
> -	if (keyring_key->type != &key_type_logon) {
> +	if (keyring_key->type == &key_type_logon) {
> +		const struct user_key_payload *ukp;
> +
> +		ukp = user_key_payload_locked(keyring_key);
> +		if (!ukp) {
> +			/* key was revoked before we acquired its semaphore */
> +			res = -EKEYREVOKED;
> +			goto out;
> +		}
> +		if (ukp->datalen != sizeof(struct fscrypt_key)) {
> +			res = -EINVAL;
> +			goto out;
> +		}
> +		master_key = (struct fscrypt_key *)ukp->data;
> +	} else if (keyring_key->type == &key_type_encrypted) {
> +		const struct encrypted_key_payload *ekp;
> +
> +		ekp = keyring_key->payload.data[0];
> +		master_key = &master_key_;
> +
> +		master_key->mode = 0;
> +		memcpy (master_key->raw, ekp->decrypted_data,
> +			min (sizeof (master_key->raw),
> +			     (size_t) ekp->decrypted_datalen));
> +		master_key->size = ekp->decrypted_datalen;
> +	} else {
>  		printk_once(KERN_WARNING
> -				"%s: key type must be logon\n", __func__);
> +				"%s: key type must be logon or encrypted\n",
> +				__func__);
>  		res = -ENOKEY;
>  		goto out;
>  	}
> -	ukp = user_key_payload_locked(keyring_key);
> -	if (!ukp) {
> -		/* key was revoked before we acquired its semaphore */
> -		res = -EKEYREVOKED;
> -		goto out;
> -	}
> -	if (ukp->datalen != sizeof(struct fscrypt_key)) {
> -		res = -EINVAL;
> -		goto out;
> -	}
> -	master_key = (struct fscrypt_key *)ukp->data;
>  	BUILD_BUG_ON(FS_AES_128_ECB_KEY_SIZE != FS_KEY_DERIVATION_NONCE_SIZE);
>  
>  	if (master_key->size < min_keysize || master_key->size > FS_MAX_KEY_SIZE
> @@ -113,9 +137,13 @@ static int validate_user_key(struct fscrypt_info *crypt_info,
>  				"%s: key size incorrect: %d\n",
>  				__func__, master_key->size);
>  		res = -ENOKEY;
> -		goto out;
> +		goto out_clear_key;
>  	}
>  	res = derive_key_aes(ctx->nonce, master_key, raw_key);
> +
> +out_clear_key:
> +	if (master_key == &master_key_)
> +		memzero_explicit(master_key->raw, sizeof (master_key->raw));
>  out:
>  	up_read(&keyring_key->sem);
>  	key_put(keyring_key);
> @@ -302,12 +330,12 @@ int fscrypt_get_encryption_info(struct inode *inode)
>  	if (!raw_key)
>  		goto out;

No need for the temporary 'struct fscrypt_key', just use ekp->decrypted_data and
ekp->decrypted_datalen directly.

Eric

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

[André Draszik]

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'

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

[André Draszik]

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'

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

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

[André Draszik]

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'

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

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

[André Draszik]

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, Andre 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'