Repository: randombit/botan
Pull request #4034 (2024-04-26 21:22:46)

Refactor AlternativeName internals
This provides a completely new and rather more sensible interface, with the old (now deprecated) interface being implemented in terms of the new interface.

It also resolves a performance issue when parsing certificates with many (thousands) of alternative names; previously this triggered some accidentally quadratic code when checking for duplicates.

Performance issue was reported by Bing Shi.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/67168980/badge)](https://coveralls.io/builds/67168980)

coverage: 91.99% (-0.03%) from 92.024%
when pulling **eaf35a1c47ff704db08416104da2b262e40c1e96 on jack/refactor-alt-name**
into **4af65b1a4bf8ec1e366e90f95cc0b84fb9926ff4 on master**.


Patch:
diff --git a/src/lib/asn1/asn1_obj.h b/src/lib/asn1/asn1_obj.h
index 84e05396a..c3264cb4d 100644
--- a/src/lib/asn1/asn1_obj.h
+++ b/src/lib/asn1/asn1_obj.h
@@ -425,6 +425,8 @@ class BOTAN_PUBLIC_API(2, 0) ASN1_String final : public ASN1_Object {
 
       bool operator==(const ASN1_String& other) const { return value() == other.value(); }
 
+      friend bool operator<(const ASN1_String& a, const ASN1_String& b) { return a.value() < b.value(); }
+
       explicit ASN1_String(std::string_view utf8 = "");
       ASN1_String(std::string_view utf8, ASN1_Type tag);
 
diff --git a/src/lib/tls/tls12/tls_channel_impl_12.cpp b/src/lib/tls/tls12/tls_channel_impl_12.cpp
index 98aea66e0..5adcc7de7 100644
--- a/src/lib/tls/tls12/tls_channel_impl_12.cpp
+++ b/src/lib/tls/tls12/tls_channel_impl_12.cpp
@@ -316,11 +316,6 @@ size_t Channel_Impl_12::from_peer(std::span<const uint8_t> data) {
          BOTAN_ASSERT_IMPLICATION(epoch0_restart, allow_epoch0_restart, "Allowed state");
 
          const bool initial_record = epoch0_restart || (!pending_state() && !active_state());
-         bool initial_handshake_message = false;
-         if(record.type() == Record_Type::Handshake && !m_record_buf.empty()) {
-            Handshake_Type type = static_cast<Handshake_Type>(m_record_buf[0]);
-            initial_handshake_message = (type == Handshake_Type::ClientHello);
-         }
 
          if(record.type() != Record_Type::Alert) {
             if(initial_record) {
@@ -329,12 +324,11 @@ size_t Channel_Impl_12::from_peer(std::span<const uint8_t> data) {
                   throw TLS_Exception(Alert::ProtocolVersion, "Received unexpected record version in initial record");
                }
             } else if(auto pending = pending_state()) {
-               if(pending->server_hello() != nullptr && !initial_handshake_message &&
-                  record.version() != pending->version()) {
+               if(pending->server_hello() != nullptr && record.version() != pending->version()) {
                   throw TLS_Exception(Alert::ProtocolVersion, "Received unexpected record version");
                }
             } else if(auto active = active_state()) {
-               if(record.version() != active->version() && !initial_handshake_message) {
+               if(record.version() != active->version()) {
                   throw TLS_Exception(Alert::ProtocolVersion, "Received unexpected record version");
                }
             }
diff --git a/src/lib/x509/alt_name.cpp b/src/lib/x509/alt_name.cpp
new file mode 100644
index 000000000..66f60a03c
--- /dev/null
+++ b/src/lib/x509/alt_name.cpp
@@ -0,0 +1,170 @@
+/*
+* (C) 2024 Jack Lloyd
+*
+* Botan is released under the Simplified BSD License (see license.txt)
+*/
+
+#include <botan/pkix_types.h>
+
+#include <botan/ber_dec.h>
+#include <botan/der_enc.h>
+#include <botan/internal/loadstor.h>
+#include <botan/internal/parsing.h>
+
+namespace Botan {
+
+void AlternativeName::add_uri(std::string_view uri) {
+   if(!uri.empty()) {
+      m_uri.insert(std::string(uri));
+   }
+}
+
+void AlternativeName::add_email(std::string_view addr) {
+   if(!addr.empty()) {
+      m_email.insert(std::string(addr));
+   }
+}
+
+void AlternativeName::add_dns(std::string_view dns) {
+   if(!dns.empty()) {
+      m_dns.insert(std::string(dns));
+   }
+}
+
+void AlternativeName::add_other_name(const OID& oid, const ASN1_String& value) {
+   m_othernames.insert(std::make_pair(oid, value));
+}
+
+void AlternativeName::add_dn(const X509_DN& dn) {
+   m_dn_names.insert(dn);
+}
+
+void AlternativeName::add_ip_address(std::string_view ip) {
+   if(!ip.empty()) {
+      m_ip_addr.insert(std::string(ip));
+   }
+}
+
+bool AlternativeName::has_items() const {
+   if(!this->dns().empty()) {
+      return true;
+   }
+   if(!this->uris().empty()) {
+      return true;
+   }
+   if(!this->email().empty()) {
+      return true;
+   }
+   if(!this->ip_address().empty()) {
+      return true;
+   }
+   if(!this->directory_names().empty()) {
+      return true;
+   }
+   if(!this->other_names().empty()) {
+      return true;
+   }
+   return false;
+}
+
+void AlternativeName::encode_into(DER_Encoder& der) const {
+   der.start_sequence();
+
+   /*
+   GeneralName ::= CHOICE {
+        otherName                       [0]     OtherName,
+        rfc822Name                      [1]     IA5String,
+        dNSName                         [2]     IA5String,
+        x400Address                     [3]     ORAddress,
+        directoryName                   [4]     Name,
+        ediPartyName                    [5]     EDIPartyName,
+        uniformResourceIdentifier       [6]     IA5String,
+        iPAddress                       [7]     OCTET STRING,
+        registeredID                    [8]     OBJECT IDENTIFIER }
+   */
+
+   for(const auto& othername : m_othernames) {
+      der.start_explicit(0)
+         .encode(othername.first)
+         .start_explicit(0)
+         .encode(othername.second)
+         .end_explicit()
+         .end_explicit();
+   }
+
+   for(const auto& name : m_email) {
+      ASN1_String str(name, ASN1_Type::Ia5String);
+      der.add_object(ASN1_Type(1), ASN1_Class::ContextSpecific, str.value());
+   }
+
+   for(const auto& name : m_dns) {
+      ASN1_String str(name, ASN1_Type::Ia5String);
+      der.add_object(ASN1_Type(2), ASN1_Class::ContextSpecific, str.value());
+   }
+
+   for(const auto& name : m_dn_names) {
+      der.add_object(ASN1_Type(4), ASN1_Class::ExplicitContextSpecific, name.DER_encode());
+   }
+
+   for(const auto& name : m_uri) {
+      ASN1_String str(name, ASN1_Type::Ia5String);
+      der.add_object(ASN1_Type(6), ASN1_Class::ContextSpecific, str.value());
+   }
+
+   for(const auto& ip : m_ip_addr) {
+      auto ip_buf = store_be(string_to_ipv4(ip));
+      der.add_object(ASN1_Type(7), ASN1_Class::ContextSpecific, ip_buf.data(), 4);
+   }
+
+   der.end_cons();
+}
+
+void AlternativeName::decode_from(BER_Decoder& source) {
+   BER_Decoder names = source.start_sequence();
+
+   while(names.more_items()) {
+      BER_Object obj = names.get_next_object();
+
+      if(obj.is_a(0, ASN1_Class::ContextSpecific)) {
+         BER_Decoder othername(obj);
+
+         OID oid;
+         othername.decode(oid);
+         if(othername.more_items()) {
+            BER_Object othername_value_outer = othername.get_next_object();
+            othername.verify_end();
+
+            if(!othername_value_outer.is_a(0, ASN1_Class::ExplicitContextSpecific)) {
+               throw Decoding_Error("Invalid tags on otherName value");
+            }
+
+            BER_Decoder othername_value_inner(othername_value_outer);
+
+            BER_Object value = othername_value_inner.get_next_object();
+            othername_value_inner.verify_end();
+
+            if(ASN1_String::is_string_type(value.type()) && value.get_class() == ASN1_Class::Universal) {
+               add_othername(oid, ASN1::to_string(value), value.type());
+            }
+         }
+      } else if(obj.is_a(1, ASN1_Class::ContextSpecific)) {
+         add_email(ASN1::to_string(obj));
+      } else if(obj.is_a(2, ASN1_Class::ContextSpecific)) {
+         add_dns(ASN1::to_string(obj));
+      } else if(obj.is_a(4, ASN1_Class::ContextSpecific | ASN1_Class::Constructed)) {
+         BER_Decoder dec(obj);
+         X509_DN dn;
+         dec.decode(dn);
+         this->add_dn(dn);
+      } else if(obj.is_a(6, ASN1_Class::ContextSpecific)) {
+         this->add_uri(ASN1::to_string(obj));
+      } else if(obj.is_a(7, ASN1_Class::ContextSpecific)) {
+         if(obj.length() == 4) {
+            const uint32_t ip = load_be<uint32_t>(obj.bits(), 0);
+            this->add_ip_address(ipv4_to_string(ip));
+         }
+      }
+   }
+}
+
+}  // namespace Botan
diff --git a/src/lib/x509/asn1_alt_name.cpp b/src/lib/x509/asn1_alt_name.cpp
index 8fff9145c..aa24b7d44 100644
--- a/src/lib/x509/asn1_alt_name.cpp
+++ b/src/lib/x509/asn1_alt_name.cpp
@@ -8,12 +8,7 @@
 
 #include <botan/pkix_types.h>
 
-#include <botan/ber_dec.h>
-#include <botan/der_enc.h>
-#include <botan/internal/loadstor.h>
-#include <botan/internal/parsing.h>
-#include <botan/internal/stl_util.h>
-
+#include <botan/internal/fmt.h>
 #include <sstream>
 
 namespace Botan {
@@ -25,10 +20,18 @@ AlternativeName::AlternativeName(std::string_view email_addr,
                                  std::string_view uri,
                                  std::string_view dns,
                                  std::string_view ip) {
-   add_attribute("RFC822", email_addr);
-   add_attribute("DNS", dns);
-   add_attribute("URI", uri);
-   add_attribute("IP", ip);
+   if(!email_addr.empty()) {
+      add_email(email_addr);
+   }
+   if(!dns.empty()) {
+      add_dns(dns);
+   }
+   if(!uri.empty()) {
+      add_uri(uri);
+   }
+   if(!ip.empty()) {
+      add_ip_address(ip);
+   }
 }
 
 /*
@@ -39,14 +42,22 @@ void AlternativeName::add_attribute(std::string_view type, std::string_view valu
       return;
    }
 
-   auto range = m_alt_info.equal_range(type);
-   for(auto j = range.first; j != range.second; ++j) {
-      if(j->second == value) {
-         return;
-      }
+   if(type == "DNS") {
+      this->add_dns(value);
+   } else if(type == "RFC822") {
+      this->add_email(value);
+   } else if(type == "URI") {
+      this->add_uri(value);
+   } else if(type == "DN") {
+      X509_DN dn;
+      std::istringstream ss{std::string(value)};
+      ss >> dn;
+      this->add_dn(dn);
+   } else if(type == "IP") {
+      this->add_ip_address(value);
+   } else {
+      throw Not_Implemented(fmt("Unknown AlternativeName name type {}", type));
    }
-
-   m_alt_info.emplace(type, value);
 }
 
 /*
@@ -56,7 +67,7 @@ void AlternativeName::add_othername(const OID& oid, std::string_view value, ASN1
    if(value.empty()) {
       return;
    }
-   m_othernames.emplace(oid, ASN1_String{value, type});
+   this->add_other_name(oid, ASN1_String(value, type));
 }
 
 /*
@@ -65,170 +76,99 @@ void AlternativeName::add_othername(const OID& oid, std::string_view value, ASN1
 std::multimap<std::string, std::string> AlternativeName::contents() const {
    std::multimap<std::string, std::string> names;
 
-   for(const auto& name : m_alt_info) {
-      names.emplace(name.first, name.second);
+   for(const auto& nm : this->dns()) {
+      names.emplace("DNS", nm);
+   }
+
+   for(const auto& nm : this->email()) {
+      names.emplace("RFC822", nm);
+   }
+
+   for(const auto& nm : this->uris()) {
+      names.emplace("URI", nm);
+   }
+
+   for(const auto& nm : this->ip_address()) {
+      names.emplace("IP", nm);
    }
 
-   for(const auto& othername : m_othernames) {
+   for(const auto& nm : this->directory_names()) {
+      names.emplace("DN", nm.to_string());
+   }
+
+   for(const auto& othername : this->other_names()) {
       names.emplace(othername.first.to_formatted_string(), othername.second.value());
    }
 
    return names;
 }
 
-bool AlternativeName::has_field(std::string_view attr) const {
-   auto range = m_alt_info.equal_range(attr);
-   return (range.first != range.second);
-}
+std::multimap<std::string, std::string, std::less<>> AlternativeName::get_attributes() const {
+   std::multimap<std::string, std::string, std::less<>> r;
 
-std::string AlternativeName::get_first_attribute(std::string_view attr) const {
-   auto i = m_alt_info.lower_bound(attr);
-   if(i != m_alt_info.end() && i->first == attr) {
-      return i->second;
+   for(const auto& c : this->contents()) {
+      r.emplace(c.first, c.second);
    }
 
-   return "";
+   return r;
 }
 
-std::vector<std::string> AlternativeName::get_attribute(std::string_view attr) const {
-   std::vector<std::string> results;
-   auto range = m_alt_info.equal_range(attr);
-   for(auto i = range.first; i != range.second; ++i) {
-      results.push_back(i->second);
-   }
-   return results;
+bool AlternativeName::has_field(std::string_view attr) const {
+   return !this->get_attribute(attr).empty();
 }
 
-X509_DN AlternativeName::dn() const {
-   X509_DN dn;
-   auto range = m_alt_info.equal_range("DN");
+std::string AlternativeName::get_first_attribute(std::string_view type) const {
+   auto attr = this->get_attribute(type);
 
-   for(auto i = range.first; i != range.second; ++i) {
-      std::istringstream strm(i->second);
-      strm >> dn;
+   if(!attr.empty()) {
+      return attr[0];
    }
 
-   return dn;
-}
-
-/*
-* Return if this object has anything useful
-*/
-bool AlternativeName::has_items() const {
-   return (!m_alt_info.empty() || !m_othernames.empty());
+   return "";
 }
 
-namespace {
-
-/*
-* DER encode an AlternativeName entry
-*/
-void encode_entries(DER_Encoder& encoder,
-                    const std::multimap<std::string, std::string, std::less<>>& attr,
-                    std::string_view type,
-                    ASN1_Type tagging) {
-   auto range = attr.equal_range(type);
-
-   for(auto i = range.first; i != range.second; ++i) {
-      if(type == "RFC822" || type == "DNS" || type == "URI") {
-         ASN1_String asn1_string(i->second, ASN1_Type::Ia5String);
-         encoder.add_object(tagging, ASN1_Class::ContextSpecific, asn1_string.value());
-      } else if(type == "IP") {
-         const uint32_t ip = string_to_ipv4(i->second);
-         uint8_t ip_buf[4] = {0};
-         store_be(ip, ip_buf);
-         encoder.add_object(tagging, ASN1_Class::ContextSpecific, ip_buf, 4);
-      } else if(type == "DN") {
-         std::stringstream ss(i->second);
-         X509_DN dn;
-         ss >> dn;
-         encoder.encode(dn);
+std::vector<std::string> AlternativeName::get_attribute(std::string_view attr) const {
+   auto set_to_vector = [](const std::set<std::string>& s) -> std::vector<std::string> { return {s.begin(), s.end()}; };
+
+   if(attr == "DNS") {
+      return set_to_vector(this->dns());
+   } else if(attr == "RFC822") {
+      return set_to_vector(this->email());
+   } else if(attr == "URI") {
+      return set_to_vector(this->uris());
+   } else if(attr == "DN") {
+      std::vector<std::string> ret;
+
+      for(const auto& nm : this->directory_names()) {
+         ret.push_back(nm.to_string());
       }
+
+      return ret;
+   } else if(attr == "IP") {
+      return set_to_vector(this->ip_address());
+   } else {
+      return {};
    }
 }
 
-}  // namespace
+X509_DN AlternativeName::dn() const {
+   // This logic really does not make any sense, but it is
+   // how this function was historically implemented.
 
-/*
-* DER encode an AlternativeName extension
-*/
-void AlternativeName::encode_into(DER_Encoder& der) const {
-   der.start_sequence();
+   X509_DN combined_dn;
 
-   encode_entries(der, m_alt_info, "RFC822", ASN1_Type(1));
-   encode_entries(der, m_alt_info, "DNS", ASN1_Type(2));
-   encode_entries(der, m_alt_info, "DN", ASN1_Type(4));
-   encode_entries(der, m_alt_info, "URI", ASN1_Type(6));
-   encode_entries(der, m_alt_info, "IP", ASN1_Type(7));
+   for(const auto& dn : this->directory_names()) {
+      std::ostringstream oss;
+      oss << dn;
 
-   for(const auto& othername : m_othernames) {
-      der.start_explicit(0)
-         .encode(othername.first)
-         .start_explicit(0)
-         .encode(othername.second)
-         .end_explicit()
-         .end_explicit();
+      std::istringstream iss(oss.str());
+      iss >> combined_dn;
    }
 
-   der.end_cons();
+   return combined_dn;
 }
 
 /*
-* Decode a BER encoded AlternativeName
+* Return if this object has anything useful
 */
-void AlternativeName::decode_from(BER_Decoder& source) {
-   BER_Decoder names = source.start_sequence();
-
-   // FIXME this is largely a duplication of GeneralName::decode_from
-
-   while(names.more_items()) {
-      BER_Object obj = names.get_next_object();
-
-      if(obj.is_a(0, ASN1_Class::ContextSpecific)) {
-         BER_Decoder othername(obj);
-
-         OID oid;
-         othername.decode(oid);
-         if(othername.more_items()) {
-            BER_Object othername_value_outer = othername.get_next_object();
-            othername.verify_end();
-
-            if(othername_value_outer.is_a(0, ASN1_Class::ExplicitContextSpecific) == false) {
-               throw Decoding_Error("Invalid tags on otherName value");
-            }
-
-            BER_Decoder othername_value_inner(othername_value_outer);
-
-            BER_Object value = othername_value_inner.get_next_object();
-            othername_value_inner.verify_end();
-
-            if(ASN1_String::is_string_type(value.type()) && value.get_class() == ASN1_Class::Universal) {
-               add_othername(oid, ASN1::to_string(value), value.type());
-            }
-         }
-      }
-      if(obj.is_a(1, ASN1_Class::ContextSpecific)) {
-         add_attribute("RFC822", ASN1::to_string(obj));
-      } else if(obj.is_a(2, ASN1_Class::ContextSpecific)) {
-         add_attribute("DNS", ASN1::to_string(obj));
-      } else if(obj.is_a(6, ASN1_Class::ContextSpecific)) {
-         add_attribute("URI", ASN1::to_string(obj));
-      } else if(obj.is_a(4, ASN1_Class::ContextSpecific | ASN1_Class::Constructed)) {
-         BER_Decoder dec(obj);
-         X509_DN dn;
-         std::stringstream ss;
-
-         dec.decode(dn);
-         ss << dn;
-
-         add_attribute("DN", ss.str());
-      } else if(obj.is_a(7, ASN1_Class::ContextSpecific)) {
-         if(obj.length() == 4) {
-            const uint32_t ip = load_be<uint32_t>(obj.bits(), 0);
-            add_attribute("IP", ipv4_to_string(ip));
-         }
-      }
-   }
-}
-
 }  // namespace Botan
diff --git a/src/lib/x509/name_constraint.cpp b/src/lib/x509/name_constraint.cpp
index 05c6bfa04..bccb14e6c 100644
--- a/src/lib/x509/name_constraint.cpp
+++ b/src/lib/x509/name_constraint.cpp
@@ -91,7 +91,7 @@ GeneralName::MatchResult GeneralName::matches(const X509_Certificate& cert) cons
       nam.push_back(dn.to_string());
 
       const auto alt_dn = alt_name.dn();
-      if(alt_dn.empty() == false) {
+      if(!alt_dn.empty()) {
          nam.push_back(alt_dn.to_string());
       }
    } else if(type() == "IP") {
diff --git a/src/lib/x509/pkcs10.cpp b/src/lib/x509/pkcs10.cpp
index cba49c232..dc8444f50 100644
--- a/src/lib/x509/pkcs10.cpp
+++ b/src/lib/x509/pkcs10.cpp
@@ -150,7 +150,7 @@ std::unique_ptr<PKCS10_Data> decode_pkcs10(const std::vector<uint8_t>& body) {
    }
 
    for(const auto& email : pkcs9_email) {
-      data->m_alt_name.add_attribute("RFC882", email);
+      data->m_alt_name.add_email(email);
    }
 
    return data;
diff --git a/src/lib/x509/pkix_types.h b/src/lib/x509/pkix_types.h
index ff52dbb43..e2d1444f9 100644
--- a/src/lib/x509/pkix_types.h
+++ b/src/lib/x509/pkix_types.h
@@ -118,32 +118,91 @@ class BOTAN_PUBLIC_API(2, 0) AlternativeName final : public ASN1_Object {
       void encode_into(DER_Encoder&) const override;
       void decode_from(BER_Decoder&) override;
 
+      /// Create an empty name
+      AlternativeName() {}
+
+      /// Add a URI to this AlternativeName
+      void add_uri(std::string_view uri);
+
+      /// Add a URI to this AlternativeName
+      void add_email(std::string_view addr);
+
+      /// Add a DNS name to this AlternativeName
+      void add_dns(std::string_view dns);
+
+      /// Add an "OtherName" identified by object identifier to this AlternativeName
+      void add_other_name(const OID& oid, const ASN1_String& value);
+
+      /// Add a directory name to this AlternativeName
+      void add_dn(const X509_DN& dn);
+
+      /// Add an IP address to this alternative name
+      ///
+      /// Note: currently only IPv4 is accepted
+      void add_ip_address(std::string_view ip_str);
+
+      /// Return the set of URIs included in this alternative name
+      const std::set<std::string>& uris() const { return m_uri; }
+
+      /// Return the set of email addresses included in this alternative name
+      const std::set<std::string>& email() const { return m_email; }
+
+      /// Return the set of DNS names included in this alternative name
+      const std::set<std::string>& dns() const { return m_dns; }
+
+      /// Return the set of IPv4 addresses included in this alternative name
+      const std::set<std::string>& ip_address() const { return m_ip_addr; }
+
+      /// Return the set of "other names" included in this alternative name
+      const std::set<std::pair<OID, ASN1_String>>& other_names() const { return m_othernames; }
+
+      /// Return the set of directory names included in this alternative name
+      const std::set<X509_DN>& directory_names() const { return m_dn_names; }
+
+      // Return true if this has any names set
+      bool has_items() const;
+
+      // Old, now deprecated interface follows:
+      BOTAN_DEPRECATED("Use AlternativeName::{uris, email, dns, othernames, directory_names}")
       std::multimap<std::string, std::string> contents() const;
 
+      BOTAN_DEPRECATED("Use AlternativeName::{uris, email, dns, othernames, directory_names}.empty()")
       bool has_field(std::string_view attr) const;
+
+      BOTAN_DEPRECATED("Use AlternativeName::{uris, email, dns, othernames, directory_names}")
       std::vector<std::string> get_attribute(std::string_view attr) const;
 
+      BOTAN_DEPRECATED("Use AlternativeName::{uris, email, dns, othernames, directory_names}")
+      std::multimap<std::string, std::string, std::less<>> get_attributes() const;
+
+      BOTAN_DEPRECATED("Use AlternativeName::{uris, email, dns, othernames, directory_names}")
       std::string get_first_attribute(std::string_view attr) const;
 
+      BOTAN_DEPRECATED("Use AlternativeName::add_{uri, dns, email, ...}")
       void add_attribute(std::string_view type, std::string_view value);
-      void add_othername(const OID& oid, std::string_view value, ASN1_Type type);
 
-      const std::multimap<std::string, std::string, std::less<>>& get_attributes() const { return m_alt_info; }
+      BOTAN_DEPRECATED("Use AlternativeName::add_other_name")
+      void add_othername(const OID& oid, std::string_view value, ASN1_Type type);
 
-      const std::multimap<OID, ASN1_String>& get_othernames() const { return m_othernames; }
+      BOTAN_DEPRECATED("Use AlternativeName::othernames")
+      std::multimap<OID, ASN1_String> get_othernames() const;
 
+      BOTAN_DEPRECATED("Use AlternativeName::directory_names")
       X509_DN dn() const;
 
-      bool has_items() const;
-
-      AlternativeName(std::string_view email_addr = "",
+      BOTAN_DEPRECATED("Use plain constructor plus add_{uri,dns,email,ip}")
+      AlternativeName(std::string_view email_addr,
                       std::string_view uri = "",
                       std::string_view dns = "",
                       std::string_view ip_address = "");
 
    private:
-      std::multimap<std::string, std::string, std::less<>> m_alt_info;
-      std::multimap<OID, ASN1_String> m_othernames;
+      std::set<std::string> m_dns;
+      std::set<std::string> m_uri;
+      std::set<std::string> m_email;
+      std::set<std::string> m_ip_addr;
+      std::set<X509_DN> m_dn_names;
+      std::set<std::pair<OID, ASN1_String>> m_othernames;
 };
 
 /**
diff --git a/src/lib/x509/x509_ext.cpp b/src/lib/x509/x509_ext.cpp
index b77223c88..3ba900e51 100644
--- a/src/lib/x509/x509_ext.cpp
+++ b/src/lib/x509/x509_ext.cpp
@@ -758,10 +758,10 @@ void CRL_Distribution_Points::decode_inner(const std::vector<uint8_t>& buf) {
 }
 
 void CRL_Distribution_Points::Distribution_Point::encode_into(DER_Encoder& der) const {
-   const auto uris = m_point.get_attribute("URI");
+   const auto uris = m_point.uris();
 
    if(uris.empty()) {
-      throw Not_Implemented("Empty CRL_Distribution_Point encoding");
+      throw Not_Implemented("Empty CRL_Distribution_Point encoding not implemented");
    }
 
    for(const auto& uri : uris) {
diff --git a/src/lib/x509/x509cert.cpp b/src/lib/x509/x509cert.cpp
index 673062229..c2de98ece 100644
--- a/src/lib/x509/x509cert.cpp
+++ b/src/lib/x509/x509cert.cpp
@@ -510,38 +510,40 @@ const AlternativeName& X509_Certificate::issuer_alt_name() const {
    return data().m_issuer_alt_name;
 }
 
-/*
-* Return information about the subject
-*/
-std::vector<std::string> X509_Certificate::subject_info(std::string_view req) const {
-   if(req == "Email") {
-      return this->subject_info("RFC822");
-   }
+namespace {
 
-   if(subject_dn().has_field(req)) {
-      return subject_dn().get_attribute(req);
+std::vector<std::string> get_cert_user_info(std::string_view req, const X509_DN& dn, const AlternativeName& alt_name) {
+   auto set_to_vector = [](const std::set<std::string>& s) -> std::vector<std::string> { return {s.begin(), s.end()}; };
+
+   if(dn.has_field(req)) {
+      return dn.get_attribute(req);
+   } else if(req == "RFC822" || req == "Email") {
+      return set_to_vector(alt_name.email());
+   } else if(req == "DNS") {
+      return set_to_vector(alt_name.dns());
+   } else if(req == "URI") {
+      return set_to_vector(alt_name.uris());
+   } else if(req == "IP") {
+      return set_to_vector(alt_name.ip_address());
+   } else {
+      return {};
    }
+}
 
-   if(subject_alt_name().has_field(req)) {
-      return subject_alt_name().get_attribute(req);
-   }
+}  // namespace
 
-   return {};
+/*
+* Return information about the subject
+*/
+std::vector<std::string> X509_Certificate::subject_info(std::string_view req) const {
+   return get_cert_user_info(req, subject_dn(), subject_alt_name());
 }
 
 /*
 * Return information about the issuer
 */
 std::vector<std::string> X509_Certificate::issuer_info(std::string_view req) const {
-   if(issuer_dn().has_field(req)) {
-      return issuer_dn().get_attribute(req);
-   }
-
-   if(issuer_alt_name().has_field(req)) {
-      return issuer_alt_name().get_attribute(req);
-   }
-
-   return {};
+   return get_cert_user_info(req, issuer_dn(), issuer_alt_name());
 }
 
 /*
diff --git a/src/lib/x509/x509self.cpp b/src/lib/x509/x509self.cpp
index 65ea6c320..2a84001ff 100644
--- a/src/lib/x509/x509self.cpp
+++ b/src/lib/x509/x509self.cpp
@@ -50,14 +50,16 @@ auto create_alt_name_ext(const X509_Cert_Options& opts, Extensions& extensions)
       subject_alt = ext->get_alt_name();
    }
 
-   subject_alt.add_attribute("DNS", opts.dns);
-   subject_alt.add_attribute("URI", opts.uri);
-   subject_alt.add_attribute("RFC822", opts.email);
-   subject_alt.add_attribute("IP", opts.ip);
-   subject_alt.add_othername(OID::from_string("PKIX.XMPPAddr"), opts.xmpp, ASN1_Type::Utf8String);
-
-   for(const auto& dns : opts.more_dns) {
-      subject_alt.add_attribute("DNS", dns);
+   subject_alt.add_dns(opts.dns);
+   for(const auto& nm : opts.more_dns) {
+      subject_alt.add_dns(nm);
+   }
+   subject_alt.add_uri(opts.uri);
+   subject_alt.add_email(opts.email);
+   subject_alt.add_ip_address(opts.ip);
+
+   if(!opts.xmpp.empty()) {
+      subject_alt.add_other_name(OID::from_string("PKIX.XMPPAddr"), ASN1_String(opts.xmpp, ASN1_Type::Utf8String));
    }
 
    return std::make_unique<Cert_Extension::Subject_Alternative_Name>(subject_alt);


Test patch:
diff --git a/src/tests/unit_x509.cpp b/src/tests/unit_x509.cpp
index 95d9aec16..33e04bd61 100644
--- a/src/tests/unit_x509.cpp
+++ b/src/tests/unit_x509.cpp
@@ -827,17 +827,19 @@ Test::Result test_pkcs10_ext(const Botan::Private_Key& key,
    Botan::AlternativeName alt_name;
    alt_name.add_attribute("DNS", "bonus.example.org");
 
+   Botan::X509_DN alt_dn;
+   alt_dn.add_attribute("X520.CommonName", "alt_cn");
+   alt_dn.add_attribute("X520.Organization", "testing");
+   alt_name.add_dn(alt_dn);
+
    opts.extensions.add(std::make_unique<Botan::Cert_Extension::Subject_Alternative_Name>(alt_name));
 
-   Botan::PKCS10_Request req = Botan::X509::create_cert_req(opts, key, hash_fn, rng);
+   const auto req = Botan::X509::create_cert_req(opts, key, hash_fn, rng);
 
-   std::vector<std::string> alt_dns_names = req.subject_alt_name().get_attribute("DNS");
+   const auto alt_dns_names = req.subject_alt_name().get_attribute("DNS");
 
    result.test_eq("Expected number of DNS names", alt_dns_names.size(), 4);
 
-   // The order is not guaranteed so sort before comparing
-   std::sort(alt_dns_names.begin(), alt_dns_names.end());
-
    if(alt_dns_names.size() == 4) {
       result.test_eq("Expected DNS name 1", alt_dns_names.at(0), "bonus.example.org");
       result.test_eq("Expected DNS name 2", alt_dns_names.at(1), "main.example.org");
@@ -845,6 +847,9 @@ Test::Result test_pkcs10_ext(const Botan::Private_Key& key,
       result.test_eq("Expected DNS name 3", alt_dns_names.at(3), "more2.example.org");
    }
 
+   result.test_eq("Expected number of alt DNs", req.subject_alt_name().directory_names().size(), 1);
+   result.confirm("Alt DN is correct", *req.subject_alt_name().directory_names().begin() == alt_dn);
+
    return result;
 }
 
@@ -1362,7 +1367,8 @@ Test::Result test_x509_extensions(const Botan::Private_Key& ca_key,
    std::vector<Botan::Cert_Extension::CRL_Distribution_Points::Distribution_Point> dps;
 
    for(const auto& uri : cdp_urls) {
-      Botan::AlternativeName cdp_alt_name("", uri);
+      Botan::AlternativeName cdp_alt_name;
+      cdp_alt_name.add_uri(uri);
       Botan::Cert_Extension::CRL_Distribution_Points::Distribution_Point dp(cdp_alt_name);
 
       dps.emplace_back(dp);