[PM-5693] CryptoService using memfd_secret on Linux #7

dani-garcia · 2024-10-22T16:11:54Z

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-5693

📔 Objective

I've been looking into using memfd_secret to protect keys in memory on Linux.

The memfd_secret API is similar to malloc in that we get some memory range allocated for us, but this memory is only visible to the process that has access to the file descriptor, which should protect us from any memory dumping from anything other than a kernel driver.

https://man.archlinux.org/man/memfd_secret.2.en

Using this requires changing how we store keys, so this is the perfect moment to go through removing the raw keys from the API surface.

Changes

Added a KeyRef trait and SymmetricKeyRef/AsymmetricKeyRef subtraits to represent the key types. Also a small macro to quickly implement them. KeyRef implementations are user defined types that implements Eq+Hash+Copy, and don't contain any key material
KeyEncryptable/KeyDecryptable are now Decryptable/Encryptable, and instead of taking the key as parameter, they take a CryptoServiceContext and a KeyRef. This way keys are never exposed to the clients.
KeyLocator trait is replaced by UsesKey. This trait only returns the KeyRef of the key required to decrypt it, instead of fetching the key itself.
Created a KeyStore trait, with some simple CRUD methods. We have two implementations, one based on memfd_secret for Linux, and another one based on a Rust boxed slice, that also applies mlock if possible.
Because both mlock and memfd_secret apply their protection over a specified memory area, we need a Rust data structure that is laid out linearly and also doesn't reallocate by itself. Also because memfd_secret allocates the memory for us, we can't use a std type like Vec (reason is the Vec must be allocated by the Rust allocator [ref]). This basically only leaves us with a Rust slice, on top of which we'd need to implement insert/get/delete. To allow for reuse and easier testing I've created SliceKeyStore, which implements the CRUD methods from the KeyStore trait on top of a plain slice. Then the actual mlock and memfd_secret implementations just need to implement a trait casting their data to a slice. The data is stored as a slice of Option<(KeyRef, KeyMaterial)>, and the keys are unique and sorted in the slice for easier lookups.
Created CryptoService, which contains the KeyStores and some encrypt/decrypt utility functions. From a CryptoService you can also initialize a CryptoServiceContext
A CryptoServiceContext contains a read only view of the keys inside the CryptoService, plus a read-write ephemeral key store, for use by Decryptable/Encryptable implementations when they need to temporarily store some keys (Cipher keys, attachment keys, send keys...).

Migrated the codebase to use these changes in a separate PR: bitwarden/sdk-sm#1117

📸 Screenshots

⏰ Reminders before review

Contributor guidelines followed
All formatters and local linters executed and passed
Written new unit and / or integration tests where applicable
Protected functional changes with optionality (feature flags)
Used internationalization (i18n) for all UI strings
CI builds passed
Communicated to DevOps any deployment requirements
Updated any necessary documentation (Confluence, contributing docs) or informed the documentation
team

🦮 Reviewer guidelines

👍 (:+1:) or similar for great changes
📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
❓ (:question:) for questions
🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed
issue and could potentially benefit from discussion
🎨 (:art:) for suggestions / improvements
❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
⛏ (:pick:) for minor or nitpick changes

Otherwise some Drop implementations can crash

dani-garcia · 2024-12-12T13:13:24Z

I've made a few changes while documenting this a bit, plus some of the review suggestions are included here as well:

memsec is disabled on windows for now, and we're not using my fork
There's been a few renames:
- CryptoService -> KeyStore. to move away from the service name
- Keys -> KeyStoreInner. This only exists to be wrapped with the RwLock, and everything in KeyStore is pretty much delegated to it, so it seems like a better name.
- CryptoServiceContext -> KeyStoreContext. To match the service rename
- KeyStore -> StoreBackend. To signify there are different backends per platform
- LinuxMemfdSecretKeyStore -> LinuxMemfdSecretBackend and RustStore -> RustBackend, to match the trait above.
Changed the generic params of the KeyStore to be a single KeyRefs, which makes the generic types everywhere much less verbose (Instead of having to do <SymmetricKeyRef, AsymmetricKeyRef> everywhere you can do <KeyRefs>.
Removed the generic type over KeyStoreContext global keys, and moved it to an enum.

The next steps for the crypto refactor after this are:

Move all the Encryptable implementations to the new trait: [PM-5693] Migrate SDK to CryptoService #8
Migrate existing APIs that deal with SymmetricCryptoKey/AsymmetricCryptoKey directly to deal with references instead (init_crypto functions, MasterKey, UserKey, PinKey, Key fingerprint etc).
Remove uses of KeyStoreContext::get_key|set_key from the non crypto crates, which shouldn't have any uses left after the previous step (maybe?).
Once no other uses of SymmetricCryptoKey/AsymmetricCryptoKey are left, make them private to bitwarden-crypto
Remove the now unnecessary boxing around SymmetricCryptoKey/AsymmetricCryptoKey

Some future API improvements:

Improve the design around local keys, instead of accepting any user type, create an opaque token and return it in the functions, so the only way for a user to get a valid local key ref is to obtain it from a function.
Currently the context implementation can be read-only or read-write over the global keys, depending on how it's initialized, and it's behavior changes slightly in both cases. I think it might be a better design to instead do something like SQL transactions. Contexts are always read-only, but they have a commit function that consumes them and applies the changes to the global KeyStore

Hinton

Still need to review the rust backend.

crates/bitwarden-crypto/Cargo.toml

crates/bitwarden-crypto/src/error.rs

crates/bitwarden-crypto/src/lib.rs

crates/bitwarden-crypto/src/keys/mod.rs

Hinton · 2025-01-13T13:54:25Z

crates/bitwarden-crypto/src/store/backend/implementation/linux_memfd_secret.rs

+
+// For Send+Sync to be safe, we need to ensure that the memory is only accessed mutably from one
+// thread. To do this, we have to make sure that any funcion in `MemfdSecretImplKeyData` that
+// accesses the pointer mutably is defined as &mut self, and that the pointer is never copied or


Suggested change

// accesses the pointer mutably is defined as &mut self, and that the pointer is never copied or

// accesses the pointer mutably is defined as `&mut self`, and that the pointer is never copied or

Hinton · 2025-01-13T13:58:20Z

crates/bitwarden-crypto/src/store/backend/implementation/linux_memfd_secret.rs

+            let ptr: NonNull<[u8]> = memsec::memfd_secret_sized(capacity * entry_size)
+                .expect("memfd_secret_sized failed");


When would memfd fail? If we run out of memory?

According to the docs (https://man7.org/linux/man-pages/man2/memfd_secret.2.html):

EINVAL flags included unknown bits. EMFILE The per-process limit on the number of open file descriptors has been reached. EMFILE The system-wide limit on the total number of open files has been reached. ENOMEM There was insufficient memory to create a new anonymous file. ENOSYS memfd_secret() is not implemented on this architecture, or has not been enabled on the kernel command-line with secretmem_enable=1.

So when the system is out of memory or the process has reached the file descriptor limit, or if the feature is disabled and not supported (but we check for that the first time)

Should we provide a better error message? This seems like an unrecoverable situation but it also sounds like we need to test what happens if we panic on desktop with napi?

Hinton · 2025-01-13T14:00:02Z

crates/bitwarden-crypto/src/store/backend/implementation/linux_memfd_secret.rs

+            // Initialize the array with Nones using MaybeUninit
+            let uninit_slice: &mut [MaybeUninit<_>] = std::slice::from_raw_parts_mut(
+                ptr.as_ptr() as *mut MaybeUninit<Option<(Key, Key::KeyValue)>>,
+                capacity,
+            );
+            for elem in uninit_slice {
+                elem.write(None);
+            }


This is probably unsafe? https://doc.rust-lang.org/std/slice/fn.from_raw_parts_mut.html

Hinton · 2025-01-13T14:02:17Z

crates/bitwarden-crypto/src/store/backend/implementation/linux_memfd_secret.rs

+        // SAFETY: The pointer is valid and points to a valid slice of the correct size.
+        // This function is &self so it only takes a immutable *const pointer.


Can you elaborate on how we know it's valid?

Hinton · 2025-01-13T14:05:50Z

crates/bitwarden-crypto/src/store/backend/implementation/mod.rs

+        return Box::new(key_store);
+    }
+
+    Box::new(rust::RustBackend::new().expect("RustKeyStore should always be available"))


Suggested change

Box::new(rust::RustBackend::new().expect("RustKeyStore should always be available"))

Box::new(rust::RustBackend::new().expect("RustKeyStore to always be available"))

Hinton

Some more comments, still need to review slice_backend properly.

crates/bitwarden-crypto/src/store/mod.rs

Hinton · 2025-01-20T12:46:20Z

crates/bitwarden-crypto/src/store/mod.rs

+    /// Initiate an encryption/decryption context. This context will have read only access to the
+    /// global keys, and will have its own local key stores with read/write access. This
+    /// context-local store will be cleared up when the context is dropped.
+    ///
+    /// This is an advanced API, use with care. Prefer to instead use
+    /// `encrypt`/`decrypt`/`encrypt_list`/`decrypt_list` methods.
+    ///
+    /// One of the pitfalls of the current implementations is that keys stored in the context-local
+    /// store only get cleared automatically when the context is dropped, and not between
+    /// operations. This means that if you are using the same context for multiple operations,
+    /// you may want to clear it manually between them.
+    pub fn context(&'_ self) -> KeyStoreContext<'_, Refs> {
+        KeyStoreContext {
+            global_keys: GlobalKeys::ReadOnly(self.inner.read().expect("RwLock is poisoned")),
+            local_symmetric_keys: create_store(),
+            local_asymmetric_keys: create_store(),
+            _phantom: std::marker::PhantomData,
+        }
+    }
+
+    /// Initiate an encryption/decryption context. This context will have MUTABLE access to the
+    /// global keys, and will have its own local key stores with read/write access. This
+    /// context-local store will be cleared up when the context is dropped.
+    ///
+    /// This is an advanced API, use with care and ONLY when needing to modify the global keys.
+    ///
+    /// The same pitfalls as `context` apply here, but with the added risk of accidentally
+    /// modifying the global keys and leaving the store in an inconsistent state.
+    ///
+    /// TODO: We should work towards making this pub(crate), and instead providing a safe API for
+    /// modifying the global keys. (i.e. `derive_master_key`, `derive_user_key`, etc.)
+    pub fn context_mut(&'_ self) -> KeyStoreContext<'_, Refs> {
+        KeyStoreContext {
+            global_keys: GlobalKeys::ReadWrite(self.inner.write().expect("RwLock is poisoned")),
+            local_symmetric_keys: create_store(),
+            local_asymmetric_keys: create_store(),
+            _phantom: std::marker::PhantomData,
+        }
+    }


question: Can we provide some examples of when it's alright to use these APIs?

crates/bitwarden-crypto/src/store/mod.rs

Hinton · 2025-01-20T12:51:14Z

crates/bitwarden-crypto/src/store/mod.rs

+
+    // In this case, the minimum chunk size is 50. This was chosen pretty arbitrarily,
+    // but it seems to work well in practice.
+    usize::max(1 + len / rayon::current_num_threads(), 50)


issue: Magic number, extract it as a constant with a good name?

crates/bitwarden-crypto/src/store/mod.rs

Hinton · 2025-01-20T13:27:54Z

crates/bitwarden-crypto/src/store/backend/implementation/rust.rs

+
+            // Note: munlock is zeroing the memory, which leaves the data in an undefined
+            // state, so we set it to None/Default again to avoid UB in the Drop implementation.
+            let uninit_slice: &mut [MaybeUninit<_>] = std::slice::from_raw_parts_mut(


question: Just checking but we fulfill all safety criteria defined in https://doc.rust-lang.org/std/slice/fn.from_raw_parts_mut.html?

Hinton · 2025-01-20T13:32:20Z

crates/bitwarden-crypto/src/store/backend/implementation/rust.rs

+            // Note: munlock is zeroing the memory, which leaves the data in an undefined
+            // state, so we set it to None/Default again to avoid UB in the Drop implementation.
+            let uninit_slice: &mut [MaybeUninit<_>] = std::slice::from_raw_parts_mut(
+                data.as_mut_ptr() as *mut MaybeUninit<T>,
+                data.len(),
+            );
+            for elem in uninit_slice {
+                elem.write(T::default());
+            }


question: Why is munlock zeroizing memory undefined state?

And do we have documentation that we're re-initializing it correctly.

Because the items inside the store can implement Drop themselves, and if they try to access their values after they have been zeroed it would cause UB.

Hinton · 2025-01-20T14:32:41Z

crates/bitwarden-crypto/src/store/backend/slice_backend.rs

+}
+
+impl<Key: KeyRef, Data: SliceLike<Key>> StoreBackend<Key> for SliceBackend<Key, Data> {
+    fn insert(&mut self, key_ref: Key, key: Key::KeyValue) {


issue: This sounds like an upsert. I think we should name it accurately if that is the expected behaviour.

Yeah, I based it on the std HashMap API, where the insert does insert and replace. I'll rename it

Hinton · 2025-01-20T14:35:42Z

crates/bitwarden-crypto/src/store/backend/slice_backend.rs

+/// This represents a key store over an arbitrary fixed size slice.
+/// This is meant to abstract over the different ways to store keys in memory, whether we're
+/// using a Box<[u8]> or a NonNull<u8>, regardless of if that memory was allocated by Rust or not.
+///
+/// Internally this is represented as a slice of `Option<(Key, Key::KeyValue)>`
+/// and elements are sorted based on Key for performance. In essence, this is almost a homegrown
+/// `HashMap`.
+///
+/// The reason why we're not using a Rust collection like `Vec` or `HashMap` is that those types
+/// expect their memory to be allocated by Rust, and they will deallocate/reallocate it as needed.
+/// That will not work for our usecases where we want to have control over allocations/deallocations
+/// and where some of our strategies rely on using system-allocated secure memory for the storage,
+/// like the Linux-only `memfd_secret` API.


question: We seem to rely on this being an ordered list but it's not documented? Can we add this.

The ordering is an implementation detail of the backend, I chose it because it allows us to do binary searches for lookup performance. It shouldn't be exposed outside its own API.

That said it would be good to document it yes.

dani-garcia · 2025-01-20T17:05:22Z

Split the secure storage implementations to #125 so this is easier to review and merge.

Hinton · 2025-01-23T10:41:52Z

crates/bitwarden-crypto/src/store/backend/implementation/basic.rs

+    }
+}
+
+// Key::KeyValue already implements ZeroizeOnDrop,


Suggested change

// Key::KeyValue already implements ZeroizeOnDrop,

// [Key::KeyValue] already implements ZeroizeOnDrop,

Hinton · 2025-01-23T10:42:41Z

crates/bitwarden-crypto/src/store/backend/mod.rs

+
+/// This trait represents a platform that can store and return keys. If possible,
+/// it will try to enable as many security protections on the keys as it can.
+/// The keys themselves implement ZeroizeOnDrop, so the store will only need to make sure


Suggested change

/// The keys themselves implement ZeroizeOnDrop, so the store will only need to make sure

/// The keys themselves implement [ZeroizeOnDrop], so the store will only need to make sure

Hinton · 2025-01-23T10:43:21Z

crates/bitwarden-crypto/src/store/backend/mod.rs

+pub trait StoreBackend<Key: KeyId>: ZeroizeOnDrop + Send + Sync {
+    /// Inserts a key into the store. If the key already exists, it will be replaced.
+    fn upsert(&mut self, key_id: Key, key: Key::KeyValue);
+    /// Retrieves a key from the store.


suggestion: New line between functions?

Hinton · 2025-01-23T10:47:29Z

crates/bitwarden-crypto/src/store/context.rs

+/// This will usually be accessed from an implementation of [crate::Decryptable] or
+/// [crate::Encryptable], but can also be obtained through [super::KeyStore::context]
+///
+/// This context contains access to the user keys stored in the [super::KeyStore] (sometimes refered


Suggested change

/// This context contains access to the user keys stored in the [super::KeyStore] (sometimes refered

/// This context contains access to the user keys stored in the [super::KeyStore] (sometimes referred

Hinton · 2025-01-23T10:50:29Z

crates/bitwarden-crypto/src/store/context.rs

+// A KeyStoreContext is usually limited to a read only access to the global keys,
+// which allows us to have multiple read only contexts at the same time and do multitheaded
+// encryption/decryption. We also have the option to create a read/write context, which allows us to
+// modify the global keys, but only allows one context at a time. This is controlled by a RWLock on


Suggested change

// modify the global keys, but only allows one context at a time. This is controlled by a RWLock on

// modify the global keys, but only allows one context at a time. This is controlled by a [RwLock] on

Hinton · 2025-01-23T11:26:42Z

crates/bitwarden-crypto/src/traits/key_id.rs

+/// Represents a key identifier that can be used to identify cryptographic keys in the
+/// key store. It is used to avoid exposing the key material directly in the public API.
+///
+/// This trait is user-implemented, and our recommended implementation is using enums with variants


Suggested change

/// This trait is user-implemented, and our recommended implementation is using enums with variants

/// This trait is user-implemented, and the recommended implementation is using enums with variants

Hinton · 2025-01-23T11:56:37Z

crates/bitwarden-crypto/src/traits/encryptable.rs

+impl<Ids: KeyIds> Encryptable<Ids, Ids::Symmetric, EncString> for &str {
+    fn encrypt(
+        &self,
+        ctx: &mut KeyStoreContext<Ids>,
+        key: Ids::Symmetric,
+    ) -> Result<EncString, CryptoError> {
+        self.as_bytes().encrypt(ctx, key)
+    }
+}
+
+impl<Ids: KeyIds> Encryptable<Ids, Ids::Asymmetric, AsymmetricEncString> for &str {
+    fn encrypt(
+        &self,
+        ctx: &mut KeyStoreContext<Ids>,
+        key: Ids::Asymmetric,
+    ) -> Result<AsymmetricEncString, CryptoError> {
+        self.as_bytes().encrypt(ctx, key)
+    }
+}


I believe we can generalize this code slightly.

Suggested change

impl<Ids: KeyIds> Encryptable<Ids, Ids::Symmetric, EncString> for &str {

fn encrypt(

&self,

ctx: &mut KeyStoreContext<Ids>,

key: Ids::Symmetric,

) -> Result<EncString, CryptoError> {

self.as_bytes().encrypt(ctx, key)

}

}

impl<Ids: KeyIds> Encryptable<Ids, Ids::Asymmetric, AsymmetricEncString> for &str {

fn encrypt(

&self,

ctx: &mut KeyStoreContext<Ids>,

key: Ids::Asymmetric,

) -> Result<AsymmetricEncString, CryptoError> {

self.as_bytes().encrypt(ctx, key)

}

}

impl<Ids, K, E> Encryptable<Ids, K, E> for &str

where

Ids: KeyIds,

K: KeyId,

E: EncStringType,

[u8]: Encryptable<Ids, K, E>,

{

fn encrypt(&self, ctx: &mut KeyStoreContext<Ids>, key: K) -> Result<E, CryptoError> {

self.as_bytes().encrypt(ctx, key)

}

}

And define a common trait for EncString.

pub trait EncStringType {} impl EncStringType for EncString {} impl EncStringType for AsymmetricEncString {}

Hinton · 2025-01-23T11:57:49Z

crates/bitwarden-crypto/src/store/mod.rs

Can we add some quick module docs? I.e. //!

Hinton · 2025-01-23T11:59:23Z

crates/bitwarden-crypto/src/store/mod.rs

+    inner: Arc<RwLock<KeyStoreInner<Ids>>>,
+}
+
+impl<Ids: KeyIds> std::fmt::Debug for KeyStore<Ids> {


Suggested change

impl<Ids: KeyIds> std::fmt::Debug for KeyStore<Ids> {

// [KeyStore] contains sensitive data, provide a dummy [Debug] implementation.

impl<Ids: KeyIds> std::fmt::Debug for KeyStore<Ids> {

Hinton · 2025-01-23T12:21:22Z

crates/bitwarden-crypto/src/store/context.rs

+    /// * `new_key_id` - The key id where the decrypted key will be stored. If it already exists, it
+    ///   will be overwritten
+    /// * `encrypted_key` - The key to decrypt
+    pub fn decrypt_symmetric_key_with_symmetric_key(


I wonder if the naming here is slightly misleading. We're not decrypting it we're adding it to the context?

dani-garcia added 30 commits August 19, 2024 14:02

Initial CryptoService impl

d7c7c3e

More work

6068e84

Refactor keystore to also handle resizes

50dc1b4

Merge branch 'main' into ps/secure-crypto-service

216eb25

Fix

4b846ed

Paralelization plus alternative to locatekey

7a01168

WASM support

c649bf0

Merge branch 'main' into ps/secure-crypto-service

b901ef7

Remove unnecessary trait

e2129ad

Inline cryptoengine

f708fcc

Impl KeyStore in Slice struct directly

30854f7

Reset the values to None after munlock has zeroized them

bed894a

Otherwise some Drop implementations can crash

Merge branch 'main' into ps/secure-crypto-service

709dfce

Fmt

f7eda88

Add benchmark

ce2343e

Respect no-memory-hardening flag

bcd712f

Fix cfg flags, silence warnings

38343d2

Export memfd correctly

f34ce02

Fix memtest

cc27320

Merge branch 'main' into ps/secure-crypto-service

281619d

Try fat LTO to fix windows

32d298f

Disable memsec on windows to fix it

c43aa08

Incorrect optional dep

dd37d1d

Try updating windows in memsec

45f3d32

Remove unnecessary bound

1f70169

Move store impl around a bit

22a8b17

Make KeyStore pub

c63f656

Merge branch 'main' into ps/secure-crypto-service-impl

db3f8d4

Some renames/simplifications

eb81684

Add some missing implementations

d279626

dani-garcia added 8 commits December 12, 2024 14:41

Add some more comments about why we're doing the custom slice

e7bfdc3

Add context doc

34b256e

Add more tests

fe6b28f

Add more docs

45c9ee2

Merge branch 'main' into ps/secure-crypto-service

ad8ed59

Fix wrong test

75b97fa

Merge branch 'main' into ps/secure-crypto-service

33ec078

Add ignore dep

275574f

Hinton self-requested a review January 13, 2025 11:28

Hinton reviewed Jan 13, 2025

View reviewed changes

Hinton reviewed Jan 20, 2025

View reviewed changes

dani-garcia added 6 commits January 20, 2025 16:06

Merge branch 'main' into ps/secure-crypto-service

1d8911a

Split backend implementations to a separate PR

b969352

MissingKey2 -> MissingKeyRef

8fddf86

Change visibilities

f03ba9e

Upsert + docs

f8c8427

Fmt

fb34e6c

dani-garcia added 9 commits January 21, 2025 16:47

KeyRef->KeyId

7a1dc2d

Reorganize traits and split encryptable

67c32fa

UsesKey -> IdentifyKey

2360396

Merge branch 'main' into ps/secure-crypto-service

4e690e6

Add default impl for KeyStore

4e6b707

Add decryptable/encryptable tests

cc1c27e

Fmt

a2afdf3

Fix test and add docs

5fa459d

Add context usage docs

628e81b

dani-garcia requested a review from Hinton January 22, 2025 14:43

Hinton reviewed Jan 23, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PM-5693] CryptoService using memfd_secret on Linux #7

[PM-5693] CryptoService using memfd_secret on Linux #7

dani-garcia commented Oct 22, 2024

dani-garcia commented Dec 12, 2024 •

edited

Loading

Hinton left a comment

Hinton Jan 13, 2025

Hinton Jan 13, 2025

dani-garcia Jan 13, 2025 •

edited

Loading

Hinton Jan 20, 2025

Hinton Jan 13, 2025

Hinton Jan 13, 2025

Hinton Jan 13, 2025

Hinton left a comment

Hinton Jan 20, 2025

Hinton Jan 20, 2025

Hinton Jan 20, 2025

Hinton Jan 20, 2025

dani-garcia Jan 20, 2025

Hinton Jan 20, 2025

dani-garcia Jan 20, 2025

Hinton Jan 20, 2025

dani-garcia Jan 20, 2025

dani-garcia commented Jan 20, 2025

Hinton Jan 23, 2025

Hinton Jan 23, 2025

Hinton Jan 23, 2025

Hinton Jan 23, 2025

Hinton Jan 23, 2025

Hinton Jan 23, 2025

Hinton Jan 23, 2025

Hinton Jan 23, 2025

Hinton Jan 23, 2025

Hinton Jan 23, 2025

	// accesses the pointer mutably is defined as &mut self, and that the pointer is never copied or
	// accesses the pointer mutably is defined as `&mut self`, and that the pointer is never copied or

		let ptr: NonNull<[u8]> = memsec::memfd_secret_sized(capacity * entry_size)
		.expect("memfd_secret_sized failed");

		// SAFETY: The pointer is valid and points to a valid slice of the correct size.
		// This function is &self so it only takes a immutable *const pointer.

	Box::new(rust::RustBackend::new().expect("RustKeyStore should always be available"))
	Box::new(rust::RustBackend::new().expect("RustKeyStore to always be available"))

	// Key::KeyValue already implements ZeroizeOnDrop,
	// [Key::KeyValue] already implements ZeroizeOnDrop,

	/// The keys themselves implement ZeroizeOnDrop, so the store will only need to make sure
	/// The keys themselves implement [ZeroizeOnDrop], so the store will only need to make sure

	/// This context contains access to the user keys stored in the [super::KeyStore] (sometimes refered
	/// This context contains access to the user keys stored in the [super::KeyStore] (sometimes referred

	// modify the global keys, but only allows one context at a time. This is controlled by a RWLock on
	// modify the global keys, but only allows one context at a time. This is controlled by a [RwLock] on

	/// This trait is user-implemented, and our recommended implementation is using enums with variants
	/// This trait is user-implemented, and the recommended implementation is using enums with variants

	impl<Ids: KeyIds> std::fmt::Debug for KeyStore<Ids> {
	// [KeyStore] contains sensitive data, provide a dummy [Debug] implementation.
	impl<Ids: KeyIds> std::fmt::Debug for KeyStore<Ids> {

[PM-5693] CryptoService using memfd_secret on Linux #7

Are you sure you want to change the base?

[PM-5693] CryptoService using memfd_secret on Linux #7

Conversation

dani-garcia commented Oct 22, 2024

🎟️ Tracking

📔 Objective

Changes

📸 Screenshots

⏰ Reminders before review

🦮 Reviewer guidelines

dani-garcia commented Dec 12, 2024 • edited Loading

Hinton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dani-garcia Jan 13, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hinton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dani-garcia commented Jan 20, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dani-garcia commented Dec 12, 2024 •

edited

Loading

dani-garcia Jan 13, 2025 •

edited

Loading