From 80a87f1708dfe46e5693fffa28b9c46f65c271c8 Mon Sep 17 00:00:00 2001 From: Yuri Schaeffer Date: Fri, 14 Feb 2014 13:58:05 +0100 Subject: [PATCH 1/5] Rework daemon code. Fix reloading, reread xmls on reload. Handle ^C (SIGINT) signal, make enforce task interruptable. Prevent 2 enforce tasks running concurrently. Allow setup to interrupt other tasks No privdrop on reload Don't reload all params in config --- enforcer-ng/src/daemon/cfg.c | 55 ++- enforcer-ng/src/daemon/cfg.h | 2 +- enforcer-ng/src/daemon/cmdhandler.c | 8 +- enforcer-ng/src/daemon/engine.c | 449 ++++++------------ enforcer-ng/src/daemon/engine.h | 34 +- enforcer-ng/src/daemon/signal.c | 27 +- enforcer-ng/src/daemon/signal.h | 5 - enforcer-ng/src/enforcer/autostart_cmd.cpp | 38 +- enforcer-ng/src/enforcer/autostart_cmd.h | 2 + enforcer-ng/src/enforcer/enforce_cmd.cpp | 21 +- enforcer-ng/src/enforcer/enforce_task.cpp | 23 +- enforcer-ng/src/enforcer/enforce_task.h | 2 +- enforcer-ng/src/enforcer/setup_cmd.cpp | 82 ++-- enforcer-ng/src/enforcer/update_all_cmd.cpp | 43 +- .../enforcer/update_repositorylist_task.cpp | 7 +- .../src/enforcer/update_repositorylist_task.h | 9 +- enforcer-ng/src/hsmkey/update_hsmkeys_task.h | 8 + .../src/keystate/update_keyzones_task.h | 8 + .../src/keystate/write_signzone_task.cpp | 12 +- enforcer-ng/src/ods-enforcerd.c | 160 +++++-- enforcer-ng/src/policy/update_kasp_task.h | 8 + enforcer-ng/src/shared/allocator.c | 1 + enforcer-ng/src/shared/locks.h | 1 + 23 files changed, 492 insertions(+), 513 deletions(-) diff --git a/enforcer-ng/src/daemon/cfg.c b/enforcer-ng/src/daemon/cfg.c index 86ca77e2ec..3cce52647b 100644 --- a/enforcer-ng/src/daemon/cfg.c +++ b/enforcer-ng/src/daemon/cfg.c @@ -52,7 +52,7 @@ static const char* conf_str = "config"; */ engineconfig_type* engine_config(allocator_type* allocator, const char* cfgfile, - int cmdline_verbosity) + int cmdline_verbosity, engineconfig_type* oldcfg) { engineconfig_type* ecfg; const char* rngfile = ODS_SE_RNGDIR "/conf.rng"; @@ -89,8 +89,35 @@ engine_config(allocator_type* allocator, const char* cfgfile, /* open cfgfile */ cfgfd = ods_fopen(cfgfile, NULL, "r"); if (cfgfd) { + if (oldcfg) { + /* This is a reload */ + ecfg->cfg_filename = allocator_strdup(allocator, oldcfg->cfg_filename); + ecfg->clisock_filename = allocator_strdup(allocator, oldcfg->clisock_filename); + ecfg->working_dir = allocator_strdup(allocator, oldcfg->working_dir); + ecfg->username = allocator_strdup(allocator, oldcfg->username); + ecfg->group = allocator_strdup(allocator, oldcfg->group); + ecfg->chroot = allocator_strdup(allocator, oldcfg->chroot); + ecfg->pid_filename = allocator_strdup(allocator, oldcfg->pid_filename); + ecfg->datastore = allocator_strdup(allocator, oldcfg->datastore); + ecfg->db_host = allocator_strdup(allocator, oldcfg->db_host); + ecfg->db_username = allocator_strdup(allocator, oldcfg->db_username); + ecfg->db_password = allocator_strdup(allocator, oldcfg->db_password); + ecfg->db_port = oldcfg->db_port; + } else { + ecfg->cfg_filename = allocator_strdup(allocator, cfgfile); + ecfg->clisock_filename = parse_conf_clisock_filename(allocator, cfgfile); + ecfg->working_dir = parse_conf_working_dir(allocator, cfgfile); + ecfg->username = parse_conf_username(allocator, cfgfile); + ecfg->group = parse_conf_group(allocator, cfgfile); + ecfg->chroot = parse_conf_chroot(allocator, cfgfile); + ecfg->pid_filename = parse_conf_pid_filename(allocator, cfgfile); + ecfg->datastore = parse_conf_datastore(allocator, cfgfile); + ecfg->db_host = parse_conf_db_host(allocator, cfgfile); + ecfg->db_username = parse_conf_db_username(allocator, cfgfile); + ecfg->db_password = parse_conf_db_password(allocator, cfgfile); + ecfg->db_port = parse_conf_db_port(cfgfile); + } /* get values */ - ecfg->cfg_filename = allocator_strdup(allocator, cfgfile); ecfg->policy_filename = parse_conf_policy_filename(allocator, cfgfile); ecfg->zonelist_filename = parse_conf_zonelist_filename(allocator, @@ -98,35 +125,19 @@ engine_config(allocator_type* allocator, const char* cfgfile, ecfg->zonefetch_filename = parse_conf_zonefetch_filename(allocator, cfgfile); ecfg->log_filename = parse_conf_log_filename(allocator, cfgfile); - ecfg->pid_filename = parse_conf_pid_filename(allocator, cfgfile); ecfg->delegation_signer_submit_command = parse_conf_delegation_signer_submit_command(allocator, cfgfile); ecfg->delegation_signer_retract_command = parse_conf_delegation_signer_retract_command(allocator, cfgfile); - ecfg->clisock_filename = parse_conf_clisock_filename(allocator, - cfgfile); - ecfg->working_dir = parse_conf_working_dir(allocator, cfgfile); - ecfg->username = parse_conf_username(allocator, cfgfile); - ecfg->group = parse_conf_group(allocator, cfgfile); - ecfg->chroot = parse_conf_chroot(allocator, cfgfile); - ecfg->datastore = parse_conf_datastore(allocator, cfgfile); - ecfg->db_host = parse_conf_db_host(allocator,cfgfile); - ecfg->db_username = parse_conf_db_username(allocator,cfgfile); - ecfg->db_password = parse_conf_db_password(allocator,cfgfile); ecfg->use_syslog = parse_conf_use_syslog(cfgfile); ecfg->num_worker_threads = parse_conf_worker_threads(cfgfile); ecfg->manual_keygen = parse_conf_manual_keygen(cfgfile); ecfg->hsm = parse_conf_repositories(cfgfile); /* If any verbosity has been specified at cmd line we will use that */ - if (cmdline_verbosity > 0) { - ecfg->verbosity = cmdline_verbosity; - } - else { - ecfg->verbosity = parse_conf_verbosity(cfgfile); - } - ecfg->db_port = parse_conf_db_port(cfgfile); - ecfg->automatic_keygen_duration = - parse_conf_automatic_keygen_period(cfgfile); + ecfg->verbosity = cmdline_verbosity > 0 ? + cmdline_verbosity : parse_conf_verbosity(cfgfile); + ecfg->automatic_keygen_duration = + parse_conf_automatic_keygen_period(cfgfile); /* done */ ods_fclose(cfgfd); diff --git a/enforcer-ng/src/daemon/cfg.h b/enforcer-ng/src/daemon/cfg.h index 9765a8f5b8..31fb50c369 100644 --- a/enforcer-ng/src/daemon/cfg.h +++ b/enforcer-ng/src/daemon/cfg.h @@ -93,7 +93,7 @@ struct engineconfig_struct { * */ engineconfig_type* engine_config(allocator_type* allocator, - const char* cfgfile, int cmdline_verbosity); + const char* cfgfile, int cmdline_verbosity, engineconfig_type* oldcfg); /** * Check configuration. diff --git a/enforcer-ng/src/daemon/cmdhandler.c b/enforcer-ng/src/daemon/cmdhandler.c index 3da8f09642..d9e3949f26 100644 --- a/enforcer-ng/src/daemon/cmdhandler.c +++ b/enforcer-ng/src/daemon/cmdhandler.c @@ -420,6 +420,9 @@ int handled_help_cmd(int sockfd, engine_type* engine,const char *cmd, ssize_t n) " the earliest scheduled task.\n" #endif "flush Execute all scheduled tasks immediately.\n" + ); + ods_writen(sockfd, buf, strlen(buf)); + (void) snprintf(buf, ODS_SE_MAXLINE, "running Returns acknowledgment that the engine is running.\n" "reload Reload the engine.\n" "stop Stop the engine and terminate the process.\n" @@ -463,6 +466,9 @@ int handled_unknown_cmd(int sockfd, engine_type* engine, const char *cmd, ssize_ " the earliest scheduled task.\n" #endif "flush Execute all scheduled tasks immediately.\n" + ); + ods_writen(sockfd, buf, strlen(buf)); + (void) snprintf(buf, ODS_SE_MAXLINE, "running Returns acknowledgment that the engine is running.\n" "reload Reload the engine.\n" "stop Stop the engine and terminate the process.\n" @@ -743,7 +749,7 @@ cmdhandler_start(cmdhandler_type* cmdhandler) } ods_log_debug("[%s] done", module_str); - engine = cmdhandler->engine; + engine = cmdhandler->engine;/*todo check & remove me*/ engine->cmdhandler_done = 1; return; } diff --git a/enforcer-ng/src/daemon/engine.c b/enforcer-ng/src/daemon/engine.c index 937aead9d6..7822f3dddc 100644 --- a/enforcer-ng/src/daemon/engine.c +++ b/enforcer-ng/src/daemon/engine.c @@ -72,8 +72,8 @@ static const char* engine_str = "engine"; * Create engine. * */ -static engine_type* -engine_create(void) +engine_type* +engine_alloc(void) { engine_type* engine; allocator_type* allocator = allocator_create(malloc, free); @@ -86,36 +86,39 @@ engine_create(void) return NULL; } engine->allocator = allocator; - engine->config = NULL; - engine->workers = NULL; - engine->cmdhandler = NULL; - engine->cmdhandler_done = 0; - engine->pid = -1; - engine->uid = -1; - engine->gid = -1; - engine->daemonize = 0; - engine->need_to_exit = 0; - engine->need_to_reload = 0; - engine->setup_error = 0; - engine->signal = SIGNAL_INIT; lock_basic_init(&engine->signal_lock); + lock_basic_init(&engine->enforce_lock); lock_basic_set(&engine->signal_cond); engine->taskq = schedule_create(engine->allocator); if (!engine->taskq) { - engine_cleanup(engine); + allocator_deallocate(allocator, (void*) engine); return NULL; } engine->signq = fifoq_create(engine->allocator); if (!engine->signq) { - engine_cleanup(engine); + schedule_cleanup(engine->taskq); + allocator_deallocate(allocator, (void*) engine); + allocator_cleanup(allocator); return NULL; } return engine; } - +void +engine_dealloc(engine_type* engine) +{ + allocator_type* allocator = engine->allocator; + schedule_cleanup(engine->taskq); + fifoq_cleanup(engine->signq); + lock_basic_destroy(&engine->enforce_lock); + lock_basic_destroy(&engine->signal_lock); + lock_basic_off(&engine->signal_cond); + allocator_deallocate(allocator, (void*) engine); + allocator_cleanup(allocator); +} + /** * Start command handler. * @@ -128,6 +131,7 @@ cmdhandler_thread_start(void* arg) cmdhandler_start(cmd); return NULL; } + static void engine_start_cmdhandler(engine_type* engine) { @@ -332,16 +336,15 @@ engine_wakeup_workers(engine_type* engine) * Set up engine and return the setup status. * */ -static ods_status -engine_setup_and_return_status(engine_type* engine) +ods_status +engine_setup(engine_type* engine) { - struct sigaction action; int fd; ods_log_debug("[%s] enforcer setup", engine_str); - if (!engine || !engine->config) { - return ODS_STATUS_ASSERT_ERR; - } + + ods_log_init(engine->config->log_filename, + engine->config->use_syslog, engine->config->verbosity); /* create command handler (before chowning socket file) */ engine->cmdhandler = cmdhandler_create(engine->allocator, @@ -352,76 +355,68 @@ engine_setup_and_return_status(engine_type* engine) return ODS_STATUS_CMDHANDLER_ERR; } - /* privdrop */ - engine->uid = privuid(engine->config->username); - engine->gid = privgid(engine->config->group); - /* TODO: does piddir exists? */ - /* remove the chown stuff: piddir? */ - ods_chown(engine->config->pid_filename, engine->uid, engine->gid, 1); - ods_chown(engine->config->clisock_filename, engine->uid, engine->gid, 0); - ods_chown(engine->config->working_dir, engine->uid, engine->gid, 0); - if (engine->config->log_filename && !engine->config->use_syslog) { - ods_chown(engine->config->log_filename, engine->uid, engine->gid, 0); - } - if (engine->config->working_dir && - chdir(engine->config->working_dir) != 0) { - ods_log_error("[%s] chdir to %s failed: %s", engine_str, - engine->config->working_dir, strerror(errno)); - return ODS_STATUS_CHDIR_ERR; - } - if (engine_privdrop(engine) != ODS_STATUS_OK) { - ods_log_error("[%s] unable to drop privileges", engine_str); - return ODS_STATUS_PRIVDROP_ERR; - } + if (!engine->init_setup_done) { + /* privdrop */ + engine->uid = privuid(engine->config->username); + engine->gid = privgid(engine->config->group); + /* TODO: does piddir exists? */ + /* remove the chown stuff: piddir? */ + ods_chown(engine->config->pid_filename, engine->uid, engine->gid, 1); + ods_chown(engine->config->clisock_filename, engine->uid, engine->gid, 0); + ods_chown(engine->config->working_dir, engine->uid, engine->gid, 0); + if (engine->config->log_filename && !engine->config->use_syslog) { + ods_chown(engine->config->log_filename, engine->uid, engine->gid, 0); + } + if (engine->config->working_dir && + chdir(engine->config->working_dir) != 0) { + ods_log_error("[%s] chdir to %s failed: %s", engine_str, + engine->config->working_dir, strerror(errno)); + return ODS_STATUS_CHDIR_ERR; + } + if (engine_privdrop(engine) != ODS_STATUS_OK) { + ods_log_error("[%s] unable to drop privileges", engine_str); + return ODS_STATUS_PRIVDROP_ERR; + } - /* daemonize */ - if (engine->daemonize) { - switch (fork()) { - case -1: /* error */ - ods_log_error("[%s] unable to fork daemon: %s", + /* daemonize */ + if (engine->daemonize) { + switch (fork()) { + case -1: /* error */ + ods_log_error("[%s] unable to fork daemon: %s", + engine_str, strerror(errno)); + hsm_close(); + return ODS_STATUS_FORK_ERR; + case 0: /* child */ + if ((fd = open("/dev/null", O_RDWR, 0)) != -1) { + (void)dup2(fd, STDIN_FILENO); + (void)dup2(fd, STDOUT_FILENO); + (void)dup2(fd, STDERR_FILENO); + if (fd > 2) (void)close(fd); + } + engine->daemonize = 0; /* don't fork again on reload */ + break; + default: /* parent */ + exit(0); + } + if (setsid() == -1) { + ods_log_error("[%s] unable to setsid daemon (%s)", engine_str, strerror(errno)); hsm_close(); - return ODS_STATUS_FORK_ERR; - case 0: /* child */ - if ((fd = open("/dev/null", O_RDWR, 0)) != -1) { - (void)dup2(fd, STDIN_FILENO); - (void)dup2(fd, STDOUT_FILENO); - (void)dup2(fd, STDERR_FILENO); - if (fd > 2) (void)close(fd); - } - break; - default: /* parent */ - engine_cleanup(engine); - engine = NULL; - xmlCleanupParser(); - xmlCleanupGlobals(); - xmlCleanupThreads(); - exit(0); - } - if (setsid() == -1) { - ods_log_error("[%s] unable to setsid daemon (%s)", - engine_str, strerror(errno)); - hsm_close(); - return ODS_STATUS_SETSID_ERR; + return ODS_STATUS_SETSID_ERR; + } } } + engine->init_setup_done = 1; + engine->pid = getpid(); ods_log_verbose("[%s] running as pid %lu", engine_str, (unsigned long) engine->pid); - /* catch signals */ - signal_set_engine(engine); - action.sa_handler = signal_handler; - sigfillset(&action.sa_mask); - action.sa_flags = 0; - sigaction(SIGHUP, &action, NULL); - sigaction(SIGTERM, &action, NULL); - /* create workers */ engine_create_workers(engine); /* start command handler */ - engine_start_cmdhandler(engine); + engine->cmdhandler_done = 0; /* write pidfile */ if (util_write_pidfile(engine->config->pid_filename, engine->pid) == -1) { @@ -434,83 +429,102 @@ engine_setup_and_return_status(engine_type* engine) } /** - * Set up engine. + * Clean up engine. * */ void -engine_setup(engine_type* engine, handled_xxxx_cmd_type *commands, - help_xxxx_cmd_type *help) +engine_teardown(engine_type* engine) { - ods_status status; + size_t i = 0; - engine->commands = commands; - engine->help = help; - status = engine_setup_and_return_status(engine); - if (status != ODS_STATUS_OK) { - ods_log_error("[%s] setup failed: %s", engine_str, - ods_status2str(status)); - engine->setup_error = 1; - if (status != ODS_STATUS_WRITE_PIDFILE_ERR) { - /* command handler had not yet been started */ - engine->cmdhandler_done = 1; + if (!engine) return; + if (engine->config) { + if (engine->config->pid_filename) { + (void)unlink(engine->config->pid_filename); + } + if (engine->config->clisock_filename) { + (void)unlink(engine->config->clisock_filename); } } + if (engine->workers && engine->config) { + for (i=0; i < (size_t) engine->config->num_worker_threads; i++) { + worker_cleanup(engine->workers[i]); + } + allocator_deallocate(engine->allocator, (void*) engine->workers); + } + cmdhandler_cleanup(engine->cmdhandler); +} + +void +engine_init(engine_type* engine, int daemonize, + handled_xxxx_cmd_type *commands, help_xxxx_cmd_type *help) +{ + struct sigaction action; + + engine->config = NULL; + engine->workers = NULL; + engine->cmdhandler = NULL; + engine->cmdhandler_done = 1; + engine->init_setup_done = 0; + engine->database_ready = 0; + engine->pid = -1; + engine->uid = -1; + engine->gid = -1; + engine->need_to_exit = 0; + engine->need_to_reload = 0; + engine->daemonize = daemonize; + engine->commands = commands; + engine->help = help; + /* catch signals */ + signal_set_engine(engine); + action.sa_handler = signal_handler; + sigfillset(&action.sa_mask); + action.sa_flags = 0; + sigaction(SIGHUP, &action, NULL); + sigaction(SIGTERM, &action, NULL); + sigaction(SIGINT, &action, NULL); } /** * Run engine, run!. * */ -static void +int engine_run(engine_type* engine, start_cb_t start, int single_run) { - if (!engine) { - return; - } + int error; ods_log_assert(engine); - + ods_log_info("[%s] enforcer started", engine_str); + + error = hsm_open(engine->config->cfg_filename, hsm_prompt_pin); + if (error != HSM_OK) { + char* errorstr = hsm_get_error(NULL); + if (errorstr != NULL) { + ods_log_error("[%s] %s", engine_str, errorstr); + free(errorstr); + } else { + ods_log_crit("[%s] error opening libhsm (errno %i)", engine_str, + error); + } + return 1; + } + + engine->need_to_reload = 0; + engine_start_cmdhandler(engine); engine_start_workers(engine); - lock_basic_lock(&engine->signal_lock); - /* [LOCK] signal */ - engine->signal = SIGNAL_RUN; - /* [UNLOCK] signal */ - lock_basic_unlock(&engine->signal_lock); - /* call the external start callback function */ start(engine); while (!engine->need_to_exit && !engine->need_to_reload) { - lock_basic_lock(&engine->signal_lock); - /* [LOCK] signal */ - engine->signal = signal_capture(engine->signal); - switch (engine->signal) { - case SIGNAL_RUN: - ods_log_assert(1); - break; - case SIGNAL_RELOAD: - engine->need_to_reload = 1; - break; - case SIGNAL_SHUTDOWN: - engine->need_to_exit = 1; - break; - default: - ods_log_warning("[%s] invalid signal captured: %d, " - "keep running", engine_str, signal); - engine->signal = SIGNAL_RUN; - break; - } - /* [UNLOCK] signal */ - lock_basic_unlock(&engine->signal_lock); - if (single_run) { engine->need_to_exit = 1; /* FIXME: all tasks need to terminate, then set need_to_exit to 1 */ } lock_basic_lock(&engine->signal_lock); - /* [LOCK] signal */ - if (engine->signal == SIGNAL_RUN && !single_run) { + /* [LOCK] signal, recheck reload and lock */ + if (!engine->need_to_exit && !engine->need_to_reload && !single_run) { ods_log_debug("[%s] taking a break", engine_str); lock_basic_sleep(&engine->signal_cond, &engine->signal_lock, 0); } @@ -519,180 +533,9 @@ engine_run(engine_type* engine, start_cb_t start, int single_run) } ods_log_debug("[%s] enforcer halted", engine_str); engine_stop_workers(engine); - return; -} - - -/** - * Engine runloop - * - */ - -void -engine_runloop(engine_type* engine, start_cb_t start, int single_run) -{ - /* run */ - while (engine->need_to_exit == 0) { - /* set up hsm */ - /* TODO: On a reload the hsm configuration could have been - * changed. Currently if we are unable to reopen the hsms we - * have no choice but bail out. We must change this and try - * to reopen the old repository list. hint: use hsm_open2 */ - int result = hsm_open(engine->config->cfg_filename, hsm_prompt_pin); - if (result != HSM_OK) { - char* error = hsm_get_error(NULL); - if (error != NULL) { - ods_log_error("[%s] %s", engine_str, error); - free(error); - } else { - ods_log_crit("[%s] error opening libhsm (errno %i)", engine_str, - result); - } - engine->setup_error = 1; - break; - } - - if (engine->need_to_reload) { - ods_log_info("[%s] enforcer reloading", engine_str); - engine->need_to_reload = 0; - } else { - ods_log_info("[%s] enforcer started", engine_str); - /* try to recover from backups */ - /* not for now: - engine_recover_from_backups(engine); - */ - } - - engine_run(engine, start, single_run); - hsm_close(); - } - - /* shutdown */ - ods_log_info("[%s] enforcer shutdown", engine_str); - if (engine->cmdhandler != NULL) { - engine_stop_cmdhandler(engine); - } -} - - -/** - * Start engine. - * - */ -engine_type * -engine_start(const char* cfgfile, int cmdline_verbosity, int daemonize, - int info) -{ - engine_type* engine = NULL; - int use_syslog = 0; - ods_status status = ODS_STATUS_OK; - - ods_log_assert(cfgfile); - ods_log_init(NULL, use_syslog, cmdline_verbosity); - ods_log_verbose("[%s] starting enforcer", engine_str); - - /* initialize */ - xmlInitGlobals(); - xmlInitParser(); - xmlInitThreads(); - engine = engine_create(); - if (!engine) { - ods_fatal_exit("[%s] create failed", engine_str); - return NULL; - } - engine->daemonize = daemonize; - - /* config */ - engine->config = engine_config(engine->allocator, cfgfile, - cmdline_verbosity); - status = engine_config_check(engine->config); - if (status != ODS_STATUS_OK) { - ods_log_error("[%s] cfgfile %s has errors", engine_str, cfgfile); - engine_stop(engine); - return NULL; - } - if (info) { - engine_config_print(stdout, engine->config); /* for debugging */ - engine_stop(engine); - return NULL; - } - - /* open log */ - ods_log_init(engine->config->log_filename, engine->config->use_syslog, - engine->config->verbosity); - - /* setup */ - tzset(); /* for portability */ - - /* initialize protobuf and protobuf-orm */ - ods_protobuf_initialize(); - ods_orm_initialize(); - - return engine; -} - - -/** - * Stop engine. - * - */ -void -engine_stop(engine_type *engine) -{ - ods_orm_shutdown(); - ods_protobuf_shutdown(); - - if (engine && engine->config) { - if (engine->config->pid_filename) { - (void)unlink(engine->config->pid_filename); - } - if (engine->config->clisock_filename) { - (void)unlink(engine->config->clisock_filename); - } - } - engine_cleanup(engine); - engine = NULL; - ods_log_close(); - xmlCleanupParser(); - xmlCleanupGlobals(); - xmlCleanupThreads(); -} - -/** - * Clean up engine. - * - */ -void -engine_cleanup(engine_type* engine) -{ - size_t i = 0; - allocator_type* allocator; - cond_basic_type signal_cond; - lock_basic_type signal_lock; - - if (!engine) { - return; - } - allocator = engine->allocator; - signal_cond = engine->signal_cond; - signal_lock = engine->signal_lock; - - if (engine->workers && engine->config) { - for (i=0; i < (size_t) engine->config->num_worker_threads; i++) { - worker_cleanup(engine->workers[i]); - } - allocator_deallocate(allocator, (void*) engine->workers); - } - schedule_cleanup(engine->taskq); - fifoq_cleanup(engine->signq); - cmdhandler_cleanup(engine->cmdhandler); - engine_config_cleanup(engine->config); - allocator_deallocate(allocator, (void*) engine); - - lock_basic_destroy(&signal_lock); - lock_basic_off(&signal_cond); - allocator_cleanup(allocator); - return; + engine_stop_cmdhandler(engine); + (void) hsm_close(); + return 0; } void diff --git a/enforcer-ng/src/daemon/engine.h b/enforcer-ng/src/daemon/engine.h index dfd088c38d..2733337579 100644 --- a/enforcer-ng/src/daemon/engine.h +++ b/enforcer-ng/src/daemon/engine.h @@ -72,6 +72,8 @@ struct engine_struct { handled_xxxx_cmd_type *commands; cmdhandler_type* cmdhandler; int cmdhandler_done; + int init_setup_done; + int database_ready; pid_t pid; uid_t uid; @@ -80,11 +82,10 @@ struct engine_struct { int daemonize; int need_to_exit; int need_to_reload; - int setup_error; - sig_atomic_t signal; cond_basic_type signal_cond; lock_basic_type signal_lock; + lock_basic_type enforce_lock; }; /** @@ -97,12 +98,9 @@ struct engine_struct { * \return engine_type* engine to use or NULL when engine couldn't start * */ -engine_type *engine_start(const char* cfgfile, int cmdline_verbosity, - int daemonize, int info); - /** - * Setup the engine started by engine_start + * Setup the engine started by engine_create * \param[in] engine the engine returned from engine_start * \param[in] commands NULL terminated list of command functions for * the engine that the command handler can run. @@ -110,8 +108,17 @@ engine_type *engine_start(const char* cfgfile, int cmdline_verbosity, * for the command to a socket. */ -void engine_setup(engine_type *engine, handled_xxxx_cmd_type *commands, - help_xxxx_cmd_type *help); +ods_status engine_setup(engine_type* engine); +/** + * Clean up engine. + * \param[in] engine engine + * + */ +void engine_teardown(engine_type* engine); + +void +engine_init(engine_type* engine, int daemonize, + handled_xxxx_cmd_type *commands, help_xxxx_cmd_type *help); typedef void (*start_cb_t)(engine_type* engine); @@ -121,9 +128,10 @@ typedef void (*start_cb_t)(engine_type* engine); * the engine is ready to stop. * \param[in] engine the engine returned from engine_start * \param[in] single_run run once + * \return 0 if terminated normally, 1 on unrecoverable error. * */ -void engine_runloop(engine_type* engine, start_cb_t start, int single_run); +int engine_run(engine_type* engine, start_cb_t start, int single_run); /** * Stop the engine after engine_runloop returns. @@ -145,12 +153,8 @@ void engine_stop_workers(engine_type* engine); * \param[in] engine engine */ void engine_start_workers(engine_type* engine); -/** - * Clean up engine. - * \param[in] engine engine - * - */ -void engine_cleanup(engine_type* engine); +engine_type* engine_alloc(void); +void engine_dealloc(engine_type* engine); /** * Set all task to immediate execution and wake up all workers. diff --git a/enforcer-ng/src/daemon/signal.c b/enforcer-ng/src/daemon/signal.c index 9a7df9694d..c780cd757f 100644 --- a/enforcer-ng/src/daemon/signal.c +++ b/enforcer-ng/src/daemon/signal.c @@ -39,8 +39,6 @@ #include -static int signal_hup_recvd = 0; -static int signal_term_recvd = 0; static engine_type* signal_engine = NULL; static const char* signal_str = "signal"; @@ -66,8 +64,8 @@ signal_handler(sig_atomic_t sig) switch (sig) { case SIGHUP: ods_log_debug("[%s] SIGHUP received", signal_str); - signal_hup_recvd++; if (signal_engine) { + signal_engine->need_to_reload = 1; lock_basic_lock(&signal_engine->signal_lock); /* [LOCK] signal */ lock_basic_alarm(&signal_engine->signal_cond); @@ -75,10 +73,11 @@ signal_handler(sig_atomic_t sig) lock_basic_unlock(&signal_engine->signal_lock); } break; + case SIGINT: case SIGTERM: ods_log_debug("[%s] SIGTERM received", signal_str); - signal_term_recvd++; if (signal_engine) { + signal_engine->need_to_exit = 1; lock_basic_lock(&signal_engine->signal_lock); /* [LOCK] signal */ lock_basic_alarm(&signal_engine->signal_cond); @@ -87,25 +86,9 @@ signal_handler(sig_atomic_t sig) } break; default: + ods_log_debug("[%s] Spurious signal %d received", + signal_str, sig); break; } return; } - - -/** - * Capture signal. - * - */ -sig_atomic_t -signal_capture(sig_atomic_t dflsig) -{ - if (signal_term_recvd) { - signal_term_recvd = 0; - return SIGNAL_SHUTDOWN; - } else if (signal_hup_recvd) { - signal_hup_recvd = 0; - return SIGNAL_RELOAD; - } - return dflsig; -} diff --git a/enforcer-ng/src/daemon/signal.h b/enforcer-ng/src/daemon/signal.h index 4288062b52..8b921044b9 100644 --- a/enforcer-ng/src/daemon/signal.h +++ b/enforcer-ng/src/daemon/signal.h @@ -42,11 +42,6 @@ extern "C" { #endif -#define SIGNAL_RUN 0 -#define SIGNAL_INIT 1 -#define SIGNAL_RELOAD 2 -#define SIGNAL_SHUTDOWN 3 - struct engine_struct; /** diff --git a/enforcer-ng/src/enforcer/autostart_cmd.cpp b/enforcer-ng/src/enforcer/autostart_cmd.cpp index ab71de1cad..5c57d22d85 100644 --- a/enforcer-ng/src/enforcer/autostart_cmd.cpp +++ b/enforcer-ng/src/enforcer/autostart_cmd.cpp @@ -42,9 +42,8 @@ #include "shared/file.h" #include "shared/str.h" #include "daemon/engine.h" - -#include "protobuf-orm/pb-orm.h" #include "daemon/orm.h" +#include "protobuf-orm/pb-orm.h" #include "policy/kasp.pb.h" static const char *module_str = "autostart_cmd"; @@ -56,6 +55,8 @@ schedule_task(engine_type* engine, task_type *task, const char * what) if (!task) { ods_log_crit("[%s] failed to create %s task", module_str, what); } else { + task->when += 2; /* quick fix race condition at startup + Allow orm/database to come up fully and prevent backoff */ char buf[ODS_SE_MAXLINE]; ods_status status = lock_and_schedule_task(engine->taskq, task, 0); if (status != ODS_STATUS_OK) { @@ -66,31 +67,32 @@ schedule_task(engine_type* engine, task_type *task, const char * what) } } } - -void -autostart(engine_type* engine) -{ - ods_log_debug("[%s] autostart", module_str); +int +database_ready(engineconfig_type* config) +{ /* Try to select from policies. This is only used to probe if the * database is already setup. If not, we don't schedule tasks which - * would otherwise pollute the logs repeatedly. + * would otherwise pollute the logs repeatedly. * TODO: I'd like to see a better probe which does not log an error */ OrmConnRef conn; OrmResultRef rows; ::ods::kasp::Policy policy; - if (!ods_orm_connect(-1, engine->config, conn)) { - ods_log_crit("Could not connect to database."); - return; - } - if (!OrmMessageEnum(conn, policy.descriptor(), rows)) { - ods_log_info("[%s] Database is not set up yet." - " Not scheduling tasks.", module_str); - ods_log_info("[%s] Run the 'ods-enforcer setup'" - " command to create the database.", module_str); - return; + if (!ods_orm_connect(-1, config, conn) || + !OrmMessageEnum(conn, policy.descriptor(), rows)) + { + return 0; } rows.release(); + return 1; +} + +void +autostart(engine_type* engine) +{ + ods_log_debug("[%s] autostart", module_str); + + if (!engine->database_ready) return; schedule_task(engine, policy_resalt_task(engine->config), "resalt"); schedule_task(engine, enforce_task(engine, 1), "enforce"); diff --git a/enforcer-ng/src/enforcer/autostart_cmd.h b/enforcer-ng/src/enforcer/autostart_cmd.h index c63db27ac3..92f028e713 100644 --- a/enforcer-ng/src/enforcer/autostart_cmd.h +++ b/enforcer-ng/src/enforcer/autostart_cmd.h @@ -40,6 +40,8 @@ extern "C" { void autostart(engine_type* engine); +int database_ready(engineconfig_type* config); + #ifdef __cplusplus } #endif diff --git a/enforcer-ng/src/enforcer/enforce_cmd.cpp b/enforcer-ng/src/enforcer/enforce_cmd.cpp index c0fbd6490b..c09203b86f 100644 --- a/enforcer-ng/src/enforcer/enforce_cmd.cpp +++ b/enforcer-ng/src/enforcer/enforce_cmd.cpp @@ -61,21 +61,18 @@ void help_enforce_zones_cmd(int sockfd) * Handle the 'enforce' command. * */ -int handled_enforce_zones_cmd(int sockfd, engine_type* engine, const char *cmd, - ssize_t n) +int +handled_enforce_zones_cmd(int sockfd, engine_type* engine, + const char *cmd, ssize_t n) { - const char *scmd = "enforce"; - - cmd = ods_check_command(cmd,n,scmd); - if (!cmd) - return 0; // not handled - + const char *scmd = "enforce"; + cmd = ods_check_command(cmd, n, scmd); + if (!cmd) return 0; // not handled ods_log_debug("[%s] %s command", module_str, scmd); - time_t tstart = time(NULL); - perform_enforce(sockfd, engine, 1, NULL); - + perform_enforce_lock(sockfd, engine, 1, NULL); + ods_printf(sockfd,"%s completed in %ld seconds.\n",scmd,time(NULL)-tstart); - return 1; + return 1; } diff --git a/enforcer-ng/src/enforcer/enforce_task.cpp b/enforcer-ng/src/enforcer/enforce_task.cpp index 9c99044a1b..ee584e64cf 100644 --- a/enforcer-ng/src/enforcer/enforce_task.cpp +++ b/enforcer-ng/src/enforcer/enforce_task.cpp @@ -164,8 +164,9 @@ reschedule_enforce(task_type *task, time_t t_when, const char *z_when) return task->when; } -time_t perform_enforce(int sockfd, engine_type *engine, int bForceUpdate, - task_type* task) +static time_t +perform_enforce(int sockfd, engine_type *engine, int bForceUpdate, + task_type* task) { #define LOG_AND_RESCHEDULE(errmsg)\ do {\ @@ -233,6 +234,7 @@ time_t perform_enforce(int sockfd, engine_type *engine, int bForceUpdate, zones_need_updating = true; } while (next) { + if (engine->need_to_reload || engine->need_to_exit) break; OrmTransactionRW transaction(conn); if (!transaction.started()) LOG_AND_RESCHEDULE_15SECS("transaction not started"); @@ -366,10 +368,25 @@ time_t perform_enforce(int sockfd, engine_type *engine, int bForceUpdate, return reschedule_enforce(task,t_when,z_when.c_str()); } +time_t perform_enforce_lock(int sockfd, engine_type *engine, + int bForceUpdate, task_type* task) +{ + time_t returntime; + int locked; + if (lock_basic_trylock(&engine->enforce_lock)) { + ods_printf(sockfd, "An other enforce task is already running." + " No action taken.\n"); + return 0; + } + returntime = perform_enforce(sockfd, engine, bForceUpdate, task); + lock_basic_unlock(&engine->enforce_lock); + return returntime; +} + static task_type * enforce_task_perform(task_type *task) { - int return_time = perform_enforce(-1, (engine_type *)task->context, + int return_time = perform_enforce_lock(-1, (engine_type *)task->context, enforce_all, task); enforce_all = 0; /* global */ if (return_time != -1) return task; diff --git a/enforcer-ng/src/enforcer/enforce_task.h b/enforcer-ng/src/enforcer/enforce_task.h index 76febb24b2..d88ff37da8 100644 --- a/enforcer-ng/src/enforcer/enforce_task.h +++ b/enforcer-ng/src/enforcer/enforce_task.h @@ -35,7 +35,7 @@ #include "daemon/cfg.h" #include "scheduler/task.h" -time_t perform_enforce(int sockfd, engine_type *engine, int bForce, +time_t perform_enforce_lock(int sockfd, engine_type *engine, int bForce, task_type *task); task_type *enforce_task(engine_type *engine, bool all); diff --git a/enforcer-ng/src/enforcer/setup_cmd.cpp b/enforcer-ng/src/enforcer/setup_cmd.cpp index 917814e204..59d0676bca 100644 --- a/enforcer-ng/src/enforcer/setup_cmd.cpp +++ b/enforcer-ng/src/enforcer/setup_cmd.cpp @@ -35,6 +35,7 @@ #include "enforcer/setup_cmd.h" #include "enforcer/autostart_cmd.h" +#include "enforcer/update_repositorylist_task.h" #include "shared/duration.h" #include "shared/file.h" @@ -133,52 +134,51 @@ drop_database_tables(int sockfd, OrmConn conn, engineconfig_type* config) int handled_setup_cmd(int sockfd, engine_type* engine, const char *cmd, ssize_t n) { - const char *scmd = "setup"; - cmd = ods_check_command(cmd,n,scmd); - if (!cmd) - return 0; // not handled + const char *scmd = "setup"; + if (!ods_check_command(cmd, n, scmd)) return 0; // not handled + ods_log_debug("[%s] %s command", module_str, scmd); + time_t tstart = time(NULL); - ods_log_debug("[%s] %s command", module_str, scmd); + lock_basic_lock(&engine->signal_lock); + /** we have got the lock, daemon thread is not going anywhere + * we can safely stop all workers */ + engine->need_to_reload = 1; + engine_stop_workers(engine); - // check that we are using a compatible protobuf version. - GOOGLE_PROTOBUF_VERIFY_VERSION; - - time_t tstart = time(NULL); - - { // Drop the database tables using a dedicated database connection. OrmConnRef conn; - if (!ods_orm_connect(sockfd, engine->config, conn)) - return 1; // errors have already been reported. - - if (!drop_database_tables(sockfd,conn,engine->config)) + if (!ods_orm_connect(sockfd, engine->config, conn) + || !drop_database_tables(sockfd,conn,engine->config) + || !ods_orm_connect(sockfd, engine->config, conn) + || !create_database_tables(sockfd, conn)) + { + engine->need_to_reload = 0; + engine_start_workers(engine); + lock_basic_unlock(&engine->signal_lock); + lock_basic_alarm(&engine->signal_cond); return 1; // errors have already been reported. - } + } - { - // Create the database tables using a dedicated database connection. - OrmConnRef conn; - if (!ods_orm_connect(sockfd, engine->config, conn)) - return 1; // errors have already been reported. - - if (!create_database_tables(sockfd, conn)) - return 1; // errors have already been reported. - } + /* we might have skipped this when starting w/o a db */ + autostart(engine); + + /* TODO: Add this function once implemented + * perform_update_conf(engine->config); */ + int error = !perform_update_kasp(sockfd, engine->config); + if (!error) + error |= !perform_update_keyzones(sockfd, engine->config); + if (!error) { + perform_update_hsmkeys(sockfd, engine->config, 0 /* automatic */); + perform_hsmkey_gen(sockfd, engine->config, 0 /* automatic */, + engine->config->automatic_keygen_duration); + } + + engine->need_to_reload = 0; + engine_start_workers(engine); + flush_all_tasks(sockfd, engine); + lock_basic_unlock(&engine->signal_lock); + lock_basic_alarm(&engine->signal_cond); - autostart(engine); - // TODO: Add this function once implemented - //perform_update_conf(engine->config); - /* This needs to be revised, DONT continue when kasp or keyzones - * fail. Maybe also call update repositorylist (and restart) - * Also, what does "autostart(engine);" do here? figure it out! */ - perform_update_kasp(sockfd, engine->config); - perform_update_keyzones(sockfd, engine->config); - perform_update_hsmkeys(sockfd, engine->config, 0 /* automatic */); - perform_hsmkey_gen(sockfd, engine->config, 0 /* automatic */, - engine->config->automatic_keygen_duration); - - flush_all_tasks(sockfd, engine); - - ods_printf(sockfd, "%s completed in %ld seconds.\n",scmd,time(NULL)-tstart); - return 1; + ods_printf(sockfd, "%s completed in %ld seconds.\n",scmd,time(NULL)-tstart); + return 1; } diff --git a/enforcer-ng/src/enforcer/update_all_cmd.cpp b/enforcer-ng/src/enforcer/update_all_cmd.cpp index bc99196483..5b388fd360 100644 --- a/enforcer-ng/src/enforcer/update_all_cmd.cpp +++ b/enforcer-ng/src/enforcer/update_all_cmd.cpp @@ -67,20 +67,9 @@ void help_update_all_cmd(int sockfd) ); } -int -handled_update_all_cmd(int sockfd, engine_type* engine, const char *cmd, - ssize_t n) +static int +check_all_task(int sockfd, engine_type* engine) { - const char *scmd = "update all"; - cmd = ods_check_command(cmd,n,scmd); - if (!cmd) return 0; // not handled - ods_log_debug("[%s] %s command", module_str, scmd); - - // check that we are using a compatible protobuf version. - GOOGLE_PROTOBUF_VERIFY_VERSION; - time_t tstart = time(NULL); - - autostart(engine); /* Check all files for errors. The perform_update_*() * functions check as well but this gives us all or nothing. @@ -104,24 +93,28 @@ handled_update_all_cmd(int sockfd, engine_type* engine, const char *cmd, ods_log_error_and_printf(sockfd, module_str, "Unable to validate '%s' consistency.", zonelist); else error = 0; - + free(kasp); free(zonelist); if (replist) { for (i = 0; i < repcount; i++) free(replist[i]); } + return error; +} + +int +handled_update_all_cmd(int sockfd, engine_type* engine, const char *cmd, + ssize_t n) +{ + const char *scmd = "update all"; + cmd = ods_check_command(cmd,n,scmd); + if (!cmd) return 0; // not handled + ods_log_debug("[%s] %s command", module_str, scmd); + time_t tstart = time(NULL); - if (!error) - error |= perform_update_repositorylist(sockfd, engine); - if (!error) - error |= perform_update_kasp(sockfd, engine->config); - if (!error) - error |= perform_update_keyzones(sockfd, engine->config); - if (!error) { - perform_update_hsmkeys(sockfd, engine->config, 0 /* automatic */); - perform_hsmkey_gen(sockfd, engine->config, 0 /* automatic */, - engine->config->automatic_keygen_duration); - flush_all_tasks(sockfd, engine); + if (!check_all_task(sockfd, engine)) { + engine->need_to_reload = 1; + lock_basic_alarm(&engine->signal_cond); } ods_printf(sockfd, "%s completed in %ld seconds.\n",scmd,time(NULL)-tstart); return 1; diff --git a/enforcer-ng/src/enforcer/update_repositorylist_task.cpp b/enforcer-ng/src/enforcer/update_repositorylist_task.cpp index 867106c07e..7179320c27 100644 --- a/enforcer-ng/src/enforcer/update_repositorylist_task.cpp +++ b/enforcer-ng/src/enforcer/update_repositorylist_task.cpp @@ -28,7 +28,6 @@ * IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. * */ -#include "update_repositorylist_task.h" #include "config.h" #include "daemon/cfg.h" @@ -38,8 +37,12 @@ #include "shared/log.h" #include "shared/status.h" #include "utils/kc_helper.h" -#include +#include "daemon/engine.h" + +#include "update_repositorylist_task.h" + +#include #include #include #include diff --git a/enforcer-ng/src/enforcer/update_repositorylist_task.h b/enforcer-ng/src/enforcer/update_repositorylist_task.h index e6e92d4890..7df48e50e6 100644 --- a/enforcer-ng/src/enforcer/update_repositorylist_task.h +++ b/enforcer-ng/src/enforcer/update_repositorylist_task.h @@ -32,8 +32,9 @@ #ifndef UPDATE_REPOSITORYLIST_TASK_H_ #define UPDATE_REPOSITORYLIST_TASK_H_ -#include "config.h" -#include "daemon/engine.h" +#ifdef __cplusplus +extern "C" { +#endif /** NOTICE: This task MUST NOT end up in the queue because I stops * all workers and wait for them to be gone. It would cause a deadlock. @@ -44,4 +45,8 @@ * @return 1 on success, 0 on failure.*/ int perform_update_repositorylist(int sockfd, engine_type* engine); +#ifdef __cplusplus +} +#endif + #endif /* UPDATE_REPOSITORYLIST_TASK_H_ */ diff --git a/enforcer-ng/src/hsmkey/update_hsmkeys_task.h b/enforcer-ng/src/hsmkey/update_hsmkeys_task.h index 7c700a3283..7b0014d0e8 100644 --- a/enforcer-ng/src/hsmkey/update_hsmkeys_task.h +++ b/enforcer-ng/src/hsmkey/update_hsmkeys_task.h @@ -35,6 +35,14 @@ #include "daemon/cfg.h" #include "scheduler/task.h" +#ifdef __cplusplus +extern "C" { +#endif + void perform_update_hsmkeys(int sockfd, engineconfig_type *config, int bManual); +#ifdef __cplusplus +} +#endif + #endif diff --git a/enforcer-ng/src/keystate/update_keyzones_task.h b/enforcer-ng/src/keystate/update_keyzones_task.h index d925ce6c12..28043b32f5 100644 --- a/enforcer-ng/src/keystate/update_keyzones_task.h +++ b/enforcer-ng/src/keystate/update_keyzones_task.h @@ -34,10 +34,18 @@ #include "daemon/cfg.h" +#ifdef __cplusplus +extern "C" { +#endif + /* Read zonelist, add or update zones in database. Drop old zones. * sockfd: Output channel for client * config: * return: 1 on succes, 0 on error*/ int perform_update_keyzones(int sockfd, engineconfig_type *config); +#ifdef __cplusplus +} +#endif + #endif diff --git a/enforcer-ng/src/keystate/write_signzone_task.cpp b/enforcer-ng/src/keystate/write_signzone_task.cpp index 2e1d7559b2..affc055461 100644 --- a/enforcer-ng/src/keystate/write_signzone_task.cpp +++ b/enforcer-ng/src/keystate/write_signzone_task.cpp @@ -48,16 +48,14 @@ static const char *module_str = "write_signzone_task"; int perform_write_signzone_file(int sockfd, engineconfig_type *config) { - //write signzone file std::string signzone_file(config->working_dir); signzone_file.append("/"); signzone_file.append(OPENDNSSEC_ENFORCER_ZONELIST); - if (!perform_zonelist_export_to_file(signzone_file,config)) { - ods_log_error_and_printf(sockfd, module_str, - "failed to write %s", signzone_file.c_str()); - } - + if (!perform_zonelist_export_to_file(signzone_file,config)) { + ods_log_error_and_printf(sockfd, module_str, + "failed to write %s", signzone_file.c_str()); + } return 1; -} \ No newline at end of file +} diff --git a/enforcer-ng/src/ods-enforcerd.c b/enforcer-ng/src/ods-enforcerd.c index 9bcdea4511..4224d33177 100644 --- a/enforcer-ng/src/ods-enforcerd.c +++ b/enforcer-ng/src/ods-enforcerd.c @@ -33,7 +33,8 @@ #include "config.h" #include "daemon/engine.h" - +#include "shared/protobuf.h" +#include "daemon/orm.h" /* Pull in the commands that have been implemented for the enforcer */ #include "enforcer/autostart_cmd.h" @@ -70,14 +71,23 @@ #include "hsmkey/hsmkey_gen_cmd.h" #include "hsmkey/backup_hsmkeys_cmd.h" +#include "enforcer/update_repositorylist_task.h" +#include "keystate/update_keyzones_task.h" +#include "hsmkey/update_hsmkeys_task.h" +#include "policy/update_kasp_task.h" +#include "keystate/update_keyzones_task.h" + /* System libraries last */ #include #include #include +#include + #define AUTHOR_NAME "Matthijs Mekking, Yuri Schaeffer, René Post" #define COPYRIGHT_STR "Copyright (C) 2010-2011 NLnet Labs OpenDNSSEC" +static const char* enforcerd_str = "engine"; /** * Prints usage. @@ -216,6 +226,38 @@ enforcer_commands[] = { NULL }; +void +program_setup(int cmdline_verbosity) +{ + /* for now just log to stderr */ + ods_log_init(NULL, 0, cmdline_verbosity); + ods_log_verbose("[%s] starting enforcer", enforcerd_str); + + /* initialize */ + xmlInitGlobals(); + xmlInitParser(); + xmlInitThreads(); + + /* setup */ + tzset(); /* for portability */ + + /* initialize protobuf and protobuf-orm */ + ods_protobuf_initialize(); + ods_orm_initialize(); +} + +void +program_teardown() +{ + ods_orm_shutdown(); + ods_protobuf_shutdown(); + + ods_log_close(); + + xmlCleanupParser(); + xmlCleanupGlobals(); + xmlCleanupThreads(); +} /** * Main. start engine and run it. @@ -224,6 +266,10 @@ enforcer_commands[] = { int main(int argc, char* argv[]) { + ods_status status; + engine_type *engine; + engineconfig_type* cfg; + int returncode; int c; int options_index = 0; int info = 0; @@ -233,9 +279,6 @@ main(int argc, char* argv[]) const char* cfgfile = ODS_SE_CFGFILE; static struct option long_options[] = { {"single-run", no_argument, 0, '1'}, -#if HAVE_READ_CONFIG_FROM_EXTERNAL_FILE - {"config", required_argument, 0, 'c'}, -#endif {"no-daemon", no_argument, 0, 'd'}, {"help", no_argument, 0, 'h'}, {"info", no_argument, 0, 'i'}, @@ -245,13 +288,7 @@ main(int argc, char* argv[]) }; /* parse the commandline */ - while ((c=getopt_long(argc, argv, -#if HAVE_READ_CONFIG_FROM_EXTERNAL_FILE - "1c:dhivV" -#else - "1dhivV" -#endif - , + while ((c=getopt_long(argc, argv, "1dhivV", long_options, &options_index)) != -1) { switch (c) { case '1': @@ -263,7 +300,6 @@ main(int argc, char* argv[]) case 'h': usage(stdout); exit(0); - break; case 'i': info = 1; break; @@ -273,11 +309,9 @@ main(int argc, char* argv[]) case 'V': version(stdout); exit(0); - break; default: usage(stderr); exit(2); - break; } } argc -= optind; @@ -289,34 +323,94 @@ main(int argc, char* argv[]) #ifdef ENFORCER_TIMESHIFT if (getenv("ENFORCER_TIMESHIFT")) { - fprintf(stdout, "WARNING: timeshift %s detected, this is a fixed point in time.\n", - getenv("ENFORCER_TIMESHIFT")); + fprintf(stdout, "WARNING: timeshift %s detected, this is a" + " fixed point in time.\n", getenv("ENFORCER_TIMESHIFT")); } else { fprintf(stdout, "DEBUG: timeshift mode enabled, but not set.\n"); } #endif /* ENFORCER_TIMESHIFT */ /* main stuff */ - fprintf(stdout, "OpenDNSSEC key and signing policy enforcer version %s\n", PACKAGE_VERSION); + fprintf(stdout, "OpenDNSSEC key and signing policy enforcer version %s\n", + PACKAGE_VERSION); - { - engine_type *engine; - if ((engine = engine_start(cfgfile, cmdline_verbosity, daemonize, info))) { - engine_setup(engine,enforcer_commands,enforcer_help); - /* if setup fails we need a non-zero exit code */ - if (engine->setup_error) { - fprintf(stdout, "Setup failed. Aborting.\n"); - exit(3); + program_setup(cmdline_verbosity); /* setup basic logging, xml, PB */ + engine = engine_alloc(); /* Let's create an engine only once */ + if (!engine) { + ods_log_crit("Could not start engine"); + program_teardown(); + return 1; + } + engine_init(engine, daemonize, enforcer_commands, enforcer_help); + + returncode = 0; + while (!engine->need_to_exit) { + /* Parse config file */ + cfg = engine_config(engine->allocator, cfgfile, + cmdline_verbosity, engine->config); + engine->database_ready = database_ready(cfg); + /* does it make sense? */ + if (engine_config_check(cfg) != ODS_STATUS_OK) { + /* it does not, do we have a previous config loaded? */ + /* + * We can not recover since hsm_open tries to parse + * this file as well, in the future we need to use + * hsm_open2 + * + * if (engine->config) { + ods_log_error("[%s] cfgfile %s has errors, continuing" + " with old config", enforcerd_str, cfgfile); + } else {*/ + ods_log_crit("[%s] cfgfile %s has errors", enforcerd_str, cfgfile); + returncode = 2; + engine_config_cleanup(cfg); /* antagonist of engine_config() */ + break; + /*}*/ + } else { + engine_config_cleanup(engine->config); /* antagonist of engine_config() */ + engine->config = cfg; + } + + /* Print config and exit */ + if (info) { + engine_config_print(stdout, engine->config); /* for debugging */ + break; + } + + if (engine->database_ready) { + if (!perform_update_kasp(-1, engine->config) || + !perform_update_keyzones(-1, engine->config)) + { + ods_log_error("Errors found in one of the XML files. " + "Please run ods-kaspcheck and consult the log files."); + returncode = 5; + break; } - engine_runloop(engine,autostart,single_run); - if (engine->setup_error) { - fprintf(stdout, "Setup failed. Aborting.\n"); - exit(4); + perform_update_hsmkeys(-1, engine->config, 0); + } + + /* do daemon housekeeping: pid, privdrop, fork, log */ + if ((status = engine_setup(engine)) != ODS_STATUS_OK) { + ods_log_error("setup failed: %s", ods_status2str(status)); + if (!daemonize) + fprintf(stderr, "setup failed: %s\n", ods_status2str(status)); + returncode = 3; + engine->need_to_exit = 1; + } else { + if (engine_run(engine, autostart, single_run)) { + returncode = 4; + engine->need_to_exit = 1; } - engine_stop(engine); + engine_teardown(engine); /* antagonist of engine_setup() */ } + if (!engine->need_to_exit) + ods_log_info("[%s] enforcer reloading", enforcerd_str); } - - /* done */ - return 0; + engine_config_cleanup(engine->config); + engine_dealloc(engine); /* antagonist of engine_alloc() */ + ods_log_info("[engine] enforcer shutdown"); /* needed for test */ + ods_log_info("[%s] enforcerd stopped with exitcode %d", + enforcerd_str, returncode); + program_teardown(); /* antagonist of program_setup() */ + return returncode; } diff --git a/enforcer-ng/src/policy/update_kasp_task.h b/enforcer-ng/src/policy/update_kasp_task.h index 223773064c..1c23ee9782 100644 --- a/enforcer-ng/src/policy/update_kasp_task.h +++ b/enforcer-ng/src/policy/update_kasp_task.h @@ -35,6 +35,14 @@ #include "daemon/cfg.h" #include "scheduler/task.h" +#ifdef __cplusplus +extern "C" { +#endif + bool perform_update_kasp(int sockfd, engineconfig_type *config); +#ifdef __cplusplus +} +#endif + #endif diff --git a/enforcer-ng/src/shared/allocator.c b/enforcer-ng/src/shared/allocator.c index 555a792432..00c124a969 100644 --- a/enforcer-ng/src/shared/allocator.c +++ b/enforcer-ng/src/shared/allocator.c @@ -121,6 +121,7 @@ allocator_alloc_init(allocator_type *allocator, size_t size, const void *init) char* allocator_strdup(allocator_type *allocator, const char *string) { + if (!string) return NULL; return (char*) allocator_alloc_init(allocator, strlen(string) + 1, string); } diff --git a/enforcer-ng/src/shared/locks.h b/enforcer-ng/src/shared/locks.h index acfc40b53c..368d3692fa 100644 --- a/enforcer-ng/src/shared/locks.h +++ b/enforcer-ng/src/shared/locks.h @@ -72,6 +72,7 @@ typedef pthread_cond_t cond_basic_type; #define lock_basic_init(lock) LOCKRET(pthread_mutex_init(lock, NULL)) #define lock_basic_destroy(lock) LOCKRET(pthread_mutex_destroy(lock)) #define lock_basic_lock(lock) LOCKRET(pthread_mutex_lock(lock)) +#define lock_basic_trylock(lock) pthread_mutex_trylock(lock) #define lock_basic_unlock(lock) LOCKRET(pthread_mutex_unlock(lock)) /** our own alarm clock */ From bee77156d1ce4eebd53d0463bb66afdf2f892efa Mon Sep 17 00:00:00 2001 From: Yuri Schaeffer Date: Tue, 4 Mar 2014 10:50:52 +0100 Subject: [PATCH 2/5] Never reload the kasp or zonelist on restart or reload, only on setup and explicit command. The config is reread so paths to kasp and zonelist are up to date. --- enforcer-ng/src/ods-enforcerd.c | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/enforcer-ng/src/ods-enforcerd.c b/enforcer-ng/src/ods-enforcerd.c index 4224d33177..977aa0fcc7 100644 --- a/enforcer-ng/src/ods-enforcerd.c +++ b/enforcer-ng/src/ods-enforcerd.c @@ -377,18 +377,6 @@ main(int argc, char* argv[]) break; } - if (engine->database_ready) { - if (!perform_update_kasp(-1, engine->config) || - !perform_update_keyzones(-1, engine->config)) - { - ods_log_error("Errors found in one of the XML files. " - "Please run ods-kaspcheck and consult the log files."); - returncode = 5; - break; - } - perform_update_hsmkeys(-1, engine->config, 0); - } - /* do daemon housekeeping: pid, privdrop, fork, log */ if ((status = engine_setup(engine)) != ODS_STATUS_OK) { ods_log_error("setup failed: %s", ods_status2str(status)); From 28578b78b0ca8b382070b9740bda97df32c1188a Mon Sep 17 00:00:00 2001 From: Yuri Schaeffer Date: Tue, 4 Mar 2014 11:07:06 +0100 Subject: [PATCH 3/5] remove some redundant code --- enforcer-ng/src/daemon/cmdhandler.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/enforcer-ng/src/daemon/cmdhandler.c b/enforcer-ng/src/daemon/cmdhandler.c index d9e3949f26..742c12a03f 100644 --- a/enforcer-ng/src/daemon/cmdhandler.c +++ b/enforcer-ng/src/daemon/cmdhandler.c @@ -696,7 +696,6 @@ cmdhandler_start(cmdhandler_type* cmdhandler) struct sockaddr_un cliaddr; socklen_t clilen; cmdhandler_type* cmdc = NULL; - engine_type* engine = NULL; fd_set rset; int connfd = 0; int ret = 0; @@ -705,7 +704,6 @@ cmdhandler_start(cmdhandler_type* cmdhandler) ods_log_assert(cmdhandler->engine); ods_log_debug("[%s] start", module_str); - engine = cmdhandler->engine; ods_thread_detach(cmdhandler->thread_id); FD_ZERO(&rset); while (cmdhandler->need_to_exit == 0) { @@ -749,8 +747,7 @@ cmdhandler_start(cmdhandler_type* cmdhandler) } ods_log_debug("[%s] done", module_str); - engine = cmdhandler->engine;/*todo check & remove me*/ - engine->cmdhandler_done = 1; + cmdhandler->engine->cmdhandler_done = 1; return; } From ab8a776dc8b7d14adb72c419a688091905c6d43a Mon Sep 17 00:00:00 2001 From: Yuri Schaeffer Date: Tue, 4 Mar 2014 11:19:08 +0100 Subject: [PATCH 4/5] libhsm.h is not a system library --- enforcer-ng/src/daemon/engine.c | 2 +- enforcer-ng/src/enforcer/enforcer.cpp | 2 +- enforcer-ng/src/enforcer/update_repositorylist_task.cpp | 3 +-- enforcer-ng/src/shared/hsm.h | 2 +- enforcer-ng/src/shared/keys.h | 2 +- libhsm/checks/hsmcheck.c | 2 +- libhsm/src/bin/hsmspeed.c | 2 +- libhsm/src/bin/hsmtest.c | 2 +- libhsm/src/bin/hsmutil.c | 2 +- signer/src/daemon/engine.c | 2 +- signer/src/shared/hsm.h | 2 +- signer/src/signer/keys.h | 2 +- signer/src/signer/rrset.h | 2 +- 13 files changed, 13 insertions(+), 14 deletions(-) diff --git a/enforcer-ng/src/daemon/engine.c b/enforcer-ng/src/daemon/engine.c index 7822f3dddc..b136a89aaa 100644 --- a/enforcer-ng/src/daemon/engine.c +++ b/enforcer-ng/src/daemon/engine.c @@ -49,9 +49,9 @@ #include "shared/status.h" #include "shared/util.h" #include "shared/protobuf.h" +#include "libhsm.h" #include -#include #include #include #include diff --git a/enforcer-ng/src/enforcer/enforcer.cpp b/enforcer-ng/src/enforcer/enforcer.cpp index a8e0560ad4..25ddec39a0 100644 --- a/enforcer-ng/src/enforcer/enforcer.cpp +++ b/enforcer-ng/src/enforcer/enforcer.cpp @@ -49,8 +49,8 @@ #include "enforcer/enforcerdata.h" #include "policy/kasp.pb.h" #include "hsmkey/hsmkey.pb.h" +#include "libhsm.h" -#include #include #include diff --git a/enforcer-ng/src/enforcer/update_repositorylist_task.cpp b/enforcer-ng/src/enforcer/update_repositorylist_task.cpp index 7179320c27..a0c5c184b5 100644 --- a/enforcer-ng/src/enforcer/update_repositorylist_task.cpp +++ b/enforcer-ng/src/enforcer/update_repositorylist_task.cpp @@ -38,11 +38,10 @@ #include "shared/status.h" #include "utils/kc_helper.h" #include "daemon/engine.h" +#include "libhsm.h" #include "update_repositorylist_task.h" - -#include #include #include #include diff --git a/enforcer-ng/src/shared/hsm.h b/enforcer-ng/src/shared/hsm.h index 1bf052183a..acb8c238ed 100644 --- a/enforcer-ng/src/shared/hsm.h +++ b/enforcer-ng/src/shared/hsm.h @@ -37,12 +37,12 @@ #include "config.h" #include "shared/status.h" #include "shared/keys.h" +#include "libhsm.h" #include #include #include -#include #include #ifdef __cplusplus diff --git a/enforcer-ng/src/shared/keys.h b/enforcer-ng/src/shared/keys.h index b926df15ce..e743ecfbc7 100644 --- a/enforcer-ng/src/shared/keys.h +++ b/enforcer-ng/src/shared/keys.h @@ -36,6 +36,7 @@ #include "shared/allocator.h" #include "shared/status.h" +#include "libhsm.h" #ifdef HAVE_SYS_TYPES_H # include @@ -44,7 +45,6 @@ # include #endif #include -#include #include #ifdef __cplusplus diff --git a/libhsm/checks/hsmcheck.c b/libhsm/checks/hsmcheck.c index bb8c1b6a4c..a96b80a4ce 100644 --- a/libhsm/checks/hsmcheck.c +++ b/libhsm/checks/hsmcheck.c @@ -33,7 +33,7 @@ #include #include -#include +#include "libhsm.h" #include extern char *optarg; diff --git a/libhsm/src/bin/hsmspeed.c b/libhsm/src/bin/hsmspeed.c index 6a54a55edb..732f625945 100644 --- a/libhsm/src/bin/hsmspeed.c +++ b/libhsm/src/bin/hsmspeed.c @@ -34,7 +34,7 @@ #include #include -#include +#include "libhsm.h" #include #define PTHREAD_THREADS_MAX 2048 diff --git a/libhsm/src/bin/hsmtest.c b/libhsm/src/bin/hsmtest.c index 1524927fc1..c40351ced9 100644 --- a/libhsm/src/bin/hsmtest.c +++ b/libhsm/src/bin/hsmtest.c @@ -34,7 +34,7 @@ #include #include -#include +#include "libhsm.h" #include diff --git a/libhsm/src/bin/hsmutil.c b/libhsm/src/bin/hsmutil.c index 46f84f4626..9cb40d2986 100644 --- a/libhsm/src/bin/hsmutil.c +++ b/libhsm/src/bin/hsmutil.c @@ -36,7 +36,7 @@ #include #include -#include +#include "libhsm.h" #include diff --git a/signer/src/daemon/engine.c b/signer/src/daemon/engine.c index ed810cde9a..add3c8ccb6 100644 --- a/signer/src/daemon/engine.c +++ b/signer/src/daemon/engine.c @@ -46,9 +46,9 @@ #include "shared/util.h" #include "signer/zonelist.h" #include "wire/tsig.h" +#include "libhsm.h" #include -#include #include #include #include diff --git a/signer/src/shared/hsm.h b/signer/src/shared/hsm.h index 17ec3c394a..9edf8d9232 100644 --- a/signer/src/shared/hsm.h +++ b/signer/src/shared/hsm.h @@ -37,12 +37,12 @@ #include "config.h" #include "shared/status.h" #include "signer/keys.h" +#include "libhsm.h" #include #include #include -#include #include /** diff --git a/signer/src/signer/keys.h b/signer/src/signer/keys.h index c648d7d6c3..ed770caaf7 100644 --- a/signer/src/signer/keys.h +++ b/signer/src/signer/keys.h @@ -36,6 +36,7 @@ #include "shared/allocator.h" #include "shared/status.h" +#include "libhsm.h" #ifdef HAVE_SYS_TYPES_H # include @@ -44,7 +45,6 @@ # include #endif #include -#include #include diff --git a/signer/src/signer/rrset.h b/signer/src/signer/rrset.h index 66fe857b7b..164b97b5a5 100644 --- a/signer/src/signer/rrset.h +++ b/signer/src/signer/rrset.h @@ -37,9 +37,9 @@ #include "config.h" #include "shared/status.h" #include "signer/stats.h" +#include "libhsm.h" #include -#include /** * RRSIG. From fe23a682bc708cb59130554153346666794dba0c Mon Sep 17 00:00:00 2001 From: Yuri Schaeffer Date: Tue, 4 Mar 2014 14:39:21 +0100 Subject: [PATCH 5/5] try workaound to pass test on RHEL. Locking issue causes race condition and delay in enforce task. --- enforcer-ng/src/enforcer/autostart_cmd.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/enforcer-ng/src/enforcer/autostart_cmd.cpp b/enforcer-ng/src/enforcer/autostart_cmd.cpp index 5c57d22d85..1d68b9c09e 100644 --- a/enforcer-ng/src/enforcer/autostart_cmd.cpp +++ b/enforcer-ng/src/enforcer/autostart_cmd.cpp @@ -90,10 +90,16 @@ database_ready(engineconfig_type* config) void autostart(engine_type* engine) { + task_type *resalt_task; ods_log_debug("[%s] autostart", module_str); if (!engine->database_ready) return; - schedule_task(engine, policy_resalt_task(engine->config), "resalt"); + if (resalt_task = policy_resalt_task(engine->config)) { + /* race condition at startup. Make sure resalt loses over + * enforce. Not fatal but disturbs test. */ + resalt_task->when += 3; + } + schedule_task(engine, resalt_task, "resalt"); schedule_task(engine, enforce_task(engine, 1), "enforce"); }