Hello,
First, thanks for adding signature checking capabilities to grub!
However, on a machine that has two or more valid signed kernels, there is currently no way to support savedefault, attempt a one-shot boot, etc. This is because, with check_signatures=enforce, only a properly signed grubenv file will be loaded (by commands load_env and save_env (save_env reads the file before overwriting it)). Even if it were possible to write to grubenv while check_signatures=enforce, that new grubenv would not load successfully since the signature would be invalid over the modified file.
I propose the following resolution:
Fully parsing an untrusted grubenv file is probably too risky, because logic would be necessary to prevent critical environment variables (e.g., check_signatures itself, or prefix, or ...) from being overwritten by values in the grubenv file. This seems like it would lead to an overly complex set of rules about what variables can or cannot be modified.
So, instead of storing name=value pairs in an untrusted file, what about just storing a single environment variable's value in a file? This way, the environment variable name that is (possibly overwritten) written on load can be specified in the signed, verified config file (grub.cfg). An arbitrarily malicious 'value' in the file could only impact the value of the environment variable of interest.
I further propose constraining both the name and value for such commands to a safe subset of allowable characters (since right now some potentially dangerous characters are legal in the name of grub environment variables -- think of bobby tables). Perhaps [A-Za-z0-9_] (upper and lowercase letters, numbers, and underscore).
This would result in the addition of two new commands:
load_env_untrusted [VARIABLE_NAME] [PATH_TO_FILE]
save_env_untrusted [VARIABLE_NAME] [PATH_TO_FILE]
load_env_untrusted would assign to the grub environment variable VARIABLE_NAME the first 0 or more bytes of PATH_TO_FILE that comply with the [A-Za-z0-9_] policy.
save_env_untrusted would write the first 0 or more bytes currently assigned to VARIABLE_NAME and that comply with [A-Za-z0-9_] into the file PATH_TO_FILE, followed by a terminator. I propose using newline '\n' as terminator to keep the file ASCII and human-readable.
These commands (load_env_untrusted, save_env_untrusted) would be invoked from a signed, verified grub.cfg. The name of the environment variable that is modified or stored is controlled only by the signed, verified grub.cfg.
I propose that the file that is read or written must already exist and must be exactly 1024 bytes in size, just like the existing grubenv. This minor amount of wasted space (since I presume most legitimate environment variable values will be significantly less than 1024 bytes, e.g., a few bytes in the case of savedefault storing a small integer) is basically harmless.
If these ideas are agreeable I will follow up with a patch that implements this functionality.
Thanks!
-Jon