]> git.ipfire.org Git - thirdparty/unbound.git/commitdiff
add check_svcbparams
authorTom Carpay <tom@nlnetlabs.nl>
Wed, 2 Jun 2021 08:10:05 +0000 (10:10 +0200)
committerTom Carpay <tom@nlnetlabs.nl>
Wed, 2 Jun 2021 08:10:05 +0000 (10:10 +0200)
sldns/str2wire.c
sldns/str2wire.h
sldns/wire2str.c

index 8eafca04204c4f1ef278289d75bebad395426770..1ae173014571cb002020988abab588f469f87a10 100644 (file)
@@ -614,41 +614,91 @@ sldns_affix_token(sldns_buffer* strbuf, char* token, size_t* token_len,
        return 1;
 }
 
-static void sldns_check_svcbparams(uint8_t* rdata, uint16_t rdata_len)
+static int sldns_str2wire_svcparam_key_cmp(const void *a, const void *b)
 {
-       size_t nparams = 0, i;
+       return sldns_read_uint16(*(uint8_t**) a)
+            - sldns_read_uint16(*(uint8_t**) b);
+}
+
+static int sldns_str2wire_check_svcbparams(uint8_t* rdata, uint16_t rdata_len)
+{
+       size_t nparams = 0, i, j;
        uint8_t* svcparams[10240]; // @TODO change array size in actual max number of svcbparams
+       uint8_t* mandatory = NULL;
 
-       // 1. Find the SvcParams
+       /* find the SvcParams */
        while (rdata_len) {
                uint16_t svcbparam_len;
 
                svcparams[nparams] = rdata;
                if (rdata_len < 4)
-                       return;
+                       // @TODO verify that these are correct
+                       return LDNS_WIREPARSE_ERR_OK;
                svcbparam_len = sldns_read_uint16(rdata + 2);
                rdata_len -= 4;
                rdata += 4;
 
                if (rdata_len < svcbparam_len)
-                       return;
+                       // @TODO verify that these are correct
+                       return LDNS_WIREPARSE_ERR_OK;
                rdata_len -= svcbparam_len;
                rdata += svcbparam_len;
 
                nparams += 1;
        }
 
-       for (i = 0; i < nparams; i++) {
-               uint8_t* svcparam_data = svcparams[i];
-               uint16_t svcparam_key = sldns_read_uint16(svcparam_data);
-               uint16_t svcparam_len = sldns_read_uint16(svcparam_data + 2);
+       /* In draft-ietf-dnsop-svcb-https-05 Section 7:
+        *
+        *     In wire format, the keys are represented by their numeric
+        *     values in network byte order, concatenated in ascending order.
+        */
+       qsort((void *)svcparams
+            ,nparams
+            ,sizeof(uint8_t*)
+            ,sldns_str2wire_svcparam_key_cmp);
 
-               fprintf(stderr, "param %zu, key: %d, len: %d\n"
-                             , i, (int)svcparam_key, (int)svcparam_len);
+       /* In draft-ietf-dnsop-svcb-https-05 Section 7:
+        *
+        *     Keys (...) MUST NOT appear more than once.
+        *
+        * If they key has already been seen, we have a duplicate
+        */
+       for (i = 0; i < nparams - 1; i++) {
+               uint16_t key = sldns_read_uint16(svcparams[i]);
+
+               if (i + 1 < nparams && key == sldns_read_uint16(svcparams[i+1]))
+                       return LDNS_WIREPARSE_ERR_SVCB_DUPLICATE_KEYS;
+
+               if (key == SVCB_KEY_MANDATORY)
+                       mandatory = svcparams[i];
        }
-       // 2. qsort the data according to the keys
-       // 3. verify that keys are unique
-       // 4. verify that mandatory keys are present and unique
+
+       /* 4. verify that all the SvcParamKeys in mandatory are present */
+       if (mandatory) {
+               /* divide by sizeof(uint16_t)*/
+               uint16_t mandatory_len = sldns_read_uint16(mandatory + 2) >> 1;
+
+               // @TODO do we need this?
+               if (mandatory_len < 1)
+                       return LDNS_WIREPARSE_ERR_SVCB_MISSING_PARAM;
+
+               for (i = 0; i < mandatory_len; i++) {
+                       // @TODO fix ugly math
+                       uint16_t mandatory_key = sldns_read_uint16(mandatory + 2 + 2 * i);
+                       uint8_t found = 0;
+
+                       for (j = 0; j < nparams; j++) {
+                               if (mandatory_key == sldns_read_uint16(svcparams[j]))
+                                       found = 1;
+                       }
+
+                       if (!found)
+                               return LDNS_WIREPARSE_ERR_SVCB_MISSING_MANDATORY;
+               }
+
+       }
+
+       return LDNS_WIREPARSE_ERR_OK;
 }
 
 /** parse rdata from string into rr buffer(-remainder after dname). */
@@ -750,7 +800,7 @@ rrinternal_parse_rdata(sldns_buffer* strbuf, char* token, size_t token_len,
        *rr_len = rr_cur_len;
        /* SVCB/HTTPS handling  */
        if (rr_type == LDNS_RR_TYPE_SVCB || rr_type == LDNS_RR_TYPE_HTTPS) {
-               uint16_t rdata_len = rr_cur_len - dname_len - 10;
+               size_t rdata_len = rr_cur_len - dname_len - 10;
                uint8_t *rdata = rr+dname_len + 10;
                
                /* skip 1st rdata field SvcPriority (uint16_t) */
@@ -780,7 +830,8 @@ rrinternal_parse_rdata(sldns_buffer* strbuf, char* token, size_t token_len,
 
                rdata_len -= 1;
                rdata += 1;
-               check_svcbparams(rdata, rdata_len);
+               return sldns_str2wire_check_svcbparams(rdata, rdata_len);
+
        }
        return LDNS_WIREPARSE_ERR_OK;
 }
@@ -1264,13 +1315,32 @@ sldns_str2wire_svcbparam_mandatory(const char* val, uint8_t* rd, size_t* rd_len)
                key_dst += 1;
        }
 
-       /* In draft-ietf-dnsop-svcb-https-04 Section 7:
+       /* In draft-ietf-dnsop-svcb-https-05 Section 7:
         *
         *    "In wire format, the keys are represented by their numeric
         *     values in network byte order, concatenated in ascending order."
         */
        qsort((void *)(rd + 4), count, sizeof(uint16_t), sldns_network_uint16_cmp);
 
+       /* Guarantee key uniqueness. After the sort we only need to
+        * compare neighbours */
+       if (count > 1) {
+               for (i = 0; i < count - 1; i++) {
+                       uint8_t* current_pos = (rd + 4 + (sizeof(uint16_t) * i));
+                       uint16_t key = sldns_read_uint16(current_pos);
+
+                       /* In draft-ietf-dnsop-svcb-https-05 Section 8
+                        * automatically mandatory MUST NOT appear in its own value-list
+                        */
+                       if (key == SVCB_KEY_MANDATORY)
+                               return LDNS_WIREPARSE_ERR_SVCB_MANDATORY_IN_MANDATORY;
+
+                       if (key == sldns_read_uint16(current_pos + 2)) {
+                               return LDNS_WIREPARSE_ERR_SVCB_MANDATORY_DUPLICATE_KEY;
+                       }
+               }
+       }
+
        return LDNS_WIREPARSE_ERR_OK;
 }
 
@@ -1407,6 +1477,8 @@ sldns_str2wire_svcparam_key_value(const char *key, size_t key_len,
 
        // @TODO add case where svcparamkey == -1
 
+       // @TODO add cases where keys cannot be vallueless -> LDNS_WIREPARSE_ERR_SVCB_MISSING_PARAM
+
        /* key and no value case*/
        if (val == NULL) {
                sldns_write_uint16(rd, svcparamkey);
index b687546a76950b17eedd49a7d81b7727c29e24f5..fbdda66e24fd1e0f89cbacc23925a7be0bd0658f 100644 (file)
@@ -219,6 +219,11 @@ uint8_t* sldns_wirerr_get_rdatawl(uint8_t* rr, size_t len, size_t dname_len);
 #define LDNS_WIREPARSE_ERR_SYNTAX_INTEGER_OVERFLOW 370
 #define LDNS_WIREPARSE_ERR_INCLUDE 371
 #define LDNS_WIREPARSE_ERR_PARENTHESIS 372
+#define LDNS_WIREPARSE_ERR_SVCB_MISSING_PARAM 373
+#define LDNS_WIREPARSE_ERR_SVCB_MISSING_MANDATORY 374
+#define LDNS_WIREPARSE_ERR_SVCB_MANDATORY_DUPLICATE_KEY 375
+#define LDNS_WIREPARSE_ERR_SVCB_MANDATORY_IN_MANDATORY 376
+#define LDNS_WIREPARSE_ERR_SVCB_DUPLICATE_KEYS 377
 
 /**
  * Get reference to a constant string for the (parse) error.
index cf87f0aa8067c83a12996267f2cd69fe0fefb4bb..9426bdb2ca5ad02122f5e4097bb887f91d27cc47 100644 (file)
@@ -149,6 +149,13 @@ static sldns_lookup_table sldns_wireparse_errors_data[] = {
        { LDNS_WIREPARSE_ERR_SYNTAX_INTEGER_OVERFLOW, "Syntax error, integer overflow" },
        { LDNS_WIREPARSE_ERR_INCLUDE, "$INCLUDE directive was seen in the zone" },
        { LDNS_WIREPARSE_ERR_PARENTHESIS, "Parse error, parenthesis mismatch" },
+       { LDNS_WIREPARSE_ERR_SVCB_MISSING_PARAM, "Value expected for SvcParam"},
+       { LDNS_WIREPARSE_ERR_SVCB_MISSING_MANDATORY, "Mandatory SvcParamKey is missing"},
+       { LDNS_WIREPARSE_ERR_SVCB_MANDATORY_DUPLICATE_KEY,
+               "Keys in SvcParam mandatory MUST be unique" },
+       { LDNS_WIREPARSE_ERR_SVCB_MANDATORY_IN_MANDATORY, 
+               "mandatory MUST not be included as mandatory parameter" },
+       { LDNS_WIREPARSE_ERR_SVCB_DUPLICATE_KEYS, "Duplicate SVCB key found"},
        { 0, NULL }
 };
 sldns_lookup_table* sldns_wireparse_errors = sldns_wireparse_errors_data;
@@ -1133,8 +1140,6 @@ int sldns_wire2str_svcparam_scan(uint8_t** d, size_t* dlen, char** s, size_t* sl
        *d    += 4;
        *dlen -= 4;
 
-       // fprintf(stderr, "data_len: %hu\n", data_len);
-
        /* verify that we have data_len data */
        if (data_len > *dlen)
                return -1;