res_pjsip: allow TLS verification of wildcard cert-bearing servers

Rightly the use of wildcards in certificates is disallowed in accordance
with RFC5922. However, RFC2818 does make some allowances with regards to
their use when using subject alt names with DNS name types.

As such this patch creates a new setting for TLS transports called
'allow_wildcard_certs', which when it and 'verify_server' are both enabled
allows DNS name types, as well as the common name that start with '*.'
to match as a wildcard.

For instance: *.example.com
will match for: foo.example.com

Partial matching is not allowed, e.g. f*.example.com, foo.*.com, etc...
And the starting wildcard only matches for a single level.

For instance: *.example.com
will NOT match for: foo.bar.example.com

The new setting is disabled by default.

ASTERISK-30072 #close

Change-Id: If0be3fdab2e09c2a66bb54824fca406ebaac3da4

configs/samples/pjsip.conf.sample

@@ -1090,6 +1090,15 @@
                         ; URI is not a hostname, the saved transport will be
                         ; used and the 'x-ast-txp' parameter stripped from the
                         ; outgoing packet.
+;allow_wildcard_certs=no ; In conjunction with verify_server, if 'yes' allow use
+                         ; of wildcards, i.e. '*.' in certs for common, and
+                         ; subject alt names of type DNS for TLS transport
+                         ; types. Note, names must start with the wildcard.
+                         ; Partial wildcards, e.g. 'f*.example.com' and
+                         ; 'foo.*.com' are disallowed. As well, names only
+                         ; match against a single level meaning '*.example.com'
+                         ; matches 'foo.example.com', but not
+                         ; 'foo.bar.example.com'. Defaults to 'no'.
 
 ;==========================AOR SECTION OPTIONS=========================
 ;[aor]

contrib/ast-db-manage/config/versions/58e440314c2a_allow_wildcard_certs.py

@@ -0,0 +1,29 @@
+"""allow_wildcard_certs
+
+Revision ID: 58e440314c2a
+Revises: 18e0805d367f
+Create Date: 2022-05-12 12:15:55.343743
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '58e440314c2a'
+down_revision = '18e0805d367f'
+
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy.dialects.postgresql import ENUM
+
+YESNO_NAME = 'yesno_values'
+YESNO_VALUES = ['yes', 'no']
+
+def upgrade():
+    yesno_values = ENUM(*YESNO_VALUES, name=YESNO_NAME, create_type=False)
+
+    op.add_column('ps_transports', sa.Column('allow_wildcard_certs', type_=yesno_values))
+
+
+def downgrade():
+    if op.get_context().bind.dialect.name == 'mssql':
+        op.drop_constraint('ck_ps_transports_allow_wildcard_certs_yesno_values', 'ps_transports')
+    op.drop_column('ps_transports', 'allow_wildcard_certs')

doc/CHANGES-staging/allow_wildcard_certs.txt

@@ -0,0 +1,9 @@
+Subject: res_pjsip
+
+A new transport option 'allow_wildcard_certs' has been added that when it
+and 'verify_server' are both set to 'yes', enables verification against
+wildcards, i.e. '*.' in certs for common, and subject alt names of type DNS
+for TLS transport types. Names must start with the wildcard. Partial wildcards,
+e.g. 'f*.example.com' and 'foo.*.com' are not allowed. As well, names only
+match against a single level meaning '*.example.com' matches 'foo.example.com',
+but not 'foo.bar.example.com'.

include/asterisk/res_pjsip.h

@@ -173,6 +173,14 @@ struct ast_sip_transport_state {
 	 * \since 17.0.0
 	 */
 	struct ast_sip_service_route_vector *service_routes;
+	/*!
+	 * Disregard RFC5922 7.2, and allow wildcard certs (TLS only)
+	 */
+	int allow_wildcard_certs;
+	/*!
+	 * If true, fail if server certificate cannot verify (TLS only)
+	 */
+	int verify_server;
 };
 
 #define ast_sip_transport_is_nonlocal(transport_state, addr) \

res/res_pjsip/config_transport.c

@@ -839,6 +839,16 @@ static int transport_apply(const struct ast_sorcery *sorcery, void *obj)
 				&temp_state->state->host, NULL, transport->async_operations,
 				&temp_state->state->factory);
 		}
+
+		if (res == PJ_SUCCESS) {
+			temp_state->state->factory->info = pj_pool_alloc(
+				temp_state->state->factory->pool, (strlen(transport_id) + 1));
+			/*
+			 * Store transport id on the factory instance so it can be used
+			 * later to look up the transport state.
+			 */
+			sprintf(temp_state->state->factory->info, "%s", transport_id);
+		}
 #else
 		ast_log(LOG_ERROR, "Transport: %s: PJSIP has not been compiled with TLS transport support, ensure OpenSSL development packages are installed\n",
 			ast_sorcery_object_get_id(obj));
@@ -1063,11 +1073,13 @@ static int transport_tls_bool_handler(const struct aco_option *opt, struct ast_v
 	}
 
 	if (!strcasecmp(var->name, "verify_server")) {
-		state->tls.verify_server = ast_true(var->value) ? PJ_TRUE : PJ_FALSE;
+		state->verify_server = ast_true(var->value);
 	} else if (!strcasecmp(var->name, "verify_client")) {
 		state->tls.verify_client = ast_true(var->value) ? PJ_TRUE : PJ_FALSE;
 	} else if (!strcasecmp(var->name, "require_client_cert")) {
 		state->tls.require_client_cert = ast_true(var->value) ? PJ_TRUE : PJ_FALSE;
+	} else if (!strcasecmp(var->name, "allow_wildcard_certs")) {
+		state->allow_wildcard_certs = ast_true(var->value);
 	} else {
 		return -1;
 	}
@@ -1084,7 +1096,7 @@ static int verify_server_to_str(const void *obj, const intptr_t *args, char **bu
 		return -1;
 	}
 
-	*buf = ast_strdup(AST_YESNO(state->tls.verify_server));
+	*buf = ast_strdup(AST_YESNO(state->verify_server));
 
 	return 0;
 }
@@ -1117,6 +1129,20 @@ static int require_client_cert_to_str(const void *obj, const intptr_t *args, cha
 	return 0;
 }
 
+static int allow_wildcard_certs_to_str(const void *obj, const intptr_t *args, char **buf)
+{
+	struct ast_sip_transport_state *state = find_state_by_transport(obj);
+
+	if (!state) {
+		return -1;
+	}
+
+	*buf = ast_strdup(AST_YESNO(state->allow_wildcard_certs));
+	ao2_ref(state, -1);
+
+	return 0;
+}
+
 /*! \brief Custom handler for TLS method setting */
 static int transport_tls_method_handler(const struct aco_option *opt, struct ast_variable *var, void *obj)
 {
@@ -1659,6 +1685,7 @@ int ast_sip_initialize_sorcery_transport(void)
 	ast_sorcery_object_field_register_custom(sorcery, "transport", "verify_server", "", transport_tls_bool_handler, verify_server_to_str, NULL, 0, 0);
 	ast_sorcery_object_field_register_custom(sorcery, "transport", "verify_client", "", transport_tls_bool_handler, verify_client_to_str, NULL, 0, 0);
 	ast_sorcery_object_field_register_custom(sorcery, "transport", "require_client_cert", "", transport_tls_bool_handler, require_client_cert_to_str, NULL, 0, 0);
+	ast_sorcery_object_field_register_custom(sorcery, "transport", "allow_wildcard_certs", "", transport_tls_bool_handler, allow_wildcard_certs_to_str, NULL, 0, 0);
 	ast_sorcery_object_field_register_custom(sorcery, "transport", "method", "", transport_tls_method_handler, tls_method_to_str, NULL, 0, 0);
 #if defined(PJ_HAS_SSL_SOCK) && PJ_HAS_SSL_SOCK != 0
 	ast_sorcery_object_field_register_custom(sorcery, "transport", "cipher", "", transport_tls_cipher_handler, transport_tls_cipher_to_str, NULL, 0, 0);

res/res_pjsip/pjsip_config.xml

@@ -1763,6 +1763,18 @@
 						in-progress calls.</para>
 					</description>
 				</configOption>
+				<configOption name="allow_wildcard_certs" default="false">
+					<synopsis>Allow use of wildcards in certificates (TLS ONLY)</synopsis>
+					<description>
+					  <para>In combination with verify_server, when enabled allow use of wildcards,
+					  i.e. '*.' in certs for common,and subject alt names of type DNS for TLS
+					  transport types. Names must start with the wildcard. Partial wildcards, e.g.
+					  'f*.example.com' and 'foo.*.com' are not allowed. As well, names only match
+					  against a single level meaning '*.example.com' matches 'foo.example.com',
+					  but not 'foo.bar.example.com'.
+					  </para>
+					</description>
+				</configOption>
 				<configOption name="symmetric_transport" default="no">
 					<synopsis>Use the same transport for outgoing requests as incoming ones.</synopsis>
 					<description>

res/res_pjsip/pjsip_transport_events.c

@@ -142,6 +142,122 @@ static void transport_state_do_reg_callbacks(struct ao2_container *transports, p
 	}
 }
 
+static void verify_log_result(int log_level, const pjsip_transport *transport,
+	pj_uint32_t verify_status)
+{
+	const char *status[32];
+	unsigned int count;
+	unsigned int i;
+
+	count = ARRAY_LEN(status);
+
+	if (pj_ssl_cert_get_verify_status_strings(verify_status, status, &count) != PJ_SUCCESS) {
+		ast_log(LOG_ERROR, "Error retrieving certificate verification result(s)\n");
+		return;
+	}
+
+	for (i = 0; i < count; ++i) {
+		ast_log(log_level, _A_, "Transport '%s' to remote '%.*s' - %s\n", transport->factory->info,
+			(int)pj_strlen(&transport->remote_name.host), pj_strbuf(&transport->remote_name.host),
+			status[i]);
+	}
+}
+
+static int verify_cert_name(const pj_str_t *local, const pj_str_t *remote)
+{
+	const char *p;
+	pj_ssize_t size;
+
+	ast_debug(3, "Verify certificate name: local = %.*s, remote = %.*s\n",
+		(unsigned int)pj_strlen(local), pj_strbuf(local),
+		(unsigned int)pj_strlen(remote), pj_strbuf(remote));
+
+	if (!pj_stricmp(remote, local)) {
+		return 1;
+	}
+
+	if (pj_strnicmp2(remote, "*.", 2)) {
+		return 0;
+	}
+
+	p = pj_strchr(local, '.');
+	if (!p) {
+		return 0;
+	}
+
+	size = pj_strbuf(local) + pj_strlen(local) - ++p;
+
+	return size == pj_strlen(remote) - 2 ?
+		!pj_memcmp(pj_strbuf(remote) + 2,  p, size) : 0;
+}
+
+static int verify_cert_names(const pj_str_t *host, const pj_ssl_cert_info *remote)
+{
+	unsigned int i;
+
+	for (i = 0; i < remote->subj_alt_name.cnt; ++i) {
+		/*
+		 * DNS is the only type we're matching wildcards against,
+		 * so only recheck those.
+		 */
+		if (remote->subj_alt_name.entry[i].type == PJ_SSL_CERT_NAME_DNS
+			&& verify_cert_name(host, &remote->subj_alt_name.entry[i].name)) {
+			return 1;
+		}
+	}
+
+	return verify_cert_name(host, &remote->subject.cn);
+}
+
+static int transport_tls_verify(const pjsip_transport *transport,
+	const pjsip_tls_state_info *state_info)
+{
+	pj_uint32_t verify_status;
+	const struct ast_sip_transport_state *state;
+
+	if (transport->dir == PJSIP_TP_DIR_INCOMING) {
+		return 1;
+	}
+
+	/* transport_id should always be in factory info (see config_transport) */
+	ast_assert(!ast_strlen_zero(transport->factory->info));
+
+	state = ast_sip_get_transport_state(transport->factory->info);
+	if (!state) {
+		/*
+		 * There should always be an associated state, but if for some
+		 * reason there is not then fail verification
+		 */
+		ast_log(LOG_ERROR, "Transport state not found for '%s'\n", transport->factory->info);
+		return 0;
+	}
+
+	verify_status = state_info->ssl_sock_info->verify_status;
+
+	/*
+	 * By this point pjsip has already completed its verification process. If
+	 * there was a name matching error it could be because they disallow wildcards.
+	 * If this transport has been configured to allow wildcards then we'll need
+	 * to re-check the name(s) for such.
+	 */
+	if (state->allow_wildcard_certs &&
+			(verify_status & PJ_SSL_CERT_EIDENTITY_NOT_MATCH)) {
+		if (verify_cert_names(&transport->remote_name.host,
+			state_info->ssl_sock_info->remote_cert_info)) {
+			/* A name matched a wildcard, so clear the error */
+			verify_status &= ~PJ_SSL_CERT_EIDENTITY_NOT_MATCH;
+		}
+	}
+
+	if (state->verify_server && verify_status != PJ_SSL_CERT_ESUCCESS) {
+		verify_log_result(__LOG_ERROR, transport, verify_status);
+		return 0;
+	}
+
+	verify_log_result(__LOG_NOTICE, transport, verify_status);
+	return 1;
+}
+
 /*! \brief Callback invoked when transport state changes occur */
 static void transport_state_callback(pjsip_transport *transport,
 	pjsip_transport_state state, const pjsip_transport_state_info *info)
@@ -157,6 +273,12 @@ static void transport_state_callback(pjsip_transport *transport,
 			transport->obj_name, transport_state2str(state));
 		switch (state) {
 		case PJSIP_TP_STATE_CONNECTED:
+			if (PJSIP_TRANSPORT_IS_SECURE(transport) &&
+				!transport_tls_verify(transport, info->ext_info)) {
+				pjsip_transport_shutdown(transport);
+				return;
+			}
+
 			monitored = ao2_alloc_options(sizeof(*monitored),
 				transport_monitor_dtor, AO2_ALLOC_OPT_LOCK_NOLOCK);
 			if (!monitored) {