Commit c3a94073 authored by Marek Vavrusa's avatar Marek Vavrusa

zone-load: fixed crash when loading non-existing zone file

- refactored the zone-loader a bit, as reading from zone file
  creates only contents and the zone is created separately
- refactored zone contents creating function, as it is always used in a
  way where an apex node is created and inserted into contents
- crash occured because created zone bound configuration and if parsing
  failed, the configuration was freed with the zone and also with the
  caller
parent 6c365493
......@@ -35,7 +35,7 @@
#include "knot/server/socket.h"
#include "knot/server/tcp-handler.h"
#include "libknot/packet/wire.h"
#include "knot/zone/zone-load.h"
#include "knot/server/zone-load.h"
#include "knot/zone/estimator.h"
/*! \brief Controller flags. */
......@@ -672,29 +672,15 @@ static int cmd_checkzone(int argc, char *argv[], unsigned flags)
}
/* Create zone loader context. */
zloader_t *l = NULL;
int ret = knot_zload_open(&l, zone);
if (ret != KNOT_EOK) {
log_zone_error("Could not open zone %s (%s).\n",
zone->name, knot_strerror(ret));
knot_zload_close(l);
rc = 1;
continue;
}
zone_t *z = knot_zload_load(l);
if (z == NULL) {
log_zone_error("Loading of zone %s failed.\n",
zone->name);
knot_zload_close(l);
zone_t *loaded_zone = load_zone_file(zone);
if (loaded_zone == NULL) {
rc = 1;
continue;
}
log_zone_info("Zone %s OK.\n", zone->name);
rem_node((node_t *)zone);
zone_free(&z);
knot_zload_close(l);
zone_free(&loaded_zone);
}
return rc;
......
......@@ -677,7 +677,7 @@ int server_update_zones(const struct conf_t *conf, void *data)
{
server_t *server = (server_t *)data;
int ret = zones_update_db_from_config(conf, server);
int ret = load_zones_from_config(conf, server);
if (ret != KNOT_EOK) {
return ret;
}
......
......@@ -85,7 +85,7 @@ static zone_t *bootstrap_zone(conf_zone_t *conf)
bool bootstrap = !EMPTY_LIST(conf->acl.xfr_in);
if (!bootstrap) {
return NULL;
return load_zone_file(conf); /* No master for this zone, fallback. */
}
zone_t *new_zone = zone_new(conf);
......@@ -98,66 +98,50 @@ static zone_t *bootstrap_zone(conf_zone_t *conf)
return new_zone;
}
/*!
* \brief Load zone.
*
* \param conf Zone configuration.
*
* \return Loaded zone, NULL in case of error.
*/
static zone_t *load_zone(conf_zone_t *conf)
zone_t *load_zone_file(conf_zone_t *conf)
{
assert(conf);
zloader_t *zl = NULL;
zone_t *zone = NULL;
/* Open zone file for parsing. */
switch (knot_zload_open(&zl, conf)) {
case KNOT_EOK:
break;
case KNOT_EACCES:
log_zone_error("No access/permission to zone file '%s'.\n", conf->file);
knot_zload_close(zl);
return NULL;
default:
log_zone_error("Failed to load zone file '%s'\n", conf->file);
knot_zload_close(zl);
zloader_t zl;
int ret = zonefile_open(&zl, conf);
if (ret != KNOT_EOK) {
log_zone_error("Failed to open zone file '%s': %s\n",
conf->file, knot_strerror(ret));
return NULL;
}
/* Check the source file */
assert(zl != NULL);
zone = knot_zload_load(zl);
if (!zone) {
log_zone_error("Zone '%s' could not be loaded.\n", conf->name);
knot_zload_close(zl);
struct stat st;
if (stat(conf->file, &st) < 0) {
/* Go silently and reset mtime to 0. */
memset(&st, 0, sizeof(struct stat));
}
/* Load the zone contents. */
knot_zone_contents_t *zone_contents = zonefile_load(&zl);
zonefile_close(&zl);
/* Check the loader result. */
if (zone_contents == NULL) {
log_zone_error("Failed to load zone file '%s'.\n", conf->file);
return NULL;
}
/* Check if loaded origin matches. */
knot_dname_t *dname_req = NULL;
dname_req = knot_dname_from_str(conf->name);
if (knot_dname_cmp(zone->name, dname_req) != 0) {
log_zone_error("Zone '%s': mismatching origin in the zone file.\n",
conf->name);
zone_free(&zone);
/* Create the new zone. */
zone_t *zone = zone_new((conf_zone_t *)conf);
if (zone == NULL) {
log_zone_error("Failed to create zone '%s': %s\n",
conf->name, knot_strerror(KNOT_ENOMEM));
knot_zone_contents_deep_free(&zone_contents);
return NULL;
} else {
/* Save the timestamp from the zone db file. */
struct stat st;
if (stat(conf->file, &st) < 0) {
dbg_zones("zones: failed to stat() zone db, "
"something is seriously wrong\n");
zone_free(&zone);
return NULL;
} else {
zone->zonefile_mtime = st.st_mtime;
zone->zonefile_serial = knot_zone_serial(zone->contents);
}
}
knot_dname_free(&dname_req);
knot_zload_close(zl);
/* Link zone contents to zone. */
zone->contents = zone_contents;
/* Save the timestamp from the zone db file. */
zone->zonefile_mtime = st.st_mtime;
zone->zonefile_serial = knot_zone_serial(zone->contents);
return zone;
}
......@@ -237,7 +221,7 @@ static zone_t *create_zone(zone_t *old_zone, conf_zone_t *conf, server_t *server
break;
case ZONE_STATUS_FOUND_NEW:
case ZONE_STATUS_FOUND_UPDATED:
new_zone = load_zone(conf);
new_zone = load_zone_file(conf);
break;
case ZONE_STATUS_FOUND_CURRENT:
new_zone = preserve_zone(conf, old_zone);
......@@ -520,7 +504,7 @@ static int remove_old_zonedb(const knot_zonedb_t *db_new, knot_zonedb_t *db_old)
/*!
* \brief Update zone database according to configuration.
*/
int zones_update_db_from_config(const conf_t *conf, struct server_t *server)
int load_zones_from_config(const conf_t *conf, struct server_t *server)
{
/* Check parameters */
if (conf == NULL || server == NULL) {
......
......@@ -30,6 +30,15 @@
struct server_t;
/*!
* \brief Load zone from zone file.
*
* \param conf Zone configuration.
*
* \return Loaded zone, NULL in case of error.
*/
zone_t *load_zone_file(conf_zone_t *conf);
/*!
* \brief Update zone database according to configuration.
*
......@@ -50,7 +59,7 @@ struct server_t;
* \retval KNOT_EINVAL
* \retval KNOT_ERROR
*/
int zones_update_db_from_config(const conf_t *conf, struct server_t *server);
int load_zones_from_config(const conf_t *conf, struct server_t *server);
#endif // _KNOTD_ZONE_LOAD_H_
......
......@@ -448,14 +448,6 @@ int xfrin_process_axfr_packet(knot_ns_xfr_t *xfr)
return KNOT_EMALF;
}
node = knot_node_new(rr->owner, NULL, 0);
if (node == NULL) {
dbg_xfrin("Failed to create new node.\n");
knot_pkt_free(&packet);
knot_rrset_deep_free(&rr, 1);
return KNOT_ENOMEM;
}
// create the zone
*constr = (xfrin_constructed_zone_t *)malloc(
......@@ -471,14 +463,16 @@ int xfrin_process_axfr_packet(knot_ns_xfr_t *xfr)
memset(*constr, 0, sizeof(xfrin_constructed_zone_t));
dbg_xfrin_verb("Creating new zone contents.\n");
(*constr)->contents = knot_zone_contents_new(node, xfr->zone);
(*constr)->contents = knot_zone_contents_new(rr->owner);
if ((*constr)->contents== NULL) {
dbg_xfrin("Failed to create new zone.\n");
knot_pkt_free(&packet);
knot_node_free(&node);
knot_rrset_deep_free(&rr, 1);
/*! \todo Cleanup. */
return KNOT_ENOMEM;
} else {
/* \note Node is already owned by zone contents. */
node = (*constr)->contents->apex;
}
in_zone = 1;
......@@ -491,7 +485,6 @@ int xfrin_process_axfr_packet(knot_ns_xfr_t *xfr)
dbg_xfrin("Failed to add RRSet to zone node: %s.\n",
knot_strerror(ret));
knot_pkt_free(&packet);
knot_node_free(&node);
knot_rrset_deep_free(&rr, 1);
/*! \todo Cleanup. */
return KNOT_ERROR;
......
......@@ -417,51 +417,43 @@ static int knot_zc_nsec3_parameters_match(const knot_rrset_t *rrset,
/* API functions */
/*----------------------------------------------------------------------------*/
knot_zone_contents_t *knot_zone_contents_new(knot_node_t *apex,
struct zone_t *zone)
knot_zone_contents_t *knot_zone_contents_new(const knot_dname_t *apex_name)
{
knot_zone_contents_t *contents = (knot_zone_contents_t *)
calloc(1, sizeof(knot_zone_contents_t));
dbg_zone("%s(%p)\n", __func__, apex_name);
if (apex_name == NULL) {
return NULL;
}
knot_zone_contents_t *contents = malloc(sizeof(knot_zone_contents_t));
if (contents == NULL) {
ERR_ALLOC_FAILED;
return NULL;
}
contents->apex = apex;
memset(contents, 0, sizeof(knot_zone_contents_t));
contents->node_count = 1;
contents->apex = knot_node_new(apex_name, NULL, 0);
if (contents->apex == NULL) {
goto cleanup;
}
dbg_zone_verb("Creating tree for normal nodes.\n");
contents->nodes = knot_zone_tree_create();
if (contents->nodes == NULL) {
ERR_ALLOC_FAILED;
goto cleanup;
}
dbg_zone_verb("Creating tree for NSEC3 nodes.\n");
contents->nsec3_nodes = knot_zone_tree_create();
if (contents->nsec3_nodes == NULL) {
ERR_ALLOC_FAILED;
goto cleanup;
}
/* Initialize NSEC3 params */
dbg_zone_verb("Initializing NSEC3 parameters.\n");
contents->nsec3_params.algorithm = 0;
contents->nsec3_params.flags = 0;
contents->nsec3_params.iterations = 0;
contents->nsec3_params.salt_length = 0;
contents->nsec3_params.salt = NULL;
dbg_zone_verb("Inserting apex into the zone tree.\n");
if (knot_zone_tree_insert(contents->nodes, apex) != KNOT_EOK) {
dbg_zone("Failed to insert apex to the zone tree.\n");
if (knot_zone_tree_insert(contents->nodes, contents->apex) != KNOT_EOK) {
goto cleanup;
}
return contents;
cleanup:
dbg_zone_verb("Cleaning up.\n");
dbg_zone("%s: failure to initialize contents %p\n", __func__, contents);
free(contents->nodes);
free(contents->nsec3_nodes);
free(contents);
......
......@@ -85,8 +85,7 @@ typedef int (*knot_zone_contents_apply_cb_t)(knot_node_t *node, void *data);
/*----------------------------------------------------------------------------*/
knot_zone_contents_t *knot_zone_contents_new(knot_node_t *apex,
struct zone_t *zone);
knot_zone_contents_t *knot_zone_contents_new(const knot_dname_t *apex_name);
int knot_zone_contents_gen_is_old(const knot_zone_contents_t *contents);
int knot_zone_contents_gen_is_new(const knot_zone_contents_t *contents);
......
......@@ -239,7 +239,7 @@ static void process_rr(const scanner_t *scanner)
return;
}
knot_zone_contents_t *contents = parser->current_zone->contents;
knot_zone_contents_t *contents = parser->current_zone;
/*!
* \todo Node/RRSet compression at this level? To avoid duplicate
......@@ -476,7 +476,7 @@ static void process_rr(const scanner_t *scanner)
assert(contents && node);
/* Do mandatory semantic checks. */
bool sem_fatal_error = false;
ret = sem_check_node_plain(parser->current_zone->contents, node,
ret = sem_check_node_plain(parser->current_zone, node,
parser->err_handler, true,
&sem_fatal_error);
if (ret != KNOT_EOK) {
......@@ -501,82 +501,71 @@ static void process_rr(const scanner_t *scanner)
parser->ret = KNOT_EOK;
}
int knot_zload_open(zloader_t **dst, const conf_zone_t *conf)
int zonefile_open(zloader_t *loader, const conf_zone_t *conf)
{
if (!dst || !conf) {
dbg_zload("zload: open: Bad arguments.\n");
if (!loader || !conf) {
return KNOT_EINVAL;
}
*dst = NULL;
/* Check zone file. */
if (access(conf->file, F_OK | R_OK) != 0) {
return KNOT_EACCES;
}
/* Create context. */
parser_context_t *context = xmalloc(sizeof(parser_context_t));
/* Create trie for DNAME duplicits. */
context->lookup_tree = hattrie_create();
if (context->lookup_tree == NULL) {
free(context);
parser_context_t *context = malloc(sizeof(parser_context_t));
if (context == NULL) {
return KNOT_ENOMEM;
}
/* Create the new zone. */
zone_t *zone = zone_new((conf_zone_t *)conf);
int result = zone_create_contents(zone);
if (result != KNOT_EOK) {
zone_free(&zone);
free(context);
return result;
/* As it's a first node, no need for compression yet. */
memset(context, 0, sizeof(parser_context_t));
context->origin_from_config = knot_dname_from_str(conf->name);
if (context->origin_from_config == NULL) {
goto fail;
}
/* As it's a first node, no need for compression yet. */
context->origin_from_config = knot_dname_copy(zone->name);
context->current_zone = zone;
context->last_node = zone->contents->apex;
context->node_rrsigs = NULL;
context->ret = KNOT_EOK;
/* Create trie for DNAME duplicits. */
context->lookup_tree = hattrie_create();
if (context->lookup_tree == NULL) {
goto fail;
}
/* Create file loader. */
file_loader_t *loader = file_loader_create(conf->file, conf->name,
KNOT_CLASS_IN, 3600,
process_rr, process_error,
context);
if (loader == NULL) {
dbg_zload("Could not initialize zone parser.\n");
hattrie_free(context->lookup_tree);
free(context);
return KNOT_ERROR;
}
/* Allocate new loader. */
zloader_t *zl = xmalloc(sizeof(zloader_t));
zl->source = strdup(conf->file);
zl->origin = strdup(conf->name);
zl->file_loader = loader;
zl->context = context;
zl->semantic_checks = conf->enable_checks;
*dst = zl;
zl->err_handler = err_handler_new();
if (zl->err_handler == NULL) {
memset(loader, 0, sizeof(zloader_t));
loader->file_loader = file_loader_create(conf->file, conf->name,
KNOT_CLASS_IN, 3600,
process_rr, process_error,
context);
if (loader->file_loader == NULL) {
goto fail;
}
/* Initialize new loader. */
loader->source = strdup(conf->file);
loader->origin = strdup(conf->name);
loader->context = context;
loader->semantic_checks = conf->enable_checks;
loader->err_handler = err_handler_new();
if (loader->err_handler == NULL) {
/* Not a reason to stop. */
log_zone_warning("Could not create semantic checks handler. "
"Semantic check skipped for zone %s\n",
conf->name);
}
context->err_handler = zl->err_handler;
context->err_handler = loader->err_handler;
return KNOT_EOK;
fail:
knot_dname_free(&context->origin_from_config);
hattrie_free(context->lookup_tree);
free(context);
return KNOT_ENOMEM;
}
zone_t *knot_zload_load(zloader_t *loader)
knot_zone_contents_t *zonefile_load(zloader_t *loader)
{
dbg_zload("zload: load: Loading zone, loader: %p.\n", loader);
if (!loader) {
......@@ -584,9 +573,16 @@ zone_t *knot_zload_load(zloader_t *loader)
return NULL;
}
/* Initialize empty zone contents. */
parser_context_t *c = loader->context;
knot_node_t *apex = c->current_zone->contents->apex;
assert(c);
c->current_zone = knot_zone_contents_new(c->origin_from_config);
if (c->current_zone == NULL) {
return NULL;
}
knot_node_t *apex = c->last_node = c->current_zone->apex;
/* Load zone contents. */
int ret = file_loader_process(loader->file_loader);
if (ret != ZSCANNER_OK) {
log_zone_error("%s: zone file could not be loaded (%s).\n",
......@@ -594,15 +590,13 @@ zone_t *knot_zload_load(zloader_t *loader)
}
if (c->last_node && c->node_rrsigs) {
process_rrsigs_in_node(c, c->current_zone->contents, c->last_node);
process_rrsigs_in_node(c, c->current_zone, c->last_node);
}
if (c->ret != KNOT_EOK) {
log_zone_error("%s: zone file could not be loaded (%s).\n",
loader->source, zscanner_strerror(c->ret));
rrset_list_delete(&c->node_rrsigs);
zone_free(&c->current_zone);
return NULL;
goto fail;
}
if (loader->file_loader->scanner->error_counter > 0) {
......@@ -610,30 +604,24 @@ zone_t *knot_zload_load(zloader_t *loader)
"%"PRIu64" errors encountered.\n",
loader->source,
loader->file_loader->scanner->error_counter);
rrset_list_delete(&c->node_rrsigs);
zone_free(&c->current_zone);
return NULL;
goto fail;
}
if (knot_node_rrset(apex, KNOT_RRTYPE_SOA) == NULL) {
log_zone_error("%s: no SOA record in the zone file.\n",
loader->source);
rrset_list_delete(&c->node_rrsigs);
zone_free(&c->current_zone);
return NULL;
goto fail;
}
knot_node_t *first_nsec3_node = NULL;
knot_node_t *last_nsec3_node = NULL;
rrset_list_delete(&c->node_rrsigs);
int kret = knot_zone_contents_adjust_full(c->current_zone->contents, &first_nsec3_node,
&last_nsec3_node);
int kret = knot_zone_contents_adjust_full(c->current_zone,
&first_nsec3_node, &last_nsec3_node);
if (kret != KNOT_EOK) {
log_zone_error("%s: Failed to finalize zone contents: %s\n",
loader->source, knot_strerror(kret));
rrset_list_delete(&c->node_rrsigs);
zone_free(&c->current_zone);
return NULL;
goto fail;
}
if (loader->semantic_checks) {
......@@ -647,7 +635,7 @@ zone_t *knot_zload_load(zloader_t *loader)
} else if (soa_rr->rrsigs && nsec3param_rr) {
check_level = 3;
}
zone_do_sem_checks(c->current_zone->contents, check_level,
zone_do_sem_checks(c->current_zone, check_level,
loader->err_handler, first_nsec3_node,
last_nsec3_node);
char *zname = knot_dname_to_str(knot_rrset_owner(soa_rr));
......@@ -656,9 +644,14 @@ zone_t *knot_zload_load(zloader_t *loader)
}
return c->current_zone;
fail:
rrset_list_delete(&c->node_rrsigs);
knot_zone_contents_deep_free(&c->current_zone);
return NULL;
}
void knot_zload_close(zloader_t *loader)
void zonefile_close(zloader_t *loader)
{
if (!loader) {
return;
......@@ -674,5 +667,4 @@ void knot_zload_close(zloader_t *loader)
free(loader->origin);
free(loader->context);
free(loader->err_handler);
free(loader);
}
......@@ -46,7 +46,7 @@ typedef struct rrset_list rrset_list_t;
struct parser_context {
rrset_list_t *node_rrsigs;
zone_t *current_zone;
knot_zone_contents_t *current_zone;
knot_rrset_t *current_rrset;
knot_dname_t *origin_from_config;
knot_node_t *last_node;
......@@ -72,32 +72,32 @@ typedef struct zloader_t
} zloader_t;
/*!
* \brief Initializes zone loader from file.
* \brief Open zone file for loading.
*
* \param dst Output zone loader.
* \param zl Output zone loader.
* \param conf Zone configuration.
*
* \retval Initialized loader on success.
* \retval NULL on error.
*/
int knot_zload_open(zloader_t **dst, const conf_zone_t *conf);
int zonefile_open(zloader_t *loader, const conf_zone_t *conf);
/*!
* \brief Loads zone from a compiled and serialized zone file.
* \brief Loads zone from a zone file.
*
* \param loader Zone loader instance.
*
* \retval Loaded zone on success.
* \retval Loaded zone contents on success.
* \retval NULL otherwise.
*/
zone_t *knot_zload_load(zloader_t *loader);
knot_zone_contents_t *zonefile_load(zloader_t *loader);
/*!
* \brief Free zone loader.
* \brief Close zone file loader.
*
* \param loader Zone loader instance.
*/
void knot_zload_close(zloader_t *loader);
void zonefile_close(zloader_t *loader);
void process_error(const scanner_t *scanner);
......
......@@ -262,27 +262,6 @@ zone_t* zone_new(conf_zone_t *conf)
return zone;
}
int zone_create_contents(zone_t *zone)
{
if (!zone) {
return KNOT_EINVAL;
}
knot_node_t *apex = knot_node_new(zone->name, NULL, 0);
if (!apex) {
return KNOT_ENOMEM;
}
knot_zone_contents_t *contents = knot_zone_contents_new(apex, zone);
if (!contents) {
knot_node_free(&apex);
return KNOT_ENOMEM;
}
zone->contents = contents;
return KNOT_EOK;
}
void zone_free(zone_t **zone_ptr)
{
if (zone_ptr == NULL || *zone_ptr == NULL) {
......
......@@ -116,15 +116,6 @@ typedef struct zone_t {
*/
zone_t *zone_new(conf_zone_t *conf);