search

QubesOS / qubes-issues

The Qubes OS Project issue tracker
https://www.qubes-os.org/doc/issue-tracking/
532 stars 46 forks source link

If tboot falls back to unmeasured boot, Anti Evil Maid appears to work but is insecure #2569

Closed mattmccutchen closed 7 years ago

mattmccutchen commented 7 years ago

Qubes OS version (e.g., R3.2): R3.2

Affected TemplateVMs (e.g., fedora-23, if applicable): N/A

If a user sets up Anti Evil Maid but tboot is unable to perform a measured boot for some reason (e.g., TXT is disabled or the wrong SINIT module is installed), tboot automatically falls back to an unmeasured boot and leaves PCR 17-19 set to all FF. Anti Evil Maid will happily seal the secret to these values and appear to be working, but the secret will be successfully unsealed regardless of changes to the boot components (as long as the condition that prevents a measured boot remains), so there is no security. This may give the user a false sense of security. In this situation, Anti Evil Maid should show a message that measured boot is not working and refuse to seal the secret.

Previous thread.

rustybird commented 7 years ago

We could make anti-evil-maid-seal abort if grep -E '^PCR-1[3789]:( (00|FF)){20}' on the pcrs file in /sys succeeds, i.e. if any of the standard registers are all zero/one bits. Now, where to find this file - does /sys/devices/pnp*/*/tpm/tpm*/pcrs work for everybody?

rustybird commented 7 years ago

Actually, let's put that check (whatever it will be, exactly) into anti-evil-maid-lib and call it from both -seal and -unseal, so existing users can also see the warning if necessary.

mattmccutchen commented 7 years ago

Now, where to find this file - does /sys/devices/pnp*/*/tpm/tpm*/pcrs work for everybody?

It doesn't work for me: I have /sys/devices/pnp0/00:06/pcrs, and /sys/devices/pnp0/00:06/tpm/tpm0 is a directory that does not contain pcrs but contains a symlink device -> ../../../00:06, so /sys/devices/pnp*/*/tpm/tpm*/device/pcrs would work for me. Other ideas: find /sys/devices -name pcrs like in the instructions, or add a tpm_pcr_read tool to the tpm-extra package that uses the Trousers API, which would be mostly a copy and paste of the existing tpm_pcr_extend tool.

rustybird commented 7 years ago

@mattmccutchen: Do you also have the /sys/devices/pnp*/*/pcrs symlink? Sorry, I've reread your comment and see that you do.

rustybird commented 7 years ago

https://github.com/QubesOS/qubes-antievilmaid/pull/19

mattmccutchen commented 7 years ago

I realized after I drafted the new error message that if the PCR check fails unexpectedly during unsealing, the user won't want to boot the system in order to view /usr/share/doc/anti-evil-maid/README. This looks silly, but do we care? The use case is that before going through the full procedure to recover from a potential compromise, the user might want to see if there was a bad configuration change that can just be reversed; if unsealing then succeeds, the user knows the reversal was successful as far as the AEM security model goes. Currently, the user would have to read the readme using another device. We could try to be more helpful by enhancing /etc/grub.d/19_linux_xen_tboot to generate another boot menu option with tboot vga logging enabled and have the error message direct the user to try that option. But I'm happy to proceed with the existing change to fix the security problem and leave additional help as a possible future enhancement.

rustybird commented 7 years ago

Good point. The user definitely shouldn't be encouraged (even subconsciously) to boot an unverified system. I've force pushed an updated commit series referring to the Anti Evil Maid README instead of /usr/share/doc/anti-evil-maid/README, because e.g. viewing it on GitHub with another device is also an option. But it's true, a recovery menu would be nice...

tasket commented 7 years ago

Instead of hardcoding it to PCRs 13,17,18,19 it would be better to check the PCRs that have been specified when sealing (i.e. /etc/anti-evil-maid.conf). The AEM README does give the impression the user can pick and choose them instead of using the defaults.

tasket commented 7 years ago

To clarify, if a user has selected PCRs so as not to rely on the TXT dependant registers, then AEM should continue to work instead of failing... correct?

rustybird commented 7 years ago

@tasket:

I suppose there is a (marginal) use case for not sealing to some of the default PCRs: If tboot is incompatible, the user might want to seal only to PCR 13, to at least verify the LUKS header(s). Unconditionally requiring nonempty PCRs 17-19 would indeed break this.

So for -seal, let's verify the intersection between standard and configured PCRs.

For -unseal, even if /etc/anti-evil-maid.conf were to be made available in the initrd, it might be out of date in case the user later changed the PCRs an reran -seal (manually or due to a Xen update). And tpm-tools doesn't seem to expose a way to find out which PCRs a blob was actually sealed to. But given that the -unseal check really only needs to be a one-time thing to warn existing users with broken AEM installations, there was an easier way:

I've updated the PR to change the sealed blob filename suffix from .sealed to .sealed2 to ensure that everyone's sealed secrets really are invalidated by the upgrade.

tasket commented 7 years ago

Sealing with PCRs 0-5 is possible. This includes the BIOS, option ROM, configuration, and MBR. Are you saying this does not help the security of the boot process?

With the above changes, someone sealing to those Non-TXT PCRs would have their boot proccess halted if TXT didn't work.

rustybird commented 7 years ago

Sealing with PCRs 0-5 is possible. This includes the BIOS, option ROM, configuration, and MBR. Are you saying this does not help the security of the boot process?

Sounds good to me, at least with an external AEM device. Not sure why I didn't include the lower PCRs in the example use case, TBH I haven't really thought a lot about non-SINIT boot.

With the above changes, someone sealing to those Non-TXT PCRs would have their boot proccess halted if TXT didn't work.

Do you mean the previous version of the PR? The newer one will only verify the standard TXT/LUKS registers if they've also been configured in /etc/anti-evil-maid.conf. And it does not affect unsealing (only sealing), aside from the one time this update is applied.

mattmccutchen commented 7 years ago

Now what about the existing HCL entries that say "AEM works"? Are we confident AEM actually worked and didn't just appear to work due to this bug?

qubesos-bot commented 7 years ago

Automated announcement from builder-github

The package anti-evil-maid-3.0.5-1.fc23 has been pushed to the r3.2 testing repository for dom0. To test this update, please install it with the following command:

sudo qubes-dom0-update --enablerepo=qubes-dom0-current-testing

Changes included in this update

qubesos-bot commented 7 years ago

Automated announcement from builder-github

The package anti-evil-maid-3.0.5-1.fc23 has been pushed to the r3.2 stable repository for dom0. To install this update, please use the standard update command:

sudo qubes-dom0-update

Or update dom0 via Qubes Manager.

Changes included in this update