RFE: allow runtime arguments when builtin command line exists

[smoser]

Currently, stubby allows either:

• builtin: adding a kernel command line via objcopy (stubby-smash -c "console=ttyS0 root=LABEL=rootdev")
• runtime: providing a kernel command line at runtime via arguments to the efi application (kernel.efi console=ttyS0 root=LABEL=rootdev).

If a the efi has a builtin command line and is provided with runtime arguments, the boot is rejected.

We would like to be able to support having both a builtin and runtime command lines.

Kernel parsing of its command line and options allowed is discussed here. Some things to note:

• The kernel parses parameters from the kernel command line up to --; if it doesn’t recognize a parameter and it doesn’t contain a ‘.’, the parameter gets passed to init: parameters with ‘=’ go into init’s environment, others are passed as command line arguments to init. Everything after -- is passed as an argument to init.
• Module parameters can specified on the kernel command line with a module name prefix (usbcore.blinkenlights=1).
• behavior of multiple values of the same type (key=value) arguments differs
  • console=ttyS0 console=tty1 : kernel output will go to both tty1 and ttyS0; /dev/console is set to the last entry (tty1)
  • acpi_osi (see doc)
  • root= (may depend on initramfs implementation)

Because of the complexity, we want to be able to tightly control the order of builtin and runtime arguments.

The following is suggested:

• if there is no builtin command line, use runtime. (current behavior)
• if builtin command line is not an empty string, does not contain a marker (STUBBY_RT_CLI1), and runtime value is not an empty string, then reject boot (current behavior)
• if builtin command line is set and contains a marker (STUBBY_RT_CLI1): replace the marker with the runtime arguments (using an empty string if none are given)
• stubby gives no special consideration to the value of builtin or runtime at this stage. For example, '--' is treated the same as 'console=X' or any other string.
• if the string STUBBY_RT is found in the builtin value, but not as part of STUBBY_RT_CLI1, then fail. This is to protect against accidental signing or potentially a different future version with different behavior.
• STUBBY_RT_CLI1 must be a complete token; if STUBBY_RT_CLI10 or STUBBY_RT_CLI1console=ttyS1 are found in the builtin cmdline , boot will be rejected.
• STUBBY_RT_CLI1 cannot occur more than once in the builtin ; if multiple instances occur boot will be rejected
• if STUBBY_RT is found in runtime arguments, boot will be rejected.

builtin	runtime	allowed?	result cmdline
console=ttyS0	\	Y	console=ttyS0
empty	console=ttyS0	Y	console=ttyS0
console=ttyS0	\	N	:x:
console=ttyS0 STUBBY_RT_CLI1 -- 3	console=tty1 STUBBY_RT	N	:x:
console=ttyS0 STUBBY_RT_CLI1 -- 3	console=tty1	Y	console=ttyS0 console=tty1 -- 3
STUBBY_RT_CLI1 console=tty1 acpi=off	acpi=on console=tty1	Y	acpi=on console=tty1 acpi=off
STUBBY_RT_CLI1 console=tty1 acpi=off	acpi=on console=tty1	Y	acpi=on console=tty1 acpi=off
STUBBY_RT_CLI1console=ttyS0	\	N	:x:
console=STUBBY_RT_CLI1,115200		N	:x:
STUBBY_RT_CLI1 console=ttyS0 STUBBY_RT_CLI1 foo=bar	\	N	:x:

Above:

• 'allowed' indicates whether the command line will be passed to 'check_cmdline' function. If false execution will stop with error immediately.
• 'result cmdline' indicates the command line that is will be passed to validation.
• \ indicates empty string "" (no other value)
• \ indicates any value including empty string.

[hallyn]

It seems reasonable. Two questions:

1. will -- and = as part of runtime cmdline be rejected? I'd argue they should.
2. What do you think about STUBBY_RT_CLI_ALLOW=console=tty1 STUBBY_RT_CLI_ALLOW=onlinecpu-whatever meaning only that argument, if given in runtime cmdline, will be inserted there? It's probably too much... But it would be a nice way of having the user (the writer of the builtin cmdline) be less reliant on our (authors of stubby) ability to predict what cmdlines are ok or unsafe.

[smoser]

It seems reasonable. Two questions:

1. will -- and = as part of runtime cmdline be rejected? I'd argue they should.

why would '=' be rejected? wrt '--', I think its not of much use. The kernel considers '--' specially so I can kind of see your argument, but systemd basically does its own parsing of /proc/cmdline (it does not, iirc, pay attention to its argc/argv, but fishes through /proc/cmdline for 'systemd.debug-shell=1' or the like).

I'm willing to accept the suggestion as a "better safe than sorry" for now, as it is easier to extend rather than restrict behavior later on.

2. What do you think about STUBBY_RT_CLI_ALLOW=console=tty1 STUBBY_RT_CLI_ALLOW=onlinecpu-whatever meaning only that argument, if given in runtime cmdline, will be inserted there? It's probably too much... But it would be a nice way of having the user (the writer of the builtin cmdline) be less reliant on our (authors of stubby) ability to predict what cmdlines are ok or unsafe.

I think "later maybe". I want to enable general function as tightly/simply as possible right now. Also, remember its not that hard to change the allowed-list if someone wanted to do that. And i had proposed allowing that at post-build-time (https://github.com/puzzleos/stubby/issues/14).

[hallyn]

why would '=' be rejected?

After '--', to keep from mucking with init's environment. Not before --

But the point of rejecting '--' is so that if you have STUBBY_RT_CLI_ALLOW before an important argument, the user doesn't add '--' making the kernel ignore the important argument. I don't have an example of such an important argument offhand, ghough.

[smoser]

why would '=' be rejected?

After '--', to keep from mucking with init's environment. Not before --

But the point of rejecting '--' is so that if you have STUBBY_RT_CLI_ALLOW before an important argument, the user doesn't add '--' making the kernel ignore the important argument. I don't have an example of such an important argument offhand, ghough.

ah, ok. that does make some sense. I guess I'm OK to reject it for now with the thought that we can enable it later.

One thing to remember is that the result still has to go through the accept-ilst verification. So unless we added '--' and 'YOUR_ENV=YOUR_VAL' into the accept-list, it would get rejected based on that.

Many of my bullet points in the list were just trying to safeguard against "user error" in the builtin command line or an attempt to exploit a bug via runtime command line.

I'm willing to add that 'key=value' cannot occur after '--' in the runtime args if you think we should.

Thoughts?