* [PATCH v3 1/3] style: indent --no-tabs --gnu-style grub-core/commands/loadenv.c
2013-09-06 23:59 [PATCH v3 0/3] Add whitelisting support for load_env Jon McCune
@ 2013-09-06 23:59 ` Jon McCune
2013-09-06 23:59 ` [PATCH v3 2/3] load_env support for whitelisting which variables are read from an env file Jon McCune
2013-09-06 23:59 ` [PATCH v3 3/3] Add (multiple) -k, --pubkey=FILE support to grub-install command Jon McCune
2 siblings, 0 replies; 5+ messages in thread
From: Jon McCune @ 2013-09-06 23:59 UTC (permalink / raw)
To: grub-devel; +Cc: Jon McCune
Added the option --no-tabs after observing that 'indent' did not
result in consistent tab usage.
An example of the inconsistent tab usage: the very first function
in loadenv.c is open_envblk_file(), and it contains a line
indented exclusively with tabs, and lines indented exclusively
with spaces. The space-idented lines use two spaces for each
level of indentation. In order for the tab-indented lines to
format properly, the tabs must consume the equivalent of 8
spaces (i.e., four levels of indentation). To my knowledge, this
is not consistent with the default GNU style.
Signed-off-by: Jon McCune <jonmccune@google.com>
---
grub-core/commands/loadenv.c | 87 ++++++++++++++++++++++----------------------
1 file changed, 44 insertions(+), 43 deletions(-)
diff --git a/grub-core/commands/loadenv.c b/grub-core/commands/loadenv.c
index c0a42c5..a431499 100644
--- a/grub-core/commands/loadenv.c
+++ b/grub-core/commands/loadenv.c
@@ -30,20 +30,19 @@
GRUB_MOD_LICENSE ("GPLv3+");
-static const struct grub_arg_option options[] =
- {
- /* TRANSLATORS: This option is used to override default filename
- for loading and storing environment. */
- {"file", 'f', 0, N_("Specify filename."), 0, ARG_TYPE_PATHNAME},
- {0, 0, 0, 0, 0, 0}
- };
+static const struct grub_arg_option options[] = {
+ /* TRANSLATORS: This option is used to override default filename
+ for loading and storing environment. */
+ {"file", 'f', 0, N_("Specify filename."), 0, ARG_TYPE_PATHNAME},
+ {0, 0, 0, 0, 0, 0}
+};
static grub_file_t
open_envblk_file (char *filename)
{
grub_file_t file;
- if (! filename)
+ if (!filename)
{
const char *prefix;
@@ -54,20 +53,21 @@ open_envblk_file (char *filename)
len = grub_strlen (prefix);
filename = grub_malloc (len + 1 + sizeof (GRUB_ENVBLK_DEFCFG));
- if (! filename)
+ if (!filename)
return 0;
grub_strcpy (filename, prefix);
filename[len] = '/';
grub_strcpy (filename + len + 1, GRUB_ENVBLK_DEFCFG);
- grub_file_filter_disable_compression ();
+ grub_file_filter_disable_compression ();
file = grub_file_open (filename);
grub_free (filename);
return file;
}
else
{
- grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("variable `%s' isn't set"), "prefix");
+ grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("variable `%s' isn't set"),
+ "prefix");
return 0;
}
}
@@ -85,7 +85,7 @@ read_envblk_file (grub_file_t file)
grub_envblk_t envblk;
buf = grub_malloc (size);
- if (! buf)
+ if (!buf)
return 0;
while (size > 0)
@@ -104,7 +104,7 @@ read_envblk_file (grub_file_t file)
}
envblk = grub_envblk_open (buf, offset);
- if (! envblk)
+ if (!envblk)
{
grub_free (buf);
grub_error (GRUB_ERR_BAD_FILE_TYPE, "invalid environment block");
@@ -124,25 +124,25 @@ set_var (const char *name, const char *value)
static grub_err_t
grub_cmd_load_env (grub_extcmd_context_t ctxt,
- int argc __attribute__ ((unused)),
- char **args __attribute__ ((unused)))
+ int argc __attribute__ ((unused)),
+ char **args __attribute__ ((unused)))
{
struct grub_arg_list *state = ctxt->state;
grub_file_t file;
grub_envblk_t envblk;
file = open_envblk_file ((state[0].set) ? state[0].arg : 0);
- if (! file)
+ if (!file)
return grub_errno;
envblk = read_envblk_file (file);
- if (! envblk)
+ if (!envblk)
goto fail;
grub_envblk_iterate (envblk, set_var);
grub_envblk_close (envblk);
- fail:
+fail:
grub_file_close (file);
return grub_errno;
}
@@ -157,25 +157,25 @@ print_var (const char *name, const char *value)
static grub_err_t
grub_cmd_list_env (grub_extcmd_context_t ctxt,
- int argc __attribute__ ((unused)),
- char **args __attribute__ ((unused)))
+ int argc __attribute__ ((unused)),
+ char **args __attribute__ ((unused)))
{
struct grub_arg_list *state = ctxt->state;
grub_file_t file;
grub_envblk_t envblk;
file = open_envblk_file ((state[0].set) ? state[0].arg : 0);
- if (! file)
+ if (!file)
return grub_errno;
envblk = read_envblk_file (file);
- if (! envblk)
+ if (!envblk)
goto fail;
grub_envblk_iterate (envblk, print_var);
grub_envblk_close (envblk);
- fail:
+fail:
grub_file_close (file);
return grub_errno;
}
@@ -253,7 +253,7 @@ check_blocklists (grub_envblk_t envblk, struct blocklist *blocklists,
return grub_errno;
if (grub_memcmp (buf + index, blockbuf, p->length) != 0)
- return grub_error (GRUB_ERR_FILE_READ_ERROR, "invalid blocklist");
+ return grub_error (GRUB_ERR_FILE_READ_ERROR, "invalid blocklist");
}
return GRUB_ERR_NONE;
@@ -293,7 +293,7 @@ struct grub_cmd_save_env_ctx
/* Store blocklists in a linked list. */
static void
save_env_read_hook (grub_disk_addr_t sector, unsigned offset, unsigned length,
- void *data)
+ void *data)
{
struct grub_cmd_save_env_ctx *ctx = data;
struct blocklist *block;
@@ -303,7 +303,7 @@ save_env_read_hook (grub_disk_addr_t sector, unsigned offset, unsigned length,
return;
block = grub_malloc (sizeof (*block));
- if (! block)
+ if (!block)
return;
block->sector = sector;
@@ -315,7 +315,7 @@ save_env_read_hook (grub_disk_addr_t sector, unsigned offset, unsigned length,
if (ctx->tail)
ctx->tail->next = block;
ctx->tail = block;
- if (! ctx->head)
+ if (!ctx->head)
ctx->head = block;
}
@@ -330,14 +330,14 @@ grub_cmd_save_env (grub_extcmd_context_t ctxt, int argc, char **args)
.tail = 0
};
- if (! argc)
+ if (!argc)
return grub_error (GRUB_ERR_BAD_ARGUMENT, "no variable is specified");
file = open_envblk_file ((state[0].set) ? state[0].arg : 0);
- if (! file)
+ if (!file)
return grub_errno;
- if (! file->device->disk)
+ if (!file->device->disk)
{
grub_file_close (file);
return grub_error (GRUB_ERR_BAD_DEVICE, "disk device required");
@@ -347,7 +347,7 @@ grub_cmd_save_env (grub_extcmd_context_t ctxt, int argc, char **args)
file->read_hook_data = &ctx;
envblk = read_envblk_file (file);
file->read_hook = 0;
- if (! envblk)
+ if (!envblk)
goto fail;
if (check_blocklists (envblk, ctx.head, file))
@@ -360,9 +360,10 @@ grub_cmd_save_env (grub_extcmd_context_t ctxt, int argc, char **args)
value = grub_env_get (args[0]);
if (value)
{
- if (! grub_envblk_set (envblk, args[0], value))
+ if (!grub_envblk_set (envblk, args[0], value))
{
- grub_error (GRUB_ERR_BAD_ARGUMENT, "environment block too small");
+ grub_error (GRUB_ERR_BAD_ARGUMENT,
+ "environment block too small");
goto fail;
}
}
@@ -373,7 +374,7 @@ grub_cmd_save_env (grub_extcmd_context_t ctxt, int argc, char **args)
write_blocklists (envblk, ctx.head, file);
- fail:
+fail:
if (envblk)
grub_envblk_close (envblk);
free_blocklists (ctx.head);
@@ -383,24 +384,24 @@ grub_cmd_save_env (grub_extcmd_context_t ctxt, int argc, char **args)
static grub_extcmd_t cmd_load, cmd_list, cmd_save;
-GRUB_MOD_INIT(loadenv)
+GRUB_MOD_INIT (loadenv)
{
cmd_load =
grub_register_extcmd ("load_env", grub_cmd_load_env, 0, N_("[-f FILE]"),
- N_("Load variables from environment block file."),
- options);
+ N_("Load variables from environment block file."),
+ options);
cmd_list =
grub_register_extcmd ("list_env", grub_cmd_list_env, 0, N_("[-f FILE]"),
- N_("List variables from environment block file."),
- options);
+ N_("List variables from environment block file."),
+ options);
cmd_save =
grub_register_extcmd ("save_env", grub_cmd_save_env, 0,
- N_("[-f FILE] variable_name [...]"),
- N_("Save variables to environment block file."),
- options);
+ N_("[-f FILE] variable_name [...]"),
+ N_("Save variables to environment block file."),
+ options);
}
-GRUB_MOD_FINI(loadenv)
+GRUB_MOD_FINI (loadenv)
{
grub_unregister_extcmd (cmd_load);
grub_unregister_extcmd (cmd_list);
--
1.8.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v3 2/3] load_env support for whitelisting which variables are read from an env file
2013-09-06 23:59 [PATCH v3 0/3] Add whitelisting support for load_env Jon McCune
2013-09-06 23:59 ` [PATCH v3 1/3] style: indent --no-tabs --gnu-style grub-core/commands/loadenv.c Jon McCune
@ 2013-09-06 23:59 ` Jon McCune
2013-09-06 23:59 ` [PATCH v3 3/3] Add (multiple) -k, --pubkey=FILE support to grub-install command Jon McCune
2 siblings, 0 replies; 5+ messages in thread
From: Jon McCune @ 2013-09-06 23:59 UTC (permalink / raw)
To: grub-devel; +Cc: Jon McCune
This gives load_env support for naming variables that is the same as save_env.
This is useful to prevent potentially unwanted variables within a grubenv file
from impacting the behavior of a configuration, especially if using
check_signatures=enforce but wishing to retain support for savedefault,
boot_once, and similar.
This version of this patch drops all direct interaction with the logic in
verify.c. The author of grub.cfg must take care to disable and re-enable
check_signatures as appropriate.
Signed-off-by: Jon McCune <jonmccune@google.com>
---
grub-core/commands/loadenv.c | 57 ++++++++++++++++++++++++++++++++++++++++----
1 file changed, 52 insertions(+), 5 deletions(-)
diff --git a/grub-core/commands/loadenv.c b/grub-core/commands/loadenv.c
index a431499..5828f5f 100644
--- a/grub-core/commands/loadenv.c
+++ b/grub-core/commands/loadenv.c
@@ -122,15 +122,59 @@ set_var (const char *name, const char *value)
return 0;
}
+#define MAX_VAR_NAME 1024
+/* TODO: eliminate the need for these to be global */
+static grub_size_t whitelist_len = 0;
+static char **whitelist_vars = NULL;
+static int
+/* Assumptions: name, value, and each element of whitelist_vars[] are non-NULL
+ and NULL-terminated. */
+set_var_whitelist (const char *name, const char *value)
+{
+ grub_size_t i;
+ if (whitelist_len < 1 || whitelist_vars == NULL)
+ return GRUB_ERR_OUT_OF_RANGE;
+
+ /* search for 'name' in the whitelist */
+ for (i = 0; i < whitelist_len; i++)
+ {
+ if (0 == grub_strncmp (name, whitelist_vars[i], MAX_VAR_NAME))
+ {
+ /* TODO: also validate the characters in the variables */
+ grub_dprintf ("crypt", "set_var_whitelist APPOVED: %s=%s\n",
+ name, value);
+ grub_env_set (name, value);
+ return 0;
+ }
+ }
+
+ grub_dprintf ("crypt",
+ "set_var_whitelist REFUSING TO SET name='%s' value='%s'\n",
+ name, value);
+ /* Still considered success */
+ return 0;
+}
+
+/* Load environment variables from a file. Accepts a flag which can override
+ the file from which variables are read. If additional parameters are passed
+ (argc > 0), then those are interpreted as a whitelist of environment
+ variables whose values can be changed based on the contents of the file, and
+ the file is never signature-checked.
+ Otherwise, all variables from the file are processed, and the file must be
+ properly signed when check_signatures=enforce. */
static grub_err_t
-grub_cmd_load_env (grub_extcmd_context_t ctxt,
- int argc __attribute__ ((unused)),
- char **args __attribute__ ((unused)))
+grub_cmd_load_env (grub_extcmd_context_t ctxt, int argc, char **args)
{
struct grub_arg_list *state = ctxt->state;
grub_file_t file;
grub_envblk_t envblk;
+ /* argc > 0 indicates caller provided a whitelist of variables to read */
+ if (argc > 0)
+ {
+ whitelist_len = (argc >= 0) ? argc : 0;
+ whitelist_vars = args;
+ }
file = open_envblk_file ((state[0].set) ? state[0].arg : 0);
if (!file)
return grub_errno;
@@ -139,10 +183,12 @@ grub_cmd_load_env (grub_extcmd_context_t ctxt,
if (!envblk)
goto fail;
- grub_envblk_iterate (envblk, set_var);
+ grub_envblk_iterate (envblk, argc > 0 ? set_var_whitelist : set_var);
grub_envblk_close (envblk);
fail:
+ whitelist_len = 0;
+ whitelist_vars = NULL;
grub_file_close (file);
return grub_errno;
}
@@ -387,7 +433,8 @@ static grub_extcmd_t cmd_load, cmd_list, cmd_save;
GRUB_MOD_INIT (loadenv)
{
cmd_load =
- grub_register_extcmd ("load_env", grub_cmd_load_env, 0, N_("[-f FILE]"),
+ grub_register_extcmd ("load_env", grub_cmd_load_env, 0,
+ N_("[-f FILE] [whitelisted_variable_name] [...]"),
N_("Load variables from environment block file."),
options);
cmd_list =
--
1.8.4
^ permalink raw reply related [flat|nested] 5+ messages in thread