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-10941] Add ssh import #82

Merged
merged 28 commits into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
0f8a7ac
Add ssh import
quexten Dec 16, 2024
822d0c0
Fix tests
quexten Dec 16, 2024
c8e1801
Run cargo fmt
quexten Dec 16, 2024
53bf274
Move test keys and remove unused dep feature
quexten Dec 16, 2024
8488c40
Merge branch 'main' into km/pm-10941/ssh-import
Hinton Dec 16, 2024
b2b9740
Fix linting and clean up deps
quexten Dec 16, 2024
799bc00
Remove unwrap
quexten Dec 16, 2024
389d776
Fix formatting
quexten Dec 16, 2024
b21255a
Undo change to serde.workspace
quexten Dec 16, 2024
2af2a63
Fix cargo sort
quexten Dec 16, 2024
3eab559
Cleanup according to feedback
quexten Dec 18, 2024
3341c5b
Further cleanup
quexten Dec 18, 2024
1172d03
Cleanup secret document parsing for pkcs8
quexten Dec 18, 2024
90f5cea
Fix build
quexten Dec 18, 2024
edbba77
Clean up label parsing
quexten Dec 18, 2024
2653a8a
Remove unnecessary enum
quexten Dec 18, 2024
03ed7eb
Rename decrypted der to der
quexten Dec 18, 2024
31a401c
Cleanup and address feedback
quexten Dec 18, 2024
8858a2d
Fix tests
quexten Dec 18, 2024
f68b747
Fix tests
quexten Dec 18, 2024
643de0f
Update crates/bitwarden-ssh/src/import.rs
quexten Dec 19, 2024
015cd8e
Update crates/bitwarden-ssh/src/import.rs
quexten Dec 19, 2024
5afaae6
Add comments
quexten Dec 19, 2024
184a009
Fix tests
quexten Dec 19, 2024
8b0352e
Merge branch 'main' into km/pm-10941/ssh-import
quexten Dec 19, 2024
929797a
Add links in comments
quexten Dec 19, 2024
ca98c48
Merge github.com:bitwarden/sdk-internal into km/pm-10941/ssh-import
quexten Dec 19, 2024
8d70e02
Merge branch 'km/pm-10941/ssh-import' of github.com:bitwarden/sdk-intโ€ฆ
quexten Dec 19, 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
42 changes: 42 additions & 0 deletions Cargo.lock

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

7 changes: 6 additions & 1 deletion crates/bitwarden-ssh/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,19 @@ wasm = [

[dependencies]
bitwarden-error = { workspace = true }
ed25519 = { version = "2.2.3", features = ["pkcs8"] }
pkcs8 = { version = "0.10.2", features = ["encryption"] }
rand = "0.8.5"
rsa = "0.9.7"
serde.workspace = true
ssh-key = { version = "0.6.7", features = [
"ed25519",
"encryption",
"rsa",
"getrandom",
Hinton marked this conversation as resolved.
Show resolved Hide resolved
] }
"rand_core",
"std",
Hinton marked this conversation as resolved.
Show resolved Hide resolved
], default-features = false }
thiserror = { workspace = true }
tsify-next = { workspace = true, optional = true }
wasm-bindgen = { workspace = true, optional = true }
Expand Down
13 changes: 13 additions & 0 deletions crates/bitwarden-ssh/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,16 @@
#[error("Failed to convert key: {0}")]
KeyConversionError(ssh_key::Error),
}

#[bitwarden_error(flat)]

Check warning on line 13 in crates/bitwarden-ssh/src/error.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-ssh/src/error.rs#L13

Added line #L13 was not covered by tests
#[derive(Error, Debug, PartialEq)]
pub enum SshKeyImportError {
#[error("Failed to parse key")]
ParsingError,
#[error("Password required")]
PasswordRequired,
#[error("Wrong password")]
WrongPassword,
#[error("Unsupported key type")]
UnsupportedKeyType,
}
Comment on lines +15 to +24
Copy link
Member

Choose a reason for hiding this comment

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

โ“ @Hinton - Is there a path forward for i18n in the SDK? These seem like the kind of errors we'd like to forward to the user, which would mean communicating the i18n key we'd like the UI to use for localization along with the error.

Copy link
Member

@Hinton Hinton Dec 19, 2024

Choose a reason for hiding this comment

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

No we leave that to the presentation layer for now. Using i18n in lower level services have always been sort of a crutch.

For WASM we support https://sdk-api-docs.bitwarden.com/bitwarden_error_macro/attr.bitwarden_error.html which exposes basic, flat and full. For non user facing errors it's safe to use basic or flat and log it. For user facing errors we generally encourage you to use flat or full, and properly handle the error in the presentation layer.

In this case if we want to show error messages to the user we should use flat or full and capture the different variants in the JS world and apply localization there.

Note: The story on mobile is slightly different due to some uniffi limitations. We're working on providing the same capabilities there.

Copy link
Member

@audreyality audreyality Dec 19, 2024

Choose a reason for hiding this comment

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

Thanks! I'm going to leave this item open for now, but it's not blocking the review.

88 changes: 88 additions & 0 deletions crates/bitwarden-ssh/src/generator.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
use serde::{Deserialize, Serialize};
use ssh_key::{rand_core::CryptoRngCore, Algorithm, HashAlg, LineEnding};
#[cfg(feature = "wasm")]
use tsify_next::Tsify;

use crate::{error, error::KeyGenerationError, SshKey};

#[derive(Serialize, Deserialize)]
#[cfg_attr(feature = "wasm", derive(Tsify), tsify(into_wasm_abi, from_wasm_abi))]

Check warning on line 9 in crates/bitwarden-ssh/src/generator.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-ssh/src/generator.rs#L8-L9

Added lines #L8 - L9 were not covered by tests
pub enum KeyAlgorithm {
Ed25519,
Rsa3072,
Rsa4096,
}

pub fn generate_sshkey(key_algorithm: KeyAlgorithm) -> Result<SshKey, error::KeyGenerationError> {
let rng = rand::thread_rng();
generate_sshkey_internal(key_algorithm, rng)
}

Check warning on line 19 in crates/bitwarden-ssh/src/generator.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-ssh/src/generator.rs#L16-L19

Added lines #L16 - L19 were not covered by tests

fn generate_sshkey_internal(
key_algorithm: KeyAlgorithm,
mut rng: impl CryptoRngCore,
) -> Result<SshKey, error::KeyGenerationError> {
let key = match key_algorithm {
KeyAlgorithm::Ed25519 => ssh_key::PrivateKey::random(&mut rng, Algorithm::Ed25519),
KeyAlgorithm::Rsa3072 | KeyAlgorithm::Rsa4096 => {
let bits = match key_algorithm {
KeyAlgorithm::Rsa3072 => 3072,
KeyAlgorithm::Rsa4096 => 4096,
_ => unreachable!(),

Check warning on line 31 in crates/bitwarden-ssh/src/generator.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-ssh/src/generator.rs#L31

Added line #L31 was not covered by tests
audreyality marked this conversation as resolved.
Show resolved Hide resolved
};

let rsa_keypair = ssh_key::private::RsaKeypair::random(&mut rng, bits)
.map_err(KeyGenerationError::KeyGenerationError)?;

let private_key =
ssh_key::PrivateKey::new(ssh_key::private::KeypairData::from(rsa_keypair), "")
.map_err(KeyGenerationError::KeyGenerationError)?;
Ok(private_key)
audreyality marked this conversation as resolved.
Show resolved Hide resolved
}
}
.map_err(KeyGenerationError::KeyGenerationError)?;

let private_key_openssh = key
.to_openssh(LineEnding::LF)
.map_err(KeyGenerationError::KeyConversionError)?;
Ok(SshKey {
private_key: private_key_openssh.to_string(),
public_key: key.public_key().to_string(),
key_fingerprint: key.fingerprint(HashAlg::Sha256).to_string(),
})
}

#[cfg(test)]
mod tests {
use rand::SeedableRng;

use super::KeyAlgorithm;
use crate::generator::generate_sshkey_internal;

#[test]
fn generate_ssh_key_ed25519() {
let rng = rand_chacha::ChaCha12Rng::from_seed([0u8; 32]);
let key_algorithm = KeyAlgorithm::Ed25519;
let result = generate_sshkey_internal(key_algorithm, rng);
let target = include_str!("../tests/ed25519_key").replace("\r\n", "\n");
assert_eq!(result.unwrap().private_key, target);
}

#[test]
fn generate_ssh_key_rsa3072() {
let rng = rand_chacha::ChaCha12Rng::from_seed([0u8; 32]);
let key_algorithm = KeyAlgorithm::Rsa3072;
let result = generate_sshkey_internal(key_algorithm, rng);
let target = include_str!("../tests/rsa3072_key").replace("\r\n", "\n");
assert_eq!(result.unwrap().private_key, target);
}

#[test]
fn generate_ssh_key_rsa4096() {
let rng = rand_chacha::ChaCha12Rng::from_seed([0u8; 32]);
let key_algorithm = KeyAlgorithm::Rsa4096;
let result = generate_sshkey_internal(key_algorithm, rng);
let target = include_str!("../tests/rsa4096_key").replace("\r\n", "\n");
assert_eq!(result.unwrap().private_key, target);
}
}
Loading
Loading