add key parsing and edge case tests

diff --git a/sldns/str2wire.c b/sldns/str2wire.c
index 8e27d52aa7ab698bfa6941f3be27dd45e1797b5c..e4a53709325e287de02d6c3ae040de4692617706 100644 (file)
--- a/sldns/str2wire.c
+++ b/sldns/str2wire.c
@@ -650,7 +650,7 @@ static int sldns_str2wire_check_svcbparams(uint8_t* rdata, uint16_t rdata_len)
                rdata_ptr += svcbparam_len;
 
                nparams += 1;
-               if (nparams > sizeof(svcparams))
+               if (nparams > sizeof(svcparams)/8)
                        return LDNS_WIREPARSE_ERR_SVCB_TOO_MANY_PARAMS;
        }
 
@@ -1086,6 +1086,7 @@ sldns_str2wire_svcparam_key_lookup(const char *key, size_t key_len)
                memcpy(buf, key + 3, key_len - 3);
                buf[key_len - 3] = 0;
                key_value = strtoul(buf, &endptr, 10);
+
                if (endptr > buf        /* digits seen */
                && *endptr == 0         /* no non-digit chars after digits */
                &&  key_value <= 65535) /* no overflow */
@@ -1384,7 +1385,7 @@ sldns_str2wire_svcbparam_ech_value(const char* val, uint8_t* rd, size_t* rd_len)
 }
 
 static const char*
-sldns_str2wire_svcbparam_parse_alpn_next_unescaped_comma(const char *val)
+sldns_str2wire_svcbparam_parse_next_unescaped_comma(const char *val)
 {
        while (*val) {
                /* Only return when the comma is not escaped*/
@@ -1401,7 +1402,7 @@ sldns_str2wire_svcbparam_parse_alpn_next_unescaped_comma(const char *val)
 }
 
 static size_t
-sldns_str2wire_svcbparam_parse_alpn_copy_unescaped(uint8_t *dst,
+sldns_str2wire_svcbparam_parse_copy_unescaped(uint8_t *dst,
        const char *src, size_t len)
 {
        uint8_t *orig_dst = dst;
@@ -1419,7 +1420,8 @@ sldns_str2wire_svcbparam_parse_alpn_copy_unescaped(uint8_t *dst,
        return (size_t)(dst - orig_dst);
 }
 
-int sldns_str2wire_svcbparam_alpn_value(const char* val,
+static int
+sldns_str2wire_svcbparam_alpn_value(const char* val,
        uint8_t* rd, size_t* rd_len)
 {
        uint8_t     unescaped_dst[65536];
@@ -1437,13 +1439,14 @@ int sldns_str2wire_svcbparam_alpn_value(const char* val,
        while (val_len) {
                size_t dst_len;
 
-               str_len = (next_str = sldns_str2wire_svcbparam_parse_alpn_next_unescaped_comma(val))
+               str_len = (next_str = sldns_str2wire_svcbparam_parse_next_unescaped_comma(val))
                        ? (size_t)(next_str - val) : val_len;
 
                if (str_len > 255) {
                        return LDNS_WIREPARSE_ERR_SVCB_ALPN_KEY_TOO_LARGE;
                }
-               dst_len = sldns_str2wire_svcbparam_parse_alpn_copy_unescaped(dst + 1, val, str_len);
+
+               dst_len = sldns_str2wire_svcbparam_parse_copy_unescaped(dst + 1, val, str_len);
                *dst++ = dst_len;
                 dst  += dst_len;
 
@@ -1465,7 +1468,51 @@ int sldns_str2wire_svcbparam_alpn_value(const char* val,
 }
 
 static int
-sldns_str2wire_svcparam_key_value(const char *key, size_t key_len,
+sldns_str2wire_svcbparam_key_value(uint16_t svcparamkey, const char* val,
+       uint8_t* rd, size_t* rd_len)
+{
+       uint8_t     unescaped_dst[65536];
+       uint8_t    *dst = unescaped_dst;
+       const char *next_str;
+       size_t      str_len;
+       size_t      dst_len;
+       size_t          val_len;
+       
+       val_len = strlen(val);
+
+       if (val_len > sizeof(unescaped_dst)) {
+               return LDNS_WIREPARSE_ERR_SYNTAX_INTEGER_OVERFLOW;
+       }
+       while (val_len) {
+               str_len = (next_str = sldns_str2wire_svcbparam_parse_next_unescaped_comma(val))
+                       ? (size_t)(next_str - val) : val_len;
+
+               if (str_len > 255) {
+                       return LDNS_WIREPARSE_ERR_SVCB_ALPN_KEY_TOO_LARGE;
+               }
+               dst_len = sldns_str2wire_svcbparam_parse_copy_unescaped(dst + 1, val, str_len);
+               *dst++ = dst_len;
+                dst  += dst_len;
+
+               if (!next_str)
+                       break;
+
+               /* skip the comma for the next iteration */
+               val_len -= next_str - val + 1;
+               val = next_str + 1;
+       }
+       dst_len = dst - unescaped_dst;
+
+       sldns_write_uint16(rd, svcparamkey);
+       sldns_write_uint16(rd + 2, dst_len);
+       memcpy(rd + 4, unescaped_dst, dst_len);
+       *rd_len = 4 + dst_len;
+       
+       return LDNS_WIREPARSE_ERR_OK;
+}
+
+static int
+sldns_str2wire_svcparam_value(const char *key, size_t key_len,
        const char *val, uint8_t* rd, size_t* rd_len)
 {
        size_t str_len;
@@ -1485,20 +1532,14 @@ sldns_str2wire_svcparam_key_value(const char *key, size_t key_len,
                case SVCB_KEY_IPV6HINT:
                        return LDNS_WIREPARSE_ERR_SVCB_MISSING_PARAM;
                default:
-               sldns_write_uint16(rd, svcparamkey);
-               sldns_write_uint16(rd + 2, 0);
-               *rd_len = 4;
+                       sldns_write_uint16(rd, svcparamkey);
+                       sldns_write_uint16(rd + 2, 0);
+                       *rd_len = 4;
 
-               return LDNS_WIREPARSE_ERR_OK;
+                       return LDNS_WIREPARSE_ERR_OK;
                }
        }
 
-       // @TODO unescape characters in the value list
-
-       // if (val[0] == '"' && val[str_len - 1]) {
-
-       // }
-
        /* value is non-empty */
        switch (svcparamkey) {
        case SVCB_KEY_PORT:
@@ -1516,15 +1557,7 @@ sldns_str2wire_svcparam_key_value(const char *key, size_t key_len,
        case SVCB_KEY_ALPN:
                return sldns_str2wire_svcbparam_alpn_value(val, rd, rd_len);
        default:
-               // @TODO escaping here -> copy from alpn?
-
-               str_len = strlen(val);
-               sldns_write_uint16(rd, svcparamkey);
-               sldns_write_uint16(rd + 2, str_len);
-               memcpy(rd + 4, val, str_len);
-               *rd_len = 4 + str_len;
-
-               return LDNS_WIREPARSE_ERR_OK;
+               return sldns_str2wire_svcbparam_key_value(svcparamkey, val, rd, rd_len);
        }
 
        // @TODO change to error?
@@ -1557,12 +1590,12 @@ int sldns_str2wire_svcparam_buf(const char* str, uint8_t* rd, size_t* rd_len)
                }
                *val_out = 0;
 
-               return sldns_str2wire_svcparam_key_value(str, eq_pos - str, 
+               return sldns_str2wire_svcparam_value(str, eq_pos - str, 
                                                         unescaped_val[0] ? unescaped_val : NULL, rd, rd_len);
        } else if (eq_pos != NULL && !(eq_pos[1])) { /* case: key= */
-               return sldns_str2wire_svcparam_key_value(str, eq_pos - str, NULL, rd, rd_len);
+               return sldns_str2wire_svcparam_value(str, eq_pos - str, NULL, rd, rd_len);
        } else { /* case: key */
-               return sldns_str2wire_svcparam_key_value(str, strlen(str), NULL, rd, rd_len);
+               return sldns_str2wire_svcparam_value(str, strlen(str), NULL, rd, rd_len);
        }
 
        return LDNS_WIREPARSE_ERR_OK;

diff --git a/testdata/svcb.tdir/svcb.failure-cases-22 b/testdata/svcb.tdir/svcb.failure-cases-22
index d01b6970077dd699266a609a3a28f5f10a585952..9d6f0186d5350d6b4ce89393168fbc426c87e482 100644 (file)
--- a/testdata/svcb.tdir/svcb.failure-cases-22
+++ b/testdata/svcb.tdir/svcb.failure-cases-22
@@ -3,6 +3,6 @@ $TTL 3600
 
 @       SOA     primary admin 0 0 0 0 0
 
-; Port mus be a positive number < 65536
+; Port must be a positive number < 65536
 
 f22    HTTPS   1 foo.example.com. port=65536

diff --git a/testdata/svcb.tdir/svcb.failure-cases-23 b/testdata/svcb.tdir/svcb.failure-cases-23
new file mode 100644 (file)
index 0000000..bb819da
--- /dev/null
+++ b/testdata/svcb.tdir/svcb.failure-cases-23
@@ -0,0 +1,8 @@
+$ORIGIN failure-cases.
+$TTL 3600
+
+@       SOA     primary admin 0 0 0 0 0
+
+; 65 SvcParams is too many SvcParams; the limit is 64
+
+f23    HTTPS   1 foo.example.com. ( key11=a key12=a key13=a key14=a key15=a key16=a key17=a key18=a key19=a key110=a key111=a key112=a key113=a key114=a key115=a key116=a key117=a key118=a key119=a key120=a key121=a key122=a key123=a key124=a key125=a key126=a key127=a key128=a key129=a key130=a key131=a key132=a key133=a key134=a key135=a key136=a key137=a key138=a key139=a key140=a key141=a key142=a key143=a key144=a key145=a key146=a key147=a key148=a key149=a key150=a key151=a key152=a key153=a key154=a key155=a key156=a key157=a key158=a key159=a key160=a key161=a key162=a key163=a key164=a key165=a )
\ No newline at end of file

diff --git a/testdata/svcb.tdir/svcb.failure-cases-24 b/testdata/svcb.tdir/svcb.failure-cases-24
new file mode 100644 (file)
index 0000000..ae02ac4
--- /dev/null
+++ b/testdata/svcb.tdir/svcb.failure-cases-24
@@ -0,0 +1,8 @@
+$ORIGIN failure-cases.
+$TTL 3600
+
+@       SOA     primary admin 0 0 0 0 0
+
+; 256 is too many characters for an alpn; maximum is 255
+
+f23    HTTPS   1 foo.example.com. ( alpn="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" )
\ No newline at end of file

diff --git a/testdata/svcb.tdir/svcb.success-cases.zone b/testdata/svcb.tdir/svcb.success-cases.zone
index 0a96659d8449b0d985f6da407dc77c20650e2e6b..1852fb207bbd6aefcb835ea97ede467e19facabf 100644 (file)
--- a/testdata/svcb.tdir/svcb.success-cases.zone
+++ b/testdata/svcb.tdir/svcb.success-cases.zone
@@ -38,3 +38,10 @@ s06     HTTPS   0 . ech="aGVsbG93b3JsZCE="
 ; echconfig is an alias for ech
 s07     HTTPS   0 . echconfig="aGVsbG93b3JsZCE="
 
+; maximum size allowed in a svcb rdata set (64 SvcParams)
+
+s07     HTTPS   0 . ( key11=a key12=a key13=a key14=a key15=a key16=a key17=a key18=a key19=a key110=a key111=a key112=a key113=a key114=a key115=a key116=a key117=a key118=a key119=a key120=a key121=a key122=a key123=a key124=a key125=a key126=a key127=a key128=a key129=a key130=a key131=a key132=a key133=a key134=a key135=a key136=a key137=a key138=a key139=a key140=a key141=a key142=a key143=a key144=a key145=a key146=a key147=a key148=a key149=a key150=a key151=a key152=a key153=a key154=a key155=a key156=a key157=a key158=a key159=a key160=a key161=a key162=a key163=a key164=a)
+
+; maximum alpn size allowed (255 characters)
+
+s07     HTTPS   0 . ( alpn="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" )