Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optional verification of hostnames in TLS certificates #3078

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
20 changes: 17 additions & 3 deletions ip_addr.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};


Expand Down Expand Up @@ -365,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:
Expand Down
3 changes: 3 additions & 0 deletions modules/tls_mgm/tls_config.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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");
Expand Down
4 changes: 3 additions & 1 deletion modules/tls_mgm/tls_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down
6 changes: 6 additions & 0 deletions modules/tls_mgm/tls_domain.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down
5 changes: 3 additions & 2 deletions modules/tls_mgm/tls_domain.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions modules/tls_mgm/tls_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
12 changes: 11 additions & 1 deletion modules/tls_mgm/tls_mgm.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
Expand All @@ -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 },
Expand Down Expand Up @@ -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)
Expand All @@ -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");
Expand Down Expand Up @@ -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])
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;

Expand Down
19 changes: 19 additions & 0 deletions modules/tls_mgm/tls_params.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions modules/tls_mgm/tls_params.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
12 changes: 12 additions & 0 deletions modules/tls_openssl/openssl_conn_ops.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include <openssl/opensslv.h>
#include <openssl/err.h>
#include <openssl/rand.h>
#include <openssl/x509v3.h>

#include <poll.h>
#include <errno.h>
Expand Down Expand Up @@ -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
*/
Expand All @@ -218,6 +221,15 @@ int openssl_tls_conn_init(struct tcp_connection* c, struct tls_domain *tls_dom)
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
* in the SSL struct as extra data */
if (!SSL_set_ex_data(c->extra_data, SSL_EX_CONN_IDX, c)) {
Expand Down
2 changes: 2 additions & 0 deletions modules/tm/h_table.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 ) {
Expand Down
5 changes: 5 additions & 0 deletions net/net_tcp.c
Original file line number Diff line number Diff line change
Expand Up @@ -961,6 +961,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",
Expand Down
1 change: 1 addition & 0 deletions net/tcp_conn_defs.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) */
};


Expand Down
12 changes: 11 additions & 1 deletion proxy.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}
Expand Down
12 changes: 11 additions & 1 deletion resolve.c
Original file line number Diff line number Diff line change
Expand Up @@ -1881,8 +1881,18 @@ struct hostent* sip_resolvehost( str* name, unsigned short* port,
}

he = do_srv_lookup( tmp, port, dn);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jes why don't we pass name->s in there directly (along with the length of the buffer, perhaps)? This way you won't need to memcpy() the result twice, first into tmp[] and then into the name->s.

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);
Expand Down