Skip to content

Commit 66e6231

Browse files
quextenquexten
authored
[PM-18772] Rename AesCbc256 to Aes256Cbc (#172)
## 🎟️ Tracking https://bitwarden.atlassian.net/browse/PM-18772 Clients bitwarden/clients#13637 ## 📔 Objective Currently we use: `Aes256Cbc`, but also `AesCbc256` in the SDK. In the web clients we use `AesCbc256`. In general the former order is more common. For instance, TLS defines: `TLS_DHE_RSA_WITH_AES_256_CBC_SHA384`. JWK defines: `AES_128_CBC_HMAC_SHA_256`, SSH defines `aes256-cbc`, COSE defines: `A256CBC`. This is because the block cipher is: AES-128-Bit / AES-256-Bit, and the mode of operation is `CBC`. Semantically, we are currently splitting up the block cipher used and interjecting the mode of operation in the middle. This PR renames the instances used in the SDK of `AesCbc256` to be consistent with other standards, and to be consistent within our codebase. There will be a corresponding PR for the web clients (this is in draft until then). ## ⏰ 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 <!-- Suggested interactions but feel free to use (or not) as you desire! --> - 👍 (`:+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
1 parent 0d5755e commit 66e6231

File tree

6 files changed

+30
-30
lines changed

6 files changed

+30
-30
lines changed

‎crates/bitwarden-core/src/client/test_accounts.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ pub fn test_bitwarden_com_account() -> TestAccount {
139139

140140
/// ### `[email protected]`
141141
///
142-
/// Account which has a user_key of type `AesCbc256_B64` which is deprecated.
142+
/// Account which has a user_key of type `Aes256Cbc_B64` which is deprecated.
143143
///
144144
/// - Email: `[email protected]`
145145
/// - Password: `asdfasdfasdf`

‎crates/bitwarden-crypto/src/aes.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ pub(crate) fn decrypt_aes256_hmac(
6161
///
6262
/// ## Returns
6363
///
64-
/// A AesCbc256_HmacSha256_B64 EncString
64+
/// A Aes256Cbc_HmacSha256_B64 EncString
6565
pub(crate) fn encrypt_aes256_hmac(
6666
data_dec: &[u8],
6767
mac_key: &GenericArray<u8, U32>,

‎crates/bitwarden-crypto/src/enc_string/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,9 @@ fn split_enc_string(s: &str) -> (&str, Vec<&str>) {
5252
// Support legacy format with no header
5353
let parts: Vec<_> = s.split('|').collect();
5454
if parts.len() == 3 {
55-
("1", parts) // AesCbc128_HmacSha256_B64
55+
("1", parts) // Aes128Cbc_HmacSha256_B64
5656
} else {
57-
("0", parts) // AesCbc256_B64
57+
("0", parts) // Aes256Cbc_B64
5858
}
5959
}
6060
}

‎crates/bitwarden-crypto/src/enc_string/symmetric.rs

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ export type EncString = string;
3030
/// variants, but we should be opinionated in which variants are used for encrypting.
3131
///
3232
/// ## Variants
33-
/// - [AesCbc256_B64](EncString::AesCbc256_B64)
34-
/// - [AesCbc256_HmacSha256_B64](EncString::AesCbc256_HmacSha256_B64)
33+
/// - [Aes256Cbc_B64](EncString::Aes256Cbc_B64)
34+
/// - [Aes256Cbc_HmacSha256_B64](EncString::Aes256Cbc_HmacSha256_B64)
3535
///
3636
/// ## Serialization
3737
///
@@ -51,10 +51,10 @@ export type EncString = string;
5151
#[allow(unused, non_camel_case_types)]
5252
pub enum EncString {
5353
/// 0
54-
AesCbc256_B64 { iv: [u8; 16], data: Vec<u8> },
55-
/// 1 was the now removed `AesCbc128_HmacSha256_B64`.
54+
Aes256Cbc_B64 { iv: [u8; 16], data: Vec<u8> },
55+
/// 1 was the now removed `Aes128Cbc_HmacSha256_B64`.
5656
/// 2
57-
AesCbc256_HmacSha256_B64 {
57+
Aes256Cbc_HmacSha256_B64 {
5858
iv: [u8; 16],
5959
mac: [u8; 32],
6060
data: Vec<u8>,
@@ -79,14 +79,14 @@ impl FromStr for EncString {
7979
let iv = from_b64(parts[0])?;
8080
let data = from_b64_vec(parts[1])?;
8181

82-
Ok(EncString::AesCbc256_B64 { iv, data })
82+
Ok(EncString::Aes256Cbc_B64 { iv, data })
8383
}
8484
("2", 3) => {
8585
let iv = from_b64(parts[0])?;
8686
let data = from_b64_vec(parts[1])?;
8787
let mac = from_b64(parts[2])?;
8888

89-
Ok(EncString::AesCbc256_HmacSha256_B64 { iv, mac, data })
89+
Ok(EncString::Aes256Cbc_HmacSha256_B64 { iv, mac, data })
9090
}
9191

9292
(enc_type, parts) => Err(EncStringParseError::InvalidTypeSymm {
@@ -116,15 +116,15 @@ impl EncString {
116116
let iv = buf[1..17].try_into().expect("Valid length");
117117
let data = buf[17..].to_vec();
118118

119-
Ok(EncString::AesCbc256_B64 { iv, data })
119+
Ok(EncString::Aes256Cbc_B64 { iv, data })
120120
}
121121
2 => {
122122
check_length(buf, 50)?;
123123
let iv = buf[1..17].try_into().expect("Valid length");
124124
let mac = buf[17..49].try_into().expect("Valid length");
125125
let data = buf[49..].to_vec();
126126

127-
Ok(EncString::AesCbc256_HmacSha256_B64 { iv, mac, data })
127+
Ok(EncString::Aes256Cbc_HmacSha256_B64 { iv, mac, data })
128128
}
129129
_ => Err(EncStringParseError::InvalidTypeSymm {
130130
enc_type: enc_type.to_string(),
@@ -138,13 +138,13 @@ impl EncString {
138138
let mut buf;
139139

140140
match self {
141-
EncString::AesCbc256_B64 { iv, data } => {
141+
EncString::Aes256Cbc_B64 { iv, data } => {
142142
buf = Vec::with_capacity(1 + 16 + data.len());
143143
buf.push(self.enc_type());
144144
buf.extend_from_slice(iv);
145145
buf.extend_from_slice(data);
146146
}
147-
EncString::AesCbc256_HmacSha256_B64 { iv, mac, data } => {
147+
EncString::Aes256Cbc_HmacSha256_B64 { iv, mac, data } => {
148148
buf = Vec::with_capacity(1 + 16 + 32 + data.len());
149149
buf.push(self.enc_type());
150150
buf.extend_from_slice(iv);
@@ -160,8 +160,8 @@ impl EncString {
160160
impl Display for EncString {
161161
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
162162
let parts: Vec<&[u8]> = match self {
163-
EncString::AesCbc256_B64 { iv, data } => vec![iv, data],
164-
EncString::AesCbc256_HmacSha256_B64 { iv, mac, data } => vec![iv, data, mac],
163+
EncString::Aes256Cbc_B64 { iv, data } => vec![iv, data],
164+
EncString::Aes256Cbc_HmacSha256_B64 { iv, mac, data } => vec![iv, data, mac],
165165
};
166166

167167
let encoded_parts: Vec<String> = parts.iter().map(|part| STANDARD.encode(part)).collect();
@@ -197,14 +197,14 @@ impl EncString {
197197
) -> Result<EncString> {
198198
let (iv, mac, data) =
199199
crate::aes::encrypt_aes256_hmac(data_dec, &key.mac_key, &key.enc_key)?;
200-
Ok(EncString::AesCbc256_HmacSha256_B64 { iv, mac, data })
200+
Ok(EncString::Aes256Cbc_HmacSha256_B64 { iv, mac, data })
201201
}
202202

203203
/// The numerical representation of the encryption type of the [EncString].
204204
const fn enc_type(&self) -> u8 {
205205
match self {
206-
EncString::AesCbc256_B64 { .. } => 0,
207-
EncString::AesCbc256_HmacSha256_B64 { .. } => 2,
206+
EncString::Aes256Cbc_B64 { .. } => 0,
207+
EncString::Aes256Cbc_HmacSha256_B64 { .. } => 2,
208208
}
209209
}
210210
}
@@ -223,11 +223,11 @@ impl KeyEncryptable<SymmetricCryptoKey, EncString> for &[u8] {
223223
impl KeyDecryptable<SymmetricCryptoKey, Vec<u8>> for EncString {
224224
fn decrypt_with_key(&self, key: &SymmetricCryptoKey) -> Result<Vec<u8>> {
225225
match (self, key) {
226-
(EncString::AesCbc256_B64 { iv, data }, SymmetricCryptoKey::Aes256CbcKey(key)) => {
226+
(EncString::Aes256Cbc_B64 { iv, data }, SymmetricCryptoKey::Aes256CbcKey(key)) => {
227227
crate::aes::decrypt_aes256(iv, data.clone(), &key.enc_key)
228228
}
229229
(
230-
EncString::AesCbc256_HmacSha256_B64 { iv, mac, data },
230+
EncString::Aes256Cbc_HmacSha256_B64 { iv, mac, data },
231231
SymmetricCryptoKey::Aes256CbcHmacKey(key),
232232
) => crate::aes::decrypt_aes256_hmac(iv, mac, data.clone(), &key.mac_key, &key.enc_key),
233233
_ => Err(CryptoError::WrongKeyType),
@@ -341,7 +341,7 @@ mod tests {
341341
let enc_string: EncString = enc_str.parse().unwrap();
342342

343343
assert_eq!(enc_string.enc_type(), 0);
344-
if let EncString::AesCbc256_B64 { iv, data } = &enc_string {
344+
if let EncString::Aes256Cbc_B64 { iv, data } = &enc_string {
345345
assert_eq!(
346346
iv,
347347
&[164, 196, 186, 254, 39, 19, 64, 0, 109, 186, 92, 57, 218, 154, 182, 150]
@@ -375,8 +375,8 @@ mod tests {
375375
let key = "hvBMMb1t79YssFZkpetYsM3deyVuQv4r88Uj9gvYe0+G8EwxvW3v1iywVmSl61iwzd17JW5C/ivzxSP2C9h7Tw==".to_string();
376376
let key = SymmetricCryptoKey::try_from(key).unwrap();
377377

378-
// A "downgraded" `EncString` from `EncString::AesCbc256_HmacSha256_B64` (2) to
379-
// `EncString::AesCbc256_B64` (0), with the mac portion removed.
378+
// A "downgraded" `EncString` from `EncString::Aes256Cbc_HmacSha256_B64` (2) to
379+
// `EncString::Aes256Cbc_B64` (0), with the mac portion removed.
380380
// <enc_string>
381381
let enc_str = "0.NQfjHLr6za7VQVAbrpL81w==|wfrjmyJ0bfwkQlySrhw8dA==";
382382
let enc_string: EncString = enc_str.parse().unwrap();

‎crates/bitwarden-crypto/src/keys/master_key.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -123,10 +123,10 @@ pub(super) fn decrypt_user_key(
123123
user_key: EncString,
124124
) -> Result<SymmetricCryptoKey> {
125125
let mut dec: Vec<u8> = match user_key {
126-
// Legacy. user_keys were encrypted using `AesCbc256_B64` a long time ago. We've since
127-
// moved to using `AesCbc256_HmacSha256_B64`. However, we still need to support
126+
// Legacy. user_keys were encrypted using `Aes256Cbc_B64` a long time ago. We've since
127+
// moved to using `Aes256Cbc_HmacSha256_B64`. However, we still need to support
128128
// decrypting these old keys.
129-
EncString::AesCbc256_B64 { .. } => {
129+
EncString::Aes256Cbc_B64 { .. } => {
130130
let legacy_key = SymmetricCryptoKey::Aes256CbcKey(super::Aes256CbcKey {
131131
enc_key: Box::pin(GenericArray::clone_from_slice(key)),
132132
});

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -331,11 +331,11 @@ impl<Ids: KeyIds> KeyStoreContext<'_, Ids> {
331331
let key = self.get_symmetric_key(key)?;
332332

333333
match (data, key) {
334-
(EncString::AesCbc256_B64 { iv, data }, SymmetricCryptoKey::Aes256CbcKey(key)) => {
334+
(EncString::Aes256Cbc_B64 { iv, data }, SymmetricCryptoKey::Aes256CbcKey(key)) => {
335335
crate::aes::decrypt_aes256(iv, data.clone(), &key.enc_key)
336336
}
337337
(
338-
EncString::AesCbc256_HmacSha256_B64 { iv, mac, data },
338+
EncString::Aes256Cbc_HmacSha256_B64 { iv, mac, data },
339339
SymmetricCryptoKey::Aes256CbcHmacKey(key),
340340
) => crate::aes::decrypt_aes256_hmac(iv, mac, data.clone(), &key.mac_key, &key.enc_key),
341341
_ => Err(CryptoError::InvalidKey),

0 commit comments

Comments
 (0)