[PATCH] discover/platform: platform boot args append after verification

Brett Grandbois brett.grandbois at opengear.com
Mon Jun 11 08:24:06 AEST 2018


In a signed-boot environment, the cmdline options are verified along
with the kernel, which is to be expected.  However this can cause
problems in environments where the system needs to make a runtime
adjustment to the cmdline args as now the signature will fail.  There
is currently boot hook support for adjusting args but that happens
before verification and will therefore fail, and also having a callout
to a possible modified script/app seems to be too much of a security
hole.  After some experimentation it appears the best solution to this
issue is to add a new platform callout that is invoked after
verification as part of the kexec cmdline args append.  Since the
platform(s) are compiled-in they can't be (easily) altered on the system
and the default NULL behavior of the platform system means that it must
be explicitly invoked by a platform designer, which would be an implicit
acknowledgement that the possible security hole is acceptable.
Runtime update of dtb's in a signed-boot environment will also face this
issue but that will be addressed separately.

Signed-off-by: Brett Grandbois <brett.grandbois at opengear.com>
---
 discover/boot.c     | 24 ++++++++++++++++++++----
 discover/platform.c |  7 +++++++
 discover/platform.h |  4 ++++
 3 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/discover/boot.c b/discover/boot.c
index 2a0d333..c66264a 100644
--- a/discover/boot.c
+++ b/discover/boot.c
@@ -62,6 +62,7 @@ static int kexec_load(struct boot_task *boot_task)
 	struct process *process;
 	char *s_initrd = NULL;
 	char *s_args = NULL;
+	char *p_args = NULL;
 	const char *argv[7];
 	char *s_dtb = NULL;
 	const char **p;
@@ -121,10 +122,19 @@ static int kexec_load(struct boot_task *boot_task)
 		*p++ = s_dtb;		 /* 4 */
 	}
 
-	s_args = talloc_asprintf(boot_task, "--append=%s",
-				boot_task->args ?: "\"\"");
-	assert(s_args);
-	*p++ = s_args;		/* 5 */
+	p_args = platform_boot_args_append(boot_task);
+
+	if (p_args || (boot_task->args && strlen(boot_task->args))) {
+		if (p_args && (boot_task->args && strlen(boot_task->args)))
+			s_args = talloc_asprintf(boot_task, "--append=%s %s",
+						boot_task->args, p_args);
+		else
+			s_args = talloc_asprintf(boot_task, "--append=%s",
+						p_args ?: boot_task->args);
+
+		assert(s_args);
+		*p++ = s_args;		/* 5 */
+	}
 
 	*p++ = local_image;		/* 6 */
 	*p++ = NULL;			/* 7 */
@@ -581,6 +591,12 @@ struct boot_task *boot(void *ctx, struct discover_boot_option *opt,
 			talloc_free(boot_task);
 			return NULL;
 		}
+		/*
+		 * no args is valid but the expectation is that the sigfile
+		 * will have hashed ""
+		 */
+		if (!boot_task->args)
+			boot_task->args = talloc_strdup(boot_task, "");
 	}
 
 	image_res = add_boot_resource(boot_task, _("kernel image"), image,
diff --git a/discover/platform.c b/discover/platform.c
index cc6306f..93cc4b7 100644
--- a/discover/platform.c
+++ b/discover/platform.c
@@ -209,6 +209,13 @@ int platform_get_sysinfo(struct system_info *info)
 	return -1;
 }
 
+char *platform_boot_args_append(const struct boot_task *task)
+{
+	if (platform && platform->boot_args_append)
+		return platform->boot_args_append(platform, task);
+	return NULL;
+}
+
 int config_set(struct config *newconfig)
 {
 	int rc;
diff --git a/discover/platform.h b/discover/platform.h
index 5aa8e3f..9960c7d 100644
--- a/discover/platform.h
+++ b/discover/platform.h
@@ -2,6 +2,7 @@
 #define PLATFORM_H
 
 #include <types/types.h>
+#include "boot.h"
 
 struct platform {
 	const char	*name;
@@ -11,6 +12,8 @@ struct platform {
 	void		(*pre_boot)(struct platform *,
 				const struct config *);
 	int		(*get_sysinfo)(struct platform *, struct system_info *);
+	char*		(*boot_args_append)(struct platform *,
+					    const struct boot_task*);
 	uint16_t	dhcp_arch_id;
 	void		*platform_data;
 };
@@ -20,6 +23,7 @@ int platform_fini(void);
 const struct platform *platform_get(void);
 int platform_get_sysinfo(struct system_info *info);
 void platform_pre_boot(void);
+char *platform_boot_args_append(const struct boot_task*);
 
 /* configuration interface */
 const struct config *config_get(void);
-- 
2.7.4



More information about the Petitboot mailing list