diff mbox

[3/3,V6] Add encrypted file support

Message ID 1753518218.78264.1471386482006.JavaMail.zimbra@raptorengineeringinc.com
State Superseded
Headers show

Commit Message

Timothy Pearson Aug. 16, 2016, 10:28 p.m. UTC 
In certain cases, such as network booting over an untrusted connection,
it may be useful to fully encrypt and sign the kernel files.

Enable fully encrypted boot using builtin keyring via the addition of
the string "ENCRYPTED" to the first line of the /etc/pb-lockdown file.
This disables detached (plaintext) signature verification.

Signed-off-by: Timothy Pearson <tpearson@raptorengineering.com>
---
 discover/boot.c             |  42 ++++++-
 discover/boot.h             |   2 +
 discover/gpg.c              | 294 +++++++++++++++++++++++++++++++++++++++-----
 discover/gpg.h              |   8 ++
 ui/ncurses/nc-boot-editor.c |   9 +-
 5 files changed, 319 insertions(+), 36 deletions(-)

Comments

Sam Mendoza-Jonas Aug. 17, 2016, 3:47 a.m. UTC | #1 
On Tue, 2016-08-16 at 17:28 -0500, Timothy Pearson wrote:
> In certain cases, such as network booting over an untrusted connection,
> it may be useful to fully encrypt and sign the kernel files.
> 
> Enable fully encrypted boot using builtin keyring via the addition of
> the string "ENCRYPTED" to the first line of the /etc/pb-lockdown file.
> This disables detached (plaintext) signature verification.
> 
> Signed-off-by: Timothy Pearson <tpearson@raptorengineering.com>
> ---
>  discover/boot.c             |  42 ++++++-
>  discover/boot.h             |   2 +
>  discover/gpg.c              | 294 +++++++++++++++++++++++++++++++++++++++-----
>  discover/gpg.h              |   8 ++
>  ui/ncurses/nc-boot-editor.c |   9 +-
>  5 files changed, 319 insertions(+), 36 deletions(-)
> 
> diff --git a/discover/boot.c b/discover/boot.c
> index 38be536..8fc4065 100644
> --- a/discover/boot.c
> +++ b/discover/boot.c
> @@ -51,9 +51,14 @@ static int kexec_load(struct boot_task *boot_task)
>  
>  	if ((result = gpg_validate_boot_files(boot_task, &local_initrd,
>  		&local_dtb, &local_image))) {
> -		if (result == KEXEC_LOAD_SIGNATURE_FAILURE) {
> +		if (result == KEXEC_LOAD_DECRYPTION_FALURE) {
>  			pb_log("%s: Aborting kexec due to"
> -				" signature verification failure\n", __func__);
> +				" decryption failure\n", __func__);
> +			goto abort_kexec;
> +		}
> +		if (result == KEXEC_LOAD_SIGNATURE_FAILURE) {
> +			pb_log("%s: Aborting kexec due to signature"
> +				" verification failure\n", __func__);
>  			goto abort_kexec;
>  		}
>  	}
> @@ -387,7 +392,13 @@ static void boot_process(struct load_url_result *result, void *data)
>  				load_pending(task->dtb_signature) ||
>  				load_pending(task->cmdline_signature))
>  			return;
> +	}
> +	if (task->decrypt_files) {
> +		if (load_pending(task->cmdline_signature))
> +			return;
> +	}
>  
> +	if (task->verify_signature) {
>  		if (check_load(task, "kernel image signature",
>  					task->image_signature) ||
>  				check_load(task, "initrd signature",
> @@ -398,6 +409,14 @@ static void boot_process(struct load_url_result *result, void *data)
>  					task->cmdline_signature))
>  			goto no_sig_load;
>  	}
> +	if (task->decrypt_files) {
> +		if (load_pending(task->cmdline_signature))
> +			return;
> +
> +		if (check_load(task, "command line signature",
> +					task->cmdline_signature))
> +			goto no_decrypt_sig_load;
> +	}
>  
>  	/* we make a copy of the local paths, as the boot hooks might update
>  	 * and/or create these */
> @@ -412,6 +431,8 @@ static void boot_process(struct load_url_result *result, void *data)
>  			task->initrd_signature->local : NULL;
>  		task->local_dtb_signature = task->dtb_signature ?
>  			task->dtb_signature->local : NULL;
> +	}
> +	if (task->verify_signature || task->decrypt_files) {
>  		task->local_cmdline_signature = task->cmdline_signature ?
>  			task->cmdline_signature->local : NULL;
>  	}
> @@ -422,7 +443,11 @@ static void boot_process(struct load_url_result *result, void *data)
>  			_("performing kexec_load"));
>  
>  	rc = kexec_load(task);
> -	if (rc == KEXEC_LOAD_SIGNATURE_FAILURE) {
> +	if (rc == KEXEC_LOAD_DECRYPTION_FALURE) {
> +		update_status(task->status_fn, task->status_arg,
> +				BOOT_STATUS_ERROR, _("decryption failed"));
> +	}
> +	else if (rc == KEXEC_LOAD_SIGNATURE_FAILURE) {
>  		update_status(task->status_fn, task->status_arg,
>  				BOOT_STATUS_ERROR,
>  				_("signature verification failed"));
> @@ -442,6 +467,8 @@ no_sig_load:
>  	cleanup_load(task->image_signature);
>  	cleanup_load(task->initrd_signature);
>  	cleanup_load(task->dtb_signature);
> +
> +no_decrypt_sig_load:
>  	cleanup_load(task->cmdline_signature);
>  
>  no_load:
> @@ -490,6 +517,7 @@ struct boot_task *boot(void *ctx, struct discover_boot_option *opt,
>  	struct boot_task *boot_task;
>  	const char *boot_desc;
>  	int rc;
> +	int lockdown_type;
>  
>  	if (opt && opt->option->name)
>  		boot_desc = opt->option->name;
> @@ -529,7 +557,9 @@ struct boot_task *boot(void *ctx, struct discover_boot_option *opt,
>  	boot_task->status_fn = status_fn;
>  	boot_task->status_arg = status_arg;
>  
> -	boot_task->verify_signature = (lockdown_status() == PB_LOCKDOWN_SIGN);
> +	lockdown_type = lockdown_status();
> +	boot_task->verify_signature = (lockdown_type == PB_LOCKDOWN_SIGN);
> +	boot_task->decrypt_files = (lockdown_type == PB_LOCKDOWN_DECRYPT);
>  
>  	if (cmd && cmd->boot_args) {
>  		boot_task->args = talloc_strdup(boot_task, cmd->boot_args);
> @@ -547,7 +577,7 @@ struct boot_task *boot(void *ctx, struct discover_boot_option *opt,
>  		boot_task->boot_tty = config ? config->boot_tty : NULL;
>  	}
>  
> -	if (boot_task->verify_signature) {
> +	if (boot_task->verify_signature || boot_task->decrypt_files) {
>  		if (cmd && cmd->args_sig_file) {
>  			cmdline_sig = pb_url_parse(opt, cmd->args_sig_file);
>  		} else if (opt && opt->args_sig_file) {
> @@ -603,7 +633,9 @@ struct boot_task *boot(void *ctx, struct discover_boot_option *opt,
>  			rc |= start_url_load(boot_task, "dtb signature",
>  				dtb_sig, &boot_task->dtb_signature);
>  		}
> +	}
>  
> +	if (boot_task->verify_signature || boot_task->decrypt_files) {
>  		rc |= start_url_load(boot_task,
>  			"kernel command line signature", cmdline_sig,
>  			&boot_task->cmdline_signature);
> diff --git a/discover/boot.h b/discover/boot.h
> index 5f6e874..72517f4 100644
> --- a/discover/boot.h
> +++ b/discover/boot.h
> @@ -26,6 +26,7 @@ struct boot_task {
>  	bool dry_run;
>  	bool cancelled;
>  	bool verify_signature;
> +	bool decrypt_files;
>  	struct load_url_result *image_signature;
>  	struct load_url_result *initrd_signature;
>  	struct load_url_result *dtb_signature;
> @@ -37,6 +38,7 @@ struct boot_task {
>  };
>  
>  enum {
> +	KEXEC_LOAD_DECRYPTION_FALURE = 252,
>  	KEXEC_LOAD_SIG_SETUP_INVALID = 253,
>  	KEXEC_LOAD_SIGNATURE_FAILURE = 254,
>  };
> diff --git a/discover/gpg.c b/discover/gpg.c
> index e1c1d04..d23d29c 100644
> --- a/discover/gpg.c
> +++ b/discover/gpg.c
> @@ -95,6 +95,173 @@ int copy_file_to_destination(const char * source_file,
>  	return result;
>  }
>  
> +int decrypt_file(const char * filename,
> +	FILE * authorized_signatures_handle, const char * keyring_path)
> +{
> +	int result = 0;
> +	int valid = 0;
> +	size_t bytes_read = 0;
> +	unsigned char buffer[8192];
> +
> +	if (filename == NULL)
> +		return -1;
> +
> +	gpgme_signature_t verification_signatures;
> +	gpgme_verify_result_t verification_result;
> +	gpgme_data_t ciphertext_data;
> +	gpgme_data_t plaintext_data;
> +	gpgme_engine_info_t enginfo;
> +	gpgme_ctx_t gpg_context;
> +	gpgme_error_t err;
> +
> +	/* Initialize gpgme */
> +	setlocale (LC_ALL, "");
> +	gpgme_check_version(NULL);
> +	gpgme_set_locale(NULL, LC_CTYPE, setlocale (LC_CTYPE, NULL));
> +	err = gpgme_engine_check_version(GPGME_PROTOCOL_OpenPGP);
> +	if (err != GPG_ERR_NO_ERROR) {
> +		pb_log("%s: OpenPGP support not available\n", __func__);
> +		return -1;
> +	}
> +	err = gpgme_get_engine_info(&enginfo);
> +	if (err != GPG_ERR_NO_ERROR) {
> +		pb_log("%s: GPG engine failed to initialize\n", __func__);
> +		return -1;
> +	}
> +	err = gpgme_new(&gpg_context);
> +	if (err != GPG_ERR_NO_ERROR) {
> +		pb_log("%s: GPG context could not be created\n", __func__);
> +		return -1;
> +	}
> +	err = gpgme_set_protocol(gpg_context, GPGME_PROTOCOL_OpenPGP);
> +	if (err != GPG_ERR_NO_ERROR) {
> +		pb_log("%s: GPG protocol could not be set\n", __func__);
> +		return -1;
> +	}
> +	if (keyring_path)
> +		err = gpgme_ctx_set_engine_info (gpg_context,
> +			GPGME_PROTOCOL_OpenPGP,
> +			enginfo->file_name, keyring_path);
> +	else
> +		err = gpgme_ctx_set_engine_info (gpg_context,
> +			GPGME_PROTOCOL_OpenPGP,
> +			enginfo->file_name, enginfo->home_dir);
> +	if (err != GPG_ERR_NO_ERROR) {
> +		pb_log("%s: Could not set GPG engine information\n", __func__);
> +		return -1;
> +	}
> +	err = gpgme_data_new(&plaintext_data);
> +	if (err != GPG_ERR_NO_ERROR) {
> +		pb_log("%s: Could not create GPG plaintext data buffer\n",
> +			__func__);
> +		return -1;
> +	}
> +	err = gpgme_data_new_from_file(&ciphertext_data, filename, 1);
> +	if (err != GPG_ERR_NO_ERROR) {
> +		pb_log("%s: Could not create GPG ciphertext data buffer"
> +			" from file '%s'\n", __func__, filename);
> +		return -1;
> +	}
> +
> +	/* Decrypt and verify file */
> +	err = gpgme_op_decrypt_verify(gpg_context, ciphertext_data,
> +		plaintext_data);
> +	if (err != GPG_ERR_NO_ERROR) {
> +		pb_log("%s: Could not decrypt file\n", __func__);
> +		return -1;
> +	}
> +	verification_result = gpgme_op_verify_result(gpg_context);
> +	verification_signatures = verification_result->signatures;
> +	while (verification_signatures) {
> +		if (verification_signatures->status == GPG_ERR_NO_ERROR) {
> +			pb_log("%s: Good signature for key ID '%s' ('%s')\n",
> +				__func__,
> +				verification_signatures->fpr, filename);
> +			/* Verify fingerprint is present in authorized
> +			 * signatures file
> +			 */
> +			char *auth_sig_line = NULL;
> +			size_t auth_sig_len = 0;
> +			ssize_t auth_sig_read;
> +			rewind(authorized_signatures_handle);
> +			while ((auth_sig_read = getline(&auth_sig_line,
> +				&auth_sig_len,
> +				authorized_signatures_handle)) != -1) {
> +				auth_sig_len = strlen(auth_sig_line);
> +				while ((auth_sig_line[auth_sig_len-1] == '\n')
> +					|| (auth_sig_line[auth_sig_len-1] == '\r'))
> +					auth_sig_len--;
> +				auth_sig_line[auth_sig_len] = 0;
> +				if (strcmp(auth_sig_line,
> +					verification_signatures->fpr) == 0)
> +					valid = 1;

Previous comment about strncmp() applies here too

> +			}
> +			free(auth_sig_line);
> +		}
> +		else {
> +			pb_log("%s: Signature for key ID '%s' ('%s') invalid."
> +				"  Status: %08x\n", __func__,
> +				verification_signatures->fpr, filename,
> +				verification_signatures->status);
> +		}
> +		verification_signatures = verification_signatures->next;
> +	}
> +
> +	gpgme_data_release(ciphertext_data);
> +
> +	if (valid) {
> +		/* Write decrypted file over ciphertext */
> +		FILE *plaintext_file_handle = NULL;
> +		plaintext_file_handle = fopen(filename, "wb");
> +		if (!plaintext_file_handle) {
> +			pb_log("%s: Could not create GPG plaintext file '%s'\n",
> +				__func__, filename);
> +			return -1;

Is some cleanup being skipped here?

> +		}
> +		gpgme_data_seek(plaintext_data, 0, SEEK_SET);
> +		if (err != GPG_ERR_NO_ERROR) {
> +			pb_log("%s: Could not seek in GPG plaintext buffer\n",
> +				__func__);
> +			return -1;
> +		}
> +		while ((bytes_read = gpgme_data_read(plaintext_data, buffer,
> +			8192)) > 0) {
> +			size_t l2 = fwrite(buffer, 1, bytes_read,
> +				plaintext_file_handle);
> +			if (l2 < bytes_read) {
> +				if (ferror(plaintext_file_handle)) {
> +					/* General error */
> +					result = -1;
> +					pb_log("%s: failed: unknown fault\n",
> +						__func__);
> +				}

Same comment from before about break-ing

> +				else {
> +					/* No space on destination device */
> +					result = -1;
> +					pb_log("%s: failed: temporary storage"
> +						" full\n", __func__);
> +				}
> +			}
> +		}
> +		fclose(plaintext_file_handle);
> +	}
> +
> +	/* Clean up */
> +	gpgme_data_release(plaintext_data);
> +	gpgme_release(gpg_context);
> +
> +	if (!valid) {
> +		pb_log("%s: Incorrect GPG signature\n", __func__);
> +		return -1;
> +	}
> +	else {

No need for else {}. Does it make sense to return an error before the
copy loop above?

> +		pb_log("%s: GPG signature for decrypted file '%s' verified\n",
> +			__func__, filename);
> +	}
> +
> +	return result;
> +}
> +
>  int verify_file_signature(const char * plaintext_filename,
>  	const char * signature_filename, FILE * authorized_signatures_handle,
>  	const char * keyring_path)
> @@ -235,10 +402,11 @@ int gpg_validate_boot_files(struct boot_task *boot_task,
>  		boot_task->local_dtb_signature : NULL;
>  	const char* local_image_signature = (boot_task->verify_signature) ?
>  		boot_task->local_image_signature : NULL;
> -	const char* local_cmdline_signature = (boot_task->verify_signature) ?
> +	const char* local_cmdline_signature =
> +		(boot_task->verify_signature || boot_task->decrypt_files) ?
>  		boot_task->local_cmdline_signature : NULL;
>  
> -	if (boot_task->verify_signature) {
> +	if ((boot_task->verify_signature) || (boot_task->decrypt_files)) {
>  		char kernel_filename[MAX_FILENAME_SIZE];
>  		char initrd_filename[MAX_FILENAME_SIZE];
>  		char dtb_filename[MAX_FILENAME_SIZE];
> @@ -324,32 +492,67 @@ int gpg_validate_boot_files(struct boot_task *boot_task,
>  			fflush(cmdline_handle);
>  		}
>  
> -		/* Check signatures */
> -		if (verify_file_signature(kernel_filename,
> -			local_image_signature,
> -			authorized_signatures_handle, "/etc/gpg"))
> -			result = KEXEC_LOAD_SIGNATURE_FAILURE;
> -		if (verify_file_signature(cmdline_template,
> -			local_cmdline_signature,
> -			authorized_signatures_handle, "/etc/gpg"))
> -			result = KEXEC_LOAD_SIGNATURE_FAILURE;
> -		if (boot_task->local_initrd_signature)
> -			if (verify_file_signature(initrd_filename,
> -				local_initrd_signature,
> -				authorized_signatures_handle, "/etc/gpg"))
> +		if (boot_task->verify_signature) {
> +			/* Check signatures */
> +			if (verify_file_signature(kernel_filename,
> +				local_image_signature,
> +				authorized_signatures_handle,
> +				"/etc/gpg"))
>  				result = KEXEC_LOAD_SIGNATURE_FAILURE;
> -		if (boot_task->local_dtb_signature)
> -			if (verify_file_signature(dtb_filename,
> -				local_dtb_signature,
> -				authorized_signatures_handle, "/etc/gpg"))
> +			if (verify_file_signature(cmdline_template,
> +				local_cmdline_signature,
> +				authorized_signatures_handle,
> +				"/etc/gpg"))
>  				result = KEXEC_LOAD_SIGNATURE_FAILURE;
> -
> -		/* Clean up */
> -		if (cmdline_handle) {
> -			fclose(cmdline_handle);
> -			unlink(cmdline_template);
> +			if (boot_task->local_initrd_signature)
> +				if (verify_file_signature(initrd_filename,
> +					local_initrd_signature,
> +					authorized_signatures_handle,
> +					"/etc/gpg"))
> +					result = KEXEC_LOAD_SIGNATURE_FAILURE;
> +			if (boot_task->local_dtb_signature)
> +				if (verify_file_signature(dtb_filename,
> +					local_dtb_signature,
> +					authorized_signatures_handle,
> +					"/etc/gpg"))
> +					result = KEXEC_LOAD_SIGNATURE_FAILURE;
> +
> +			/* Clean up */
> +			if (cmdline_handle) {
> +				fclose(cmdline_handle);
> +				unlink(cmdline_template);
> +			}
> +			fclose(authorized_signatures_handle);
> +		}
> +		else if (boot_task->decrypt_files) {

if () {
	// foo
} else {
	// bar
}

> +			/* Decrypt files */
> +			if (decrypt_file(kernel_filename,
> +				authorized_signatures_handle,
> +				"/etc/gpg"))
> +				result = KEXEC_LOAD_DECRYPTION_FALURE;
> +			if (verify_file_signature(cmdline_template,
> +				local_cmdline_signature,
> +				authorized_signatures_handle,
> +				"/etc/gpg"))
> +				result = KEXEC_LOAD_SIGNATURE_FAILURE;
> +			if (boot_task->local_initrd)
> +				if (decrypt_file(initrd_filename,
> +					authorized_signatures_handle,
> +					"/etc/gpg"))
> +					result = KEXEC_LOAD_DECRYPTION_FALURE;
> +			if (boot_task->local_dtb)
> +				if (decrypt_file(dtb_filename,
> +					authorized_signatures_handle,
> +					"/etc/gpg"))
> +					result = KEXEC_LOAD_DECRYPTION_FALURE;
> +
> +			/* Clean up */
> +			if (cmdline_handle) {
> +				fclose(cmdline_handle);
> +				unlink(cmdline_template);
> +			}
> +			fclose(authorized_signatures_handle);
>  		}
> -		fclose(authorized_signatures_handle);
>  	}
>  
>  	return result;
> @@ -358,7 +561,7 @@ int gpg_validate_boot_files(struct boot_task *boot_task,
>  void gpg_validate_boot_files_cleanup(struct boot_task *boot_task,
>  	const char** local_initrd, const char** local_dtb,
>  	const char** local_image) {
> -	if (boot_task->verify_signature) {
> +	if ((boot_task->verify_signature) || (boot_task->decrypt_files)) {
>  		unlink(*local_image);
>  		if (*local_initrd)
>  			unlink(*local_initrd);
> @@ -374,8 +577,39 @@ void gpg_validate_boot_files_cleanup(struct boot_task *boot_task,
>  }
>  
>  int lockdown_status() {
> -	if (access(LOCKDOWN_FILE, F_OK) == -1)
> -		return PB_LOCKDOWN_NONE;
> -	else
> -		return PB_LOCKDOWN_SIGN;
> +	/* assume most restrictive lockdown type */
> +	int ret = PB_LOCKDOWN_SIGN;
> +
> +	if (access(LOCKDOWN_FILE, F_OK) == -1) {
> +		ret = PB_LOCKDOWN_NONE;
> +	}
> +	else {
> +		/* determine lockdown type */
> +		FILE *authorized_signatures_handle = NULL;
> +		authorized_signatures_handle = fopen(LOCKDOWN_FILE, "r");
> +		if (authorized_signatures_handle) {
> +			char *auth_sig_line = NULL;
> +			size_t auth_sig_len = 0;
> +			ssize_t auth_sig_read;
> +			rewind(authorized_signatures_handle);
> +			if ((auth_sig_read = getline(&auth_sig_line,
> +				&auth_sig_len,
> +				authorized_signatures_handle)) != -1) {
> +				auth_sig_len = strlen(auth_sig_line);
> +				while ((auth_sig_line[auth_sig_len-1] == '\n')
> +					|| (auth_sig_line[auth_sig_len-1] == '\r'))
> +					auth_sig_len--;
> +				auth_sig_line[auth_sig_len] = 0;
> +				if (strcmp(auth_sig_line, "ENCRYPTED") == 0) {
> +					/* first line indicates encrypted files
> +					 * expected.  enable decryption.
> +					 */
> +					ret = PB_LOCKDOWN_DECRYPT;

Same comment about strncmp()

> +				}
> +			}
> +			free(auth_sig_line);
> +		}
> +	}
> +
> +	return ret;
>  }
> \ No newline at end of file
> diff --git a/discover/gpg.h b/discover/gpg.h
> index 9b452b8..0194236 100644
> --- a/discover/gpg.h
> +++ b/discover/gpg.h
> @@ -26,6 +26,7 @@
>  enum {
>  	PB_LOCKDOWN_NONE	= 0,
>  	PB_LOCKDOWN_SIGN	= 1,
> +	PB_LOCKDOWN_DECRYPT	= 2,
>  };
>  
>  #if defined(HAVE_LIBGPGME)
> @@ -41,6 +42,9 @@ int verify_file_signature(const char * plaintext_filename,
>  	const char * signature_filename, FILE * authorized_signatures_handle,
>  	const char * keyring_path);
>  
> +int decrypt_file(const char * filename,
> +	FILE * authorized_signatures_handle, const char * keyring_path);
> +
>  int gpg_validate_boot_files(struct boot_task *boot_task,
>  	const char** local_initrd, const char** local_dtb,
>  	const char** local_image);
> @@ -60,6 +64,10 @@ int verify_file_signature(const char * plaintext_filename,
>  	const char * signature_filename, FILE * authorized_signatures_handle,
>  	const char * keyring_path) { return -1; }
>  
> +int decrypt_file(const char * filename,
> +	FILE * authorized_signatures_handle, const char * keyring_path)
> +	{ return -1; }
> +
>  int gpg_validate_boot_files(struct boot_task *boot_task,
>  	const char** local_initrd, const char** local_dtb,
>  	const char** local_image) { return 0; }
> diff --git a/ui/ncurses/nc-boot-editor.c b/ui/ncurses/nc-boot-editor.c
> index 9788fef..13fd6e5 100644
> --- a/ui/ncurses/nc-boot-editor.c
> +++ b/ui/ncurses/nc-boot-editor.c
> @@ -208,6 +208,9 @@ static struct pb_boot_data *boot_editor_prepare_data(
>  			boot_editor->widgets.args_sig_file_f);
>  		bd->args_sig_file = conditional_prefix(bd, prefix, s);
>  	}
> +	else {
> +		bd->args_sig_file = NULL;
> +	}
>  
>  	return bd;
>  }
> @@ -532,6 +535,10 @@ static void boot_editor_setup_widgets(struct boot_editor *boot_editor,
>  		boot_editor->widgets.args_sig_file_f = widget_new_textbox(set,
>  				0, 0, field_size, boot_editor->args_sig_file);
>  	}
> +	else {
> +		boot_editor->widgets.args_sig_file_l = NULL;
> +		boot_editor->widgets.args_sig_file_f = NULL;

These nc-boot-editor changes look like they should be in the first patch?

> +	}
>  
>  	boot_editor->widgets.ok_b = widget_new_button(set, 0, 0, 10,
>  					_("OK"), ok_click, boot_editor);
> @@ -629,7 +636,7 @@ struct boot_editor *boot_editor_init(struct cui *cui,
>  		if (boot_editor->use_signature_files)
>  			boot_editor->args_sig_file = bd->args_sig_file;
>  		else
> -			boot_editor->args_sig_file = "";
> +			boot_editor->args_sig_file = talloc_strdup(bd, "");
>  		boot_editor_find_device(boot_editor, bd, sysinfo);
>  	} else {
>  		boot_editor->image = boot_editor->initrd =
Timothy Pearson Aug. 18, 2016, 9:48 a.m. UTC | #2 
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 08/16/2016 10:47 PM, Samuel Mendoza-Jonas wrote:
> Previous comment about strncmp() applies here too

Previous comment about this comment applies here too. :-)

> Is some cleanup being skipped here?

Yep!  Fixed in patch V7.

> Same comment from before about break-ing

Fixed in patch V7.

> No need for else {}. Does it make sense to return an error before the
> copy loop above?

Fixed in patch V7.  I'd prefer to leave the copy / error sequence as-is
for future flexibility.

> if () {
> 	// foo
> } else {
> 	// bar
> }

This particular instance was fixed in patch V7.  Not sure if any other
instances remain; this is not our normal coding style so there may be
other examples.

> Same comment about strncmp()

Same comment to the comment.

> These nc-boot-editor changes look like they should be in the first patch?

Oops!  Fixed in patch V7.

- -- 
Timothy Pearson
Raptor Engineering
+1 (415) 727-8645 (direct line)
+1 (512) 690-0200 (switchboard)
https://www.raptorengineering.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJXtYRqAAoJEK+E3vEXDOFbBAwH/1voxNnsHPSX7O2d5y7osgO3
yWJ7ZyMxt4n9YbGdjB8kUWJwwc0d2SZXC8+40eQnHkHTSVzbXaufASFHLQKW148a
v3eYpU2/SVF0Pec8bQykRgx7dNFd/NXUIz+ONZj1+EEvuSoD8LQKI9KnON254NRj
Hsb7nhkNsF8Nl6/OkYX+3csGvMh1D2rHub0n3WMRM56VKHz7jr9DXDsUfXmc2+9/
sv9k3qi7Q16LuT/m3iVbxYOXt+4mnhNYxQC1Plyw0b4oGVNOT6PVhKt0dTVEYbEo
K5rF/ny8aSNEe4NfW/1iT4G9rhg7K1eUeXffnE6s0YkdfKwdGPMrTf9GTXi9Q0Y=
=az3Y
-----END PGP SIGNATURE-----
diff mbox

Patch

diff --git a/discover/boot.c b/discover/boot.c
index 38be536..8fc4065 100644
--- a/discover/boot.c
+++ b/discover/boot.c
@@ -51,9 +51,14 @@  static int kexec_load(struct boot_task *boot_task)
 
 	if ((result = gpg_validate_boot_files(boot_task, &local_initrd,
 		&local_dtb, &local_image))) {
-		if (result == KEXEC_LOAD_SIGNATURE_FAILURE) {
+		if (result == KEXEC_LOAD_DECRYPTION_FALURE) {
 			pb_log("%s: Aborting kexec due to"
-				" signature verification failure\n", __func__);
+				" decryption failure\n", __func__);
+			goto abort_kexec;
+		}
+		if (result == KEXEC_LOAD_SIGNATURE_FAILURE) {
+			pb_log("%s: Aborting kexec due to signature"
+				" verification failure\n", __func__);
 			goto abort_kexec;
 		}
 	}
@@ -387,7 +392,13 @@  static void boot_process(struct load_url_result *result, void *data)
 				load_pending(task->dtb_signature) ||
 				load_pending(task->cmdline_signature))
 			return;
+	}
+	if (task->decrypt_files) {
+		if (load_pending(task->cmdline_signature))
+			return;
+	}
 
+	if (task->verify_signature) {
 		if (check_load(task, "kernel image signature",
 					task->image_signature) ||
 				check_load(task, "initrd signature",
@@ -398,6 +409,14 @@  static void boot_process(struct load_url_result *result, void *data)
 					task->cmdline_signature))
 			goto no_sig_load;
 	}
+	if (task->decrypt_files) {
+		if (load_pending(task->cmdline_signature))
+			return;
+
+		if (check_load(task, "command line signature",
+					task->cmdline_signature))
+			goto no_decrypt_sig_load;
+	}
 
 	/* we make a copy of the local paths, as the boot hooks might update
 	 * and/or create these */
@@ -412,6 +431,8 @@  static void boot_process(struct load_url_result *result, void *data)
 			task->initrd_signature->local : NULL;
 		task->local_dtb_signature = task->dtb_signature ?
 			task->dtb_signature->local : NULL;
+	}
+	if (task->verify_signature || task->decrypt_files) {
 		task->local_cmdline_signature = task->cmdline_signature ?
 			task->cmdline_signature->local : NULL;
 	}
@@ -422,7 +443,11 @@  static void boot_process(struct load_url_result *result, void *data)
 			_("performing kexec_load"));
 
 	rc = kexec_load(task);
-	if (rc == KEXEC_LOAD_SIGNATURE_FAILURE) {
+	if (rc == KEXEC_LOAD_DECRYPTION_FALURE) {
+		update_status(task->status_fn, task->status_arg,
+				BOOT_STATUS_ERROR, _("decryption failed"));
+	}
+	else if (rc == KEXEC_LOAD_SIGNATURE_FAILURE) {
 		update_status(task->status_fn, task->status_arg,
 				BOOT_STATUS_ERROR,
 				_("signature verification failed"));
@@ -442,6 +467,8 @@  no_sig_load:
 	cleanup_load(task->image_signature);
 	cleanup_load(task->initrd_signature);
 	cleanup_load(task->dtb_signature);
+
+no_decrypt_sig_load:
 	cleanup_load(task->cmdline_signature);
 
 no_load:
@@ -490,6 +517,7 @@  struct boot_task *boot(void *ctx, struct discover_boot_option *opt,
 	struct boot_task *boot_task;
 	const char *boot_desc;
 	int rc;
+	int lockdown_type;
 
 	if (opt && opt->option->name)
 		boot_desc = opt->option->name;
@@ -529,7 +557,9 @@  struct boot_task *boot(void *ctx, struct discover_boot_option *opt,
 	boot_task->status_fn = status_fn;
 	boot_task->status_arg = status_arg;
 
-	boot_task->verify_signature = (lockdown_status() == PB_LOCKDOWN_SIGN);
+	lockdown_type = lockdown_status();
+	boot_task->verify_signature = (lockdown_type == PB_LOCKDOWN_SIGN);
+	boot_task->decrypt_files = (lockdown_type == PB_LOCKDOWN_DECRYPT);
 
 	if (cmd && cmd->boot_args) {
 		boot_task->args = talloc_strdup(boot_task, cmd->boot_args);
@@ -547,7 +577,7 @@  struct boot_task *boot(void *ctx, struct discover_boot_option *opt,
 		boot_task->boot_tty = config ? config->boot_tty : NULL;
 	}
 
-	if (boot_task->verify_signature) {
+	if (boot_task->verify_signature || boot_task->decrypt_files) {
 		if (cmd && cmd->args_sig_file) {
 			cmdline_sig = pb_url_parse(opt, cmd->args_sig_file);
 		} else if (opt && opt->args_sig_file) {
@@ -603,7 +633,9 @@  struct boot_task *boot(void *ctx, struct discover_boot_option *opt,
 			rc |= start_url_load(boot_task, "dtb signature",
 				dtb_sig, &boot_task->dtb_signature);
 		}
+	}
 
+	if (boot_task->verify_signature || boot_task->decrypt_files) {
 		rc |= start_url_load(boot_task,
 			"kernel command line signature", cmdline_sig,
 			&boot_task->cmdline_signature);
diff --git a/discover/boot.h b/discover/boot.h
index 5f6e874..72517f4 100644
--- a/discover/boot.h
+++ b/discover/boot.h
@@ -26,6 +26,7 @@  struct boot_task {
 	bool dry_run;
 	bool cancelled;
 	bool verify_signature;
+	bool decrypt_files;
 	struct load_url_result *image_signature;
 	struct load_url_result *initrd_signature;
 	struct load_url_result *dtb_signature;
@@ -37,6 +38,7 @@  struct boot_task {
 };
 
 enum {
+	KEXEC_LOAD_DECRYPTION_FALURE = 252,
 	KEXEC_LOAD_SIG_SETUP_INVALID = 253,
 	KEXEC_LOAD_SIGNATURE_FAILURE = 254,
 };
diff --git a/discover/gpg.c b/discover/gpg.c
index e1c1d04..d23d29c 100644
--- a/discover/gpg.c
+++ b/discover/gpg.c
@@ -95,6 +95,173 @@  int copy_file_to_destination(const char * source_file,
 	return result;
 }
 
+int decrypt_file(const char * filename,
+	FILE * authorized_signatures_handle, const char * keyring_path)
+{
+	int result = 0;
+	int valid = 0;
+	size_t bytes_read = 0;
+	unsigned char buffer[8192];
+
+	if (filename == NULL)
+		return -1;
+
+	gpgme_signature_t verification_signatures;
+	gpgme_verify_result_t verification_result;
+	gpgme_data_t ciphertext_data;
+	gpgme_data_t plaintext_data;
+	gpgme_engine_info_t enginfo;
+	gpgme_ctx_t gpg_context;
+	gpgme_error_t err;
+
+	/* Initialize gpgme */
+	setlocale (LC_ALL, "");
+	gpgme_check_version(NULL);
+	gpgme_set_locale(NULL, LC_CTYPE, setlocale (LC_CTYPE, NULL));
+	err = gpgme_engine_check_version(GPGME_PROTOCOL_OpenPGP);
+	if (err != GPG_ERR_NO_ERROR) {
+		pb_log("%s: OpenPGP support not available\n", __func__);
+		return -1;
+	}
+	err = gpgme_get_engine_info(&enginfo);
+	if (err != GPG_ERR_NO_ERROR) {
+		pb_log("%s: GPG engine failed to initialize\n", __func__);
+		return -1;
+	}
+	err = gpgme_new(&gpg_context);
+	if (err != GPG_ERR_NO_ERROR) {
+		pb_log("%s: GPG context could not be created\n", __func__);
+		return -1;
+	}
+	err = gpgme_set_protocol(gpg_context, GPGME_PROTOCOL_OpenPGP);
+	if (err != GPG_ERR_NO_ERROR) {
+		pb_log("%s: GPG protocol could not be set\n", __func__);
+		return -1;
+	}
+	if (keyring_path)
+		err = gpgme_ctx_set_engine_info (gpg_context,
+			GPGME_PROTOCOL_OpenPGP,
+			enginfo->file_name, keyring_path);
+	else
+		err = gpgme_ctx_set_engine_info (gpg_context,
+			GPGME_PROTOCOL_OpenPGP,
+			enginfo->file_name, enginfo->home_dir);
+	if (err != GPG_ERR_NO_ERROR) {
+		pb_log("%s: Could not set GPG engine information\n", __func__);
+		return -1;
+	}
+	err = gpgme_data_new(&plaintext_data);
+	if (err != GPG_ERR_NO_ERROR) {
+		pb_log("%s: Could not create GPG plaintext data buffer\n",
+			__func__);
+		return -1;
+	}
+	err = gpgme_data_new_from_file(&ciphertext_data, filename, 1);
+	if (err != GPG_ERR_NO_ERROR) {
+		pb_log("%s: Could not create GPG ciphertext data buffer"
+			" from file '%s'\n", __func__, filename);
+		return -1;
+	}
+
+	/* Decrypt and verify file */
+	err = gpgme_op_decrypt_verify(gpg_context, ciphertext_data,
+		plaintext_data);
+	if (err != GPG_ERR_NO_ERROR) {
+		pb_log("%s: Could not decrypt file\n", __func__);
+		return -1;
+	}
+	verification_result = gpgme_op_verify_result(gpg_context);
+	verification_signatures = verification_result->signatures;
+	while (verification_signatures) {
+		if (verification_signatures->status == GPG_ERR_NO_ERROR) {
+			pb_log("%s: Good signature for key ID '%s' ('%s')\n",
+				__func__,
+				verification_signatures->fpr, filename);
+			/* Verify fingerprint is present in authorized
+			 * signatures file
+			 */
+			char *auth_sig_line = NULL;
+			size_t auth_sig_len = 0;
+			ssize_t auth_sig_read;
+			rewind(authorized_signatures_handle);
+			while ((auth_sig_read = getline(&auth_sig_line,
+				&auth_sig_len,
+				authorized_signatures_handle)) != -1) {
+				auth_sig_len = strlen(auth_sig_line);
+				while ((auth_sig_line[auth_sig_len-1] == '\n')
+					|| (auth_sig_line[auth_sig_len-1] == '\r'))
+					auth_sig_len--;
+				auth_sig_line[auth_sig_len] = 0;
+				if (strcmp(auth_sig_line,
+					verification_signatures->fpr) == 0)
+					valid = 1;
+			}
+			free(auth_sig_line);
+		}
+		else {
+			pb_log("%s: Signature for key ID '%s' ('%s') invalid."
+				"  Status: %08x\n", __func__,
+				verification_signatures->fpr, filename,
+				verification_signatures->status);
+		}
+		verification_signatures = verification_signatures->next;
+	}
+
+	gpgme_data_release(ciphertext_data);
+
+	if (valid) {
+		/* Write decrypted file over ciphertext */
+		FILE *plaintext_file_handle = NULL;
+		plaintext_file_handle = fopen(filename, "wb");
+		if (!plaintext_file_handle) {
+			pb_log("%s: Could not create GPG plaintext file '%s'\n",
+				__func__, filename);
+			return -1;
+		}
+		gpgme_data_seek(plaintext_data, 0, SEEK_SET);
+		if (err != GPG_ERR_NO_ERROR) {
+			pb_log("%s: Could not seek in GPG plaintext buffer\n",
+				__func__);
+			return -1;
+		}
+		while ((bytes_read = gpgme_data_read(plaintext_data, buffer,
+			8192)) > 0) {
+			size_t l2 = fwrite(buffer, 1, bytes_read,
+				plaintext_file_handle);
+			if (l2 < bytes_read) {
+				if (ferror(plaintext_file_handle)) {
+					/* General error */
+					result = -1;
+					pb_log("%s: failed: unknown fault\n",
+						__func__);
+				}
+				else {
+					/* No space on destination device */
+					result = -1;
+					pb_log("%s: failed: temporary storage"
+						" full\n", __func__);
+				}
+			}
+		}
+		fclose(plaintext_file_handle);
+	}
+
+	/* Clean up */
+	gpgme_data_release(plaintext_data);
+	gpgme_release(gpg_context);
+
+	if (!valid) {
+		pb_log("%s: Incorrect GPG signature\n", __func__);
+		return -1;
+	}
+	else {
+		pb_log("%s: GPG signature for decrypted file '%s' verified\n",
+			__func__, filename);
+	}
+
+	return result;
+}
+
 int verify_file_signature(const char * plaintext_filename,
 	const char * signature_filename, FILE * authorized_signatures_handle,
 	const char * keyring_path)
@@ -235,10 +402,11 @@  int gpg_validate_boot_files(struct boot_task *boot_task,
 		boot_task->local_dtb_signature : NULL;
 	const char* local_image_signature = (boot_task->verify_signature) ?
 		boot_task->local_image_signature : NULL;
-	const char* local_cmdline_signature = (boot_task->verify_signature) ?
+	const char* local_cmdline_signature =
+		(boot_task->verify_signature || boot_task->decrypt_files) ?
 		boot_task->local_cmdline_signature : NULL;
 
-	if (boot_task->verify_signature) {
+	if ((boot_task->verify_signature) || (boot_task->decrypt_files)) {
 		char kernel_filename[MAX_FILENAME_SIZE];
 		char initrd_filename[MAX_FILENAME_SIZE];
 		char dtb_filename[MAX_FILENAME_SIZE];
@@ -324,32 +492,67 @@  int gpg_validate_boot_files(struct boot_task *boot_task,
 			fflush(cmdline_handle);
 		}
 
-		/* Check signatures */
-		if (verify_file_signature(kernel_filename,
-			local_image_signature,
-			authorized_signatures_handle, "/etc/gpg"))
-			result = KEXEC_LOAD_SIGNATURE_FAILURE;
-		if (verify_file_signature(cmdline_template,
-			local_cmdline_signature,
-			authorized_signatures_handle, "/etc/gpg"))
-			result = KEXEC_LOAD_SIGNATURE_FAILURE;
-		if (boot_task->local_initrd_signature)
-			if (verify_file_signature(initrd_filename,
-				local_initrd_signature,
-				authorized_signatures_handle, "/etc/gpg"))
+		if (boot_task->verify_signature) {
+			/* Check signatures */
+			if (verify_file_signature(kernel_filename,
+				local_image_signature,
+				authorized_signatures_handle,
+				"/etc/gpg"))
 				result = KEXEC_LOAD_SIGNATURE_FAILURE;
-		if (boot_task->local_dtb_signature)
-			if (verify_file_signature(dtb_filename,
-				local_dtb_signature,
-				authorized_signatures_handle, "/etc/gpg"))
+			if (verify_file_signature(cmdline_template,
+				local_cmdline_signature,
+				authorized_signatures_handle,
+				"/etc/gpg"))
 				result = KEXEC_LOAD_SIGNATURE_FAILURE;
-
-		/* Clean up */
-		if (cmdline_handle) {
-			fclose(cmdline_handle);
-			unlink(cmdline_template);
+			if (boot_task->local_initrd_signature)
+				if (verify_file_signature(initrd_filename,
+					local_initrd_signature,
+					authorized_signatures_handle,
+					"/etc/gpg"))
+					result = KEXEC_LOAD_SIGNATURE_FAILURE;
+			if (boot_task->local_dtb_signature)
+				if (verify_file_signature(dtb_filename,
+					local_dtb_signature,
+					authorized_signatures_handle,
+					"/etc/gpg"))
+					result = KEXEC_LOAD_SIGNATURE_FAILURE;
+
+			/* Clean up */
+			if (cmdline_handle) {
+				fclose(cmdline_handle);
+				unlink(cmdline_template);
+			}
+			fclose(authorized_signatures_handle);
+		}
+		else if (boot_task->decrypt_files) {
+			/* Decrypt files */
+			if (decrypt_file(kernel_filename,
+				authorized_signatures_handle,
+				"/etc/gpg"))
+				result = KEXEC_LOAD_DECRYPTION_FALURE;
+			if (verify_file_signature(cmdline_template,
+				local_cmdline_signature,
+				authorized_signatures_handle,
+				"/etc/gpg"))
+				result = KEXEC_LOAD_SIGNATURE_FAILURE;
+			if (boot_task->local_initrd)
+				if (decrypt_file(initrd_filename,
+					authorized_signatures_handle,
+					"/etc/gpg"))
+					result = KEXEC_LOAD_DECRYPTION_FALURE;
+			if (boot_task->local_dtb)
+				if (decrypt_file(dtb_filename,
+					authorized_signatures_handle,
+					"/etc/gpg"))
+					result = KEXEC_LOAD_DECRYPTION_FALURE;
+
+			/* Clean up */
+			if (cmdline_handle) {
+				fclose(cmdline_handle);
+				unlink(cmdline_template);
+			}
+			fclose(authorized_signatures_handle);
 		}
-		fclose(authorized_signatures_handle);
 	}
 
 	return result;
@@ -358,7 +561,7 @@  int gpg_validate_boot_files(struct boot_task *boot_task,
 void gpg_validate_boot_files_cleanup(struct boot_task *boot_task,
 	const char** local_initrd, const char** local_dtb,
 	const char** local_image) {
-	if (boot_task->verify_signature) {
+	if ((boot_task->verify_signature) || (boot_task->decrypt_files)) {
 		unlink(*local_image);
 		if (*local_initrd)
 			unlink(*local_initrd);
@@ -374,8 +577,39 @@  void gpg_validate_boot_files_cleanup(struct boot_task *boot_task,
 }
 
 int lockdown_status() {
-	if (access(LOCKDOWN_FILE, F_OK) == -1)
-		return PB_LOCKDOWN_NONE;
-	else
-		return PB_LOCKDOWN_SIGN;
+	/* assume most restrictive lockdown type */
+	int ret = PB_LOCKDOWN_SIGN;
+
+	if (access(LOCKDOWN_FILE, F_OK) == -1) {
+		ret = PB_LOCKDOWN_NONE;
+	}
+	else {
+		/* determine lockdown type */
+		FILE *authorized_signatures_handle = NULL;
+		authorized_signatures_handle = fopen(LOCKDOWN_FILE, "r");
+		if (authorized_signatures_handle) {
+			char *auth_sig_line = NULL;
+			size_t auth_sig_len = 0;
+			ssize_t auth_sig_read;
+			rewind(authorized_signatures_handle);
+			if ((auth_sig_read = getline(&auth_sig_line,
+				&auth_sig_len,
+				authorized_signatures_handle)) != -1) {
+				auth_sig_len = strlen(auth_sig_line);
+				while ((auth_sig_line[auth_sig_len-1] == '\n')
+					|| (auth_sig_line[auth_sig_len-1] == '\r'))
+					auth_sig_len--;
+				auth_sig_line[auth_sig_len] = 0;
+				if (strcmp(auth_sig_line, "ENCRYPTED") == 0) {
+					/* first line indicates encrypted files
+					 * expected.  enable decryption.
+					 */
+					ret = PB_LOCKDOWN_DECRYPT;
+				}
+			}
+			free(auth_sig_line);
+		}
+	}
+
+	return ret;
 }
\ No newline at end of file
diff --git a/discover/gpg.h b/discover/gpg.h
index 9b452b8..0194236 100644
--- a/discover/gpg.h
+++ b/discover/gpg.h
@@ -26,6 +26,7 @@ 
 enum {
 	PB_LOCKDOWN_NONE	= 0,
 	PB_LOCKDOWN_SIGN	= 1,
+	PB_LOCKDOWN_DECRYPT	= 2,
 };
 
 #if defined(HAVE_LIBGPGME)
@@ -41,6 +42,9 @@  int verify_file_signature(const char * plaintext_filename,
 	const char * signature_filename, FILE * authorized_signatures_handle,
 	const char * keyring_path);
 
+int decrypt_file(const char * filename,
+	FILE * authorized_signatures_handle, const char * keyring_path);
+
 int gpg_validate_boot_files(struct boot_task *boot_task,
 	const char** local_initrd, const char** local_dtb,
 	const char** local_image);
@@ -60,6 +64,10 @@  int verify_file_signature(const char * plaintext_filename,
 	const char * signature_filename, FILE * authorized_signatures_handle,
 	const char * keyring_path) { return -1; }
 
+int decrypt_file(const char * filename,
+	FILE * authorized_signatures_handle, const char * keyring_path)
+	{ return -1; }
+
 int gpg_validate_boot_files(struct boot_task *boot_task,
 	const char** local_initrd, const char** local_dtb,
 	const char** local_image) { return 0; }
diff --git a/ui/ncurses/nc-boot-editor.c b/ui/ncurses/nc-boot-editor.c
index 9788fef..13fd6e5 100644
--- a/ui/ncurses/nc-boot-editor.c
+++ b/ui/ncurses/nc-boot-editor.c
@@ -208,6 +208,9 @@  static struct pb_boot_data *boot_editor_prepare_data(
 			boot_editor->widgets.args_sig_file_f);
 		bd->args_sig_file = conditional_prefix(bd, prefix, s);
 	}
+	else {
+		bd->args_sig_file = NULL;
+	}
 
 	return bd;
 }
@@ -532,6 +535,10 @@  static void boot_editor_setup_widgets(struct boot_editor *boot_editor,
 		boot_editor->widgets.args_sig_file_f = widget_new_textbox(set,
 				0, 0, field_size, boot_editor->args_sig_file);
 	}
+	else {
+		boot_editor->widgets.args_sig_file_l = NULL;
+		boot_editor->widgets.args_sig_file_f = NULL;
+	}
 
 	boot_editor->widgets.ok_b = widget_new_button(set, 0, 0, 10,
 					_("OK"), ok_click, boot_editor);
@@ -629,7 +636,7 @@  struct boot_editor *boot_editor_init(struct cui *cui,
 		if (boot_editor->use_signature_files)
 			boot_editor->args_sig_file = bd->args_sig_file;
 		else
-			boot_editor->args_sig_file = "";
+			boot_editor->args_sig_file = talloc_strdup(bd, "");
 		boot_editor_find_device(boot_editor, bd, sysinfo);
 	} else {
 		boot_editor->image = boot_editor->initrd =