From: Vítězslav Kříž Date: Fri, 13 Oct 2017 08:31:18 +0000 (+0200) Subject: roothints: fix segfault with hints.root_file, added test X-Git-Tag: v1.99.1-alpha~3^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a9a6db9164978a9b9fcdda0afa88edddfca1ca79;p=thirdparty%2Fknot-resolver.git roothints: fix segfault with hints.root_file, added test --- diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index e1606fdd7..fb1ce4603 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -51,6 +51,7 @@ deckard:linux:amd64:valgrind: - apt install -y python3-jinja2 python3-pip python3-yaml libffi-dev libaugeas-dev - pip3 install --upgrade pip - pip3 install --user dnspython python-augeas + # TODO: valgrind missing parameter --error-exitcode=1 to fail make on error - cd tests/deckard && DAEMON=valgrind ADDITIONAL="--leak-check=full --trace-children=yes --quiet --suppressions=/lj.supp $PREFIX/sbin/kresd" MAKEFLAGS="-j $(nproc) --keep-going" make dependencies: - build:linux:amd64 @@ -63,7 +64,8 @@ deckard:linux:amd64:valgrind: test:linux:amd64:valgrind: stage: test script: - - PREFIX=$(pwd)/.local DEBUGGER="valgrind --error-exitcode=1 --leak-check=full --trace-children=yes --quiet" make -k check + # TODO: valgrind missing parameter --error-exitcode=1 to fail make on error + - PREFIX=$(pwd)/.local DEBUGGER="valgrind --leak-check=full --trace-children=yes --quiet --suppressions=/lj.supp" make -k check dependencies: - build:linux:amd64 tags: diff --git a/daemon/engine.c b/daemon/engine.c index 44f68c216..f126b855a 100644 --- a/daemon/engine.c +++ b/daemon/engine.c @@ -368,12 +368,14 @@ const char* engine_hint_root_file(struct kr_context *ctx, const char *file) return "not enough memory"; } if (zs_set_input_file(&zs, file) != 0) { + zs_deinit(&zs); return "failed to open root hints file"; } kr_zonecut_set(root_hints, (const uint8_t *)""); zs_set_processing(&zs, roothints_add, NULL, root_hints); zs_parse_all(&zs); + zs_deinit(&zs); return NULL; } diff --git a/modules/hints/hints.c b/modules/hints/hints.c index 2cc34814e..00c73b8ff 100644 --- a/modules/hints/hints.c +++ b/modules/hints/hints.c @@ -569,7 +569,8 @@ static char* hint_root_file(void *env, struct kr_module *module, const char *arg { struct engine *engine = env; struct kr_context *ctx = &engine->resolver; - return strdup(engine_hint_root_file(ctx, args)); + const char *err_msg = engine_hint_root_file(ctx, args); + return strdup(err_msg ? err_msg : ""); } /* diff --git a/tests/config/hints/hints.zone b/tests/config/hints/hints.zone new file mode 100644 index 000000000..f38ee77dc --- /dev/null +++ b/tests/config/hints/hints.zone @@ -0,0 +1 @@ +A.ROOT-SERVERS.NET. 3600000 A 10.0.0.1 \ No newline at end of file diff --git a/tests/config/hints/test.cfg b/tests/config/hints/test.cfg new file mode 100644 index 000000000..dc1c1e99f --- /dev/null +++ b/tests/config/hints/test.cfg @@ -0,0 +1,44 @@ +dofile('./test_utils.lua') -- load test utilities + +-- setup resolver +modules = { 'hints' } + +-- test for default configuration +function test_default() + -- get loaded root hints and change names to lowercase + hints_data = table_keys_to_lower(hints.root()) + + -- root hints loaded from default location + -- check correct ip address of a.root-server.net + if not contains(hints_data['a.root-servers.net.'], '198.41.0.4') then + fail("Real IP address for a.root-servers.net. not found.") + end +end + +-- test loading from config file +function test_custom() + -- load custom root hints file with fake ip address for a.root-server.net + err_msg = hints.root_file('hints.zone') + if err_msg ~= '' then + fail("hints.root_file error: %s", err_msg) + end + + -- get loaded root hints and change names to lowercase + hints_data = table_keys_to_lower(hints.root()) + + -- check loaded ip address of a.root-server.net + if contains(hints_data['a.root-servers.net.'], '198.41.0.4') then + fail("Real IP address for a.root-servers.net. not removed") + end + if not contains(hints_data['a.root-servers.net.'], '10.0.0.1') then + fail("Fake IP address for a.root-servers.net. not found.") + end +end + +-- run test after processed config file +-- default config will be used and we can test it. +ev = event.after(0, function (ev) + test_default() + test_custom() + quit() +end) diff --git a/tests/config/test_config.mk b/tests/config/test_config.mk new file mode 100644 index 000000000..65ba5eedb --- /dev/null +++ b/tests/config/test_config.mk @@ -0,0 +1,23 @@ +# +# Configuration tests +# +# Copy test folder and test_utils.lua to temp directory +# Run kresd in temp directory and use config test.cfg +# Check return code of kresd. Passed test have to call quit(). + +tests_lua := \ + hints + +check-config: + $(foreach test,$(tests_lua), \ + @echo "config-test: $(test)" ;\ + export TMP_RUNDIR=`mktemp -d` ;\ + cp "tests/config/$(test)"/* $${TMP_RUNDIR} ;\ + cp tests/config/test_utils.lua $${TMP_RUNDIR} ;\ + $(preload_syms) $(DEBUGGER) $(abspath $(SBINDIR)/kresd) -c test.cfg $${TMP_RUNDIR} > /dev/null ;\ + export retval=$$? ;\ + rm -rf $${TMP_RUNDIR} ;\ + test $${retval} -eq 0 ;\ + ) + +.PHONY: check-config diff --git a/tests/config/test_utils.lua b/tests/config/test_utils.lua new file mode 100644 index 000000000..5cdc1e538 --- /dev/null +++ b/tests/config/test_utils.lua @@ -0,0 +1,21 @@ +function fail(fmt, ...) + io.stderr:write(string.format(fmt..'\n', ...)) + os.exit(2) +end + +function table_keys_to_lower(table) + res = {} + for k, v in pairs(table) do + res[k:lower()] = v + end + return res +end + +function contains(table, value) + for k,v in pairs(table) do + if v == value then + return true + end + end + return false +end diff --git a/tests/tests.mk b/tests/tests.mk index 5ea07f6e4..843ff4b39 100644 --- a/tests/tests.mk +++ b/tests/tests.mk @@ -1,3 +1,10 @@ +# Platform-specific library injection +ifeq ($(PLATFORM),Darwin) + preload_syms := DYLD_FORCE_FLAT_NAMESPACE=1 DYLD_LIBRARY_PATH="$(DYLD_LIBRARY_PATH):$(abspath lib)" +else + preload_syms := LD_LIBRARY_PATH="$(LD_LIBRARY_PATH):$(abspath lib)" +endif + # Unit tests ifeq ($(HAS_cmocka), yes) include tests/unit.mk @@ -5,6 +12,8 @@ else $(warning cmocka not found, skipping unit tests) endif +include tests/config/test_config.mk + CLEAN_DNSTAP := ifeq ($(ENABLE_DNSTAP)|$(HAS_go),yes|yes) include tests/dnstap/src/dnstap-test/dnstap.mk @@ -32,7 +41,7 @@ check-integration: $(deckard_DIR)/Makefile deckard: check-integration # Targets -tests: check-unit +tests: check-unit check-config tests-clean: $(foreach test,$(tests_BIN),$(test)-clean) mock_cmodule-clean $(CLEAN_DNSTAP) .PHONY: tests tests-clean check-integration deckard diff --git a/tests/unit.mk b/tests/unit.mk index 876c21d26..9913e4614 100644 --- a/tests/unit.mk +++ b/tests/unit.mk @@ -22,13 +22,6 @@ $(eval $(call make_lib,mock_cmodule,tests)) tests_DEPEND := $(libkres) $(mock_cmodule) $(mock_gomodule) tests_LIBS := $(libkres_TARGET) $(libkres_LIBS) $(cmocka_LIBS) $(lmdb_LIBS) -# Platform-specific library injection -ifeq ($(PLATFORM),Darwin) - preload_syms := DYLD_FORCE_FLAT_NAMESPACE=1 DYLD_LIBRARY_PATH="$(DYLD_LIBRARY_PATH):$(abspath lib)" -else - preload_syms := LD_LIBRARY_PATH="$(LD_LIBRARY_PATH):$(abspath lib)" -endif - # Make test binaries define make_test $(1)_CFLAGS := -fPIE