From 8a7255c9fc8f6b25d2c92d7feb2259b8521e79c1 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 70e2499cf1..b7b66f3d37 100644 --- a/lib/dns/tests/rdata_test.c +++ b/lib/dns/tests/rdata_test.c @@ -236,6 +236,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); /* @@ -254,6 +260,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. @@ -282,9 +331,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 2c5652067f7151b150191185c3a5a6450b608d09 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 e9fa15d39a..7edef6ad9b 100644 --- a/lib/dns/masterdump.c +++ b/lib/dns/masterdump.c @@ -86,14 +86,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_totext_ctx_t; LIBDNS_EXTERNAL_DATA const dns_master_style_t From 1a036f324fa12f42587f81e671f9bf20b499e317 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 d6e13ffe6b..2a87bca3bc 100644 --- a/lib/dns/master.c +++ b/lib/dns/master.c @@ -574,6 +574,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 3658696ce7..e0f668fef1 100644 --- a/lib/dns/tests/dnstest.c +++ b/lib/dns/tests/dnstest.c @@ -510,6 +510,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; @@ -533,6 +534,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 c6ca84a0c896045944e3a4697b9683e25f7b96d0 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 e0f668fef1..4f49b30709 100644 --- a/lib/dns/tests/dnstest.c +++ b/lib/dns/tests/dnstest.c @@ -545,6 +545,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 b7b66f3d37..0dd7042986 100644 --- a/lib/dns/tests/rdata_test.c +++ b/lib/dns/tests/rdata_test.c @@ -1941,8 +1941,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 478de1f76172d598bd2c568ff99a0ca1b88ad4b0 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 0dd7042986..dbb477209f 100644 --- a/lib/dns/tests/rdata_test.c +++ b/lib/dns/tests/rdata_test.c @@ -303,6 +303,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. @@ -333,12 +380,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 f3922dd9c17297808244b348681831e906906420 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 dbb477209f..d5d65604d3 100644 --- a/lib/dns/tests/rdata_test.c +++ b/lib/dns/tests/rdata_test.c @@ -105,7 +105,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) /* @@ -1682,11 +1689,8 @@ eid(void **state) { TEXT_SENTINEL() }; wire_ok_t wire_ok[] = { - /* - * Too short. - */ - WIRE_INVALID(), WIRE_VALID(0x00), + WIRE_VALID(0xAA, 0xBB, 0xCC), /* * Sentinel. */ @@ -1875,11 +1879,8 @@ nimloc(void **state) { TEXT_SENTINEL() }; wire_ok_t wire_ok[] = { - /* - * Too short. - */ - WIRE_INVALID(), WIRE_VALID(0x00), + WIRE_VALID(0xAA, 0xBB, 0xCC), /* * Sentinel. */ @@ -1971,7 +1972,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 cba5989651af0b18c3b42e3581fecc9e8d74c0f2 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 d5d65604d3..de3bc89782 100644 --- a/lib/dns/tests/rdata_test.c +++ b/lib/dns/tests/rdata_test.c @@ -38,6 +38,8 @@ #include "dnstest.h" +static bool debug = false; + /* * An array of these structures is passed to compare_ok(). */ @@ -223,8 +225,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); } @@ -249,6 +257,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); /* @@ -298,6 +310,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. @@ -344,6 +359,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 @@ -2374,7 +2392,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), @@ -2402,6 +2420,12 @@ main(void) { cmocka_unit_test_setup_teardown(iszonecutauth, NULL, NULL), }; + UNUSED(argv); + + if (argc > 1) { + debug = true; + } + return (cmocka_run_group_tests(tests, dns_test_init, dns_test_final)); } From d37c85a30298090698b8629296a3d124aee6c0db 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 4ed4aafdeb..514432eda5 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] 5205. [bug] Enforce that a DS hash exists. [GL #899]