Ed25519::{from_pkcs8, from_pkcs8_maybe_unchecked}: Also accept correc…

…t tagging public key.

See the added API documentation for more details.

Also update the test private key to the standard format.

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<Self, error::KeyRejected> {
-        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<Self, error::KeyRejected> {
-        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(),

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<untrusted::Input<'a>, 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<untrusted::Input<'a>, 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);

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