cherry pick keyless into v1.10.1 (#12616)

* Fix public_inputs_hash generation when extra_field not set and serde deserialization bugs (#12476)

* [keyless] update feature gating & match alg field (#12521)


---------

Co-authored-by: Oliver He <[email protected]>

CODEOWNERS

@@ -10,9 +10,12 @@
 /aptos-move/aptos-aggregator/ @georgemitenkov @gelash @zekun000
 /aptos-move/aptos-gas/ @vgao1996
 /aptos-move/aptos-vm/ @davidiw @wrwg @zekun000 @vgao1996 @georgemitenkov
+/aptos-move/aptos-vm/src/keyless_validation.rs @alinush @heliuchuan
 /aptos-move/aptos-vm-types/ @georgemitenkov @gelash @vgao1996
 /aptos-move/framework/ @davidiw @movekevin @wrwg
 /aptos-move/framework/aptos-framework/sources/account.move @alinush
+/aptos-move/framework/aptos-framework/sources/jwks.move @zjma
+/aptos-move/framework/aptos-framework/sources/keyless_account.move @alinush
 /aptos-move/framework/aptos-stdlib/sources/cryptography/ @alinush @zjma @mstraka100
 /aptos-move/framework/**/*.spec.move @junkil-park
 /aptos-move/framework/aptos-stdlib/sources/hash.move @alinush
@@ -39,7 +42,7 @@
 /consensus/src/quorum_store/ @bchocho @sasha8 @gelash
 
 # Owners for the `/crates/aptos` directory and all its subdirectories.
-/crates/aptos @gregnazario @0xjinn @banool
+/crates/aptos @gregnazario @banool
 
 # Owners for the `/crates/aptos-crypto*` directories.
 /crates/aptos-crypto-derive/ @alinush @zjma @mstraka100 @rex1fernando
@@ -86,8 +89,8 @@
 /docker/ @aptos-labs/prod-eng
 
 # Owners for the `SDKs`.
-/ecosystem/python/sdk @davidiw @gregnazario @banool @0xmaayan @0xjinn
-/ecosystem/typescript/sdk @gregnazario @banool @0xmaayan @0xjinn
+/ecosystem/python/sdk @davidiw @gregnazario @banool @0xmaayan
+/ecosystem/typescript/sdk @gregnazario @banool @0xmaayan
 
 # Owners for execution and storage.
 /execution/ @msmouse @lightmark @grao1991
@@ -110,4 +113,5 @@
 # Owners for the `aptos-dkg` crate.
 /crates/aptos-dkg @alinush @rex1fernando
 
+/types/src/keyless/ @alinush @heliuchuan
 /types/src/transaction/authenticator.rs @alinush @mstraka100

aptos-move/aptos-vm/Cargo.toml

@@ -45,6 +45,7 @@ crossbeam-channel = { workspace = true }
 dashmap = { workspace = true }
 fail = { workspace = true }
 futures = { workspace = true }
+hex = { workspace = true }
 jsonwebtoken = { workspace = true }
 move-binary-format = { workspace = true }
 move-bytecode-utils = { workspace = true }

aptos-move/aptos-vm/src/aptos_vm.rs

@@ -1425,11 +1425,6 @@ impl AptosVM {
 
         // If there are keyless TXN authenticators, validate them all.
         if !authenticators.is_empty() {
-            // Feature-gating keyless TXNs: if they are *not* enabled, return `FEATURE_UNDER_GATING`,
-            // which will discard the TXN from being put on-chain.
-            if !self.features.is_keyless_enabled() {
-                return Err(VMStatus::error(StatusCode::FEATURE_UNDER_GATING, None));
-            }
             keyless_validation::validate_authenticators(&authenticators, &self.features, resolver)?;
         }
 

aptos-move/aptos-vm/src/keyless_validation.rs

@@ -95,20 +95,44 @@ fn get_jwk_for_authenticator(
 
     let jwk = JWK::try_from(jwk_move_struct)
         .map_err(|_| invalid_signature!("Could not unpack Any in JWK Move struct"))?;
+
+    match &jwk {
+        JWK::RSA(rsa_jwk) => {
+            if rsa_jwk.alg != jwt_header.alg {
+                return Err(invalid_signature!(format!(
+                    "JWK alg ({}) does not match JWT header's alg ({})",
+                    rsa_jwk.alg, jwt_header.alg
+                )));
+            }
+        },
+        JWK::Unsupported(jwk) => {
+            return Err(invalid_signature!(format!(
+                "JWK with KID {} and hex-encoded payload {} is not supported",
+                jwt_header.kid,
+                hex::encode(&jwk.payload)
+            )))
+        },
+    }
+
     Ok(jwk)
 }
 
+/// Ensures that **all** keyless authenticators in the transaction are valid.
 pub(crate) fn validate_authenticators(
     authenticators: &Vec<(KeylessPublicKey, KeylessSignature)>,
     features: &Features,
     resolver: &impl AptosMoveResolver,
 ) -> Result<(), VMStatus> {
     for (_, sig) in authenticators {
-        // Feature-gating for keyless-but-zkless TXNs: If keyless TXNs *are* enabled, and (1) this
-        // is a ZKless transaction but (2) ZKless TXNs are not yet enabled, discard the TXN from
-        // being put on-chain.
+        // Feature-gating for keyless TXNs (whether ZK or ZKless, whether passkey-based or not)
+        if matches!(sig.cert, EphemeralCertificate::ZeroKnowledgeSig { .. })
+            && !features.is_zk_keyless_enabled()
+        {
+            return Err(VMStatus::error(StatusCode::FEATURE_UNDER_GATING, None));
+        }
+
         if matches!(sig.cert, EphemeralCertificate::OpenIdSig { .. })
-            && !features.is_keyless_zkless_enabled()
+            && !features.is_zkless_keyless_enabled()
         {
             return Err(VMStatus::error(StatusCode::FEATURE_UNDER_GATING, None));
         }

aptos-move/e2e-move-tests/src/tests/keyless_feature_gating.rs

@@ -24,89 +24,114 @@ use move_core_types::{
     vm_status::StatusCode::FEATURE_UNDER_GATING,
 };
 
-#[test]
-fn test_keyless_disabled() {
-    let mut h = MoveHarness::new_with_features(vec![], vec![FeatureFlag::KEYLESS_ACCOUNTS]);
+fn init_feature_gating(
+    enabled_features: Vec<FeatureFlag>,
+    disabled_features: Vec<FeatureFlag>,
+) -> (MoveHarness, Account) {
+    let mut h = MoveHarness::new_with_features(enabled_features, disabled_features);
 
-    let (sig, pk) = get_sample_groth16_sig_and_pk();
-    let bob = h.new_account_at(AccountAddress::from_hex_literal("0xb0b").unwrap());
+    let recipient = h.new_account_at(AccountAddress::from_hex_literal("0xb0b").unwrap());
 
-    let transaction = get_keyless_txn(&mut h, sig, pk, bob);
+    // initialize JWKs
+    run_setup_script(&mut h);
 
-    let output = h.run_raw(transaction);
-    match output.status() {
-        TransactionStatus::Discard(status) => {
-            assert_eq!(*status, FEATURE_UNDER_GATING)
-        },
-        _ => {
-            panic!("Expected to get FEATURE_UNDER_GATING DiscardedVMStatus")
-        },
-    }
+    (h, recipient)
 }
 
-#[test]
-fn test_keyless_enabled() {
-    let mut h = MoveHarness::new_with_features(vec![FeatureFlag::KEYLESS_ACCOUNTS], vec![]);
-
-    let (sig, pk) = get_sample_groth16_sig_and_pk();
-    let bob = h.new_account_at(AccountAddress::from_hex_literal("0xb0b").unwrap());
-
-    // initialize JWK
-    run_setup_script(&mut h);
-
-    let transaction = get_keyless_txn(&mut h, sig, pk, bob);
+fn test_feature_gating(
+    h: &mut MoveHarness,
+    recipient: &Account,
+    get_sig_and_pk: fn() -> (KeylessSignature, KeylessPublicKey),
+    should_succeed: bool,
+) {
+    let (sig, pk) = get_sig_and_pk();
 
+    let transaction = get_keyless_txn(h, sig, pk, *recipient.address());
     let output = h.run_raw(transaction);
-    assert_success!(output.status().clone());
+
+    if !should_succeed {
+        match output.status() {
+            TransactionStatus::Discard(status) => {
+                assert_eq!(
+                    *status, FEATURE_UNDER_GATING,
+                    "Expected TransactionStatus::Discard to be FEATURE_UNDER_GATING, but got: {:?}",
+                    status
+                )
+            },
+            _ => {
+                panic!(
+                    "Expected to get a TransactionStatus::Discard, but got: {:?}",
+                    output.status()
+                )
+            },
+        }
+    } else {
+        assert_success!(
+            output.status().clone(),
+            "Expected TransactionStatus::Keep(ExecutionStatus::Success), but got: {:?}",
+            output.status()
+        );
+    }
 }
 
 #[test]
-fn test_keyless_enabled_but_zkless_disabled() {
-    let mut h = MoveHarness::new_with_features(vec![FeatureFlag::KEYLESS_ACCOUNTS], vec![
+fn test_feature_gating_with_zk_on() {
+    //
+    // ZK & ZKless
+    let (mut h, recipient) = init_feature_gating(
+        vec![
+            FeatureFlag::KEYLESS_ACCOUNTS,
+            FeatureFlag::KEYLESS_BUT_ZKLESS_ACCOUNTS,
+        ],
+        vec![],
+    );
+    // Groth16-based sig => success
+    test_feature_gating(&mut h, &recipient, get_sample_groth16_sig_and_pk, true);
+    // OIDC-based sig => success
+    test_feature_gating(&mut h, &recipient, get_sample_openid_sig_and_pk, true);
+
+    //
+    // ZK & !ZKless
+    let (mut h, recipient) = init_feature_gating(vec![FeatureFlag::KEYLESS_ACCOUNTS], vec![
         FeatureFlag::KEYLESS_BUT_ZKLESS_ACCOUNTS,
     ]);
-
-    let (sig, pk) = get_sample_openid_sig_and_pk();
-    let bob = h.new_account_at(AccountAddress::from_hex_literal("0xb0b").unwrap());
-
-    // initialize JWK
-    run_setup_script(&mut h);
-
-    let transaction = get_keyless_txn(&mut h, sig, pk, bob);
-
-    let output = h.run_raw(transaction);
-    match output.status() {
-        TransactionStatus::Discard(status) => {
-            assert_eq!(*status, FEATURE_UNDER_GATING)
-        },
-        _ => {
-            panic!("Expected to get FEATURE_UNDER_GATING DiscardedVMStatus")
-        },
-    }
+    // Groth16-based sig => success
+    test_feature_gating(&mut h, &recipient, get_sample_groth16_sig_and_pk, true);
+    // OIDC-based sig => discard
+    test_feature_gating(&mut h, &recipient, get_sample_openid_sig_and_pk, false);
 }
 
 #[test]
-fn test_keyless_enabled_but_zkless_enabled() {
-    let mut h = MoveHarness::new_with_features(vec![FeatureFlag::KEYLESS_ACCOUNTS], vec![]);
-
-    let (sig, pk) = get_sample_openid_sig_and_pk();
-    let bob = h.new_account_at(AccountAddress::from_hex_literal("0xb0b").unwrap());
-
-    // initialize JWK
-    run_setup_script(&mut h);
-
-    let transaction = get_keyless_txn(&mut h, sig, pk, bob);
-
-    let output = h.run_raw(transaction);
-    assert_success!(output.status().clone());
+fn test_feature_gating_with_zk_off() {
+    //
+    // !ZK & ZKless
+    let (mut h, recipient) =
+        init_feature_gating(vec![FeatureFlag::KEYLESS_BUT_ZKLESS_ACCOUNTS], vec![
+            FeatureFlag::KEYLESS_ACCOUNTS,
+        ]);
+    // Groth16-based sig => discard
+    test_feature_gating(&mut h, &recipient, get_sample_groth16_sig_and_pk, false);
+    // OIDC-based sig => success
+    test_feature_gating(&mut h, &recipient, get_sample_openid_sig_and_pk, true);
+
+    //
+    // !ZK & !ZKless
+    let (mut h, recipient) = init_feature_gating(vec![], vec![
+        FeatureFlag::KEYLESS_ACCOUNTS,
+        FeatureFlag::KEYLESS_BUT_ZKLESS_ACCOUNTS,
+    ]);
+    // Groth16-based sig => discard
+    test_feature_gating(&mut h, &recipient, get_sample_groth16_sig_and_pk, false);
+    // OIDC-based sig => discard
+    test_feature_gating(&mut h, &recipient, get_sample_openid_sig_and_pk, false);
 }
 
 /// Creates and funds a new account at `pk` and sends coins to `recipient`.
 fn get_keyless_txn(
     h: &mut MoveHarness,
     mut sig: KeylessSignature,
     pk: KeylessPublicKey,
-    recipient: Account,
+    recipient: AccountAddress,
 ) -> SignedTransaction {
     let apk = AnyPublicKey::keyless(pk.clone());
     let addr = AuthenticationKey::any_key(apk.clone()).account_address();
@@ -119,7 +144,7 @@ fn get_keyless_txn(
     println!("Actual address: {}", addr.to_hex());
     println!("Account address: {}", account.address().to_hex());
 
-    let payload = aptos_stdlib::aptos_coin_transfer(*recipient.address(), 1);
+    let payload = aptos_stdlib::aptos_coin_transfer(recipient, 1);
     //println!("Payload: {:?}", payload);
     let raw_txn = TransactionBuilder::new(account.clone())
         .payload(payload)

aptos-move/vm-genesis/src/lib.rs

@@ -560,6 +560,7 @@ fn initialize_keyless_accounts(session: &mut SessionExt, chain_id: ChainId) {
             ]),
         );
     }
+
     exec_function(
         session,
         JWKS_MODULE_NAME,

crates/aptos-crypto/src/secp256r1_ecdsa/secp256r1_ecdsa_sigs.rs

@@ -17,7 +17,8 @@ use serde::Serialize;
 use signature::Verifier;
 use std::{cmp::Ordering, fmt};
 
-/// A Secp256r1 ECDSA signature
+/// A secp256r1 ECDSA signature.
+/// NOTE: The max size on this struct is enforced in its `TryFrom<u8>` trait implementation.
 #[derive(DeserializeKey, Clone, SerializeKey)]
 #[key_name("Secp256r1EcdsaSignature")]
 pub struct Signature(pub(crate) p256::ecdsa::Signature);

testsuite/generate-format/tests/staged/api.yaml

@@ -237,6 +237,11 @@ EphemeralPublicKey:
         STRUCT:
           - public_key:
               TYPENAME: Ed25519PublicKey
+    1:
+      Secp256r1Ecdsa:
+        STRUCT:
+          - public_key:
+              TYPENAME: Secp256r1EcdsaPublicKey
 EphemeralSignature:
   ENUM:
     0:

testsuite/generate-format/tests/staged/aptos.yaml

@@ -214,6 +214,11 @@ EphemeralPublicKey:
         STRUCT:
           - public_key:
               TYPENAME: Ed25519PublicKey
+    1:
+      Secp256r1Ecdsa:
+        STRUCT:
+          - public_key:
+              TYPENAME: Secp256r1EcdsaPublicKey
 EphemeralSignature:
   ENUM:
     0:

testsuite/generate-format/tests/staged/consensus.yaml

@@ -459,6 +459,11 @@ EphemeralPublicKey:
         STRUCT:
           - public_key:
               TYPENAME: Ed25519PublicKey
+    1:
+      Secp256r1Ecdsa:
+        STRUCT:
+          - public_key:
+              TYPENAME: Secp256r1EcdsaPublicKey
 EphemeralSignature:
   ENUM:
     0:

types/src/keyless/bn254_circom.rs

@@ -1,5 +1,6 @@
 // Copyright © Aptos Foundation
 
+use super::circuit_constants::MAX_EXTRA_FIELD_BYTES;
 use crate::{
     jwks::rsa::RSA_JWK,
     keyless::{
@@ -14,6 +15,7 @@ use ark_bn254::{Fq, Fq2, Fr, G1Affine, G1Projective, G2Affine, G2Projective};
 use ark_ff::PrimeField;
 use ark_serialize::{CanonicalDeserialize, CanonicalSerialize};
 use num_traits::{One, Zero};
+use once_cell::sync::Lazy;
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
 use serde_big_array::BigArray;
 
@@ -22,6 +24,10 @@ use serde_big_array::BigArray;
 pub const G1_PROJECTIVE_COMPRESSED_NUM_BYTES: usize = 32;
 pub const G2_PROJECTIVE_COMPRESSED_NUM_BYTES: usize = 64;
 
+// When the extra_field is none, use this hash value which is equal to the hash of a single space string.
+static EMPTY_EXTRA_FIELD_HASH: Lazy<Fr> =
+    Lazy::new(|| poseidon_bn254::pad_and_hash_string(" ", MAX_EXTRA_FIELD_BYTES as usize).unwrap());
+
 /// This will do the proper subgroup membership checks.
 pub fn g1_projective_str_to_affine(x: &str, y: &str) -> anyhow::Result<G1Affine> {
     let g1_affine = G1Bytes::new_unchecked(x, y)?.deserialize_into_affine()?;
@@ -240,7 +246,7 @@ pub fn get_public_inputs_hash(
 ) -> anyhow::Result<Fr> {
     if let EphemeralCertificate::ZeroKnowledgeSig(proof) = &sig.cert {
         let (has_extra_field, extra_field_hash) = match &proof.extra_field {
-            None => (Fr::zero(), Fr::zero()),
+            None => (Fr::zero(), *Lazy::force(&EMPTY_EXTRA_FIELD_HASH)),
             Some(extra_field) => (
                 Fr::one(),
                 poseidon_bn254::pad_and_hash_string(

types/src/keyless/mod.rs

@@ -132,7 +132,7 @@ pub struct JWTHeader {
 }
 
 impl KeylessSignature {
-    /// A reasonable upper bound for the number of bytes we expect in a keyless public key. This is
+    /// A reasonable upper bound for the number of bytes we expect in a keyless signature. This is
     /// enforced by our full nodes when they receive TXNs.
     pub const MAX_LEN: usize = 4000;