[PATCH 0/5 v2] Using the hash in MOKx to blacklist kernel module

[PATCH 0/5 v2] Using the hash in MOKx to blacklist kernel module

[Lee, Chun-Yi]

This patch set is base on the efi-lock-down and keys-uefi branchs in
David Howells's linux-fs git tree.
    https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-uefi

The main purpose is using the MOKx to blacklist kernel module.

As the MOK (Machine Owner Key), MOKx is a EFI boot time variable which
is maintained by shim boot loader. We can enroll the hash of blacklisted
kernel module (with or without signature) to MOKx by mokutil. Kernel loads
the hash from MOKx to blacklist keyring when booting. Kernel will prevent
to load the kernel module when its hash be found in blacklist.

This function is useful to revoke a kernel module that it has exploit. Or
revoking a kernel module that it was signed by a unsecure key.

Except MOKx, this patch set fixs another two issues: The MOK/MOKx should
not be loaded when secure boot is disabled. And, modified error message
prints out appropriate status string for reading by human being.

v2:
Chekcikng the attributes of db and mok before loading certificates.

Lee, Chun-Yi (5):
  MODSIGN: do not load mok when secure boot disabled
  MODSIGN: print appropriate status message when getting UEFI
    certificates list
  MODSIGN: load blacklist from MOKx
  MODSIGN: checking the blacklisted hash before loading a kernel module
  MODSIGN: check the attributes of db and mok

 certs/load_uefi.c       | 92 +++++++++++++++++++++++++++++++++++--------------
 include/linux/efi.h     | 25 ++++++++++++++
 kernel/module_signing.c | 62 +++++++++++++++++++++++++++++++--
 3 files changed, 152 insertions(+), 27 deletions(-)

-- 
2.10.2

[PATCH 1/5] MODSIGN: do not load mok when secure boot disabled

[Lee, Chun-Yi]

The mok can not be trusted when the secure boot is disabled. Which
means that the kernel embedded certificate is the only trusted key.

Due to db/dbx are authenticated variables, they needs manufacturer's
KEK for update. So db/dbx are secure when secureboot disabled.

Cc: David Howells <dhowells@redhat.com>
Cc: Josh Boyer <jwboyer@fedoraproject.org>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Signed-off-by: Lee, Chun-Yi <jlee@suse.com>
---
 certs/load_uefi.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/certs/load_uefi.c b/certs/load_uefi.c
index 3d88459..d6de4d0 100644
--- a/certs/load_uefi.c
+++ b/certs/load_uefi.c
@@ -164,17 +164,6 @@ static int __init load_uefi_certs(void)
 		}
 	}
 
-	mok = get_cert_list(L"MokListRT", &mok_var, &moksize);
-	if (!mok) {
-		pr_info("MODSIGN: Couldn't get UEFI MokListRT\n");
-	} else {
-		rc = parse_efi_signature_list("UEFI:MokListRT",
-					      mok, moksize, get_handler_for_db);
-		if (rc)
-			pr_err("Couldn't parse MokListRT signatures: %d\n", rc);
-		kfree(mok);
-	}
-
 	dbx = get_cert_list(L"dbx", &secure_var, &dbxsize);
 	if (!dbx) {
 		pr_info("MODSIGN: Couldn't get UEFI dbx list\n");
@@ -187,6 +176,21 @@ static int __init load_uefi_certs(void)
 		kfree(dbx);
 	}
 
+	/* the MOK can not be trusted when secure boot is disabled */
+	if (!efi_enabled(EFI_SECURE_BOOT))
+		return 0;
+
+	mok = get_cert_list(L"MokListRT", &mok_var, &moksize);
+	if (!mok) {
+		pr_info("MODSIGN: Couldn't get UEFI MokListRT\n");
+	} else {
+		rc = parse_efi_signature_list("UEFI:MokListRT",
+					      mok, moksize, get_handler_for_db);
+		if (rc)
+			pr_err("Couldn't parse MokListRT signatures: %d\n", rc);
+		kfree(mok);
+	}
+
 	return rc;
 }
 late_initcall(load_uefi_certs);
-- 
2.10.2

[PATCH 2/5] MODSIGN: print appropriate status message when getting UEFI certificates list

[Lee, Chun-Yi]

When getting certificates list from UEFI variable, the original error
message shows the state number from UEFI firmware. It's hard to be read
by human. This patch changed the error message to show the appropriate
string.

The message will be showed as:

[    0.788529] MODSIGN: Couldn't get UEFI MokListRT: EFI_NOT_FOUND
[    0.788537] MODSIGN: Couldn't get UEFI MokListXRT: EFI_NOT_FOUND

Cc: David Howells <dhowells@redhat.com>
Cc: Josh Boyer <jwboyer@fedoraproject.org>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Signed-off-by: Lee, Chun-Yi <jlee@suse.com>
---
 certs/load_uefi.c   | 43 ++++++++++++++++++++++++++++++-------------
 include/linux/efi.h | 25 +++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 13 deletions(-)

diff --git a/certs/load_uefi.c b/certs/load_uefi.c
index d6de4d0..f2f372b 100644
--- a/certs/load_uefi.c
+++ b/certs/load_uefi.c
@@ -4,6 +4,7 @@
 #include <linux/err.h>
 #include <linux/efi.h>
 #include <linux/slab.h>
+#include <linux/ucs2_string.h>
 #include <keys/asymmetric-type.h>
 #include <keys/system_keyring.h>
 #include "internal.h"
@@ -32,6 +33,24 @@ static __init bool uefi_check_ignore_db(void)
 	return status == EFI_SUCCESS;
 }
 
+static __init void print_get_fail(efi_char16_t *char16_str, efi_status_t status)
+{
+	char *utf8_str;
+	unsigned long utf8_size;
+
+	if (!char16_str)
+		return;
+	utf8_size = ucs2_utf8size(char16_str) + 1;
+	utf8_str = kmalloc(utf8_size, GFP_KERNEL);
+	if (!utf8_str)
+		return;
+	ucs2_as_utf8(utf8_str, char16_str, utf8_size);
+
+	pr_info("MODSIGN: Couldn't get UEFI %s: %s\n",
+		utf8_str, efi_status_to_str(status));
+	kfree(utf8_str);
+}
+
 /*
  * Get a certificate list blob from the named EFI variable.
  */
@@ -45,25 +64,29 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
 
 	status = efi.get_variable(name, guid, NULL, &lsize, &tmpdb);
 	if (status != EFI_BUFFER_TOO_SMALL) {
-		pr_err("Couldn't get size: 0x%lx\n", status);
-		return NULL;
+		if (status != EFI_NOT_FOUND)
+			pr_err("Couldn't get size: 0x%lx\n", status);
+		goto err;
 	}
 
 	db = kmalloc(lsize, GFP_KERNEL);
 	if (!db) {
 		pr_err("Couldn't allocate memory for uefi cert list\n");
-		return NULL;
+		goto err;
 	}
 
 	status = efi.get_variable(name, guid, NULL, &lsize, db);
 	if (status != EFI_SUCCESS) {
 		kfree(db);
 		pr_err("Error reading db var: 0x%lx\n", status);
-		return NULL;
+		goto err;
 	}
 
 	*size = lsize;
 	return db;
+err:
+	print_get_fail(name, status);
+	return NULL;
 }
 
 /*
@@ -153,9 +176,7 @@ static int __init load_uefi_certs(void)
 	 */
 	if (!uefi_check_ignore_db()) {
 		db = get_cert_list(L"db", &secure_var, &dbsize);
-		if (!db) {
-			pr_err("MODSIGN: Couldn't get UEFI db list\n");
-		} else {
+		if (db) {
 			rc = parse_efi_signature_list("UEFI:db",
 						      db, dbsize, get_handler_for_db);
 			if (rc)
@@ -165,9 +186,7 @@ static int __init load_uefi_certs(void)
 	}
 
 	dbx = get_cert_list(L"dbx", &secure_var, &dbxsize);
-	if (!dbx) {
-		pr_info("MODSIGN: Couldn't get UEFI dbx list\n");
-	} else {
+	if (dbx) {
 		rc = parse_efi_signature_list("UEFI:dbx",
 					      dbx, dbxsize,
 					      get_handler_for_dbx);
@@ -181,9 +200,7 @@ static int __init load_uefi_certs(void)
 		return 0;
 
 	mok = get_cert_list(L"MokListRT", &mok_var, &moksize);
-	if (!mok) {
-		pr_info("MODSIGN: Couldn't get UEFI MokListRT\n");
-	} else {
+	if (mok) {
 		rc = parse_efi_signature_list("UEFI:MokListRT",
 					      mok, moksize, get_handler_for_db);
 		if (rc)
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 2729d6f..c44946c 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1600,4 +1600,29 @@ struct linux_efi_random_seed {
 	u8	bits[];
 };
 
+#define EFI_STATUS_STR(_status)				\
+	case EFI_##_status:				\
+		return "EFI_" __stringify(_status);	\
+
+static inline char *
+efi_status_to_str(efi_status_t status)
+{
+	switch (status) {
+	EFI_STATUS_STR(SUCCESS)
+	EFI_STATUS_STR(LOAD_ERROR)
+	EFI_STATUS_STR(INVALID_PARAMETER)
+	EFI_STATUS_STR(UNSUPPORTED)
+	EFI_STATUS_STR(BAD_BUFFER_SIZE)
+	EFI_STATUS_STR(BUFFER_TOO_SMALL)
+	EFI_STATUS_STR(NOT_READY)
+	EFI_STATUS_STR(DEVICE_ERROR)
+	EFI_STATUS_STR(WRITE_PROTECTED)
+	EFI_STATUS_STR(OUT_OF_RESOURCES)
+	EFI_STATUS_STR(NOT_FOUND)
+	EFI_STATUS_STR(ABORTED)
+	EFI_STATUS_STR(SECURITY_VIOLATION)
+	}
+
+	return "";
+}
 #endif /* _LINUX_EFI_H */
-- 
2.10.2

Re: [PATCH 2/5] MODSIGN: print appropriate status message when getting UEFI certificates list

[James Bottomley]

On Tue, 2018-03-13 at 18:35 +0800, Lee, Chun-Yi wrote:
> When getting certificates list from UEFI variable, the original error
> message shows the state number from UEFI firmware. It's hard to be
> read by human. This patch changed the error message to show the
> appropriate string.
> 
> The message will be showed as:
> 
> [    0.788529] MODSIGN: Couldn't get UEFI MokListRT: EFI_NOT_FOUND
> [    0.788537] MODSIGN: Couldn't get UEFI MokListXRT: EFI_NOT_FOUND

I keep saying this, but these error messages need to be gated on the
presence of shim for the non-shim secure boot case.  You can't assume
the shim variables are there because they won't be in the case of a
fully owned system.

James

Re: [PATCH 2/5] MODSIGN: print appropriate status message when getting UEFI certificates list

[joeyli]

Hi James,

Thanks for your review.

On Tue, Mar 13, 2018 at 10:17:50AM -0700, James Bottomley wrote:
> On Tue, 2018-03-13 at 18:35 +0800, Lee, Chun-Yi wrote:
> > When getting certificates list from UEFI variable, the original error
> > message shows the state number from UEFI firmware. It's hard to be
> > read by human. This patch changed the error message to show the
> > appropriate string.
> > 
> > The message will be showed as:
> > 
> > [    0.788529] MODSIGN: Couldn't get UEFI MokListRT: EFI_NOT_FOUND
> > [    0.788537] MODSIGN: Couldn't get UEFI MokListXRT: EFI_NOT_FOUND
> 
> I keep saying this, but these error messages need to be gated on the
> presence of shim for the non-shim secure boot case.  You can't assume
> the shim variables are there because they won't be in the case of a
> fully owned system.

I will change the message to pr_debug.

Thanks a lot!
Joey Lee 

Re: [PATCH 1/5] MODSIGN: do not load mok when secure boot disabled

[joeyli]

Hi Ard, 

First! Thanks for your review!

On Tue, Mar 13, 2018 at 05:25:30PM +0000, Ard Biesheuvel wrote:
> On 13 March 2018 at 10:37, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote:
> > The mok can not be trusted when the secure boot is disabled. Which
> > means that the kernel embedded certificate is the only trusted key.
> >
> > Due to db/dbx are authenticated variables, they needs manufacturer's
> > KEK for update. So db/dbx are secure when secureboot disabled.
> >
> 
> Did you consider the case where secure boot is not implemented? I
> don't think db/dbx are secure in that case, although perhaps it may
> not matter (a bit more information on the purpose of these patches and
> all the shim lingo etc would be appreciated)
> 

The patch 5 in this series checks that the db/dbx must have
EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS attribute. But I agree
with you that kernel should checks the SecureBoot variable must
exist in system. I will add patch to detect it.

> > Cc: David Howells <dhowells@redhat.com>
> > Cc: Josh Boyer <jwboyer@fedoraproject.org>
> > Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
> > Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
> > ---
> >  certs/load_uefi.c | 26 +++++++++++++++-----------
> >  1 file changed, 15 insertions(+), 11 deletions(-)
> >
> > diff --git a/certs/load_uefi.c b/certs/load_uefi.c
> > index 3d88459..d6de4d0 100644
> > --- a/certs/load_uefi.c
> > +++ b/certs/load_uefi.c
> > @@ -164,17 +164,6 @@ static int __init load_uefi_certs(void)
> >                 }
> >         }
> >
> > -       mok = get_cert_list(L"MokListRT", &mok_var, &moksize);
> 
> Which tree does this apply to? My tree doesn't have get_cert_list()
>

This patch set is base on the efi-lock-down and keys-uefi branchs in
David Howells's linux-fs git tree.
    https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-uefi

Thanks a lot!
Joey Lee 

Re: [PATCH 1/5] MODSIGN: do not load mok when secure boot disabled

[Ard Biesheuvel]

On 13 March 2018 at 10:37, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote:
> The mok can not be trusted when the secure boot is disabled. Which
> means that the kernel embedded certificate is the only trusted key.
>
> Due to db/dbx are authenticated variables, they needs manufacturer's
> KEK for update. So db/dbx are secure when secureboot disabled.
>

Did you consider the case where secure boot is not implemented? I
don't think db/dbx are secure in that case, although perhaps it may
not matter (a bit more information on the purpose of these patches and
all the shim lingo etc would be appreciated)

> Cc: David Howells <dhowells@redhat.com>
> Cc: Josh Boyer <jwboyer@fedoraproject.org>
> Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
> Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
> ---
>  certs/load_uefi.c | 26 +++++++++++++++-----------
>  1 file changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/certs/load_uefi.c b/certs/load_uefi.c
> index 3d88459..d6de4d0 100644
> --- a/certs/load_uefi.c
> +++ b/certs/load_uefi.c
> @@ -164,17 +164,6 @@ static int __init load_uefi_certs(void)
>                 }
>         }
>
> -       mok = get_cert_list(L"MokListRT", &mok_var, &moksize);

Which tree does this apply to? My tree doesn't have get_cert_list()

> -       if (!mok) {
> -               pr_info("MODSIGN: Couldn't get UEFI MokListRT\n");
> -       } else {
> -               rc = parse_efi_signature_list("UEFI:MokListRT",
> -                                             mok, moksize, get_handler_for_db);
> -               if (rc)
> -                       pr_err("Couldn't parse MokListRT signatures: %d\n", rc);
> -               kfree(mok);
> -       }
> -
>         dbx = get_cert_list(L"dbx", &secure_var, &dbxsize);
>         if (!dbx) {
>                 pr_info("MODSIGN: Couldn't get UEFI dbx list\n");
> @@ -187,6 +176,21 @@ static int __init load_uefi_certs(void)
>                 kfree(dbx);
>         }
>
> +       /* the MOK can not be trusted when secure boot is disabled */
> +       if (!efi_enabled(EFI_SECURE_BOOT))
> +               return 0;
> +
> +       mok = get_cert_list(L"MokListRT", &mok_var, &moksize);
> +       if (!mok) {
> +               pr_info("MODSIGN: Couldn't get UEFI MokListRT\n");
> +       } else {
> +               rc = parse_efi_signature_list("UEFI:MokListRT",
> +                                             mok, moksize, get_handler_for_db);
> +               if (rc)
> +                       pr_err("Couldn't parse MokListRT signatures: %d\n", rc);
> +               kfree(mok);
> +       }
> +
>         return rc;
>  }
>  late_initcall(load_uefi_certs);
> --
> 2.10.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-efi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/5] MODSIGN: do not load mok when secure boot disabled

[Lee, Chun-Yi]

The mok can not be trusted when the secure boot is disabled. Which
means that the kernel embedded certificate is the only trusted key.

Due to db/dbx are authenticated variables, they needs manufacturer's
KEK for update. So db/dbx are secure when secureboot disabled.

Cc: David Howells <dhowells@redhat.com>
Cc: Josh Boyer <jwboyer@fedoraproject.org>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
---
 certs/load_uefi.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/certs/load_uefi.c b/certs/load_uefi.c
index 3d88459..d6de4d0 100644
--- a/certs/load_uefi.c
+++ b/certs/load_uefi.c
@@ -164,17 +164,6 @@ static int __init load_uefi_certs(void)
 		}
 	}
 
-	mok = get_cert_list(L"MokListRT", &mok_var, &moksize);
-	if (!mok) {
-		pr_info("MODSIGN: Couldn't get UEFI MokListRT\n");
-	} else {
-		rc = parse_efi_signature_list("UEFI:MokListRT",
-					      mok, moksize, get_handler_for_db);
-		if (rc)
-			pr_err("Couldn't parse MokListRT signatures: %d\n", rc);
-		kfree(mok);
-	}
-
 	dbx = get_cert_list(L"dbx", &secure_var, &dbxsize);
 	if (!dbx) {
 		pr_info("MODSIGN: Couldn't get UEFI dbx list\n");
@@ -187,6 +176,21 @@ static int __init load_uefi_certs(void)
 		kfree(dbx);
 	}
 
+	/* the MOK can not be trusted when secure boot is disabled */
+	if (!efi_enabled(EFI_SECURE_BOOT))
+		return 0;
+
+	mok = get_cert_list(L"MokListRT", &mok_var, &moksize);
+	if (!mok) {
+		pr_info("MODSIGN: Couldn't get UEFI MokListRT\n");
+	} else {
+		rc = parse_efi_signature_list("UEFI:MokListRT",
+					      mok, moksize, get_handler_for_db);
+		if (rc)
+			pr_err("Couldn't parse MokListRT signatures: %d\n", rc);
+		kfree(mok);
+	}
+
 	return rc;
 }
 late_initcall(load_uefi_certs);
-- 
2.10.2