Skip to content

[PM-20614] Cose Content Type #203

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

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions bitwarden_license/bitwarden-sm/src/projects/create.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use bitwarden_api_api::models::ProjectCreateRequestModel;
use bitwarden_core::{key_management::SymmetricKeyId, Client};
use bitwarden_crypto::Encryptable;
use bitwarden_crypto::{ContentFormat, Encryptable};
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
use uuid::Uuid;
Expand Down Expand Up @@ -34,7 +34,7 @@
.name
.clone()
.trim()
.encrypt(&mut key_store.context(), key)?
.encrypt(&mut key_store.context(), key, ContentFormat::Utf8)?

Check warning on line 37 in bitwarden_license/bitwarden-sm/src/projects/create.rs

View check run for this annotation

Codecov / codecov/patch

bitwarden_license/bitwarden-sm/src/projects/create.rs#L37

Added line #L37 was not covered by tests
.to_string(),
});

Expand Down
4 changes: 2 additions & 2 deletions bitwarden_license/bitwarden-sm/src/projects/update.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use bitwarden_api_api::models::ProjectUpdateRequestModel;
use bitwarden_core::{key_management::SymmetricKeyId, Client};
use bitwarden_crypto::Encryptable;
use bitwarden_crypto::{ContentFormat, Encryptable};
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
use uuid::Uuid;
Expand Down Expand Up @@ -36,7 +36,7 @@
.name
.clone()
.trim()
.encrypt(&mut key_store.context(), key)?
.encrypt(&mut key_store.context(), key, ContentFormat::Utf8)?

Check warning on line 39 in bitwarden_license/bitwarden-sm/src/projects/update.rs

View check run for this annotation

Codecov / codecov/patch

bitwarden_license/bitwarden-sm/src/projects/update.rs#L39

Added line #L39 was not covered by tests
.to_string(),
});

Expand Down
17 changes: 13 additions & 4 deletions bitwarden_license/bitwarden-sm/src/secrets/create.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use bitwarden_api_api::models::SecretCreateRequestModel;
use bitwarden_core::{key_management::SymmetricKeyId, Client};
use bitwarden_crypto::Encryptable;
use bitwarden_crypto::{ContentFormat, Encryptable};
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
use uuid::Uuid;
Expand Down Expand Up @@ -40,13 +40,22 @@
let secret = {
let mut ctx = key_store.context();
Some(SecretCreateRequestModel {
key: input.key.clone().trim().encrypt(&mut ctx, key)?.to_string(),
value: input.value.clone().encrypt(&mut ctx, key)?.to_string(),
key: input
.key
.clone()
.trim()
.encrypt(&mut ctx, key, ContentFormat::Utf8)?
.to_string(),
value: input
.value
.clone()
.encrypt(&mut ctx, key, ContentFormat::Utf8)?
.to_string(),

Check warning on line 53 in bitwarden_license/bitwarden-sm/src/secrets/create.rs

View check run for this annotation

Codecov / codecov/patch

bitwarden_license/bitwarden-sm/src/secrets/create.rs#L43-L53

Added lines #L43 - L53 were not covered by tests
note: input
.note
.clone()
.trim()
.encrypt(&mut ctx, key)?
.encrypt(&mut ctx, key, ContentFormat::Utf8)?

Check warning on line 58 in bitwarden_license/bitwarden-sm/src/secrets/create.rs

View check run for this annotation

Codecov / codecov/patch

bitwarden_license/bitwarden-sm/src/secrets/create.rs#L58

Added line #L58 was not covered by tests
.to_string(),
project_ids: input.project_ids.clone(),
access_policies_requests: None,
Expand Down
17 changes: 13 additions & 4 deletions bitwarden_license/bitwarden-sm/src/secrets/update.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use bitwarden_api_api::models::SecretUpdateRequestModel;
use bitwarden_core::{key_management::SymmetricKeyId, Client};
use bitwarden_crypto::Encryptable;
use bitwarden_crypto::{ContentFormat, Encryptable};
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
use uuid::Uuid;
Expand Down Expand Up @@ -39,13 +39,22 @@
let secret = {
let mut ctx = key_store.context();
Some(SecretUpdateRequestModel {
key: input.key.clone().trim().encrypt(&mut ctx, key)?.to_string(),
value: input.value.clone().encrypt(&mut ctx, key)?.to_string(),
key: input
.key
.clone()
.trim()
.encrypt(&mut ctx, key, ContentFormat::Utf8)?
.to_string(),
value: input
.value
.clone()
.encrypt(&mut ctx, key, ContentFormat::Utf8)?
.to_string(),

Check warning on line 52 in bitwarden_license/bitwarden-sm/src/secrets/update.rs

View check run for this annotation

Codecov / codecov/patch

bitwarden_license/bitwarden-sm/src/secrets/update.rs#L42-L52

Added lines #L42 - L52 were not covered by tests
note: input
.note
.clone()
.trim()
.encrypt(&mut ctx, key)?
.encrypt(&mut ctx, key, ContentFormat::Utf8)?

Check warning on line 57 in bitwarden_license/bitwarden-sm/src/secrets/update.rs

View check run for this annotation

Codecov / codecov/patch

bitwarden_license/bitwarden-sm/src/secrets/update.rs#L57

Added line #L57 was not covered by tests
.to_string(),
project_ids: input.project_ids.clone(),
access_policies_requests: None,
Expand Down
6 changes: 3 additions & 3 deletions crates/bitwarden-core/src/mobile/crypto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ pub enum MobileCryptoError {
Crypto(#[from] bitwarden_crypto::CryptoError),
}

/// State used for initializing the user cryptographic state.
/// thState used for initializing the user cryptographic state.
#[derive(Serialize, Deserialize, Debug)]
#[serde(rename_all = "camelCase", deny_unknown_fields)]
#[cfg_attr(feature = "uniffi", derive(uniffi::Record))]
Expand Down Expand Up @@ -329,7 +329,7 @@ pub(super) fn derive_pin_key(

Ok(DerivePinKeyResponse {
pin_protected_user_key,
encrypted_pin: pin.encrypt_with_key(user_key)?,
encrypted_pin: pin.encrypt_with_key(user_key, &bitwarden_crypto::ContentFormat::Utf8)?,
})
}

Expand Down Expand Up @@ -860,7 +860,7 @@ mod tests {
let invalid_private_key = "bad_key"
.to_string()
.into_bytes()
.encrypt_with_key(&user_key.0)
.encrypt_with_key(&user_key.0, &bitwarden_crypto::ContentFormat::Utf8)
.unwrap();

let request = VerifyAsymmetricKeysRequest {
Expand Down
4 changes: 2 additions & 2 deletions crates/bitwarden-core/src/secrets_manager/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

use std::{fmt::Debug, path::Path};

use bitwarden_crypto::{EncString, KeyDecryptable, KeyEncryptable};
use bitwarden_crypto::{ContentFormat, EncString, KeyDecryptable, KeyEncryptable};
use serde::{Deserialize, Serialize};

use crate::auth::AccessToken;
Expand Down Expand Up @@ -73,7 +73,7 @@
) -> Result<(), StateFileError> {
let serialized_state: String = serde_json::to_string(&state)?;
let encrypted_state: EncString =
serialized_state.encrypt_with_key(&access_token.encryption_key)?;
serialized_state.encrypt_with_key(&access_token.encryption_key, &ContentFormat::Utf8)?;

Check warning on line 76 in crates/bitwarden-core/src/secrets_manager/state.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-core/src/secrets_manager/state.rs#L76

Added line #L76 was not covered by tests
let state_string: String = encrypted_state.to_string();

Ok(std::fs::write(state_file, state_string)?)
Expand Down
4 changes: 2 additions & 2 deletions crates/bitwarden-crypto/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@ secure.
## Example:

```rust
use bitwarden_crypto::{SymmetricCryptoKey, KeyEncryptable, KeyDecryptable, CryptoError};
use bitwarden_crypto::{SymmetricCryptoKey, KeyEncryptable, KeyDecryptable, CryptoError, ContentFormat};

async fn example() -> Result<(), CryptoError> {
let key = SymmetricCryptoKey::make_aes256_cbc_hmac_key();

let data = "Hello, World!".to_owned();
let encrypted = data.clone().encrypt_with_key(&key)?;
let encrypted = data.clone().encrypt_with_key(&key, &ContentFormat::Utf8)?;
let decrypted: String = encrypted.decrypt_with_key(&key)?;

assert_eq!(data, decrypted);
Expand Down
59 changes: 55 additions & 4 deletions crates/bitwarden-crypto/src/cose.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,14 @@
//! unless there is a a clear benefit, such as a clear cryptographic benefit, which MUST
//! be documented publicly.

use coset::{iana, CborSerializable, Label};
use coset::{
iana::{self, CoapContentFormat},
CborSerializable, ContentType, Label,
};
use generic_array::GenericArray;
use serde::{Deserialize, Serialize};
#[cfg(feature = "wasm")]
use tsify_next::Tsify;
use typenum::U32;

use crate::{
Expand All @@ -15,22 +21,59 @@
/// to be able to randomly generate nonces, and to not have to worry about key wearout. Since
/// the draft was never published as an RFC, we use a private-use value for the algorithm.
pub(crate) const XCHACHA20_POLY1305: i64 = -70000;
const XCHACHA20_TEXT_PAD_BLOCK_SIZE: usize = 32;
const CONTENT_TYPE_PADDED_UTF8: &str = "application/utf8-padded";

#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
#[cfg_attr(feature = "wasm", derive(Tsify), tsify(into_wasm_abi, from_wasm_abi))]
pub enum ContentFormat {
Utf8,
Pkcs8,
CoseKey,
OctetStream,
/// Domain object should never be serialized. It is used to indicate when we call an encrypt
/// operation on a complex object that consists of multiple, individually encrypted fields
DomainObject,
}

/// Encrypts a plaintext message using XChaCha20Poly1305 and returns a COSE Encrypt0 message
pub(crate) fn encrypt_xchacha20_poly1305(
plaintext: &[u8],
key: &crate::XChaCha20Poly1305Key,
content_format: &ContentFormat,
) -> Result<Vec<u8>, CryptoError> {
let mut protected_header = coset::HeaderBuilder::new().build();
let protected_header = coset::HeaderBuilder::new();
let protected_header = match content_format {
// UTF-8 directly would leak the plaintext size. This is not acceptable for certain data
// (passwords).
ContentFormat::Utf8 => protected_header.content_type(CONTENT_TYPE_PADDED_UTF8.to_string()),
ContentFormat::Pkcs8 => protected_header.content_format(CoapContentFormat::Pkcs8),
ContentFormat::CoseKey => protected_header.content_format(CoapContentFormat::CoseKey),

Check warning on line 51 in crates/bitwarden-crypto/src/cose.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-crypto/src/cose.rs#L50-L51

Added lines #L50 - L51 were not covered by tests
ContentFormat::OctetStream => {
protected_header.content_format(CoapContentFormat::OctetStream)
}
// This should panic, and should never be implemented to be reachable!
ContentFormat::DomainObject => unreachable!(),

Check warning on line 56 in crates/bitwarden-crypto/src/cose.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-crypto/src/cose.rs#L56

Added line #L56 was not covered by tests
};
let mut protected_header = protected_header.build();
// This should be adjusted to use the builder pattern once implemented in coset.
// The related coset upstream issue is:
// https://github.com/google/coset/issues/105
protected_header.alg = Some(coset::Algorithm::PrivateUse(XCHACHA20_POLY1305));

let encoded_plaintext = if *content_format == ContentFormat::Utf8 {
// Pad the data to a block size in order to hide plaintext length
let mut plaintext = plaintext.to_vec();
crate::keys::utils::pad_bytes(&mut plaintext, XCHACHA20_TEXT_PAD_BLOCK_SIZE);
plaintext
} else {
plaintext.to_vec()
};

let mut nonce = [0u8; xchacha20::NONCE_SIZE];
let cose_encrypt0 = coset::CoseEncrypt0Builder::new()
.protected(protected_header)
.create_ciphertext(plaintext, &[], |data, aad| {
.create_ciphertext(&encoded_plaintext, &[], |data, aad| {
let ciphertext =
crate::xchacha20::encrypt_xchacha20_poly1305(&(*key.enc_key).into(), data, aad);
nonce = ciphertext.nonce();
Expand Down Expand Up @@ -71,6 +114,13 @@
aad,
)
})?;

if let Some(ref content_type) = msg.protected.header.content_type {
if *content_type == ContentType::Text(CONTENT_TYPE_PADDED_UTF8.to_string()) {
// Unpad the data to get the original plaintext
return crate::keys::utils::unpad_bytes(&decrypted_message).map(|bytes| bytes.to_vec());
}
}

Check warning on line 123 in crates/bitwarden-crypto/src/cose.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-crypto/src/cose.rs#L123

Added line #L123 was not covered by tests
Ok(decrypted_message)
}

Expand Down Expand Up @@ -125,7 +175,8 @@
};

let plaintext = b"Hello, world!";
let encrypted = encrypt_xchacha20_poly1305(plaintext, key).unwrap();
let encrypted =
encrypt_xchacha20_poly1305(plaintext, key, &ContentFormat::OctetStream).unwrap();
let decrypted = decrypt_xchacha20_poly1305(&encrypted, key).unwrap();
assert_eq!(decrypted, plaintext);
}
Expand Down
51 changes: 37 additions & 14 deletions crates/bitwarden-crypto/src/enc_string/symmetric.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
use super::{check_length, from_b64, from_b64_vec, split_enc_string};
use crate::{
error::{CryptoError, EncStringParseError, Result, UnsupportedOperation},
Aes256CbcHmacKey, KeyDecryptable, KeyEncryptable, SymmetricCryptoKey, XChaCha20Poly1305Key,
Aes256CbcHmacKey, ContentFormat, KeyDecryptable, KeyEncryptable, SymmetricCryptoKey,
XChaCha20Poly1305Key,
};

#[cfg(feature = "wasm")]
Expand Down Expand Up @@ -258,8 +259,9 @@
pub(crate) fn encrypt_xchacha20_poly1305(
data_dec: &[u8],
key: &XChaCha20Poly1305Key,
content_format: &ContentFormat,
) -> Result<EncString> {
let data = crate::cose::encrypt_xchacha20_poly1305(data_dec, key)?;
let data = crate::cose::encrypt_xchacha20_poly1305(data_dec, key, content_format)?;
Ok(EncString::Cose_Encrypt0_B64 { data })
}

Expand All @@ -274,11 +276,15 @@
}

impl KeyEncryptable<SymmetricCryptoKey, EncString> for &[u8] {
fn encrypt_with_key(self, key: &SymmetricCryptoKey) -> Result<EncString> {
fn encrypt_with_key(
self,
key: &SymmetricCryptoKey,
content_format: &ContentFormat,
) -> Result<EncString> {
match key {
SymmetricCryptoKey::Aes256CbcHmacKey(key) => EncString::encrypt_aes256_hmac(self, key),
SymmetricCryptoKey::XChaCha20Poly1305Key(inner_key) => {
EncString::encrypt_xchacha20_poly1305(self, inner_key)
EncString::encrypt_xchacha20_poly1305(self, inner_key, content_format)
}
SymmetricCryptoKey::Aes256CbcKey(_) => Err(CryptoError::OperationNotSupported(
UnsupportedOperation::EncryptionNotImplementedForKey,
Expand Down Expand Up @@ -311,14 +317,22 @@
}

impl KeyEncryptable<SymmetricCryptoKey, EncString> for String {
fn encrypt_with_key(self, key: &SymmetricCryptoKey) -> Result<EncString> {
self.as_bytes().encrypt_with_key(key)
fn encrypt_with_key(
self,
key: &SymmetricCryptoKey,
content_format: &ContentFormat,
) -> Result<EncString> {
self.as_bytes().encrypt_with_key(key, content_format)
}
}

impl KeyEncryptable<SymmetricCryptoKey, EncString> for &str {
fn encrypt_with_key(self, key: &SymmetricCryptoKey) -> Result<EncString> {
self.as_bytes().encrypt_with_key(key)
fn encrypt_with_key(
self,
key: &SymmetricCryptoKey,
content_format: &ContentFormat,
) -> Result<EncString> {
self.as_bytes().encrypt_with_key(key, content_format)

Check warning on line 335 in crates/bitwarden-crypto/src/enc_string/symmetric.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-crypto/src/enc_string/symmetric.rs#L330-L335

Added lines #L330 - L335 were not covered by tests
}
}

Expand Down Expand Up @@ -347,8 +361,8 @@

use super::EncString;
use crate::{
derive_symmetric_key, CryptoError, KeyDecryptable, KeyEncryptable, SymmetricCryptoKey,
KEY_ID_SIZE,
derive_symmetric_key, ContentFormat, CryptoError, KeyDecryptable, KeyEncryptable,
SymmetricCryptoKey, KEY_ID_SIZE,
};

#[test]
Expand All @@ -361,7 +375,10 @@
});

let test_string = "encrypted_test_string";
let cipher = test_string.to_owned().encrypt_with_key(&key).unwrap();
let cipher = test_string
.to_owned()
.encrypt_with_key(&key, &ContentFormat::Utf8)
.unwrap();
let decrypted_str: String = cipher.decrypt_with_key(&key).unwrap();
assert_eq!(decrypted_str, test_string);
}
Expand All @@ -371,7 +388,10 @@
let key = SymmetricCryptoKey::Aes256CbcHmacKey(derive_symmetric_key("test"));

let test_string = "encrypted_test_string";
let cipher = test_string.to_owned().encrypt_with_key(&key).unwrap();
let cipher = test_string
.to_string()
.encrypt_with_key(&key, &ContentFormat::Utf8)
.unwrap();

let decrypted_str: String = cipher.decrypt_with_key(&key).unwrap();
assert_eq!(decrypted_str, test_string);
Expand All @@ -381,8 +401,11 @@
fn test_enc_string_ref_roundtrip() {
let key = SymmetricCryptoKey::Aes256CbcHmacKey(derive_symmetric_key("test"));

let test_string = "encrypted_test_string";
let cipher = test_string.encrypt_with_key(&key).unwrap();
let test_string: &'static str = "encrypted_test_string";
let cipher = test_string
.to_string()
.encrypt_with_key(&key, &ContentFormat::Utf8)
.unwrap();

let decrypted_str: String = cipher.decrypt_with_key(&key).unwrap();
assert_eq!(decrypted_str, test_string);
Expand Down
3 changes: 3 additions & 0 deletions crates/bitwarden-crypto/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ pub enum CryptoError {

#[error("Invalid nonce length")]
InvalidNonceLength,

#[error("Invalid padding")]
InvalidPadding,
}

#[derive(Debug, Error)]
Expand Down
Loading