[asterisk-commits] Add X.509 subject alternative name support to TLS certificat... (asterisk[13])

[SVN commits to the Asterisk project]

Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/4451 )

Change subject: Add X.509 subject alternative name support to TLS certificate verification.
......................................................................


Add X.509 subject alternative name support to TLS certificate
verification.

This way one X.509 certificate can be used for hosts that
can be reached under multiple DNS names or for multiple hosts.

Signed-off-by: Maciej Szmigiero <mail at maciej.szmigiero.name>

ASTERISK-25063 #close

Change-Id: I13302c80490a0b44c43f1b45376c9bd7b15a538f
---
M CHANGES
M include/asterisk/tcptls.h
M main/tcptls.c
3 files changed, 62 insertions(+), 16 deletions(-)

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



diff --git a/CHANGES b/CHANGES
index 463c884..612711e 100644
--- a/CHANGES
+++ b/CHANGES
@@ -54,6 +54,12 @@
  * A new dialplan variable, ABANDONED, is set when the call is not answered
    by an agent.
 
+Core
+------------------
+ * The TLS core in Asterisk now supports X.509 certificate subject alternative
+   names. This way one X.509 certificate can be used for hosts that can be
+   reached under multiple DNS names or for multiple hosts.
+
 ------------------------------------------------------------------------------
 --- Functionality changes from Asterisk 13.11.0 to Asterisk 13.12.0 ----------
 ------------------------------------------------------------------------------
diff --git a/include/asterisk/tcptls.h b/include/asterisk/tcptls.h
index e1a632c..3c5f450 100644
--- a/include/asterisk/tcptls.h
+++ b/include/asterisk/tcptls.h
@@ -65,6 +65,7 @@
 #ifdef DO_SSL
 #include <openssl/ssl.h>
 #include <openssl/err.h>
+#include <openssl/x509v3.h>
 #else
 /* declare dummy types so we can define a pointer to them */
 typedef struct {} SSL;
diff --git a/main/tcptls.c b/main/tcptls.c
index 34baf9a..bccb03d 100644
--- a/main/tcptls.c
+++ b/main/tcptls.c
@@ -555,6 +555,34 @@
 	ao2_cleanup(i->private_data);
 }
 
+#ifdef DO_SSL
+static int check_tcptls_cert_name(ASN1_STRING *cert_str, const char *hostname, const char *desc)
+{
+	unsigned char *str;
+	int ret;
+
+	ret = ASN1_STRING_to_UTF8(&str, cert_str);
+	if (ret < 0 || !str) {
+		return -1;
+	}
+
+	if (strlen((char *) str) != ret) {
+		ast_log(LOG_WARNING, "Invalid certificate %s length (contains NULL bytes?)\n", desc);
+
+		ret = -1;
+	} else if (!strcasecmp(hostname, (char *) str)) {
+		ret = 0;
+	} else {
+		ret = -1;
+	}
+
+	ast_debug(3, "SSL %s compare s1='%s' s2='%s'\n", desc, hostname, str);
+	OPENSSL_free(str);
+
+	return ret;
+}
+#endif
+
 /*! \brief
 * creates a FILE * from the fd passed by the accept thread.
 * This operation is potentially expensive (certificate verification),
@@ -631,8 +659,8 @@
 				}
 				if (!ast_test_flag(&tcptls_session->parent->tls_cfg->flags, AST_SSL_IGNORE_COMMON_NAME)) {
 					ASN1_STRING *str;
-					unsigned char *str2;
 					X509_NAME *name = X509_get_subject_name(peer);
+					STACK_OF(GENERAL_NAME) *alt_names;
 					int pos = -1;
 					int found = 0;
 
@@ -643,25 +671,36 @@
 						if (pos < 0) {
 							break;
 						}
-						str = X509_NAME_ENTRY_get_data(X509_NAME_get_entry(name, pos));
-						ret = ASN1_STRING_to_UTF8(&str2, str);
-						if (ret < 0) {
-							continue;
-						}
 
-						if (str2) {
-							if (strlen((char *) str2) != ret) {
-								ast_log(LOG_WARNING, "Invalid certificate common name length (contains NULL bytes?)\n");
-							} else if (!strcasecmp(tcptls_session->parent->hostname, (char *) str2)) {
-								found = 1;
-							}
-							ast_debug(3, "SSL Common Name compare s1='%s' s2='%s'\n", tcptls_session->parent->hostname, str2);
-							OPENSSL_free(str2);
-						}
-						if (found) {
+						str = X509_NAME_ENTRY_get_data(X509_NAME_get_entry(name, pos));
+						if (!check_tcptls_cert_name(str, tcptls_session->parent->hostname, "common name")) {
+							found = 1;
 							break;
 						}
 					}
+
+					if (!found) {
+						alt_names = X509_get_ext_d2i(peer, NID_subject_alt_name, NULL, NULL);
+						if (alt_names != NULL) {
+							int alt_names_count = sk_GENERAL_NAME_num(alt_names);
+
+							for (pos = 0; pos < alt_names_count; pos++) {
+								const GENERAL_NAME *alt_name = sk_GENERAL_NAME_value(alt_names, pos);
+
+								if (alt_name->type != GEN_DNS) {
+									continue;
+								}
+
+								if (!check_tcptls_cert_name(alt_name->d.dNSName, tcptls_session->parent->hostname, "alt name")) {
+									found = 1;
+									break;
+								}
+							}
+
+							sk_GENERAL_NAME_pop_free(alt_names, GENERAL_NAME_free);
+						}
+					}
+
 					if (!found) {
 						ast_log(LOG_ERROR, "Certificate common name did not match (%s)\n", tcptls_session->parent->hostname);
 						X509_free(peer);

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I13302c80490a0b44c43f1b45376c9bd7b15a538f
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Michael Kuron <m.kuron at gmx.de>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Maciej Szmigiero <mail at maciej.szmigiero.name>
Gerrit-Reviewer: Michael Kuron <m.kuron at gmx.de>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>