chipsalliance / caliptra-sw

Caliptra software (ROM, FMC, runtime firmware), and libraries/tools needed to build and test
Apache License 2.0
94 stars 44 forks source link

Owner pub key is only verified if the key is programmed in fuses, and is not pinned across hitless updates #222

Closed bluegate010 closed 1 year ago

bluegate010 commented 1 year ago

In verify_owner_pk_digest:

let expected = self.env.owner_pub_key_digest(image);

// Skip Verification if owner public key digest is zero
if expected == ZERO_DIGEST {
    return Ok(None);
}

// Hash image's owner pub key
let actual = ...

if expected != actual {
    raise_err!(OwnerPubKeyDigestMismatch)
}

if reason == ResetReason::UpdateReset {
    let cold_boot_digest = self.env.owner_pub_key_digest(image);
    if cold_boot_digest != expected {
        raise_err!(UpdateResetOwnerDigestFailure)
    }
}

The owner_pub_key_digest function inspects the fuse map, which may not be programmed (will often be the case if the owner wishes to resell the SoC). If the owner pub key is not in fuses, verify_owner_pk_digest() returns early indicating that there is no owner signature to check. However, owner signature verification should be performed even if the owner key is not in fuses, to allow for volatile ownership transfer flows.

In addition, the hitless-update check at the end invokes self.env.owner_pub_key_digest() a second time, which renders the check superfluous. This should likely invoke self.env.owner_pub_key_digest_dv() instead, to query what was written to DataVault on last cold boot.

This routine should probably read,

// Hash image's owner pub key
let actual = ...

let fuses_digest = self.env.owner_pub_key_digest(image);

if fuses_digest != ZERO_DIGEST && fuses_digest != actual {
    raise_err!(OwnerPubKeyDigestMismatch)
}

if reason == ResetReason::UpdateReset {
    let cold_boot_digest = self.env.owner_pub_key_digest_dv();
    if cold_boot_digest != actual {
        raise_err!(UpdateResetOwnerDigestFailure)
    }
}

Ok(actual)
varuns-nvidia commented 1 year ago

Requesting that the ROM spec be updated too, it currently states in the "Preamble Validation Steps" flowchart that boot will fail if the owner key is not valid