Merge GH #1335 Support uninitialized X509 objects

12 files changed, 167 insertions, 136 deletions

diff --git a/src/lib/x509/crl_ent.cpp b/src/lib/x509/crl_ent.cpp
index 61fd5d31f..046659661 100644
--- a/src/lib/x509/crl_ent.cpp
+++ b/src/lib/x509/crl_ent.cpp
@@ -109,7 +109,10 @@ void CRL_Entry::decode_from(BER_Decoder& source)
 const CRL_Entry_Data& CRL_Entry::data() const
    {
    if(!m_data)
-      throw Decoding_Error("Uninitialized CRL_Entry");
+      {
+      throw Invalid_State("CRL_Entry_Data uninitialized");
+      }
+
    return *m_data.get();
    }
 
diff --git a/src/lib/x509/crl_ent.h b/src/lib/x509/crl_ent.h
index 967dc92d2..b902eecfa 100644
--- a/src/lib/x509/crl_ent.h
+++ b/src/lib/x509/crl_ent.h
@@ -71,7 +71,7 @@ class BOTAN_PUBLIC_API(2,0) CRL_Entry final : public ASN1_Object
       /**
       * Create uninitialized CRL_Entry object
       */
-      CRL_Entry() {}
+      CRL_Entry() = default;
 
       /**
       * Construct an CRL entry.
diff --git a/src/lib/x509/pkcs10.cpp b/src/lib/x509/pkcs10.cpp
index 82ef0945d..1f7e915ff 100644
--- a/src/lib/x509/pkcs10.cpp
+++ b/src/lib/x509/pkcs10.cpp
@@ -23,35 +23,35 @@ struct PKCS10_Data
    Extensions m_extensions;
    };
 
-/*
-* PKCS10_Request Constructor
-*/
-PKCS10_Request::PKCS10_Request(DataSource& in) :
-   X509_Object(in, "CERTIFICATE REQUEST/NEW CERTIFICATE REQUEST")
+std::string PKCS10_Request::PEM_label() const
    {
-   do_decode();
+   return "CERTIFICATE REQUEST";
    }
 
-#if defined(BOTAN_TARGET_OS_HAS_FILESYSTEM)
-/*
-* PKCS10_Request Constructor
-*/
-PKCS10_Request::PKCS10_Request(const std::string& fsname) :
-   X509_Object(fsname, "CERTIFICATE REQUEST/NEW CERTIFICATE REQUEST")
+std::vector<std::string> PKCS10_Request::alternate_PEM_labels() const
    {
-   do_decode();
+   return { "NEW CERTIFICATE REQUEST" };
    }
-#endif
 
-/*
-* PKCS10_Request Constructor
-*/
-PKCS10_Request::PKCS10_Request(const std::vector<uint8_t>& in) :
-   X509_Object(in, "CERTIFICATE REQUEST/NEW CERTIFICATE REQUEST")
+PKCS10_Request::PKCS10_Request(DataSource& src)
    {
-   do_decode();
+   load_data(src);
    }
 
+PKCS10_Request::PKCS10_Request(const std::vector<uint8_t>& vec)
+   {
+   DataSource_Memory src(vec.data(), vec.size());
+   load_data(src);
+   }
+
+#if defined(BOTAN_TARGET_OS_HAS_FILESYSTEM)
+PKCS10_Request::PKCS10_Request(const std::string& fsname)
+   {
+   DataSource_Stream src(fsname, true);
+   load_data(src);
+   }
+#endif
+
 /*
 * Decode the CertificateRequestInfo
 */
diff --git a/src/lib/x509/pkcs10.h b/src/lib/x509/pkcs10.h
index abed5fa75..731402960 100644
--- a/src/lib/x509/pkcs10.h
+++ b/src/lib/x509/pkcs10.h
@@ -110,6 +110,10 @@ class BOTAN_PUBLIC_API(2,0) PKCS10_Request final : public X509_Object
       */
       explicit PKCS10_Request(const std::vector<uint8_t>& vec);
    private:
+      std::string PEM_label() const override;
+
+      std::vector<std::string> alternate_PEM_labels() const override;
+
       void force_decode() override;
 
       const PKCS10_Data& data() const;
diff --git a/src/lib/x509/x509_crl.cpp b/src/lib/x509/x509_crl.cpp
index 4a6c4249a..a739d2f60 100644
--- a/src/lib/x509/x509_crl.cpp
+++ b/src/lib/x509/x509_crl.cpp
@@ -25,31 +25,34 @@ struct CRL_Data
    std::vector<uint8_t> m_auth_key_id;
    };
 
-/*
-* Load a X.509 CRL
-*/
-X509_CRL::X509_CRL(DataSource& in) :
-   X509_Object(in, "X509 CRL/CRL")
+std::string X509_CRL::PEM_label() const
    {
-   do_decode();
+   return "X509 CRL";
    }
 
-#if defined(BOTAN_TARGET_OS_HAS_FILESYSTEM)
-/*
-* Load a X.509 CRL
-*/
-X509_CRL::X509_CRL(const std::string& fsname) :
-   X509_Object(fsname, "CRL/X509 CRL")
+std::vector<std::string> X509_CRL::alternate_PEM_labels() const
    {
-   do_decode();
+   return { "CRL" };
+   }
+
+X509_CRL::X509_CRL(DataSource& src)
+   {
+   load_data(src);
+   }
+
+X509_CRL::X509_CRL(const std::vector<uint8_t>& vec)
+   {
+   DataSource_Memory src(vec.data(), vec.size());
+   load_data(src);
    }
-#endif
 
-X509_CRL::X509_CRL(const std::vector<uint8_t>& in) :
-   X509_Object(in, "CRL/X509 CRL")
+#if defined(BOTAN_TARGET_OS_HAS_FILESYSTEM)
+X509_CRL::X509_CRL(const std::string& fsname)
    {
-   do_decode();
+   DataSource_Stream src(fsname, true);
+   load_data(src);
    }
+#endif
 
 X509_CRL::X509_CRL(const X509_DN& issuer,
                    const X509_Time& this_update,
@@ -174,7 +177,9 @@ void X509_CRL::force_decode()
 const CRL_Data& X509_CRL::data() const
    {
    if(!m_data)
-      throw Decoding_Error("Error decoding X509 CRL");
+      {
+      throw Invalid_State("X509_CRL uninitialized");
+      }
    return *m_data.get();
    }
 
diff --git a/src/lib/x509/x509_crl.h b/src/lib/x509/x509_crl.h
index 7c510712a..fb8307d5a 100644
--- a/src/lib/x509/x509_crl.h
+++ b/src/lib/x509/x509_crl.h
@@ -83,6 +83,12 @@ class BOTAN_PUBLIC_API(2,0) X509_CRL final : public X509_Object
       const X509_Time& next_update() const;
 
       /**
+      * Create an uninitialized CRL object. Any attempts to access
+      * this object will throw an exception.
+      */
+      X509_CRL() = default;
+
+      /**
       * Construct a CRL from a data source.
       * @param source the data source providing the DER or PEM encoded CRL.
       */
@@ -113,6 +119,10 @@ class BOTAN_PUBLIC_API(2,0) X509_CRL final : public X509_Object
                const X509_Time& nextUpdate, const std::vector<CRL_Entry>& revoked);
 
    private:
+      std::string PEM_label() const override;
+
+      std::vector<std::string> alternate_PEM_labels() const override;
+
       void force_decode() override;
 
       const CRL_Data& data() const;
diff --git a/src/lib/x509/x509_obj.cpp b/src/lib/x509/x509_obj.cpp
index dad27d6ff..4450df7bb 100644
--- a/src/lib/x509/x509_obj.cpp
+++ b/src/lib/x509/x509_obj.cpp
@@ -47,45 +47,10 @@ Pss_params decode_pss_params(const std::vector<uint8_t>& encoded_pss_params)
 }
 
 /*
-* Create a generic X.509 object
-*/
-X509_Object::X509_Object(DataSource& stream, const std::string& labels)
-   {
-   init(stream, labels);
-   }
-
-#if defined(BOTAN_TARGET_OS_HAS_FILESYSTEM)
-/*
-* Create a generic X.509 object
-*/
-X509_Object::X509_Object(const std::string& file, const std::string& labels)
-   {
-   DataSource_Stream stream(file, true);
-   init(stream, labels);
-   }
-#endif
-
-/*
-* Create a generic X.509 object
-*/
-X509_Object::X509_Object(const std::vector<uint8_t>& vec, const std::string& labels)
-   {
-   DataSource_Memory stream(vec.data(), vec.size());
-   init(stream, labels);
-   }
-
-/*
 * Read a PEM or BER X.509 object
 */
-void X509_Object::init(DataSource& in, const std::string& labels)
+void X509_Object::load_data(DataSource& in)
    {
-   m_PEM_labels_allowed = split_on(labels, '/');
-   if(m_PEM_labels_allowed.size() < 1)
-      throw Invalid_Argument("Bad labels argument to X509_Object");
-
-   m_PEM_label_pref = m_PEM_labels_allowed[0];
-   std::sort(m_PEM_labels_allowed.begin(), m_PEM_labels_allowed.end());
-
    try {
       if(ASN1::maybe_BER(in) && !PEM_Code::matches(in))
          {
@@ -97,9 +62,21 @@ void X509_Object::init(DataSource& in, const std::string& labels)
          std::string got_label;
          DataSource_Memory ber(PEM_Code::decode(in, got_label));
 
-         if(!std::binary_search(m_PEM_labels_allowed.begin(),
-                                m_PEM_labels_allowed.end(), got_label))
-            throw Decoding_Error("Invalid PEM label: " + got_label);
+         if(got_label != PEM_label())
+            {
+            bool is_alternate = false;
+            for(std::string alt_label : alternate_PEM_labels())
+               {
+               if(got_label == alt_label)
+                  {
+                  is_alternate = true;
+                  break;
+                  }
+               }
+
+            if(!is_alternate)
+               throw Decoding_Error("Unexpected PEM label for " + PEM_label() + " of " + got_label);
+            }
 
          BER_Decoder dec(ber);
          decode_from(dec);
@@ -107,7 +84,7 @@ void X509_Object::init(DataSource& in, const std::string& labels)
       }
    catch(Decoding_Error& e)
       {
-      throw Decoding_Error(m_PEM_label_pref + " decoding failed: " + e.what());
+      throw Decoding_Error(PEM_label() + " decoding failed: " + e.what());
       }
    }
 
@@ -135,6 +112,8 @@ void X509_Object::decode_from(BER_Decoder& from)
          .decode(m_sig_algo)
          .decode(m_sig, BIT_STRING)
       .end_cons();
+
+   force_decode();
    }
 
 /*
@@ -152,7 +131,7 @@ std::vector<uint8_t> X509_Object::BER_encode() const
 */
 std::string X509_Object::PEM_encode() const
    {
-   return PEM_Code::encode(BER_encode(), m_PEM_label_pref);
+   return PEM_Code::encode(BER_encode(), PEM_label());
    }
 
 /*
@@ -199,7 +178,7 @@ std::string X509_Object::hash_used_for_signature() const
 bool X509_Object::check_signature(const Public_Key* pub_key) const
    {
    if(!pub_key)
-      throw Exception("No key provided for " + m_PEM_label_pref + " signature check");
+      throw Exception("No key provided for " + PEM_label() + " signature check");
    std::unique_ptr<const Public_Key> key(pub_key);
    return check_signature(*key);
    }
@@ -280,31 +259,15 @@ std::vector<uint8_t> X509_Object::make_signed(PK_Signer* signer,
                                             const AlgorithmIdentifier& algo,
                                             const secure_vector<uint8_t>& tbs_bits)
    {
+   const std::vector<uint8_t> signature = signer->sign_message(tbs_bits, rng);
+
    return DER_Encoder()
       .start_cons(SEQUENCE)
          .raw_bytes(tbs_bits)
          .encode(algo)
-         .encode(signer->sign_message(tbs_bits, rng), BIT_STRING)
+         .encode(signature, BIT_STRING)
       .end_cons()
    .get_contents_unlocked();
    }
 
-/*
-* Try to decode the actual information
-*/
-void X509_Object::do_decode()
-   {
-   try {
-      force_decode();
-      }
-   catch(Decoding_Error& e)
-      {
-      throw Decoding_Error(m_PEM_label_pref + " decoding failed", e.what());
-      }
-   catch(Invalid_Argument& e)
-      {
-      throw Decoding_Error(m_PEM_label_pref + " decoding failed", e.what());
-      }
-   }
-
 }
diff --git a/src/lib/x509/x509_obj.h b/src/lib/x509/x509_obj.h
index 720cd1a39..c9ff0f761 100644
--- a/src/lib/x509/x509_obj.h
+++ b/src/lib/x509/x509_obj.h
@@ -18,8 +18,8 @@ class Public_Key;
 class RandomNumberGenerator;
 
 /**
-* This class represents abstract X.509 signed objects as
-* in the X.500 SIGNED macro
+* This class represents abstract X.509 signed objects as in the X.500
+* SIGNED macro
 */
 class BOTAN_PUBLIC_API(2,0) X509_Object : public ASN1_Object
    {
@@ -102,26 +102,28 @@ class BOTAN_PUBLIC_API(2,0) X509_Object : public ASN1_Object
 
       X509_Object(const X509_Object&) = default;
       X509_Object& operator=(const X509_Object&) = default;
+
+      virtual std::string PEM_label() const = 0;
+
+      virtual std::vector<std::string> alternate_PEM_labels() const
+         { return std::vector<std::string>(); }
+
       virtual ~X509_Object() = default;
    protected:
-      X509_Object(DataSource& src, const std::string& pem_labels);
-      X509_Object(const std::vector<uint8_t>& vec, const std::string& labels);
-
-#if defined(BOTAN_TARGET_OS_HAS_FILESYSTEM)
-      X509_Object(const std::string& file, const std::string& pem_labels);
-#endif
 
-      void do_decode();
       X509_Object() = default;
 
+      /**
+      * Decodes from src as either DER or PEM data, then calls force_decode()
+      */
+      void load_data(DataSource& src);
+
    private:
       virtual void force_decode() = 0;
-      void init(DataSource&, const std::string&);
 
       AlgorithmIdentifier m_sig_algo;
-      std::vector<uint8_t> m_tbs_bits, m_sig;
-      std::vector<std::string> m_PEM_labels_allowed;
-      std::string m_PEM_label_pref;
+      std::vector<uint8_t> m_tbs_bits;
+      std::vector<uint8_t> m_sig;
    };
 
 }
diff --git a/src/lib/x509/x509cert.cpp b/src/lib/x509/x509cert.cpp
index ca1fe8e3c..acd6b3362 100644
--- a/src/lib/x509/x509cert.cpp
+++ b/src/lib/x509/x509cert.cpp
@@ -60,32 +60,32 @@ struct X509_Certificate_Data
    Data_Store m_issuer_ds;
    };
 
-/*
-* X509_Certificate Constructor
-*/
-X509_Certificate::X509_Certificate(DataSource& in) :
-   X509_Object(in, "CERTIFICATE/X509 CERTIFICATE")
+std::string X509_Certificate::PEM_label() const
    {
-   do_decode();
+   return "CERTIFICATE";
    }
 
-/*
-* X509_Certificate Constructor
-*/
-X509_Certificate::X509_Certificate(const std::vector<uint8_t>& in) :
-   X509_Object(in, "CERTIFICATE/X509 CERTIFICATE")
+std::vector<std::string> X509_Certificate::alternate_PEM_labels() const
+   {
+   return { "X509 CERTIFICATE" };
+   }
+
+X509_Certificate::X509_Certificate(DataSource& src)
+   {
+   load_data(src);
+   }
+
+X509_Certificate::X509_Certificate(const std::vector<uint8_t>& vec)
    {
-   do_decode();
+   DataSource_Memory src(vec.data(), vec.size());
+   load_data(src);
    }
 
 #if defined(BOTAN_TARGET_OS_HAS_FILESYSTEM)
-/*
-* X509_Certificate Constructor
-*/
-X509_Certificate::X509_Certificate(const std::string& fsname) :
-   X509_Object(fsname, "CERTIFICATE/X509 CERTIFICATE")
+X509_Certificate::X509_Certificate(const std::string& fsname)
    {
-   do_decode();
+   DataSource_Stream src(fsname, true);
+   load_data(src);
    }
 #endif
 
@@ -302,7 +302,7 @@ const X509_Certificate_Data& X509_Certificate::data() const
    {
    if(m_data == nullptr)
       {
-      throw Decoding_Error("Failed to parse X509 certificate");
+      throw Invalid_State("X509_Certificate uninitialized");
       }
    return *m_data.get();
    }
diff --git a/src/lib/x509/x509cert.h b/src/lib/x509/x509cert.h
index 3b35c6575..dc32e70c1 100644
--- a/src/lib/x509/x509cert.h
+++ b/src/lib/x509/x509cert.h
@@ -395,15 +395,22 @@ class BOTAN_PUBLIC_API(2,0) X509_Certificate : public X509_Object
       */
       explicit X509_Certificate(const std::vector<uint8_t>& in);
 
+      /**
+      * Create an uninitialized certificate object. Any attempts to
+      * access this object will throw an exception.
+      */
+      X509_Certificate() = default;
+
       X509_Certificate(const X509_Certificate& other) = default;
 
       X509_Certificate& operator=(const X509_Certificate& other) = default;
 
    private:
-      void force_decode() override;
-      friend class X509_CA;
+      std::string PEM_label() const override;
 
-      X509_Certificate() = default;
+      std::vector<std::string> alternate_PEM_labels() const override;
+
+      void force_decode() override;
 
       const X509_Certificate_Data& data() const;
 
diff --git a/src/tests/data/x509/misc/cert_seq.der b/src/tests/data/x509/misc/cert_seq.der
new file mode 100644
index 000000000..4d93ccaff
--- /dev/null
+++ b/src/tests/data/x509/misc/cert_seq.der
diff --git a/src/tests/unit_x509.cpp b/src/tests/unit_x509.cpp
index 72c497ca7..da5439a05 100644
--- a/src/tests/unit_x509.cpp
+++ b/src/tests/unit_x509.cpp
@@ -755,6 +755,41 @@ Test::Result test_self_issued(const std::string& sig_algo, const std::string& ha
    return result;
    }
 
+Test::Result test_x509_uninit()
+   {
+   Test::Result result("X509 object uninitialized access");
+
+   Botan::X509_Certificate cert;
+   result.test_throws("uninitialized cert access causes exception",
+                      "X509_Certificate uninitialized",
+                      [&cert]() { cert.x509_version(); });
+
+   Botan::X509_CRL crl;
+   result.test_throws("uninitialized crl access causes exception",
+                      "X509_CRL uninitialized",
+                      [&crl]() { crl.crl_number(); });
+
+   return result;
+   }
+
+Test::Result test_x509_decode_list()
+   {
+   Test::Result result("X509_Certificate list decode");
+
+   Botan::DataSource_Stream input(Test::data_file("x509/misc/cert_seq.der"), true);
+
+   Botan::BER_Decoder dec(input);
+   std::vector<Botan::X509_Certificate> certs;
+   dec.decode_list(certs);
+
+   result.test_eq("Expected number of certs in list", certs.size(), 2);
+
+   result.test_eq("Expected cert 1 CN", certs[0].subject_dn().get_first_attribute("CN"), "CA1-PP.01.02");
+   result.test_eq("Expected cert 2 CN", certs[1].subject_dn().get_first_attribute("CN"), "User1-PP.01.02");
+
+   return result;
+   }
+
 
 using Botan::Key_Constraints;
 
@@ -1250,6 +1285,8 @@ class X509_Cert_Unit_Tests final : public Test
          results.push_back(test_x509_utf8());
          results.push_back(test_x509_bmpstring());
          results.push_back(test_crl_dn_name());
+         results.push_back(test_x509_uninit());
+         results.push_back(test_x509_decode_list());
 
          return results;
          }