feat: add utility to verify attestation #20017
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
joyqvq
force-pushed
the
joy/attestation
branch
from
0a7034c to
8f86df1
Compare
joyqvq
force-pushed
the
joy/attestation
branch
from
8f86df1 to
90a91c6
Compare
joyqvq
force-pushed
the
joy/attestation
branch
2 times, most recently
from
f056421 to
de1eacd
Compare
joyqvq
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
joyqvq
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
joyqvq
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
joyqvq
force-pushed
the
joy/attestation
branch
from
0f9b9a8 to
0a911f0
Compare
joyqvq
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
joyqvq
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
joyqvq
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
joyqvq
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
joyqvq
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
joyqvq
force-pushed
the
joy/attestation
branch
from
7734654 to
63320e2
Compare
joyqvq
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
joyqvq
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
joyqvq
force-pushed
the
joy/attestation
branch
from
272320e to
953eb3a
Compare
joyqvq
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
feel free to review this pr as is and i will address before merging. while i understand we should align across teams, this pr itself should be self contained and reviewed since it doesnt expose to move interfaces. |
Oh i didn't realize that, I thought the move interface was still a part of the pr. I'll remove my gating review then |
| let cose_sign1 = CoseSign1::parse_and_validate(attestation_bytes)?; | ||
|
|
||
| // Parse attestation document payload and verify cert against AWS root of trust. | ||
| let doc = |
There was a problem hiding this comment.
let's move this parsing to after the sig verification (to reduce the risk of complex parsing)
There was a problem hiding this comment.
the public key used for verifying signature is parsed from the cert in the doc
| /// Given an attestation document bytes, deserialize and verify its validity according to | ||
| /// <https://docs.aws.amazon.com/enclaves/latest/user/verify-root.html> | ||
| /// and check the user_data is consistent with the enclave public key. | ||
| pub fn attestation_verify_inner( |
There was a problem hiding this comment.
let's change this function to return SuiResult and only give it the attestation_bytes and timestamp? later we could expose both in move to developers using something like
Struct NitroAttestation { user_data: vector<u8>, pcrs: vector<Pcr> }
There was a problem hiding this comment.
agree that the move interface can take i the struct, but shouldn't the native function take the raw types?
There was a problem hiding this comment.
yes, the native function will take and return raw types, something like attestation_verify_inner(attetation_bytes, timestamp) -> SuiResult<vec<u8>, vec<vec<u8>>)>.
Later the move struct would be the object we will construct in the function that calls the native function, i.e.
public fun create_nitro_attestation(bytes, bytes...): NitroAttestation { // call native function to get user data and pcrs -> construct NitroAttestation
sui-execution/latest/sui-move-natives/src/crypto/attestation.rs
Outdated
Show resolved
Hide resolved
joyqvq
force-pushed
the
joy/attestation
branch
from
8b6d4ae to
770151e
Compare
joyqvq
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
joyqvq
force-pushed
the
joy/attestation
branch
from
770151e to
482f701
Compare
joyqvq
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
joyqvq
force-pushed
the
joy/attestation
branch
from
482f701 to
256a237
Compare
joyqvq
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
joyqvq
force-pushed
the
joy/attestation
branch
from
256a237 to
4df3c9d
Compare
joyqvq
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
joyqvq
force-pushed
the
joy/attestation
branch
from
4df3c9d to
9915a98
Compare
joyqvq
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
|
|
||
| // Verify certificate signature. | ||
| let verifying_key = match issuer_cert.public_key().parsed() { | ||
| Ok(PublicKey::EC(ec)) => VerifyingKey::from_sec1_bytes(ec.data()).map_err(|_| { |
There was a problem hiding this comment.
The previous implementation worked with any signature scheme for the cert chain, no? is it safe to assume that future cert chains will use p384 as well? i think it's ok, but let's document the right reference for that.
|
|
||
| /// Validate the certificate chain against the root of trust. | ||
| fn validate_cert_chain(cert_chain: &[&[u8]], now_ms: u64) -> Result<(), NitroError> { | ||
| if cert_chain.is_empty() || cert_chain.len() > 20 { |
There was a problem hiding this comment.
let's change it to 10 now to be on the safer side
Description
Describe the changes or additions included in this PR.
Test plan
How did you test the new or updated feature?
Release notes
Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.
For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.