From: Tom Carpay Date: Mon, 7 Jun 2021 07:54:02 +0000 (+0200) Subject: add key parsing and edge case tests X-Git-Tag: release-1.13.2rc1~42^2~18 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=19c63fdaf62f67e4834482ce6db1e30e0634300f;p=thirdparty%2Funbound.git add key parsing and edge case tests --- diff --git a/sldns/str2wire.c b/sldns/str2wire.c index 8e27d52aa..e4a537093 100644 --- 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 d01b69700..9d6f0186d 100644 --- 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 index 000000000..bb819daae --- /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 index 000000000..ae02ac417 --- /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 0a96659d8..1852fb207 100644 --- 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" )