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.

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

Add transaction authenticator to support OIDC signature signing #11681

Merged
merged 17 commits into from Jan 19, 2024

Conversation

heliuchuan
Copy link
Contributor

@heliuchuan heliuchuan commented Jan 17, 2024

zkID is the integration of the OpenID Connect specification to do transaction signing and authentication.

This PR implements the OpenIdSig authenticator described in the "leaky mode" subsection from AIP-61, which does not use zero-knowledge proofs and is thus not privacy preserving.

This OpenIdSig path is meant to be a "dead man's switch" in the case that the ZKP path is compromised and must be turned off. It will allow users to continue to access their accounts although usage will sacrifice privacy.

ZkIdPublicKey

pub struct ZkIdPublicKey {
    /// The OIDC provider.
    pub iss: String,

    /// SNARK-friendly commitment to:
    /// 1. The application's ID; i.e., the `aud` field in the signed OIDC JWT representing the OAuth client ID.
    /// 2. The OIDC provider's internal identifier for the user; e.g., the `sub` field in the signed OIDC JWT
    ///    which is Google's internal user identifier for bob@gmail.com, or the `email` field.
    ///
    /// e.g., H(aud || uid_key || uid_val || pepper), where `pepper` is the commitment's randomness used to hide
    ///  `aud` and `sub`.
    pub idc: IdCommitment,
}

pub struct IdCommitment(pub(crate) [u8; IDC_NUM_BYTES]);

ZkIdSignature

pub struct ZkIdSignature {
    /// A \[ZKPoK of an\] OpenID signature over several relevant fields: `aud, sub, iss, nonce` where
    /// `nonce` contains a commitment to `ephemeral_pubkey`.
    pub sig: ZkpOrOpenIdSig,

    /// The header contains two relevant fields:
    ///  1. `kid`, which indicates which of the OIDC provider's JWKs should be used to verify the
    ///     \[ZKPoK of an\] OpenID signature.,
    ///  2. `alg`, which indicates which type of signature scheme was used to sign the JWT
    pub jwt_header: String,

    /// The expiry time of the `ephemeral_pubkey` represented as a UNIX epoch timestamp in seconds.
    pub exp_timestamp_secs: u64,

    /// A short lived public key used to verify the `ephemeral_signature`.
    pub ephemeral_pubkey: EphemeralPublicKey,
    /// The signature of the transaction signed by the private key of the `ephemeral_pubkey`.
    pub ephemeral_signature: EphemeralSignature,
}

pub enum EphemeralPublicKey {
    Ed25519 { public_key: Ed25519PublicKey },
}

pub enum EphemeralSignature {
    Ed25519 { signature: Ed25519Signature },
}

ZkpOrOpenIdSig

pub enum ZkpOrOpenIdSig {
    Groth16Zkp(Groth16Zkp),
    OpenIdSig(OpenIdSig),
}

OpenIdSig

pub struct OpenIdSig {
    /// The base64url encoded JWS signature of the OIDC JWT (https://datatracker.ietf.org/doc/html/rfc7515#section-3)
    pub jwt_sig: String,
    /// The base64url encoded JSON payload of the OIDC JWT (https://datatracker.ietf.org/doc/html/rfc7519#section-3)
    pub jwt_payload: String,
    /// The name of the key in the claim that maps to the user idemtifier e.g., "sub" or "email"
    pub uid_key: String,
    /// The random value used to obfuscate the the EPK from OIDC providers in the nonce field
    pub epk_blinder: [u8; EPK_BLINDER_NUM_BYTES],
    /// The privacy preserving value used to calculate the address identity commitment (IDC).  Should be unique per (iss, client_id, uid_val)
    pub pepper: Pepper,
}

pub struct Pepper(pub(crate) [u8; PEPPER_NUM_BYTES]);

Signature size

The signature size will be variable as many fields to not technically have a limit.

Total Size Estimate
OpenIdSig.jwt_sig -> ~350 bytes
OpenIdSig.jwt_payload -> ~600 bytes
OpenIdSig.uid_key -> ~5 bytes
OpenIdSig.epk_blinder -> 31 bytes
OpenIdSig.pepper -> 31 bytes
OpenIdSig = ~1017 bytes

ZkIdSignature.sig -> 1 bytes (enum) + ~1017 bytes
ZkIdSignature.jwt_header -> ~100 byte
ZkIdSignature.exp_timestamp_secs -> 8 bytes (u64)
ZkIdSignature.ephemeral_pubkey -> 34 bytes (when Ed25519 is used but up to 93 bytes)
ZkIdSignature.ephemeral_signature -> 66 bytes (when Ed25519 is used, schemes in the future could differ)
ZkIdSignature=~1226 bytes

ZkIdPublicKey.iss -> ~27 bytes (google)
ZkIdPublicKey.idc -> ~32 byte
ZkIdPublicKey=~59 bytes

Signature verification

To verify an OpenIdSig with a ZkIdSignature, the following checks must pass

  1. ZkIdPublicKey.iss must match the iss claim in OpenIdSig.jwt_payload
  2. ZkIdPublicKey.idc must match the the addresss IDC computed from OpenIdSig.jwt_payload
  3. Check that ZkIdSignature.exp_timestamp_secs < MAX_EPK_LIFESPAN_SECS + iat (the value of the iat claim OpenIdSig.jwt_payload)
  4. Check the nonce reconstructed from ZkIdSignature.exp_timestamp_secs, ZkIdSignature.ephemeral_pubkey, and OpenIdSig.epk_blinder matches the nonce claim in OpenIdSig.jwt_payload
  5. Check that ZkIdSignature.exp_timestamp_secs < now (the current time on-chain at validation time)
  6. Check that the ZkIdSignature.jwt_header.OpenIdSig.jwt_payload.OpenIdSig.jwt_sig is a valid JWT token using the corresponding JWK stored on chain (still TODO)

Again, more implementation details can be found in AIP-61.

Be sure to

  1. Run ./scripts/authenticator_regenerate.sh to regenerate all API files and protobuf files (and other dependencies when modifying the Rust authenticator code). Otherwise, you'll get CI/CD failures. If you want to learn more:
  2. ecosystem/indexer-grpc/indexer-grpc-fullnode/src/convert.rs must be manually updated

Notes

Copy link

trunk-io bot commented Jan 17, 2024

⏱️ 29h 31m total CI duration on this PR
Job Cumulative Duration Recent Runs
windows-build 6h 22m 🟩🟩🟩🟩🟩 (+20 more)
rust-unit-tests 5h 17m 🟥🟥🟩 (+20 more)
rust-move-unit-coverage 3h 11m 🟥🟥🟥🟥 (+13 more)
run-tests-main-branch 1h 59m 🟩🟩🟩🟩🟩 (+24 more)
rust-lints 1h 59m 🟥🟩🟩 (+20 more)
rust-smoke-tests 1h 49m 🟥🟥🟩 (+1 more)
forge-framework-upgrade-test / forge 1h 40m 🟥🟥
execution-performance / single-node-performance 1h 37m 🟩🟩🟩🟩🟩
check 1h 35m 🟩🟩🟩🟩 (+20 more)
general-lints 1h 1m 🟩🟩🟩🟩 (+20 more)
check-dynamic-deps 53m 🟩🟩🟩🟩🟩 (+20 more)
rust-images / rust-all 31m 🟥🟩🟩
forge-e2e-test / forge 28m 🟩🟩
forge-compat-test / forge 25m 🟩🟩
cli-e2e-tests / run-cli-tests 14m 🟩🟩
semgrep/ci 10m 🟩🟩🟩🟩🟩 (+20 more)
file_change_determinator 6m 🟩🟩🟩🟩🟩 (+24 more)
file_change_determinator 5m 🟩🟩🟩🟩🟩 (+20 more)
node-api-compatibility-tests / node-api-compatibility-tests 2m 🟩🟩
permission-check 1m 🟩🟩🟩🟩🟩 (+24 more)
permission-check 1m 🟩🟩🟩🟩🟩 (+24 more)
permission-check 1m 🟩🟩🟩🟩🟩 (+24 more)
permission-check 1m 🟩🟩🟩🟩🟩 (+24 more)
file_change_determinator 59s 🟩🟩🟩🟩🟩
execution-performance / file_change_determinator 47s 🟩🟩🟩🟩🟩
execution-performance / parallel-execution-performance 37s 🟩🟩🟩🟩🟩
execution-performance / sequential-execution-performance 35s 🟩🟩🟩🟩🟩
determine-docker-build-metadata 12s 🟩🟩🟩🟩🟩
permission-check 11s 🟩🟩🟩🟩🟩

🚨 1 job on the last run was significantly faster/slower than expected

Job Duration vs 7d avg Delta
rust-images / rust-all 13m 10m +22%

settingsfeedbackdocs ⋅ learn more about trunk.io

Copy link

codecov bot commented Jan 17, 2024

Codecov Report

Attention: 525 lines in your changes are missing coverage. Please review.

Comparison is base (8f0c41a) 69.8% compared to head (b5b7d43) 68.6%.
Report is 5 commits behind head on main.

❗ Current head b5b7d43 differs from pull request most recent head 4b6590e. Consider uploading reports for the commit 4b6590e to get more accurate results

Files Patch % Lines
types/src/zkid.rs 6.5% 170 Missing ⚠️
crates/aptos-crypto/src/poseidon_bn254.rs 0.0% 128 Missing ⚠️
aptos-move/aptos-vm/src/zkid_validation.rs 12.0% 95 Missing ⚠️
types/src/transaction/authenticator.rs 0.0% 49 Missing ⚠️
types/src/jwks/mod.rs 0.0% 34 Missing ⚠️
types/src/transaction/mod.rs 0.0% 18 Missing ⚠️
aptos-move/aptos-vm/src/aptos_vm.rs 7.1% 13 Missing ⚠️
types/src/jwks/rsa.rs 0.0% 7 Missing ⚠️
types/src/on_chain_config/aptos_features.rs 0.0% 6 Missing ⚠️
types/src/jwks/jwk.rs 0.0% 5 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main   #11681       +/-   ##
===========================================
- Coverage    69.8%    68.6%     -1.2%     
===========================================
  Files        2117      778     -1339     
  Lines      402321   178777   -223544     
===========================================
- Hits       280836   122756   -158080     
+ Misses     121485    56021    -65464     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

types/src/jwks/mod.rs Outdated Show resolved Hide resolved
aptos-move/aptos-vm/src/aptos_vm.rs Show resolved Hide resolved
aptos-move/aptos-vm/src/aptos_vm.rs Show resolved Hide resolved
aptos-move/aptos-vm/src/aptos_vm.rs Show resolved Hide resolved
aptos-move/aptos-vm/src/zkid_validation.rs Outdated Show resolved Hide resolved
aptos-move/aptos-vm/src/zkid_validation.rs Outdated Show resolved Hide resolved
aptos-move/aptos-vm/src/zkid_validation.rs Outdated Show resolved Hide resolved
aptos-move/aptos-vm/src/zkid_validation.rs Outdated Show resolved Hide resolved
third_party/move/move-core/types/src/vm_status.rs Outdated Show resolved Hide resolved
@georgemitenkov
Copy link
Contributor

georgemitenkov commented Jan 17, 2024

@heliuchuan @alinush @zjma can we please make sure we propagate any errors in the VM from storage and do not remap them please? Particularly important when getting resources from resolvers fails. In Block-STM, we catch errors and based on how bad the error is can fallback to sequential execution, etc. so propagating errors is crucial

types/src/zkid.rs Outdated Show resolved Hide resolved
types/src/zkid.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@alinush alinush left a comment

Choose a reason for hiding this comment

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

I finished my pass: really nice work @heliuchuan! Let's go through the comments one-by-one, especially the error codes and the fetching of JWKs via the Move function call.

Once everything is addressed I can stamp!

aptos-move/aptos-vm/src/zkid_validation.rs Show resolved Hide resolved
aptos-move/aptos-vm/src/zkid_validation.rs Outdated Show resolved Hide resolved
crates/aptos-crypto/src/poseidon_bn254.rs Outdated Show resolved Hide resolved
crates/aptos-crypto/src/poseidon_bn254.rs Outdated Show resolved Hide resolved
types/src/transaction/authenticator.rs Show resolved Hide resolved
@heliuchuan heliuchuan enabled auto-merge (squash) January 19, 2024 19:14

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

✅ Forge suite compat success on aptos-node-v1.8.3 ==> 4b6590e2856219dfe2d0c79d308d12f1a99a0664

Compatibility test results for aptos-node-v1.8.3 ==> 4b6590e2856219dfe2d0c79d308d12f1a99a0664 (PR)
1. Check liveness of validators at old version: aptos-node-v1.8.3
compatibility::simple-validator-upgrade::liveness-check : committed: 5426 txn/s, latency: 6184 ms, (p50: 6300 ms, p90: 8800 ms, p99: 11700 ms), latency samples: 195340
2. Upgrading first Validator to new version: 4b6590e2856219dfe2d0c79d308d12f1a99a0664
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 1790 txn/s, latency: 16492 ms, (p50: 18400 ms, p90: 22300 ms, p99: 22400 ms), latency samples: 93080
3. Upgrading rest of first batch to new version: 4b6590e2856219dfe2d0c79d308d12f1a99a0664
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 1778 txn/s, latency: 16100 ms, (p50: 19200 ms, p90: 22000 ms, p99: 22600 ms), latency samples: 92480
4. upgrading second batch to new version: 4b6590e2856219dfe2d0c79d308d12f1a99a0664
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 3271 txn/s, latency: 9174 ms, (p50: 9900 ms, p90: 13600 ms, p99: 14200 ms), latency samples: 137400
5. check swarm health
Compatibility test for aptos-node-v1.8.3 ==> 4b6590e2856219dfe2d0c79d308d12f1a99a0664 passed
Test Ok

Copy link
Contributor

✅ Forge suite realistic_env_max_load success on 4b6590e2856219dfe2d0c79d308d12f1a99a0664

two traffics test: inner traffic : committed: 7395 txn/s, latency: 5179 ms, (p50: 4500 ms, p90: 6800 ms, p99: 13000 ms), latency samples: 3195020
two traffics test : committed: 100 txn/s, latency: 2492 ms, (p50: 2100 ms, p90: 2600 ms, p99: 9400 ms), latency samples: 1700
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.219, avg: 0.198", "QsPosToProposal: max: 0.211, avg: 0.140", "ConsensusProposalToOrdered: max: 0.558, avg: 0.531", "ConsensusOrderedToCommit: max: 0.477, avg: 0.457", "ConsensusProposalToCommit: max: 1.021, avg: 0.988"]
Max round gap was 1 [limit 4] at version 1500542. Max no progress secs was 6.48999 [limit 10] at version 1500542.
Test Ok

@heliuchuan heliuchuan merged commit 354e997 into main Jan 19, 2024
51 of 52 checks passed
@heliuchuan heliuchuan deleted the alin/zkid branch January 19, 2024 22:38
Copy link
Contributor

❌ Forge suite framework_upgrade failure on aptos-node-v1.8.3 ==> 4b6590e2856219dfe2d0c79d308d12f1a99a0664

Compatibility test results for aptos-node-v1.8.3 ==> 4b6590e2856219dfe2d0c79d308d12f1a99a0664 (PR)
Upgrade the nodes to version: 4b6590e2856219dfe2d0c79d308d12f1a99a0664
Test Failed: API error: Unknown error error sending request for url (http://aptos-node-3-validator.forge-framework-upgrade-pr-11681.svc:8080/v1/accounts/0000000000000000000000000000000000000000000000000000000000000001/resource/0x1::block::BlockResource): error trying to connect: dns error: failed to lookup address information: Name or service not known

Stack backtrace:
   0: <unknown>
   1: <unknown>
   2: <unknown>
   3: <unknown>
   4: <unknown>
   5: <unknown>
   6: <unknown>
   7: <unknown>
   8: <unknown>
   9: <unknown>
  10: <unknown>
  11: <unknown>
  12: <unknown>
  13: <unknown>
  14: __libc_start_main
  15: <unknown>
Trailing Log Lines:
   7: <unknown>
   8: <unknown>
   9: <unknown>
  10: <unknown>
  11: <unknown>
  12: <unknown>
  13: <unknown>
  14: __libc_start_main
  15: <unknown>


Swarm logs can be found here: See fgi output for more information.
thread 'main' panicked at testsuite/forge/src/backend/k8s/swarm.rs:696:18:
called `Result::unwrap()` on an `Err` value: ApiError: namespaces "forge-framework-upgrade-pr-11681" not found: NotFound (ErrorResponse { status: "Failure", message: "namespaces \"forge-framework-upgrade-pr-11681\" not found", reason: "NotFound", code: 404 })

Caused by:
    namespaces "forge-framework-upgrade-pr-11681" not found: NotFound

Stack backtrace:
   0: <unknown>
   1: <unknown>
   2: <unknown>
   3: <unknown>
   4: <unknown>
   5: <unknown>
   6: <unknown>
   7: <unknown>
   8: <unknown>
   9: <unknown>
  10: <unknown>
  11: <unknown>
  12: <unknown>
  13: <unknown>
  14: <unknown>
  15: __libc_start_main
  16: <unknown>
stack backtrace:
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
Debugging output:

return Ok(());
}

if zkid_authenticators.len() > MAX_ZK_ID_AUTHENTICATORS_ALLOWED {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we set a limit on the max number of authenticators?

Copy link
Contributor

Choose a reason for hiding this comment

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

They are much more expensive to verify compared to a normal authenticator (Groth16 ZKP vs Ed25519; 2ms versus 40 microseconds)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants