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

Fix ed25519 key format in pkcs8 #272

Merged
merged 2 commits into from
Apr 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
45 changes: 24 additions & 21 deletions russh-keys/src/format/pkcs8.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,17 +124,11 @@ fn write_key_v1(writer: &mut yasna::DERWriterSeq, secret: &ed25519_dalek::Signin
.next()
.write_oid(&ObjectIdentifier::from_slice(ED25519));
});
let seed = yasna::construct_der(|writer| {
writer.write_bytes(
[secret.to_bytes().as_slice(), public.as_bytes().as_slice()]
.concat()
.as_slice(),
)
});
let seed = yasna::construct_der(|writer| writer.write_bytes(secret.to_bytes().as_slice()));
writer.next().write_bytes(&seed);
writer
.next()
.write_tagged(yasna::Tag::context(1), |writer| {
.write_tagged_implicit(yasna::Tag::context(1), |writer| {
writer.write_bitvec(&BitVec::from_bytes(public.as_bytes()))
})
}
Expand All @@ -144,24 +138,29 @@ fn read_key_v1(reader: &mut BERReaderSeq) -> Result<key::KeyPair, Error> {
.next()
.read_sequence(|reader| reader.next().read_oid())?;
if oid.components().as_slice() == ED25519 {
use ed25519_dalek::SigningKey;
let secret = {
let s = yasna::parse_der(&reader.next().read_bytes()?, |reader| reader.read_bytes())?;

s.get(..ed25519_dalek::SECRET_KEY_LENGTH)
.ok_or(Error::KeyIsCorrupt)
.and_then(|s| SigningKey::try_from(s).map_err(|_| Error::CouldNotReadKey))?
};
// Consume the public key
reader
.next()
.read_tagged(yasna::Tag::context(1), |reader| reader.read_bitvec())?;
Ok(key::KeyPair::Ed25519(secret))
let key = parse_ed25519_private_key(&reader.next().read_bytes()?)?;
let _attributes = reader.read_optional(|reader| {
reader.read_tagged_implicit(yasna::Tag::context(0), |reader| reader.read_der())
})?;
let _public_key = reader.read_optional(|reader| {
reader.read_tagged_implicit(yasna::Tag::context(1), |reader| reader.read_bitvec())
})?;
Ok(key)
} else {
Err(Error::CouldNotReadKey)
}
}

fn parse_ed25519_private_key(der: &[u8]) -> Result<key::KeyPair, Error> {
use ed25519_dalek::SigningKey;
let secret_bytes = yasna::parse_der(der, |reader| reader.read_bytes())?;
let secret_key = SigningKey::from(
<&[u8; ed25519_dalek::SECRET_KEY_LENGTH]>::try_from(secret_bytes.as_slice())
.map_err(|_| Error::CouldNotReadKey)?,
);
Ok(key::KeyPair::Ed25519(secret_key))
}

fn write_key_v0_rsa(writer: &mut yasna::DERWriterSeq, key: &key::RsaPrivate) {
writer.next().write_u32(0);
// write OID
Expand Down Expand Up @@ -240,6 +239,8 @@ fn write_key_v0_ec(writer: &mut yasna::DERWriterSeq, key: &crate::ec::PrivateKey
enum KeyType {
Unknown(ObjectIdentifier),
#[allow(clippy::upper_case_acronyms)]
ED25519,
#[allow(clippy::upper_case_acronyms)]
RSA,
EC(ObjectIdentifier),
}
Expand All @@ -248,6 +249,7 @@ fn read_key_v0(reader: &mut BERReaderSeq) -> Result<key::KeyPair, Error> {
let key_type = reader.next().read_sequence(|reader| {
let oid = reader.next().read_oid()?;
Ok(match oid.components().as_slice() {
ED25519 => KeyType::ED25519,
RSA => {
reader.next().read_null()?;
KeyType::RSA
Expand All @@ -258,6 +260,7 @@ fn read_key_v0(reader: &mut BERReaderSeq) -> Result<key::KeyPair, Error> {
})?;
match key_type {
KeyType::Unknown(_) => Err(Error::CouldNotReadKey),
KeyType::ED25519 => parse_ed25519_private_key(&reader.next().read_bytes()?),
KeyType::RSA => {
let seq = &reader.next().read_bytes()?;
let (sk, extra) = yasna::parse_der(seq, |reader| {
Expand Down
32 changes: 32 additions & 0 deletions russh-keys/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,38 @@ QR+u0AypRPmzHnOPAAAAEXJvb3RAMTQwOTExNTQ5NDBkAQ==
decode_secret_key(ED25519_AESCTR_KEY, Some("test")).unwrap();
}

// Key from RFC 8410 Section 10.3. This is a key using PrivateKeyInfo structure.
const RFC8410_ED25519_PRIVATE_ONLY_KEY: &str = "-----BEGIN PRIVATE KEY-----
MC4CAQAwBQYDK2VwBCIEINTuctv5E1hK1bbY8fdp+K06/nwoy/HU++CXqI9EdVhC
-----END PRIVATE KEY-----";

#[test]
fn test_decode_rfc8410_ed25519_private_only_key() {
env_logger::try_init().unwrap_or(());
assert!(matches!(
decode_secret_key(RFC8410_ED25519_PRIVATE_ONLY_KEY, None),
Ok(key::KeyPair::Ed25519 { .. })
));
// We always encode public key, skip test_decode_encode_symmetry.
}

// Key from RFC 8410 Section 10.3. This is a key using OneAsymmetricKey structure.
const RFC8410_ED25519_PRIVATE_PUBLIC_KEY: &str = "-----BEGIN PRIVATE KEY-----
MHICAQEwBQYDK2VwBCIEINTuctv5E1hK1bbY8fdp+K06/nwoy/HU++CXqI9EdVhC
oB8wHQYKKoZIhvcNAQkJFDEPDA1DdXJkbGUgQ2hhaXJzgSEAGb9ECWmEzf6FQbrB
Z9w7lshQhqowtrbLDFw4rXAxZuE=
-----END PRIVATE KEY-----";

#[test]
fn test_decode_rfc8410_ed25519_private_public_key() {
env_logger::try_init().unwrap_or(());
assert!(matches!(
decode_secret_key(RFC8410_ED25519_PRIVATE_PUBLIC_KEY, None),
Ok(key::KeyPair::Ed25519 { .. })
));
// We can't encode attributes, skip test_decode_encode_symmetry.
}

#[test]
fn test_decode_rsa_secret_key() {
env_logger::try_init().unwrap_or(());
Expand Down
Loading