[Asterisk-code-review] tcptls: Use new certificate upon sip reload (asterisk[14])

Anonymous Coward asteriskteam at digium.com
Fri Dec 2 07:40:50 CST 2016


Anonymous Coward #1000019 has submitted this change and it was merged. ( https://gerrit.asterisk.org/4449 )

Change subject: tcptls: Use new certificate upon sip reload
......................................................................


tcptls: Use new certificate upon sip reload

Previously, a TLS server socket would only be restarted upon sip reload if the
bind address had changed. This commit adds checking for changes to TLS
parameters like certificate, ciphers, etc. so they get picked up without
requiring a reload of the entire chan_sip module. This does not affect open
connections in any way, but new connections will use the new TLS parameters.
The changes also apply to HTTP and Manager.

ASTERISK-26604 #close

Change-Id: I169e86cefc6dcd627c915134015a6a1ab1aadbe6
---
M include/asterisk/tcptls.h
M main/tcptls.c
2 files changed, 89 insertions(+), 1 deletion(-)

Approvals:
  Anonymous Coward #1000019: Verified
  Joshua Colp: Looks good to me, approved
  Corey Farrell: Looks good to me, but someone else must approve



diff --git a/include/asterisk/tcptls.h b/include/asterisk/tcptls.h
index 3c5f450..d19ec52 100644
--- a/include/asterisk/tcptls.h
+++ b/include/asterisk/tcptls.h
@@ -107,6 +107,9 @@
 	char *capath;
 	struct ast_flags flags;
 	SSL_CTX *ssl_ctx;
+	char certhash[41];
+	char pvthash[41];
+	char cahash[41];
 };
 
 /*! \page AstTlsOverview TLS Implementation Overview
@@ -151,6 +154,7 @@
 	void (*periodic_fn)(void *);/*!< something we may want to run before after select on the accept socket */
 	void *(*worker_fn)(void *); /*!< the function in charge of doing the actual work */
 	const char *name;
+	struct ast_tls_config *old_tls_cfg; /*!< copy of the SSL configuration to determine whether changes have been made */
 };
 
 struct ast_tcptls_stream;
diff --git a/main/tcptls.c b/main/tcptls.c
index 046501b..a570f6c 100644
--- a/main/tcptls.c
+++ b/main/tcptls.c
@@ -38,6 +38,7 @@
 #endif
 
 #include <signal.h>
+#include <sys/stat.h>
 
 #include "asterisk/compat.h"
 #include "asterisk/tcptls.h"
@@ -48,6 +49,7 @@
 #include "asterisk/manager.h"
 #include "asterisk/astobj2.h"
 #include "asterisk/pbx.h"
+#include "asterisk/app.h"
 
 /*! ao2 object used for the FILE stream fopencookie()/funopen() cookie. */
 struct ast_tcptls_stream {
@@ -1104,9 +1106,64 @@
 {
 	int flags;
 	int x = 1;
+	int tls_changed = 0;
+
+	if (desc->tls_cfg) {
+		char hash[41];
+		char *str = NULL;
+		struct stat st;
+
+		/* Store the hashes of the TLS certificate etc. */
+		if (stat(desc->tls_cfg->certfile, &st) || NULL == (str = ast_read_textfile(desc->tls_cfg->certfile))) {
+			memset(hash, 0, 41);
+		} else {
+			ast_sha1_hash(hash, str);
+		}
+		ast_free(str);
+		str = NULL;
+		memcpy(desc->tls_cfg->certhash, hash, 41);
+		if (stat(desc->tls_cfg->pvtfile, &st) || NULL == (str = ast_read_textfile(desc->tls_cfg->pvtfile))) {
+			memset(hash, 0, 41);
+		} else {
+			ast_sha1_hash(hash, str);
+		}
+		ast_free(str);
+		str = NULL;
+		memcpy(desc->tls_cfg->pvthash, hash, 41);
+		if (stat(desc->tls_cfg->cafile, &st) || NULL == (str = ast_read_textfile(desc->tls_cfg->cafile))) {
+			memset(hash, 0, 41);
+		} else {
+			ast_sha1_hash(hash, str);
+		}
+		ast_free(str);
+		str = NULL;
+		memcpy(desc->tls_cfg->cahash, hash, 41);
+
+		/* Check whether TLS configuration has changed */
+		if (!desc->old_tls_cfg) { /* No previous configuration */
+			tls_changed = 1;
+			desc->old_tls_cfg = ast_calloc(1, sizeof(*desc->old_tls_cfg));
+		} else if (memcmp(desc->tls_cfg->certhash, desc->old_tls_cfg->certhash, 41)) {
+			tls_changed = 1;
+		} else if (memcmp(desc->tls_cfg->pvthash, desc->old_tls_cfg->pvthash, 41)) {
+			tls_changed = 1;
+		} else if (strcmp(desc->tls_cfg->cipher, desc->old_tls_cfg->cipher)) {
+			tls_changed = 1;
+		} else if (memcmp(desc->tls_cfg->cahash, desc->old_tls_cfg->cahash, 41)) {
+			tls_changed = 1;
+		} else if (strcmp(desc->tls_cfg->capath, desc->old_tls_cfg->capath)) {
+			tls_changed = 1;
+		} else if (memcmp(&desc->tls_cfg->flags, &desc->old_tls_cfg->flags, sizeof(desc->tls_cfg->flags))) {
+			tls_changed = 1;
+		}
+
+		if (tls_changed) {
+			ast_debug(1, "Changed parameters for %s found\n", desc->name);
+		}
+	}
 
 	/* Do nothing if nothing has changed */
-	if (!ast_sockaddr_cmp(&desc->old_address, &desc->local_address)) {
+	if (!tls_changed && !ast_sockaddr_cmp(&desc->old_address, &desc->local_address)) {
 		ast_debug(1, "Nothing changed in %s\n", desc->name);
 		return;
 	}
@@ -1162,6 +1219,22 @@
 
 	/* Set current info */
 	ast_sockaddr_copy(&desc->old_address, &desc->local_address);
+	if (desc->old_tls_cfg) {
+		ast_free(desc->old_tls_cfg->certfile);
+		ast_free(desc->old_tls_cfg->pvtfile);
+		ast_free(desc->old_tls_cfg->cipher);
+		ast_free(desc->old_tls_cfg->cafile);
+		ast_free(desc->old_tls_cfg->capath);
+		desc->old_tls_cfg->certfile = ast_strdup(desc->tls_cfg->certfile);
+		desc->old_tls_cfg->pvtfile = ast_strdup(desc->tls_cfg->pvtfile);
+		desc->old_tls_cfg->cipher = ast_strdup(desc->tls_cfg->cipher);
+		desc->old_tls_cfg->cafile = ast_strdup(desc->tls_cfg->cafile);
+		desc->old_tls_cfg->capath = ast_strdup(desc->tls_cfg->capath);
+		memcpy(desc->old_tls_cfg->certhash, desc->tls_cfg->certhash, 41);
+		memcpy(desc->old_tls_cfg->pvthash, desc->tls_cfg->pvthash, 41);
+		memcpy(desc->old_tls_cfg->cahash, desc->tls_cfg->cahash, 41);
+		memcpy(&desc->old_tls_cfg->flags, &desc->tls_cfg->flags, sizeof(desc->old_tls_cfg->flags));
+	}
 
 	return;
 
@@ -1207,6 +1280,17 @@
 		close(desc->accept_fd);
 	}
 	desc->accept_fd = -1;
+
+	if (desc->old_tls_cfg) {
+		ast_free(desc->old_tls_cfg->certfile);
+		ast_free(desc->old_tls_cfg->pvtfile);
+		ast_free(desc->old_tls_cfg->cipher);
+		ast_free(desc->old_tls_cfg->cafile);
+		ast_free(desc->old_tls_cfg->capath);
+		ast_free(desc->old_tls_cfg);
+		desc->old_tls_cfg = NULL;
+	}
+
 	ast_debug(2, "Stopped server :: %s\n", desc->name);
 }
 

-- 
To view, visit https://gerrit.asterisk.org/4449
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I169e86cefc6dcd627c915134015a6a1ab1aadbe6
Gerrit-PatchSet: 6
Gerrit-Project: asterisk
Gerrit-Branch: 14
Gerrit-Owner: Michael Kuron <m.kuron at gmx.de>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>



More information about the asterisk-code-review mailing list