]> git.ipfire.org Git - thirdparty/asterisk.git/commitdiff
res_pjsip: allow TLS verification of wildcard cert-bearing servers
authorKevin Harwell <kharwell@sangoma.com>
Wed, 8 Jun 2022 23:32:33 +0000 (18:32 -0500)
committerKevin Harwell <kharwell@digium.com>
Thu, 30 Jun 2022 21:54:16 +0000 (16:54 -0500)
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
contrib/ast-db-manage/config/versions/58e440314c2a_allow_wildcard_certs.py [new file with mode: 0644]
doc/CHANGES-staging/allow_wildcard_certs.txt [new file with mode: 0644]
include/asterisk/res_pjsip.h
res/res_pjsip/config_transport.c
res/res_pjsip/pjsip_config.xml
res/res_pjsip/pjsip_transport_events.c

index a019708c3947ea9809294d0c0a0c38dd377598fe..9d73843030f2b27139d7b878137ecd1aebbc5b3b 100644 (file)
                         ; 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]
diff --git a/contrib/ast-db-manage/config/versions/58e440314c2a_allow_wildcard_certs.py b/contrib/ast-db-manage/config/versions/58e440314c2a_allow_wildcard_certs.py
new file mode 100644 (file)
index 0000000..2e56f73
--- /dev/null
@@ -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')
diff --git a/doc/CHANGES-staging/allow_wildcard_certs.txt b/doc/CHANGES-staging/allow_wildcard_certs.txt
new file mode 100644 (file)
index 0000000..29a53dd
--- /dev/null
@@ -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'.
index ba95c276f20489073626491cb655edec4a286836..b5b5a7256de229fdfe5371353f50ae8c5196eb19 100644 (file)
@@ -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) \
index 6319ecfafb1c259da73cb6e4fd41d2977b6ec2b5..38bbecb9cfe4e7131e0d3ce578251fcb0c77ed61 100644 (file)
@@ -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);
index 84736d6ce5008982aa07674deece01556272efc2..e6fca2e86b0437866451812dfc72f2ebb6803688 100644 (file)
                                                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>
index 4df1d5e6e6229987bf568044f74fa6549da7a59e..a816f4674c914e0d7ba34a630ec8d897a8f4fe00 100644 (file)
@@ -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) {