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

MFA: Sec Structure of Factor <Source | ID | Instance>s #150

Merged
merged 278 commits into from
Jun 28, 2024

Conversation

CyonAlexRDX
Copy link
Contributor

@CyonAlexRDX CyonAlexRDX commented May 28, 2024

Note

Target branch is not main it is "SargonOS" branch...

Tip

Video demos in bottom, maybe best if you read the description first, and then look at the videos!

Multi-Factor | Security Structure Configurations

image

CLOC

Note

For not the LOC delta (at least not too much), I've updated the
huge_profile.json file which make up a big portion of the LOC delta.

The actual change is ~6000 lines of Rust added and ~6000 lines of Swift (mostly example app).

cloc --git-diff-rel ddbc911e81d399dacd92230bedf6a94f32f9d133 fac7e6bc7d4cb694b351e9ae235142d49bedf550
     168 text files.
     318 text files.
       9 files ignored.

github.com/AlDanial/cloc v 1.98  T=2.38 s (140.1 files/s, 38808.4 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
JSON
 same                            0              0              0              0
 modified                        2              0              0              2
 added                           2              0              0          48835
 removed                         2              0              0             27
Rust
 same                            0              0           1510          19937
 modified                       94              0             17            649
 added                          75            937            882           6325
 removed                        11            163            284           1122
Swift
 same                            0              0            105           1741
 modified                       31              0              4            119
 added                          87           1053            454           5615
 removed                         6             84             18            443
Kotlin
 same                            0              0              4           1078
 modified                       11              0              0             25
 added                           4             16              0             47
 removed                         0              0              0              5
-------------------------------------------------------------------------------
SUM:
 same                            0              0           1679          23333
 modified                      145              0             21            800
 added                         169           2011           1336          60918
 removed                        19            247            302           1597
-------------------------------------------------------------------------------

MFA Recap

Let us start with a short MFA (Multi-Factor) recap. Currently all "entities" (accounts / personas) are protected by a single factor, a single private key. This private key is an HD key, derived from a mnemonic, stored in secure storage and referenced to from Profile as a DeviceFactorSource. Each entity uses a per factor source based derivation path, meaning the first account created by Ledger L gets derivation path:
"m/44H/1022H/1H/525H/1460H/0H" and the second account for the same Ledger L is derived with path: "m/44H/1022H/1H/525H/1460H/1H". The first account for another factor source, DeviceFactorSource lets call it D, is derived at path "m/44H/1022H/1H/525H/1460H/0H" - same as the first account derived with L, and the second account derived using D is derived at "m/44H/1022H/1H/525H/1460H/1". All those 4 keys will be different, since the mnemonic backing L and D are different.

The public key of each of those private keys are stored in an UnsecuredEntityControl:

pub struct UnsecuredEntityControl {
    // /// The factor instance which was used to create this unsecured entity, which
    // /// also controls this entity and is used for signing transactions.
    pub transaction_signing: HierarchicalDeterministicFactorInstance,

which is stored EntitySecurityState

pub enum EntitySecurityState {
    /// The account is controlled by a single factor (private key)
    Unsecured(UnsecuredEntityControl)
}

which is stored in the entity (account / persona):

pub struct Account {
	// ... abbreviated
    pub address: AccountAddress,
    pub security_state: EntitySecurityState,

Factors

Lets recap "factor theory" (I just made that term up...):

There are 4 different kinds of "abstraction levels" of "factors":

  • FactorSourceKind
  • FactorSource
  • FactorSourceID
  • FactorInstance

In my mind: it is verboten to say just "factor", because it can mean any of those four levels! Lets recap them!

FactorSourceKind

We say FactorSourceKind when we talk about the notion of any factor source, but of a specific kind for something. E.g. We support using Ledger Hardware Wallets for signing transactions. We do not talk about any specfic Ledger Hardware device, but that kind of factor source. We do not say "type" because it would be confusing in code FactorSource.Type has very specific meaning in code.

FactorSource

The FactorSource level is talking about a specific Ledger Hardware wallet, one that you can hold in your hand!

FactorSourceID

The FactorSourceID is a content based and "content stable" identifier of a certain FactorSource, and it is the hash of a special public key derived at a special derivation path for any mnemonic. It is used to identify a FactorSource, e.g. "the ID of my orange Ledger NanoS is: ...". If I would factory restore my Ledger it gets initialized with a new mnemonic and its ID will have changed.

We use the FactorSourceID in Profile, all over the place, so that we can skip having duplication of data which would quickly go stale, and also this drastically minimizes the size of the Profile! Each derived HD Key references which FactorSource which was used to derive it, by the FactorSourceID of the factor source.

FactorInstance

An "instance of a factor", is a private & public key pair, which was derived using a FactorSource at some derivation path. A factor instance references the FactorSource used to derive it by the FactorSourceID, contains the public key and the derivation path.

MFA

MFA adds a new variant (case) to EntitySecurityState:

pub enum EntitySecurityState {
    /// The account is controlled by a single factor (private key)
    Unsecured(UnsecuredEntityControl),
+	/// References to an AccessController by address with the references to all
+   /// FactorSources used to produce the factors set in the AccessController.
+	Secured(SecuredEntityControl)
}

The shape of SecuredEntityControl is not final, this PR introduced this initial implementation, expect it to change!:

pub struct SecuredEntityControl {
    /// The address of the access controller which controls this entity.
    ///
    /// Looking up the public key (hashes) set in the key-value store at
    /// this address reveils the true factors (public keys) used to protect
    /// this entity. It will be the same as the ones in `security_structure`
    /// if we have not changed them locally, which we should not do unless
    /// we are sure the Ledger corresponds to the values in `security_structure`.
    pub access_controller_address: AccessControllerAddress,

    /// The believed-to-be-current security structure of FactorInstances which
    /// secures this entity.
    pub security_structure: SecurityStructureOfFactorInstances,
}

The SecurityStructureOfFactorInstances model alongside its "FactorSource" and "FactorSourceID" level counterparts are the primary content of this PR.

Roles

When an entity is "securitified", it has been upgraded from being controlled by a single factor to potentially many factors configured in an AccessController. The AccessController is an on-chain/on-ledger/on-network component, exactly like an account is an on-chain component. The on-chain component of a securified account (or identity (persona)) will reference its AccessController component, which has a set of public key hashes, representing the setup of keys ("factors") which control it. An AccessController has three different roles:

  • Primary Role: used for making transactions
  • Recovery Role: used to initiate recovery (and lock)
  • Confirmation Role: used to confirm recovery

For more details see any of the public articles about this (e.g. this one), or search on Confluence or Slack.

Changes

This PR introduces some of the first models needed for upgrading the hosts (iOS and Android wallets) to be able to offer users the ability to control their "entities" (accounts / personas) with multiple "factors".

The most important models introduced by this PRs are those modelling the "structure" - of which factors to use for which "role" (Primary / Recovery / Confirmation).

There exists three different kinds of structures, at different "abstraction levels" of "factor", at:

  • FactorInstance "level"
  • FactorSourceID "level"
  • FactorSource "level"

Those are named:

  • New model: SecurityStructureOfFactorInstances
  • New model: SecurityStructureOfFactorSources
  • New model: SecurityStructureOfFactorSourceIDs

Those are all defined using macros, since they are very similar. The most important field (stored property) of each are their respective matrix_of_factors, which are of different types:

Different matrix types:

  • MatrixOfFactorInstances
  • MatrixOfFactorSources
  • MatrixOfFactorSourceIDs

Each matrix is also defined by a macro, and their "shape" is:

// PSEUDOCODE
pub struct MatrixOfFactor + $factor_level {
    pub primary_role: PrimaryRoleWith + $factor_level ,
    pub recovery_role: RecoveryRoleWith + $factor_level ,
    pub confirmation_role: ConfirmationRoleWith + $factor_level ,
}

(Where + $factor_level is pseudocode for actual paste! code: [< MatrixOfFactor $factor_level>])

Where each role of each matrix, for each factor "level" too is defined by a macro, and their shape is:

pub struct PrimaryRoleWith + $factor_level {
    pub threshold_factors: Vec<factor_level>,
    pub threshold: u16,
    pub override_factors: Vec<factor_level>,
}

And repeated for RecoveryRole and ConfirmationRole.

Threshold vs Override factors

The field threshold_factors contains threshold factors (at their respective factor "level") which requires the user to sign with N of M factors. For example, user might specify threshold_factors to contain:
1 Ledger(FactorSource)
1 Device(FactorSource)
1 ArculusCard(FactorSource)
and then set threshold to 2, meaning any two of those three factors can be used for to "exercise this role" / "perform an action of this role", e.g. sign a transaction. The user can, of course, sign with all 3 factors as well. But 1 factor is not enough. If she sets threshold to 1 she need only sign with 1 of these factors ("any of" these factors). A hardcore crypto user might wanna protect their life savings with many factors, e.g. 7, and set a threshold of maybe 5 or 6.

The other list: override_factors are "sudo factors" / "god factors", where only one of these are enought to exercise this role.

Important

It would be completely meaningless let a factor X be present in both the threshold_factors and the override_factors list. So I propose we disallow it, which is how I've implemented the POC example app.

SecurityStructureOfFactorSources

The MatrixOfFactor is accompanied by some metadata and the number of epochs until confirmation role is automatically exercised.

pub struct SecurityStructureOfFactorSources {
    pub metadata: SecurityStructureMetadata,
    pub number_of_epochs_until_auto_confirmation: u64,
    pub matrix_of_factors: MatrixOfFactorSources,
}

The SecurityStructureOfFactorSources is called Security Shield in Figma and I chose to call it that in the example iOS app in this repo (see videos).

The user only ever thinks about SecurityStructureOfFactorSources level. But what is stored in Profile is SecurityStructureOfFactorSourceIDs at FactorSourceID "level", just like FactorInstances in UnsecuredEntityControl does not contain a (will-go-stale-and-make-Profile-big) clone of a FactorSource, instead it references it just by its id!

The mapping SecurityStructureOfFactorSources -> SecurityStructureOfFactorSourceIDs is trivial, and the inverse mapping is throwing, and needs access to factor_sources in Profile. It should never fail, if it does, it means that user somehow corrupted their profile by removing previously saved factor sources, or that we had a bug in hosts (iOS / Android wallet) and/or Sargon which allowed user to specify Factor Source which was not present in the users Profile.

The user builds up a SecurityStructureOfFactorSources (Shield) by selecting which factor sources to use for which role, and if they wanna use it as a "threshold factor" or an "override factor".

Demo Videos

Demo 1of3 - FactorSource managment (all but security questions)

Demo 2of3 - SecurityQuestionsFactorSource add and use

Demo 3of3 - Create, View and Edit "Security Shields"

Copy link
Contributor

@matiasbzurovski matiasbzurovski left a comment

Choose a reason for hiding this comment

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

Great work! Asides from the diffie hellman logic on the Security Questions (which I plan to revisit), everything else was very clear and easy to follow.

Regarding those Security Questions, do we have any test for validating different combinations (e.g. that any 4 correct answers out of 6 allow the user to confirm recovery)?

@@ -9,9 +9,10 @@ class EventBusDriverTests: DriverTest<EventBus> {
func test() async throws {
let sut = SUT()

let expectedEvents = Array<EventKind>([.booted, .profileSaved, .factorSourceUpdated, .accountAdded, .profileSaved])
Copy link
Contributor

Choose a reason for hiding this comment

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

.profileSaved is repeated here, but it won't actually matter for the test since Set removes duplicates. Should we actually verify that such event is dispatched twice?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, needed! first we

  1. update "last used on" on factor source => [profileSaved, .factorSourceUpdated]
  2. then we add account => [accountAdded, .profileSaved]

We need the correct length since we prefix in the Task .prefix(expectedEvents.count) { so expected cannot be set here => results in wrong length => we dont get all events we need. but this solution works fine :)

Copy link
Contributor

Choose a reason for hiding this comment

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

oh! didn't notice the prefix, thanks for explanation!

).asGeneral

case .ledgerHqHardwareWallet:
LedgerHardwareWalletFactorSource.init(
Copy link
Contributor

Choose a reason for hiding this comment

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

Guess all of these will move under sargon_os_factors eventually?

Copy link
Contributor

@Sajjon Sajjon Jun 13, 2024

Choose a reason for hiding this comment

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

yes, we will not create LedgerHardwareWalletFactorSource in hosts at all, Sargon will do it! :) and similar with Arculus and others

public func toString(kind: FactorSourceKind) -> String {
switch self {
case let .canNeverBeUsedForRole(role): "Cannot be used as \(role)"
case let .exceededLimitOfKindPerRoleReached(exceededLimit, role): "Cannot use more than \(exceededLimit) factors of kind \(kind) for \(role)"
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe keep the kind in the associated value as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

mm thought about that too!

} else if count == 1 {
self = .any
} else {
self = .threshold(count)
Copy link
Contributor

Choose a reason for hiding this comment

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

assert count > 0?

Copy link
Contributor

Choose a reason for hiding this comment

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

it can be 0. for a Role where we specify override factors only, but no thresholdFactor.

Ok(true)
}

pub async fn add_factor_sources(
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing doc

Copy link
Contributor

Choose a reason for hiding this comment

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

fixed

Ok(ids)
}

pub async fn debug_add_all_sample_factors(
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be under #[cfg(test)] ?

Copy link
Contributor

Choose a reason for hiding this comment

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

nope, it is UniFFI exported.

Copy link
Contributor

Choose a reason for hiding this comment

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

where is that? couldn't find it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

check the impl scope in which this method is defined, it has #[uniffi::export]. This us a UniFFI object not a record, and for Objects in UniFFI we add the #[uniffi::export] attached to the impl itself, so all methods inside that impl are exported.

.await
}

pub async fn create_device_factor_source(
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing docs

Copy link
Contributor

Choose a reason for hiding this comment

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

fixed

})
}

/// Adds the security stricture of factor sources to Profile if none with the
Copy link
Contributor

Choose a reason for hiding this comment

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

typo structure

Copy link
Contributor

Choose a reason for hiding this comment

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

why did not typos CLI tool pick this up?!

Copy link
Contributor

Choose a reason for hiding this comment

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

Screenshot 2024-06-14 at 10 11 06

Copy link
Contributor Author

Choose a reason for hiding this comment

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

aha!

Copy link
Contributor

Choose a reason for hiding this comment

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

fixed

&self,
structure_of_ids: &SecurityStructureOfFactorSourceIDs,
) -> Result<SecurityStructureOfFactorSources> {
self.profile_holder.try_access_profile_with(|p| {
Copy link
Contributor

Choose a reason for hiding this comment

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

When should we use access_profile_with and when the try_access_profile_with ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Those named try_ is for when the closure returns Result<T>. Compare the signature of the methods:

<T, F>(&self, access: F) -> T where F: Fn(RwLockReadGuard<'_, Profile>) -> T;
<T, F>(&self, access: F) -> Result<T> where F: Fn(RwLockReadGuard<'_, Profile>) -> Result<T>;

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, noticed such difference, but my question was more like:
"how do we know when it is safe to just access_profile (and expect T), and one should we play safe and use try_access_profile (which can return Err)?"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be easy to know, here I'm calling a faillable function, SecurityStructureOfFactorSources::try_from, so I MUST use try_access_profile_with.

And we really SHOULD NOT use faillable unnecessarily, so when in doubt use a non-faillable one, and then the Rust compiler will yell at you if you call something faillable :D

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know what was I thinking last week, it is perfectly clear to me now

@Sajjon
Copy link
Contributor

Sajjon commented Jun 13, 2024

Great work! Asides from the diffie hellman logic on the Security Questions (which I plan to revisit), everything else was very clear and easy to follow.

Let me know if I should do a video explaining it in depth! Maybe a 30min meeting with the whole team which we record. And you can ask questions and I can explain it? Would that be helpful?

Regarding those Security Questions, do we have any test for validating different combinations (e.g. that any 4 correct answers out of 6 allow the user to confirm recovery)?

I think it is pretty OK tested, did you see all the fn roundtrip_ tests in security_questions_factor_source.rs? Do you want me to add some tests which randomly selects different Answers to change to incorrect?

@matiasbzurovski
Copy link
Contributor

Let me know if I should do a video explaining it in depth! Maybe a 30min meeting with the whole team which we record. And you can ask questions and I can explain it? Would that be helpful?

I think a recorded meeting to be used as reference in the future would be amazing!

I think it is pretty OK tested, did you see all the fn roundtrip_ tests in security_questions_factor_source.rs? Do you want me to add some tests which randomly selects different Answers to change to incorrect?

I had missed those tests, sorry. I think we cover the cases of 6/5/4 correct answers, but we don't test than when having less than 4 correct decryption fails.
Also, we can replace the index of the answer updated with wrong value with randomness.

Copy link
Contributor

@micbakos-rdx micbakos-rdx left a comment

Choose a reason for hiding this comment

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

Very nice work especially with documentation! Easy to understand.

@@ -33,27 +33,28 @@ impl PartialOrd for BIP39Word {
}

impl BIP39Word {
pub fn new(word: &'static str, language: BIP39Language) -> Result<Self> {
pub fn new(word: impl AsRef<str>, language: BIP39Language) -> Result<Self> {
Copy link
Contributor

Choose a reason for hiding this comment

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

I have seen this change (impl AsRef<str>) in many of your recent PRs. Is this something we need to keep in mind when developing in Rust?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is nice to use impl AsRef<str> or impl Into<String> instead of String or &str since both can be passed!

This allows us to skip calling .to_owned() on a &str, but it also allows us to pass a String if we have one.

mnemonics
}

#[cfg(test)]
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't know that we can do that to just top level functions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you can use it on:
modules
functions
or impls

I see that I actually simply can move these functions into the mod tests. At first the were used by other files too.

Comment on lines +47 to +48
source.common.last_used_on = Timestamp::sample();
source.common.added_on = Timestamp::sample();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this function also used elsewhere? Could this live inside the impl HasSampleValues ?

}

#[uniffi::export]
pub fn trim_security_questions_answer(answer: String) -> String {
Copy link
Contributor

Choose a reason for hiding this comment

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

So this method is intended to be used in case the host needs to display the trimmed version to the user? The method that passes the answer to sargon in order to validate will use the free form string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes our real hosts (Radix Wallet on Android and iOS) will most likely not use this (pretty sure it is not something Aftab/Matt wanna show to the user), at least not their PROD/BETA build channels. But we might wanna use it for Alpha/PreAlpha and I use it in Demo App here in Sargon.

I think I only use it to display which answer will "actually" be used. I dont think I pass this String back to Sargon, Sargon calls trim_answer internally in bytes_from_answer called from derive_key_exchange_key_from_question_and_answer

/// least `threshold` many instances to perform some function with this role.
///
/// # Implementation
/// Must allow duplicates, thus using `Vec` since at FactorSourceKind level
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

///
/// And also emits `Event::ProfileSaved` after having successfully written the JSON
/// of the active profile to secure storage.
async fn add_factor_sources_without_emitting_factor_sources_added(
Copy link
Contributor

Choose a reason for hiding this comment

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

Why such notification shouldn't be emitted here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because this is a "lower level" function which is used by two other higher level functions:

  • add_factor_source
  • add_factor_sources (note the plural)
    And for these we wanna emit two different Notifications: EventProfileModified::FactorSourceAdded and EventProfileModified::FactorSourcesAdded (note the plural) respectively.

Copy link
Contributor

@sergiupuhalschi-rdx sergiupuhalschi-rdx left a comment

Choose a reason for hiding this comment

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

Great work! Very easy to understand documentation and examples. I'm keeping an eye on discussions.

@@ -0,0 +1,8 @@
use crate::prelude::*;

pub trait IsSecurityQuestionsKDFScheme {
Copy link
Contributor

Choose a reason for hiding this comment

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

Couldn't trait just be named SecurityQuestionsKDFScheme?

Copy link
Contributor

Choose a reason for hiding this comment

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

It sounds a bit like it is about an actual scheme.

I am not yet to used to naming traits, but Scrypto and RET names some with prefix "Is", so I tried copying that style

Copy link
Contributor

@jakub-rdx jakub-rdx Jun 18, 2024

Choose a reason for hiding this comment

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

ahh ok, I was thinking if there is a pattern using Is prefix, if it is established convention in other project as well, it makes sense

})
.collect_vec();

Ok(Self {
Copy link
Contributor

@jakub-rdx jakub-rdx Jun 18, 2024

Choose a reason for hiding this comment

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

just want to double check if I understand the procedure correctly. We store multiple encrypted contents of a mnemonic (to allow for some errors in Q & A set), and then when we decrypt and generate multiple decryption keys, we try to decrypt until we succeed?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you understand it correctly, but your description was a bit inaccurate/unclear:

The word "generate" entails randomness, so it is inaccurate (we say we generate entropy, from which we encode a BIP39 mnemonic). What we do is we "derive" Symmetric (En/De)cryption keys from set of pairs of (Question, Answer) tuples. "Derive" is not random, it is determinisitc.

We derive the same encryption keys part of creation (=encryption) and usage (=decryption) of a of a SecurityQuestionsFactorSource. Or well, the same if not too many questions were incorrectly answered.

And yes we try all combinations, 4*4 = 16, of encryptions of mnemonic and decryption keys, the first combination to succeed => return the decrypted mnemonic.

Copy link
Contributor

@danvleju-rdx danvleju-rdx left a comment

Choose a reason for hiding this comment

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

Amazing work as usual! 👍

Base automatically changed from ac/system_with_drivers to sargon_os June 28, 2024 13:17
@CyonAlexRDX CyonAlexRDX merged commit df13633 into sargon_os Jun 28, 2024
8 checks passed
@CyonAlexRDX CyonAlexRDX deleted the mfa_batch_1 branch June 28, 2024 13:27
GhenadieVP added a commit that referenced this pull request Jul 29, 2024
* Rust: `SargonOS` &`Drivers` more Profile Logic | Swift: Shared State  (working example app)! (#131)

Introducing SargonOS

* MFA: Sec Structure of Factor <Source | ID | Instance>s (#150)

MFA Batch 1: SecurityStructureOfFactorSources and MatrixOfFactorSoures and also on FactorSourceID and FactorInstance level. Also add some new FactorSourceKinds at least a POC version of them or stubbed out ones.

* Sargon OS undo Profile JSON format change (#177)

Revert Profile JSON format changes in DeviceInfo

* Merge 'main' into 'sargon_os'

* Implement HostInfo struct (#189)

* Implement HostInfo struct

* Store host id in secure storage

* Fix doc

* Separate id and date from host info

* Implement HostOS that will encapsulate the different values and constants of each host os

* Expose uniffi method for creating device info from host info

* Small fix for AppleHostDriver (#192)

wip

---------

Co-authored-by: micbakos-rdx <michael.bakogiannis@rdx.works>
Co-authored-by: micbakos-rdx <125959264+micbakos-rdx@users.noreply.github.com>
Co-authored-by: Ghenadie <118184705+GhenadieVP@users.noreply.github.com>
GhenadieVP added a commit that referenced this pull request Aug 12, 2024
* Rust: `SargonOS` &`Drivers` more Profile Logic | Swift: Shared State  (working example app)! (#131)

Introducing SargonOS

* MFA: Sec Structure of Factor <Source | ID | Instance>s (#150)

MFA Batch 1: SecurityStructureOfFactorSources and MatrixOfFactorSoures and also on FactorSourceID and FactorInstance level. Also add some new FactorSourceKinds at least a POC version of them or stubbed out ones.

* Sargon OS undo Profile JSON format change (#177)

Revert Profile JSON format changes in DeviceInfo

* Merge 'main' into 'sargon_os'

* wip

* Implement HostInfo struct (#189)

* Implement HostInfo struct

* Store host id in secure storage

* Fix doc

* Separate id and date from host info

* Implement HostOS that will encapsulate the different values and constants of each host os

* Expose uniffi method for creating device info from host info

* wip

* Sargon os v1 android (#196)

* Implement sargon os drivers

* Boot sargon os in example app

* Remove active profile id

* Add more unit tests

* Fix apple tests

* pr review

* version bump

---------

Co-authored-by: Alexander Cyon <116169792+CyonAlexRDX@users.noreply.github.com>
Co-authored-by: micbakos-rdx <michael.bakogiannis@rdx.works>
Co-authored-by: micbakos-rdx <125959264+micbakos-rdx@users.noreply.github.com>
micbakos-rdx added a commit that referenced this pull request Aug 14, 2024
* Rust: `SargonOS` &`Drivers` more Profile Logic | Swift: Shared State  (working example app)! (#131)

Introducing SargonOS

* MFA: Sec Structure of Factor <Source | ID | Instance>s (#150)

MFA Batch 1: SecurityStructureOfFactorSources and MatrixOfFactorSoures and also on FactorSourceID and FactorInstance level. Also add some new FactorSourceKinds at least a POC version of them or stubbed out ones.

* Sargon OS undo Profile JSON format change (#177)

Revert Profile JSON format changes in DeviceInfo

* Merge 'main' into 'sargon_os'

* Implement HostInfo struct (#189)

* Implement HostInfo struct

* Store host id in secure storage

* Fix doc

* Separate id and date from host info

* Implement HostOS that will encapsulate the different values and constants of each host os

* Expose uniffi method for creating device info from host info

* wip

* Sargon os v1 android (#196)

* Implement sargon os drivers

* Boot sargon os in example app

* Remove active profile id

* Add more unit tests

* Fix apple tests

* Change profile holder to keep profile state instead of profile

* Driver emits profile states instead of profile

* Implement new and delete wallet functions

* Remove unused comment

* Implement import functionality

* Propagate CRUD operations on profile to the profile state change driver.

* Remove unnecessary comment

* Fix swift tests

* Fix doc

* Prioritize bdfs save and then save profile in new wallet

* Implement retry read mechanism for profile snapshot.

This currently exists on Android codebase and needs revisiting.

* Bump version

---------

Co-authored-by: Alexander Cyon <116169792+CyonAlexRDX@users.noreply.github.com>
Co-authored-by: Ghenadie Vasiliev-Pusca <ghenadie.vasiliev-pusca@rdx.works>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Coverage OK ☂️ ✅ False Negatives in Code Coverage by Tarpaulin Rust 🦀 Changes in Rust Sargon UniFFI 🦄 Changes in UniFFI exported APIs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants