Skip to content

Commit

Permalink
fix(fc-pallet-pass): ensure credentials are verified by the device, n…
Browse files Browse the repository at this point in the history
…ot just by themselves
  • Loading branch information
pandres95 committed Oct 13, 2024
1 parent a3a8644 commit f588a50
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 27 deletions.
41 changes: 38 additions & 3 deletions pallets/pass/src/mock/authenticators.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,11 @@ pub mod authenticator_a {
fn device_id(&self) -> &DeviceId {
&self.device_id
}

// Note: This authenticator should pass intentionally, to pass on simpler tests
fn verify_credential(&self, _: &Self::Credential) -> Option<()> {
Some(())
}
}

impl DeviceChallengeResponse<()> for DeviceAttestation {
Expand Down Expand Up @@ -134,9 +139,31 @@ pub mod authenticator_b {
TypeInfo, DebugNoBound, EqNoBound, PartialEq, Clone, Encode, Decode, MaxEncodedLen,
)]
pub struct Credential {
pub(crate) device_id: DeviceId,
pub(crate) user_id: HashedUserId,
pub(crate) challenge: Challenge,
pub(crate) signature: Option<[u8; 32]>,
}

impl Credential {
pub fn new(user_id: HashedUserId, challenge: Challenge) -> Self {
Self {
user_id,
challenge,
signature: None,
}
}

pub fn sign(self, signer: &DeviceId) -> Self {
Self {
signature: Some(Self::signature(signer, &self)),
..self
}
}

// A dummy "signature", to test the signing capabilities
pub fn signature(signer: &DeviceId, credential: &Self) -> [u8; 32] {
blake2_256(&(signer, credential.user_id, credential.challenge).encode())
}
}

impl Authenticator for AuthenticatorB {
Expand Down Expand Up @@ -169,6 +196,14 @@ pub mod authenticator_b {
fn device_id(&self) -> &DeviceId {
&self.device_id
}

fn verify_credential(&self, credential: &Self::Credential) -> Option<()> {
credential.signature.and_then(|signature| {
Credential::signature(self.device_id(), &credential)
.eq(&signature)
.then_some(())
})
}
}

impl DeviceChallengeResponse<DeviceId> for DeviceAttestation {
Expand All @@ -191,11 +226,11 @@ pub mod authenticator_b {

impl UserChallengeResponse<DeviceId> for Credential {
fn is_valid(&self) -> bool {
true
self.signature.is_some()
}

fn used_challenge(&self) -> (DeviceId, Challenge) {
(self.device_id, self.challenge)
(self.user_id, self.challenge)
}

fn authority(&self) -> AuthorityId {
Expand Down
45 changes: 25 additions & 20 deletions pallets/pass/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,11 +212,10 @@ mod authenticate {
Pass::authenticate(
RuntimeOrigin::signed(OTHER),
THE_DEVICE,
PassCredential::AuthenticatorB(authenticator_b::Credential {
user_id: AccountNameA::get(),
device_id: THE_DEVICE,
challenge: AuthenticatorB::generate(&OTHER_DEVICE),
}),
PassCredential::AuthenticatorB(authenticator_b::Credential::new(
AccountNameA::get(),
AuthenticatorB::generate(&OTHER_DEVICE)
)),
Some(DURATION),
),
Error::<Test>::CredentialInvalid
Expand All @@ -240,11 +239,13 @@ mod authenticate {
assert_ok!(Pass::authenticate(
RuntimeOrigin::signed(SIGNER),
THE_DEVICE,
PassCredential::AuthenticatorB(authenticator_b::Credential {
user_id: AccountNameA::get(),
device_id: THE_DEVICE,
challenge: AuthenticatorB::generate(&THE_DEVICE),
}),
PassCredential::AuthenticatorB(
authenticator_b::Credential::new(
AccountNameA::get(),
AuthenticatorB::generate(&AccountNameA::get()),
)
.sign(&THE_DEVICE)
),
None,
));
assert_ok!(Pass::add_device(
Expand All @@ -259,11 +260,13 @@ mod authenticate {
Pass::authenticate(
RuntimeOrigin::signed(OTHER),
THE_DEVICE,
PassCredential::AuthenticatorB(authenticator_b::Credential {
user_id: AccountNameA::get(),
device_id: OTHER_DEVICE,
challenge: AuthenticatorB::generate(&OTHER_DEVICE),
}),
PassCredential::AuthenticatorB(
authenticator_b::Credential::new(
AccountNameA::get(),
AuthenticatorB::generate(&AccountNameA::get())
)
.sign(&OTHER_DEVICE)
),
Some(DURATION),
),
Error::<Test>::CredentialInvalid
Expand Down Expand Up @@ -297,11 +300,13 @@ mod authenticate {
Pass::authenticate(
RuntimeOrigin::signed(OTHER),
THE_DEVICE,
PassCredential::AuthenticatorB(authenticator_b::Credential {
user_id: AccountNameA::get(),
device_id: OTHER_DEVICE,
challenge: AuthenticatorB::generate(&OTHER_DEVICE),
}),
PassCredential::AuthenticatorB(
authenticator_b::Credential::new(
AccountNameA::get(),
AuthenticatorB::generate(&AccountNameA::get()),
)
.sign(&OTHER_DEVICE)
),
Some(DURATION),
),
Error::<Test>::CredentialInvalid
Expand Down
21 changes: 19 additions & 2 deletions traits/authn/proc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,14 +161,24 @@ pub fn composite_authenticator(input: TokenStream) -> TokenStream {
}
});

let match_credentials = authenticators.clone().into_iter().map(|(id, _)| {
let match_verify_user = authenticators.clone().into_iter().map(|(id, _)| {
quote! {
(
#device::#id(device),
#credential::#id(credential),
) => device.verify_user(credential)
}
});

let match_verify_credential = authenticators.clone().into_iter().map(|(id, _)| {
quote! {
(
#device::#id(device),
#credential::#id(credential),
) => device.verify_credential(credential)
}
});

let match_user_id = authenticators.clone().into_iter().map(|(id, _)| {
quote! {
#credential::#id(credential) => credential.user_id()
Expand Down Expand Up @@ -252,7 +262,14 @@ pub fn composite_authenticator(input: TokenStream) -> TokenStream {

fn verify_user(&self, credential: &Self::Credential) -> Option<()> {
match (self, credential) {
#(#match_credentials),*,
#(#match_verify_user),*,
_ => None,
}
}

fn verify_credential(&self, credential: &Self::Credential) -> Option<()> {
match (self, credential) {
#(#match_verify_credential),*,
_ => None,
}
}
Expand Down
5 changes: 4 additions & 1 deletion traits/authn/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,12 @@ pub trait UserAuthenticator: FullCodec + MaxEncodedLen + TypeInfo {
.then_some(())?;
let (cx, challenge) = credential.used_challenge();
Self::Challenger::check_challenge(&cx, &challenge)?;
credential.is_valid().then_some(())
credential.is_valid().then_some(())?;
self.verify_credential(credential)
}

fn verify_credential(&self, credential: &Self::Credential) -> Option<()>;

fn device_id(&self) -> &DeviceId;
}

Expand Down
10 changes: 9 additions & 1 deletion traits/authn/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ where
}
}

trait VerifyCredential<Cred> {
fn verify(&self, credential: &Cred) -> Option<()>;
}

/// Convenient auto-implemtator of the UserAuthenticator trait
#[derive(Encode, Decode, TypeInfo)]
#[scale_info(skip_type_params(A, Ch, Cred))]
Expand All @@ -51,7 +55,7 @@ impl<T, A, Ch, Cred> Dev<T, A, Ch, Cred> {

impl<T, A, Ch, Cred> UserAuthenticator for Dev<T, A, Ch, Cred>
where
T: AsRef<DeviceId> + FullCodec + MaxEncodedLen + TypeInfo + 'static,
T: VerifyCredential<Cred> + AsRef<DeviceId> + FullCodec + MaxEncodedLen + TypeInfo + 'static,
A: Get<AuthorityId> + 'static,
Ch: Challenger + 'static,
Cred: UserChallengeResponse<Ch::Context> + 'static,
Expand All @@ -63,6 +67,10 @@ where
fn device_id(&self) -> &DeviceId {
self.0.as_ref()
}

fn verify_credential(&self, credential: &Self::Credential) -> Option<()> {
self.0.verify(credential)
}
}

impl<T: MaxEncodedLen, A, Ch, Cr> MaxEncodedLen for Dev<T, A, Ch, Cr> {
Expand Down

0 comments on commit f588a50

Please sign in to comment.