diff --git a/Cargo.lock b/Cargo.lock index d72665936..71c551551 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -627,9 +627,9 @@ dependencies = [ [[package]] name = "base16ct" -version = "0.1.1" +version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "349a06037c7bf932dd7e7d1f653678b2038b9ad46a74102f1fc7bd7872678cce" +checksum = "4c7f02d4ea65f2c1853089ffd8d2787bdbc63de2f0d29dedbcf8ccdfa0ccd4cf" [[package]] name = "base64" @@ -935,9 +935,9 @@ checksum = "7a81dae078cea95a014a339291cec439d2f232ebe854a9d672b796c6afafa9b7" [[package]] name = "crypto-bigint" -version = "0.4.9" +version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ef2b4b23cddf68b89b8f8069890e8c270d54e2d5fe1b143820234805e4cb17ef" +checksum = "cf4c2f4e1afd912bc40bfd6fed5d9dc1f288e0ba01bfcc835cc5bc3eb13efe15" dependencies = [ "generic-array", "rand_core 0.6.4", @@ -1076,17 +1076,6 @@ version = "2.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2e66c9d817f1720209181c316d28635c050fa304f9c79e47a520882661b7308" -[[package]] -name = "der" -version = "0.6.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f1a467a65c5e759bce6e65eaf91cc29f466cdc57cb65777bd646872a8a1fd4de" -dependencies = [ - "const-oid", - "pem-rfc7468 0.6.0", - "zeroize", -] - [[package]] name = "der" version = "0.7.7" @@ -1094,7 +1083,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0c7ed52955ce76b1554f509074bb357d3fb8ac9b51288a65a3fd480d1dfba946" dependencies = [ "const-oid", - "pem-rfc7468 0.7.0", + "pem-rfc7468", "zeroize", ] @@ -1292,14 +1281,16 @@ dependencies = [ [[package]] name = "ecdsa" -version = "0.14.8" +version = "0.16.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "413301934810f597c1d19ca71c8710e99a3f1ba28a0d2ebc01551a2daeea3c5c" +checksum = "a4b1e0c257a9e9f25f90ff76d7a68360ed497ee519c8e428d1825ef0000799d4" dependencies = [ - "der 0.6.1", + "der", + "digest 0.10.7", "elliptic-curve", "rfc6979", - "signature 1.6.4", + "signature 2.1.0", + "spki", ] [[package]] @@ -1346,17 +1337,17 @@ checksum = "a26ae43d7bcc3b814de94796a5e736d4029efb0ee900c12e2d54c993ad1a1e07" [[package]] name = "elliptic-curve" -version = "0.12.3" +version = "0.13.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e7bb888ab5300a19b8e5bceef25ac745ad065f3c9f7efc6de1b91958110891d3" +checksum = "968405c8fdc9b3bf4df0a6638858cc0b52462836ab6b1c87377785dd09cf1c0b" dependencies = [ "base16ct", "crypto-bigint", - "der 0.6.1", "digest 0.10.7", "ff", "generic-array", "group", + "pkcs8", "rand_core 0.6.4", "sec1", "subtle", @@ -1485,9 +1476,9 @@ checksum = "6999dc1837253364c2ebb0704ba97994bd874e8f195d665c50b7548f6ea92764" [[package]] name = "ff" -version = "0.12.1" +version = "0.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d013fc25338cc558c5c2cfbad646908fb23591e2404481826742b651c9af7160" +checksum = "ded41244b729663b1e574f1b4fb731469f69f79c17667b5d776b16cda0479449" dependencies = [ "rand_core 0.6.4", "subtle", @@ -1742,6 +1733,7 @@ checksum = "85649ca51fd72272d7821adaf274ad91c288277713d9c18820d8499a7ff69e9a" dependencies = [ "typenum", "version_check", + "zeroize", ] [[package]] @@ -1788,9 +1780,9 @@ dependencies = [ [[package]] name = "group" -version = "0.12.1" +version = "0.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5dfbfb3a6cfbd390d5c9564ab283a0349b9b9fcd46a706c1eb10e0db70bfbac7" +checksum = "f0f9ef7462f7c099f518d754361858f86d8a07af53ba9af0fe635bbccb151a63" dependencies = [ "ff", "rand_core 0.6.4", @@ -2639,23 +2631,25 @@ checksum = "b15813163c1d831bf4a13c3610c05c0d03b39feb07f7e09fa234dac9b15aaf39" [[package]] name = "p256" -version = "0.11.1" +version = "0.13.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "51f44edd08f51e2ade572f141051021c5af22677e42b7dd28a88155151c33594" +checksum = "c9863ad85fa8f4460f9c48cb909d38a0d689dba1f6f6988a5e3e0d31071bcd4b" dependencies = [ "ecdsa", "elliptic-curve", + "primeorder", "sha2 0.10.7", ] [[package]] name = "p384" -version = "0.11.2" +version = "0.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dfc8c5bf642dde52bb9e87c0ecd8ca5a76faac2eeed98dedb7c717997e1080aa" +checksum = "70786f51bcc69f6a4c0360e063a4cac5419ef7c5cd5b3c99ad70f3be5ba79209" dependencies = [ "ecdsa", "elliptic-curve", + "primeorder", "sha2 0.10.7", ] @@ -2711,15 +2705,6 @@ dependencies = [ "regex", ] -[[package]] -name = "pem-rfc7468" -version = "0.6.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "24d159833a9105500e0398934e205e0773f0b27529557134ecfc51c27646adac" -dependencies = [ - "base64ct", -] - [[package]] name = "pem-rfc7468" version = "0.7.0" @@ -2815,37 +2800,15 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184" -[[package]] -name = "pkcs1" -version = "0.4.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eff33bdbdfc54cc98a2eca766ebdec3e1b8fb7387523d5c9c9a2891da856f719" -dependencies = [ - "der 0.6.1", - "pkcs8 0.9.0", - "spki 0.6.0", - "zeroize", -] - [[package]] name = "pkcs1" version = "0.7.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c8ffb9f10fa047879315e6625af03c164b16962a5368d724ed16323b68ace47f" dependencies = [ - "der 0.7.7", - "pkcs8 0.10.2", - "spki 0.7.2", -] - -[[package]] -name = "pkcs8" -version = "0.9.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9eca2c590a5f85da82668fa685c09ce2888b9430e83299debf1f34b65fd4a4ba" -dependencies = [ - "der 0.6.1", - "spki 0.6.0", + "der", + "pkcs8", + "spki", ] [[package]] @@ -2854,8 +2817,8 @@ version = "0.10.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f950b2377845cebe5cf8b5165cb3cc1a5e0fa5cfa3e1f7f55707d8fd82e0a7b7" dependencies = [ - "der 0.7.7", - "spki 0.7.2", + "der", + "spki", ] [[package]] @@ -2901,6 +2864,15 @@ version = "0.2.17" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5b40af805b3121feab8a3c29f04d8ad262fa8e0561883e7653e024ae4479e6de" +[[package]] +name = "primeorder" +version = "0.13.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3c2fcef82c0ec6eefcc179b978446c399b3cdf73c392c35604e399eee6df1ee3" +dependencies = [ + "elliptic-curve", +] + [[package]] name = "proc-macro-crate" version = "1.3.1" @@ -3154,13 +3126,12 @@ dependencies = [ [[package]] name = "rfc6979" -version = "0.3.1" +version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7743f17af12fa0b03b803ba12cd6a8d9483a587e89c69445e3909655c0b9fabb" +checksum = "f8dd2a808d456c4a54e300a23e9f5a67e122c3024119acbfd73e3bf664491cb2" dependencies = [ - "crypto-bigint", "hmac", - "zeroize", + "subtle", ] [[package]] @@ -3209,27 +3180,6 @@ dependencies = [ "serde", ] -[[package]] -name = "rsa" -version = "0.7.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "094052d5470cbcef561cb848a7209968c9f12dfa6d668f4bca048ac5de51099c" -dependencies = [ - "byteorder", - "digest 0.10.7", - "num-bigint-dig", - "num-integer", - "num-iter", - "num-traits", - "pkcs1 0.4.1", - "pkcs8 0.9.0", - "rand_core 0.6.4", - "signature 1.6.4", - "smallvec", - "subtle", - "zeroize", -] - [[package]] name = "rsa" version = "0.9.2" @@ -3243,11 +3193,12 @@ dependencies = [ "num-integer", "num-iter", "num-traits", - "pkcs1 0.7.5", - "pkcs8 0.10.2", + "pkcs1", + "pkcs8", "rand_core 0.6.4", + "sha2 0.10.7", "signature 2.1.0", - "spki 0.7.2", + "spki", "subtle", "zeroize", ] @@ -3414,14 +3365,14 @@ dependencies = [ [[package]] name = "sec1" -version = "0.3.0" +version = "0.7.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3be24c1842290c45df0a7bf069e0c268a747ad05a192f2fd7dcfdbc1cba40928" +checksum = "d3e97a565f76233a6003f9f5c54be1d9c5bdfa3eccfb189469f11ec4901c47dc" dependencies = [ "base16ct", - "der 0.6.1", + "der", "generic-array", - "pkcs8 0.9.0", + "pkcs8", "subtle", "zeroize", ] @@ -3700,10 +3651,6 @@ name = "signature" version = "1.6.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "74233d3b3b2f6d4b006dc19dee745e73e2a6bfb6f93607cd3b02bd5b00797d7c" -dependencies = [ - "digest 0.10.7", - "rand_core 0.6.4", -] [[package]] name = "signature" @@ -3786,16 +3733,6 @@ version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6e63cff320ae2c57904679ba7cb63280a3dc4613885beafb148ee7bf9aa9042d" -[[package]] -name = "spki" -version = "0.6.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "67cf02bbac7a337dc36e4f5a693db6c21e7863f45070f7064577eb4367a3212b" -dependencies = [ - "base64ct", - "der 0.6.1", -] - [[package]] name = "spki" version = "0.7.2" @@ -3803,35 +3740,46 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9d1e996ef02c474957d681f1b05213dfb0abab947b446a62d37770b23500184a" dependencies = [ "base64ct", - "der 0.7.7", + "der", +] + +[[package]] +name = "ssh-cipher" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "caac132742f0d33c3af65bfcde7f6aa8f62f0e991d80db99149eb9d44708784f" +dependencies = [ + "cipher", + "ssh-encoding", ] [[package]] name = "ssh-encoding" -version = "0.1.0" +version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "19cfdc32e0199062113edf41f344fbf784b8205a94600233c84eb838f45191e1" +checksum = "eb9242b9ef4108a78e8cd1a2c98e193ef372437f8c22be363075233321dd4a15" dependencies = [ "base64ct", - "pem-rfc7468 0.6.0", + "pem-rfc7468", "sha2 0.10.7", ] [[package]] name = "ssh-key" -version = "0.5.1" +version = "0.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "288d8f5562af5a3be4bda308dd374b2c807b940ac370b5efa1c99311da91d9a1" +checksum = "4b0a17fec6ea344bfa1cda3aed2f0696fddc6295cfcc8c454a3bf58b8ffaabeb" dependencies = [ - "ed25519-dalek", "p256", "p384", "rand_core 0.6.4", - "rsa 0.7.2", + "rsa", "sec1", "sha2 0.10.7", - "signature 1.6.4", + "signature 2.1.0", + "ssh-cipher", "ssh-encoding", + "subtle", "zeroize", ] @@ -4707,6 +4655,7 @@ dependencies = [ "fs-mistrust", "itertools", "rand 0.8.5", + "sec1", "serde", "ssh-key", "tempfile", @@ -4765,7 +4714,7 @@ dependencies = [ "rand 0.8.5", "rand_core 0.5.1", "rand_core 0.6.4", - "rsa 0.9.2", + "rsa", "safelog", "serde", "serde_test", diff --git a/crates/tor-keymgr/Cargo.toml b/crates/tor-keymgr/Cargo.toml index 9404fa64b..ba70168c2 100644 --- a/crates/tor-keymgr/Cargo.toml +++ b/crates/tor-keymgr/Cargo.toml @@ -31,8 +31,18 @@ dyn-clone = "1.0.11" fs-mistrust = { path = "../fs-mistrust", version = "0.7.3", features = ["serde", "walkdir"] } itertools = "0.11.0" rand = "0.8" +# TODO HSS: this is not actually a direct dependency of tor-keymgr. It is added +# here to satisfy the minimal-versions check, which would otherwise fail, as +# ssh-key has a hard dependency on 0.7.3. + +# Adding `-p sec1:0.7.3` to the update command from in +# maint/downgrade_dependencies doesn't work because cargo isn't won't be able +# select sec1@0.7.3 after downgrading the dependencies. +# +# See https://github.com/RustCrypto/SSH/issues/153 +sec1 = "0.7.3" serde = { version = "1.0.103", features = ["derive"] } -ssh-key = { version = "0.5.1", features = ["std"] } +ssh-key = { version = "0.6.0", features = ["std"] } thiserror = "1" tor-config = { path = "../tor-config", version = "0.9.3" } tor-error = { path = "../tor-error", version = "0.5.3" } diff --git a/crates/tor-keymgr/semver.md b/crates/tor-keymgr/semver.md index 049fd9ef3..85bd805af 100644 --- a/crates/tor-keymgr/semver.md +++ b/crates/tor-keymgr/semver.md @@ -1,3 +1,5 @@ ADDED: `EncodableKey::as_ssh_keypair_data` ADDED: `SshKeypairData` (re-exported from `ssh-key`) REMOVED: `KeyType::to_ssh_format` +BREAKING: re-export `ssh_key` rather than just `ssh_key::private::KeypairData` +BREAKING: ssh-key is bumped to 0.6.0 (we re-export `ssh_key`) diff --git a/crates/tor-keymgr/src/key_type/ssh.rs b/crates/tor-keymgr/src/key_type/ssh.rs index ddff9c211..4e035d69b 100644 --- a/crates/tor-keymgr/src/key_type/ssh.rs +++ b/crates/tor-keymgr/src/key_type/ssh.rs @@ -8,13 +8,66 @@ use ssh_key::Algorithm; use crate::{ErasedKey, KeyType, KeystoreError, Result}; -use tor_error::{ErrorKind, HasKind}; -use tor_llcrypto::pk::{ed25519, keymanip}; +use tor_error::{internal, ErrorKind, HasKind}; +use tor_llcrypto::pk::{curve25519, ed25519}; use zeroize::Zeroizing; use std::path::PathBuf; use std::sync::Arc; +/// The algorithm string for x25519 SSH keys. +/// +// TODO HSS: start a protocol name registry in the torspec repo and document the usage and purpose +// of this "protocol" name: +// +// ### Assigned Additional Algorithm Names +// +// #### Registration Procedure(s) +// +// TODO +// +// #### NOTE +// +// The algorithm names MUST meet the criteria for additional algorithm names described in [RFC4251 +// § 6]. +// +// We reserve the following custom OpenSSH key types: +// +// +---------------------------+--------------------+---------------------+------------------------+ +// | Public Key Algorithm Name | Public Key Format | Private Key Format | Purpose | +// |---------------------------|--------------------|---------------------|------------------------| +// | x25519@torproject.org | [TODO link to spec | [TODO link to spec | Arti keystore storage | +// | | describing the key | describing the key | format | +// | | format] | format] | | +// | | | | | +// +---------------------------+--------------------+---------------------+------------------------+ +// +// [RFC4251 § 6]: https://www.rfc-editor.org/rfc/rfc4251.html#section-6 +// +// +// +// # x25519@torproject.org OpenSSH Keys +// +// ## Introduction +// +// X25519 keys do not have a predefined SSH key algorithm name in [IANA's Secure Shell(SSH) +// Protocol Parameters], so in order to be able to store this type of key in OpenSSH format, +// we need to define a custom OpenSSH key type. +// +// ## Key Format +// +// An x25519@torproject.org public key file is encoded in the format specified in +// [RFC4716 § 3.4]. +// +// Private keys use the format specified in [PROTOCOL.key]. +// +// TODO: flesh out the RFC and write down a concrete example for clarity. +// +// [IANA's Secure Shell(SSH) Protocol Parameters]: https://www.iana.org/assignments/ssh-parameters/ssh-parameters.xhtml#ssh-parameters-19 +// [RFC4716 § 3.4]: https://datatracker.ietf.org/doc/html/rfc4716#section-3.4 +// [PROTOCOL.key]: https://cvsweb.openbsd.org/src/usr.bin/ssh/PROTOCOL.key?annotate=HEAD +pub(crate) const X25519_ALGORITHM_NAME: &str = "x25519@torproject.org"; + /// An unparsed OpenSSH key. /// /// Note: This is a wrapper around the contents of a file we think is an OpenSSH key. The inner @@ -43,7 +96,7 @@ impl UnparsedOpenSshKey { /// SSH key algorithms. // // Note: this contains all the types supported by ssh_key, plus X25519. -#[derive(Copy, Clone, Debug, PartialEq, derive_more::Display)] +#[derive(Clone, Debug, PartialEq, derive_more::Display)] pub(crate) enum SshKeyAlgorithm { /// Digital Signature Algorithm Dsa, @@ -72,6 +125,9 @@ impl From for SshKeyAlgorithm { Algorithm::Rsa { .. } => SshKeyAlgorithm::Rsa, Algorithm::SkEcdsaSha2NistP256 => SshKeyAlgorithm::SkEcdsaSha2NistP256, Algorithm::SkEd25519 => SshKeyAlgorithm::SkEd25519, + Algorithm::Other(name) if name.as_str() == X25519_ALGORITHM_NAME => { + SshKeyAlgorithm::X25519 + } // Note: ssh_key::Algorithm is non_exhaustive, so we need this catch-all variant _ => SshKeyAlgorithm::Unknown(algo), } @@ -113,39 +169,6 @@ impl HasKind for SshKeyError { } } -/// A helper for reading Ed25519 OpenSSH private keys from disk. -fn read_ed25519_keypair(key_type: KeyType, key: UnparsedOpenSshKey) -> Result { - let sk = - ssh_key::PrivateKey::from_openssh(&*key.inner).map_err(|e| SshKeyError::SshKeyParse { - // TODO: rust thinks this clone is necessary because key.path is also used below (but - // if we get to this point, we're going to return an error and never reach the other - // error handling branches where we use key.path). - path: key.path.clone(), - key_type, - err: e.into(), - })?; - - // Build the expected key type (i.e. convert ssh_key key types to the key types - // we're using internally). - let key = match sk.key_data() { - KeypairData::Ed25519(key) => { - ed25519::Keypair::from_bytes(&key.to_bytes()).map_err(|_| { - tor_error::internal!("failed to build ed25519 key out of ed25519 OpenSSH key") - })? - } - _ => { - return Err(SshKeyError::UnexpectedSshKeyType { - path: key.path, - wanted_key_algo: key_type.ssh_algorithm(), - found_key_algo: sk.algorithm().into(), - } - .boxed()); - } - }; - - Ok(key) -} - impl KeyType { /// Get the algorithm of this key type. pub(crate) fn ssh_algorithm(&self) -> SshKeyAlgorithm { @@ -161,20 +184,53 @@ impl KeyType { /// The caller is expected to downcast the value returned to a concrete type. pub(crate) fn parse_ssh_format_erased(&self, key: UnparsedOpenSshKey) -> Result { // TODO HSS: perhaps this needs to be a method on EncodableKey instead? - match self { - KeyType::Ed25519Keypair => { - read_ed25519_keypair(*self, key).map(|key| Box::new(key) as ErasedKey) - } - KeyType::X25519StaticSecret => { - // NOTE: The ssh-key crate doesn't support storing curve25519 keys in OpenSSH - // format. As a workaround, this type of key will be stored as ssh-ed25519 and - // converted back to x25519 upon retrieval. - let key = read_ed25519_keypair(KeyType::Ed25519Keypair, key)?; - Ok(Box::new(keymanip::convert_ed25519_to_curve25519_private( - &key, - ))) + let key_type = *self; + let sk = ssh_key::PrivateKey::from_openssh(&*key.inner).map_err(|e| { + SshKeyError::SshKeyParse { + // TODO: rust thinks this clone is necessary because key.path is also used below (but + // if we get to this point, we're going to return an error and never reach the other + // error handling branches where we use key.path). + path: key.path.clone(), + key_type, + err: e.into(), } + })?; + + let wanted_key_algo = key_type.ssh_algorithm(); + + if SshKeyAlgorithm::from(sk.algorithm()) != wanted_key_algo { + return Err(SshKeyError::UnexpectedSshKeyType { + path: key.path, + wanted_key_algo, + found_key_algo: sk.algorithm().into(), + } + .boxed()); + } + + // Build the expected key type (i.e. convert ssh_key key types to the key types + // we're using internally). + match sk.key_data() { + KeypairData::Ed25519(key) => Ok(ed25519::Keypair::from_bytes(&key.to_bytes()) + .map_err(|_| internal!("failed to build ed25519 key out of ed25519 OpenSSH key")) + .map(Box::new)?), + KeypairData::Other(key) + if SshKeyAlgorithm::from(key.algorithm()) == SshKeyAlgorithm::X25519 => + { + let key: [u8; 32] = key + .private + .as_ref() + .try_into() + .map_err(|_| internal!("invalid x25519 private key"))?; + + Ok(Box::new(curve25519::StaticSecret::from(key))) + } + _ => Err(SshKeyError::UnexpectedSshKeyType { + path: key.path, + wanted_key_algo, + found_key_algo: sk.algorithm().into(), + } + .boxed()), } } } @@ -197,6 +253,9 @@ mod tests { const OPENSSH_ED25519: &[u8] = include_bytes!("../../testdata/ed25519_openssh.private"); const OPENSSH_ED25519_BAD: &[u8] = include_bytes!("../../testdata/ed25519_openssh_bad.private"); const OPENSSH_DSA: &[u8] = include_bytes!("../../testdata/dsa_openssh.private"); + const OPENSSH_X25519: &[u8] = include_bytes!("../../testdata/x25519_openssh.private"); + const OPENSSH_X25519_UNKNOWN_ALGORITHM: &[u8] = + include_bytes!("../../testdata/x25519_openssh_unknown_algorithm.private"); #[test] fn wrong_key_type() { @@ -240,4 +299,31 @@ mod tests { assert!(erased_key.downcast::().is_ok()); } + + #[test] + fn x25519_key() { + let key_type = KeyType::X25519StaticSecret; + let key = UnparsedOpenSshKey::new(OPENSSH_X25519.into(), PathBuf::from("/dummy/path")); + let erased_key = key_type.parse_ssh_format_erased(key).unwrap(); + + assert!(erased_key.downcast::().is_ok()); + } + + #[test] + fn invalid_x25519_key() { + let key_type = KeyType::X25519StaticSecret; + let key = UnparsedOpenSshKey::new( + OPENSSH_X25519_UNKNOWN_ALGORITHM.into(), + PathBuf::from("/dummy/path"), + ); + let err = key_type + .parse_ssh_format_erased(key) + .map(|_| "") + .unwrap_err(); + + assert_eq!( + err.to_string(), + "Unexpected OpenSSH key type: wanted X25519, found pangolin@torproject.org" + ); + } } diff --git a/crates/tor-keymgr/src/keystore.rs b/crates/tor-keymgr/src/keystore.rs index 90cc1a1e9..23795024b 100644 --- a/crates/tor-keymgr/src/keystore.rs +++ b/crates/tor-keymgr/src/keystore.rs @@ -2,15 +2,15 @@ pub(crate) mod arti; -pub use ssh_key::private::KeypairData as SshKeypairData; - use rand::{CryptoRng, RngCore}; -use ssh_key::private::{Ed25519Keypair, Ed25519PrivateKey}; -use ssh_key::public::Ed25519PublicKey; +use ssh_key::private::{Ed25519Keypair, Ed25519PrivateKey, OpaqueKeypair}; +use ssh_key::public::{Ed25519PublicKey, OpaquePublicKey}; +use ssh_key::{Algorithm, AlgorithmName}; use tor_error::internal; use tor_hscrypto::pk::{HsClientDescEncSecretKey, HsClientIntroAuthKeypair}; use tor_llcrypto::pk::{curve25519, ed25519}; +use crate::key_type::ssh::X25519_ALGORITHM_NAME; use crate::key_type::KeyType; use crate::{KeySpecifier, KeystoreId, Result}; @@ -91,8 +91,8 @@ pub trait EncodableKey: Downcast { where Self: Sized; - /// Return the [`SshKeypairData`][crate::SshKeypairData] of this key. - fn as_ssh_keypair_data(&self) -> Result; + /// Return the [`KeypairData`][ssh_key::private::KeypairData] of this key. + fn as_ssh_keypair_data(&self) -> Result; } impl_downcast!(EncodableKey); @@ -112,9 +112,16 @@ impl EncodableKey for curve25519::StaticSecret { Ok(curve25519::StaticSecret::new(rng)) } - fn as_ssh_keypair_data(&self) -> Result { - // TODO HSS: implement when the ssh-key changes from #936 are released. - Err(internal!("not implemented").into()) + fn as_ssh_keypair_data(&self) -> Result { + let algorithm_name = AlgorithmName::new(X25519_ALGORITHM_NAME) + .map_err(|_| internal!("invalid algorithm name"))?; + + let public = curve25519::PublicKey::from(self); + let ssh_public = + OpaquePublicKey::new(public.to_bytes().to_vec(), Algorithm::Other(algorithm_name)); + let keypair = OpaqueKeypair::new(self.to_bytes().to_vec(), ssh_public); + + Ok(ssh_key::private::KeypairData::Other(keypair)) } } @@ -135,13 +142,13 @@ impl EncodableKey for ed25519::Keypair { Ok(ed25519::Keypair::generate(&mut rng.rng_compat())) } - fn as_ssh_keypair_data(&self) -> Result { + fn as_ssh_keypair_data(&self) -> Result { let keypair = Ed25519Keypair { public: Ed25519PublicKey(self.public.to_bytes()), private: Ed25519PrivateKey::from_bytes(self.secret.as_bytes()), }; - Ok(SshKeypairData::Ed25519(keypair)) + Ok(ssh_key::private::KeypairData::Ed25519(keypair)) } } diff --git a/crates/tor-keymgr/src/lib.rs b/crates/tor-keymgr/src/lib.rs index c32da8deb..758158041 100644 --- a/crates/tor-keymgr/src/lib.rs +++ b/crates/tor-keymgr/src/lib.rs @@ -65,8 +65,9 @@ pub use key_specifier::{ArtiPath, ArtiPathComponent, CTorPath, KeySpecifier}; pub use { key_type::KeyType, keystore::arti::ArtiNativeKeystore, - keystore::{EncodableKey, ErasedKey, KeygenRng, Keystore, SshKeypairData, ToEncodableKey}, + keystore::{EncodableKey, ErasedKey, KeygenRng, Keystore, ToEncodableKey}, mgr::KeyMgr, + ssh_key, }; #[cfg(not(feature = "keymgr"))] diff --git a/crates/tor-keymgr/src/mgr.rs b/crates/tor-keymgr/src/mgr.rs index 20df58e3a..c28c7a487 100644 --- a/crates/tor-keymgr/src/mgr.rs +++ b/crates/tor-keymgr/src/mgr.rs @@ -192,7 +192,7 @@ mod tests { #![allow(clippy::useless_vec)] //! use super::*; - use crate::{ArtiPath, ErasedKey, KeyType, SshKeypairData}; + use crate::{ArtiPath, ErasedKey, KeyType}; use std::collections::HashMap; use std::str::FromStr; use std::sync::RwLock; @@ -217,9 +217,11 @@ mod tests { Ok("generated_test_key".into()) } - fn as_ssh_keypair_data(&self) -> Result { + fn as_ssh_keypair_data(&self) -> Result { // (Ab)use the encrypted variant for testing purposes - Ok(SshKeypairData::Encrypted(self.as_bytes().to_vec())) + Ok(ssh_key::private::KeypairData::Encrypted( + self.as_bytes().to_vec(), + )) } } diff --git a/crates/tor-keymgr/testdata/x25519_openssh.private b/crates/tor-keymgr/testdata/x25519_openssh.private new file mode 100644 index 000000000..33552585f --- /dev/null +++ b/crates/tor-keymgr/testdata/x25519_openssh.private @@ -0,0 +1,7 @@ +-----BEGIN OPENSSH PRIVATE KEY----- +b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAPQAAABV4MjU1MT +lAdG9ycHJvamVjdC5vcmcAAAAgD3MDTeSLGUGD1Re2s6aF6g73KSyiteXjV1SL+pu6ASUA +AAB4wA/uXMAP7lwAAAAVeDI1NTE5QHRvcnByb2plY3Qub3JnAAAAIA9zA03kixlBg9UXtr +OmheoO9yksorXl41dUi/qbugElAAAAIDCADBQ5fbboMIMbmOgHMfHZEaIcdm0TF2Kt8bHc +pz5zAAAACHRlc3Qta2V5AQID +-----END OPENSSH PRIVATE KEY----- diff --git a/crates/tor-keymgr/testdata/x25519_openssh_unknown_algorithm.private b/crates/tor-keymgr/testdata/x25519_openssh_unknown_algorithm.private new file mode 100644 index 000000000..a048a9b70 --- /dev/null +++ b/crates/tor-keymgr/testdata/x25519_openssh_unknown_algorithm.private @@ -0,0 +1,7 @@ +-----BEGIN OPENSSH PRIVATE KEY----- +b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAPwAAABdwYW5nb2 +xpbkB0b3Jwcm9qZWN0Lm9yZwAAACBFTBevfAXZy6Ut6i9PSHeXHUpaJ6+NKabGzeoT6rFV +DgAAAHjiJEYk4iRGJAAAABdwYW5nb2xpbkB0b3Jwcm9qZWN0Lm9yZwAAACBFTBevfAXZy6 +Ut6i9PSHeXHUpaJ6+NKabGzeoT6rFVDgAAACBo1W5vk9S8KAG7icAnKfq0NasfGTLO2+MJ +ifSnMVvXSgAAAAh0ZXN0LWtleQE= +-----END OPENSSH PRIVATE KEY----- diff --git a/tests/shadow/shadow.data.template/hosts/articlient-onion-auth/keystore/client/default/yr4tcjsgag3l7ar4kt5j2gav22nfs5uaktys5yy2yuuisjreu7ty6aad.onion/KP_hsc_desc_enc.x25519_private b/tests/shadow/shadow.data.template/hosts/articlient-onion-auth/keystore/client/default/yr4tcjsgag3l7ar4kt5j2gav22nfs5uaktys5yy2yuuisjreu7ty6aad.onion/KP_hsc_desc_enc.x25519_private index 5c6846eb9..362199f6a 100644 --- a/tests/shadow/shadow.data.template/hosts/articlient-onion-auth/keystore/client/default/yr4tcjsgag3l7ar4kt5j2gav22nfs5uaktys5yy2yuuisjreu7ty6aad.onion/KP_hsc_desc_enc.x25519_private +++ b/tests/shadow/shadow.data.template/hosts/articlient-onion-auth/keystore/client/default/yr4tcjsgag3l7ar4kt5j2gav22nfs5uaktys5yy2yuuisjreu7ty6aad.onion/KP_hsc_desc_enc.x25519_private @@ -1,7 +1,7 @@ -----BEGIN OPENSSH PRIVATE KEY----- -b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtzc2gtZW -QyNTUxOQAAACD4Tjq19rMz8G49hAAqsAP4MjQUwvU1mYPWxsmVLigDAgAAAJBopPdlaKT3 -ZQAAAAtzc2gtZWQyNTUxOQAAACD4Tjq19rMz8G49hAAqsAP4MjQUwvU1mYPWxsmVLigDAg -AAAEAIsbCNVF+6EdbfJtuw5ehopIUpQtKCQzCDpoaTp9Hfq/hOOrX2szPwbj2EACqwA/gy -NBTC9TWZg9bGyZUuKAMCAAAACHRlc3Qta2V5AQIDBAU= +b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAPQAAABV4MjU1MT +lAdG9ycHJvamVjdC5vcmcAAAAgct8Xu0pzmH8ODOtxWfiHcMf504eIwnsAgvD7C6/+7nUA +AAB419he5tfYXuYAAAAVeDI1NTE5QHRvcnByb2plY3Qub3JnAAAAIHLfF7tKc5h/DgzrcV +n4h3DH+dOHiMJ7AILw+wuv/u51AAAAIKCrmmcBaNqttEiJg5mzr3dlk/RQvGcuMGYctsjk +CFRwAAAACHRlc3Qta2V5AQID -----END OPENSSH PRIVATE KEY----- diff --git a/tests/shadow/shadow.data.template/hosts/fileserver-onion-auth/hs/authorized_clients/default.auth b/tests/shadow/shadow.data.template/hosts/fileserver-onion-auth/hs/authorized_clients/default.auth index e54034ec3..f59e6edb0 100644 --- a/tests/shadow/shadow.data.template/hosts/fileserver-onion-auth/hs/authorized_clients/default.auth +++ b/tests/shadow/shadow.data.template/hosts/fileserver-onion-auth/hs/authorized_clients/default.auth @@ -1,2 +1,2 @@ -descriptor:x25519:IOKTNZA7YBQYJSTZR4V7NSPIPVAGWO3QGTRCJKOVKCMYFTXDQJIA +descriptor:x25519:OLPRPO2KOOMH6DQM5NYVT6EHODD7TU4HRDBHWAEC6D5QXL765Z2Q