Re: [PATCH v4] KEYS: encrypted: Instantiate key with user-provided decrypted data

From: Nayna
Date: Thu Jan 20 2022 - 12:14:07 EST



On 1/13/22 14:01, Yael Tiomkin wrote:
On Mon, Jan 10, 2022 at 11:04 AM Nayna <nayna@xxxxxxxxxxxxxxxxxx> wrote:

On 12/29/21 16:53, Yael Tiomkin wrote:
The encrypted.c class supports instantiation of encrypted keys with
either an already-encrypted key material, or by generating new key
material based on random numbers. This patch defines a new datablob
format: [<format>] <master-key name> <decrypted data length>
<decrypted data> that allows to instantiate encrypted keys using
user-provided decrypted data, and therefore allows to perform key
encryption from userspace. The decrypted key material will be
inaccessible from userspace.

Reviewed-by: Mimi Zohar <zohar@xxxxxxxxxxxxx>
Signed-off-by: Yael Tiomkin <yaelt@xxxxxxxxxx>
---

Notes:
v -> v2: fixed compilation error.

v2 -> v3: modified documentation.

v3 -> v4: modified commit message.

.../security/keys/trusted-encrypted.rst | 25 ++++++--
security/keys/encrypted-keys/encrypted.c | 62 +++++++++++++------
2 files changed, 63 insertions(+), 24 deletions(-)

diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst
index 80d5a5af62a1..f614dad7de12 100644
--- a/Documentation/security/keys/trusted-encrypted.rst
+++ b/Documentation/security/keys/trusted-encrypted.rst
@@ -107,12 +107,13 @@ Encrypted Keys
--------------

Encrypted keys do not depend on a trust source, and are faster, as they use AES
-for encryption/decryption. New keys are created from kernel-generated random
-numbers, and are encrypted/decrypted using a specified ‘master’ key. The
-‘master’ key can either be a trusted-key or user-key type. The main disadvantage
-of encrypted keys is that if they are not rooted in a trusted key, they are only
-as secure as the user key encrypting them. The master user key should therefore
-be loaded in as secure a way as possible, preferably early in boot.
+for encryption/decryption. New keys are created either from kernel-generated
+random numbers or user-provided decrypted data, and are encrypted/decrypted
+using a specified ‘master’ key. The ‘master’ key can either be a trusted-key or
+user-key type. The main disadvantage of encrypted keys is that if they are not
+rooted in a trusted key, they are only as secure as the user key encrypting
+them. The master user key should therefore be loaded in as secure a way as
+possible, preferably early in boot.


Usage
@@ -199,6 +200,8 @@ Usage::

keyctl add encrypted name "new [format] key-type:master-key-name keylen"
ring
+ keyctl add encrypted name "new [format] key-type:master-key-name keylen
+ decrypted-data" ring
keyctl add encrypted name "load hex_blob" ring
keyctl update keyid "update key-type:master-key-name"

@@ -303,6 +306,16 @@ Load an encrypted key "evm" from saved blob::
82dbbc55be2a44616e4959430436dc4f2a7a9659aa60bb4652aeb2120f149ed197c564e0
24717c64 5972dcb82ab2dde83376d82b2e3c09ffc

+Instantiate an encrypted key "evm" using user-provided decrypted data::
+
+ $ keyctl add encrypted evm "new default user:kmk 32 `cat evm_decrypted_data.blob`" @u
+ 794890253
+
+ $ keyctl print 794890253
+ default user:kmk 32 2375725ad57798846a9bbd240de8906f006e66c03af53b1b382d
+ bbc55be2a44616e4959430436dc4f2a7a9659aa60bb4652aeb2120f149ed197c564e0247
+ 17c64 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 order to use encrypted keys to mount an eCryptfs filesystem. More details
diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index 87432b35d771..baf6fba5e05e 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -159,6 +159,7 @@ static int valid_master_desc(const char *new_desc, const char *orig_desc)
*
* datablob format:
* new [<format>] <master-key name> <decrypted data length>
+ * new [<format>] <master-key name> <decrypted data length> <decrypted data>
* load [<format>] <master-key name> <decrypted data length>
* <encrypted iv + data>
* update <new-master-key name>
@@ -170,7 +171,7 @@ static int valid_master_desc(const char *new_desc, const char *orig_desc)
*/
static int datablob_parse(char *datablob, const char **format,
char **master_desc, char **decrypted_datalen,
- char **hex_encoded_iv)
+ char **hex_encoded_iv, char **decrypted_data)
{
substring_t args[MAX_OPT_ARGS];
int ret = -EINVAL;
@@ -231,6 +232,8 @@ static int datablob_parse(char *datablob, const char **format,
"when called from .update method\n", keyword);
break;
}
+ *decrypted_data = strsep(&datablob, " \t");
+
ret = 0;
break;
case Opt_load:
@@ -595,7 +598,8 @@ static int derived_key_decrypt(struct encrypted_key_payload *epayload,
static struct encrypted_key_payload *encrypted_key_alloc(struct key *key,
const char *format,
const char *master_desc,
- const char *datalen)
+ const char *datalen,
+ const char *decrypted_data)
{
struct encrypted_key_payload *epayload = NULL;
unsigned short datablob_len;
@@ -604,6 +608,7 @@ static struct encrypted_key_payload *encrypted_key_alloc(struct key *key,
unsigned int encrypted_datalen;
unsigned int format_len;
long dlen;
+ int i;
int ret;

ret = kstrtol(datalen, 10, &dlen);
@@ -613,6 +618,20 @@ static struct encrypted_key_payload *encrypted_key_alloc(struct key *key,
format_len = (!format) ? strlen(key_format_default) : strlen(format);
decrypted_datalen = dlen;
payload_datalen = decrypted_datalen;
+
+ if (decrypted_data) {
+ if (strlen(decrypted_data) != decrypted_datalen) {
+ pr_err("encrypted key: decrypted data provided does not match decrypted data length provided\n");
+ return ERR_PTR(-EINVAL);
+ }
+ for (i = 0; i < strlen(decrypted_data); i++) {
+ if (!isalnum(decrypted_data[i])) {
User-provided decrypted data may have special characters, commonly used
in passwords or key phrases, apart from alphanumeric. Replace isalnum
with !iscntrl() to validate against control characters but allow special
characters.

Thanks & Regards,

- Nayna

Hi Nayna,
I wonder if we should use isprint() instead?

I have been thinking more about this. Encrypted keys documentation says, "All user level blobs, are displayed and loaded in hex ascii for convenience."

Should we use a similar hex ascii encoding for user-provided data?  Verification would then be isxdigit().

Users should convert their input to hex ascii before passing it to keyctl.

Thanks & Regards,

     - Nayna