feat(sdk-crypto): Add Identity local tofu trust flag. #3610
Conversation
BillCarsonFr
force-pushed
the
valere/invisible_crypto/identity_local_tofu
branch
from
371c257 to
77d05a4
Compare
| serialize_with = "atomic_bool_serializer", | ||
| deserialize_with = "atomic_bool_deserializer" | ||
| )] | ||
| tofu_trusted: Arc<AtomicBool>, |
There was a problem hiding this comment.
tofu_trusted doesn't seem like a great name for for a flag that means either "the first identity we saw for this user" or "not the first identity, but the user acknowledged it". TBH, I'm wondering if we should actually split it into two different flags.
There was a problem hiding this comment.
Or an enum, given that the two flags would be mutually exclusive?
There was a problem hiding this comment.
Or an enum, given that the two flags would be mutually exclusive?
I did think about that, but we still have the problem of what to call the damn thing
There was a problem hiding this comment.
I discussed this with @BillCarsonFr and the semantics of the flag are roughly whether the identity is pinned or not, so perhaps the name could be pinned.
However, the way it's currently implemented—where we immediately replace the stored identity as soon as a new one is returned from the server, but with this flag set to false—means that a server can delete your pin by simply presenting a new identity and then switching back to the old one, resulting in the flag switching from true to false.
Valere is going to experiment with an alternative approach, where we
- Remember the old pinned identity as long as a new identity doesn't become pinned, replacing the old one
- Using the logic
current != pinnedto control whether the user should be notified / prompted to confirm
| @@ -447,6 +474,21 @@ impl ReadOnlyUserIdentity { | |||
| &self.self_signing_key | |||
| } | |||
|
|
|||
| /// Check if our identity is verified. | |||
There was a problem hiding this comment.
This certainly doesn't mean that it is "verified", which would imply we have done the cross-signing dance.
| @@ -406,19 +425,22 @@ impl ReadOnlyUserIdentity { | |||
| /// * `master_key` - The master key of the user identity. | |||
| /// | |||
| /// * `self signing key` - The self signing key of user identity. | |||
| /// * `tofu_trusted` - Is this identity tofu trusted. | |||
There was a problem hiding this comment.
If we're sticking with a single flag, we will need to explain what "tofu trusted" actually means each time we reference it.
| } | ||
|
|
||
| // The tofu_trusted field introduced a new field in the database | ||
| // schema, for backwards compatibility we assume that if the identity is |
There was a problem hiding this comment.
| // schema, for backwards compatibility we assume that if the identity is | |
| // schema. For backwards compatibility, we assume that if the identity is |
|
Here is the alternative proposal #3639 |
part of invisible crypto, follow up of #3607
Fixes #3564
Signed-off-by: