Rename some methods, add more tests and docs

4 files changed, 80 insertions, 32 deletions

diff --git a/src/lib/x509/pkcs10.cpp b/src/lib/x509/pkcs10.cpp
index bac220277..22508f131 100644
--- a/src/lib/x509/pkcs10.cpp
+++ b/src/lib/x509/pkcs10.cpp
@@ -171,7 +171,7 @@ AlternativeName PKCS10_Request::subject_alt_name() const
 */
 Key_Constraints PKCS10_Request::constraints() const
    {
-   if(Certificate_Extension* ext = m_extensions.get(OIDS::lookup("X509v3.KeyUsage")))
+   if(auto ext = m_extensions.get(OIDS::lookup("X509v3.KeyUsage")))
       {
       return dynamic_cast<Cert_Extension::Key_Usage&>(*ext).get_constraints();
       }
@@ -184,7 +184,7 @@ Key_Constraints PKCS10_Request::constraints() const
 */
 std::vector<OID> PKCS10_Request::ex_constraints() const
    {
-   if(Certificate_Extension* ext = m_extensions.get(OIDS::lookup("X509v3.ExtendedKeyUsage")))
+   if(auto ext = m_extensions.get(OIDS::lookup("X509v3.ExtendedKeyUsage")))
       {
       return dynamic_cast<Cert_Extension::Extended_Key_Usage&>(*ext).get_oids();
       }
@@ -197,7 +197,7 @@ std::vector<OID> PKCS10_Request::ex_constraints() const
 */
 bool PKCS10_Request::is_CA() const
    {
-   if(Certificate_Extension* ext = m_extensions.get(OIDS::lookup("X509v3.BasicConstraints")))
+   if(auto ext = m_extensions.get(OIDS::lookup("X509v3.BasicConstraints")))
       {
       return dynamic_cast<Cert_Extension::Basic_Constraints&>(*ext).get_is_ca();
       }
@@ -210,7 +210,7 @@ bool PKCS10_Request::is_CA() const
 */
 size_t PKCS10_Request::path_limit() const
    {
-   if(Certificate_Extension* ext = m_extensions.get(OIDS::lookup("X509v3.BasicConstraints")))
+   if(auto ext = m_extensions.get(OIDS::lookup("X509v3.BasicConstraints")))
       {
       Cert_Extension::Basic_Constraints& basic_constraints = dynamic_cast<Cert_Extension::Basic_Constraints&>(*ext);
       if(basic_constraints.get_is_ca())
diff --git a/src/lib/x509/x509_ext.cpp b/src/lib/x509/x509_ext.cpp
index f475c50c2..9ef14e88d 100644
--- a/src/lib/x509/x509_ext.cpp
+++ b/src/lib/x509/x509_ext.cpp
@@ -22,7 +22,7 @@ namespace Botan {
 /*
 * List of X.509 Certificate Extensions
 */
-Certificate_Extension* Extensions::get_extension(const OID& oid, bool critical)
+Certificate_Extension* Extensions::create_extension(const OID& oid, bool critical)
    {
 #define X509_EXTENSION(NAME, TYPE) \
    if(oid == OIDS::lookup(NAME)) { return new Cert_Extension::TYPE(); }
@@ -123,13 +123,13 @@ void Extensions::replace(Certificate_Extension* extn, bool critical)
    m_extensions_raw[extn->oid_of()] = std::make_pair(extn->encode_inner(), critical);
    }
 
-Certificate_Extension* Extensions::get(const OID& oid) const
+std::unique_ptr<Certificate_Extension> Extensions::get(const OID& oid) const
    {
    for(auto& ext : m_extensions)
       {
       if(ext.first->oid_of() == oid)
          {
-         return ext.first.get();
+         return std::unique_ptr<Certificate_Extension>(ext.first->copy());
          }
       }
 
@@ -224,7 +224,7 @@ void Extensions::decode_from(BER_Decoder& from_source)
 
       m_extensions_raw.emplace(oid, std::make_pair(value, critical));
 
-      std::unique_ptr<Certificate_Extension> ext(get_extension(oid, critical));
+      std::unique_ptr<Certificate_Extension> ext(create_extension(oid, critical));
 
       if(!ext && critical && m_throw_on_unknown_critical)
          throw Decoding_Error("Encountered unknown X.509 extension marked "
diff --git a/src/lib/x509/x509_ext.h b/src/lib/x509/x509_ext.h
index ee7589ea5..1657613e7 100644
--- a/src/lib/x509/x509_ext.h
+++ b/src/lib/x509/x509_ext.h
@@ -88,16 +88,40 @@ class BOTAN_DLL Extensions : public ASN1_Object
    public:
       void encode_into(class DER_Encoder&) const override;
       void decode_from(class BER_Decoder&) override;
-
       void contents_to(Data_Store&, Data_Store&) const;
 
+      /**
+      * Adds a new extension to the list.
+      * @param extn the certificate extension
+      * @param critical whether this extension should be marked as critical
+      * @throw Invalid_Argument if the extension is already present in the list
+      */
       void add(Certificate_Extension* extn, bool critical = false);
+
+      /**
+      * Adds an extension to the list or replaces it.
+      * @param extn the certificate extension
+      * @param critical whether this extension should be marked as critical
+      */
       void replace(Certificate_Extension* extn, bool critical = false);
 
-      Certificate_Extension* get(const OID& oid) const;
+      /**
+      * Searches for an extension by OID and returns the result.
+      * Only the known extensions types declared in this header
+      * are searched for by this function.
+      * @return Pointer to extension with oid, nullptr if not found.
+      */
+      std::unique_ptr<Certificate_Extension> get(const OID& oid) const;
 
+      /**
+      * Searches for an extension by OID and returns the result.
+      * Only the unknown extensions, that is, extensions
+      * types that are not declared in this header, are searched
+      * for by this function.
+      * @return Pointer to extension with oid, nullptr if not found.
+      */
       template<typename T>
-      std::unique_ptr<T> get_extension(const OID& oid)
+      std::unique_ptr<T> get_raw(const OID& oid)
       {
       try
          {
@@ -116,18 +140,32 @@ class BOTAN_DLL Extensions : public ASN1_Object
       return nullptr;
       }
 
+      /**
+      * Returns the list of extensions together with the corresponding
+      * criticality flag. Only contains the known extensions
+      * types declared in this header.
+      */
       std::vector<std::pair<std::unique_ptr<Certificate_Extension>, bool>> extensions() const;
 
+      /**
+      * Returns the list of extensions as raw, encoded bytes
+      * together with the corresponding criticality flag.
+      * Contains all extensions, known as well as unknown extensions.
+      */
       std::map<OID, std::pair<std::vector<byte>, bool>> extensions_raw() const;
 
       Extensions& operator=(const Extensions&);
 
       Extensions(const Extensions&);
 
+      /**
+      * @param st whether to throw an exception when encountering an unknown
+      * extension type during decoding
+      */
       explicit Extensions(bool st = true) : m_throw_on_unknown_critical(st) {}
 
    private:
-      static Certificate_Extension* get_extension(const OID&, bool);
+      static Certificate_Extension* create_extension(const OID&, bool);
 
       std::vector<std::pair<std::unique_ptr<Certificate_Extension>, bool>> m_extensions;
       bool m_throw_on_unknown_critical;
diff --git a/src/tests/unit_x509.cpp b/src/tests/unit_x509.cpp
index ceed26c75..56a6e8b82 100644
--- a/src/tests/unit_x509.cpp
+++ b/src/tests/unit_x509.cpp
@@ -19,6 +19,7 @@
 #include <botan/pk_algs.h>
 #include <botan/ber_dec.h>
 #include <botan/der_enc.h>
+#include <botan/oids.h>
 
 #endif
 
@@ -718,11 +719,12 @@ Test::Result test_valid_constraints(const std::string& pk_algo)
 class String_Extension : public Botan::Certificate_Extension
    {
    public:
-      String_Extension(const Botan::OID& oid, const std::string& val) : m_oid(oid), m_contents(val) {}
+      String_Extension() : m_contents() {}
+      String_Extension(const std::string& val) : m_contents(val) {}
 
       std::string value() const { return m_contents; }
 
-      String_Extension* copy() const override { return new String_Extension(m_oid, m_contents); }
+      String_Extension* copy() const override { return new String_Extension(m_contents); }
 
       Botan::OID oid_of() const override { return m_oid; }
       std::string oid_name() const override { return "String Extension"; }
@@ -742,7 +744,7 @@ class String_Extension : public Botan::Certificate_Extension
       }
 
    private:
-      Botan::OID m_oid;
+      Botan::OID m_oid {"1.2.3.4.5.6.7.8.9.1"};
       std::string m_contents;
    };
 
@@ -780,22 +782,25 @@ Test::Result test_x509_extensions(const std::string& sig_algo, const std::string
    // include a custom extension in the request
    Botan::Extensions req_extensions;
    Botan::OID oid("1.2.3.4.5.6.7.8.9.1");
-   req_extensions.add(new String_Extension(oid, "1Test"), false);
+   req_extensions.add(new String_Extension("1Test"), false);
    opts.extensions = req_extensions;
 
    /* Create a self-signed certificate */
    Botan::X509_Certificate self_signed_cert = Botan::X509::create_self_signed_cert(opts, *user_key, hash_fn, Test::rng());
 
-   // check if custom extension is present in cert
-   // it would be nice if we could use v3_extensions().extensions() instead
-   auto ext_raw = self_signed_cert.v3_extensions().extensions_raw();
-   if(result.confirm("Custom extension present in self-signed certificate", ext_raw.count(oid) > 0))
+   // check if known Key_Usage extension is present in self-signed cert
+   auto key_usage_ext = self_signed_cert.v3_extensions().get(Botan::OIDS::lookup("X509v3.KeyUsage"));
+   if(result.confirm("Key_Usage extension present in self-signed certificate", key_usage_ext != nullptr))
       {
-      std::vector<byte> in = ext_raw.at(oid).first;
-      String_Extension ext(oid, "");
-      ext.decode_inner(in);
+      result.confirm("Key_Usage extension value matches in self-signed certificate",
+            dynamic_cast<Botan::Cert_Extension::Key_Usage&>(*key_usage_ext).get_constraints() == opts.constraints);
+      }
 
-      result.confirm("Custom extension value matches in self-signed certificate", ext.value() == "1Test");
+   // check if custom extension is present in self-signed cert
+   auto string_ext = self_signed_cert.v3_extensions().get_raw<String_Extension>(oid);
+   if(result.confirm("Custom extension present in self-signed certificate", string_ext != nullptr))
+      {
+      result.test_eq("Custom extension value matches in self-signed certificate", string_ext->value(), "1Test");
       }
 
 
@@ -811,15 +816,20 @@ Test::Result test_x509_extensions(const std::string& sig_algo, const std::string
                       from_date(2008, 01, 01),
                       from_date(2033, 01, 01));
 
-   ext_raw = user_cert.v3_extensions().extensions_raw();
-   if(result.confirm("Custom extension present in user certificate", ext_raw.count(oid) > 0))
-   {
-      std::vector<byte> in = ext_raw.at(oid).first;
-      String_Extension ext(oid, "");
-      ext.decode_inner(in);
+   // check if known Key_Usage extension is present in CA-signed cert
+   key_usage_ext = self_signed_cert.v3_extensions().get(Botan::OIDS::lookup("X509v3.KeyUsage"));
+   if(result.confirm("Key_Usage extension present in user certificate", key_usage_ext != nullptr))
+      {
+      result.confirm("Key_Usage extension value matches in user certificate",
+            dynamic_cast<Botan::Cert_Extension::Key_Usage&>(*key_usage_ext).get_constraints() == Botan::DIGITAL_SIGNATURE);
+      }
 
-      result.confirm("Custom extension value matches in user certificate", ext.value() == "1Test");
-   }
+   // check if custom extension is present in CA-signed cert
+   string_ext = user_cert.v3_extensions().get_raw<String_Extension>(oid);
+   if(result.confirm("Custom extension present in user certificate", string_ext != nullptr))
+      {
+      result.test_eq("Custom extension value matches in user certificate", string_ext->value(), "1Test");
+      }
 
    return result;
    }