diff --git a/src/ec/curve25519/ed25519/signing.rs b/src/ec/curve25519/ed25519/signing.rs index 6e1684d34..bad5ad9bd 100644 --- a/src/ec/curve25519/ed25519/signing.rs +++ b/src/ec/curve25519/ed25519/signing.rs @@ -71,11 +71,18 @@ impl Ed25519KeyPair { /// verify that the public key and the private key are consistent with each /// other. /// + /// Some early implementations of PKCS#8 v2, including earlier versions of + /// *ring* and other implementations, wrapped the public key in the wrong + /// ASN.1 tags. Both that incorrect form and the standardized form are + /// accepted. + /// /// If you need to parse PKCS#8 v1 files (without the public key) then use /// `Ed25519KeyPair::from_pkcs8_maybe_unchecked()` instead. pub fn from_pkcs8(pkcs8: &[u8]) -> Result { - let (seed, public_key) = - unwrap_pkcs8(pkcs8::Version::V2Only, untrusted::Input::from(pkcs8))?; + let version = pkcs8::Version::V2Only(pkcs8::PublicKeyOptions { + accept_legacy_ed25519_public_key_tag: true, + }); + let (seed, public_key) = unwrap_pkcs8(version, untrusted::Input::from(pkcs8))?; Self::from_seed_and_public_key( seed.as_slice_less_safe(), public_key.unwrap().as_slice_less_safe(), @@ -95,10 +102,17 @@ impl Ed25519KeyPair { /// computed from the private key, and there will be no consistency check /// between the public key and the private key. /// + /// Some early implementations of PKCS#8 v2, including earlier versions of + /// *ring* and other implementations, wrapped the public key in the wrong + /// ASN.1 tags. Both that incorrect form and the standardized form are + /// accepted. + /// /// PKCS#8 v2 files are parsed exactly like `Ed25519KeyPair::from_pkcs8()`. pub fn from_pkcs8_maybe_unchecked(pkcs8: &[u8]) -> Result { - let (seed, public_key) = - unwrap_pkcs8(pkcs8::Version::V1OrV2, untrusted::Input::from(pkcs8))?; + let version = pkcs8::Version::V1OrV2(pkcs8::PublicKeyOptions { + accept_legacy_ed25519_public_key_tag: true, + }); + let (seed, public_key) = unwrap_pkcs8(version, untrusted::Input::from(pkcs8))?; if let Some(public_key) = public_key { Self::from_seed_and_public_key( seed.as_slice_less_safe(), diff --git a/src/io/der.rs b/src/io/der.rs index 50a8f79ea..06b207f89 100644 --- a/src/io/der.rs +++ b/src/io/der.rs @@ -35,6 +35,8 @@ pub enum Tag { UTCTime = 0x17, GeneralizedTime = 0x18, + ContextSpecific1 = CONTEXT_SPECIFIC | 1, + ContextSpecificConstructed0 = CONTEXT_SPECIFIC | CONSTRUCTED | 0, ContextSpecificConstructed1 = CONTEXT_SPECIFIC | CONSTRUCTED | 1, ContextSpecificConstructed3 = CONTEXT_SPECIFIC | CONSTRUCTED | 3, @@ -101,10 +103,18 @@ pub fn read_tag_and_get_value<'a>( Ok((tag, inner)) } +#[inline] pub fn bit_string_with_no_unused_bits<'a>( input: &mut untrusted::Reader<'a>, ) -> Result, error::Unspecified> { - nested(input, Tag::BitString, error::Unspecified, |value| { + bit_string_tagged_with_no_unused_bits(Tag::BitString, input) +} + +pub(crate) fn bit_string_tagged_with_no_unused_bits<'a>( + tag: Tag, + input: &mut untrusted::Reader<'a>, +) -> Result, error::Unspecified> { + nested(input, tag, error::Unspecified, |value| { let unused_bits_at_end = value.read_byte().map_err(|_| error::Unspecified)?; if unused_bits_at_end != 0 { return Err(error::Unspecified); diff --git a/src/pkcs8.rs b/src/pkcs8.rs index c1c69281e..5d1a49e71 100644 --- a/src/pkcs8.rs +++ b/src/pkcs8.rs @@ -18,10 +18,16 @@ use crate::{ec, error, io::der}; +pub(crate) struct PublicKeyOptions { + /// Should the wrong public key ASN.1 tagging used by early implementations + /// of PKCS#8 v2 (including earlier versions of *ring*) be accepted? + pub accept_legacy_ed25519_public_key_tag: bool, +} + pub(crate) enum Version { V1Only, - V1OrV2, - V2Only, + V1OrV2(PublicKeyOptions), + V2Only(PublicKeyOptions), } /// A template for constructing PKCS#8 documents. @@ -123,10 +129,10 @@ fn unwrap_key__<'a>( return Err(error::KeyRejected::wrong_algorithm()); } - let require_public_key = match (actual_version, version) { - (0, Version::V1Only) => false, - (0, Version::V1OrV2) => false, - (1, Version::V1OrV2) | (1, Version::V2Only) => true, + let public_key_options = match (actual_version, version) { + (0, Version::V1Only) => None, + (0, Version::V1OrV2(_)) => None, + (1, Version::V1OrV2(options)) | (1, Version::V2Only(options)) => Some(options), _ => { return Err(error::KeyRejected::version_not_supported()); } @@ -141,17 +147,25 @@ fn unwrap_key__<'a>( .map_err(|error::Unspecified| error::KeyRejected::invalid_encoding())?; } - let public_key = if require_public_key { + let public_key = if let Some(options) = public_key_options { if input.at_end() { return Err(error::KeyRejected::public_key_is_missing()); } - let public_key = der::nested( - input, - der::Tag::ContextSpecificConstructed1, - error::Unspecified, - der::bit_string_with_no_unused_bits, - ) - .map_err(|error::Unspecified| error::KeyRejected::invalid_encoding())?; + + const INCORRECT_LEGACY: der::Tag = der::Tag::ContextSpecificConstructed1; + let result = + if options.accept_legacy_ed25519_public_key_tag && input.peek(INCORRECT_LEGACY as u8) { + der::nested( + input, + INCORRECT_LEGACY, + error::Unspecified, + der::bit_string_with_no_unused_bits, + ) + } else { + der::bit_string_tagged_with_no_unused_bits(der::Tag::ContextSpecific1, input) + }; + let public_key = + result.map_err(|error::Unspecified| error::KeyRejected::invalid_encoding())?; Some(public_key) } else { None diff --git a/tests/ed25519_test_private_key.p8 b/tests/ed25519_test_private_key.p8 index f60cc19b9..fc9017666 100644 Binary files a/tests/ed25519_test_private_key.p8 and b/tests/ed25519_test_private_key.p8 differ