canonical / pc-gadget

The gadget snap for Personal Computers using 64bit Intel or AMD processors
GNU General Public License v3.0
31 stars 73 forks source link

grub: boot configuration is shipped and managed by snapd #50

Closed bboozzoo closed 4 years ago

bboozzoo commented 4 years ago

Since v2.46, snapd can ship and manage the boot configuration for grub for both the run mode and recovery. Drop the recovery mode config and truncate grub.conf so that we are left with a suitable marker for snapd. Also, remove references to grub*.conf from gadget.yaml, as there is no need to list those files in content section anymore.

xnox commented 4 years ago

How does snapd_extra_cmdline_args work?

Is it curated, or arbitrary or placeholder for the future?

Because it is trivial to append things there, to subvert boot and compromise sealings.

xnox commented 4 years ago

So it looks like snapd_extra_cmdline_args can be used to poison the device, and compromise install, which will then persist.

bboozzoo commented 4 years ago

So it looks like snapd_extra_cmdline_args can be used to poison the device, and compromise install, which will then persist.

With the limitation that snapd does not read it, so the adding arbitrary arguments will not influence the command lines used when sealing the key.

xnox commented 4 years ago

So it looks like snapd_extra_cmdline_args can be used to poison the device, and compromise install, which will then persist.

With the limitation that snapd does not read it, so the adding arbitrary arguments will not influence the command lines used when sealing the key.

so, why is it there right now? cause grub will execute that command, and will generate grub config command measurement affecting the tpm measurements.

We currently don't seal to the grub measured commands. But the PCR values will change. because the load command is different versus previous grub.cfg.

bboozzoo commented 4 years ago

The plan was (maybe still is) to support adding command line arguments via snapd. Those would end up in the snapd state, and be used when deriving the command line when (re-)sealing the keys, so the expected load command is composed only from elements contrrolled by snapd. If you access the device, and arbitrarily change grubenv the measurements won't match. If you inject something before snapd reseals the keys it won't work either, because snapd will only use whatever is in its state, not grubenv. Or am I missing something there?

anonymouse64 commented 4 years ago

I may be misremembering here, but my recollection of the bootenv variable snapd_extra_cmdline_args is that it is currently just left empty everywhere as a probable implementation of the eventual mechanism for the gadget to provide input on what kernel command lines should be used with a device. It is not read by snapd anywhere today, nor is it written anywhere today.

Currently, snapd always ignores this value when sealing against the kernel command line, as the kernel command line we seal against stored in snapd internally, so as Maciej says if someone modifies this grubenv after the image is prepared but before it is booted in install mode, they will boot with a different kernel command line for install mode, then snapd will write out what it expects and uses normally for run mode and that will be used for run mode and sealed against.

The situation for recover mode is similarly protected against poisoning because if the modification to grubenv happens, and we transition to boot from recovery, since snapd totally ignored the grubenv value when sealing for the recovery command line during install mode, the system will be booted with an unexpected kernel command line and not be able to unlock the data partition because the kernel command line measurement is wrong.

The implementation of allowing the gadget to extend the kernel command line will probably be somewhat more complex than just having install mode snapd read the bootenv directly to get the value of this snapd_extra_cmdline_args, and instead will read something from the gadget snap itself directly which will be cryptographically signed and not mutable by attackers unlike the real bootenv so we are protected against the poisoning attack you mention. Snapd will probably read something from the gadget snap, then seal against that, then write that into the bootenv of the run and recover mode systems when setting up to use those respectively, but it should never read from those to determine what to seal against.

pedronis commented 4 years ago

as @anonymouse64 and @bboozzoo have explained the implementation and design is such that the input to seal and reseal are either signed data (assertions, snaps, snapd snap itself) or from the encrypted partition itself

xnox commented 4 years ago

as @anonymouse64 and @bboozzoo have explained the implementation and design is such that the input to seal and reseal are either signed dat a (assertions, snaps, snapd snap itself) or from the encrypted partition itself

@pedronis @anonymouse64 @bboozzoo even then, it may not accept and seal to arbitrary additional command lines. It must be curated and filtered. It may not override the mode and/or system. Ordering matters too.

I understand that grubenv is susceptible to outside manipulation, I am worried about sealing to insecure "extras" which allow bypassing measurements, or staging prefix attack, or hiding those facts.

@chrisccoulson what is allowed, and sealed to, in the extra cmdline flags must be reviewed.

pedronis commented 4 years ago

I understand that grubenv is susceptible to outside manipulation, I am worried about sealing to insecure "extras" which allow bypassing measurements, or staging prefix attack, or hiding those facts.

yes, we have the exact same worry. We don't use extra atm, and as we said the plan is never to read a *env from seed or boot and take it to reasel based on it. What we do is we have code to compose a command line, that then it is used to seal/reseal. The input to that code comes stricly from snapd own codebase ATM.

At a later point some of that input could come from the gadget, again order would be imposed and probably we would have to consider an allow-list or similar. Or give very precise higher-level options. This is not done nor finalized atm. But those are the thinking guardrails around it.

Anyway at that point that input would be used to compose the command line for sealing and be written to the env. As we said not the other way around where env is the input.

xnox commented 4 years ago

I understand that grubenv is susceptible to outside manipulation, I am worried about sealing to insecure "extras" which allow bypassing measurements, or staging prefix attack, or hiding those facts.

yes, we have the exact same worry. We don't use extra atm, and as we said the plan is never to read a *env from seed or boot and take it to reasel based on it. What we do is we have code to compose a command line, that then it is used to seal/reseal. The input to that code comes stricly from snapd own codebase ATM.

At a later point some of that input could come from the gadget, again order would be imposed and probably we would have to consider an allow-list or similar. Or give very precise higher-level options. This is not done nor finalized atm. But those are the thinking guardrails around it.

Anyway at that point that input would be used to compose the command line for sealing and be written to the env. As we said not the other way around where env is the input.

Ack. Also do recall that grub had shell quoting/escaping CVEs before. Thus I would suggest trying to avoid any quotes, escape characters, unicode, just pure allowlist of keywords and/or keyword=key pairs. At the very most.

bboozzoo commented 4 years ago

Shall we land the PR then?

xnox commented 4 years ago

This has now been promoted to the edge channel; thus tomorrow's daily edge images should have it.

It should promote to beta channel on Monday.