From: Jon McCune <jonmccune@google.com>
To: grub-devel@gnu.org
Cc: Jon McCune <jonmccune@google.com>
Subject: [PATCH v1 2/2] introduce new commands load_env_untrusted and save_env_untrusted:
Date: Mon, 26 Aug 2013 12:29:06 -0700 [thread overview]
Message-ID: <1377545346-23642-3-git-send-email-jonmccune@google.com> (raw)
In-Reply-To: <1377545346-23642-1-git-send-email-jonmccune@google.com>
These commands will load / store a single named environment variable
from / to a 1024-byte file, regardless of the value of check_signatures.
Signed-off-by: Jon McCune <jonmccune@google.com>
---
grub-core/commands/loadenv.c | 284 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 284 insertions(+)
diff --git a/grub-core/commands/loadenv.c b/grub-core/commands/loadenv.c
index 691d163..f6a363c 100644
--- a/grub-core/commands/loadenv.c
+++ b/grub-core/commands/loadenv.c
@@ -26,6 +26,7 @@
#include <grub/partition.h>
#include <grub/lib/envblk.h>
#include <grub/extcmd.h>
+#include <grub/command.h>
#include <grub/i18n.h>
GRUB_MOD_LICENSE ("GPLv3+");
@@ -379,7 +380,280 @@ grub_cmd_save_env (grub_extcmd_context_t ctxt, int argc, char **args)
return grub_errno;
}
+/* It is an error for a file being read or written with load_env_untrusted or
+ save_env_untrusted to be any other size. */
+#define UNTRUSTED_VAR_FILE_SIZE 1024
+
+/* Returns 0 if c is not in the set [A-Za-z0-9_] */
+static int
+is_AZaz09_(int c)
+{
+ return (grub_isalpha(c) || grub_isdigit(c) || '_' == c);
+}
+
+/* Opens 'filename' with all filters disabled (in particular, the PUBKEY filter
+ that insists upon properly signed files, and any kind of compression, since
+ we don't want untrusted input being processed by complex compression
+ algorithms). */
+static grub_file_t
+open_file_untrusted (char *filename)
+{
+ grub_file_filter_t curfilt[GRUB_FILE_FILTER_MAX];
+ grub_file_t file;
+
+ /* Temporarily disable all filters so as to read the untrusted file */
+ grub_memcpy (curfilt, grub_file_filters_enabled, sizeof (curfilt));
+ grub_file_filter_disable_all();
+ file = grub_file_open(filename);
+ /* Restore filters */
+ grub_memcpy (grub_file_filters_enabled, curfilt, sizeof (curfilt));
+
+ return file;
+}
+
+/* Reads an already-opened file into a freshly allocated buffer of size
+ UNTRUSTED_VAR_FILE_SIZE. Returns pointer to buffer, or NULL on error. The
+ returned data is *not* guaranteed to be NULL-terminated.
+ TODO: Reduce duplication of logic with read_envblk_file(). (Requires
+ refactoring read_envblk_file() and first-pass goal is to keep changes as
+ self-contained as possible.) */
+static char*
+read_file_untrusted (grub_file_t file)
+{
+ char *buf = NULL;
+ grub_size_t size;
+ grub_off_t offset;
+
+ if (! file)
+ return NULL;
+
+ /* Read file's size, allocate space, and read the file's contents */
+ size = grub_file_size(file);
+ if (size != UNTRUSTED_VAR_FILE_SIZE)
+ {
+ grub_dprintf ("crypt", "ERROR: size %d != %d\n", size,
+ UNTRUSTED_VAR_FILE_SIZE);
+ return NULL;
+ }
+
+ buf = grub_malloc (size);
+ if (! buf)
+ return NULL;
+
+ grub_dprintf ("crypt", "malloc succeeded\n");
+
+ offset = 0;
+ while (size > 0)
+ {
+ grub_ssize_t ret = grub_file_read (file, buf + offset, size);
+ if (ret == 0)
+ break;
+ else if (ret < 0)
+ {
+ grub_free (buf);
+ return NULL;
+ }
+
+ size -= ret;
+ offset += ret;
+ }
+ grub_dprintf ("crypt", "successfully read %llu bytes from file!\n", offset);
+
+ if (offset != UNTRUSTED_VAR_FILE_SIZE)
+ {
+ grub_dprintf ("crypt", "ERROR read too few bytes (%llu instead of %d) from "
+ "file!\n", offset, UNTRUSTED_VAR_FILE_SIZE);
+ grub_free (buf);
+ return NULL;
+ }
+
+ return buf;
+}
+
+/* NULL-terminates buf by changing all characters after the first run of 0 or
+ more characters in the set [A-Za-z0-9_]. Assumes buf points to at least
+ UNTRUSTED_VAR_FILE_SIZE bytes. */
+static void
+sanitize_untrusted_string (char buf[UNTRUSTED_VAR_FILE_SIZE])
+{
+ grub_size_t i;
+
+ /* First, NULL-terminate the input for safety */
+ buf[UNTRUSTED_VAR_FILE_SIZE - 1] = '\0';
+ /* Now nullify all characters not in [A-Za-z0-9_] */
+ for (i = 0; i < UNTRUSTED_VAR_FILE_SIZE; i++)
+ if (!is_AZaz09_(buf[i]))
+ buf[i] = '\0';
+}
+
+/* Now that we have read the untrusted data, assign it to the requested
+ environment variable. It is *not* an error for the data to be a string of
+ length 0 (i.e., for buf[0] == '\0'), it just results in an empty
+ environment variable. */
+static grub_err_t
+grub_cmd_load_env_untrusted (grub_command_t cmd __attribute__ ((unused)),
+ int argc, char **argv)
+{
+ grub_file_t file;
+ grub_err_t err;
+ char *buf;
+ unsigned int i;
+
+ grub_dprintf ("crypt", "load_env_untrusted\n");
+
+ if (argc != 2)
+ return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("two arguments expected"));
+
+ for (i = 0; argv[0][i] != '\0'; i++)
+ if (! is_AZaz09_ (argv[0][i]))
+ return grub_error (GRUB_ERR_BAD_ARGUMENT,
+ N_("VARIABLE_NAME must be in set [A-Za-z0-9_]"));
+
+ grub_dprintf ("crypt", "variable_name: %s\n", argv[0]);
+ grub_dprintf ("crypt", "file: %s\n", argv[1]);
+
+ file = open_file_untrusted(argv[1]);
+ if (! file)
+ {
+ grub_dprintf ("crypt", "ERROR: couldn't open file: %s\n", argv[1]);
+ return grub_errno;
+ }
+
+ buf = read_file_untrusted(file);
+ grub_file_close (file);
+
+ if (NULL == buf)
+ return GRUB_ERR_FILE_READ_ERROR;
+
+ sanitize_untrusted_string(buf);
+
+ err = grub_env_set(argv[0], buf);
+
+ if (buf)
+ grub_free (buf);
+ return err;
+}
+
+/* Overwrites the contents of an existing 1024-byte file with the first run of
+ bytes from the specified environment variable that comply with [A-Za-z0-9_].
+ Two arguments: the name of the environment variable, and the name of the file
+ into which that variable's contents should be written. Extra space in the
+ file is padded out with a single '\n' followed by '#' characters. */
+static grub_err_t
+grub_cmd_save_env_untrusted (grub_command_t cmd __attribute__ ((unused)),
+ int argc, char **argv)
+{
+ grub_file_t file;
+ const char* var_contents;
+ grub_size_t var_contents_len;
+ char* old_file_contents;
+ grub_err_t err;
+ char *new_file_contents;
+ struct grub_cmd_save_env_ctx ctx = {
+ .head = 0,
+ .tail = 0
+ };
+ grub_disk_t disk;
+ grub_disk_addr_t part_start;
+ struct blocklist *p;
+ grub_size_t index;
+
+ grub_dprintf ("crypt", "save_env_untrusted\n");
+
+ if (argc != 2)
+ return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("two arguments expected"));
+
+ for (index = 0; argv[0][index] != '\0'; index++)
+ if (! is_AZaz09_ (argv[0][index]))
+ return grub_error (GRUB_ERR_BAD_ARGUMENT,
+ N_("VARIABLE_NAME must be in set [A-Za-z0-9_]"));
+
+ var_contents = grub_env_get(argv[0]);
+ if (NULL == var_contents)
+ return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("no such variable"));
+ /* This ensures var_contents is NULL-terminated for processing below */
+ var_contents_len = grub_strlen (var_contents); /* TODO: get strnlen */
+ if (var_contents_len >= UNTRUSTED_VAR_FILE_SIZE)
+ return grub_error (GRUB_ERR_OUT_OF_RANGE, N_("variable value too long"));
+
+ /* Make sure the variable only contains characters in [A-Za-z0-9_] */
+ for (index = 0; index < var_contents_len; index++)
+ if (!is_AZaz09_ (var_contents[index]))
+ return grub_error (GRUB_ERR_OUT_OF_RANGE,
+ N_("illegal characters in variable"));
+
+ file = open_file_untrusted(argv[1]);
+ if (! file)
+ return grub_errno;
+
+ grub_dprintf ("crypt", "save_env_untrusted opened file %s for reading\n",
+ argv[1]);
+
+ if (! file->device->disk)
+ {
+ grub_file_close (file);
+ return grub_error (GRUB_ERR_BAD_DEVICE, "disk device required");
+ }
+
+ file->read_hook = save_env_read_hook;
+ file->read_hook_data = &ctx;
+ old_file_contents = read_file_untrusted(file);
+ file->read_hook = 0;
+ if (NULL == old_file_contents)
+ {
+ err = grub_error (GRUB_ERR_FILE_READ_ERROR,
+ N_("Unable to read existing file blocks"));
+ goto fail;
+ }
+
+ grub_dprintf ("crypt", "read file, blocklist is built.\n");
+
+ err = check_blocklists (old_file_contents, ctx.head, file);
+ if (GRUB_ERR_NONE != err)
+ goto fail;
+
+ grub_dprintf ("crypt", "blocklist checked successfully.\n");
+
+ new_file_contents = grub_malloc (UNTRUSTED_VAR_FILE_SIZE);
+ if (NULL == new_file_contents)
+ {
+ err = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("malloc failed"));
+ goto fail;
+ }
+
+ grub_memcpy(new_file_contents, old_file_contents, UNTRUSTED_VAR_FILE_SIZE);
+ grub_strncpy(new_file_contents, var_contents, UNTRUSTED_VAR_FILE_SIZE);
+ new_file_contents[var_contents_len] = '\n';
+ for (index = var_contents_len + 1; index < UNTRUSTED_VAR_FILE_SIZE; index++)
+ new_file_contents[index] = '#'; /* backfill file with '#' character */
+
+ grub_dprintf ("crypt", "ready to write: ---\n%s\n---\n", new_file_contents);
+
+ /* TODO: Refactor write_blocklists() so we can just call it here instead. */
+ disk = file->device->disk;
+ part_start = grub_partition_get_start (disk->partition);
+
+ index = 0;
+ for (p = ctx.head; p; index += p->length, p = p->next)
+ {
+ err = grub_disk_write (disk, p->sector - part_start,
+ p->offset, p->length, new_file_contents + index);
+ if (GRUB_ERR_NONE != err)
+ goto fail;
+ }
+
+fail:
+ if (old_file_contents)
+ grub_free (old_file_contents);
+ if (new_file_contents)
+ grub_free (new_file_contents);
+ free_blocklists (ctx.head);
+ grub_file_close (file);
+ return err;
+}
+
static grub_extcmd_t cmd_load, cmd_list, cmd_save;
+static grub_command_t cmd_load_env_untrusted, cmd_save_env_untrusted;
GRUB_MOD_INIT(loadenv)
{
@@ -396,6 +670,14 @@ GRUB_MOD_INIT(loadenv)
N_("[-f FILE] variable_name [...]"),
N_("Save variables to environment block file."),
options);
+ cmd_load_env_untrusted =
+ grub_register_command ("load_env_untrusted", grub_cmd_load_env_untrusted,
+ N_("VARIABLE_NAME FILE"),
+ N_("Assign the contents of FILE to VARIABLE_NAME"));
+ cmd_save_env_untrusted =
+ grub_register_command ("save_env_untrusted", grub_cmd_save_env_untrusted,
+ N_("VARIABLE_NAME FILE"),
+ N_("Write the contents of VARIABLE_NAME into pre-existing FILE"));
}
GRUB_MOD_FINI(loadenv)
@@ -403,4 +685,6 @@ GRUB_MOD_FINI(loadenv)
grub_unregister_extcmd (cmd_load);
grub_unregister_extcmd (cmd_list);
grub_unregister_extcmd (cmd_save);
+ grub_unregister_command (cmd_load_env_untrusted);
+ grub_unregister_command (cmd_save_env_untrusted);
}
--
1.8.3
prev parent reply other threads:[~2013-08-26 19:29 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-26 19:29 [PATCH v1 0/2] New commands: load_env_untrusted, save_env_untrusted Jon McCune
2013-08-26 19:29 ` [PATCH v1 1/2] make check_blocklists() not depend on an envblk data type Jon McCune
2013-08-26 19:29 ` Jon McCune [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1377545346-23642-3-git-send-email-jonmccune@google.com \
--to=jonmccune@google.com \
--cc=grub-devel@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).