From 26b5c29dab079dc0a10713771453e89f5ec00611 Mon Sep 17 00:00:00 2001 From: James Stanley Date: Tue, 30 May 2023 11:03:44 +0100 Subject: [PATCH 1/8] Add hostname field to sockaddr_union --- ip_addr.h | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/ip_addr.h b/ip_addr.h index de8028b87ca..6954858f41c 100644 --- a/ip_addr.h +++ b/ip_addr.h @@ -80,10 +80,19 @@ struct net{ struct ip_addr mask; }; +union sockaddr_union_no_hostname{ + struct sockaddr s; + struct sockaddr_in sin; + struct sockaddr_in6 sin6; +}; union sockaddr_union{ - struct sockaddr s; - struct sockaddr_in sin; - struct sockaddr_in6 sin6; + struct sockaddr s; + struct sockaddr_in sin; + struct sockaddr_in6 sin6; + struct { + union sockaddr_union_no_hostname _padding; + char hostname[256]; + } h; }; From 50ff9bf8b19a7590de00bbc5804b5ac7f516971a Mon Sep 17 00:00:00 2001 From: James Stanley Date: Tue, 30 May 2023 11:06:30 +0100 Subject: [PATCH 2/8] hostent2su: copy hostnames into sockaddr_union_struct --- ip_addr.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ip_addr.h b/ip_addr.h index 6954858f41c..74a4e79e397 100644 --- a/ip_addr.h +++ b/ip_addr.h @@ -374,6 +374,11 @@ static inline int hostent2su( union sockaddr_union* su, unsigned short port ) { memset(su, 0, sizeof(union sockaddr_union)); /*needed on freebsd*/ + + /* copy the hostname into the sockaddr_union */ + strncpy(su->h.hostname, he->h_name, sizeof(su->h.hostname)-1); + su->h.hostname[sizeof(su->h.hostname)-1] = 0; + su->s.sa_family=he->h_addrtype; switch(he->h_addrtype){ case AF_INET6: From 080fce6ca5819d67417affc89642216f6fde599a Mon Sep 17 00:00:00 2001 From: James Stanley Date: Fri, 5 May 2023 12:05:25 +0100 Subject: [PATCH 3/8] Copy peer hostname from sockaddr_union into tcp_connection This is so that hostnames in TLS certificates can be validated. --- net/net_tcp.c | 5 +++++ net/tcp_conn_defs.h | 1 + 2 files changed, 6 insertions(+) diff --git a/net/net_tcp.c b/net/net_tcp.c index da2dcb42880..e25da53f419 100644 --- a/net/net_tcp.c +++ b/net/net_tcp.c @@ -958,6 +958,11 @@ struct tcp_connection* tcp_conn_create(int sock, union sockaddr_union* su, return NULL; } + /* copy peer hostname into the tcp_connection so that tls_openssl can verify + * the certificate hostname */ + strncpy(c->hostname, su->h.hostname, sizeof(c->hostname)-1); + c->hostname[sizeof(c->hostname)-1] = 0; + if (protos[c->type].net.conn_init && protos[c->type].net.conn_init(c) < 0) { LM_ERR("failed to do proto %d specific init for conn %p\n", diff --git a/net/tcp_conn_defs.h b/net/tcp_conn_defs.h index d2216d65892..9632dac90ef 100644 --- a/net/tcp_conn_defs.h +++ b/net/tcp_conn_defs.h @@ -182,6 +182,7 @@ struct tcp_connection{ struct tcp_async_data *async; /* protocol specific data attached to this connection */ void *proto_data; + char hostname[256]; /* remote side hostname (used for TLS certificate hostname verification) */ }; From 78af5919a8deaa390a8e004c2eb2b4fd8e315d1d Mon Sep 17 00:00:00 2001 From: James Stanley Date: Fri, 5 May 2023 12:05:26 +0100 Subject: [PATCH 4/8] tls_openssl: enable certificate hostname validation Based on example from https://wiki.openssl.org/index.php/Hostname_validation --- modules/tls_openssl/openssl_conn_ops.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/modules/tls_openssl/openssl_conn_ops.c b/modules/tls_openssl/openssl_conn_ops.c index 7f30b01b46a..3eb9a40e4f0 100644 --- a/modules/tls_openssl/openssl_conn_ops.c +++ b/modules/tls_openssl/openssl_conn_ops.c @@ -25,6 +25,7 @@ #include #include #include +#include #include #include @@ -199,6 +200,8 @@ int openssl_tls_update_fd(struct tcp_connection *c, int fd) int openssl_tls_conn_init(struct tcp_connection* c, struct tls_domain *tls_dom) { + X509_VERIFY_PARAM *param = NULL; + /* * new connection within a single process, no lock necessary */ @@ -218,6 +221,13 @@ int openssl_tls_conn_init(struct tcp_connection* c, struct tls_domain *tls_dom) return -1; } + param = SSL_get0_param(c->extra_data); + X509_VERIFY_PARAM_set_hostflags(param, X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS); + if (!X509_VERIFY_PARAM_set1_host(param, c->hostname, strlen(c->hostname))) { + LM_ERR("failed to set hostname for SSL context\n"); + return -1; + } + /* put pointers to the tcp_connection and tls_domain structs * in the SSL struct as extra data */ if (!SSL_set_ex_data(c->extra_data, SSL_EX_CONN_IDX, c)) { From e201c1445fe4a428cf5ddb13e2062e348e80f578 Mon Sep 17 00:00:00 2001 From: James Stanley Date: Fri, 5 May 2023 12:05:26 +0100 Subject: [PATCH 5/8] SRV lookups: take hostname from original lookup instead of SRV record This is so that TLS certificate validation looks at the correct hostname instead of the one in the SRV record. --- resolve.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/resolve.c b/resolve.c index 4d3b084145e..6f3abd7b2e8 100644 --- a/resolve.c +++ b/resolve.c @@ -1881,8 +1881,18 @@ struct hostent* sip_resolvehost( str* name, unsigned short* port, } he = do_srv_lookup( tmp, port, dn); - if (he) + if (he) { + /* we need to check TLS certificates against the original requested + * hostname, not the name from the SRV record */ + if (name->len >= MAX_DNS_NAME) { + LM_ERR("domain name too long\n"); + return 0; + } + memcpy(tmp, name->s, name->len); + tmp[name->len] = '\0'; + he->h_name = tmp; return he; + } LM_DBG("no valid SRV record found for %s, trying A record lookup...\n", tmp); From dc9862f93277702e942173aacd4bee7b8f6a3ae2 Mon Sep 17 00:00:00 2001 From: James Stanley Date: Fri, 5 May 2023 13:25:46 +0100 Subject: [PATCH 6/8] Gate TLS hostname verification behind 'verify_hostname' config param --- modules/tls_mgm/tls_config.c | 3 +++ modules/tls_mgm/tls_config.h | 4 +++- modules/tls_mgm/tls_domain.c | 6 ++++++ modules/tls_mgm/tls_domain.h | 5 +++-- modules/tls_mgm/tls_helper.h | 1 + modules/tls_mgm/tls_mgm.c | 12 +++++++++++- modules/tls_mgm/tls_params.c | 19 +++++++++++++++++++ modules/tls_mgm/tls_params.h | 2 ++ modules/tls_openssl/openssl_conn_ops.c | 12 +++++++----- 9 files changed, 55 insertions(+), 9 deletions(-) diff --git a/modules/tls_mgm/tls_config.c b/modules/tls_mgm/tls_config.c index 02c0b23fd55..fecc4495a92 100644 --- a/modules/tls_mgm/tls_config.c +++ b/modules/tls_mgm/tls_config.c @@ -49,6 +49,8 @@ int tls_default_method = TLS_USE_SSLv23; int tls_verify_client_cert = 1; int tls_verify_server_cert = 1; int tls_require_client_cert = 1; +/* disable hostname verification by default */ +int tls_verify_hostname = 0; /* disable CRL validation for all the certificates from the chain */ int crl_check_all = 0; /* default location of certificates */ @@ -70,6 +72,7 @@ str match_address_col = str_init("match_ip_address"); str match_domain_col = str_init("match_sip_domain"); str method_col = str_init("method"); str verify_cert_col = str_init("verify_cert"); +str verify_hostname_col = str_init("verify_hostname"); str require_cert_col = str_init("require_cert"); str certificate_col = str_init("certificate"); str pk_col = str_init("private_key"); diff --git a/modules/tls_mgm/tls_config.h b/modules/tls_mgm/tls_config.h index f10aae40cbf..58aa95a3419 100644 --- a/modules/tls_mgm/tls_config.h +++ b/modules/tls_mgm/tls_config.h @@ -38,13 +38,14 @@ #include "tls_config_helper.h" #include "../../str.h" -#define TLS_TABLE_VERSION 3 +#define TLS_TABLE_VERSION 4 extern int tls_default_method; extern int tls_verify_client_cert; extern int tls_verify_server_cert; extern int tls_require_client_cert; +extern int tls_verify_hostname; extern int crl_check_all; extern char *tls_cert_file; extern char *tls_pkey_file; @@ -63,6 +64,7 @@ extern str match_domain_col; extern str method_col; extern str verify_cert_col; extern str require_cert_col; +extern str verify_hostname_col; extern str certificate_col; extern str pk_col; extern str crl_check_col; diff --git a/modules/tls_mgm/tls_domain.c b/modules/tls_mgm/tls_domain.c index af8cf979b17..6f4fd098af0 100644 --- a/modules/tls_mgm/tls_domain.c +++ b/modules/tls_mgm/tls_domain.c @@ -243,6 +243,10 @@ int set_all_domain_attr(struct tls_domain **dom, char **str_vals, int *int_vals, d->require_client_cert = int_vals[INT_VALS_REQUIRE_CERT_COL]; } + if (int_vals[INT_VALS_VERIFY_HOSTNAME_COL] != -1) { + d->verify_hostname = int_vals[INT_VALS_VERIFY_HOSTNAME_COL]; + } + p = (char *) (d + 1); d->name.s = p; @@ -537,9 +541,11 @@ int tls_new_domain(str *name, int type, struct tls_domain **dom) if (type == DOM_FLAG_SRV) { d->verify_cert = tls_verify_client_cert; d->require_client_cert = tls_require_client_cert; + d->verify_hostname = 0; } else { d->verify_cert = tls_verify_server_cert; d->require_client_cert = 0; + d->verify_hostname = tls_verify_hostname; } d->method = TLS_METHOD_UNSPEC; diff --git a/modules/tls_mgm/tls_domain.h b/modules/tls_mgm/tls_domain.h index ae2067f7966..8a69315b529 100644 --- a/modules/tls_mgm/tls_domain.h +++ b/modules/tls_mgm/tls_domain.h @@ -59,13 +59,14 @@ #define STR_VALS_CPLIST_COL 6 #define STR_VALS_ECCURVE_COL 7 -#define NO_INT_VALS 5 +#define NO_INT_VALS 6 #define INT_VALS_ID_COL 0 #define INT_VALS_TYPE_COL 1 #define INT_VALS_VERIFY_CERT_COL 2 #define INT_VALS_REQUIRE_CERT_COL 3 #define INT_VALS_CRL_CHECK_COL 4 +#define INT_VALS_VERIFY_HOSTNAME_COL 5 #define NO_BLOB_VALS 4 @@ -74,7 +75,7 @@ #define BLOB_VALS_CALIST_COL 2 #define BLOB_VALS_DHPARAMS_COL 3 -#define NO_DB_COLS 17 +#define NO_DB_COLS 18 #define CLIENT_DOMAIN_TYPE 1 #define SERVER_DOMAIN_TYPE 2 diff --git a/modules/tls_mgm/tls_helper.h b/modules/tls_mgm/tls_helper.h index adfecd557d1..408cc08621c 100644 --- a/modules/tls_mgm/tls_helper.h +++ b/modules/tls_mgm/tls_helper.h @@ -82,6 +82,7 @@ struct tls_domain { void *ctx; /* openssl's SSL_CTX or wolfSSL's WOLFSSL_CTX */ int ctx_no; /* number of allocated contexts */ int verify_cert; + int verify_hostname; int require_client_cert; int crl_check_all; str cert; diff --git a/modules/tls_mgm/tls_mgm.c b/modules/tls_mgm/tls_mgm.c index 6b0ba092642..37851ad9218 100644 --- a/modules/tls_mgm/tls_mgm.c +++ b/modules/tls_mgm/tls_mgm.c @@ -124,6 +124,7 @@ static const param_export_t params[] = { { "tls_method", STR_PARAM|USE_FUNC_PARAM, (void*)tlsp_set_method }, { "verify_cert", STR_PARAM|USE_FUNC_PARAM, (void*)tlsp_set_verify }, { "require_cert", STR_PARAM|USE_FUNC_PARAM, (void*)tlsp_set_require }, + { "verify_hostname", STR_PARAM|USE_FUNC_PARAM, (void*)tlsp_set_verify_hostname }, { "certificate", STR_PARAM|USE_FUNC_PARAM, (void*)tlsp_set_certificate}, { "private_key", STR_PARAM|USE_FUNC_PARAM, (void*)tlsp_set_pk }, { "crl_check_all", STR_PARAM|USE_FUNC_PARAM, (void*)tlsp_set_crl_check }, @@ -141,6 +142,7 @@ static const param_export_t params[] = { { "tls_method_col", STR_PARAM, &method_col.s }, { "verify_cert_col", STR_PARAM, &verify_cert_col.s }, { "require_cert_col", STR_PARAM, &require_cert_col.s }, + { "verify_hostname_col", STR_PARAM, &verify_hostname_col.s }, { "certificate_col", STR_PARAM, &certificate_col.s }, { "private_key_col", STR_PARAM, &pk_col.s }, { "crl_check_all_col", STR_PARAM, &crl_check_col.s }, @@ -417,6 +419,7 @@ int load_info(struct tls_domain **serv_dom, struct tls_domain **cli_dom, columns[14] = &cplist_col; columns[15] = &dhparams_col; columns[16] = &eccurve_col; + columns[17] = &verify_hostname_col; /* checking if the table version is up to date*/ if (db_check_table_version(&dr_dbf, db_hdl, &tls_db_table, TLS_TABLE_VERSION) != 0) @@ -435,7 +438,7 @@ int load_info(struct tls_domain **serv_dom, struct tls_domain **cli_dom, goto error; } no_rows = estimate_available_rows(4 + 45 + 4 + 45 + 4 + 4 + 45 + - 45 + 4 + 45 + 45 + 4 * 4096, db_cols); + 45 + 4 + 45 + 45 + 4 * 4096 + 4, db_cols); if (no_rows == 0) no_rows = 5; if (dr_dbf.fetch_result(db_hdl, &res, no_rows) < 0) { LM_ERR("Error fetching rows\n"); @@ -517,6 +520,9 @@ int load_info(struct tls_domain **serv_dom, struct tls_domain **cli_dom, check_val(eccurve_col, ROW_VALUES(row) + 16, DB_STRING, 0, 0); str_vals[STR_VALS_ECCURVE_COL] = (char *) VAL_STRING(ROW_VALUES(row) + 16); + check_val(verify_hostname_col, ROW_VALUES(row) + 17, DB_INT, 0, 0); + int_vals[INT_VALS_VERIFY_HOSTNAME_COL] = VAL_INT(ROW_VALUES(row) + 17); + if (db_add_domain(str_vals, int_vals, blob_vals, serv_dom, cli_dom, script_srv_doms, script_cli_doms) < 0) { if (str_vals[STR_VALS_DOMAIN_COL]) @@ -994,6 +1000,7 @@ static int mod_init(void) { method_col.len = strlen(method_col.s); verify_cert_col.len = strlen(verify_cert_col.s); require_cert_col.len = strlen(require_cert_col.s); + verify_hostname_col.len = strlen(verify_hostname_col.s); certificate_col.len = strlen(certificate_col.s); pk_col.len = strlen(pk_col.s); crl_check_col.len = strlen(crl_check_col.s); @@ -1253,6 +1260,9 @@ static int list_domain(mi_item_t *domains_arr, struct tls_domain *d) if (add_mi_bool(domain_item, MI_SSTR("REQ_CLI_CERT"), d->require_client_cert) < 0) goto error; + if (add_mi_bool(domain_item, MI_SSTR("VERIFY_HOSTNAME"), d->verify_hostname) < 0) + goto error; + if (add_mi_bool(domain_item, MI_SSTR("CRL_CHECKALL"), d->crl_check_all) < 0) goto error; diff --git a/modules/tls_mgm/tls_params.c b/modules/tls_mgm/tls_params.c index bb1cb9f8a67..d468f234e8f 100644 --- a/modules/tls_mgm/tls_params.c +++ b/modules/tls_mgm/tls_params.c @@ -262,6 +262,25 @@ int tlsp_set_require(modparam_t type, void *in) return 1; } +int tlsp_set_verify_hostname(modparam_t type, void *in) +{ + str name; + str val; + unsigned int verify; + + if (split_param_val((char*)in, &name, &val) < 0) + return -1; + + if (str2int(&val, &verify)!=0) { + LM_ERR("option is not a number [%s]\n",val.s); + return -1; + } + + set_domain_attr(name, verify_hostname, verify); + + return 1; +} + int tlsp_set_crl_check(modparam_t type, void *in) { str name; diff --git a/modules/tls_mgm/tls_params.h b/modules/tls_mgm/tls_params.h index b60df2574bd..fd798d17c55 100644 --- a/modules/tls_mgm/tls_params.h +++ b/modules/tls_mgm/tls_params.h @@ -57,6 +57,8 @@ int tlsp_set_verify(modparam_t type, void *val); int tlsp_set_require(modparam_t type, void *val); +int tlsp_set_verify_hostname(modparam_t type, void *val); + int tlsp_set_crl_check(modparam_t type, void *val); int tlsp_set_certificate(modparam_t type, void *val); diff --git a/modules/tls_openssl/openssl_conn_ops.c b/modules/tls_openssl/openssl_conn_ops.c index 3eb9a40e4f0..b1672ab10f2 100644 --- a/modules/tls_openssl/openssl_conn_ops.c +++ b/modules/tls_openssl/openssl_conn_ops.c @@ -221,11 +221,13 @@ int openssl_tls_conn_init(struct tcp_connection* c, struct tls_domain *tls_dom) return -1; } - param = SSL_get0_param(c->extra_data); - X509_VERIFY_PARAM_set_hostflags(param, X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS); - if (!X509_VERIFY_PARAM_set1_host(param, c->hostname, strlen(c->hostname))) { - LM_ERR("failed to set hostname for SSL context\n"); - return -1; + if (tls_dom->verify_hostname) { + param = SSL_get0_param(c->extra_data); + X509_VERIFY_PARAM_set_hostflags(param, X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS); + if (!X509_VERIFY_PARAM_set1_host(param, c->hostname, strlen(c->hostname))) { + LM_ERR("failed to set hostname for SSL context\n"); + return -1; + } } /* put pointers to the tcp_connection and tls_domain structs From 2efbf6439f88012112b8aa51089c9db79b6ceb97 Mon Sep 17 00:00:00 2001 From: James Stanley Date: Mon, 10 Jul 2023 10:23:06 +0100 Subject: [PATCH 7/8] hostent_shm_cpy: copy h_name field --- proxy.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/proxy.c b/proxy.c index 6cb8a821479..ca8d66a4f7b 100644 --- a/proxy.c +++ b/proxy.c @@ -57,15 +57,23 @@ int disable_dns_failover=0; int hostent_shm_cpy(struct hostent *dst, struct hostent* src) { + unsigned int len; int i; char *p; + len = strlen(src->h_name)+1; + dst->h_name = (char*)shm_malloc(sizeof(char) * len); + if (dst->h_name) strncpy(dst->h_name, src->h_name, len); + else return -1; + for( i=0 ; src->h_addr_list[i] ; i++ ); dst->h_addr_list = (char**)shm_malloc (i * (src->h_length + sizeof(char*)) + sizeof(char*)); - if (dst->h_addr_list==NULL) + if (dst->h_addr_list==NULL) { + shm_free(dst->h_name); return -1; + } p = ((char*)dst->h_addr_list) + (i+1)*sizeof(char*); dst->h_addr_list[i] = 0; @@ -85,6 +93,8 @@ int hostent_shm_cpy(struct hostent *dst, struct hostent* src) void free_shm_hostent(struct hostent *dst) { + if (dst->h_name) + shm_free(dst->h_name); if (dst->h_addr_list) shm_free(dst->h_addr_list); } From 0eccdbfc45a9aae3f008f332c279a3e3c9c52f49 Mon Sep 17 00:00:00 2001 From: James Stanley Date: Fri, 1 Dec 2023 20:34:55 +0000 Subject: [PATCH 8/8] tm: free allocated h_name in hostent --- modules/tm/h_table.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/modules/tm/h_table.c b/modules/tm/h_table.c index 63cb218dc42..f9eb1cd8c58 100644 --- a/modules/tm/h_table.c +++ b/modules/tm/h_table.c @@ -153,6 +153,8 @@ void free_cell( struct cell* dead_cell ) free_cloned_msg_unsafe( rpl ); } if ( (p=dead_cell->uac[i].proxy)!=NULL ) { + if ( p->host.h_name ) + shm_free_bulk( p->host.h_name ); if ( p->host.h_addr_list ) shm_free_bulk( p->host.h_addr_list ); if ( p->dn ) {