refactor(pass-webauthn): set authority_id explicitly on meta for …

…`Attestation` and `Assertion`. This ensures we can specify which authority to use, with the tradeoff of not having it cryptographically bounded to the `PublicKeyCredential` overarching structure.
virto-network · Dec 6, 2024 · 1990fea · 1990fea
1 parent 8246f50
commit 1990fea
Show file tree

Hide file tree

Showing 6 changed files with 42 additions and 30 deletions.
diff --git a/pass-webauthn/src/lib.rs b/pass-webauthn/src/lib.rs
@@ -6,7 +6,7 @@ use alloc::vec::Vec;
 use codec::{Decode, Encode};
 use traits_authn::{
     util::{Auth, Dev},
-    Challenger, DeviceId, HashedUserId,
+    AuthorityId, Challenger, DeviceId, HashedUserId,
 };
 
 #[cfg(any(feature = "runtime", test))]
@@ -38,6 +38,7 @@ pub struct Credential {
 
 #[derive(Encode, Decode, TypeInfo, Debug, PartialEq, Eq, Clone, Copy)]
 pub struct AttestationMeta<Cx> {
+    pub(crate) authority_id: AuthorityId,
     pub(crate) device_id: DeviceId,
     pub(crate) context: Cx,
 }
@@ -52,6 +53,7 @@ pub struct Attestation<Cx> {
 
 #[derive(Encode, Decode, TypeInfo, Debug, PartialEq, Eq, Clone, Copy)]
 pub struct AssertionMeta<Cx> {
+    pub(crate) authority_id: AuthorityId,
     pub(crate) user_id: HashedUserId,
     pub(crate) context: Cx,
 }

diff --git a/pass-webauthn/src/runtime_helpers.rs b/pass-webauthn/src/runtime_helpers.rs
@@ -2,27 +2,16 @@ use codec::Decode;
 use frame_support::sp_runtime::traits::TrailingZeroInput;
 use scale_info::prelude::{string::String, vec::Vec};
 
-use traits_authn::{AuthorityId, Challenge};
+use traits_authn::Challenge;
 
 use base64::prelude::BASE64_URL_SAFE_NO_PAD;
-use url::Url;
 
 pub fn find_challenge_from_client_data(client_data: Vec<u8>) -> Option<Challenge> {
     get_from_json_then_map(client_data, "challenge", |challenge| {
         base64::decode_engine(challenge.as_bytes(), &BASE64_URL_SAFE_NO_PAD).ok()
     })
 }
 
-pub fn find_authority_id_from_client_data(client_data: Vec<u8>) -> Option<AuthorityId> {
-    get_from_json_then_map(client_data, "origin", |origin| {
-        Url::parse(&origin)
-            .ok()?
-            .domain()?
-            .split_once(".")
-            .map(|(authority_id, _)| authority_id.as_bytes().to_vec())
-    })
-}
-
 pub fn get_from_json_then_map<T>(
     json: Vec<u8>,
     key: &str,

diff --git a/pass-webauthn/src/runtime_impls/assertion.rs b/pass-webauthn/src/runtime_impls/assertion.rs
@@ -23,7 +23,7 @@ where
     }
 
     fn authority(&self) -> AuthorityId {
-        find_authority_id_from_client_data(self.client_data.clone()).unwrap_or_default()
+        self.meta.authority_id
     }
 
     fn user_id(&self) -> HashedUserId {

diff --git a/pass-webauthn/src/runtime_impls/attestation.rs b/pass-webauthn/src/runtime_impls/attestation.rs
@@ -28,7 +28,7 @@ where
     /// WebAuthn RpID should be a subdomain of the origin that is calling the create credentials request.
     /// Therefore, `authority` should be a URL-safe name, so it can be allocated in a valid URL domain.
     fn authority(&self) -> AuthorityId {
-        find_authority_id_from_client_data(self.client_data.clone()).unwrap_or_default()
+        self.meta.authority_id
     }
 
     fn device_id(&self) -> &DeviceId {

diff --git a/pass-webauthn/src/tests.rs b/pass-webauthn/src/tests.rs
@@ -7,7 +7,6 @@ use frame_support::{
     PalletId,
 };
 use frame_system::{pallet_prelude::BlockNumberFor, Config, EnsureRootWithSuccess};
-use sp_io::hashing::blake2_256;
 use traits_authn::{util::AuthorityFromPalletId, Challenger, HashedUserId};
 
 use crate::Authenticator;
@@ -49,7 +48,8 @@ impl frame_system::Config for Test {
     type AccountData = pallet_balances::AccountData<AccountId>;
 }
 
-#[derive_impl(pallet_balances::config_preludes::TestDefaultConfig as pallet_balances::DefaultConfig)]
+#[derive_impl(pallet_balances::config_preludes::TestDefaultConfig as pallet_balances::DefaultConfig
+)]
 impl pallet_balances::Config for Test {
     type AccountStore = System;
 }
@@ -95,14 +95,23 @@ impl pallet_pass::BenchmarkHelper<Test> for Helper {
 
     fn device_attestation(_: traits_authn::DeviceId) -> pallet_pass::DeviceAttestationOf<Test, ()> {
         WebAuthnClient::new("https://pass_web.pass.int", 1)
-            .attestation(blake2_256(b"USER_ID"), System::block_number())
+            .attestation(
+                blake2_256(b"USER_ID"),
+                System::block_number(),
+                AuthorityId::get(),
+            )
             .1
     }
 
     fn credential(user_id: HashedUserId) -> pallet_pass::CredentialOf<Test, ()> {
         let mut client = WebAuthnClient::new("https://helper.pass.int", 2);
-        let (credential_id, _) = client.attestation(user_id, System::block_number());
-        client.assertion(credential_id.as_slice(), System::block_number())
+        let (credential_id, _) =
+            client.attestation(user_id, System::block_number(), AuthorityId::get());
+        client.assertion(
+            credential_id.as_slice(),
+            System::block_number(),
+            AuthorityId::get(),
+        )
     }
 }
 
@@ -123,21 +132,24 @@ fn new_test_ext(times: usize) -> TestExt {
 
 const USER: HashedUserId = s("the_user");
 
+use traits_authn::composite_prelude::Get;
+
 mod attestation {
     use super::*;
 
     #[test]
     fn registration_fails_if_attestation_is_invalid() {
         new_test_ext(1).execute_with(|client| {
-            let (_, mut attestation) = client.attestation(USER, System::block_number());
+            let (_, mut attestation) =
+                client.attestation(USER, System::block_number(), AuthorityId::get());
 
             // Alters "challenge", so this will fail
             attestation.client_data = String::from_utf8(attestation.client_data)
-                .and_then(|client_data| {
-                    Ok(client_data
+                .map(|client_data| {
+                    client_data
                         .replace("challenge", "chellang")
                         .as_bytes()
-                        .to_vec())
+                        .to_vec()
                 })
                 .expect("`client_data` is a buffer representation of a utf-8 encoded json");
 
@@ -154,7 +166,9 @@ mod attestation {
             assert_ok!(Pass::register(
                 RuntimeOrigin::root(),
                 USER,
-                client.attestation(USER, System::block_number()).1
+                client
+                    .attestation(USER, System::block_number(), AuthorityId::get())
+                    .1
             ));
         })
     }
@@ -168,15 +182,17 @@ mod assertion {
     #[test]
     fn authentication_fails_if_credentials_are_invalid() {
         new_test_ext(2).execute_with(|client| {
-            let (credential_id, attestation) = client.attestation(USER, System::block_number());
+            let (credential_id, attestation) =
+                client.attestation(USER, System::block_number(), AuthorityId::get());
 
             assert_ok!(Pass::register(
                 RuntimeOrigin::root(),
                 USER,
                 attestation.clone()
             ));
 
-            let mut assertion = client.assertion(credential_id, System::block_number());
+            let mut assertion =
+                client.assertion(credential_id, System::block_number(), AuthorityId::get());
             assertion.signature = [assertion.signature, b"Whoops".to_vec()].concat();
 
             assert_noop!(
@@ -194,7 +210,8 @@ mod assertion {
     #[test]
     fn authentication_works_if_credentials_are_valid() {
         new_test_ext(2).execute_with(|client| {
-            let (credential_id, attestation) = client.attestation(USER, System::block_number());
+            let (credential_id, attestation) =
+                client.attestation(USER, System::block_number(), AuthorityId::get());
 
             assert_ok!(Pass::register(
                 RuntimeOrigin::root(),
@@ -205,7 +222,7 @@ mod assertion {
             assert_ok!(Pass::authenticate(
                 RuntimeOrigin::signed(1),
                 *(attestation.device_id()),
-                client.assertion(credential_id, System::block_number()),
+                client.assertion(credential_id, System::block_number(), AuthorityId::get()),
                 None
             ));
         })

diff --git a/pass-webauthn/src/tests/authenticator_client.rs b/pass-webauthn/src/tests/authenticator_client.rs
@@ -8,7 +8,7 @@ use passkey_client::{Client, DefaultClientData};
 use passkey_types::{ctap2::Aaguid, webauthn::*, Bytes, Passkey};
 
 use sp_io::hashing::blake2_256;
-use traits_authn::{Challenger, HashedUserId};
+use traits_authn::{AuthorityId, Challenger, HashedUserId};
 use url_evil::Url;
 
 use crate::{AssertionMeta, DEREncodedPublicKey};
@@ -138,6 +138,7 @@ impl WebAuthnClient {
         &mut self,
         user_id: HashedUserId,
         context: BlockNumberFor<Test>,
+        authority_id: AuthorityId,
     ) -> (Vec<u8>, crate::Attestation<BlockNumberFor<Test>>) {
         let challenge = BlockChallenger::generate(&context);
 
@@ -149,6 +150,7 @@ impl WebAuthnClient {
             credential_id.clone(),
             crate::Attestation {
                 meta: crate::AttestationMeta {
+                    authority_id,
                     device_id: blake2_256(&credential_id),
                     context,
                 },
@@ -163,6 +165,7 @@ impl WebAuthnClient {
         &mut self,
         credential_id: impl Into<Bytes>,
         context: BlockNumberFor<Test>,
+        authority_id: AuthorityId,
     ) -> crate::Assertion<BlockNumberFor<Test>> {
         let challenge = BlockChallenger::generate(&context);
 
@@ -172,6 +175,7 @@ impl WebAuthnClient {
 
         crate::Assertion {
             meta: AssertionMeta {
+                authority_id,
                 user_id: Decode::decode(&mut TrailingZeroInput::new(&user_handle)).expect("`user_handle` corresponds to the `user_id` inserted when creating credential; qed"),
                 context,
             },