From 56a534ab067544b029b4e76158400bc1a23e4d59 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 27 Feb 2019 10:21:33 +1100 Subject: [PATCH 1/8] Add dns_rdata_totext() and dns_rdata_fromtext() to fromwire Add dns_rdata_totext() and dns_rdata_fromtext() to fromwire for valid inputs to ensure that what we accept in dns_rdata_fromwire() can be written out and read back in. (cherry picked from commit 36f30f57313747c536ea9afcd037086edea3ecb0) --- lib/dns/tests/rdata_test.c | 55 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/lib/dns/tests/rdata_test.c b/lib/dns/tests/rdata_test.c index 84f11ab08e..f42cf59a6c 100644 --- a/lib/dns/tests/rdata_test.c +++ b/lib/dns/tests/rdata_test.c @@ -234,6 +234,12 @@ check_text_ok_single(const text_ok_t *text_ok, dns_rdataclass_t rdclass, isc_buffer_init(&target, buf_totext, sizeof(buf_totext)); result = dns_rdata_totext(&rdata, NULL, &target); assert_int_equal(result, ISC_R_SUCCESS); + /* + * Ensure buf_totext is properly NUL terminated as dns_rdata_totext() + * may attempt different output formats writing into the apparently + * unused part of the buffer. + */ + isc_buffer_putuint8(&target, 0); assert_string_equal(buf_totext, text_ok->text_out); /* @@ -252,6 +258,49 @@ check_text_ok_single(const text_ok_t *text_ok, dns_rdataclass_t rdclass, check_struct_conversions(&rdata, structsize); } +/* + * Test whether converting rdata to text form and then parsing the result of + * that conversion again results in the same uncompressed wire form. This + * checks whether totext_*() output is parsable by fromtext_*() for given RR + * class and type. + * + * This function is called for every input RDATA which is successfully parsed + * by check_wire_ok_single() and whose type is not a meta-type. + */ +static void +check_text_conversions(dns_rdata_t *rdata) { + char buf_totext[1024] = { 0 }; + unsigned char buf_fromtext[1024]; + isc_result_t result; + isc_buffer_t target; + dns_rdata_t rdata2 = DNS_RDATA_INIT; + + /* + * Convert uncompressed wire form RDATA into text form. This + * conversion must succeed since input RDATA was successfully + * parsed by check_wire_ok_single(). + */ + isc_buffer_init(&target, buf_totext, sizeof(buf_totext)); + result = dns_rdata_totext(rdata, NULL, &target); + assert_int_equal(result, ISC_R_SUCCESS); + /* + * Ensure buf_totext is properly NUL terminated as dns_rdata_totext() + * may attempt different output formats writing into the apparently + * unused part of the buffer. + */ + isc_buffer_putuint8(&target, 0); + + /* + * Try parsing text form RDATA output by dns_rdata_totext() again. + */ + result = dns_test_rdatafromstring(&rdata2, rdata->rdclass, rdata->type, + buf_fromtext, sizeof(buf_fromtext), + buf_totext, false); + assert_int_equal(result, ISC_R_SUCCESS); + assert_int_equal(rdata2.length, rdata->length); + assert_memory_equal(buf_fromtext, rdata->data, rdata->length); +} + /* * Test whether supplied wire form RDATA is properly handled as being either * valid or invalid for an RR of given rdclass and type. @@ -280,9 +329,15 @@ check_wire_ok_single(const wire_ok_t *wire_ok, dns_rdataclass_t rdclass, /* * If data was parsed correctly, perform two-way conversion checks * between uncompressed wire form and type-specific struct. + * + * If the RR type is not a meta-type, additionally perform two-way + * conversion checks between uncompressed wire form and text form. */ if (result == ISC_R_SUCCESS) { check_struct_conversions(&rdata, structsize); + if (!dns_rdatatype_ismeta(rdata.type)) { + check_text_conversions(&rdata); + } } } From c5b191e78fd151171461cd4f1b72840863c0a792 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 28 Feb 2019 16:58:56 +1100 Subject: [PATCH 2/8] Fix whitespace so that the names align (cherry picked from commit cc5e16e4d3fcbde42d35ed6d6eec8dcab1482d71) --- lib/dns/masterdump.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/dns/masterdump.c b/lib/dns/masterdump.c index 6872dd35ad..0dac5a402a 100644 --- a/lib/dns/masterdump.c +++ b/lib/dns/masterdump.c @@ -89,14 +89,14 @@ struct dns_master_style { */ typedef struct dns_totext_ctx { dns_master_style_t style; - bool class_printed; + bool class_printed; char * linebreak; char linebreak_buf[DNS_TOTEXT_LINEBREAK_MAXLEN]; dns_name_t * origin; dns_name_t * neworigin; dns_fixedname_t origin_fixname; uint32_t current_ttl; - bool current_ttl_valid; + bool current_ttl_valid; dns_ttl_t serve_stale_ttl; } dns_totext_ctx_t; From 4395311280b9a077635c9b02c85c2e65ea29caf6 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 28 Feb 2019 17:00:15 +1100 Subject: [PATCH 3/8] Set 'specials' to match 'specials' in 'lib/dns/master.c' (cherry picked from commit 7941a9554fe00697c81b52051b41912966a1e36a) --- lib/dns/master.c | 4 ++++ lib/dns/tests/dnstest.c | 12 ++++++++++++ 2 files changed, 16 insertions(+) diff --git a/lib/dns/master.c b/lib/dns/master.c index 4002dbc2e7..e703abc357 100644 --- a/lib/dns/master.c +++ b/lib/dns/master.c @@ -571,6 +571,10 @@ loadctx_create(dns_masterformat_t format, isc_mem_t *mctx, if (result != ISC_R_SUCCESS) goto cleanup_inc; lctx->keep_lex = false; + /* + * If specials change update dns_test_rdatafromstring() + * in lib/dns/tests/dnstest.c. + */ memset(specials, 0, sizeof(specials)); specials[0] = 1; specials['('] = 1; diff --git a/lib/dns/tests/dnstest.c b/lib/dns/tests/dnstest.c index 5c448702a7..ea3c694e25 100644 --- a/lib/dns/tests/dnstest.c +++ b/lib/dns/tests/dnstest.c @@ -483,6 +483,7 @@ dns_test_rdatafromstring(dns_rdata_t *rdata, dns_rdataclass_t rdclass, dns_rdatacallbacks_t callbacks; isc_buffer_t source, target; isc_lex_t *lex = NULL; + isc_lexspecials_t specials = { 0 }; isc_result_t result; size_t length; @@ -506,6 +507,17 @@ dns_test_rdatafromstring(dns_rdata_t *rdata, dns_rdataclass_t rdclass, return (result); } + /* + * Set characters which will be treated as valid multi-line RDATA + * delimiters while reading the source string. These should match + * specials from lib/dns/master.c. + */ + specials[0] = 1; + specials['('] = 1; + specials[')'] = 1; + specials['"'] = 1; + isc_lex_setspecials(lex, specials); + /* * Point lexer at source. */ From 14c2db8c5df4933848d5cce64ee346641f92b378 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 11 Apr 2019 18:54:24 +1000 Subject: [PATCH 4/8] Process master file comments and make input invalid again (cherry picked from commit 1a75a5cee6a8c0157cb9ed86361ba4b3f179bdd1) --- lib/dns/tests/dnstest.c | 5 +++++ lib/dns/tests/rdata_test.c | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/dns/tests/dnstest.c b/lib/dns/tests/dnstest.c index ea3c694e25..7f7da0cf21 100644 --- a/lib/dns/tests/dnstest.c +++ b/lib/dns/tests/dnstest.c @@ -518,6 +518,11 @@ dns_test_rdatafromstring(dns_rdata_t *rdata, dns_rdataclass_t rdclass, specials['"'] = 1; isc_lex_setspecials(lex, specials); + /* + * Expect DNS masterfile comments. + */ + isc_lex_setcomments(lex, ISC_LEXCOMMENT_DNSMASTERFILE); + /* * Point lexer at source. */ diff --git a/lib/dns/tests/rdata_test.c b/lib/dns/tests/rdata_test.c index f42cf59a6c..4c14c8f0ad 100644 --- a/lib/dns/tests/rdata_test.c +++ b/lib/dns/tests/rdata_test.c @@ -1895,8 +1895,8 @@ nsec3(void **state) { TEXT_INVALID("."), TEXT_INVALID(". RRSIG"), TEXT_INVALID("1 0 10 76931F"), - TEXT_INVALID("1 0 10 76931F IMQ912BREQP1POLAH3RMONG;UED541AS"), - TEXT_INVALID("1 0 10 76931F IMQ912BREQP1POLAH3RMONG;UED541AS A RRSIG"), + TEXT_INVALID("1 0 10 76931F IMQ912BREQP1POLAH3RMONG&UED541AS"), + TEXT_INVALID("1 0 10 76931F IMQ912BREQP1POLAH3RMONGAUED541AS A RRSIG BADTYPE"), TEXT_VALID("1 0 10 76931F AJHVGTICN6K0VDA53GCHFMT219SRRQLM A RRSIG"), TEXT_VALID("1 0 10 76931F AJHVGTICN6K0VDA53GCHFMT219SRRQLM"), TEXT_VALID("1 0 10 - AJHVGTICN6K0VDA53GCHFMT219SRRQLM"), From 1eb7267e609757b96d9ca0984fa43b60f24c0975 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 28 Feb 2019 17:06:01 +1100 Subject: [PATCH 5/8] Check multi-line output from dns_rdata_tofmttext() Check that multi-line output from dns_rdata_tofmttext() can be read back in by dns_rdata_fromtext(). (cherry picked from commit b089f43b7a4f0c3b51dc88fbe60d9c79b87e9893) --- lib/dns/tests/rdata_test.c | 53 +++++++++++++++++++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/lib/dns/tests/rdata_test.c b/lib/dns/tests/rdata_test.c index 4c14c8f0ad..81d2c2ba82 100644 --- a/lib/dns/tests/rdata_test.c +++ b/lib/dns/tests/rdata_test.c @@ -301,6 +301,53 @@ check_text_conversions(dns_rdata_t *rdata) { assert_memory_equal(buf_fromtext, rdata->data, rdata->length); } +/* + * Test whether converting rdata to multi-line text form and then parsing the + * result of that conversion again results in the same uncompressed wire form. + * This checks whether multi-line totext_*() output is parsable by fromtext_*() + * for given RR class and type. + * + * This function is called for every input RDATA which is successfully parsed + * by check_wire_ok_single() and whose type is not a meta-type. + */ +static void +check_multiline_text_conversions(dns_rdata_t *rdata) { + char buf_totext[1024] = { 0 }; + unsigned char buf_fromtext[1024]; + isc_result_t result; + isc_buffer_t target; + dns_rdata_t rdata2 = DNS_RDATA_INIT; + unsigned int flags; + + /* + * Convert uncompressed wire form RDATA into multi-line text form. + * This conversion must succeed since input RDATA was successfully + * parsed by check_wire_ok_single(). + */ + isc_buffer_init(&target, buf_totext, sizeof(buf_totext)); + flags = dns_master_styleflags(&dns_master_style_default); + result = dns_rdata_tofmttext(rdata, dns_rootname, flags, 80 - 32, 4, + "\n", &target); + assert_int_equal(result, ISC_R_SUCCESS); + /* + * Ensure buf_totext is properly NUL terminated as + * dns_rdata_tofmttext() may attempt different output formats + * writing into the apparently unused part of the buffer. + */ + isc_buffer_putuint8(&target, 0); + + /* + * Try parsing multi-line text form RDATA output by + * dns_rdata_tofmttext() again. + */ + result = dns_test_rdatafromstring(&rdata2, rdata->rdclass, rdata->type, + buf_fromtext, sizeof(buf_fromtext), + buf_totext, false); + assert_int_equal(result, ISC_R_SUCCESS); + assert_int_equal(rdata2.length, rdata->length); + assert_memory_equal(buf_fromtext, rdata->data, rdata->length); +} + /* * Test whether supplied wire form RDATA is properly handled as being either * valid or invalid for an RR of given rdclass and type. @@ -331,12 +378,16 @@ check_wire_ok_single(const wire_ok_t *wire_ok, dns_rdataclass_t rdclass, * between uncompressed wire form and type-specific struct. * * If the RR type is not a meta-type, additionally perform two-way - * conversion checks between uncompressed wire form and text form. + * conversion checks between: + * + * - uncompressed wire form and text form, + * - uncompressed wire form and multi-line text form. */ if (result == ISC_R_SUCCESS) { check_struct_conversions(&rdata, structsize); if (!dns_rdatatype_ismeta(rdata.type)) { check_text_conversions(&rdata); + check_multiline_text_conversions(&rdata); } } } From 86bb6e23cefe0f1cf863159b069f4a11139c9bfe Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 28 Feb 2019 18:04:02 +1100 Subject: [PATCH 6/8] Prevent WIRE_INVALID() being called without a argument (cherry picked from commit e73a5b0ce3c5364ab9ac66587be413bfe51080d8) --- lib/dns/tests/rdata_test.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/lib/dns/tests/rdata_test.c b/lib/dns/tests/rdata_test.c index 81d2c2ba82..55b9b96c25 100644 --- a/lib/dns/tests/rdata_test.c +++ b/lib/dns/tests/rdata_test.c @@ -103,7 +103,14 @@ typedef struct wire_ok { ok \ } #define WIRE_VALID(...) WIRE_TEST(true, __VA_ARGS__) -#define WIRE_INVALID(...) WIRE_TEST(false, __VA_ARGS__) +/* + * WIRE_INVALID() test cases must always have at least one octet specified to + * distinguish them from WIRE_SENTINEL(). Use the 'empty_ok' parameter passed + * to check_wire_ok() for indicating whether empty RDATA is allowed for a given + * RR type or not. + */ +#define WIRE_INVALID(FIRST, ...) \ + WIRE_TEST(false, FIRST, __VA_ARGS__) #define WIRE_SENTINEL() WIRE_TEST(false) /* @@ -1636,11 +1643,8 @@ eid(void **state) { TEXT_SENTINEL() }; wire_ok_t wire_ok[] = { - /* - * Too short. - */ - WIRE_INVALID(), WIRE_VALID(0x00), + WIRE_VALID(0xAA, 0xBB, 0xCC), /* * Sentinel. */ @@ -1829,11 +1833,8 @@ nimloc(void **state) { TEXT_SENTINEL() }; wire_ok_t wire_ok[] = { - /* - * Too short. - */ - WIRE_INVALID(), WIRE_VALID(0x00), + WIRE_VALID(0xAA, 0xBB, 0xCC), /* * Sentinel. */ @@ -1925,7 +1926,7 @@ nsec(void **state) { WIRE_INVALID(0x00, 0x00), WIRE_INVALID(0x00, 0x00, 0x00), WIRE_VALID(0x00, 0x00, 0x01, 0x02), - WIRE_INVALID() + WIRE_SENTINEL() }; UNUSED(state); From b27ef87c389d9c69704ccb22e6d2900cb5b366b9 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 28 Feb 2019 18:04:02 +1100 Subject: [PATCH 7/8] Add debug printfs (cherry picked from commit b78e128a2ff26950bb9ff186b0614279e6f450c2) --- lib/dns/tests/rdata_test.c | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/lib/dns/tests/rdata_test.c b/lib/dns/tests/rdata_test.c index 55b9b96c25..bb2249bd1e 100644 --- a/lib/dns/tests/rdata_test.c +++ b/lib/dns/tests/rdata_test.c @@ -36,6 +36,8 @@ #include "dnstest.h" +static bool debug = false; + /* * An array of these structures is passed to compare_ok(). */ @@ -221,8 +223,14 @@ check_text_ok_single(const text_ok_t *text_ok, dns_rdataclass_t rdclass, * Check whether result is as expected. */ if (text_ok->text_out != NULL) { + if (debug && result != ISC_R_SUCCESS) { + fprintf(stdout, "#'%s'\n", text_ok->text_in); + } assert_int_equal(result, ISC_R_SUCCESS); } else { + if (debug && result == ISC_R_SUCCESS) { + fprintf(stdout, "#'%s'\n", text_ok->text_in); + } assert_int_not_equal(result, ISC_R_SUCCESS); } @@ -247,6 +255,10 @@ check_text_ok_single(const text_ok_t *text_ok, dns_rdataclass_t rdclass, * unused part of the buffer. */ isc_buffer_putuint8(&target, 0); + if (debug && strcmp(buf_totext, text_ok->text_out) != 0) { + fprintf(stdout, "# '%s' != '%s'\n", + buf_totext, text_ok->text_out); + } assert_string_equal(buf_totext, text_ok->text_out); /* @@ -296,6 +308,9 @@ check_text_conversions(dns_rdata_t *rdata) { * unused part of the buffer. */ isc_buffer_putuint8(&target, 0); + if (debug) { + fprintf(stdout, "#'%s'\n", buf_totext); + } /* * Try parsing text form RDATA output by dns_rdata_totext() again. @@ -342,6 +357,9 @@ check_multiline_text_conversions(dns_rdata_t *rdata) { * writing into the apparently unused part of the buffer. */ isc_buffer_putuint8(&target, 0); + if (debug) { + fprintf(stdout, "#'%s'\n", buf_totext); + } /* * Try parsing multi-line text form RDATA output by @@ -2342,7 +2360,7 @@ iszonecutauth(void **state) { } int -main(void) { +main(int argc, char **argv) { const struct CMUnitTest tests[] = { cmocka_unit_test_setup_teardown(amtrelay, _setup, _teardown), cmocka_unit_test_setup_teardown(apl, _setup, _teardown), @@ -2370,6 +2388,12 @@ main(void) { cmocka_unit_test_setup_teardown(iszonecutauth, NULL, NULL), }; + UNUSED(argv); + + if (argc > 1) { + debug = true; + } + return (cmocka_run_group_tests(tests, NULL, NULL)); } From ad73e08b078e156780758bcaf1c5f1e80a8b8ec7 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 21 Mar 2019 22:36:02 +1100 Subject: [PATCH 8/8] Add CHANGES (cherry picked from commit 307a1b563b1c771573ef97e52add98bcff0ea193) --- CHANGES | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES b/CHANGES index 0d0bf39f82..9bd04956d6 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5208. [test] Run valid rdata wire encodings through totext+fromtext + and tofmttext+fromtext methods to check these methods. + [GL #899] + 5207. [test] Check delv and dig TTL values. [GL #965] 5206. [bug] Delv could print out bad TTLs. [GL #965]