Sign a new attest key under the caller's attest key instead of the keybox - #263
Merged
Conversation
…ybox `generateKey` mints every `ATTEST_KEY`-purpose key in the TA and, until now, always self-attested it under the keybox, discarding any attestation key keystore2 injected with the request. That is right for a bare attestation key -- keystore2 injects an RKP-provisioned hardware key we cannot re-root, so rooting the new key at the keybox is the only option -- but wrong once the caller has built a key graph with [`setAttestKeyAlias`](<https://developer.android.com/reference/android/security/keystore/KeyGenParameterSpec.Builder#setAttestKeyAlias(java.lang.String)>). There the caller names an attest key A and expects the new key B's leaf to be signed by A, so that verifying B against A's public key succeeds. When A is one of ours, self-attesting B under the keybox breaks the graph: B's leaf carries A's subject as its issuer but a keybox signature, and the chain the caller assembled verifies under neither A nor the keybox. The attest-key branch now checks whether the injected attestation key is ours and, if so, signs B's leaf with it, keeping the A -> B relationship intact; it falls back to keybox self-attestation only when no key was injected or the injected one is a foreign RKP key we cannot re-root. The ordinary business-key path already did this for leaves that are not themselves attest keys -- this extends the same rule to the attest-key case.
The header above the ATTEST_KEY block still read as the old absolute behaviour -- "always IGNORES any injected attest key" and "Forward std::nullopt: the TA self-attests under the keybox" -- which the ours-key branch just below now contradicts on its first line. Reword it to state the split (mint in the TA; use the injected key only when it is ours, else self-attest under the keybox) and let the two inner comments carry the detail. The forced-generation log line likewise said "ignoring any injected attest key", true only in the fallback now, so it reads "no usable injected attest key". Comment and log text only.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
generateKeymints everyATTEST_KEY-purpose key in the TA and, until now, always self-attested it under the keybox, discarding any attestation key keystore2 injected with the request. That is right for a bare attestation key -- keystore2 injects an RKP-provisioned hardware key we cannot re-root, so rooting the new key at the keybox is the only option -- but wrong once the caller has built a key graph withsetAttestKeyAlias. There the caller names an attest key A and expects the new key B's leaf to be signed by A, so that verifying B against A's public key succeeds. When A is one of ours, self-attesting B under the keybox breaks the graph: B's leaf carries A's subject as its issuer but a keybox signature, and the chain the caller assembled verifies under neither A nor the keybox.The attest-key branch now checks whether the injected attestation key is ours and, if so, signs B's leaf with it, keeping the A -> B relationship intact; it falls back to keybox self-attestation only when no key was injected or the injected one is a foreign RKP key we cannot re-root. The ordinary business-key path already did this for leaves that are not themselves attest keys -- this extends the same rule to the attest-key case.