Skip to content

Commit

Permalink
[PM-7840] Implement the stubbed out Passkey uniffi API (#779)
Browse files Browse the repository at this point in the history
## Type of change
```
- [ ] Bug fix
- [x] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other
```

## Objective
This PR adds our fork of `passkey-rs` to bitwarden to implement the
previously stubbed out API. With it come some changes:
- Swapped most parts of the stubbed implementations to use `passkey-rs`
instead
- Some API callbacks were changed to take decrypted items, the clients
would need these decrypted to show the user anyway, and we can skip a
few rounds of decrypting/encrypting this way. Note that the FIDO2
credentials private keys are never exposed decrypted to the clients.
- Added a separate `Fido2CredentialNewView`, the only difference being
that it doesn't contain the private key. This is used to send to the
clients in the cipher select callback. Everywhere else we send back the
encrypted field but at this point we don't have a key to encrypt the
field yet. We could send a dummy value but that seems error prone.
- Changed `CheckUserOptions` to be a struct instead of an enum. This was
a mistake from the previous PR.
- Moved a lot of types that were previously distributed among a few
files into a specific `types.rs` file. Also implemented conversion
between these types and `passkey` types using `From`/`TryFrom` when
possible.

There are still some open questions:
- Some of the callbacks from `passkey-rs` force us to return
`StatusCode` or `Ctap2Error`, how do we want to handle these? At the
moment I'm just logging the error and returing a constant value. Do we
need specific error values for certain errors to be spec compliant?
- The conversion from `Passkey` to `Fido2CredentialView` has a few
hardcoded values, is that expected?
- I left a few `// TODO(Fido2):` comments around to mention some other
small questions I have

---------

Co-authored-by: Andreas Coroiu <[email protected]>
  • Loading branch information
dani-garcia and coroiu authored Jun 4, 2024
1 parent f1e6a33 commit ea93ac5
Show file tree
Hide file tree
Showing 18 changed files with 1,406 additions and 614 deletions.
15 changes: 9 additions & 6 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/bitwarden-uniffi/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ chrono = { version = ">=0.4.26, <0.5", features = [
log = "0.4.20"
env_logger = "0.11.1"
schemars = { version = ">=0.8, <0.9", optional = true }
thiserror = ">=1.0.40, <2.0"
uniffi = "=0.27.2"
uuid = ">=1.3.3, <2"

Expand Down
17 changes: 0 additions & 17 deletions crates/bitwarden-uniffi/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,6 @@ impl From<bitwarden::error::Error> for BitwardenError {
}
}

impl From<BitwardenError> for bitwarden::error::Error {
fn from(val: BitwardenError) -> Self {
match val {
BitwardenError::E(e) => e,
}
}
}

// Need to implement this From<> impl in order to handle unexpected callback errors. See the
// following page in the Uniffi user guide:
// <https://mozilla.github.io/uniffi-rs/foreign_traits.html#error-handling>
impl From<uniffi::UnexpectedUniFFICallbackError> for BitwardenError {
fn from(e: uniffi::UnexpectedUniFFICallbackError) -> Self {
Self::E(bitwarden::error::Error::UniffiCallback(e))
}
}

impl Display for BitwardenError {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
Expand Down
149 changes: 109 additions & 40 deletions crates/bitwarden-uniffi/src/platform/fido2.rs
Original file line number Diff line number Diff line change
@@ -1,29 +1,27 @@
use std::sync::Arc;

use bitwarden::{
error::Result as BitResult,
platform::fido2::{
CheckUserOptions, ClientData, GetAssertionRequest, GetAssertionResult,
MakeCredentialRequest, MakeCredentialResult,
CheckUserOptions, ClientData, Fido2CallbackError as BitFido2CallbackError,
GetAssertionRequest, GetAssertionResult, MakeCredentialRequest, MakeCredentialResult,
PublicKeyCredentialAuthenticatorAssertionResponse,
PublicKeyCredentialAuthenticatorAttestationResponse,
PublicKeyCredentialAuthenticatorAttestationResponse, PublicKeyCredentialRpEntity,
PublicKeyCredentialUserEntity,
},
vault::{Cipher, CipherView, Fido2Credential, Fido2CredentialView},
vault::{Cipher, CipherView, Fido2CredentialNewView, Fido2CredentialView},
};

use crate::{error::Result, Client};

/// At the moment this is just a stub implementation that doesn't do anything. It's here to make
/// it possible to check the usability API on the native clients.
#[derive(uniffi::Object)]
pub struct ClientFido2(pub(crate) Arc<Client>);

#[uniffi::export]
impl ClientFido2 {
pub fn authenticator(
self: Arc<Self>,
user_interface: Arc<dyn UserInterface>,
credential_store: Arc<dyn CredentialStore>,
user_interface: Arc<dyn Fido2UserInterface>,
credential_store: Arc<dyn Fido2CredentialStore>,
) -> Arc<ClientFido2Authenticator> {
Arc::new(ClientFido2Authenticator(
self.0.clone(),
Expand All @@ -34,8 +32,8 @@ impl ClientFido2 {

pub fn client(
self: Arc<Self>,
user_interface: Arc<dyn UserInterface>,
credential_store: Arc<dyn CredentialStore>,
user_interface: Arc<dyn Fido2UserInterface>,
credential_store: Arc<dyn Fido2CredentialStore>,
) -> Arc<ClientFido2Client> {
Arc::new(ClientFido2Client(ClientFido2Authenticator(
self.0.clone(),
Expand All @@ -48,8 +46,8 @@ impl ClientFido2 {
#[derive(uniffi::Object)]
pub struct ClientFido2Authenticator(
pub(crate) Arc<Client>,
pub(crate) Arc<dyn UserInterface>,
pub(crate) Arc<dyn CredentialStore>,
pub(crate) Arc<dyn Fido2UserInterface>,
pub(crate) Arc<dyn Fido2CredentialStore>,
);

#[uniffi::export]
Expand Down Expand Up @@ -152,35 +150,67 @@ pub struct CheckUserResult {
user_verified: bool,
}

#[derive(Debug, thiserror::Error, uniffi::Error)]
pub enum Fido2CallbackError {
#[error("The operation requires user interaction")]
UserInterfaceRequired,

#[error("The operation was cancelled by the user")]
OperationCancelled,

#[error("Unknown error: {reason}")]
Unknown { reason: String },
}

// Need to implement this From<> impl in order to handle unexpected callback errors. See the
// following page in the Uniffi user guide:
// <https://mozilla.github.io/uniffi-rs/foreign_traits.html#error-handling>
impl From<uniffi::UnexpectedUniFFICallbackError> for Fido2CallbackError {
fn from(e: uniffi::UnexpectedUniFFICallbackError) -> Self {
Self::Unknown { reason: e.reason }
}
}

impl From<Fido2CallbackError> for BitFido2CallbackError {
fn from(val: Fido2CallbackError) -> Self {
match val {
Fido2CallbackError::UserInterfaceRequired => Self::UserInterfaceRequired,
Fido2CallbackError::OperationCancelled => Self::OperationCancelled,
Fido2CallbackError::Unknown { reason } => Self::Unknown(reason),
}
}
}

#[uniffi::export(with_foreign)]
#[async_trait::async_trait]
pub trait UserInterface: Send + Sync {
pub trait Fido2UserInterface: Send + Sync {
async fn check_user(
&self,
options: CheckUserOptions,
credential: Option<CipherView>,
) -> Result<CheckUserResult>;
hint: UIHint,
) -> Result<CheckUserResult, Fido2CallbackError>;
async fn pick_credential_for_authentication(
&self,
available_credentials: Vec<Cipher>,
) -> Result<CipherViewWrapper>;
async fn pick_credential_for_creation(
available_credentials: Vec<CipherView>,
) -> Result<CipherViewWrapper, Fido2CallbackError>;
async fn check_user_and_pick_credential_for_creation(
&self,
available_credentials: Vec<Cipher>,
new_credential: Fido2Credential,
) -> Result<CipherViewWrapper>;
options: CheckUserOptions,
new_credential: Fido2CredentialNewView,
) -> Result<CipherViewWrapper, Fido2CallbackError>;
async fn is_verification_enabled(&self) -> bool;
}

#[uniffi::export(with_foreign)]
#[async_trait::async_trait]
pub trait CredentialStore: Send + Sync {
pub trait Fido2CredentialStore: Send + Sync {
async fn find_credentials(
&self,
ids: Option<Vec<Vec<u8>>>,
rip_id: String,
) -> Result<Vec<Cipher>>;
) -> Result<Vec<CipherView>, Fido2CallbackError>;

async fn save_credential(&self, cred: Cipher) -> Result<()>;
async fn save_credential(&self, cred: Cipher) -> Result<(), Fido2CallbackError>;
}

// Because uniffi doesn't support external traits, we have to make a copy of the trait here.
Expand All @@ -190,19 +220,21 @@ pub trait CredentialStore: Send + Sync {
struct UniffiTraitBridge<T>(T);

#[async_trait::async_trait]
impl bitwarden::platform::fido2::CredentialStore for UniffiTraitBridge<&dyn CredentialStore> {
impl bitwarden::platform::fido2::Fido2CredentialStore
for UniffiTraitBridge<&dyn Fido2CredentialStore>
{
async fn find_credentials(
&self,
ids: Option<Vec<Vec<u8>>>,
rip_id: String,
) -> BitResult<Vec<Cipher>> {
) -> Result<Vec<CipherView>, BitFido2CallbackError> {
self.0
.find_credentials(ids, rip_id)
.await
.map_err(Into::into)
}

async fn save_credential(&self, cred: Cipher) -> BitResult<()> {
async fn save_credential(&self, cred: Cipher) -> Result<(), BitFido2CallbackError> {
self.0.save_credential(cred).await.map_err(Into::into)
}
}
Expand All @@ -216,15 +248,49 @@ pub struct CipherViewWrapper {
cipher: CipherView,
}

#[derive(uniffi::Enum)]
pub enum UIHint {
InformExcludedCredentialFound(CipherView),
InformNoCredentialsFound,
RequestNewCredential(PublicKeyCredentialUserEntity, PublicKeyCredentialRpEntity),
RequestExistingCredential(CipherView),
}

impl From<bitwarden::platform::fido2::UIHint<'_, CipherView>> for UIHint {
fn from(hint: bitwarden::platform::fido2::UIHint<'_, CipherView>) -> Self {
use bitwarden::platform::fido2::UIHint as BWUIHint;
match hint {
BWUIHint::InformExcludedCredentialFound(cipher) => {
UIHint::InformExcludedCredentialFound(cipher.clone())
}
BWUIHint::InformNoCredentialsFound => UIHint::InformNoCredentialsFound,
BWUIHint::RequestNewCredential(user, rp) => UIHint::RequestNewCredential(
PublicKeyCredentialUserEntity {
id: user.id.clone().into(),
name: user.name.clone().unwrap_or_default(),
display_name: user.display_name.clone().unwrap_or_default(),
},
PublicKeyCredentialRpEntity {
id: rp.id.clone(),
name: rp.name.clone(),
},
),
BWUIHint::RequestExistingCredential(cipher) => {
UIHint::RequestExistingCredential(cipher.clone())
}
}
}
}

#[async_trait::async_trait]
impl bitwarden::platform::fido2::UserInterface for UniffiTraitBridge<&dyn UserInterface> {
async fn check_user(
impl bitwarden::platform::fido2::Fido2UserInterface for UniffiTraitBridge<&dyn Fido2UserInterface> {
async fn check_user<'a>(
&self,
options: CheckUserOptions,
credential: Option<CipherView>,
) -> BitResult<bitwarden::platform::fido2::CheckUserResult> {
hint: bitwarden::platform::fido2::UIHint<'a, CipherView>,
) -> Result<bitwarden::platform::fido2::CheckUserResult, BitFido2CallbackError> {
self.0
.check_user(options, credential)
.check_user(options.clone(), hint.into())
.await
.map(|r| bitwarden::platform::fido2::CheckUserResult {
user_present: r.user_present,
Expand All @@ -234,23 +300,26 @@ impl bitwarden::platform::fido2::UserInterface for UniffiTraitBridge<&dyn UserIn
}
async fn pick_credential_for_authentication(
&self,
available_credentials: Vec<Cipher>,
) -> BitResult<CipherView> {
available_credentials: Vec<CipherView>,
) -> Result<CipherView, BitFido2CallbackError> {
self.0
.pick_credential_for_authentication(available_credentials)
.await
.map(|v| v.cipher)
.map_err(Into::into)
}
async fn pick_credential_for_creation(
async fn check_user_and_pick_credential_for_creation(
&self,
available_credentials: Vec<Cipher>,
new_credential: Fido2Credential,
) -> BitResult<CipherView> {
options: CheckUserOptions,
new_credential: Fido2CredentialNewView,
) -> Result<CipherView, BitFido2CallbackError> {
self.0
.pick_credential_for_creation(available_credentials, new_credential)
.check_user_and_pick_credential_for_creation(options, new_credential)
.await
.map(|v| v.cipher)
.map_err(Into::into)
}
async fn is_verification_enabled(&self) -> bool {
self.0.is_verification_enabled().await
}
}
7 changes: 6 additions & 1 deletion crates/bitwarden/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ uniffi = [
"bitwarden-crypto/uniffi",
"bitwarden-generators/uniffi",
"dep:uniffi",
"dep:passkey",
"dep:coset",
"dep:p256",
] # Uniffi bindings
secrets = [] # Secrets manager API
wasm-bindgen = ["chrono/wasmbind"]
Expand All @@ -44,11 +47,13 @@ chrono = { version = ">=0.4.26, <0.5", features = [
"serde",
"std",
], default-features = false }
coset = { version = "0.3.7", optional = true }
# We don't use this directly (it's used by rand), but we need it here to enable WASM support
getrandom = { version = ">=0.2.9, <0.3", features = ["js"] }
hmac = ">=0.12.1, <0.13"
log = ">=0.4.18, <0.5"
passkey = { git = "https://github.com/bitwarden/passkey-rs", rev = "12da886102707f87ad97e499c857c0857ece0b85" }
p256 = { version = ">=0.13.2, <0.14", optional = true }
passkey = { git = "https://github.com/bitwarden/passkey-rs", rev = "c48c2ddfd6b884b2d754432576c66cb2b1985a3a", optional = true }
rand = ">=0.8.5, <0.9"
reqwest = { version = ">=0.12, <0.13", features = [
"http2",
Expand Down
Loading

0 comments on commit ea93ac5

Please sign in to comment.