From af9bc12055b5eb7a7e4ef4ddd77182fd8d003328 Mon Sep 17 00:00:00 2001 From: Pavel Punsky Date: Sun, 16 Oct 2022 23:44:30 -0700 Subject: [PATCH] Sanitize DB connection string before printing to log (#1020) Store sanitized version of DB connection string with password masked (replace all chars with * which exposes its length) Use sanitized version when logging connection string Fixes #1017 and #272 --- src/apps/relay/dbdrivers/dbd_mysql.c | 12 ++++----- src/apps/relay/dbdrivers/dbd_pgsql.c | 8 +++--- src/apps/relay/dbdrivers/dbd_redis.c | 14 +++++----- src/apps/relay/dbdrivers/dbd_sqlite.c | 4 +-- src/apps/relay/dbdrivers/dbdriver.c | 36 ++++++++++++++++++++++++++ src/apps/relay/dbdrivers/dbdriver.h | 1 + src/apps/relay/mainrelay.c | 29 +++++++++++++++------ src/apps/relay/mainrelay.h | 2 +- src/apps/relay/netengine.c | 8 +++--- src/apps/relay/ns_ioalib_engine_impl.c | 6 ++--- src/apps/relay/ns_ioalib_impl.h | 2 +- src/apps/relay/turn_admin_server.c | 10 +++---- src/apps/relay/userdb.h | 8 +++++- 13 files changed, 97 insertions(+), 43 deletions(-) diff --git a/src/apps/relay/dbdrivers/dbd_mysql.c b/src/apps/relay/dbdrivers/dbd_mysql.c index 7717df8..5ee5421 100644 --- a/src/apps/relay/dbdrivers/dbd_mysql.c +++ b/src/apps/relay/dbdrivers/dbd_mysql.c @@ -239,17 +239,17 @@ static MYSQL *get_mydb_connection(void) { Myconninfo *co=MyconninfoParse(pud->userdb, &errmsg); if(!co) { if(errmsg) { - TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "Cannot open MySQL DB connection <%s>, connection string format error: %s\n",pud->userdb,errmsg); + TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "Cannot open MySQL DB connection <%s>, connection string format error: %s\n",pud->userdb_sanitized,errmsg); free(errmsg); } else { - TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "Cannot open MySQL DB connection <%s>, connection string format error\n",pud->userdb); + TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "Cannot open MySQL DB connection <%s>, connection string format error\n",pud->userdb_sanitized); } } else if(errmsg) { - TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "Cannot open MySQL DB connection <%s>, connection string format error: %s\n",pud->userdb,errmsg); + TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "Cannot open MySQL DB connection <%s>, connection string format error: %s\n",pud->userdb_sanitized,errmsg); free(errmsg); MyconninfoFree(co); } else if(!(co->dbname)) { - TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "MySQL Database name is not provided: <%s>\n",pud->userdb); + TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "MySQL Database name is not provided: <%s>\n",pud->userdb_sanitized); MyconninfoFree(co); } else { mydbconnection = mysql_init(NULL); @@ -270,7 +270,7 @@ static MYSQL *get_mydb_connection(void) { MYSQL *conn = mysql_real_connect(mydbconnection, co->host, co->user, co->password, co->dbname, co->port, NULL, CLIENT_IGNORE_SIGPIPE); if(!conn) { - TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "Cannot open MySQL DB connection: <%s>, runtime error\n",pud->userdb); + TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "Cannot open MySQL DB connection: <%s>, runtime error\n",pud->userdb_sanitized); mysql_close(mydbconnection); mydbconnection=NULL; } else if(mysql_select_db(mydbconnection, co->dbname)) { @@ -278,7 +278,7 @@ static MYSQL *get_mydb_connection(void) { mysql_close(mydbconnection); mydbconnection=NULL; } else if(!donot_print_connection_success) { - TURN_LOG_FUNC(TURN_LOG_LEVEL_INFO, "MySQL DB connection success: %s\n",pud->userdb); + TURN_LOG_FUNC(TURN_LOG_LEVEL_INFO, "MySQL DB connection success: %s\n",pud->userdb_sanitized); if(turn_params.secret_key_file[0]) { TURN_LOG_FUNC(TURN_LOG_LEVEL_INFO, "Encryption with AES is activated.\n"); TURN_LOG_FUNC(TURN_LOG_LEVEL_INFO, "Connection is secure.\n"); diff --git a/src/apps/relay/dbdrivers/dbd_pgsql.c b/src/apps/relay/dbdrivers/dbd_pgsql.c index 67b6f0c..b76d703 100644 --- a/src/apps/relay/dbdrivers/dbd_pgsql.c +++ b/src/apps/relay/dbdrivers/dbd_pgsql.c @@ -57,10 +57,10 @@ static PGconn *get_pqdb_connection(void) { PQconninfoOption *co = PQconninfoParse(pud->userdb, &errmsg); if(!co) { if(errmsg) { - TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "Cannot open PostgreSQL DB connection <%s>, connection string format error: %s\n",pud->userdb,errmsg); + TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "Cannot open PostgreSQL DB connection <%s>, connection string format error: %s\n",pud->userdb_sanitized,errmsg); free(errmsg); } else { - TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "Cannot open PostgreSQL DB connection: <%s>, unknown connection string format error\n",pud->userdb); + TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "Cannot open PostgreSQL DB connection: <%s>, unknown connection string format error\n",pud->userdb_sanitized); } } else { PQconninfoFree(co); @@ -68,13 +68,13 @@ static PGconn *get_pqdb_connection(void) { free(errmsg); pqdbconnection = PQconnectdb(pud->userdb); if(!pqdbconnection) { - TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "Cannot open PostgreSQL DB connection: <%s>, runtime error\n",pud->userdb); + TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "Cannot open PostgreSQL DB connection: <%s>, runtime error\n",pud->userdb_sanitized); } else { ConnStatusType status = PQstatus(pqdbconnection); if(status != CONNECTION_OK) { PQfinish(pqdbconnection); pqdbconnection = NULL; - TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "Cannot open PostgreSQL DB connection: <%s>, runtime error\n",pud->userdb); + TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "Cannot open PostgreSQL DB connection: <%s>, runtime error\n",pud->userdb_sanitized); } else if(!donot_print_connection_success){ TURN_LOG_FUNC(TURN_LOG_LEVEL_INFO, "PostgreSQL DB connection success: %s\n",pud->userdb); donot_print_connection_success = 1; diff --git a/src/apps/relay/dbdrivers/dbd_redis.c b/src/apps/relay/dbdrivers/dbd_redis.c index 7969164..d312621 100644 --- a/src/apps/relay/dbdrivers/dbd_redis.c +++ b/src/apps/relay/dbdrivers/dbd_redis.c @@ -161,22 +161,22 @@ static Ryconninfo *RyconninfoParse(const char *userdb, char **errmsg) { return co; } -redis_context_handle get_redis_async_connection(struct event_base *base, const char* connection_string, int delete_keys) { +redis_context_handle get_redis_async_connection(struct event_base *base, redis_stats_db_t* redis_stats_db, int delete_keys) { redis_context_handle ret = NULL; char *errmsg = NULL; - if(base && connection_string && connection_string[0]) { - Ryconninfo *co = RyconninfoParse(connection_string, &errmsg); + if(base && redis_stats_db->connection_string[0]) { + Ryconninfo *co = RyconninfoParse(redis_stats_db->connection_string_sanitized, &errmsg); if (!co) { if (errmsg) { - TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "Cannot open Redis DB connection <%s>, connection string format error: %s\n", connection_string, errmsg); + TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "Cannot open Redis DB connection <%s>, connection string format error: %s\n", redis_stats_db->connection_string_sanitized, errmsg); free(errmsg); } else { - TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "Cannot open Redis DB connection <%s>, connection string format error\n", connection_string); + TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "Cannot open Redis DB connection <%s>, connection string format error\n", redis_stats_db->connection_string_sanitized); } } else if (errmsg) { - TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "Cannot open Redis DB connection <%s>, connection string format error: %s\n", connection_string, errmsg); + TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "Cannot open Redis DB connection <%s>, connection string format error: %s\n", redis_stats_db->connection_string_sanitized, errmsg); free(errmsg); RyconninfoFree(co); } else { @@ -254,7 +254,7 @@ redis_context_handle get_redis_async_connection(struct event_base *base, const c if (!ret) { TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "Cannot initialize Redis DB connection\n"); } else if (is_redis_asyncconn_good(ret) && !donot_print_connection_success) { - TURN_LOG_FUNC(TURN_LOG_LEVEL_INFO, "Redis DB async connection to be used: %s\n", connection_string); + TURN_LOG_FUNC(TURN_LOG_LEVEL_INFO, "Redis DB async connection to be used: %s\n", redis_stats_db->connection_string_sanitized); donot_print_connection_success = 1; } RyconninfoFree(co); diff --git a/src/apps/relay/dbdrivers/dbd_sqlite.c b/src/apps/relay/dbdrivers/dbd_sqlite.c index 48803fa..69aecbf 100644 --- a/src/apps/relay/dbdrivers/dbd_sqlite.c +++ b/src/apps/relay/dbdrivers/dbd_sqlite.c @@ -184,7 +184,7 @@ static sqlite3 * get_sqlite_connection(void) { int rc = sqlite3_open(pud->userdb, &sqliteconnection); if(!sqliteconnection || (rc != SQLITE_OK)) { const char* errmsg = sqlite3_errmsg(sqliteconnection); - TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "Cannot open SQLite DB connection: <%s>, runtime error:\n %s\n (If your intention is to use an SQLite database for the TURN server, then\n check and fix, if necessary, the effective permissions of the TURN server\n process and of the DB directory and then re-start the TURN server)\n",pud->userdb,errmsg); + TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "Cannot open SQLite DB connection: <%s>, runtime error:\n %s\n (If your intention is to use an SQLite database for the TURN server, then\n check and fix, if necessary, the effective permissions of the TURN server\n process and of the DB directory and then re-start the TURN server)\n",pud->userdb_sanitized,errmsg); if(sqliteconnection) { sqlite3_close(sqliteconnection); sqliteconnection=NULL; @@ -193,7 +193,7 @@ static sqlite3 * get_sqlite_connection(void) { } else { init_sqlite_database(sqliteconnection); if(!donot_print_connection_success){ - TURN_LOG_FUNC(TURN_LOG_LEVEL_INFO, "SQLite DB connection success: %s\n",pud->userdb); + TURN_LOG_FUNC(TURN_LOG_LEVEL_INFO, "SQLite DB connection success: %s\n",pud->userdb_sanitized); donot_print_connection_success = 1; } } diff --git a/src/apps/relay/dbdrivers/dbdriver.c b/src/apps/relay/dbdrivers/dbdriver.c index 987666e..cc407f4 100644 --- a/src/apps/relay/dbdrivers/dbdriver.c +++ b/src/apps/relay/dbdrivers/dbdriver.c @@ -111,3 +111,39 @@ const turn_dbdriver_t * get_dbdriver() return _driver; } +char* sanitize_userdb_string(char* udb) { + char * ret = NULL; + char * pstart; + char * pend; + + /* host=localhost dbname=coturn user=turn password=turn connect_timeout=30 */ + ret = strdup(udb); + pstart = strstr (ret,"password="); + if (pstart != NULL) + { + pstart += strlen("password="); + pend = strstr (pstart," "); + size_t plen = pend - pstart; + if (pend == NULL) + { + plen = strlen(pstart); + } + memset(pstart,'*',plen); + } + else + { + /* postgresql://username:password@/databasename */ + pstart = strstr (ret,"postgresql://"); + if (pstart != NULL) + { + pstart += strlen("postgresql://"); + pend = strstr (pstart,"@"); + if (pend != NULL) + { + size_t plen = pend - pstart; + memset(pstart,'*',plen); + } + } + } + return ret; +} diff --git a/src/apps/relay/dbdrivers/dbdriver.h b/src/apps/relay/dbdrivers/dbdriver.h index aac9be8..ddaac28 100644 --- a/src/apps/relay/dbdrivers/dbdriver.h +++ b/src/apps/relay/dbdrivers/dbdriver.h @@ -81,6 +81,7 @@ typedef struct _turn_dbdriver_t { int convert_string_key_to_binary(char* keysource, hmackey_t key, size_t sz); persistent_users_db_t * get_persistent_users_db(void); const turn_dbdriver_t * get_dbdriver(void); +char* sanitize_userdb_string(char* udb); //////////////////////////////////////////// diff --git a/src/apps/relay/mainrelay.c b/src/apps/relay/mainrelay.c index f95f900..8018ee8 100644 --- a/src/apps/relay/mainrelay.c +++ b/src/apps/relay/mainrelay.c @@ -122,7 +122,7 @@ DEFAULT_STUN_TLS_PORT, /* tls_listener_port */ 0, /* no_tcp_relay */ 0, /* no_udp_relay */ "", -"",0, +{"",""},0, { NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,0,0,NULL,NULL,NULL }, @@ -165,7 +165,7 @@ DEFAULT_PROM_SERVER_PORT, /* prometheus port */ 0, /* prometheus username labelling disabled by default when prometheus is enabled */ #endif ///////////// Users DB ////////////// -{ (TURN_USERDB_TYPE)0, {"\0"}, {0,NULL, {NULL,0}} }, +{ (TURN_USERDB_TYPE)0, {"\0","\0"}, {0,NULL, {NULL,0}} }, ///////////// CPUs ////////////////// DEFAULT_CPUS_NUMBER, ///////// Encryption ///////// @@ -1501,12 +1501,14 @@ static void set_option(int c, char *value) add_static_user_account(value); break; case 'b': + { #if defined(TURN_NO_SQLITE) - TURN_LOG_FUNC(TURN_LOG_LEVEL_WARNING, "WARNING: Options -b, --userdb and --db are not supported because SQLite is not supported in this build.\n"); + TURN_LOG_FUNC(TURN_LOG_LEVEL_WARNING, "WARNING: Options -b, --userdb and --db are not supported because SQLite is not supported in this build.\n"); #else STRCPY(turn_params.default_users_db.persistent_users_db.userdb, value); turn_params.default_users_db.userdb_type = TURN_USERDB_TYPE_SQLITE; #endif + } break; #if !defined(TURN_NO_PQ) case 'e': @@ -1527,14 +1529,14 @@ static void set_option(int c, char *value) break; #endif #if !defined(TURN_NO_HIREDIS) - case 'N': + case 'N': STRCPY(turn_params.default_users_db.persistent_users_db.userdb, value); turn_params.default_users_db.userdb_type = TURN_USERDB_TYPE_REDIS; break; - case 'O': - STRCPY(turn_params.redis_statsdb, value); - turn_params.use_redis_statsdb = 1; - break; + case 'O': + STRCPY(turn_params.redis_statsdb.connection_string, value); + turn_params.use_redis_statsdb = 1; + break; #endif #if !defined(TURN_NO_PROMETHEUS) case PROMETHEUS_OPT: @@ -1700,6 +1702,17 @@ static void set_option(int c, char *value) fprintf(stderr,"\n%s\n", Usage); exit(-1); } + + if (turn_params.default_users_db.persistent_users_db.userdb[0]) { + char *userdb_sanitized = sanitize_userdb_string(turn_params.default_users_db.persistent_users_db.userdb); + STRCPY(turn_params.default_users_db.persistent_users_db.userdb_sanitized, userdb_sanitized); + free(userdb_sanitized); + } + if (turn_params.redis_statsdb.connection_string[0]) { + char *connection_string = sanitize_userdb_string(turn_params.redis_statsdb.connection_string); + STRCPY(turn_params.redis_statsdb.connection_string_sanitized, connection_string); + free(connection_string); + } } static int parse_arg_string(char *sarg, int *c, char **value) diff --git a/src/apps/relay/mainrelay.h b/src/apps/relay/mainrelay.h index 273a1f4..7eaa9be 100644 --- a/src/apps/relay/mainrelay.h +++ b/src/apps/relay/mainrelay.h @@ -229,7 +229,7 @@ typedef struct _turn_params_ { char listener_ifname[1025]; - char redis_statsdb[1025]; + redis_stats_db_t redis_statsdb; int use_redis_statsdb; struct listener_server listener; diff --git a/src/apps/relay/netengine.c b/src/apps/relay/netengine.c index 7f5f5bd..76c277d 100644 --- a/src/apps/relay/netengine.c +++ b/src/apps/relay/netengine.c @@ -1029,7 +1029,7 @@ static ioa_engine_handle create_new_listener_engine(void) ioa_engine_handle e = create_ioa_engine(sm, eb, turn_params.listener.tp, turn_params.relay_ifname, turn_params.relays_number, turn_params.relay_addrs, turn_params.default_relays, turn_params.verbose #if !defined(TURN_NO_HIREDIS) - ,turn_params.redis_statsdb + ,&turn_params.redis_statsdb #endif ); set_ssl_ctx(e, &turn_params); @@ -1068,7 +1068,7 @@ static void setup_listener(void) turn_params.relay_ifname, turn_params.relays_number, turn_params.relay_addrs, turn_params.default_relays, turn_params.verbose #if !defined(TURN_NO_HIREDIS) - ,turn_params.redis_statsdb + ,&turn_params.redis_statsdb #endif ); @@ -1634,7 +1634,7 @@ static void setup_relay_server(struct relay_server *rs, ioa_engine_handle e, int rs->ioa_eng = create_ioa_engine(rs->sm, rs->event_base, turn_params.listener.tp, turn_params.relay_ifname, turn_params.relays_number, turn_params.relay_addrs, turn_params.default_relays, turn_params.verbose #if !defined(TURN_NO_HIREDIS) - ,turn_params.redis_statsdb + ,&turn_params.redis_statsdb #endif ); set_ssl_ctx(rs->ioa_eng, &turn_params); @@ -1794,7 +1794,7 @@ static void* run_auth_server_thread(void *arg) bufferevent_enable(as->in_buf, EV_READ); #if !defined(TURN_NO_HIREDIS) - as->rch = get_redis_async_connection(as->event_base, turn_params.redis_statsdb, 1); + as->rch = get_redis_async_connection(as->event_base, &turn_params.redis_statsdb, 1); #endif barrier_wait(); diff --git a/src/apps/relay/ns_ioalib_engine_impl.c b/src/apps/relay/ns_ioalib_engine_impl.c index 89a40e3..9651dca 100644 --- a/src/apps/relay/ns_ioalib_engine_impl.c +++ b/src/apps/relay/ns_ioalib_engine_impl.c @@ -358,7 +358,7 @@ ioa_engine_handle create_ioa_engine(super_memory_t *sm, size_t relays_number, char **relay_addrs, int default_relays, int verbose #if !defined(TURN_NO_HIREDIS) - ,const char* redis_report_connection_string + ,redis_stats_db_t* redis_stats_db #endif ) { @@ -403,9 +403,7 @@ ioa_engine_handle create_ioa_engine(super_memory_t *sm, } #if !defined(TURN_NO_HIREDIS) - if(redis_report_connection_string && *redis_report_connection_string) { - e->rch = get_redis_async_connection(e->event_base, redis_report_connection_string, 0); - } + e->rch = get_redis_async_connection(e->event_base, redis_stats_db, 0); #endif { diff --git a/src/apps/relay/ns_ioalib_impl.h b/src/apps/relay/ns_ioalib_impl.h index 61f758f..03a6ea2 100644 --- a/src/apps/relay/ns_ioalib_impl.h +++ b/src/apps/relay/ns_ioalib_impl.h @@ -242,7 +242,7 @@ ioa_engine_handle create_ioa_engine(super_memory_t *sm, size_t relays_number, char **relay_addrs, int default_relays, int verbose #if !defined(TURN_NO_HIREDIS) - ,const char* redis_report_connection_string + ,redis_stats_db_t* redis_stats_db #endif ); diff --git a/src/apps/relay/turn_admin_server.c b/src/apps/relay/turn_admin_server.c index 883192a..8398f0e 100644 --- a/src/apps/relay/turn_admin_server.c +++ b/src/apps/relay/turn_admin_server.c @@ -785,8 +785,8 @@ static void cli_print_configuration(struct cli_session* cs) } #if !defined(TURN_NO_HIREDIS) - if(turn_params.use_redis_statsdb && turn_params.redis_statsdb[0]) - cli_print_str(cs,turn_params.redis_statsdb,"Redis Statistics DB",0); + if(turn_params.use_redis_statsdb && turn_params.redis_statsdb.connection_string[0]) + cli_print_str(cs,turn_params.redis_statsdb.connection_string,"Redis Statistics DB",0); #endif myprintf(cs,"\n"); @@ -1301,7 +1301,7 @@ void setup_admin_thread(void) adminserver.e = create_ioa_engine(sm, adminserver.event_base, turn_params.listener.tp, turn_params.relay_ifname, turn_params.relays_number, turn_params.relay_addrs, turn_params.default_relays, turn_params.verbose #if !defined(TURN_NO_HIREDIS) - ,turn_params.redis_statsdb + ,&turn_params.redis_statsdb #endif ); @@ -2185,8 +2185,8 @@ static void write_pc_page(ioa_socket_handle s) #if !defined(TURN_NO_HIREDIS) if(is_superuser()) { - if(turn_params.use_redis_statsdb && turn_params.redis_statsdb[0]) { - https_print_str(sb,turn_params.redis_statsdb,"Redis Statistics DB",0); + if(turn_params.use_redis_statsdb && turn_params.redis_statsdb.connection_string_sanitized[0]) { + https_print_str(sb,turn_params.redis_statsdb.connection_string_sanitized,"Redis Statistics DB",0); } } #endif diff --git a/src/apps/relay/userdb.h b/src/apps/relay/userdb.h index a1c0372..8141413 100644 --- a/src/apps/relay/userdb.h +++ b/src/apps/relay/userdb.h @@ -146,6 +146,11 @@ typedef struct _secrets_list secrets_list_t; #define TURN_LONG_STRING_SIZE (1025) +typedef struct _redis_stats_db_t { + char connection_string[TURN_LONG_STRING_SIZE]; + char connection_string_sanitized[TURN_LONG_STRING_SIZE]; +} redis_stats_db_t; + typedef struct _ram_users_db_t { size_t users_number; ur_string_map *static_accounts; @@ -154,6 +159,7 @@ typedef struct _ram_users_db_t { typedef struct _persistent_users_db_t { char userdb[TURN_LONG_STRING_SIZE]; + char userdb_sanitized[TURN_LONG_STRING_SIZE]; } persistent_users_db_t; typedef struct _default_users_db_t @@ -209,7 +215,7 @@ void ip_list_free(ip_range_list_t *l); ///////////// Redis ////////////////////// #if !defined(TURN_NO_HIREDIS) -redis_context_handle get_redis_async_connection(struct event_base *base, const char* connection_string, int delete_keys); +redis_context_handle get_redis_async_connection(struct event_base *base, redis_stats_db_t* redis_stats_db, int delete_keys); #endif ////////////////////////////////////////////