Reload the certificates and private keys on SIGHUP

Until now this part of the configuration was static, but
there is the need to reload certificates and keys, e.g., on
renewal.

doc/sample.config

@@ -139,7 +139,13 @@ ca-cert = ../tests/ca.pem
 
 
 ### All configuration options below this line are reloaded on a SIGHUP.
-### The options above, will remain unchanged.
+### The options above, will remain unchanged. Note however, that the 
+### server-cert, server-key, dh-params and ca-cert options will be reloaded
+### if the provided file changes, on server reload. That allows certificate
+### rotation, but requires the server key to remain the same for seamless
+### operation. If the server key changes on reload, there may be connection
+### failures during the reloading time.
+
 
 # Whether to enable seccomp/Linux namespaces worker isolation. That restricts the number of 
 # system calls allowed to a worker process, in order to reduce damage from a

src/main.c

@@ -933,10 +933,20 @@ static void reload_sig_watcher_cb(struct ev_loop *loop, ev_signal *w, int revent
 	main_server_st *s = ev_userdata(loop);
 
 	mslog(s, NULL, LOG_INFO, "reloading configuration");
+	kill(s->sec_mod_pid, SIGHUP);
+
 	reload_cfg_file(s->main_pool, s->perm_config, 1);
 	s->config = s->perm_config->config;
+
+	/* These must be called after sec-mod has been sent SIGHUP.
+	 * That's because of a test that the certificate matches the
+	 * used key in certain gnutls versions */
+#if GNUTLS_VERSION_NUMBER < 0x030407
+	ms_sleep(1000);
+#endif
+	tls_load_files(s, s->creds);
+	tls_load_prio(s, s->creds);
 	tls_reload_crl(s, s->creds, 1);
-	kill(s->sec_mod_pid, SIGHUP);
 }
 
 static void cmd_watcher_cb (EV_P_ ev_io *w, int revents)
@@ -1261,7 +1271,8 @@ int main(int argc, char** argv)
 	ms_sleep(100); /* give some time for sec-mod to initialize */
 
 	/* Initialize certificates */
-	tls_load_certs(s, &creds);
+	tls_load_files(s, &creds);
+	tls_load_prio(s, s->creds);
 
 	s->secmod_addr.sun_family = AF_UNIX;
 	p = s->socket_file;

src/ocserv-args.def

@@ -222,7 +222,13 @@ server-key = /path/to/key.pem
 
 
 ### All configuration options below this line are reloaded on a SIGHUP.
-### The options above, will remain unchanged.
+### The options above, will remain unchanged. Note however, that the 
+### server-cert, server-key, dh-params and ca-cert options will be reloaded
+### if the provided file changes, on server reload. That allows certificate
+### rotation, but requires the server key to remain the same for seamless
+### operation. If the server key changes on reload, there may be connection
+### failures during the reloading time.
+
 
 # Whether to enable seccomp/Linux namespaces worker isolation. That restricts the number of 
 # system calls allowed to a worker process, in order to reduce damage from a

src/sec-mod.c

@@ -61,6 +61,8 @@ struct pin_st {
 	char srk_pin[MAX_PIN_SIZE];
 };
 
+static int load_keys(sec_mod_st *sec, unsigned force);
+
 static
 int pin_callback(void *user, int attempt, const char *token_url,
 		 const char *token_label, unsigned int flags, char *pin,
@@ -535,6 +537,7 @@ static void check_other_work(sec_mod_st *sec)
 		seclog(sec, LOG_DEBUG, "reloading configuration");
 		reload_cfg_file(sec, sec->perm_config, 0);
 		sec->config = sec->perm_config->config;
+		load_keys(sec, 0);
 		need_reload = 0;
 	}
 
@@ -654,6 +657,95 @@ int serve_request_worker(sec_mod_st *sec, int cfd, pid_t pid, uint8_t *buffer, u
 	return ret;
 }
 
+#define CHECK_LOOP_ERR(x) \
+	if (force != 0) { GNUTLS_FATAL_ERR(x); } \
+	else { if (ret < 0) { \
+		seclog(sec, LOG_ERR, "could not reload key %s", sec->perm_config->key[i]); \
+		continue; } \
+	}
+
+static int load_keys(sec_mod_st *sec, unsigned force)
+{
+	unsigned i, need_reload = 0;
+	int ret;
+	struct pin_st pins;
+	static time_t last_access = 0;
+
+	for (i = 0; i < sec->perm_config->key_size; i++) {
+		if (need_file_reload(sec->perm_config->key[i], last_access) != 0) {
+			need_reload = 1;
+			break;
+		}
+	}
+
+	if (need_reload == 0)
+		return 0;
+
+	last_access = time(0);
+
+	ret = load_pins(sec->perm_config, &pins);
+	if (ret < 0) {
+		seclog(sec, LOG_ERR, "error loading PIN files");
+		exit(1);
+	}
+
+	/* Reminder: the number of private keys or their filenames cannot be changed on reload
+	 */
+	if (sec->key == NULL) {
+		sec->key_size = sec->perm_config->key_size;
+		sec->key = talloc_size(sec, sizeof(*sec->key) * sec->perm_config->key_size);
+		if (sec->key == NULL) {
+			seclog(sec, LOG_ERR, "error in memory allocation");
+			exit(1);
+		}
+	}
+
+	/* read private keys */
+	for (i = 0; i < sec->key_size; i++) {
+		gnutls_privkey_t p;
+
+		ret = gnutls_privkey_init(&p);
+		CHECK_LOOP_ERR(ret);
+		/* load the private key */
+		if (gnutls_url_is_supported(sec->perm_config->key[i]) != 0) {
+			gnutls_privkey_set_pin_function(p,
+							pin_callback, &pins);
+			ret =
+			    gnutls_privkey_import_url(p,
+						      sec->perm_config->key[i], 0);
+			CHECK_LOOP_ERR(ret);
+		} else {
+			gnutls_datum_t data;
+			ret = gnutls_load_file(sec->perm_config->key[i], &data);
+			if (ret < 0) {
+				seclog(sec, LOG_ERR, "error loading file '%s'",
+				       sec->perm_config->key[i]);
+				CHECK_LOOP_ERR(ret);
+			}
+
+			ret =
+			    gnutls_privkey_import_x509_raw(p, &data,
+							   GNUTLS_X509_FMT_PEM,
+							   NULL, 0);
+			if (ret == GNUTLS_E_DECRYPTION_FAILED && pins.pin[0]) {
+				ret =
+				    gnutls_privkey_import_x509_raw(p, &data,
+								   GNUTLS_X509_FMT_PEM,
+								   pins.pin, 0);
+			}
+			CHECK_LOOP_ERR(ret);
+
+			gnutls_free(data.data);
+		}
+
+		if (sec->key[i] != NULL)
+			gnutls_privkey_deinit(sec->key[i]);
+		sec->key[i] = p;
+	}
+
+	return 0;
+}
+
 /* sec_mod_server:
  * @config: server configuration
  * @socket_file: the name of the socket
@@ -689,10 +781,9 @@ void sec_mod_server(void *main_pool, struct perm_cfg_st *perm_config, const char
 	struct sockaddr_un sa;
 	socklen_t sa_len;
 	int cfd, ret, e, n;
-	unsigned i, buffer_size;
+	unsigned buffer_size;
 	uid_t uid;
 	uint8_t *buffer;
-	struct pin_st pins;
 	int sd;
 	sec_mod_st *sec;
 	void *sec_mod_pool;
@@ -803,58 +894,12 @@ void sec_mod_server(void *main_pool, struct perm_cfg_st *perm_config, const char
 		exit(1);
 	}
 
-	ret = load_pins(sec->perm_config, &pins);
+	ret = load_keys(sec, 1);
 	if (ret < 0) {
-		seclog(sec, LOG_ERR, "error loading PIN files");
+		seclog(sec, LOG_ERR, "error loading private key files");
 		exit(1);
 	}
 
-	/* FIXME: the private key isn't reloaded on reload */
-	sec->key_size = sec->perm_config->key_size;
-	sec->key = talloc_size(sec, sizeof(*sec->key) * sec->perm_config->key_size);
-	if (sec->key == NULL) {
-		seclog(sec, LOG_ERR, "error in memory allocation");
-		exit(1);
-	}
-
-	/* read private keys */
-	for (i = 0; i < sec->key_size; i++) {
-		ret = gnutls_privkey_init(&sec->key[i]);
-		GNUTLS_FATAL_ERR(ret);
-
-		/* load the private key */
-		if (gnutls_url_is_supported(sec->perm_config->key[i]) != 0) {
-			gnutls_privkey_set_pin_function(sec->key[i],
-							pin_callback, &pins);
-			ret =
-			    gnutls_privkey_import_url(sec->key[i],
-						      sec->perm_config->key[i], 0);
-			GNUTLS_FATAL_ERR(ret);
-		} else {
-			gnutls_datum_t data;
-			ret = gnutls_load_file(sec->perm_config->key[i], &data);
-			if (ret < 0) {
-				seclog(sec, LOG_ERR, "error loading file '%s'",
-				       sec->perm_config->key[i]);
-				GNUTLS_FATAL_ERR(ret);
-			}
-
-			ret =
-			    gnutls_privkey_import_x509_raw(sec->key[i], &data,
-							   GNUTLS_X509_FMT_PEM,
-							   NULL, 0);
-			if (ret == GNUTLS_E_DECRYPTION_FAILED && pins.pin[0]) {
-				ret =
-				    gnutls_privkey_import_x509_raw(sec->key[i], &data,
-								   GNUTLS_X509_FMT_PEM,
-								   pins.pin, 0);
-			}
-			GNUTLS_FATAL_ERR(ret);
-
-			gnutls_free(data.data);
-		}
-	}
-
 	sigprocmask(SIG_BLOCK, &blockset, &sig_default_set);
 	alarm(MAINTAINANCE_TIME);
 	seclog(sec, LOG_INFO, "sec-mod initialized (socket: %s)", SOCKET_FILE);

src/tlslib.c

@@ -626,7 +626,7 @@ int key_cb_common_func (gnutls_privkey_t key, void* userdata, const gnutls_datum
 	ret = connect(sd, (struct sockaddr *)&cdata->sa, cdata->sa_len);
 	if (ret == -1) {
 		e = errno;
-		syslog(LOG_ERR, "error connecting to sec-mod socket '%s': %s", 
+		syslog(LOG_ERR, "error connecting to sec-mod socket '%s': %s",
 			cdata->sa.sun_path, strerror(e));
 		goto error;
 	}
@@ -648,7 +648,7 @@ int key_cb_common_func (gnutls_privkey_t key, void* userdata, const gnutls_datum
 		       DEFAULT_SOCKET_TIMEOUT);
 	if (ret < 0) {
 		e = errno;
-		syslog(LOG_ERR, "error receiving sec-mod reply: %s", 
+		syslog(LOG_ERR, "error receiving sec-mod reply: %s",
 				strerror(e));
 		goto error;
 	}
@@ -698,13 +698,13 @@ static void key_cb_deinit_func(gnutls_privkey_t key, void* userdata)
 static
 int load_cert_files(main_server_st *s, tls_st *creds)
 {
-int ret;
-gnutls_pcert_st *pcert_list;
-unsigned pcert_list_size, i;
-gnutls_privkey_t key;
-gnutls_datum_t data;
-struct key_cb_data * cdata;
-unsigned flags;
+	int ret;
+	gnutls_pcert_st *pcert_list;
+	unsigned pcert_list_size, i;
+	gnutls_privkey_t key;
+	gnutls_datum_t data;
+	struct key_cb_data * cdata;
+	unsigned flags;
 
 	for (i=0;i<s->perm_config->key_size;i++) {
 		/* load the certificate */
@@ -770,17 +770,66 @@ unsigned flags;
 	return 0;
 }
 
-/* reload key files etc. */
-void tls_load_certs(main_server_st *s, tls_st *creds)
+unsigned need_file_reload(const char *file, time_t last_access)
 {
-int ret;
-const char* perr;
+	struct stat st;
+	int ret, e;
+
+	if (file == NULL || file[0] == 0)
+		return 0;
+
+	if (last_access == 0)
+		return 1;
+
+	ret = stat(file, &st);
+	if (ret == -1) {
+		e = errno;
+		syslog(LOG_INFO, "file %s (to be reloaded) was not found: %s",
+		      file, strerror(e));
+		return 0;
+	}
+
+	/* reload only if it is a newer file */
+	if (st.st_mtime > last_access)
+		return 1;
+	return 0;
+}
+
+/* reload key files etc. */
+void tls_load_files(main_server_st *s, tls_st *creds)
+{
+	int ret;
+	unsigned i;
+	static time_t last_access = 0;
+	unsigned need_reload = 0;
+
+	if (last_access != 0) {
+		for (i=0;i<s->perm_config->key_size;i++) {
+			if (need_file_reload(s->perm_config->cert[i], last_access) != 0) {
+				need_reload = 1;
+				break;
+			}
+		}
+
+		if (need_file_reload(s->perm_config->ca, last_access) ||
+		    need_file_reload(s->config->ocsp_response, last_access) ||
+		    need_file_reload(s->perm_config->dh_params_file, last_access)) {
+			need_reload = 1;
+		}
+
+		if (need_reload == 0)
+			return;
+
+		mslog(s, NULL, LOG_INFO, "reloading server certificates");
+	}
 
 	if (s->perm_config->debug >= DEBUG_TLS) {
 		gnutls_global_set_log_function(tls_log_func);
 		gnutls_global_set_log_level(9);
 	}
 
+ 	last_access = time(0);
+
 	if (creds->xcred != NULL)
 		gnutls_certificate_free_credentials(creds->xcred);
 
@@ -790,11 +839,17 @@ const char* perr;
 	set_dh_params(s, creds);
 
 	if (s->perm_config->key_size == 0 || s->perm_config->cert_size == 0) {
-		mslog(s, NULL, LOG_ERR, "no certificate or key files were specified"); 
+		mslog(s, NULL, LOG_ERR, "no certificate or key files were specified");
 		exit(1);
 	}
 
-	certificate_check(s);
+	/* on reload reduce any checks done */
+	if (need_reload) {
+#if GNUTLS_VERSION_NUMBER >= 0x030407
+		gnutls_certificate_set_flags(creds->xcred, GNUTLS_CERTIFICATE_SKIP_KEY_CERT_MATCH);
+#endif
+		certificate_check(s);
+	}
 
 	ret = load_cert_files(s, creds);
 	if (ret < 0) {
@@ -823,11 +878,6 @@ const char* perr;
 						       verify_certificate_cb);
 	}
 
-	ret = gnutls_priority_init(&creds->cprio, s->config->priorities, &perr);
-	if (ret == GNUTLS_E_PARSING_ERROR)
-		mslog(s, NULL, LOG_ERR, "error in TLS priority string: %s", perr);
-	GNUTLS_FATAL_ERR(ret);
-
 	if (s->config->ocsp_response != NULL) {
 		ret = gnutls_certificate_set_ocsp_status_request_file(creds->xcred,
 			s->config->ocsp_response, 0);
@@ -837,52 +887,57 @@ const char* perr;
 	return;
 }
 
+void tls_load_prio(main_server_st *s, tls_st *creds)
+{
+int ret;
+const char* perr;
+
+	ret = gnutls_priority_init(&creds->cprio, s->config->priorities, &perr);
+	if (ret == GNUTLS_E_PARSING_ERROR)
+		mslog(s, NULL, LOG_ERR, "error in TLS priority string: %s", perr);
+	GNUTLS_FATAL_ERR(ret);
+
+	return;
+}
+
 void tls_reload_crl(main_server_st* s, tls_st *creds, unsigned force)
 {
-int ret, e, saved_ret;
-static time_t old_mtime = 0;
+int ret, saved_ret;
+static time_t last_access = 0;
 static unsigned crl_type = GNUTLS_X509_FMT_PEM;
-struct stat st;
 
 	if (force)
-		old_mtime = 0;
+		last_access = 0;
 
 	if (s->config->cert_req != GNUTLS_CERT_IGNORE && s->config->crl != NULL) {
-		ret = stat(s->config->crl, &st);
-		if (ret == -1) {
-			e = errno;
-			mslog(s, NULL, LOG_INFO, "CRL file (%s) not found: %s; check documentation to generate an empty CRL",
-				s->config->crl, strerror(e));
+		if (need_file_reload(s->config->crl, last_access) == 0) {
+			mslog(s, NULL, LOG_DEBUG, "skipping already loaded CRL: %s", s->config->crl);
 			return;
 		}
 
-		/* reload only if it is a newer file */
-		if (st.st_mtime > old_mtime) {
+		last_access = time(0);
+
+		ret =
+		    gnutls_certificate_set_x509_crl_file(creds->xcred,
+							 s->config->crl,
+							 crl_type);
+		if (ret == GNUTLS_E_BASE64_DECODING_ERROR && crl_type == GNUTLS_X509_FMT_PEM) {
+			crl_type = GNUTLS_X509_FMT_DER;
+			saved_ret = ret;
 			ret =
 			    gnutls_certificate_set_x509_crl_file(creds->xcred,
 								 s->config->crl,
 								 crl_type);
-			if (ret == GNUTLS_E_BASE64_DECODING_ERROR && crl_type == GNUTLS_X509_FMT_PEM) {
-				crl_type = GNUTLS_X509_FMT_DER;
-				saved_ret = ret;
-				ret =
-				    gnutls_certificate_set_x509_crl_file(creds->xcred,
-									 s->config->crl,
-									 crl_type);
-				if (ret < 0)
-					ret = saved_ret;
-			}
-			if (ret < 0) {
-				/* ignore the CRL file when empty */
-				mslog(s, NULL, LOG_ERR, "error reading the CRL (%s) file: %s",
-					s->config->crl, gnutls_strerror(ret));
-				exit(1);
-			}
-			mslog(s, NULL, LOG_INFO, "loaded CRL: %s", s->config->crl);
-			old_mtime = st.st_mtime;
-		} else {
-			mslog(s, NULL, LOG_DEBUG, "skipping already loaded CRL: %s", s->config->crl);
+			if (ret < 0)
+				ret = saved_ret;
 		}
+		if (ret < 0) {
+			/* ignore the CRL file when empty */
+			mslog(s, NULL, LOG_ERR, "error reading the CRL (%s) file: %s",
+				s->config->crl, gnutls_strerror(ret));
+			exit(1);
+		}
+		mslog(s, NULL, LOG_INFO, "loaded CRL: %s", s->config->crl);
 	}
 }
 

src/tlslib.h

@@ -51,7 +51,8 @@ typedef struct tls_st {
 void tls_reload_crl(struct main_server_st* s, struct tls_st *creds, unsigned force);
 void tls_global_init(struct tls_st *creds);
 void tls_global_deinit(struct tls_st *creds);
-void tls_load_certs(struct main_server_st* s, struct tls_st *creds);
+void tls_load_files(struct main_server_st* s, struct tls_st *creds);
+void tls_load_prio(struct main_server_st *s, tls_st *creds);
 
 size_t tls_get_overhead(gnutls_protocol_t, gnutls_cipher_algorithm_t, gnutls_mac_algorithm_t);
 
@@ -144,4 +145,7 @@ inline static void packet_deinit(void *p)
 ssize_t cstp_recv_packet(struct worker_st *ws, gnutls_datum_t *data, void **p);
 ssize_t dtls_recv_packet(struct worker_st *ws, gnutls_datum_t *data, void **p);
 
+/* Helper functions */
+unsigned need_file_reload(const char *file, time_t last_access);
+
 #endif