Skip to content

Commit

Permalink
Cache DB: Escape all logged URL passwords
Browse files Browse the repository at this point in the history
Additionally:
    * fix similar security issues in the Mongo and Redis clients
    * rewrite some cachedb unit tests

Suggested by @ChrisZhangJin

Related to #2416

(cherry picked from commit ed74875)
  • Loading branch information
liviuchircu committed Feb 24, 2021
1 parent c6baca4 commit c093360
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 16 deletions.
2 changes: 1 addition & 1 deletion cachedb/cachedb.c
Expand Up @@ -650,7 +650,7 @@ cachedb_con* cachedb_do_init(str *url,void* (*new_connection)(struct cachedb_id

id = new_cachedb_id(url);
if (!id) {
LM_ERR("cannot parse url [%.*s]\n",url->len,url->s);
LM_ERR("cannot parse url [%s]\n", db_url_escape(url));
pkg_free(res);
return 0;
}
Expand Down
3 changes: 2 additions & 1 deletion cachedb/cachedb_id.c
Expand Up @@ -346,7 +346,8 @@ struct cachedb_id* new_cachedb_id(const str* url)
memset(ptr, 0, sizeof(struct cachedb_id));

if (parse_cachedb_url(ptr, url) < 0) {
LM_ERR("error while parsing database URL: '%.*s' \n", url->len, url->s);
LM_ERR("error while parsing database URL: '%s'\n",
db_url_escape(url));
goto err;
}

Expand Down
14 changes: 7 additions & 7 deletions cachedb/test/test_cachedb.c
Expand Up @@ -444,11 +444,11 @@ static void test_cachedb_url(void)
db = new_cachedb_id(_str("redis:group1://:devxxxxxx@172.31.180.127:6379"));
if (!ok(db != NULL))
return;
ok(str_match(_str(db->scheme), &str_init("redis")));
ok(str_match(_str(db->group_name), &str_init("group1")));
ok(str_match(_str(db->username), &str_init("")));
ok(str_match(_str(db->password), &str_init("devxxxxxx")));
ok(str_match(_str(db->host), &str_init("172.31.180.127")));
ok(!strcmp(db->scheme, "redis"));
ok(!strcmp(db->group_name, "group1"));
ok(!strcmp(db->username, ""));
ok(!strcmp(db->password, "devxxxxxx"));
ok(!strcmp(db->host, "172.31.180.127"));
ok(db->port == 6379);
ok(!db->database);
ok(!db->extra_options);
Expand All @@ -463,6 +463,6 @@ static void test_cachedb_url(void)
db = new_cachedb_id(_str("redis:group1://:devxxxxxx@172.31.180.127:6379/d?x=1&q=2"));
if (!ok(db != NULL))
return;
ok(str_match(_str(db->database), &str_init("d")));
ok(str_match(_str(db->extra_options), &str_init("x=1&q=2")));
ok(!strcmp(db->database, "d"));
ok(!strcmp(db->extra_options, "x=1&q=2"));
}
2 changes: 1 addition & 1 deletion modules/cachedb_mongodb/cachedb_mongodb.c
Expand Up @@ -148,7 +148,7 @@ static int child_init(int rank)
cachedb_con *con;

for (it = mongodb_script_urls;it;it=it->next) {
LM_DBG("iterating through conns - [%.*s]\n",it->url.len,it->url.s);
LM_DBG("iterating through conns - [%s]\n", db_url_escape(&it->url));
con = mongo_con_init(&it->url);
if (con == NULL) {
LM_ERR("failed to open connection\n");
Expand Down
10 changes: 5 additions & 5 deletions modules/cachedb_mongodb/cachedb_mongodb_dbase.c
Expand Up @@ -126,12 +126,12 @@ mongo_con* mongo_new_connection(struct cachedb_id* id)

snprintf(osips_appname, MONGOC_HANDSHAKE_APPNAME_MAX, "opensips-%d", my_pid());

LM_DBG("MongoDB conn for [%s]: %s:%s %s:%s |%s|:%u\n", osips_appname,
id->scheme, id->group_name, id->username, id->password, id->host, id->port);
LM_DBG("MongoDB conn for [%s]: %s:%s://%s:xxxxxx@%s:%u\n", osips_appname,
id->scheme, id->group_name, id->username, id->host, id->port);

conn_str = build_mongodb_connect_string(id);

LM_DBG("cstr: %s\n", conn_str);
LM_DBG("cstr: %s\n", _db_url_escape(conn_str));

con = pkg_malloc(sizeof *con);
if (!con) {
Expand All @@ -144,7 +144,7 @@ mongo_con* mongo_new_connection(struct cachedb_id* id)

con->client = mongoc_client_new(conn_str);
if (!con->client) {
LM_ERR("failed to connect to Mongo (%s)\n", conn_str);
LM_ERR("failed to connect to Mongo (%s)\n", _db_url_escape(conn_str));
return NULL;
}

Expand Down Expand Up @@ -1673,7 +1673,7 @@ int mongo_truncate(cachedb_con *con)
start_expire_timer(start, mongo_exec_threshold);
if (!mongoc_collection_remove(MONGO_COLLECTION(con),
MONGOC_REMOVE_NONE, &empty_doc, NULL, &error)) {
LM_ERR("failed to truncate con %.*s!\n", con->url.len, con->url.s);
LM_ERR("failed to truncate collection '%s'!\n", MONGO_COL_STR(con));
ret = -1;
}
stop_expire_timer(start, mongo_exec_threshold, "MongoDB truncate",
Expand Down
2 changes: 1 addition & 1 deletion modules/cachedb_redis/cachedb_redis.c
Expand Up @@ -123,7 +123,7 @@ static int child_init(int rank)
cachedb_con *con;

for (it = redis_script_urls;it;it=it->next) {
LM_DBG("iterating through conns - [%.*s]\n",it->url.len,it->url.s);
LM_DBG("iterating through conns - [%s]\n", db_url_escape(&it->url));
con = redis_init(&it->url);
if (con == NULL) {
LM_ERR("failed to open connection\n");
Expand Down
42 changes: 42 additions & 0 deletions ut.c
Expand Up @@ -421,3 +421,45 @@ int word64decode(unsigned char *out, unsigned char *in, int len)

return out_len;
}

char *db_url_escape(const str *url)
{
static str buf;
char *at, *slash, *scn;
str upw;

if (!url)
return NULL;

if (pkg_str_extend(&buf, url->len + 6 + 1) < 0) {
LM_ERR("oom\n");
return NULL;
}

/* if there's no '@' sign, the URL has no password */
at = q_memchr(url->s, '@', url->len);
if (!at)
goto url_is_safe;

/* locate the end of the scheme (typical start for the user:password) */
slash = q_memchr(url->s, '/', url->len);
if (!slash || slash >= at)
goto url_is_safe;

upw.s = slash;
upw.len = at - slash;

/* if the semicolon is missing, the URL has no password (only username) */
scn = q_memchr(upw.s, ':', upw.len);
if (!scn)
goto url_is_safe;

sprintf(buf.s, "%.*s:xxxxxx@%.*s", (int)(scn - url->s), url->s,
(int)(url->len - (at - url->s) - 1), at + 1);

return buf.s;

url_is_safe:
sprintf(buf.s, "%.*s", url->len, url->s);
return buf.s;
}
9 changes: 9 additions & 0 deletions ut.h
Expand Up @@ -1288,6 +1288,15 @@ static inline void * l_memmem(const void *b1, const void *b2,
return NULL;
}

/**
* Make any database URL log-friendly by masking its password, if any
* Note: makes use of a single, static buffer -- use accordingly!
*/
char *db_url_escape(const str *url);
static inline char *_db_url_escape(char *url)
{
return db_url_escape(_str(url));
}

int user2uid(int* uid, int* gid, char* user);

Expand Down

0 comments on commit c093360

Please sign in to comment.