From 0e1ac853fb889b3d8d00e3a4751f388b0b8d8f26 Mon Sep 17 00:00:00 2001 From: Gary Ching-Pang Lin Date: Wed, 5 Dec 2012 11:12:43 +0800 Subject: [PATCH 1/4] Correct MOK size and SignatureSize The MOK size didn't include the SignatureOwner GUID. The SignatureData header size was added twice accidentally. --- src/mokutil.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/mokutil.c b/src/mokutil.c index 1c32828..1b8465f 100644 --- a/src/mokutil.c +++ b/src/mokutil.c @@ -143,7 +143,7 @@ build_mok_list (void *data, unsigned long data_size, uint32_t *mok_num) return NULL; } - list[count].mok_size = CertList->SignatureSize; + list[count].mok_size = CertList->SignatureSize - sizeof(efi_guid_t); list[count].mok = (void *)Cert->SignatureData; count++; @@ -497,8 +497,7 @@ import_moks (char **files, uint32_t total) CertList->SignatureListSize = sizes[i] + sizeof(EFI_SIGNATURE_LIST) + sizeof(EFI_SIGNATURE_DATA) - 1; CertList->SignatureHeaderSize = 0; - CertList->SignatureSize = sizes[i] + - sizeof(EFI_SIGNATURE_DATA) + 16; + CertList->SignatureSize = sizes[i] + sizeof(efi_guid_t); CertData->SignatureOwner = SHIM_LOCK_GUID; fd = open (files[i], O_RDONLY); -- 1.7.10.4 From 69955da3819da3abaf198e5dae038c44814df5c0 Mon Sep 17 00:00:00 2001 From: Gary Ching-Pang Lin Date: Wed, 5 Dec 2012 11:24:58 +0800 Subject: [PATCH 2/4] Don't import duplicate keys This commit compares keys in PK, KEK, db, MokListRT, and MokNew before issuing a new request to avoid enrolling keys twice. --- src/mokutil.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 124 insertions(+), 4 deletions(-) diff --git a/src/mokutil.c b/src/mokutil.c index 1b8465f..cf38422 100644 --- a/src/mokutil.c +++ b/src/mokutil.c @@ -333,8 +333,8 @@ get_password (char **password, int *len, int min, int max) } static int -generate_auth (void *new_list, int list_len, char *password, int pw_len, - uint8_t *auth) +generate_auth (void *new_list, unsigned long list_len, char *password, + int pw_len, uint8_t *auth) { efi_char16_t efichar_pass[PASSWORD_MAX+1]; unsigned long efichar_len; @@ -444,12 +444,97 @@ is_valid_cert (void *cert, uint32_t cert_size) } static int +is_duplicate (const void *cert, const uint32_t cert_size, const char *db_name, + efi_guid_t guid) +{ + efi_variable_t var; + uint32_t mok_num; + MokListNode *list; + int i, ret = 0; + + if (!cert || cert_size == 0 || !db_name) + return 0; + + memset (&var, 0, sizeof(var)); + var.VariableName = db_name; + var.VendorGuid = guid; + + if (read_variable (&var) != EFI_SUCCESS) + return 0; + + list = build_mok_list (var.Data, var.DataSize, &mok_num); + if (list == NULL) { + goto done; + } + + for (i = 0; i < mok_num; i++) { + if (list[i].mok_size != cert_size) + continue; + + if (memcmp (list[i].mok, cert, cert_size) == 0) { + ret = 1; + break; + } + } + +done: + free (var.Data); + + return ret; +} + +static int +verify_mok_new (void *mok_new, unsigned long mok_new_size) +{ + efi_variable_t mok_auth; + uint8_t auth[SHA256_DIGEST_LENGTH]; + char *password; + int pw_len, fail = 0; + size_t n; + int ret = 0; + + memset (&mok_auth, 0, sizeof(mok_auth)); + mok_auth.VariableName = "MokAuth"; + mok_auth.VendorGuid = SHIM_LOCK_GUID; + if (read_variable (&mok_auth) == EFI_SUCCESS) + return 0; + + while (fail < 3) { + printf ("input old password: "); + pw_len = read_hidden_line (&password, &n); + printf ("\n"); + + if (pw_len > PASSWORD_MAX || pw_len < PASSWORD_MIN) { + free (password); + fprintf (stderr, "invalid password\n"); + fail++; + continue; + } + + generate_auth (mok_new, mok_new_size, password, pw_len, auth); + if (memcmp (auth, mok_auth.Data, SHA256_DIGEST_LENGTH) == 0) { + ret = 1; + break; + } + + fail++; + } + + if (mok_auth.Data) + free (mok_auth.Data); + + return ret; +} + +static int import_moks (char **files, uint32_t total) { + efi_variable_t mok_new; void *new_list = NULL; void *ptr; struct stat buf; unsigned long list_size = 0; + unsigned long real_size = 0; uint32_t *sizes = NULL; int fd = -1; ssize_t read_size; @@ -481,6 +566,12 @@ import_moks (char **files, uint32_t total) list_size += sizeof(EFI_SIGNATURE_LIST) * total; list_size += sizeof(efi_guid_t) * total; + memset (&mok_new, 0, sizeof(mok_new)); + mok_new.VariableName = "MokNew"; + mok_new.VendorGuid = SHIM_LOCK_GUID; + if (read_variable (&mok_new) == EFI_SUCCESS) + list_size += mok_new.DataSize; + new_list = malloc (list_size); if (!new_list) { fprintf (stderr, "Failed to allocate space for MokNew\n"); @@ -518,17 +609,46 @@ import_moks (char **files, uint32_t total) fprintf (stderr, "Warning!!! %s is not a valid x509 certificate in DER format\n", files[i]); } - ptr += sizes[i]; + + /* whether this key is already enrolled... */ + if (!is_duplicate (ptr, sizes[i], "PK", EFI_GLOBAL_VARIABLE) && + !is_duplicate (ptr, sizes[i], "KEK", EFI_GLOBAL_VARIABLE) && + !is_duplicate (ptr, sizes[i], "db", EFI_GLOBAL_VARIABLE) && + !is_duplicate (ptr, sizes[i], "MokListRT", SHIM_LOCK_GUID) && + !is_duplicate (ptr, sizes[i], "MokNew", SHIM_LOCK_GUID)) { + ptr += sizes[i]; + real_size += sizes[i] + sizeof(EFI_SIGNATURE_LIST) + sizeof(efi_guid_t); + } else { + ptr -= sizeof(EFI_SIGNATURE_LIST) + sizeof(efi_guid_t); + } close (fd); } - if (update_request (new_list, list_size) < 0) { + /* All keys are enrolled, nothing to do here... */ + if (real_size == 0) { + ret = 0; + goto error; + } + + /* append the keys in MokNew */ + if (mok_new.Data) { + /* request the previous password to verify the keys */ + if (!verify_mok_new (mok_new.Data, mok_new.DataSize)) { + goto error; + } + + memcpy (ptr, mok_new.Data, mok_new.DataSize); + } + + if (update_request (new_list, real_size) < 0) { goto error; } ret = 0; error: + if (mok_new.Data) + free (mok_new.Data); if (sizes) free (sizes); if (new_list) -- 1.7.10.4 From 10046350e223b6912bd9c3a7031f06779cb326bb Mon Sep 17 00:00:00 2001 From: Gary Ching-Pang Lin Date: Fri, 7 Dec 2012 15:57:50 +0800 Subject: [PATCH 3/4] Check MokAuth correctly --- src/mokutil.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/mokutil.c b/src/mokutil.c index cf38422..9d56a90 100644 --- a/src/mokutil.c +++ b/src/mokutil.c @@ -496,8 +496,10 @@ verify_mok_new (void *mok_new, unsigned long mok_new_size) memset (&mok_auth, 0, sizeof(mok_auth)); mok_auth.VariableName = "MokAuth"; mok_auth.VendorGuid = SHIM_LOCK_GUID; - if (read_variable (&mok_auth) == EFI_SUCCESS) + if (read_variable (&mok_auth) != EFI_SUCCESS) { + fprintf (stderr, "Failed to read MokAuth\n"); return 0; + } while (fail < 3) { printf ("input old password: "); -- 1.7.10.4 From 9674b3249fef0d2ba00364f9f120f1ef17b710fc Mon Sep 17 00:00:00 2001 From: Gary Ching-Pang Lin Date: Fri, 7 Dec 2012 15:58:30 +0800 Subject: [PATCH 4/4] Really append the old request to the new one... --- src/mokutil.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/mokutil.c b/src/mokutil.c index 9d56a90..aba1cfb 100644 --- a/src/mokutil.c +++ b/src/mokutil.c @@ -640,7 +640,8 @@ import_moks (char **files, uint32_t total) goto error; } - memcpy (ptr, mok_new.Data, mok_new.DataSize); + memcpy (new_list + real_size, mok_new.Data, mok_new.DataSize); + real_size += mok_new.DataSize; } if (update_request (new_list, real_size) < 0) { -- 1.7.10.4