Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: add utility to verify attestation #20017

Merged
merged 3 commits into from
Jan 16, 2025
Merged

feat: add utility to verify attestation #20017

merged 3 commits into from
Jan 16, 2025

Conversation

joyqvq
Copy link
Contributor

@joyqvq joyqvq commented Oct 24, 2024 •
edited
Loading

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.

  • Protocol:
  • Nodes (Validators and Full nodes):
  • JSON-RPC:
  • GraphQL:
  • CLI:
  • Rust SDK:

@vercel Vercel
Copy link

vercel bot commented Oct 24, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
sui-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 16, 2025 4:48pm
3 Skipped Deployments
Name Status Preview Comments Updated (UTC)
multisig-toolkit ⬜️ Ignored (Inspect) Visit Preview Jan 16, 2025 4:48pm
sui-kiosk ⬜️ Ignored (Inspect) Visit Preview Jan 16, 2025 4:48pm
sui-typescript-docs ⬜️ Ignored (Inspect) Visit Preview Jan 16, 2025 4:48pm

@joyqvq joyqvq force-pushed the joy/attestation branch 2 times, most recently from f056421 to de1eacd Compare October 31, 2024 21:31
@joyqvq joyqvq temporarily deployed to sui-typescript-aws-kms-test-env October 31, 2024 21:31 — with GitHub Actions Inactive
@joyqvq joyqvq temporarily deployed to sui-typescript-aws-kms-test-env October 31, 2024 22:41 — with GitHub Actions Inactive
@joyqvq joyqvq temporarily deployed to sui-typescript-aws-kms-test-env October 31, 2024 22:45 — with GitHub Actions Inactive
@joyqvq joyqvq temporarily deployed to sui-typescript-aws-kms-test-env November 1, 2024 18:11 — with GitHub Actions Inactive
@joyqvq joyqvq temporarily deployed to sui-typescript-aws-kms-test-env November 1, 2024 19:16 — with GitHub Actions Inactive
@joyqvq joyqvq temporarily deployed to sui-typescript-aws-kms-test-env November 1, 2024 19:23 — with GitHub Actions Inactive
@joyqvq joyqvq temporarily deployed to sui-typescript-aws-kms-test-env November 1, 2024 21:55 — with GitHub Actions Inactive
@joyqvq joyqvq temporarily deployed to sui-typescript-aws-kms-test-env November 1, 2024 22:18 — with GitHub Actions Inactive
@joyqvq joyqvq temporarily deployed to sui-typescript-aws-kms-test-env November 1, 2024 22:18 — with GitHub Actions Inactive
@joyqvq joyqvq temporarily deployed to sui-typescript-aws-kms-test-env November 1, 2024 23:56 — with GitHub Actions Inactive
@joyqvq joyqvq temporarily deployed to sui-typescript-aws-kms-test-env November 4, 2024 22:07 — with GitHub Actions Inactive
benr-ml
benr-ml approved these changes Jan 14, 2025
View reviewed changes
Copy link
Contributor

@benr-ml benr-ml left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, left small naming suggestions

crates/sui-types/src/nitro_attestation.rs Outdated Show resolved Hide resolved
crates/sui-types/src/nitro_attestation.rs Outdated Show resolved Hide resolved
crates/sui-types/src/nitro_attestation.rs Show resolved Hide resolved
crates/sui-types/src/nitro_attestation.rs Outdated Show resolved Hide resolved
crates/sui-types/src/nitro_attestation.rs Outdated Show resolved Hide resolved
crates/sui-types/src/nitro_attestation.rs Outdated Show resolved Hide resolved
crates/sui-types/src/nitro_attestation.rs
impl CoseSign1 {
/// Parse CBOR bytes into struct. Adapted from <https://github.com/awslabs/aws-nitro-enclaves-cose/blob/main/src/sign.rs>
pub fn parse_and_validate(bytes: &[u8]) -> Result<Self, NitroAttestationVerifyError> {
let tagged_value: ciborium::value::Value = ciborium::de::from_reader(bytes)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

general question - is ciborium decoding linear in the input? (if it's quadratic or worse we might want more checks)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes its linear- the parsing basically iterate the entire document_map per variable name to parse each value. we charge per bytes of the total attestation bytes.

however for the pcrs, it is quadratic wrt to the number of pcrs here. but there should only be 6 valid indices so its bounded.

crates/sui-types/src/unit_tests/nitro_attestation_tests.rs Show resolved Hide resolved
@joyqvq joyqvq temporarily deployed to sui-typescript-aws-kms-test-env January 15, 2025 14:52 — with GitHub Actions Inactive
@joyqvq
Copy link
Contributor Author

joyqvq commented Jan 15, 2025 

attestation/parse_attestation
                        time:   [6.3399 µs 6.3472 µs 6.3545 µs]
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high severe
attestation/verify_p384_single_cert
                        time:   [909.87 µs 922.26 µs 939.28 µs]
Found 14 outliers among 100 measurements (14.00%)
  4 (4.00%) high mild
  10 (10.00%) high severe
attestation/parse_and_verify_attestation_with_entire_cert_chain
                        time:   [3.8098 ms 3.8145 ms 3.8197 ms]
Found 15 outliers among 100 measurements (15.00%)
  6 (6.00%) high mild
  9 (9.00%) high severe

each cert is verified roughly with the time (3814 - 922 - 6) / 4 = 721 µs

@joyqvq joyqvq temporarily deployed to sui-typescript-aws-kms-test-env January 15, 2025 14:54 — with GitHub Actions Inactive
@joyqvq joyqvq temporarily deployed to sui-typescript-aws-kms-test-env January 15, 2025 15:01 — with GitHub Actions Inactive
@joyqvq joyqvq temporarily deployed to sui-typescript-aws-kms-test-env January 15, 2025 20:07 — with GitHub Actions Inactive
@joyqvq joyqvq temporarily deployed to sui-typescript-aws-kms-test-env January 16, 2025 15:27 — with GitHub Actions Inactive
@joyqvq joyqvq enabled auto-merge (squash) January 16, 2025 15:27
@joyqvq joyqvq temporarily deployed to sui-typescript-aws-kms-test-env January 16, 2025 15:36 — with GitHub Actions Inactive
@joyqvq joyqvq temporarily deployed to sui-typescript-aws-kms-test-env January 16, 2025 16:47 — with GitHub Actions Inactive
@joyqvq joyqvq merged commit 39e1000 into main Jan 16, 2025
51 checks passed
@joyqvq joyqvq deleted the joy/attestation branch January 16, 2025 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benr-ml benr-ml benr-ml approved these changes

@jnaulty jnaulty Awaiting requested review from jnaulty

@mystenmark mystenmark Awaiting requested review from mystenmark

@chargarlic chargarlic Awaiting requested review from chargarlic

@bmwill bmwill Awaiting requested review from bmwill

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@joyqvq @bmwill @mystenmark @benr-ml @chargarlic