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

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

Open
wants to merge 57 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
d7c7c3e
Initial CryptoService impl
dani-garcia Jul 26, 2024
6068e84
More work
dani-garcia Aug 23, 2024
50dc1b4
Refactor keystore to also handle resizes
dani-garcia Aug 23, 2024
216eb25
Merge branch 'main' into ps/secure-crypto-service
dani-garcia Aug 23, 2024
4b846ed
Fix
dani-garcia Aug 23, 2024
7a01168
Paralelization plus alternative to locatekey
dani-garcia Aug 23, 2024
c649bf0
WASM support
dani-garcia Aug 23, 2024
b901ef7
Merge branch 'main' into ps/secure-crypto-service
dani-garcia Sep 20, 2024
e2129ad
Remove unnecessary trait
dani-garcia Sep 23, 2024
f708fcc
Inline cryptoengine
dani-garcia Sep 23, 2024
30854f7
Impl KeyStore in Slice struct directly
dani-garcia Sep 23, 2024
bed894a
Reset the values to None after munlock has zeroized them
dani-garcia Sep 23, 2024
709dfce
Merge branch 'main' into ps/secure-crypto-service
dani-garcia Sep 23, 2024
f7eda88
Fmt
dani-garcia Sep 23, 2024
ce2343e
Add benchmark
dani-garcia Sep 24, 2024
bcd712f
Respect no-memory-hardening flag
dani-garcia Sep 24, 2024
38343d2
Fix cfg flags, silence warnings
dani-garcia Sep 24, 2024
f34ce02
Export memfd correctly
dani-garcia Sep 24, 2024
cc27320
Fix memtest
dani-garcia Sep 24, 2024
281619d
Merge branch 'main' into ps/secure-crypto-service
dani-garcia Sep 24, 2024
32d298f
Try fat LTO to fix windows
dani-garcia Sep 24, 2024
c43aa08
Disable memsec on windows to fix it
dani-garcia Sep 24, 2024
dd37d1d
Incorrect optional dep
dani-garcia Sep 24, 2024
45f3d32
Try updating windows in memsec
dani-garcia Sep 24, 2024
1f70169
Remove unnecessary bound
dani-garcia Sep 25, 2024
22a8b17
Move store impl around a bit
dani-garcia Sep 25, 2024
c63f656
Make KeyStore pub
dani-garcia Sep 25, 2024
db3f8d4
Merge branch 'main' into ps/secure-crypto-service-impl
dani-garcia Oct 2, 2024
eb81684
Some renames/simplifications
dani-garcia Oct 2, 2024
d279626
Add some missing implementations
dani-garcia Oct 2, 2024
d5f1ede
Rename
dani-garcia Oct 3, 2024
8652d79
Introduce mutable context
dani-garcia Oct 3, 2024
fdb0263
Refactor keyref/encryptable locations
dani-garcia Oct 4, 2024
6ea6267
Some additions needed to migrate the code
dani-garcia Oct 7, 2024
32088c7
Merge branch 'main' into ps/secure-crypto-service
dani-garcia Oct 7, 2024
f882fb2
Remove bench
dani-garcia Oct 7, 2024
bec4786
Remove using
dani-garcia Oct 7, 2024
c2ffea6
Fix TODO
dani-garcia Oct 8, 2024
c0d2b63
Comment
dani-garcia Oct 8, 2024
cacf4db
Merge branch 'main' into ps/secure-crypto-service
dani-garcia Oct 8, 2024
f4ca816
Fmt
dani-garcia Oct 8, 2024
748ba75
Merge branch 'main' into ps/secure-crypto-service
dani-garcia Oct 22, 2024
713ec80
Merge branch 'main' into ps/secure-crypto-service
dani-garcia Nov 4, 2024
8f8e378
Merge branch 'main' into ps/secure-crypto-service
dani-garcia Dec 11, 2024
14485e8
Documentation improvements, renames and refactors
dani-garcia Dec 12, 2024
84c0aca
Fmt
dani-garcia Dec 12, 2024
f38398e
Fix lints and tests
dani-garcia Dec 12, 2024
fada9c1
Search/replace was a bit too powerful hah
dani-garcia Dec 12, 2024
f9ed542
Fix doclink
dani-garcia Dec 12, 2024
e7bfdc3
Add some more comments about why we're doing the custom slice
dani-garcia Dec 12, 2024
34b256e
Add context doc
dani-garcia Dec 12, 2024
fe6b28f
Add more tests
dani-garcia Dec 14, 2024
45c9ee2
Add more docs
dani-garcia Dec 16, 2024
ad8ed59
Merge branch 'main' into ps/secure-crypto-service
dani-garcia Dec 16, 2024
75b97fa
Fix wrong test
dani-garcia Dec 16, 2024
33ec078
Merge branch 'main' into ps/secure-crypto-service
dani-garcia Dec 16, 2024
275574f
Add ignore dep
dani-garcia Dec 16, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions Cargo.lock

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

7 changes: 7 additions & 0 deletions crates/bitwarden-crypto/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ wasm-bindgen = { workspace = true, optional = true }
zeroize = { version = ">=1.7.0, <2.0", features = ["derive", "aarch64"] }
zeroizing-alloc = ">=0.1.0, <0.2"

[target.'cfg(all(not(target_arch = "wasm32"), not(windows)))'.dependencies]
memsec = { version = "0.7.0", features = ["alloc_ext"] }

[dev-dependencies]
criterion = "0.5.1"
rand_chacha = "0.3.1"
Expand All @@ -67,3 +70,7 @@ required-features = ["no-memory-hardening"]

[lints]
workspace = true

[package.metadata.cargo-udeps.ignore]
# This is unused when using --all-features, as that disables memory-hardening
normal = ["memsec"]
4 changes: 4 additions & 0 deletions crates/bitwarden-crypto/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ pub enum CryptoError {
MissingKey(Uuid),
#[error("The item was missing a required field: {0}")]
MissingField(&'static str),
#[error("Missing Key for Ref. {0}")]
MissingKey2(String),
Copy link
Member

Choose a reason for hiding this comment

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

MissingKey2 ๐Ÿ˜ญ Will this be removed once we remove all usages of the old interfaces?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, that's removed in the other PR that starts using this code in the rest of the SDK:
https://github.com/bitwarden/sdk-internal/pull/8/files#diff-3a39fb8ed55a85cd232b4ac7681c85bb81462727f6d311b1a4c33be7c5b7f4ed

I didn't want to update it here to avoid needing to touch up code which was going to need to be updated in the other PR anyway

#[error("Crypto store is read-only")]
ReadOnlyKeyStore,

#[error("Insufficient KDF parameters")]
InsufficientKdfParameters,
Expand Down
191 changes: 191 additions & 0 deletions crates/bitwarden-crypto/src/keys/encryptable.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,191 @@
use crate::{store::KeyStoreContext, AsymmetricEncString, CryptoError, EncString, KeyRef, KeyRefs};

/// Types implementing [UsesKey] are capable of knowing which cryptographic key is
/// needed to encrypt/decrypt them.
pub trait UsesKey<Key: KeyRef> {
fn uses_key(&self) -> Key;
}

/// An encryption operation that takes the input value and encrypts it into the output value.
/// Implementations should generally consist of calling [Encryptable::encrypt] for all the fields of
/// the type.
pub trait Encryptable<Refs: KeyRefs, Key: KeyRef, Output> {
fn encrypt(
&self,
ctx: &mut KeyStoreContext<Refs>,
key: Key,
) -> Result<Output, crate::CryptoError>;
}

/// A decryption operation that takes the input value and decrypts it into the output value.
/// Implementations should generally consist of calling [Decryptable::decrypt] for all the fields of
/// the type.
pub trait Decryptable<Refs: KeyRefs, Key: KeyRef, Output> {
fn decrypt(
&self,
ctx: &mut KeyStoreContext<Refs>,
key: Key,
) -> Result<Output, crate::CryptoError>;
}

// Basic Encryptable/Decryptable implementations to and from bytes
Copy link
Member

Choose a reason for hiding this comment

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

Should these be doc comments?

Copy link
Member Author

Choose a reason for hiding this comment

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

I've mostly added these to split up and organize all the Encryptable/Decryptable implementations.

They all look very similar and when I didn't have these comments is was quite annoying to find one in particular. Maybe what that is telling us is that we should move those impls into separate modules or separate files.


impl<Refs: KeyRefs> Decryptable<Refs, Refs::Symmetric, Vec<u8>> for EncString {
fn decrypt(
&self,
ctx: &mut KeyStoreContext<Refs>,
key: Refs::Symmetric,
) -> Result<Vec<u8>, crate::CryptoError> {
ctx.decrypt_data_with_symmetric_key(key, self)
}
}

impl<Refs: KeyRefs> Decryptable<Refs, Refs::Asymmetric, Vec<u8>> for AsymmetricEncString {
fn decrypt(
&self,
ctx: &mut KeyStoreContext<Refs>,
key: Refs::Asymmetric,
) -> Result<Vec<u8>, crate::CryptoError> {
ctx.decrypt_data_with_asymmetric_key(key, self)
}

Check warning on line 50 in crates/bitwarden-crypto/src/keys/encryptable.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-crypto/src/keys/encryptable.rs#L44-L50

Added lines #L44 - L50 were not covered by tests
}

impl<Refs: KeyRefs> Encryptable<Refs, Refs::Symmetric, EncString> for &[u8] {
fn encrypt(
&self,
ctx: &mut KeyStoreContext<Refs>,
key: Refs::Symmetric,
) -> Result<EncString, crate::CryptoError> {
ctx.encrypt_data_with_symmetric_key(key, self)
}
}

impl<Refs: KeyRefs> Encryptable<Refs, Refs::Asymmetric, AsymmetricEncString> for &[u8] {
fn encrypt(
&self,
ctx: &mut KeyStoreContext<Refs>,
key: Refs::Asymmetric,
) -> Result<AsymmetricEncString, crate::CryptoError> {
ctx.encrypt_data_with_asymmetric_key(key, self)
}

Check warning on line 70 in crates/bitwarden-crypto/src/keys/encryptable.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-crypto/src/keys/encryptable.rs#L64-L70

Added lines #L64 - L70 were not covered by tests
}

// Encryptable/Decryptable implementations to and from strings

impl<Refs: KeyRefs> Decryptable<Refs, Refs::Symmetric, String> for EncString {
fn decrypt(
&self,
ctx: &mut KeyStoreContext<Refs>,
key: Refs::Symmetric,
) -> Result<String, crate::CryptoError> {
let bytes: Vec<u8> = self.decrypt(ctx, key)?;
String::from_utf8(bytes).map_err(|_| CryptoError::InvalidUtf8String)
}
}

impl<Refs: KeyRefs> Decryptable<Refs, Refs::Asymmetric, String> for AsymmetricEncString {
fn decrypt(
&self,
ctx: &mut KeyStoreContext<Refs>,
key: Refs::Asymmetric,
) -> Result<String, crate::CryptoError> {
let bytes: Vec<u8> = self.decrypt(ctx, key)?;
String::from_utf8(bytes).map_err(|_| CryptoError::InvalidUtf8String)
}

Check warning on line 94 in crates/bitwarden-crypto/src/keys/encryptable.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-crypto/src/keys/encryptable.rs#L87-L94

Added lines #L87 - L94 were not covered by tests
}

impl<Refs: KeyRefs> Encryptable<Refs, Refs::Symmetric, EncString> for &str {
fn encrypt(
&self,
ctx: &mut KeyStoreContext<Refs>,
key: Refs::Symmetric,
) -> Result<EncString, crate::CryptoError> {
self.as_bytes().encrypt(ctx, key)
}

Check warning on line 104 in crates/bitwarden-crypto/src/keys/encryptable.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-crypto/src/keys/encryptable.rs#L98-L104

Added lines #L98 - L104 were not covered by tests
}

impl<Refs: KeyRefs> Encryptable<Refs, Refs::Asymmetric, AsymmetricEncString> for &str {
fn encrypt(
&self,
ctx: &mut KeyStoreContext<Refs>,
key: Refs::Asymmetric,
) -> Result<AsymmetricEncString, crate::CryptoError> {
self.as_bytes().encrypt(ctx, key)
}

Check warning on line 114 in crates/bitwarden-crypto/src/keys/encryptable.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-crypto/src/keys/encryptable.rs#L108-L114

Added lines #L108 - L114 were not covered by tests
}

impl<Refs: KeyRefs> Encryptable<Refs, Refs::Symmetric, EncString> for String {
fn encrypt(
&self,
ctx: &mut KeyStoreContext<Refs>,
key: Refs::Symmetric,
) -> Result<EncString, crate::CryptoError> {
self.as_bytes().encrypt(ctx, key)
}
}

impl<Refs: KeyRefs> Encryptable<Refs, Refs::Asymmetric, AsymmetricEncString> for String {
fn encrypt(
&self,
ctx: &mut KeyStoreContext<Refs>,
key: Refs::Asymmetric,
) -> Result<AsymmetricEncString, crate::CryptoError> {
self.as_bytes().encrypt(ctx, key)
}

Check warning on line 134 in crates/bitwarden-crypto/src/keys/encryptable.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-crypto/src/keys/encryptable.rs#L128-L134

Added lines #L128 - L134 were not covered by tests
}

// Generic implementations for Optional values

impl<Refs: KeyRefs, Key: KeyRef, T: Encryptable<Refs, Key, Output>, Output>
Encryptable<Refs, Key, Option<Output>> for Option<T>
{
fn encrypt(
&self,
ctx: &mut KeyStoreContext<Refs>,
key: Key,
) -> Result<Option<Output>, crate::CryptoError> {
self.as_ref()
.map(|value| value.encrypt(ctx, key))
.transpose()
}

Check warning on line 150 in crates/bitwarden-crypto/src/keys/encryptable.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-crypto/src/keys/encryptable.rs#L142-L150

Added lines #L142 - L150 were not covered by tests
}

impl<Refs: KeyRefs, Key: KeyRef, T: Decryptable<Refs, Key, Output>, Output>
Decryptable<Refs, Key, Option<Output>> for Option<T>
{
fn decrypt(
&self,
ctx: &mut KeyStoreContext<Refs>,
key: Key,
) -> Result<Option<Output>, crate::CryptoError> {
self.as_ref()
.map(|value| value.decrypt(ctx, key))
.transpose()
}

Check warning on line 164 in crates/bitwarden-crypto/src/keys/encryptable.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-crypto/src/keys/encryptable.rs#L156-L164

Added lines #L156 - L164 were not covered by tests
}

// Generic implementations for Vec values

impl<Refs: KeyRefs, Key: KeyRef, T: Encryptable<Refs, Key, Output>, Output>
Encryptable<Refs, Key, Vec<Output>> for Vec<T>
{
fn encrypt(
&self,
ctx: &mut KeyStoreContext<Refs>,
key: Key,
) -> Result<Vec<Output>, crate::CryptoError> {
self.iter().map(|value| value.encrypt(ctx, key)).collect()
}

Check warning on line 178 in crates/bitwarden-crypto/src/keys/encryptable.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-crypto/src/keys/encryptable.rs#L172-L178

Added lines #L172 - L178 were not covered by tests
}

impl<Refs: KeyRefs, Key: KeyRef, T: Decryptable<Refs, Key, Output>, Output>
Decryptable<Refs, Key, Vec<Output>> for Vec<T>
{
fn decrypt(
&self,
ctx: &mut KeyStoreContext<Refs>,
key: Key,
) -> Result<Vec<Output>, crate::CryptoError> {
self.iter().map(|value| value.decrypt(ctx, key)).collect()
}

Check warning on line 190 in crates/bitwarden-crypto/src/keys/encryptable.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-crypto/src/keys/encryptable.rs#L184-L190

Added lines #L184 - L190 were not covered by tests
}
Loading
Loading