lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1512057063.3020.11.camel@HansenPartnership.com>
Date:   Thu, 30 Nov 2017 07:51:03 -0800
From:   James Bottomley <James.Bottomley@...senPartnership.com>
To:     "Lee, Chun-Yi" <joeyli.kernel@...il.com>,
        David Howells <dhowells@...hat.com>
Cc:     linux-fs@...r.kernel.org, linux-efi@...r.kernel.org,
        linux-kernel@...r.kernel.org, "Lee, Chun-Yi" <jlee@...e.com>,
        Josh Boyer <jwboyer@...oraproject.org>
Subject: Re: [PATCH 1/4] MODSIGN: do not load mok when secure boot disabled

On Wed, 2017-11-29 at 22:11 +0800, Lee, Chun-Yi 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.
> 
> Cc: David Howells <dhowells@...hat.com> 
> Cc: Josh Boyer <jwboyer@...oraproject.org>
> Signed-off-by: "Lee, Chun-Yi" <jlee@...e.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);

This isn't really a criticism of your patch but the underlying: all of
the *RT variables, like MokListRT are insecure runtime variables and
can be tampered with.  I can agree that I can't see a tamper attack
between exit boot services and pulling this in to the key list, but I'd
feel a lot happier if the values were obtained directly from the BS
variable before exit boot services because that's means the path for
getting the values is directly within the secure envelope and doesn't
rely on passing via an insecure element.

James

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ