Skip to content

Commit

Permalink
Swap out base32 decoding with custom implementation for totp (#528)
Browse files Browse the repository at this point in the history
Change base32 decoding for totp to use the same method as our other
clients.
  • Loading branch information
Hinton authored Jan 22, 2024
1 parent 0f07c2a commit 57133cf
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 40 deletions.
7 changes: 0 additions & 7 deletions Cargo.lock

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

1 change: 0 additions & 1 deletion crates/bitwarden/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ chrono = { version = ">=0.4.26, <0.5", features = [
"serde",
"std",
], default-features = false }
data-encoding = ">=2.5.0, <3.0"
# We don't use this directly (it's used by rand), but we need it here to enable WASM support
getrandom = { version = ">=0.2.9, <0.3", features = ["js"] }
hmac = ">=0.12.1, <0.13"
Expand Down
74 changes: 42 additions & 32 deletions crates/bitwarden/src/vault/totp.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use std::{collections::HashMap, str::FromStr};

use chrono::{DateTime, Utc};
use data_encoding::BASE32_NOPAD;
use hmac::{Hmac, Mac};
use reqwest::Url;
use schemars::JsonSchema;
Expand Down Expand Up @@ -129,19 +128,6 @@ impl FromStr for Totp {
/// - OTP Auth URI
/// - Steam URI
fn from_str(key: &str) -> Result<Self> {
fn decode_secret(secret: &str) -> Result<Vec<u8>> {
// Sanitize the secret to only contain allowed characters
let secret = secret
.to_uppercase()
.chars()
.filter(|c| BASE32_CHARS.contains(*c))
.collect::<String>();

BASE32_NOPAD
.decode(secret.as_bytes())
.map_err(|e| e.to_string().into())
}

let params = if key.starts_with("otpauth://") {
let url = Url::parse(key).map_err(|_| "Unable to parse URL")?;
let parts: HashMap<_, _> = url.query_pairs().collect();
Expand All @@ -166,26 +152,26 @@ impl FromStr for Totp {
.and_then(|v| v.parse().ok())
.map(|v: u32| v.max(1))
.unwrap_or(DEFAULT_PERIOD),
secret: decode_secret(
secret: decode_b32(
&parts
.get("secret")
.map(|v| v.to_string())
.ok_or("Missing secret in otpauth URI")?,
)?,
),
}
} else if let Some(secret) = key.strip_prefix("steam://") {
Totp {
algorithm: Algorithm::Steam,
digits: 5,
period: DEFAULT_PERIOD,
secret: decode_secret(secret)?,
secret: decode_b32(secret),
}
} else {
Totp {
algorithm: DEFAULT_ALGORITHM,
digits: DEFAULT_DIGITS,
period: DEFAULT_PERIOD,
secret: decode_secret(key)?,
secret: decode_b32(key),
}
};

Expand Down Expand Up @@ -220,12 +206,42 @@ fn derive_binary(hash: Vec<u8>) -> u32 {
| hash[offset + 3] as u32
}

/// This code is migrated from our javascript implementation and is not technically a correct base32 decoder since we filter out various characters, and use exact chunking.
fn decode_b32(s: &str) -> Vec<u8> {
let s = s.to_uppercase();

let mut bits = String::new();
for c in s.chars() {
if let Some(i) = BASE32_CHARS.find(c) {
bits.push_str(&format!("{:05b}", i));
}
}
let mut bytes = Vec::new();

for chunk in bits.as_bytes().chunks_exact(8) {
let byte_str = std::str::from_utf8(chunk).unwrap();
let byte = u8::from_str_radix(byte_str, 2).unwrap();
bytes.push(byte);
}

bytes
}

#[cfg(test)]
mod tests {
use chrono::Utc;

use super::*;

#[test]
fn test_decode_b32() {
let res = decode_b32("WQIQ25BRKZYCJVYP");
assert_eq!(res, vec![180, 17, 13, 116, 49, 86, 112, 36, 215, 15]);

let res = decode_b32("ABCD123");
assert_eq!(res, vec![0, 68, 61]);
}

#[test]
fn test_generate_totp() {
let cases = vec![
Expand All @@ -234,6 +250,14 @@ mod tests {
("PIUDISEQYA", "829846"), // non padded
("PIUDISEQYA======", "829846"), // padded
("PIUD1IS!EQYA=", "829846"), // sanitized
// Steam
("steam://HXDMVJECJJWSRB3HWIZR4IFUGFTMXBOZ", "7W6CJ"),
("steam://ABCD123", "N26DF"),
// Various weird lengths
("ddfdf", "932653"),
("HJSGFJHDFDJDJKSDFD", "000034"),
("xvdsfasdfasdasdghsgsdfg", "403786"),
("KAKFJWOSFJ12NWL", "093430"),
];

let time = Some(
Expand Down Expand Up @@ -277,18 +301,4 @@ mod tests {
assert_eq!(response.code, "730364".to_string());
assert_eq!(response.period, 60);
}

#[test]
fn test_generate_steam() {
let key = "steam://HXDMVJECJJWSRB3HWIZR4IFUGFTMXBOZ".to_string();
let time = Some(
DateTime::parse_from_rfc3339("2023-01-01T00:00:00.000Z")
.unwrap()
.with_timezone(&Utc),
);
let response = generate_totp(key, time).unwrap();

assert_eq!(response.code, "7W6CJ".to_string());
assert_eq!(response.period, 30);
}
}

0 comments on commit 57133cf

Please sign in to comment.