diff mbox series

lib/security: hard_lockdown flag to stop runtime disable of signed boot

Message ID 1528666618-29496-1-git-send-email-brett.grandbois@opengear.com
State Accepted
Headers show
Series lib/security: hard_lockdown flag to stop runtime disable of signed boot | expand

Commit Message

Grandbois, Brett June 10, 2018, 9:36 p.m. UTC 
Currently if signed-boot is enabled in configure the presence of the
LOCKDOWN_FILE is used as a runtime determination to perform the actual
verification.  In some environments this may be acceptable or even the
intended operation but in other environments could be a security hole
since the removal of the file will then cause boot task verification.
Add a 'hard_lockdown' enable flag to generate a HARD_LOCKDOWN
preprocessor definition to force the system to always do a signed boot
verification for each boot task, which in the case of a missing file the
boot will fail.

Signed-off-by: Brett Grandbois <brett.grandbois@opengear.com>
---
 configure.ac                | 8 ++++++++
 lib/security/gpg.c          | 2 ++
 lib/security/openssl.c      | 4 +++-
 ui/ncurses/nc-boot-editor.c | 2 ++
 ui/ncurses/nc-cui.c         | 4 ++++
 5 files changed, 19 insertions(+), 1 deletion(-)

Comments

Sam Mendoza-Jonas June 12, 2018, 5:18 a.m. UTC | #1 
On Mon, 2018-06-11 at 07:36 +1000, Brett Grandbois wrote:
> Currently if signed-boot is enabled in configure the presence of the
> LOCKDOWN_FILE is used as a runtime determination to perform the actual
> verification.  In some environments this may be acceptable or even the
> intended operation but in other environments could be a security hole
> since the removal of the file will then cause boot task verification.
> Add a 'hard_lockdown' enable flag to generate a HARD_LOCKDOWN
> preprocessor definition to force the system to always do a signed boot
> verification for each boot task, which in the case of a missing file the
> boot will fail.
> 
> Signed-off-by: Brett Grandbois <brett.grandbois@opengear.com>

Sounds reasonable, merged as 18a47a31

> ---
>  configure.ac                | 8 ++++++++
>  lib/security/gpg.c          | 2 ++
>  lib/security/openssl.c      | 4 +++-
>  ui/ncurses/nc-boot-editor.c | 2 ++
>  ui/ncurses/nc-cui.c         | 4 ++++
>  5 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/configure.ac b/configure.ac
> index bdd7f70..e856bdc 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -239,6 +239,14 @@ AC_ARG_VAR(
>  AS_IF([test "x$VERIFY_DIGEST" = x], [VERIFY_DIGEST="sha256"])
>  AC_DEFINE_UNQUOTED(VERIFY_DIGEST, "$VERIFY_DIGEST", [openssl verify dgst])
>  
> +AC_ARG_ENABLE([hard-lockdown],
> +	      [AS_HELP_STRING([--enable-hard-lockdown],
> +			      [if signed boot configured, the absence of the
> +			       LOCKDOWN_FILE does not disable signed boot at
> +			       runtime @<:@default=no@:>@])],
> +	      [AC_DEFINE(HARD_LOCKDOWN, 1, [Enable hard lockdown])],
> +	      [])
> +
>  AC_ARG_ENABLE(
>  	[busybox],
>  	[AS_HELP_STRING(
> diff --git a/lib/security/gpg.c b/lib/security/gpg.c
> index 761d6ce..aae85aa 100644
> --- a/lib/security/gpg.c
> +++ b/lib/security/gpg.c
> @@ -354,8 +354,10 @@ int lockdown_status() {
>  	/* assume most restrictive lockdown type */
>  	int ret = PB_LOCKDOWN_SIGN;
>  
> +#if !defined(HARD_LOCKDOWN)
>  	if (access(LOCKDOWN_FILE, F_OK) == -1)
>  		return PB_LOCKDOWN_NONE;
> +#endif
>  
>  	/* determine lockdown type */
>  	FILE *authorized_signatures_handle = NULL;
> diff --git a/lib/security/openssl.c b/lib/security/openssl.c
> index 03ea332..6454f8a 100644
> --- a/lib/security/openssl.c
> +++ b/lib/security/openssl.c
> @@ -456,8 +456,10 @@ int lockdown_status(void)
>  	int ret = PB_LOCKDOWN_SIGN;
>  	PKCS12 *p12 = NULL;
>  
> +#if !defined(HARD_LOCKDOWN)
>  	if (access(LOCKDOWN_FILE, F_OK) == -1)
>  		return PB_LOCKDOWN_NONE;
> +#endif
>  
>  	/* determine lockdown type */
>  
> @@ -471,6 +473,6 @@ int lockdown_status(void)
>  		fclose(authorized_signatures_handle);
>  	}
>  
> -    return ret;
> +	return ret;
>  }
>  
> diff --git a/ui/ncurses/nc-boot-editor.c b/ui/ncurses/nc-boot-editor.c
> index 2e5749b..3f7c5e5 100644
> --- a/ui/ncurses/nc-boot-editor.c
> +++ b/ui/ncurses/nc-boot-editor.c
> @@ -637,9 +637,11 @@ struct boot_editor *boot_editor_init(struct cui *cui,
>  		return NULL;
>  
>  #if defined(SIGNED_BOOT)
> +#if !defined(HARD_LOCKDOWN)
>  	if (access(LOCKDOWN_FILE, F_OK) == -1)
>  		boot_editor->use_signature_files = false;
>  	else
> +#endif
>  		boot_editor->use_signature_files = true;
>  #else
>  	boot_editor->use_signature_files = false;
> diff --git a/ui/ncurses/nc-cui.c b/ui/ncurses/nc-cui.c
> index 20a9048..8a3f97d 100644
> --- a/ui/ncurses/nc-cui.c
> +++ b/ui/ncurses/nc-cui.c
> @@ -61,10 +61,14 @@ static void cui_cancel_autoboot_on_exit(struct cui *cui);
>  
>  static bool lockdown_active(void)
>  {
> +#if defined(SIGNED_BOOT) && defined(HARD_LOCKDOWN)
> +	return true;
> +#else
>  	bool lockdown = false;
>  	if (access(LOCKDOWN_FILE, F_OK) != -1)
>  		lockdown = true;
>  	return lockdown;
> +#endif
>  }
>  
>  static void cui_start(void)
diff mbox series

Patch

diff --git a/configure.ac b/configure.ac
index bdd7f70..e856bdc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -239,6 +239,14 @@  AC_ARG_VAR(
 AS_IF([test "x$VERIFY_DIGEST" = x], [VERIFY_DIGEST="sha256"])
 AC_DEFINE_UNQUOTED(VERIFY_DIGEST, "$VERIFY_DIGEST", [openssl verify dgst])
 
+AC_ARG_ENABLE([hard-lockdown],
+	      [AS_HELP_STRING([--enable-hard-lockdown],
+			      [if signed boot configured, the absence of the
+			       LOCKDOWN_FILE does not disable signed boot at
+			       runtime @<:@default=no@:>@])],
+	      [AC_DEFINE(HARD_LOCKDOWN, 1, [Enable hard lockdown])],
+	      [])
+
 AC_ARG_ENABLE(
 	[busybox],
 	[AS_HELP_STRING(
diff --git a/lib/security/gpg.c b/lib/security/gpg.c
index 761d6ce..aae85aa 100644
--- a/lib/security/gpg.c
+++ b/lib/security/gpg.c
@@ -354,8 +354,10 @@  int lockdown_status() {
 	/* assume most restrictive lockdown type */
 	int ret = PB_LOCKDOWN_SIGN;
 
+#if !defined(HARD_LOCKDOWN)
 	if (access(LOCKDOWN_FILE, F_OK) == -1)
 		return PB_LOCKDOWN_NONE;
+#endif
 
 	/* determine lockdown type */
 	FILE *authorized_signatures_handle = NULL;
diff --git a/lib/security/openssl.c b/lib/security/openssl.c
index 03ea332..6454f8a 100644
--- a/lib/security/openssl.c
+++ b/lib/security/openssl.c
@@ -456,8 +456,10 @@  int lockdown_status(void)
 	int ret = PB_LOCKDOWN_SIGN;
 	PKCS12 *p12 = NULL;
 
+#if !defined(HARD_LOCKDOWN)
 	if (access(LOCKDOWN_FILE, F_OK) == -1)
 		return PB_LOCKDOWN_NONE;
+#endif
 
 	/* determine lockdown type */
 
@@ -471,6 +473,6 @@  int lockdown_status(void)
 		fclose(authorized_signatures_handle);
 	}
 
-    return ret;
+	return ret;
 }
 
diff --git a/ui/ncurses/nc-boot-editor.c b/ui/ncurses/nc-boot-editor.c
index 2e5749b..3f7c5e5 100644
--- a/ui/ncurses/nc-boot-editor.c
+++ b/ui/ncurses/nc-boot-editor.c
@@ -637,9 +637,11 @@  struct boot_editor *boot_editor_init(struct cui *cui,
 		return NULL;
 
 #if defined(SIGNED_BOOT)
+#if !defined(HARD_LOCKDOWN)
 	if (access(LOCKDOWN_FILE, F_OK) == -1)
 		boot_editor->use_signature_files = false;
 	else
+#endif
 		boot_editor->use_signature_files = true;
 #else
 	boot_editor->use_signature_files = false;
diff --git a/ui/ncurses/nc-cui.c b/ui/ncurses/nc-cui.c
index 20a9048..8a3f97d 100644
--- a/ui/ncurses/nc-cui.c
+++ b/ui/ncurses/nc-cui.c
@@ -61,10 +61,14 @@  static void cui_cancel_autoboot_on_exit(struct cui *cui);
 
 static bool lockdown_active(void)
 {
+#if defined(SIGNED_BOOT) && defined(HARD_LOCKDOWN)
+	return true;
+#else
 	bool lockdown = false;
 	if (access(LOCKDOWN_FILE, F_OK) != -1)
 		lockdown = true;
 	return lockdown;
+#endif
 }
 
 static void cui_start(void)