From 6b536ce30c3e7910b5af682315f8258d224bcbf2 Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Jul 10 2014 19:49:48 +0000 Subject: Handle "wait" status for CA data When attempting to retrieve CA data, don't treat "wait" exit status values as if we were indicating that the server was unreachable. Instead, poll the helper for the information later (#1118468). Also, if the server's unreachable, poll later at a fixed time even if nothing interesting happens to the routing table. --- diff --git a/configure.ac b/configure.ac index fcd699b..4707d94 100644 --- a/configure.ac +++ b/configure.ac @@ -577,6 +577,7 @@ if ! ${configure_dist_target_only:-false} ; then AC_DEFINE(CM_DELAY_SOON,5,[Define to the time to wait for something that will happen soon.]) AC_DEFINE(CM_DELAY_SOONISH,30,[Define to the time to wait for something that will happen soon, but not that soon.]) AC_DEFINE(CM_DELAY_CA_POLL,(7 * 24 * 60 * 60),[Define to the time to wait between attempts to see if the CA issued a certificate.]) + AC_DEFINE(CM_DELAY_CADATA_POLL,(6 * 60 * 60),[Define to the time to wait between attempts to fetch the CA certificate, if it was unreachable.]) AC_DEFINE(CM_DELAY_CA_POLL_MINIMUM,(5 * 60),[Define to the absolute minimum time to wait between attempts to see if the CA issued a certificate.]) AC_DEFINE(CM_DELAY_MONITOR_POLL,(24 * 60 * 60),[Define to the time to wait between attempts to re-read a certificate and check for expiration.]) AC_DEFINE(CM_DELAY_MONITOR_POLL_MINIMUM,(30 * 60),[Define to the absolute minimum time to wait between attempts to re-read a certificate and check for expiration.]) diff --git a/doc/design.txt b/doc/design.txt index cbe52d1..41a121c 100644 --- a/doc/design.txt +++ b/doc/design.txt @@ -711,6 +711,9 @@ And this state machine: if ca-gave-us-data state_next = NEED_TO_SAVE_DATA state_transition = now + elseif ca-needs-retry + state_next = NEED_TO_REFRESH + state_transition = later elseif ca-is-unreachable state_next = UNREACHABLE state_transition = later @@ -783,3 +786,7 @@ And this state machine: else: state_next = IDLE state_transition = now + + UNREACHABLE: + state_next = NEED_TO_REFRESH + state_transition = now diff --git a/src/cadata.c b/src/cadata.c index b805e09..47df25d 100644 --- a/src/cadata.c +++ b/src/cadata.c @@ -51,7 +51,7 @@ struct cm_cadata_state { void (*parse)(struct cm_store_ca *ca, struct cm_cadata_state *state, const char *msg); const char *op; - int error_fd; + int error_fd, delay; unsigned int modified: 1; }; @@ -479,6 +479,7 @@ cm_cadata_start_generic(struct cm_store_ca *ca, const char *op, memset(ret, 0, sizeof(*ret)); ret->ca = ca; ret->error_fd = error_fd[1]; + ret->delay = -1; ret->op = op; ret->modified = 0; ret->subproc = cm_subproc_start(fetch, ca, NULL, ret); @@ -545,13 +546,35 @@ cm_cadata_start_renew_reqs(struct cm_store_ca *ca) int cm_cadata_ready(struct cm_cadata_state *state) { - int ready, length; + int ready, status, length; + const char *msg = NULL; + char *p = NULL; + long delay = -1; ready = cm_subproc_ready(state->subproc); - if ((ready == 0) && - (cm_subproc_get_exitstatus(state->subproc) == 0)) { - (*(state->parse))(state->ca, state, - cm_subproc_get_msg(state->subproc, &length)); + if (ready == 0) { + status = cm_subproc_get_exitstatus(state->subproc); + msg = cm_subproc_get_msg(state->subproc, &length); + if (WIFEXITED(status)) { + switch (WEXITSTATUS(status)) { + case CM_SUBMIT_STATUS_ISSUED: + (*(state->parse))(state->ca, state, msg); + break; + case CM_SUBMIT_STATUS_WAIT_WITH_DELAY: + delay = -1; + if (length > 0) { + delay = strtol(msg, &p, 10); + if ((p != NULL) && + ((*p == '\0') || + (strchr("\r\n", *p) != NULL))) { + state->delay = delay; + } + } + break; + default: + break; + } + } } return ready; } @@ -582,20 +605,13 @@ cm_cadata_unsupported(struct cm_cadata_state *state) } int -cm_cadata_unreachable(struct cm_cadata_state *state) +cm_cadata_needs_retry(struct cm_cadata_state *state) { int status; status = cm_subproc_get_exitstatus(state->subproc); - /* Go ahead and treat "try later" as an "unreachable" error, even - * though helpers aren't supposed to ever return either of these values - * for these cases, so that we don't permanently disable the helper - * when it's just telling us to try again, even though it's doing it - * wrong. We leave out "rejected" errors, because that's not something - * we'd retry even if the result made sense for these cases. */ if (WIFEXITED(status) && - ((WEXITSTATUS(status) == CM_SUBMIT_STATUS_UNREACHABLE) || - (WEXITSTATUS(status) == CM_SUBMIT_STATUS_WAIT) || + ((WEXITSTATUS(status) == CM_SUBMIT_STATUS_WAIT) || (WEXITSTATUS(status) == CM_SUBMIT_STATUS_WAIT_WITH_DELAY))) { return 0; } @@ -603,6 +619,25 @@ cm_cadata_unreachable(struct cm_cadata_state *state) } int +cm_cadata_specified_delay(struct cm_cadata_state *state) +{ + return state->delay; +} + +int +cm_cadata_unreachable(struct cm_cadata_state *state) +{ + int status; + + status = cm_subproc_get_exitstatus(state->subproc); + if (WIFEXITED(status) && + (WEXITSTATUS(status) == CM_SUBMIT_STATUS_UNREACHABLE)) { + return 0; + } + return -1; +} + +int cm_cadata_unconfigured(struct cm_cadata_state *state) { int status; diff --git a/src/cadata.h b/src/cadata.h index 53a477f..08e0279 100644 --- a/src/cadata.h +++ b/src/cadata.h @@ -38,6 +38,12 @@ int cm_cadata_get_fd(struct cm_cadata_state *state); /* Check if the CA data was modified. */ int cm_cadata_modified(struct cm_cadata_state *state); +/* Check if we need to retry. */ +int cm_cadata_needs_retry(struct cm_cadata_state *state); + +/* Check when we need to retry. */ +int cm_cadata_specified_delay(struct cm_cadata_state *state); + /* Check if the CA was unreachable. */ int cm_cadata_unreachable(struct cm_cadata_state *state); diff --git a/src/iterate.c b/src/iterate.c index c29e360..84750eb 100644 --- a/src/iterate.c +++ b/src/iterate.c @@ -252,6 +252,7 @@ static time_t cm_decide_ca_delay(time_t remaining) { time_t delay; + delay = CM_DELAY_CA_POLL; if ((remaining != (time_t) -1) && (remaining < 2 * delay)) { delay = remaining / 2; @@ -267,6 +268,7 @@ static time_t cm_decide_monitor_delay(time_t remaining) { time_t delay; + delay = CM_DELAY_MONITOR_POLL; if ((remaining != (time_t) -1) && (remaining < 2 * delay)) { delay = remaining / 2; @@ -277,6 +279,17 @@ cm_decide_monitor_delay(time_t remaining) return delay; } +/* Decide how long to wait before attempting to contact the CA to retrieve + * information again. */ +static time_t +cm_decide_cadata_delay(void) +{ + time_t delay; + + delay = CM_DELAY_CADATA_POLL; + return delay; +} + /* Manage a "lock" that we use to serialize access to THE REST OF THE WORLD. */ static void *writing_lock; static enum cm_ca_phase writing_lock_ca_phase = cm_ca_phase_invalid; @@ -833,6 +846,9 @@ cm_iterate_entry(struct cm_store_entry *entry, struct cm_store_ca *ca, /* Saved CA's identifier for our request; give * it the specified time, or a little time, and * then ask for a progress update. */ + cm_log(4, "%s('%s') provided CA " + "cookie \"%s\"\n", entry->cm_busname, + entry->cm_nickname, entry->cm_ca_cookie); *delay = cm_submit_specified_delay(state->cm_submit_state); cm_submit_done(state->cm_submit_state); state->cm_submit_state = NULL; @@ -1853,6 +1869,19 @@ cm_iterate_ca(struct cm_store_ca *ca, } *when = cm_time_now; } else + if (cm_cadata_needs_retry(state->cm_task_state) == 0) { + *when = cm_time_delay; + *delay = cm_cadata_specified_delay(state->cm_task_state); + if (*delay < 0) { + *delay = cm_decide_cadata_delay(); + } + cm_cadata_done(state->cm_task_state); + state->cm_task_state = NULL; + cm_log(3, "%s('%s').%s server needs retry\n", + ca->cm_busname, ca->cm_nickname, + cm_store_ca_phase_as_string(state->cm_phase)); + ca->cm_ca_state[state->cm_phase] = CM_CA_NEED_TO_REFRESH; + } else if (cm_cadata_unreachable(state->cm_task_state) == 0) { cm_cadata_done(state->cm_task_state); state->cm_task_state = NULL; @@ -1860,7 +1889,8 @@ cm_iterate_ca(struct cm_store_ca *ca, ca->cm_busname, ca->cm_nickname, cm_store_ca_phase_as_string(state->cm_phase)); ca->cm_ca_state[state->cm_phase] = CM_CA_DATA_UNREACHABLE; - *when = cm_time_now; + *when = cm_time_delay; + *delay = cm_decide_cadata_delay(); } else if (cm_cadata_unsupported(state->cm_task_state) == 0) { cm_cadata_done(state->cm_task_state); @@ -2096,8 +2126,10 @@ cm_iterate_ca(struct cm_store_ca *ca, } } break; - break; case CM_CA_DATA_UNREACHABLE: + ca->cm_ca_state[state->cm_phase] = CM_CA_NEED_TO_REFRESH; + *when = cm_time_soonish; + break; case CM_CA_IDLE: case CM_CA_DISABLED: *when = cm_time_no_time; diff --git a/tests/024-citerate/expected.out b/tests/024-citerate/expected.out index bd430a0..672f3e0 100644 --- a/tests/024-citerate/expected.out +++ b/tests/024-citerate/expected.out @@ -22,7 +22,8 @@ ca_external_helper=${tmpdir}/ca-data [identify:UNREACHABLE] UNREACHABLE -START- --STUCK- (4:0) +NEED_TO_REFRESH +-STOP- id=Test CA ca_is_default=0 ca_type=EXTERNAL @@ -121,7 +122,8 @@ ca_root_certs=Root [certs:UNREACHABLE] UNREACHABLE -START- --STUCK- (4:0) +NEED_TO_REFRESH +-STOP- id=Test CA ca_is_default=0 ca_type=EXTERNAL @@ -226,7 +228,8 @@ ca_profiles=None [profiles:UNREACHABLE] UNREACHABLE -START- --STUCK- (4:0) +NEED_TO_REFRESH +-STOP- id=Test CA ca_is_default=0 ca_type=EXTERNAL @@ -301,7 +304,8 @@ ca_default_profile=None [default_profile:UNREACHABLE] UNREACHABLE -START- --STUCK- (4:0) +NEED_TO_REFRESH +-STOP- id=Test CA ca_is_default=0 ca_type=EXTERNAL @@ -375,7 +379,8 @@ ca_external_helper=${tmpdir}/ca-data [enrollment_reqs:UNREACHABLE] UNREACHABLE -START- --STUCK- (4:0) +NEED_TO_REFRESH +-STOP- id=Test CA ca_is_default=0 ca_type=EXTERNAL @@ -448,7 +453,8 @@ ca_external_helper=${tmpdir}/ca-data [renewal_reqs:UNREACHABLE] UNREACHABLE -START- --STUCK- (4:0) +NEED_TO_REFRESH +-STOP- id=Test CA ca_is_default=0 ca_type=EXTERNAL