`ed448_goldilocks::SigningKey::from_pkcs8_pem` fails due to slice length mismatch (in `0.14.0-pre.2`)

[LukasKalbertodt]

I tried using version of 0.14.0-pre.2 of ed448_goldilocks and encountered a problem that I'm pretty sure is a bug in the library:

const PRIVATE_KEY: &str = "-----BEGIN PRIVATE KEY-----
MEcCAQAwBQYDK2VxBDsEOYrsF28Jlv/NB2SCjTz0Ax578DThtcGskl01aPFyjIsQ
yx0O34c/sstZ3SeqFbpP62izrpcSrMDXJA==
-----END PRIVATE KEY-----
";

use ed448_goldilocks::{SigningKey, pkcs8::DecodePrivateKey};

SigningKey::from_pkcs8_pem(PRIVATE_KEY)

This returns an Err, despite the key being correct. I think the bug is here:

elliptic-curves/ed448-goldilocks/src/sign/signing_key.rs

Lines 339 to 371 in 20eeeff

	#[cfg(feature = "pkcs8")]
	impl TryFrom<pkcs8::PrivateKeyInfoRef<'_>> for KeypairBytes {
	type Error = pkcs8::Error;
	fn try_from(value: pkcs8::PrivateKeyInfoRef<'_>) -> Result<Self, Self::Error> {
	if value.algorithm.oid != super::ALGORITHM_OID {
	return Err(pkcs8::Error::KeyMalformed);
	}
	if value.private_key.as_bytes().len() != SECRET_KEY_LENGTH {
	return Err(pkcs8::Error::KeyMalformed);
	}
	let mut secret_key = [0u8; SECRET_KEY_LENGTH];
	secret_key.copy_from_slice(value.private_key.as_bytes());
	let verifying_key = if let Some(public_key) = value.public_key {
	if public_key.has_unused_bits() {
	return Err(pkcs8::Error::KeyMalformed);
	}
	let public_key = public_key.raw_bytes();
	if public_key.len() != PUBLIC_KEY_LENGTH {
	return Err(pkcs8::Error::KeyMalformed);
	}
	let mut bytes = [0u8; PUBLIC_KEY_LENGTH];
	bytes.copy_from_slice(public_key);
	Some(bytes)
	} else {
	None
	};
	Ok(KeypairBytes {
	secret_key,
	verifying_key,
	})
	}
	}

It checks value.private_key.as_bytes().len() != SECRET_KEY_LENGTH, but value.private_key.as_bytes() returns the whole base64 encoded data, including the PKCS8 header.

let (_, pkcs8_bytes) = pem_rfc7468::decode_vec(PRIVATE_KEY.as_bytes()).unwrap();
let info = pkcs8::PrivateKeyInfoRef::try_from(pkcs8_bytes.as_slice()).unwrap();
println!("{:02x?}", info.private_key.as_bytes());

This prints [04, 39, 8a, ec, 17, 6f, 09, 96, ff, cd, 07, 64, 82, 8d, 3c, f4, 03, 1e, 7b, f0, 34, e1, b5, c1, ac, 92, 5d, 35, 68, f1, 72, 8c, 8b, 10, cb, 1d, 0e, df, 87, 3f, b2, cb, 59, dd, 27, aa, 15, ba, 4f, eb, 68, b3, ae, 97, 12, ac, c0, d7, 24]

Letting openssl print the info about the key:

➜  openssl pkey -in ed448-key.pem -text -noout
ED448 Private-Key:
priv:
    8a:ec:17:6f:09:96:ff:cd:07:64:82:8d:3c:f4:03:
    1e:7b:f0:34:e1:b5:c1:ac:92:5d:35:68:f1:72:8c:
    8b:10:cb:1d:0e:df:87:3f:b2:cb:59:dd:27:aa:15:
    ba:4f:eb:68:b3:ae:97:12:ac:c0:d7:24

We see that it's the same data, without the first 2 bytes.

[tarcieri]

Yeah, this looks like a bug. RFC8410 specifies the privateKey field contains an inner OCTET STRING:

https://datatracker.ietf.org/doc/html/rfc8410#section-7

For the keys defined in this document, the private key is always an
opaque byte sequence. The ASN.1 type CurvePrivateKey is defined in
this document to hold the byte sequence. Thus, when encoding a
OneAsymmetricKey object, the private key is wrapped in a
CurvePrivateKey object and wrapped by the OCTET STRING of the
"privateKey" field.

CurvePrivateKey ::= OCTET STRING

Those leading 04 39 bytes are the tag and length (57-bytes) of that inner nested OCTET STRING (i.e. CurvePrivateKey).

In the ed25519 crate we have a type for decoding this format: https://docs.rs/ed25519/latest/ed25519/pkcs8/struct.KeypairBytes.html

We should probably add a similar one to the ed448 crate and leverage it here.

[tarcieri]

So it turns out ed448 actually already has a similar type, it just wasn't showing up in the rustdoc:

https://docs.rs/ed448/0.5.0-rc.0/ed448/pkcs8/struct.KeypairBytes.html

That's been tested with the CURDLE test vectors, which would also be good to use with ed448-goldilocks:

https://github.com/RustCrypto/signatures/tree/master/ed448/tests/examples

[tarcieri]

I should also mention, for anyone interested in working on this feature, it should be pretty easy to adapt/copy-and-paste the implementation I wrote for ed25519-dalek:

https://github.com/dalek-cryptography/curve25519-dalek/blob/59ab400f1ba4975d25dd4d6614836eeef3e093f3/ed25519-dalek/src/signing.rs#L688-L757