From: Tom Carpay Date: Mon, 7 Jun 2021 12:05:14 +0000 (+0200) Subject: fix key parsing and incorporate testcases X-Git-Tag: release-1.13.2rc1~42^2~16 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9beea6a00ccfa47f045e32ecb2d6a5a53ab52ed1;p=thirdparty%2Funbound.git fix key parsing and incorporate testcases --- diff --git a/sldns/str2wire.c b/sldns/str2wire.c index 978839ba5..25d2f1337 100644 --- a/sldns/str2wire.c +++ b/sldns/str2wire.c @@ -1168,7 +1168,7 @@ sldns_str2wire_svcbparam_ipv4hint(const char* val, uint8_t* rd, size_t* rd_len) int count; char ip_str[INET_ADDRSTRLEN+1]; char *next_ip_str; - uint32_t *ip_wire_dst; + uint32_t *ip_wire_dst = NULL; size_t i; for (i = 0, count = 1; val[i]; i++) { @@ -1225,7 +1225,7 @@ sldns_str2wire_svcbparam_ipv6hint(const char* val, uint8_t* rd, size_t* rd_len) int count; char ip_str[INET6_ADDRSTRLEN+1]; char *next_ip_str; - uint32_t *ip_wire_dst; + uint32_t *ip_wire_dst = NULL; size_t i; for (i = 0, count = 1; val[i]; i++) { @@ -1289,7 +1289,7 @@ sldns_str2wire_svcbparam_mandatory(const char* val, uint8_t* rd, size_t* rd_len) { size_t i, count, val_len; char* next_key; - uint16_t* key_dst; + uint16_t* key_dst = NULL; val_len = strlen(val); @@ -1369,8 +1369,7 @@ sldns_str2wire_svcbparam_ech_value(const char* val, uint8_t* rd, size_t* rd_len) wire_len = sldns_b64_pton(val, buffer, LDNS_MAX_RDFLEN); - if (wire_len == -1) { - // zc_error_prev_line("invalid base64 data in ech"); + if (wire_len == 0) { return LDNS_WIREPARSE_ERR_SYNTAX_B64; } else if (wire_len + 4 > *rd_len) { return LDNS_WIREPARSE_ERR_BUFFER_TOO_SMALL; @@ -1467,50 +1466,6 @@ sldns_str2wire_svcbparam_alpn_value(const char* val, return LDNS_WIREPARSE_ERR_OK; } -static int -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) @@ -1557,21 +1512,21 @@ sldns_str2wire_svcparam_value(const char *key, size_t key_len, case SVCB_KEY_ALPN: return sldns_str2wire_svcbparam_alpn_value(val, rd, rd_len); default: + str_len = strlen(val); sldns_write_uint16(rd, svcparamkey); - sldns_write_uint16(rd + 2, strlen(val)); - memcpy(rd + 4, val, strlen(val)); - *rd_len = 4 + strlen(val); - break; - //return sldns_str2wire_svcbparam_key_value(svcparamkey, val, rd, rd_len); + sldns_write_uint16(rd + 2, str_len); + memcpy(rd + 4, val, str_len); + *rd_len = 4 + str_len; + + return LDNS_WIREPARSE_ERR_OK; } - // @TODO change to error? - return LDNS_WIREPARSE_ERR_OK; + // @TODO is this supposed to be an error? + return LDNS_WIREPARSE_ERR_GENERAL; } int sldns_str2wire_svcparam_buf(const char* str, uint8_t* rd, size_t* rd_len) { - size_t str_len; const char* eq_pos; char unescaped_val[65536]; char* val_out = unescaped_val; diff --git a/sldns/wire2str.c b/sldns/wire2str.c index a7e6ebc90..99ce5574e 100644 --- a/sldns/wire2str.c +++ b/sldns/wire2str.c @@ -997,7 +997,6 @@ static int sldns_wire2str_svcparam_ipv4hint2str(char** s, { char ip_str[INET_ADDRSTRLEN + 1]; - // @TODO actually incorporate this int w = 0; assert(data_len > 0); @@ -1026,13 +1025,10 @@ static int sldns_wire2str_svcparam_ipv6hint2str(char** s, { char ip_str[INET6_ADDRSTRLEN + 1]; - // @TODO actually incorporate this -> is this correct now? int w = 0; assert(data_len > 0); - // @TODO fix ntohs -> see output - if ((data_len % LDNS_IP6ADDRLEN) == 0) { if (inet_ntop(AF_INET6, data, ip_str, sizeof(ip_str)) == NULL) return 0; /* wireformat error, incorrect size or inet family */ @@ -1121,11 +1117,8 @@ static int sldns_wire2str_svcparam_ech2str(char** s, w += sldns_str_print(s, slen, "=\""); - /* b64_ntop_calculate size includes null at the end */ - size = sldns_b64_ntop_calculate_size(data_len) - 1; + size = sldns_b64_ntop(data, data_len, *s, *slen); - // @TODO store return value? - sldns_b64_ntop(data, data_len, *s, *slen); (*s) += size; (*slen) -= size; @@ -1141,7 +1134,7 @@ static int sldns_wire2str_svcparam_ech2str(char** s, int sldns_wire2str_svcparam_scan(uint8_t** d, size_t* dlen, char** s, size_t* slen) { - char ch; + uint8_t ch; uint16_t svcparamkey, data_len; int written_chars = 0; int r, i; @@ -1197,7 +1190,7 @@ int sldns_wire2str_svcparam_scan(uint8_t** d, size_t* dlen, char** s, size_t* sl r = sldns_wire2str_svcparam_ech2str(s, slen, data_len, *d); break; default: - r += sldns_str_print(s, slen, "=\""); + r = sldns_str_print(s, slen, "=\""); for (i = 0; i < data_len; i++) { ch = (*d)[i]; diff --git a/testdata/svcb.tdir/svcb.success-cases.zone b/testdata/svcb.tdir/svcb.success-cases.zone index 1852fb207..896304757 100644 --- a/testdata/svcb.tdir/svcb.success-cases.zone +++ b/testdata/svcb.tdir/svcb.success-cases.zone @@ -40,8 +40,8 @@ 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) +s08 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" ) +s09 HTTPS 0 . ( alpn="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" ) diff --git a/testdata/svcb.tdir/svcb.success-cases.zone.cmp b/testdata/svcb.tdir/svcb.success-cases.zone.cmp index 540b541c4..f28bd2ce5 100644 --- a/testdata/svcb.tdir/svcb.success-cases.zone.cmp +++ b/testdata/svcb.tdir/svcb.success-cases.zone.cmp @@ -1,12 +1,10 @@ -$ORIGIN . -success-cases 3600 IN SOA primary.success-cases. admin.success-cases. ( - 0 0 0 0 0 ) -$ORIGIN success-cases. -s01 3600 IN SVCB 0 . key123 -s02 3600 IN SVCB 0 . ech -s03 3600 IN HTTPS 0 . alpn="h2,h3" no-default-alpn -s04 3600 IN HTTPS 0 . no-default-alpn -s05 3600 IN HTTPS 0 . mandatory=port alpn="dot" no-default-alpn port=853 -s06 3600 IN HTTPS 0 . ech=aGVsbG93b3JsZCE= -s07 3600 IN HTTPS 0 . ech=aGVsbG93b3JsZCE= -; zone success-cases is ok +success-cases. 3600 IN SOA primary.success-cases. admin.success-cases. 0 0 0 0 0 +s01.success-cases. 3600 IN SVCB 0 . key123 +s02.success-cases. 3600 IN SVCB 0 . ech +s03.success-cases. 3600 IN HTTPS 0 . alpn="h2,h3" no-default-alpn +s04.success-cases. 3600 IN HTTPS 0 . no-default-alpn +s05.success-cases. 3600 IN HTTPS 0 . mandatory=port alpn="dot" no-default-alpn port=853 +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" key164="a" +s09.success-cases. 3600 IN HTTPS 0 . alpn="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" diff --git a/testdata/svcb.tdir/svcb.test b/testdata/svcb.tdir/svcb.test index 48a754512..47968be5f 100644 --- a/testdata/svcb.tdir/svcb.test +++ b/testdata/svcb.tdir/svcb.test @@ -183,52 +183,35 @@ then echo "Failure case 22: port value needs to be a positive integer < 65536" echo "Incorrectly succeeded" exit 1 + +elif $PRE/readzone svcb.failure-cases-23 +then + echo "Failure case 23: 65 SvcParams is too many SvcParams; the limit is 64" + echo "Incorrectly succeeded" + exit 1 + +elif $PRE/readzone svcb.failure-cases-23 +then + echo "Failure case 24: 256 is too many characters for an alpn; maximum is 255" + echo "Incorrectly succeeded" + exit 1 else echo "All failure cases test successfully" fi # check all the succes and write them -if ! $PRE/nsd-checkzone -p success-cases svcb.success-cases.zone > svcb.success-cases.zone.out +if ! $PRE/readzone svcb.success-cases.zone > svcb.success-cases.zone.out then - echo "Some particular success cases did not succeed to parse" - exit 1 + echo "Some particular success cases did not succeed to parse" + exit 1 elif ! diff svcb.success-cases.zone.out svcb.success-cases.zone.cmp then echo "Some success cases could not be printed" - exit 1 + exit 1 else - echo "All particular success cases parsed and printed successfully" + echo "All particular success cases parsed and printed successfully" fi -rem $PRE/nsd-control -c svcb.secondary.conf write -rem while [ ! -f test-vectors-secondary.zone ] -rem do -rem sleep 1 -rem done -rem while ! grep '^v20' test-vectors-secondary.zone -rem do -rem sleep 1 -rem done -rem grep -v '^;' svcb.test-vectors-pf.zone.out > svcb.test-vectors-pf.zone.out2 -rem grep -v '^;' test-vectors-secondary.zone > test-vectors-secondary.zone.out -rem if ! diff svcb.test-vectors-pf.zone.out2 test-vectors-secondary.zone.out -rem then -rem echo "Output from secondary did not match output from primary" -rem exit 1 -rem else -rem echo "Output from secondary did match output from primary" -rem fi - -rem dig @127.0.0.1 -p $TPKG_SEC_PORT f01.failure-cases. TYPE64 > f01.failure-cases.out -rem if grep 'status: NOERROR' f01.failure-cases.out -rem then -rem echo "Failure case 1: Multiple instances of the same SvcParamKey" -rem echo "allowed for secondary" -rem else -rem echo "Could not load failure-cases zone in secondary" -rem exit 1 -rem fi -