simplify encode OID and catch parse errors

so that when the OID string is not an OID string, it fails
rather than encoding "something"

diff --git a/src/protocols/der/encode.c b/src/protocols/der/encode.c
index 23ec4f2deac8658612dfb2da9d85a6bc30f73ee2..870b83e46653d3f1b13afeb550878699b24c6847 100644 (file)
--- a/src/protocols/der/encode.c
+++ b/src/protocols/der/encode.c
@@ -453,12 +453,11 @@ static ssize_t fr_der_encode_null(UNUSED fr_dbuff_t *dbuff, fr_dcursor_t *cursor
 static ssize_t fr_der_encode_oid_to_str(fr_dbuff_t *dbuff, const char *oid_str)
 {
        fr_dbuff_t our_dbuff = FR_DBUFF(dbuff);
-       char     buffer[21] = { 0 };
-       uint64_t subidentifier         = 0;
-       uint8_t  first_component       = 0;
-       size_t   buffer_len            = 0;
-       size_t   index                 = 0, bit_index;
-       bool     started_subidentifier = false, subsequent = false;
+       bool    first = true;
+       uint8_t first_component;
+       unsigned long long oid;
+       char const *start = oid_str;
+       char     *end = NULL;
 
        /*
         *      The first subidentifier is the encoding of the first two object identifier components, encoded as:
@@ -467,84 +466,61 @@ static ssize_t fr_der_encode_oid_to_str(fr_dbuff_t *dbuff, const char *oid_str)
         *      The first number is 0, 1, or 2.
         */
 
-       first_component = (uint8_t)(strtol(&oid_str[0], NULL, 10));
-
-       index += 2; /* Advance past the first number and the delimiter '.' */
-
-       for (; index < strlen(oid_str) + 1; index++) {
-               uint8_t byte = 0;
-
-               if (oid_str[index] == '.' || oid_str[index] == '\0') {
-                       /*
-                        *      We have a subidentifier
-                        */
-                       started_subidentifier = false;
-                       bit_index             = sizeof(subidentifier) * 8;
-
-                       if (buffer_len == 0) {
-                               fr_strerror_const("Empty buffer for final subidentifier");
-                               return -1;
-                       }
-
-                       if (!subsequent) {
-                               subidentifier = (first_component * 40) + (uint64_t)strtol(buffer, NULL, 10);
-                               subsequent    = true;
-
-                       } else {
-                               subidentifier = (uint64_t)strtol(buffer, NULL, 10);
-                       }
-
-                       /*
-                        *      We will be reading the subidentifier 7 bits at a time. This is because the
-                        *      OID components are encoded in a variable length format, where the high bit
-                        *      of each byte indicates if there are more bytes to follow.
-                        */
-                       while (bit_index > 7) {
-                               if (!started_subidentifier && ((uint8_t)(subidentifier >> (bit_index - 8)) == 0)) {
-                                       bit_index -= 8;
-                                       continue;
-                               }
-
-                               if (!started_subidentifier) {
-                                       byte = (uint8_t)(subidentifier >> (bit_index -= (bit_index % 7)));
-
-                                       if (byte == 0) {
-                                               if (bit_index <= 7) {
-                                                       break;
-                                               }
+       oid = strtoull(start, &end, 10);
+       if (!((oid == 0) || (oid == 1) || (oid == 2)) ||
+           !end || (*end != '.')) {
+       invalid_oid:
+               fr_strerror_const("Invalid OID");
+               return -1;
+       }
 
-                                               byte = (uint8_t)(subidentifier >> (bit_index -= 7));
+       first_component = oid;
 
-                                               if (byte == 0) {
-                                                       byte = (uint8_t)(subidentifier >> (bit_index -= 7));
-                                               }
-                                       }
+       /*
+        *      Loop until we're done the string.
+        */
+       while (*end) {
+               int i;
 
-                               } else {
-                                       byte = (uint8_t)(subidentifier >> (bit_index -= 7));
-                               }
+               /*
+                *      The previous round MUST have ended with '.'
+                */
+               start = end + 1;
+               if (!*start) goto invalid_oid; /* OID=1 or OID=1.2. is not allowed */
 
-                               byte = byte | 0x80; /* Set the high bit to indicate more bytes to follow */
+               /*
+                *      Parse the component.
+                */
+               oid = strtoull(start, &end, 10);
+               if (oid == ULLONG_MAX) goto invalid_oid;
+               if (*end && (*end != '.')) goto invalid_oid;
 
-                               FR_DBUFF_IN_RETURN(&our_dbuff, byte);
-                               started_subidentifier = true;
-                       }
+               /*
+                *      The initial packed field has the first two compenents included, as (x * 40) + y.
+                */
+               if (first) {
+                       if (oid > (((unsigned long long) 1) << 60)) goto invalid_oid; /* avoid overflow */
+                       first = false;
+                       oid += first_component * 40;
+               }
 
-                       /*
-                        *      Tack on the last byte
-                        */
-                       byte = (uint8_t)(subidentifier);
+               /*
+                *      Encode the number as 7-bit chunks.  Just brute-force over all bits, as doing that ends
+                *      up being fast enough.
+                *
+                *      i.e. if we did anything else to count bits, it would end up with pretty much the same
+                *      code.
+                */
+               for (i = 63; i >= 0; i -= 7) {
+                       uint8_t more, part;
 
-                       byte = byte & 0x7f;
+                       part = (oid >> i) & 0x7f;
+                       if (!part) continue;
 
-                       FR_DBUFF_IN_RETURN(&our_dbuff, byte);
-                       memset(buffer, 0, sizeof(buffer));
-                       buffer_len = 0;
+                       more = ((uint8_t) (i > 0)) << 7;
 
-                       continue;
+                       FR_DBUFF_IN_RETURN(&our_dbuff, (uint8_t) (more | part));
                }
-
-               buffer[buffer_len++] = oid_str[index];
        }
 
        return fr_dbuff_set(dbuff, &our_dbuff);