Address review comments

Co-authored-by: René Meusel <[email protected]>
randombit · May 23, 2024 · cf82307 · cf82307
1 parent ec02630
commit cf82307
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 22 deletions.
diff --git a/src/lib/asn1/asn1_obj.h b/src/lib/asn1/asn1_obj.h
@@ -12,6 +12,7 @@
 #include <chrono>
 #include <iosfwd>
 #include <optional>
+#include <span>
 #include <string>
 #include <string_view>
 #include <unordered_map>
@@ -151,6 +152,8 @@ class BOTAN_PUBLIC_API(2, 0) BER_Object final {
 
       size_t length() const { return m_value.size(); }
 
+      std::span<const uint8_t> data() const { return std::span{m_value}; }
+
       void assert_is_a(ASN1_Type type_tag, ASN1_Class class_tag, std::string_view descr = "object") const;
 
       bool is_a(ASN1_Type type_tag, ASN1_Class class_tag) const;
@@ -219,7 +222,9 @@ class BOTAN_PUBLIC_API(2, 0) OID final : public ASN1_Object {
 
       /**
       * Construct an OID from a string.
-      * @param str a string in the form "a.b.c" etc., where a,b,c are numbers
+      * @param str a string in the form "a.b.c" etc., where a,b,c are integers
+      *
+      * Note: it is currently required that each integer fit into 32 bits
       */
       explicit OID(std::string_view str);
 
@@ -231,7 +236,7 @@ class BOTAN_PUBLIC_API(2, 0) OID final : public ASN1_Object {
       /**
       * Initialize an OID from a vector of integer values
       */
-      explicit OID(std::vector<uint32_t>&& init);
+      BOTAN_DEPRECATED("Use another contructor") explicit OID(std::vector<uint32_t>&& init);
 
       /**
       * Construct an OID from a string.

diff --git a/src/lib/asn1/asn1_oid.cpp b/src/lib/asn1/asn1_oid.cpp
@@ -105,7 +105,7 @@ OID::OID(std::initializer_list<uint32_t> init) : m_id(init) {
    oid_valid_check(m_id);
 }
 
-OID::OID(std::vector<uint32_t>&& init) : m_id(init) {
+OID::OID(std::vector<uint32_t>&& init) : m_id(std::move(init)) {
    oid_valid_check(m_id);
 }
 
@@ -214,46 +214,48 @@ void OID::decode_from(BER_Decoder& decoder) {
       throw BER_Decoding_Error("OID encoding is too short");
    }
 
-   auto consume = [](std::span<const uint8_t> data) -> std::pair<std::span<const uint8_t>, uint32_t> {
+   auto consume = [](BufferSlicer& data) -> uint32_t {
       BOTAN_ASSERT_NOMSG(!data.empty());
+      uint32_t b = data.take_byte();
 
-      uint32_t b = data.front();
-
-      if(b <= 0x7F) {
-         return std::make_pair(data.subspan(1), b);
-      } else {
+      if(b > 0x7F) {
          b &= 0x7F;
+
          // Even BER requires that the OID have minimal length, ie that
          // the first byte of a multibyte encoding cannot be zero
          // See X.690 section 8.19.2
          if(b == 0) {
             throw Decoding_Error("Leading zero byte in multibyte OID encoding");
          }
-         data = data.subspan(1);
-         while(!data.empty()) {
-            const auto next = data.front();
-            data = data.subspan(1);
+
+         while(true) {
+            if(data.empty()) {
+               throw Decoding_Error("Truncated OID value");
+            }
+
+            const uint8_t next = data.take_byte();
             const bool more = (next & 0x80);
+            const uint8_t value = next & 0x7F;
 
-            if(b >> (32 - 7)) {
+            if((b >> (32 - 7)) != 0) {
                throw Decoding_Error("OID component overflow");
             }
-            b <<= 7;
-            b |= (next & 0x7F);
+
+            b = (b << 7) | value;
 
             if(!more) {
-               return std::make_pair(data, b);
+               break;
             }
          }
-         throw Decoding_Error("Truncated OID value");
       }
+
+      return b;
    };
 
-   std::span<const uint8_t> span(obj.bits(), obj.length());
+   BufferSlicer data(obj.data());
    std::vector<uint32_t> parts;
-   while(!span.empty()) {
-      auto [subspan, comp] = consume(span);
-      span = subspan;
+   while(!data.empty()) {
+      const uint32_t comp = consume(data);
 
       if(parts.empty()) {
          // divide into root and second arc