Messages in this thread | | | Date | Thu, 20 Jan 2022 12:13:33 -0500 | Subject | Re: [PATCH v4] KEYS: encrypted: Instantiate key with user-provided decrypted data | From | Nayna <> |
| |
On 1/13/22 14:01, Yael Tiomkin wrote: > On Mon, Jan 10, 2022 at 11:04 AM Nayna <nayna@linux.vnet.ibm.com> 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@linux.ibm.com> >>> Signed-off-by: Yael Tiomkin <yaelt@google.com> >>> --- >>> >>> 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
| |