Fix for bug 209. Required some reworking of the ASN.1 bytestring

decoding code but seems an improvement.

8 files changed, 86 insertions, 47 deletions

diff --git a/checks/ecc_testdata/ecc_private_with_rfc5915_ext.pem b/checks/ecc_testdata/ecc_private_with_rfc5915_ext.pem
new file mode 100644
index 000000000..a8699fce7
--- /dev/null
+++ b/checks/ecc_testdata/ecc_private_with_rfc5915_ext.pem
@@ -0,0 +1,6 @@
+-----BEGIN ENCRYPTED PRIVATE KEY-----
+MIGwMBsGCSqGSIb3DQEFAzAOBAhLqOHiUDFjTwICCAAEgZD1k1BiBROTLBRoFQG5
+gNEipqwBXlKKv+cen7laWHdABXBPGSXBTZGiwsfVPitW+mT3kLHjPZOwJ+55Chka
+QkBardzHxD2LwX8BXxDqiv61R/NsGh376+KXxTbZApobC3p40T24wMvX0O04HXaZ
+6qPBsRo1byuhn0jM6Qr0O/HnYHH4/fiIN6Iq2HF6/QaUnak=
+-----END ENCRYPTED PRIVATE KEY-----
diff --git a/checks/ecdsa.cpp b/checks/ecdsa.cpp
index 1c7df5255..a43de69c5 100644
--- a/checks/ecdsa.cpp
+++ b/checks/ecdsa.cpp
@@ -413,6 +413,24 @@ void test_read_pkcs8(RandomNumberGenerator& rng)
       }
    }
 
+void test_ecc_key_with_rfc5915_extensions(RandomNumberGenerator& rng)
+   {
+   const std::string pw = "G3bz1L1gmB5ULietOZdoLPu63D7uwTLMEk";
+
+   try
+      {
+      std::unique_ptr<PKCS8_PrivateKey> pkcs8(
+         PKCS8::load_key(TEST_DATA_DIR "/ecc_private_with_rfc5915_ext.pem", rng, pw));
+
+      if(!dynamic_cast<ECDSA_PrivateKey*>(pkcs8.get()))
+         std::cout << "Loaded RFC 5915 key, but got something other than an ECDSA key\n";
+      }
+   catch(std::exception& e)
+      {
+      std::cout << "Exception in " << __func__ << " - " << e.what() << "\n";
+      }
+   }
+
 }
 
 u32bit do_ecdsa_tests(Botan::RandomNumberGenerator& rng)
@@ -430,6 +448,8 @@ u32bit do_ecdsa_tests(Botan::RandomNumberGenerator& rng)
    test_curve_registry(rng);
    test_read_pkcs8(rng);
 
+   test_ecc_key_with_rfc5915_extensions(rng);
+
    std::cout << std::endl;
 
    return 0;
diff --git a/doc/relnotes/1_11_0.rst b/doc/relnotes/1_11_0.rst
index d8bc5515c..1d3909dfe 100644
--- a/doc/relnotes/1_11_0.rst
+++ b/doc/relnotes/1_11_0.rst
@@ -36,3 +36,13 @@ limit of what we can mlock) is mmap'ed, with allocations being done
 using a best-fit allocator and all metadata held outside the mmap'ed
 range, in an effort to make best use of the very limited amount of
 memory current Linux kernels allow unpriveledged users to lock.
+
+:rfc:`5915` adds some extended information which can be included in
+ECC private keys which the ECC key decoder did not expect, causing an
+exception when such a key was loaded. In particular, recent versions
+of OpenSSL use these fields. Now these fields are decoded properly,
+and if the public key value is included it is used, as otherwise the
+public key needs to be rederived from the private key. However the
+library does not include these fields on encoding keys for
+compatability with software that does not expect them (including older
+versions of botan).
diff --git a/src/asn1/asn1_int.h b/src/asn1/asn1_int.h
index 6d254e4db..564f4ecdb 100644
--- a/src/asn1/asn1_int.h
+++ b/src/asn1/asn1_int.h
@@ -23,6 +23,8 @@ enum ASN1_Tag {
 
    CONSTRUCTED      = 0x20,
 
+   PRIVATE          = CONSTRUCTED | CONTEXT_SPECIFIC,
+
    EOC              = 0x00,
    BOOLEAN          = 0x01,
    INTEGER          = 0x02,
diff --git a/src/asn1/asn1_oid.h b/src/asn1/asn1_oid.h
index 85e863907..9d712b256 100644
--- a/src/asn1/asn1_oid.h
+++ b/src/asn1/asn1_oid.h
@@ -27,7 +27,7 @@ class BOTAN_DLL OID : public ASN1_Object
       * Find out whether this OID is empty
       * @return true is no OID value is set
       */
-      bool is_empty() const { return id.size() == 0; }
+      bool empty() const { return id.size() == 0; }
 
       /**
       * Get this OID as list (vector) of its components.
diff --git a/src/asn1/ber_dec.cpp b/src/asn1/ber_dec.cpp
index 25c412600..81b2e34f6 100644
--- a/src/asn1/ber_dec.cpp
+++ b/src/asn1/ber_dec.cpp
@@ -558,41 +558,4 @@ BER_Decoder& BER_Decoder::decode(std::vector<byte>& buffer,
    return (*this);
    }
 
-/*
-* Decode an OPTIONAL string type
-*/
-BER_Decoder& BER_Decoder::decode_optional_string(secure_vector<byte>& out,
-                                                 ASN1_Tag real_type,
-                                                 u16bit type_no)
-   {
-   BER_Object obj = get_next_object();
-
-   ASN1_Tag type_tag = static_cast<ASN1_Tag>(type_no);
-
-   out.clear();
-   push_back(obj);
-
-   if(obj.type_tag == type_tag && obj.class_tag == CONTEXT_SPECIFIC)
-      decode(out, real_type, type_tag, CONTEXT_SPECIFIC);
-
-   return (*this);
-   }
-
-BER_Decoder& BER_Decoder::decode_optional_string(std::vector<byte>& out,
-                                                 ASN1_Tag real_type,
-                                                 u16bit type_no)
-   {
-   BER_Object obj = get_next_object();
-
-   ASN1_Tag type_tag = static_cast<ASN1_Tag>(type_no);
-
-   out.clear();
-   push_back(obj);
-
-   if(obj.type_tag == type_tag && obj.class_tag == CONTEXT_SPECIFIC)
-      decode(out, real_type, type_tag, CONTEXT_SPECIFIC);
-
-   return (*this);
-   }
-
 }
diff --git a/src/asn1/ber_dec.h b/src/asn1/ber_dec.h
index 0d0d0ec72..ebfa91c85 100644
--- a/src/asn1/ber_dec.h
+++ b/src/asn1/ber_dec.h
@@ -123,13 +123,37 @@ class BOTAN_DLL BER_Decoder
          return (*this);
          }
 
-      BER_Decoder& decode_optional_string(std::vector<byte>& out,
+      /*
+      * Decode an OPTIONAL string type
+      */
+      template<typename Alloc>
+      BER_Decoder& decode_optional_string(std::vector<byte, Alloc>& out,
                                           ASN1_Tag real_type,
-                                          u16bit type_no);
+                                          u16bit type_no,
+                                          ASN1_Tag class_tag = CONTEXT_SPECIFIC)
+         {
+         BER_Object obj = get_next_object();
+
+         ASN1_Tag type_tag = static_cast<ASN1_Tag>(type_no);
+
+         if(obj.type_tag == type_tag && obj.class_tag == class_tag)
+            {
+            if((class_tag & CONSTRUCTED) && (class_tag & CONTEXT_SPECIFIC))
+               BER_Decoder(obj.value).decode(out, real_type).verify_end();
+            else
+               {
+               push_back(obj);
+               decode(out, real_type, type_tag, class_tag);
+               }
+            }
+         else
+            {
+            out.clear();
+            push_back(obj);
+            }
 
-      BER_Decoder& decode_optional_string(secure_vector<byte>& out,
-                                          ASN1_Tag real_type,
-                                          u16bit type_no);
+         return (*this);
+         }
 
       BER_Decoder& operator=(const BER_Decoder&) = delete;
 
diff --git a/src/pubkey/ecc_key/ecc_key.cpp b/src/pubkey/ecc_key/ecc_key.cpp
index ead129ec6..2b6deea44 100644
--- a/src/pubkey/ecc_key/ecc_key.cpp
+++ b/src/pubkey/ecc_key/ecc_key.cpp
@@ -113,17 +113,31 @@ EC_PrivateKey::EC_PrivateKey(const AlgorithmIdentifier& alg_id,
    domain_params = EC_Group(alg_id.parameters);
    domain_encoding = EC_DOMPAR_ENC_EXPLICIT;
 
+   OID key_parameters;
+   secure_vector<byte> public_key_bits;
+
    BER_Decoder(key_bits)
       .start_cons(SEQUENCE)
          .decode_and_check<size_t>(1, "Unknown version code for ECC key")
          .decode_octet_string_bigint(private_key)
-      .verify_end()
+         .decode_optional(key_parameters, ASN1_Tag(0), PRIVATE)
+         .decode_optional_string(public_key_bits, BIT_STRING, 1, PRIVATE)
       .end_cons();
 
-   public_key = domain().get_base_point() * private_key;
+   if(!key_parameters.empty() && key_parameters != alg_id.oid)
+      throw Decoding_Error("EC_PrivateKey - inner and outer OIDs did not match");
 
-   BOTAN_ASSERT(public_key.on_the_curve(),
-                "Loaded ECC private key not on the curve");
+   if(public_key_bits.empty())
+      {
+      public_key = domain().get_base_point() * private_key;
+      BOTAN_ASSERT(public_key.on_the_curve(),
+                   "Public key derived from private key was on the curve");
+      }
+   else
+      {
+      public_key = OS2ECP(public_key_bits, domain().get_curve());
+      // OS2ECP verifies that the point is on the curve
+      }
    }
 
 }