From b02081d423dd9f0f038112a6fed32c15dfce248f Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Mon, 23 Mar 2020 10:28:33 +1100 Subject: [PATCH 1/5] Escape double quote when printing quoted string. When we were printing quoted string, the double quotes where unescaped leading to prematurely ending the quoted string. --- lib/isccfg/parser.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/isccfg/parser.c b/lib/isccfg/parser.c index 4cef5bc83f..154311ace4 100644 --- a/lib/isccfg/parser.c +++ b/lib/isccfg/parser.c @@ -1671,7 +1671,12 @@ cfg_print_ustring(cfg_printer_t *pctx, const cfg_obj_t *obj) { static void print_qstring(cfg_printer_t *pctx, const cfg_obj_t *obj) { cfg_print_cstr(pctx, "\""); - cfg_print_ustring(pctx, obj); + for (size_t i = 0; i < obj->value.string.length; i++) { + if (obj->value.string.base[i] == '"') { + cfg_print_cstr(pctx, "\\"); + } + cfg_print_chars(pctx, &obj->value.string.base[i], 1); + } cfg_print_cstr(pctx, "\""); } From 5ab9b5b1e6544d74df3edb6e37b0f3b5797b74c9 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Mon, 23 Mar 2020 11:22:48 +1100 Subject: [PATCH 2/5] Add more special zones to addzone restart test. Test zones with various escape sequences and filesystem seperator characters. * escaped double quote (\") * escaped escape (\\) * escaped decimal byte value (\032) * slash seperator (/) --- bin/tests/system/addzone/tests.sh | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/bin/tests/system/addzone/tests.sh b/bin/tests/system/addzone/tests.sh index 4ecfd73275..e76051ce70 100755 --- a/bin/tests/system/addzone/tests.sh +++ b/bin/tests/system/addzone/tests.sh @@ -719,9 +719,30 @@ $RNDCCMD 10.53.0.3 addzone "test4.baz" '{ type master; file "e.db"; };' > /dev/n $RNDCCMD 10.53.0.3 addzone "test5.baz" '{ type master; file "e.db"; };' > /dev/null 2>&1 || ret=1 $RNDCCMD 10.53.0.3 addzone '"test/.baz"' '{ type master; check-names ignore; file "e.db"; };' > /dev/null 2>&1 || ret=1 $RNDCCMD 10.53.0.3 addzone '"test\".baz"' '{ type master; check-names ignore; file "e.db"; };' > /dev/null 2>&1 || ret=1 +$RNDCCMD 10.53.0.3 addzone '"test\\.baz"' '{ type master; check-names ignore; file "e.db"; };' > /dev/null 2>&1 || ret=1 +$RNDCCMD 10.53.0.3 addzone '"test\032.baz"' '{ type master; check-names ignore; file "e.db"; };' > /dev/null 2>&1 || ret=1 +$RNDCCMD 10.53.0.3 addzone '"test\010.baz"' '{ type master; check-names ignore; file "e.db"; };' > /dev/null 2>&1 || ret=1 stop addzone ns3 start --noclean --restart --port ${PORT} addzone ns3 || ret=1 retry_quiet 10 _check_version_bind || ret=1 +$DIG $DIGOPTS @10.53.0.3 SOA "test4.baz" > dig.out.1.test$n || ret=1 +grep "status: NOERROR" dig.out.1.test$n > /dev/null || ret=1 +grep "ANSWER: 1," dig.out.1.test$n > /dev/null || ret=1 +$DIG $DIGOPTS @10.53.0.3 SOA "test5.baz" > dig.out.2.test$n || ret=1 +grep "status: NOERROR" dig.out.2.test$n > /dev/null || ret=1 +grep "ANSWER: 1," dig.out.2.test$n > /dev/null || ret=1 +$DIG $DIGOPTS @10.53.0.3 SOA 'test/.baz' > dig.out.3.test$n || ret=1 +grep "status: NOERROR" dig.out.3.test$n > /dev/null || ret=1 +grep "ANSWER: 1," dig.out.3.test$n > /dev/null || ret=1 +$DIG $DIGOPTS @10.53.0.3 SOA 'test\\.baz' > dig.out.4.test$n || ret=1 +grep "status: NOERROR" dig.out.4.test$n > /dev/null || ret=1 +grep "ANSWER: 1," dig.out.4.test$n > /dev/null || ret=1 +$DIG $DIGOPTS @10.53.0.3 SOA 'test\032.baz' > dig.out.5.test$n || ret=1 +grep "status: NOERROR" dig.out.5.test$n > /dev/null || ret=1 +grep "ANSWER: 1," dig.out.5.test$n > /dev/null || ret=1 +$DIG $DIGOPTS @10.53.0.3 SOA 'test\010.baz' > dig.out.6.test$n || ret=1 +grep "status: NOERROR" dig.out.6.test$n > /dev/null || ret=1 +grep "ANSWER: 1," dig.out.6.test$n > /dev/null || ret=1 if [ $ret != 0 ]; then echo_i "failed"; fi status=`expr $status + $ret` n=`expr $n + 1` From 060b56dc70f2001fef21ab0eb954663feb62d4d0 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Mon, 23 Mar 2020 12:04:30 +1100 Subject: [PATCH 3/5] Add jobs for 'configure --with{out}-lmdb' to the GitLab CI We missed a case where LMDB was not installed/used in the build and that broke system tests on such systems. --- .gitlab-ci.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 3f04664718..2096ec31b7 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -670,12 +670,13 @@ scan-build: when: on_failure # Jobs for regular GCC builds on Debian Sid (amd64) +# Also tests configration option: --without-lmdb. gcc:sid:amd64: variables: CC: gcc CFLAGS: "${CFLAGS_COMMON} -O3" - EXTRA_CONFIGURE: "--enable-dnstap --with-libidn2" + EXTRA_CONFIGURE: "--enable-dnstap --with-libidn2 --without-lmdb" RUN_MAKE_INSTALL: 1 <<: *debian_sid_amd64_image <<: *build_job @@ -699,13 +700,14 @@ cppcheck: <<: *cppcheck_job # Job for out-of-tree GCC build on Debian Sid (amd64) +# Also tests configration option: --with-lmdb. gcc:out-of-tree: variables: CC: gcc CFLAGS: "${CFLAGS_COMMON} -Og" CONFIGURE: ../configure - EXTRA_CONFIGURE: "--enable-dnstap --with-libidn2" + EXTRA_CONFIGURE: "--enable-dnstap --with-libidn2 --with-lmdb" RUN_MAKE_INSTALL: 1 OOT_BUILD_WORKSPACE: workspace <<: *base_image From a66c6fc8831aa93a6a20a3e1422486332d016f1c Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Mon, 23 Mar 2020 13:27:37 +1100 Subject: [PATCH 4/5] Mimic nzf_append from bin/named/server.c nzf_append is conditionally compiled and this is intended to catch error introduced by changes to the called functions on all systems before the changes are run through the CI. --- lib/isccfg/tests/parser_test.c | 96 ++++++++++++++++++++++++++++++++-- 1 file changed, 91 insertions(+), 5 deletions(-) diff --git a/lib/isccfg/tests/parser_test.c b/lib/isccfg/tests/parser_test.c index e218927704..c0f1df8740 100644 --- a/lib/isccfg/tests/parser_test.c +++ b/lib/isccfg/tests/parser_test.c @@ -27,6 +27,8 @@ #include #include #include +#include +#include #include #include @@ -56,6 +58,7 @@ static isc_logcategory_t categories[] = { { "", 0 }, static void cleanup(void) { if (lctx != NULL) { + isc_log_setcontext(NULL); isc_log_destroy(&lctx); } if (mctx != NULL) { @@ -92,6 +95,92 @@ cleanup: return (result); } +static int +_setup(void **state) { + isc_result_t result; + + UNUSED(state); + + result = setup(); + assert_int_equal(result, ISC_R_SUCCESS); + + return (0); +} + +static int +_teardown(void **state) { + UNUSED(state); + + cleanup(); + + return (0); +} + +/* mimic calling nzf_append() */ +static void +append(void *arg, const char *str, int len) { + char *buf = arg; + size_t l = strlen(buf); + snprintf(buf + l, 1024 - l, "%.*s", len, str); +} + +static void +addzoneconf(void **state) { + isc_result_t result; + isc_buffer_t b; + cfg_parser_t *p = NULL; + const char *tests[] = { + "zone \"test4.baz\" { type master; file \"e.db\"; };", + "zone \"test/.baz\" { type master; file \"e.db\"; };", + "zone \"test\\\".baz\" { type master; file \"e.db\"; };", + "zone \"test\\.baz\" { type master; file \"e.db\"; };", + "zone \"test\\\\.baz\" { type master; file \"e.db\"; };", + "zone \"test\\032.baz\" { type master; file \"e.db\"; };", + "zone \"test\\010.baz\" { type master; file \"e.db\"; };" + }; + char buf[1024]; + + UNUSED(state); + + /* Parse with default line numbering */ + result = cfg_parser_create(mctx, lctx, &p); + assert_int_equal(result, ISC_R_SUCCESS); + +#define ARRAYSIZE(x) (sizeof(x) / sizeof(x[0])) + + for (size_t i = 0; i < ARRAYSIZE(tests); i++) { + cfg_obj_t *conf = NULL; + const cfg_obj_t *obj = NULL, *zlist = NULL; + + isc_buffer_constinit(&b, tests[i], strlen(tests[i])); + isc_buffer_add(&b, strlen(tests[i])); + + result = cfg_parse_buffer(p, &b, "text1", 0, + &cfg_type_namedconf, 0, &conf); + assert_int_equal(result, ISC_R_SUCCESS); + + /* + * Mimic calling nzf_append() from bin/named/server.c + * and check that the output matches the input. + */ + result = cfg_map_get(conf, "zone", &zlist); + assert_int_equal(result, ISC_R_SUCCESS); + + obj = cfg_listelt_value(cfg_list_first(zlist)); + assert_ptr_not_equal(obj, NULL); + + strlcpy(buf, "zone ", sizeof(buf)); + cfg_printx(obj, CFG_PRINTER_ONELINE, append, buf); + strlcat(buf, ";", sizeof(buf)); + assert_string_equal(tests[i], buf); + + cfg_obj_destroy(p, &conf); + cfg_parser_reset(p); + } + + cfg_parser_destroy(&p); +} + /* test cfg_parse_buffer() */ static void parse_buffer_test(void **state) { @@ -103,8 +192,6 @@ parse_buffer_test(void **state) { UNUSED(state); - setup(); - isc_buffer_init(&buf1, &text[0], sizeof(text) - 1); isc_buffer_add(&buf1, sizeof(text) - 1); @@ -134,8 +221,6 @@ parse_buffer_test(void **state) { cfg_parser_destroy(&p1); cfg_parser_destroy(&p2); - - cleanup(); } /* test cfg_map_firstclause() */ @@ -181,12 +266,13 @@ cfg_map_nextclause_test(void **state) { int main(void) { const struct CMUnitTest tests[] = { + cmocka_unit_test(addzoneconf), cmocka_unit_test(parse_buffer_test), cmocka_unit_test(cfg_map_firstclause_test), cmocka_unit_test(cfg_map_nextclause_test), }; - return (cmocka_run_group_tests(tests, NULL, NULL)); + return (cmocka_run_group_tests(tests, _setup, _teardown)); } #else /* HAVE_CMOCKA */ From d63479a48d4e8e84aec9ea00f6d167e0c990a92f Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Mon, 6 Apr 2020 10:33:02 +1000 Subject: [PATCH 5/5] Add CHANGES entry --- CHANGES | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES b/CHANGES index 0dd1b890a6..a0cb38fde5 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5398. [bug] Named could fail to restart if a zone added with + 'rndc addzone' contained a double quote (") in + its name. [GL #1695] + 5397. [func] Update PKCS#11 EdDSA implementation to PKCS#11 v3.0. Thanks to Aaron Thompson. [GL !3326]