Skip to content

Commit

Permalink
Remove unsafe hacks in CertificatesStore::for_read/for_write not th…
Browse files Browse the repository at this point in the history
…at async closure is available
  • Loading branch information
touilleMan committed Feb 28, 2025
1 parent 3a602e5 commit d58f1a0
Show file tree
Hide file tree
Showing 17 changed files with 88 additions and 125 deletions.
2 changes: 1 addition & 1 deletion libparsec/crates/client/src/certif/block_validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ pub(super) async fn validate_block(

let block = ops
.store
.for_read_with_requirements(ops, &needed_timestamps, |store| async move {
.for_read_with_requirements(ops, &needed_timestamps, async |store| {
realm_keys_bundle::decrypt_for_realm(
ops,
store,
Expand Down
22 changes: 15 additions & 7 deletions libparsec/crates/client/src/certif/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub(super) async fn get_current_self_profile(
ops: &CertificateOps,
) -> Result<UserProfile, CertifGetCurrentSelfProfileError> {
ops.store
.for_read(|store| store.get_current_self_profile())
.for_read(async |store| store.get_current_self_profile().await)
.await?
.map_err(|err| err.into())
}
Expand All @@ -29,7 +29,11 @@ pub(super) async fn get_current_self_realms_role(
// TODO: cache !
let certifs = ops
.store
.for_read(|store| store.get_user_realms_roles(UpTo::Current, ops.device.user_id))
.for_read(async |store| {
store
.get_user_realms_roles(UpTo::Current, ops.device.user_id)
.await
})
.await??;

// Replay the history of all changes
Expand Down Expand Up @@ -58,7 +62,7 @@ pub(super) async fn get_current_self_realm_role(
) -> Result<Option<Option<RealmRole>>, CertifGetCurrentSelfRealmRoleError> {
let role = ops
.store
.for_read(|store| async move { store.get_self_user_realm_role(realm_id).await })
.for_read(async |store| store.get_self_user_realm_role(realm_id).await)
.await??;
Ok(role)
}
Expand Down Expand Up @@ -87,7 +91,7 @@ pub(super) async fn list_users(
limit: Option<u32>,
) -> Result<Vec<UserInfo>, CertifListUsersError> {
ops.store
.for_read(|store| async move {
.for_read(async |store| {
let certifs = {
// If `skip_revoked` is enabled, we cannot limit the number of users
// certificates to fetch given we will then filter out the ones corresponding
Expand Down Expand Up @@ -163,7 +167,11 @@ pub(super) async fn list_user_devices(
) -> Result<Vec<DeviceInfo>, CertifListUserDevicesError> {
let certifs = ops
.store
.for_read(|store| store.get_user_devices_certificates(UpTo::Current, user_id))
.for_read(async |store| {
store
.get_user_devices_certificates(UpTo::Current, user_id)
.await
})
.await??;

let items = certifs
Expand Down Expand Up @@ -211,7 +219,7 @@ pub(super) async fn get_user_device(
device_id: DeviceID,
) -> Result<(UserInfo, DeviceInfo), CertifGetUserDeviceError> {
ops.store
.for_read(|store| async move {
.for_read(async |store| {
let device_certif = match store.get_device_certificate(UpTo::Current, device_id).await {
Ok(certif) => certif,
Err(GetCertificateError::ExistButTooRecent { .. }) => {
Expand Down Expand Up @@ -319,7 +327,7 @@ pub(super) async fn list_workspace_users(
let mut infos = Vec::new();

ops.store
.for_read(|store| async move {
.for_read(async |store| {
let per_user_certifs = store
.get_realm_current_users_roles(UpTo::Current, realm_id)
.await?;
Expand Down
6 changes: 3 additions & 3 deletions libparsec/crates/client/src/certif/manifest_validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ pub(super) async fn validate_user_manifest(
);

ops.store
.for_read_with_requirements(ops, &needed_timestamps, |store| async move {
.for_read_with_requirements(ops, &needed_timestamps, async |store| {
// 1) Decrypt the vlob

let cleartext = &ops.device.user_realm_key.decrypt(encrypted).map_err(|_| {
Expand Down Expand Up @@ -195,7 +195,7 @@ pub(super) async fn validate_workspace_manifest(
);

ops.store
.for_read_with_requirements(ops, &needed_timestamps, |store| async move {
.for_read_with_requirements(ops, &needed_timestamps, async |store| {
// 1) Decrypt the vlob

let cleartext = realm_keys_bundle::decrypt_for_realm(
Expand Down Expand Up @@ -312,7 +312,7 @@ pub(super) async fn validate_child_manifest(
);

ops.store
.for_read_with_requirements(ops, &needed_timestamps, |store| async move {
.for_read_with_requirements(ops, &needed_timestamps, async |store| {
// 1) Decrypt the vlob

let cleartext = realm_keys_bundle::decrypt_for_realm(
Expand Down
16 changes: 7 additions & 9 deletions libparsec/crates/client/src/certif/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ impl CertificateOps {
realm_certificates: &std::collections::HashMap<VlobID, Vec<Bytes>>,
) -> Result<MaybeRedactedSwitch, CertifAddCertificatesBatchError> {
self.store
.for_write(move |store| async move {
.for_write(async |store| {
add::add_certificates_batch(
self,
store,
Expand Down Expand Up @@ -261,7 +261,7 @@ impl CertificateOps {
// ) -> anyhow::Result<()> {
// use crate::certif::realm_keys_bundle::RealmKeys;

// self.store.for_read(|store| async move {
// self.store.for_read(async |store| {
// let keys = Arc::new(RealmKeys {
// realm_id,
// keys
Expand All @@ -280,7 +280,7 @@ impl CertificateOps {
latest_known_timestamps: Option<&PerTopicLastTimestamps>,
) -> Result<(), CertifPollServerError> {
self.store
.for_write(|store| async move {
.for_write(async |store| {
poll::poll_server_for_new_certificates(self, store, latest_known_timestamps).await
})
.await?
Expand Down Expand Up @@ -392,7 +392,7 @@ impl CertificateOps {
data: &[u8],
) -> Result<(Vec<u8>, IndexInt), CertifEncryptForRealmError> {
self.store
.for_read(|store| async move {
.for_read(async |store| {
realm_keys_bundle::encrypt_for_realm(self, store, usage, realm_id, data).await
})
.await
Expand All @@ -418,7 +418,7 @@ impl CertificateOps {
) -> Result<Vec<u8>, CertifDecryptForRealmError> {
let outcome = self
.store
.for_read(|store| async move {
.for_read(async |store| {
realm_keys_bundle::decrypt_for_realm(
self, store, usage, realm_id, key_index, encrypted,
)
Expand Down Expand Up @@ -450,7 +450,7 @@ impl CertificateOps {
})?;

self.store
.for_read(|store| async move {
.for_read(async |store| {
realm_keys_bundle::decrypt_for_realm(
self, store, usage, realm_id, key_index, encrypted,
)
Expand All @@ -470,9 +470,7 @@ impl CertificateOps {
) -> Result<Option<Vec<(SequesterServiceID, Bytes)>>, CertifEncryptForSequesterServicesError>
{
self.store
.for_read(
|store| async move { encrypt::encrypt_for_sequester_services(store, data).await },
)
.for_read(async |store| encrypt::encrypt_for_sequester_services(store, data).await)
.await
.map_err(|e| match e {
CertifStoreError::Stopped => CertifEncryptForSequesterServicesError::Stopped,
Expand Down
2 changes: 1 addition & 1 deletion libparsec/crates/client/src/certif/realm_create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ pub(super) async fn ensure_realm_created(
) -> Result<CertificateBasedActionOutcome, CertifEnsureRealmCreatedError> {
let realm_state = ops
.store
.for_read(|store| async move { store.get_realm_bootstrap_state(realm_id).await })
.for_read(async |store| store.get_realm_bootstrap_state(realm_id).await)
.await??;
if matches!(realm_state, RealmBootstrapState::CreatedInServer) {
return Ok(CertificateBasedActionOutcome::LocalIdempotent);
Expand Down
2 changes: 1 addition & 1 deletion libparsec/crates/client/src/certif/realm_decrypt_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ pub(super) async fn decrypt_current_realm_name(
realm_id: VlobID,
) -> Result<(EntryName, DateTime), CertifDecryptCurrentRealmNameError> {
ops.store
.for_read(|store| async move {
.for_read(async |store| {
// 1) Retrieve the realm name certificate

let maybe_certif = store
Expand Down
2 changes: 1 addition & 1 deletion libparsec/crates/client/src/certif/realm_key_rotation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ async fn generate_realm_rotate_key_req(
) -> Result<(authenticated_cmds::latest::realm_rotate_key::Req, IndexInt), CertifRotateRealmKeyError>
{
ops.store
.for_read(|store| async move {
.for_read(async |store| {
// 1) Generate the next keys bundle

// Note that given this function is idempotent, this new keys bundle might get
Expand Down
2 changes: 1 addition & 1 deletion libparsec/crates/client/src/certif/realm_keys_bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ pub(super) enum AttemptRealmKeysBundleHealingError {
// realm_id: VlobID,
// ) -> Result<KeysBundleHealingOutcome, AttemptRealmKeysBundleHealingError> {
// loop {
// let outcome = ops.store.for_read(|store| async move {
// let outcome = ops.store.for_read(async |store| {
// recover_realm_keys_from_previous_bundles(ops, realm_id).await
// }).await??;

Expand Down
2 changes: 1 addition & 1 deletion libparsec/crates/client/src/certif/realm_rename.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ pub(super) async fn ensure_realm_initial_rename(

let has_initial_rename = ops
.store
.for_read(|store| async move {
.for_read(async |store| {
store
.get_realm_last_name_certificate(UpTo::Current, realm_id)
.await
Expand Down
2 changes: 1 addition & 1 deletion libparsec/crates/client/src/certif/realm_share.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ async fn share_do_server_command(
let (encrypted_keys_bundle_access, key_index) = ops
.store
.for_read({
|store| async move {
async |store| {
super::realm_keys_bundle::encrypt_realm_keys_bundle_access_for_user(
ops, store, realm_id, recipient,
)
Expand Down
2 changes: 1 addition & 1 deletion libparsec/crates/client/src/certif/realms_needs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ pub async fn get_realm_needs(
realm_id: VlobID,
) -> Result<RealmNeeds, CertifGetRealmNeedsError> {
ops.store
.for_read(|store| async move {
.for_read(async |store| {
// TODO: This implementation is not efficient, as it fetches all the roles
// and users certificates. However this should be good enough for now,
// and optimization requires a complex API changes in the storage.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ pub(super) async fn delete_shamir_recovery(
let last_shamir_recovery = ops
.store
.for_read({
|store| async move {
async |store| {
store
.get_last_shamir_recovery_for_author(UpTo::Current, author_user_id)
.await
Expand Down
6 changes: 3 additions & 3 deletions libparsec/crates/client/src/certif/shamir_recovery_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ pub async fn get_self_shamir_recovery(
ops: &CertificateOps,
) -> Result<SelfShamirRecoveryInfo, CertifGetSelfShamirRecoveryError> {
ops.store
.for_read(|store| async move {
.for_read(async |store| {
// 1. Retrieve the shamir recovery and it potential deletion

let brief = match store
Expand Down Expand Up @@ -205,7 +205,7 @@ pub async fn list_shamir_recoveries_for_others(
ops: &CertificateOps,
) -> Result<Vec<OtherShamirRecoveryInfo>, CertifListShamirRecoveriesForOthersError> {
ops.store
.for_read(|store| async move {
.for_read(async |store| {
let mut per_user_last_shamir: HashMap<
UserID,
(
Expand Down Expand Up @@ -395,7 +395,7 @@ pub async fn get_shamir_recovery_share_data(
) -> Result<ShamirRecoveryShareData, CertifGetShamirRecoveryShareDataError> {
let needed_timestamps = PerTopicLastTimestamps::new_for_shamir(shamir_recovery_created_on);
ops.store
.for_read_with_requirements(ops, &needed_timestamps, |store| async move {
.for_read_with_requirements(ops, &needed_timestamps, async |store| {

// 1. Check that the corresponding shamir recovery has not been deleted

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ async fn check_against_local_certificates(
let author_user_id = ops.device.user_id;
ops.store
.for_read({
|store| async move {
async |store| {
// 1. Shamir already exists ?

match store
Expand Down
Loading

0 comments on commit d58f1a0

Please sign in to comment.