snapcore/secboot

secboot's Issues

SealKeyToTPM and UpdateKeyPCRProtectionPolicy should reject a profile that generates a policy with unsupported PCRs or algorithms

PCRProtectionProfile allows arbitrary PCR policies to be created. When these policies are computed by SealKeyToTPM and UpdateKeyPCRProtectionPolicy, an error should be returned if those policies include values for PCRs or algorithms that are unsupported by the TPM.

WithSecureBootPolicyProfile should accommodate systems that support timestamp revocation

WithSecureBootPolicyProfile will generate an incorrect profile on systems that support timestamp revocation (ie, that have a dbt database), not that I've seen any of these devices yet. This is detected explicitly in the new pre-install checks I'm working on, disabling support for WithSecureBootPolicyProfile in this case, but support for dbt should probably be added.

Add an additional cases to TestUnsealErrorHandling

There's a couple of cases that should be tested in TestUnsealErrorHandling and aren't:

• Missing PIN NV index (should result in a failure with InvalidKeyFileError).
• Missing lock NV index (should result in a failure with ErrTPMProvisioning).
• Some additional cases with broken metadata.

Switch tests to use gopkg.in/check.v1

The tests are currently just using go's testing package.

Use snap.Container.RandomAccessFile

We currently use snap.Container.ReadFile, but RandomAccessFile provides access to a io.ReaderAt interface which is more appropriate for our requirements.

Add tests for UpdateKeyPCRProtectionPolicy

There currently aren't any tests for this.

Please specify the license

This repo doesn't mention about any license. I would be helpful if you can specify the license (MIT, BSD, GPL etc.) for this code

tpm2: try a transient primary key if the sealed key object fails to load for any reason

The TPM unsealing code will fall back to trying with a transient primary key if the initial unsealing attempt with the persistent primary key fails in some cases - if the TPM2_Load command fails with a handle error, indicating that the supplied parent handle is not a valid storage key.

But it fails immediately if TPM2_Load returns a TPM_RC_INTEGRITY error for the inPrivate parameter, which may indicate that although the parent handle corresponds to a valid storage key, it may not be the correct parent key. This might happen if other software that uses the TPM evicts the primary key we create during install and then creates a new one with a non-standard template.

The unsealing code should probably adopt a belt and braces approach to error handling here and fall back to trying with a transient primary key if the TPM2_Load command fails for any reason, rather than implementing handling that is specific to each error type. I would also extend the existing error handling to work like this:

• Initial try with the persistent shared SRK at the well-known handle (0x81000001).
• If that fails, try every persistent key on the TPM.
• If that fails, try creating a transient SRK.

Add WithPlatformFirmwareConfigProfile() (for PCR1)

If PCR7 isn't supported for some reason, eg, because secure boot is disabled or the device is not in deployed mode, then we should be able to fall back to a safe profile that includes PCRs 1, 2 and 4 and 5. PCR 2 and 4 are for binding to all code that's loaded outside of the platform firmware.

PCR1 is to bind to all of the platform firmware's configuration - currently, changes to security-sensitive settings such as enabling a debugger or disabling DMA remapping change the value of PCR7 by adding extra EVI_EFI_ACTION events.

PCR5 contains the GPT of the IBL and can be used for any bootloader configuration, and support for this will be tracked in a separate issue.

SealedKeyObject.UnsealFromTPM should check that the sealed key object's name algorithm is valid

An invalid name algorithm for the sealed key object will result in an error being returned from TPMContext.StartAuthSession, but this error should be a InvalidKeyFileError error.

Use Resource Manager (`/dev/tpmrrm0`) rather then `/dev/tpm0`

Hi,

The snapd reports the following error message when an application snap tries to open up the /dev/tpm0.
stateengine.go:149: state ensure error: devicemgr: cannot connect to TPM: no TPM2 device is available

Any application snap could block the use of /dev/tpm0 and therefore, we might consider using the resource manager (/dev/tpmrmx) rather than using the /dev/tpm0 as described here => https://github.com/snapcore/secboot/blob/master/internal/tcti/tcti.go#L29

In addition to that, potentially, we might want to change the snapd's tpm interface to block access to /dev/tpmx directly. => https://github.com/snapcore/snapd/blob/master/interfaces/builtin/tpm.go#L36

What do you think?

Thanks,
Bugra

Add support for connecting to the TPM via the kernel resource manager

We currently connect to /dev/tpm0, which only one process can access at a time. Whilst this is fine during install and early boot, we should make use of the in-kernel resource manager (/dev/tpmrm0) at other times in order to facilitate the ability for permitted snaps to make use of the TPM too.

Investigate switching to go.mozilla.org/pkcs7

We currently depend on a fork of github.com/fullsailor/pkcs7 which contains a bug fix to work with Authenticode signatures. It seems that Mozilla have a better maintained fork of this repo, and we should investigate switching to it instead.

Remove internal use of ProvisionStatus

ProvisionStatus is used internally in one place - at the start of ProvisionTPM, and only to check whether owner clear has been disabled. It doesn't make use of any other of the returned attributes. This should be removed in favour of just attempting the TPM2_Clear command and checking if the returned error is a *tpm2.TPMError with an error code of tpm2.ErrorDisabled.

WithSecureBootPolicyProfile should support user mode

WithSecureBootPolicyProfile only generates valid profiles on systems that implement UEFI >= 2.5 if deployed mode is enabled. It should support devices that are in user mode as well, as it does already for systems that implement UEFI < 2.5. The pre-install checks I'm working on right now catch this case and disable support for WithSecureBootPolicyProfile, but it should be relaxed to not require that deployed mode is enabled.

We may provide an option to make deployed mode a requirement. Note that the UEFI specification only permits platform specific mechanisms to exit deployed mode, so it's possible that enabling it is a non-reversible operation on some hardware.

Tests do not need cryptographically secure random numbers in most cases

A bunch of tests where SealKeyToTPM is used generate cryptographically secure random numbers for each individual test. This is unnecessary, and they should use less secure random numbers (using math/rand?)

SealedKeyObject.UnsealFromTPM should return a more useful error if the final TPM2_PolicyNV fails in some circumstances

If the final TPM2_PolicyNV command fails with an authorization failure, then UnsealFromTPM should return ErrTPMProvisioning rather than the error returned from go-tpm2.

Add additional tests for EK cert verification functions in tpm.go

The functions get some limited testing already, as the tests certify the TPM simulator with a EK certificate and use the secure connection path, but they could do with some additional unit tests.

WithSecureBootPolicyProfile should support systems that include new SPDM related measurements

These are described in newer versions of the TCG PFP spec (since 1.06), and currently unsupported by WithSecureBootPolicyProfile. The new pre-install checks will catch this and disable support for WithSecureBootPolicyProfile, but we will have to add
support for these new measurements at some point.

Expose an API for validating a key file

There is an internal API for validating key files already. Ideally this would be improved to make it more useful and then exposed for diagnostic purposes in the event that the TPM sealed key could not be unsealed, and should provide a hint as to whether the TPM isn't provisioned correctly, a new TPM sealed key is required, or the metadata associated with the PCR policy is invalid.

Test that ProvisionTPM doesn't leave NV indices defined on failure

ProvisionTPM defiens 2 NV indices, and there is code to undefine these if the function returns early with an error. There aren't any tests that ensure this works though.

Make SnapFileEFIImage work with AddEFIBootManagerProfile

The handle returned from SnapFileEFIImage.Open doesn't support the Size() method required by AddEFIBootManagerProifle, and will panic. This requires a change in snapd.

Rename references to "PIN" to "passphrase"

The PIN can be an arbitrary passphrase - nowhere is it enforced that it is actually a PIN. Whilst consumers of this package could enforce this, we don't and so references to "PIN" should just be renamed to "passphrase" everywhere it appears.

Add metadata format compatibility tests

We should have a set of tests that check unsealing, pin changing and PCR profile updating work correctly on a keyfile that is checked in to the git repository to ensure that we don't make backwards incompatible changes to the metadata format.

Additional keyfiles should be checked in each time the metadata format version is bumped.

This is slightly complicated by the fact that keyfiles are associated with a TPM owner, and TPM ownership changes with each test run (when using a simulator). We'll need to check in the associated TPM simulator nvram image and figure out a way to run the simulator with that just for these tests.

SealKeyToTPM should take an array rather than a slice for the key

The key is supplied to SealKeyToTPM via a byte slice. The first thing that SealKeyToTPM does is check that the supplied key is the correct length (32 bytes), but it would probably be better if the key was supplied as a 32byte array instead ([32]byte).

Add README

The repository doesn't have a readme file yet, and should do.

Add API to add PCR profile for UEFI boot manager code and boot attempts (PCR4)

Sealing a key against only the UEFI secure boot policy (PCR7) component of the pre-OS environment requires that a system is booted without the Microsoft UEFI CA chain of trust, because that can verify versions of shim prior to rhboot/shim@8af7c4c. In order to support devices where an alternative chain of trust cannot be supported, we might need to also seal against the UEFI boot manager code and boot attempts PCR (PCR4) just for devices that can only use the Microsoft chain of trust. This means we need an API that can compute the correct digests and add them to a PCRProtectionProfile.

Possible bug? "no bootable paths with current EFI signature database"

Trying Ubuntu Core 20 (image created by myself) with secboot and disk encryption enabled, I get following fatal error message twice on initial setup:

taskrunner.go: 271: [change 2 "Setup system for run mode" task] failed: cannot make system runnable: cannot seal the encryption keys: cannot add EFI secure boot policy profile: cannot compute secure boot policy profile: no bootable paths with current EFI signature database.

The message is the same as in https://bugs.launchpad.net/intel/+bug/1938678/comments/39 .

In my setup, the UC20 disk is not the main hard drive (but making it the only drive does also not help) on the amd64/x86 box. I added the UC20 boot entries manually to the UEFI Boot setup.

Any idea if this is a bug in secboot or if some other system settings etc. need to be changed?

WithSecureBootPolicyProfile and WithBootManagerCodeProfile should support shim's revocations.efi binary

Shim has the ability to apply SBAT revocations that are wrapped up in a separate signed PE image. We need support for this in the profile generation, as verifying the new binary will result in additional verification and measurements to PCR4 even though no code is executed.

Provide a PCR protection profile hash

Resealing is a critical operation and we wish to do it as little as possible. We need a mechanism to decide if the operation is necessary before updating, and one simple way to do it would be to obtain a hash of the PCR protection profile and compare it to a previously stored value.

The application could compute a hash based on the data it provides to secboot, however it would be cleaner and more accurate if secboot could provide such a digest (perhaps a Hash() call based on the final profile instructions, similar to what String() does?)

Perhaps there are better ways to achieve the same goal, so it can be something completely different as long as we have a way to update the profile only if necessary.

Add nil checks on arguments to AddEFISecureBootPolicyProfile

readAndValidateLockNVIndexPublic needs to attempt a TPM2_NV_Read command

readAndValidateLockNVIndexPublic is used to ensure that the supplied NV index is safe to use in an authorization policy during key sealing, but it isn't safe if the index has a non-empty authorization value even if the public area appears to be ok, as a non-empty authorization value will cause TPM2_NV_ReadLock to fail.

Don't try to be too clever on failure in UnsealFromTPM

Currently, UnsealFromTPM tries to determine between an invalid key file or a TPM provisioning issue, and executePolicySession goes even further - trying to determine whether the static metadata or PCR policy metadata is invalid, although that isn't exposed at the moment. I think this should just return a single error on unseal failure (InvalidKeyFile), and then it should be up to the running system to perform additional diagnostics (see #33)

UC20 can't be installed on Dell edge 3000/5000 due to TPM error

When trying to install UC20 on Dell edge 3000 gateway, it encounters the TPM error messages “cannot compute secure boot policy profile: the TCG event log does not have the requested algorithm".

Chris commented eariler:
Given the value of /sys/kernel/security/tpm0/binary_bios_measurements (binary form), it seems like that TPM is suggested to be disabled in the firmware…

AddEFISecureBootPolicyProfile should consider contents of dbx

AddEFISecureBootPolicyProfile has to determine which CA certificate will be used to authenticate an image in order to compute the corresponding authentication measurement. For dual-signed binaries, it can guess incorrectly because it doesn't consider the contents of the forbidden signature database.

Add FetchEKCertificateChain

Add a FetchEKCertificateChain function that returns (*x509.Certificate, []*x509.Certificate, error), which behaves a bit like FetchAndSaveEKCertificateChain but without the save-to-file step.

Add WithBootManagerCodeConfigProfile() (PCR 5)

If PCR7 isn't supported for some reason, eg, because secure boot is disabled or the device is not in deployed mode, then we should be able to fall back to a safe profile that includes PCRs 1, 2 and 4 and 5. PCR 2 and 4 are for binding to all code that's loaded outside of the platform firmware.

PCR1 is to bind to all of the platform firmware's configuration - currently, changes to security-sensitive settings such as enabling a debugger or disabling DMA remapping change the value of PCR7 by adding extra EVI_EFI_ACTION events.

PCR5 contains the GPT of the IBL and can also be used for any OS bootloader configuration, although security sensitive bootloader settings should probably also modify the value of PCR7 in the same way that disabling DMA remapping does.

Support non-EFI devices

Currently secboot only supports EFI-based secure boot. But, there are other implementations, for instance, signed u-boot with signed FIT image. I wonder if it's possible to decouple EFI from secboot to support such devices.

I haven't yet read all the related codes, and just have some initial thoughts about the implementation/modification:

• how to verify secure boot is enabled for non-EFI systems
• allow to skip the check of secure boot (maybe depend on the grade?)
• implement other policy/stub profile generation for different boot-loader

Any thoughts?

AddEFISecureBootPolicyProfile should return the chains of trust added to the profile

AddEFISecureBootPolicyProfile should return the chains of trust to which the profile protects to, so that the caller can detect chains (that perhaps come from drivers or system prep applications) that may require changes to the profile.

Add PCR profile for sealing to a specific snap model

Some properties of the snap model are going to be measured by the initramfs to PCR12, and the initramfs will perform integrity checking of the system against this model in some circumstances before handing off the boot to later stages. There needs to be an API for adding this to PCRProtectionProfile.

AddEFISecureBootPolicyProfile should support computing events for images authenticated with a MOK

AddEFISecureBootPolicyProfile currently doesn't support computing events for images that are authenticated using a MOK.

Add additional tests for sealing / unsealing with more complex PCR profiles

WithBootManagerCodeProfile needs changes to accommodate TCG PFP spec v1.06

Previous versions of the TCG PFP spec describe a EV_EFI_ACTION event "Calling EFI Application from Boot Option" optionally measured to PCR4 before the first launch event. If this exists in the current log, we copy it to the computed profile.

v1.06 of this spec adds an additional EV_EFI_ACTION event - "Booting to <Boot####> Option". Whilst the current WithBootManagerCodeProfile will also copy this event to the computed profile if it exists in the current log, there needs to be an API to specify which boot entry will be used to boot a specific path so that we predict the correct PCR values, rather than just blindly copying the entry in the log, which might contain the wrong boot number. This needs coordinating with whatever is writing to BootXXXX, BootOrder and BootNext variables.

Right now, the pre-install checks I'm working on will detect this and disable support for WithBootManagerCodeProfile, but we need to fix this fairly urgently

Please add FIPS arguments

Some of the cryptsetup options are not FIPS compliant, thus please add detection for FIPS mode and use more FIPS appropriate options.

I will provide a pull request, filing this just to not forget.

validateKeyData always fails after LockAccessToSealedKeys is called

After LockAccessToSealedKeys is called, the sealed key authorization policy validation check fails because the expected policy is computed from the current name of the lock NV index, which is wrong once the read lock attribute has been set.

UC20 can't be installed on Dell Embedded Box PC 3000 due to TPM error

When trying to install UC20 on Dell Embedded Box PC 3000 it encounters the TPM error messages “cannot identify initial OS launch verification event: boot manager image load event occurred without a preceding verification event".

TPMDeviceAttributes.FirmwareVersion should be uint64

Although the format of the firmware version is manufacturer specific, it is represented in the TPM as a uint64. The parseTPMDeviceAttributesFromDirectoryName() function in tpm.go extracts the firmware version from the EK certificate, but currently interprets this as a uint32, which isn't correct.