diff --git a/sldns/str2wire.c b/sldns/str2wire.c index 303d49ba6..45e247613 100644 --- a/sldns/str2wire.c +++ b/sldns/str2wire.c @@ -357,7 +357,7 @@ rrinternal_get_delims(sldns_rdf_type rdftype, size_t r_cnt, size_t r_max) break; default : break; } - return "\n\t "; + return "\n\t "; } /* Syntactic sugar for sldns_rr_new_frm_str_internal */ @@ -448,7 +448,7 @@ rrinternal_parse_unknown(sldns_buffer* strbuf, char* token, size_t token_len, sldns_buffer_position(strbuf)); } hex_data_size = (size_t)atoi(token); - if(hex_data_size > LDNS_MAX_RDFLEN || + if(hex_data_size > LDNS_MAX_RDFLEN || *rr_cur_len + hex_data_size > *rr_len) { return RET_ERR(LDNS_WIREPARSE_ERR_BUFFER_TOO_SMALL, sldns_buffer_position(strbuf)); @@ -567,7 +567,7 @@ sldns_parse_rdf_token(sldns_buffer* strbuf, char* token, size_t token_len, /* check if not quoted yet, and we have encountered quotes */ if(!*quoted && sldns_rdf_type_maybe_quoted(rdftype) && slen >= 2 && - (token[0] == '"' || token[0] == '\'') && + (token[0] == '"' || token[0] == '\'') && (token[slen-1] == '"' || token[slen-1] == '\'')) { /* move token two smaller (quotes) with endnull */ memmove(token, token+1, slen-2); @@ -698,7 +698,7 @@ static int sldns_str2wire_check_svcbparams(uint8_t* rdata, uint16_t rdata_len) mandatory = svcparams[i]; } - /* 4. verify that all the SvcParamKeys in mandatory are present */ + /* Verify that all the SvcParamKeys in mandatory are present */ if(mandatory) { /* Divide by sizeof(uint16_t)*/ uint16_t mandatory_nkeys = sldns_read_uint16(mandatory + 2) / sizeof(uint16_t); @@ -785,7 +785,7 @@ rrinternal_parse_rdata(sldns_buffer* strbuf, char* token, size_t token_len, token[2]=='\t')) { was_unknown_rr_format = 1; if((status=rrinternal_parse_unknown(strbuf, token, - token_len, rr, rr_len, &rr_cur_len, + token_len, rr, rr_len, &rr_cur_len, pre_data_pos)) != 0) return status; } else if(token_strlen > 0 || quoted) { @@ -844,7 +844,7 @@ rrinternal_parse_rdata(sldns_buffer* strbuf, char* token, size_t token_len, if (rr_type == LDNS_RR_TYPE_SVCB || rr_type == LDNS_RR_TYPE_HTTPS) { size_t rdata_len = rr_cur_len - dname_len - 10; uint8_t *rdata = rr+dname_len + 10; - + /* skip 1st rdata field SvcPriority (uint16_t) */ if (rdata_len < sizeof(uint16_t)) return LDNS_WIREPARSE_ERR_OK; @@ -1123,36 +1123,40 @@ sldns_str2wire_svcparam_key_lookup(const char *key, size_t key_len) return key_value; } else switch (key_len) { - case sizeof("mandatory")-1: - if (!strncmp(key, "mandatory", sizeof("mandatory")-1)) - return SVCB_KEY_MANDATORY; - if (!strncmp(key, "echconfig", sizeof("echconfig")-1)) - return SVCB_KEY_ECH; /* allow "echconfig" as well as "ech" */ + case 3: + if (!strncmp(key, "ech", key_len)) + return SVCB_KEY_ECH; break; - case sizeof("alpn")-1: - if (!strncmp(key, "alpn", sizeof("alpn")-1)) + case 4: + if (!strncmp(key, "alpn", key_len)) return SVCB_KEY_ALPN; - if (!strncmp(key, "port", sizeof("port")-1)) + if (!strncmp(key, "port", key_len)) return SVCB_KEY_PORT; break; - case sizeof("no-default-alpn")-1: - if (!strncmp( key , "no-default-alpn" - , sizeof("no-default-alpn")-1)) - return SVCB_KEY_NO_DEFAULT_ALPN; + case 7: + if (!strncmp(key, "dohpath", key_len)) + return SVCB_KEY_DOHPATH; break; - case sizeof("ipv4hint")-1: - if (!strncmp(key, "ipv4hint", sizeof("ipv4hint")-1)) + case 8: + if (!strncmp(key, "ipv4hint", key_len)) return SVCB_KEY_IPV4HINT; - if (!strncmp(key, "ipv6hint", sizeof("ipv6hint")-1)) + if (!strncmp(key, "ipv6hint", key_len)) return SVCB_KEY_IPV6HINT; break; - case sizeof("ech")-1: - if (!strncmp(key, "ech", sizeof("ech")-1)) - return SVCB_KEY_ECH; + case 9: + if (!strncmp(key, "mandatory", key_len)) + return SVCB_KEY_MANDATORY; + if (!strncmp(key, "echconfig", key_len)) + return SVCB_KEY_ECH; /* allow "echconfig" as well as "ech" */ + break; + + case 15: + if (!strncmp(key, "no-default-alpn", key_len)) + return SVCB_KEY_NO_DEFAULT_ALPN; break; default: @@ -1477,7 +1481,7 @@ sldns_str2wire_svcbparam_alpn_value(const char* val, size_t str_len; size_t dst_len; size_t val_len; - + val_len = strlen(val); if (val_len > sizeof(unescaped_dst)) { @@ -1511,7 +1515,34 @@ sldns_str2wire_svcbparam_alpn_value(const char* val, 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_svcbparam_dohpath_value(const char* val, + uint8_t* rd, size_t* rd_len) +{ + size_t val_len; + + /* RFC6570#section-2.1 + * "The characters outside of expressions in a URI Template string are + * intended to be copied literally" + * Practically this means we do not have to look for "double escapes" + * like in the alpn value list. + */ + + val_len = strlen(val); + + if (*rd_len < 4 + val_len) { + return LDNS_WIREPARSE_ERR_BUFFER_TOO_SMALL; + } + + sldns_write_uint16(rd, SVCB_KEY_DOHPATH); + sldns_write_uint16(rd + 2, val_len); + memcpy(rd + 4, val, val_len); + *rd_len = 4 + val_len; + return LDNS_WIREPARSE_ERR_OK; } @@ -1535,6 +1566,7 @@ sldns_str2wire_svcparam_value(const char *key, size_t key_len, case SVCB_KEY_PORT: case SVCB_KEY_IPV4HINT: case SVCB_KEY_IPV6HINT: + case SVCB_KEY_DOHPATH: return LDNS_WIREPARSE_ERR_SVCB_MISSING_PARAM; #endif default: @@ -1566,6 +1598,8 @@ sldns_str2wire_svcparam_value(const char *key, size_t key_len, return sldns_str2wire_svcbparam_ech_value(val, rd, rd_len); case SVCB_KEY_ALPN: return sldns_str2wire_svcbparam_alpn_value(val, rd, rd_len); + case SVCB_KEY_DOHPATH: + return sldns_str2wire_svcbparam_dohpath_value(val, rd, rd_len); default: str_len = strlen(val); if (*rd_len < 4 + str_len) @@ -1593,7 +1627,7 @@ static int sldns_str2wire_svcparam_buf(const char* str, uint8_t* rd, size_t* rd_ /* case: key=value */ if (eq_pos != NULL && eq_pos[1]) { val_in = eq_pos + 1; - + /* unescape characters and "" blocks */ if (*val_in == '"') { val_in++; @@ -1610,11 +1644,11 @@ static int sldns_str2wire_svcparam_buf(const char* str, uint8_t* rd, size_t* rd_ } *val_out = 0; - return sldns_str2wire_svcparam_value(str, eq_pos - str, - unescaped_val[0] ? unescaped_val : NULL, rd, rd_len); + return sldns_str2wire_svcparam_value(str, eq_pos - str, + unescaped_val[0] ? unescaped_val : NULL, rd, rd_len); } /* case: key= */ - else if (eq_pos != NULL && !(eq_pos[1])) { + else if (eq_pos != NULL && !(eq_pos[1])) { return sldns_str2wire_svcparam_value(str, eq_pos - str, NULL, rd, rd_len); } /* case: key */ diff --git a/sldns/str2wire.h b/sldns/str2wire.h index baee4236f..5e4d146d3 100644 --- a/sldns/str2wire.h +++ b/sldns/str2wire.h @@ -38,7 +38,8 @@ struct sldns_struct_lookup_table; #define SVCB_KEY_IPV4HINT 4 #define SVCB_KEY_ECH 5 #define SVCB_KEY_IPV6HINT 6 -#define SVCPARAMKEY_COUNT 7 +#define SVCB_KEY_DOHPATH 7 +#define SVCPARAMKEY_COUNT 8 #define MAX_NUMBER_OF_SVCPARAMS 64 @@ -236,6 +237,7 @@ uint8_t* sldns_wirerr_get_rdatawl(uint8_t* rr, size_t len, size_t dname_len); #define LDNS_WIREPARSE_ERR_SVCB_NO_DEFAULT_ALPN_VALUE 385 #define LDNS_WIREPARSE_ERR_SVCPARAM_BROKEN_RDATA 386 + /** * Get reference to a constant string for the (parse) error. * @param e: error return value diff --git a/sldns/wire2str.c b/sldns/wire2str.c index 74d1b62df..e6278ff56 100644 --- a/sldns/wire2str.c +++ b/sldns/wire2str.c @@ -159,7 +159,7 @@ static sldns_lookup_table sldns_wireparse_errors_data[] = { "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, + { LDNS_WIREPARSE_ERR_SVCB_MANDATORY_IN_MANDATORY, "mandatory MUST not be included as mandatory parameter" }, { LDNS_WIREPARSE_ERR_SVCB_PORT_VALUE_SYNTAX, "Could not parse port SvcParamValue" }, @@ -224,7 +224,7 @@ sldns_lookup_table* sldns_tsig_errors = sldns_tsig_errors_data; /* draft-ietf-dnsop-svcb-https-06: 6. Initial SvcParamKeys */ const char *svcparamkey_strs[] = { "mandatory", "alpn", "no-default-alpn", "port", - "ipv4hint", "ech", "ipv6hint" + "ipv4hint", "ech", "ipv6hint", "dohpath" }; char* sldns_wire2str_pkt(uint8_t* data, size_t len) @@ -487,7 +487,7 @@ int sldns_wire2str_rr_scan(uint8_t** d, size_t* dlen, char** s, size_t* slen, uint8_t* rr = *d; size_t rrlen = *dlen, dname_off, rdlen, ordlen; uint16_t rrtype = 0; - + if(*dlen >= 3 && (*d)[0]==0 && sldns_read_uint16((*d)+1)==LDNS_RR_TYPE_OPT) { /* perform EDNS OPT processing */ @@ -1119,7 +1119,7 @@ static int sldns_wire2str_svcparam_alpn2str(char** s, w += sldns_str_print(s, slen, "%s", ","); } w += sldns_str_print(s, slen, "\""); - + return w; } @@ -1139,7 +1139,7 @@ static int sldns_wire2str_svcparam_ech2str(char** s, (*s) += size; (*slen) -= size; - w += sldns_str_print(s, slen, "\""); + w += sldns_str_print(s, slen, "\""); return w + size; } @@ -1162,7 +1162,7 @@ int sldns_wire2str_svcparam_scan(uint8_t** d, size_t* dlen, char** s, size_t* sl /* verify that we have data_len data */ if (data_len > *dlen) - return -1; + return -1; written_chars += sldns_print_svcparamkey(s, slen, svcparamkey); if (!data_len) { @@ -1174,6 +1174,7 @@ int sldns_wire2str_svcparam_scan(uint8_t** d, size_t* dlen, char** s, size_t* sl case SVCB_KEY_IPV4HINT: case SVCB_KEY_IPV6HINT: case SVCB_KEY_MANDATORY: + case SVCB_KEY_DOHPATH: return -1; default: return written_chars; @@ -1201,6 +1202,8 @@ int sldns_wire2str_svcparam_scan(uint8_t** d, size_t* dlen, char** s, size_t* sl case SVCB_KEY_ECH: r = sldns_wire2str_svcparam_ech2str(s, slen, data_len, *d); break; + case SVCB_KEY_DOHPATH: + /* fallthrough */ default: r = sldns_str_print(s, slen, "=\""); @@ -1222,7 +1225,7 @@ int sldns_wire2str_svcparam_scan(uint8_t** d, size_t* dlen, char** s, size_t* sl } if (r <= 0) return -1; /* wireformat error */ - + written_chars += r; *d += data_len; *dlen -= data_len; @@ -1551,7 +1554,7 @@ int sldns_wire2str_nsec_scan(uint8_t** d, size_t* dl, char** s, size_t* sl) unsigned i, bit, window, block_len; uint16_t t; int w = 0; - + /* check for errors */ while(pl) { if(pl < 2) return -1; diff --git a/testdata/svcb.tdir/svcb.failure-cases-01 b/testdata/svcb.tdir/svcb.failure-cases-01 index c60151692..6d57584f3 100644 --- a/testdata/svcb.tdir/svcb.failure-cases-01 +++ b/testdata/svcb.tdir/svcb.failure-cases-01 @@ -3,7 +3,7 @@ $TTL 3600 @ SOA primary admin 0 0 0 0 0 -; Here there are multiple instances of the same SvcParamKey in the mandatory list +; These cases should be base64 encoded but aren't f21 HTTPS 1 foo.example.com. ech="123" f21 HTTPS 1 foo.example.com. echconfig="123" diff --git a/testdata/svcb.tdir/svcb.success-cases.zone b/testdata/svcb.tdir/svcb.success-cases.zone index 5d6339542..c3d015ec0 100644 --- a/testdata/svcb.tdir/svcb.success-cases.zone +++ b/testdata/svcb.tdir/svcb.success-cases.zone @@ -45,3 +45,17 @@ s08 HTTPS 0 . ( key11=a key12=a key13=a key14=a key15=a key16=a key17=a ke ; maximum alpn size allowed (255 characters) s09 HTTPS 0 . ( alpn="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" ) + +; dohpath can be (non-)quoted and MUST contain "?dns" +; currently there is no validation from Unbound, it can be anything +; maybe needs changing if Unbound is the primary authoritative for SVCB records. +; Then SVCB_SEMANTIC_CHECKS parts of the code could be used per authoritative role. + +_dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath +_dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath= +_dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath="" +_dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath="/" +_dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath="/dns-query{?dns}" +_dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath=/dns-query{?abcd}{!abcd}{?dns} +_dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath=/dns-query{?abcdabcd?dns?defedf} +_dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath=/dns-queryéè{?dns} diff --git a/testdata/svcb.tdir/svcb.success-cases.zone.cmp b/testdata/svcb.tdir/svcb.success-cases.zone.cmp index e504e7b18..3a42393ba 100644 --- a/testdata/svcb.tdir/svcb.success-cases.zone.cmp +++ b/testdata/svcb.tdir/svcb.success-cases.zone.cmp @@ -8,3 +8,11 @@ s06.success-cases. 3600 IN HTTPS 0 . ech="aGVsbG93b3JsZCE=" s07.success-cases. 3600 IN HTTPS 0 . ech="aGVsbG93b3JsZCE=" s08.success-cases. 3600 IN 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" s09.success-cases. 3600 IN HTTPS 0 . alpn="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" +_dns.doh.example. 7200 IN SVCB \# 26 000103646F68076578616D706C65000001000302683200070000 +_dns.doh.example. 7200 IN SVCB \# 26 000103646F68076578616D706C65000001000302683200070000 +_dns.doh.example. 7200 IN SVCB \# 26 000103646F68076578616D706C65000001000302683200070000 +_dns.doh.example. 7200 IN SVCB 1 doh.example. alpn="h2" dohpath="/" +_dns.doh.example. 7200 IN SVCB 1 doh.example. alpn="h2" dohpath="/dns-query{?dns}" +_dns.doh.example. 7200 IN SVCB 1 doh.example. alpn="h2" dohpath="/dns-query{?abcd}{!abcd}{?dns}" +_dns.doh.example. 7200 IN SVCB 1 doh.example. alpn="h2" dohpath="/dns-query{?abcdabcd?dns?defedf}" +_dns.doh.example. 7200 IN SVCB 1 doh.example. alpn="h2" dohpath="/dns-query\195\169\195\168{?dns}" diff --git a/testdata/svcb.tdir/svcb.test b/testdata/svcb.tdir/svcb.test index 17330e08f..280c58fc8 100644 --- a/testdata/svcb.tdir/svcb.test +++ b/testdata/svcb.tdir/svcb.test @@ -66,7 +66,7 @@ then elif $PRE/readzone svcb.failure-cases-03 then - echo "Failure case 02: 65 SvcParams is too many SvcParams; the limit is 64" + echo "Failure case 03: 65 SvcParams is too many SvcParams; the limit is 64" echo "Incorrectly succeeded" exit 1 @@ -75,6 +75,7 @@ then echo "Failure case 04: 256 is too many characters for an alpn; maximum is 255" echo "Incorrectly succeeded" exit 1 + else echo "All failure cases test successfully" fi