Skip to content
Open
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
2899fc4
Add a direct 1Password client for the importer
detunized Aug 10, 2026
fc74c58
Drop the native model parsing
detunized Aug 10, 2026
313da4d
Merge branch 'main' into onepassword-access
itsadrago Aug 11, 2026
5d8b6e8
Explain why the expect calls cannot fail
detunized Aug 12, 2026
02586c0
Bump aes-gcm and pbkdf2 to their stable releases
detunized Aug 12, 2026
8bd94e4
Exclude test resources from the package and mark test-utils temporary
detunized Aug 12, 2026
57233d2
Mark the test-utils re-export and dead-code allow as temporary
detunized Aug 12, 2026
eef1376
Report the actual length in account key length errors
detunized Aug 12, 2026
40297ce
Extract the API endpoints into constants
detunized Aug 12, 2026
8e8f736
Note the 1P and BW naming differences and the import error policy
detunized Aug 12, 2026
affa85b
Move Region into its own file
detunized Aug 12, 2026
3c0db34
Reduce Region to the three regions 1Password operates
detunized Aug 12, 2026
1e431c7
Make the keychain-mutating functions Keychain methods
detunized Aug 12, 2026
043d5d1
Move the new 1Password importer deps to Tools ownership
detunized Aug 12, 2026
dbbbc10
Name the device Bitwarden and drop the unused descriptor fields
detunized Aug 12, 2026
eeeaa17
Merge branch 'main' into onepassword-access
itsadrago Aug 12, 2026
69c395a
Use crypto-bigint for the SRP modular arithmetic
detunized Aug 14, 2026
5a51ce1
Note the open review items in the module README
detunized Aug 14, 2026
178b7b8
Reject non-ASCII hex from the server before padding it
detunized Aug 14, 2026
bc51972
Restrict the access module to its actual API surface
detunized Aug 14, 2026
e3f32f7
Rename the test resources directory to fixtures
detunized Aug 14, 2026
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
2 changes: 1 addition & 1 deletion .github/renovate.json5
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@
},
{
matchManagers: ["cargo"],
matchPackageNames: ["keepass"],
matchPackageNames: ["aes-gcm", "hkdf", "icu_normalizer", "keepass", "num-bigint", "pbkdf2"],
commitMessagePrefix: "[deps] Tools:",
reviewers: ["team:team-tools-dev"],
},
Expand Down
1 change: 1 addition & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ crates/bitwarden-uniffi/swift/*

# Test fixtures
crates/bitwarden-exporters/resources/*
crates/bitwarden-importers/src/importers/onepassword/access/resources/*

# CI output
clippy_result.sarif
18 changes: 18 additions & 0 deletions Cargo.lock

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

8 changes: 8 additions & 0 deletions Cargo.toml

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Another reminder to begin a review process for any new dependencies

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note we have both aes-gcm and pbkdf2 already approved in the clients repo and so I think we'd need approval for...

hkdf (Pure Rust implementation of theΒ HMAC-based Extract-and-Expand Key Derivation Function (HKDF)Β generic over hash function.)
168 KiB

icu_normalizer (Normalizing text into Unicode Normalization Forms.)
76.3 KiB

num-bigint (Big integer types for Rust,Β BigIntΒ andΒ BigUint.)
109 KiB

Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ keywords = ["bitwarden"]
[workspace.dependencies]

# External crates that are expected to maintain a consistent version across all crates
aes-gcm = { version = "0.11.0", features = ["zeroize"] }
async-trait = ">=0.1.80, <0.2"
bitwarden-api-api = { path = "crates/bitwarden-api-api", version = "=3.0.0" }
bitwarden-api-base = { path = "crates/bitwarden-api-base", version = "=3.0.0" }
Expand Down Expand Up @@ -80,12 +81,19 @@ ciborium = ">=0.2.2, <0.3"
data-encoding = ">=2.0, <3"
ed25519-dalek = { version = "3.0.0-pre.7" }
futures = ">=0.3.31, <0.4"
hkdf = "0.13.0"
hmac = "0.13.0"
http = ">=1.4.0, <2.0"
icu_normalizer = { version = "2.2.0", features = [
"compiled_data",
"utf8_iter",
], default-features = false }
js-sys = { version = ">=0.3.72, <0.4" }
keepass = { version = ">=0.13.7, <0.14", default-features = false }
mockall = { version = ">=0.13.1, <0.16" }
num-bigint = ">=0.4, <0.5"
Comment thread
detunized marked this conversation as resolved.
Outdated
password-rules-parser = ">=1.1.0, <2"
pbkdf2 = { version = "0.13.0", default-features = false }
proc-macro2 = ">=1.0.89, <2"
quote = ">=1.0.37, <2"
rand = ">=0.10.0, <0.11"
Expand Down
22 changes: 22 additions & 0 deletions crates/bitwarden-importers/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ name = "bitwarden-importers"
description = """
Internal crate for the bitwarden crate. Do not use.
"""
exclude = ["**/resources"]

version.workspace = true
authors.workspace = true
Expand All @@ -15,6 +16,10 @@ license = "GPL-3.0-only OR LicenseRef-Bitwarden-SDK"
keywords.workspace = true

[features]
# Re-exports the 1Password access module so the out-of-tree CLI can drive it against a real
# account. Never enable in production builds.
# TODO: Remove once the importer consumes the module directly.
test-utils = []
Comment thread
detunized marked this conversation as resolved.
uniffi = [
"dep:uniffi",
"bitwarden-core/uniffi",
Expand All @@ -31,29 +36,46 @@ wasm = [
]

[dependencies]
aes-gcm = { workspace = true }
async-trait = { workspace = true }
bitwarden-api-api = { workspace = true }
bitwarden-collections = { workspace = true }
bitwarden-core = { workspace = true }
bitwarden-crypto = { workspace = true }
bitwarden-error = { workspace = true }
bitwarden-exporters = { workspace = true }
bitwarden-random = { workspace = true }
bitwarden-vault = { workspace = true }
chrono = { workspace = true, features = ["std"] }
data-encoding = { workspace = true }
hkdf = { workspace = true }
hmac = { workspace = true }
icu_normalizer = { workspace = true }
keepass = { workspace = true }
num-bigint = { workspace = true }
pbkdf2 = { workspace = true }
rand = { workspace = true }
reqwest = { workspace = true }
rsa = { workspace = true }
serde = { workspace = true }
serde_json = { workspace = true }
sha1 = { workspace = true }
sha2 = { workspace = true }
thiserror = { workspace = true }
tsify = { workspace = true, optional = true }
uniffi = { workspace = true, optional = true }
url = { workspace = true }
uuid = { workspace = true }
wasm-bindgen = { workspace = true, optional = true }
wasm-bindgen-futures = { workspace = true, optional = true }
zeroize = { workspace = true }

[dev-dependencies]
bitwarden-api-base = { workspace = true }
bitwarden-core = { workspace = true, features = ["internal", "test-fixtures"] }
keepass = { workspace = true, features = ["save_kdbx4"] }
tokio = { workspace = true, features = ["rt"] }
wiremock = { workspace = true }

[lints]
workspace = true
1 change: 1 addition & 0 deletions crates/bitwarden-importers/src/importers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@
//! [`crate::pipeline::ParsedImport`]; the generic pipeline encrypts and submits it.

pub(crate) mod kdbx;
pub(crate) mod onepassword;
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# 1Password access module
Comment thread
detunized marked this conversation as resolved.

Read access to a 1Password account. Logging in needs the username, master password and Secret Key,
plus a TOTP passcode when the account has 2FA. Once authenticated it downloads and decrypts every
accessible vault into a native 1Password model.

A Rust port of the OnePassword module in Bitwarden's C# `password-manager-access` library.

The 1P and BW name things differently. 1P has vaults that are independent, could be shared

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@detunized To make this easy to maintain for the tools team, would it be possible to get a higher-level spec for this? Right now it is unclear how the crypto works at a high level, and how it interacts. (cc @harr1424 I know we dm'd about this for the other crates / importers).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey @quexten ! Yes I spoke about this with @itsadrago and we agreed having a better understanding of the reverse engineering and crypto would go a long ways to improving maintainability.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@harr1424 @quexten I will write what I can. There's a white paper from 1P with their protocol and vault structure description, but I mainly got that from reversing their minified js code and studying the http logs.

separately, could have different access rights, encrypted with different keys. They will be imported
into Bitwarden collections. 1P doesn't have folders, only tags.

## Notes

- Supports TOTP 2FA only ATM
- No SSO support
- No service account support (they are not so good for export/import)
- One entry point, `Client::download_all_vaults`. No vault selection, no random access
- Added `aes-gcm`, `hkdf`, `pbkdf2`, `num-bigint` and `icu_normalizer` to the workspace, will
increase the wasm size
- Uses RustCrypto directly rather than `bitwarden-crypto`, which keeps HKDF, AES-GCM and RSA-OAEP
private and has no PBKDF2-SHA512
- `icu_normalizer` only NFC-normalizes the password before PBKDF2. Heavy for one call,
`unicode-normalization` would be smaller
- The client fingerprint lives in `identity.rs`: app version, HTTP library and per-platform strings.
Question: do we need per-platform impersonation, or is one fixed identity enough?
- There are many tests converted from the C# repo, they became very noisy in Rust. Do we even need
them? See start_registers_an_unknown_device_then_retries for an example.
- Do we need to import password history?
- Only the credentials and the keys are zeroed. The decrypted vault data is not
Comment on lines +26 to +31

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@itsadrago more open questions to review

Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
//! Account Key (Secret Key): A2/A3 parse, HKDF-SHA256 hash, and XOR combine.

use zeroize::Zeroize;

use super::{error::OnePasswordError, kdf};

/// A parsed 1Password Account Key (also called the Secret Key), split into its format, uuid, and
/// key.
pub struct AccountKey {
pub format: String,
pub uuid: String,
pub key: String,
}

impl Drop for AccountKey {
fn drop(&mut self) {
self.key.zeroize();
}
}

impl AccountKey {
/// Parses a key string such as `A3-RTN9SA-DY9445Y5FF96X6E7B5GPFA95R9`. The string is uppercased
/// and its dashes removed before splitting into `format` (2), `uuid` (6), and `key` (the rest).
pub fn parse(input: &str) -> Result<AccountKey, OnePasswordError> {
let s = input.to_uppercase().replace('-', "");

let Some(format) = s.get(..2) else {
return Err(OnePasswordError::Internal(format!(
"invalid account key: too short, got {}",
s.len()
)));
};

// Only A3 has ever been seen on a real account. A2 comes from reverse-engineered code and
// is untested against anything, so treat its 33-byte length as unverified.
match format {
"A2" if s.len() == 33 => {}
"A3" if s.len() == 34 => {}
"A2" => {
return Err(OnePasswordError::Internal(format!(
"invalid account key: 'A2' needs 33 characters without dashes, got {}",
s.len()
)));
}
"A3" => {
return Err(OnePasswordError::Internal(format!(
"invalid account key: 'A3' needs 34 characters without dashes, got {}",
s.len()
)));
}
_ => {
return Err(OnePasswordError::Internal(format!(
"invalid account key: unknown format '{format}'"
)));
}
}

let invalid = || OnePasswordError::Internal("invalid account key".into());
Ok(AccountKey {
format: format.to_string(),
uuid: s.get(2..8).ok_or_else(invalid)?.to_string(),
key: s.get(8..).ok_or_else(invalid)?.to_string(),
})
}

/// `HKDF-SHA256(ikm = key, salt = uuid, info = format)`, 32 bytes.
pub fn hash(&self) -> [u8; 32] {
kdf::hkdf_sha256(&self.format, self.key.as_bytes(), self.uuid.as_bytes())
}

/// XORs the hash with `bytes`, which must be exactly 32 bytes long.
pub fn combine_with(&self, bytes: &[u8]) -> Result<[u8; 32], OnePasswordError> {
Comment thread
detunized marked this conversation as resolved.
Outdated
let mut h = self.hash();
if h.len() != bytes.len() {
return Err(OnePasswordError::Internal(
"size doesn't match hash function".into(),
));
}

for (byte, other) in h.iter_mut().zip(bytes) {
*byte ^= other;
}

Ok(h)
}
}

#[cfg(test)]
mod tests {
use data_encoding::BASE64URL_NOPAD;

use super::*;

fn key() -> AccountKey {
AccountKey {
format: "A3".into(),
uuid: "RTN9SA".into(),
key: "DY9445Y5FF96X6E7B5GPFA95R9".into(),
Comment thread
mandreko-bitwarden marked this conversation as resolved.
}
}

#[test]
fn parse_returns_parsed_format_a3_key() {
let key = AccountKey::parse("A3-RTN9SA-DY9445Y5FF96X6E7B5GPFA95R9").expect("valid key");
assert_eq!(key.format, "A3");
assert_eq!(key.uuid, "RTN9SA");
assert_eq!(key.key, "DY9445Y5FF96X6E7B5GPFA95R9");
}

// Made up: no real A2 key was ever available to test against.
#[test]
fn parse_returns_parsed_format_a2_key() {
let key = AccountKey::parse("A2-RTN9SA-DY9445Y5FF96X6E7B5GPFA95R").expect("valid key");
assert_eq!(key.format, "A2");
assert_eq!(key.uuid, "RTN9SA");
assert_eq!(key.key, "DY9445Y5FF96X6E7B5GPFA95R");
}

#[test]
fn parse_throws_on_invalid_key_format() {
let cases = [
"",
"A",
"A2",
"A3",
"A2-RTN9SA-DY9445Y5FF96X6E7B5GPFA95",
"A2-RTN9SA-DY9445Y5FF96X6E7B5GPFA95R9",
"A3-RTN9SA-DY9445Y5FF96X6E7B5GPFA95R",
"A3-RTN9SA-DY9445Y5FF96X6E7B5GPFA95R99",
"A3-RTN9SA-DY9445Y-FF96X6E7B-GPFA95R9",
];
for case in cases {
match AccountKey::parse(case) {
Ok(_) => panic!("expected {case:?} to be invalid"),
Err(err) => assert!(
err.to_string().contains("invalid account key"),
"unexpected error for {case:?}: {err}"
),
}
}
}

#[test]
fn hash_returns_hashed_key() {
assert_eq!(
BASE64URL_NOPAD.encode(&key().hash()),
"ZlI2kRote1dv7uflTenyIp5jBE0u-7Fl4aIiE0D9L-g"
);
}

#[test]
fn combine_with_returns_hashed_key() {
let combined = key()
.combine_with(b"All your base are belong to us!!")
.expect("32 byte input");
assert_eq!(
BASE64URL_NOPAD.encode(&combined),
"Jz5asWNCDiVPjIaWKMmTUPtDZihClN8CwdZNMzWODsk"
);
}

#[test]
fn combine_with_throws_on_incorrect_length() {
let cases: [&[u8]; 5] = [
b"",
b"A",
b"All your base are belong to us",
b"All your base are belong to us!",
b"All your base are belong to us!!!",
];
for case in cases {
let err = key().combine_with(case).expect_err("wrong length");
assert!(
err.to_string().contains("hash function"),
"unexpected error: {err}"
);
}
}
}
Loading
Loading