Message-ID: From: "jarvis24young (@jarvis24young)" To: "postgresql-interfaces/psqlodbc" Date: Sat, 09 May 2026 09:02:56 +0000 Subject: [postgresql-interfaces/psqlodbc] PR #184: Redact sensitive connection parameters in logs List-Id: X-GitHub-Additions: 146 X-GitHub-Author-Id: 48787405 X-GitHub-Author-Login: jarvis24young X-GitHub-Base: main X-GitHub-Changed-Files: 1 X-GitHub-Commits: 1 X-GitHub-Deletions: 5 X-GitHub-Head-Branch: redact-sensitive-conn-logs X-GitHub-Head-SHA: a8dae8f57158fddac3bf6757ca409718cf8a05a0 X-GitHub-Issue: 184 X-GitHub-Merge-SHA: 53e01fdc2f2d7ae5c6cdc7cd7657123acfce502d X-GitHub-Merged-By: davecramer X-GitHub-Repo: postgresql-interfaces/psqlodbc X-GitHub-State: merged X-GitHub-Type: pull_request X-GitHub-Url: https://github.com/postgresql-interfaces/psqlodbc/pull/184 Content-Type: text/plain; charset=utf-8 ## Summary This avoids writing sensitive libpq connection parameter values to the driver logs when debug or communication logging is enabled. The previous LIBPQ_connect() logging path printed: - the raw pqopt string - every expanded PQconnectdbParams keyword/value pair That can expose values such as password, passfile, sslpassword, sslkey, sslcert, sslrootcert, sslcrl, and sslcrldir in debug logs. ## Changes - Treat the raw pqopt string as sensitive in the initial connection trace. - Add a small is_sensitive_conninfo_param() helper for libpq connection keywords. - Keep parameter names in the PQconnectdbParams trace, but replace sensitive values with xxxxx. - Leave non-sensitive parameters unchanged so the log remains useful for diagnostics. ## Verification - git diff --check I did not add a regression test because this path depends on driver log file output and logging configuration. The change is limited to log formatting and does not affect the values passed to PQconnectdbParams(). diff --git a/connection.c b/connection.c index ae751c0f..37e13f5e 100644 --- a/connection.c +++ b/connection.c @@ -46,6 +46,7 @@ #include "pgapifunc.h" #define SAFE_STR(s) (NULL != (s) ? (s) : "(null)") +#define REDACTED_CONNINFO_VALUE "xxxxx" #define STMT_INCREMENT 16 /* how many statement holders to allocate * at a time */ @@ -166,7 +167,7 @@ PGAPI_Connect(HDBC hdbc, free(tmpstr); } - MYLOG(0, "conn = %p (DSN='%s', UID='%s', PWD='%s')\n", conn, ci->dsn, ci->username, NAME_IS_VALID(ci->password) ? "xxxxx" : ""); + MYLOG(0, "conn = %p (DSN='%s', UID='%s', PWD='%s')\n", conn, ci->dsn, ci->username, NAME_IS_VALID(ci->password) ? REDACTED_CONNINFO_VALUE : ""); if ((fchar = CC_connect(conn, NULL)) <= 0) { @@ -1070,7 +1071,7 @@ static char CC_initial_log(ConnectionClass *self, const char *func) return 0; } - MYLOG(0, "DSN = '%s', server = '%s', port = '%s', database = '%s', username = '%s', password='%s'\n", ci->dsn, ci->server, ci->port, ci->database, ci->username, NAME_IS_VALID(ci->password) ? "xxxxx" : ""); + MYLOG(0, "DSN = '%s', server = '%s', port = '%s', database = '%s', username = '%s', password='%s'\n", ci->dsn, ci->server, ci->port, ci->database, ci->username, NAME_IS_VALID(ci->password) ? REDACTED_CONNINFO_VALUE : ""); return 1; } @@ -2786,6 +2787,132 @@ LIBPQ_update_transaction_status(ConnectionClass *self) #define PROTOCOL3_OPTS_MAX 30 +static BOOL +is_sensitive_conninfo_param(const char *keyword) +{ + static const char * const sensitive_keywords[] = { + /* + * Keep certificate and CRL paths visible for SSL diagnostics. + * Redact only secrets or files that can contain them. + */ + "password", + "passfile", + "sslpassword", + "sslkey", + NULL + }; + const char * const *sensitive_keyword; + + if (keyword == NULL) + return FALSE; + + for (sensitive_keyword = sensitive_keywords; *sensitive_keyword != NULL; + sensitive_keyword++) + { + if (stricmp(keyword, *sensitive_keyword) == 0) + return TRUE; + } + return FALSE; +} + +/* + * Best-effort: log a libpq conninfo string with sensitive values redacted. + * Uses MYPRINTF so the caller must already have opened the line with MYLOG. + * The per-keyword PQconnectdbParams trace below provides the authoritative + * diagnostic view — this routine only needs to handle the common cases. + */ +static void +log_redacted_pqopt(const char *conninfo) +{ + const char *p = conninfo; + BOOL first = TRUE; + + if (NULL == p || '\0' == p[0]) + return; + + while (*p) + { + const char *keystart, *keyend; + const char *valstart, *valend; + BOOL is_sensitive; + size_t klen; + char kbuf[32]; + + /* Skip whitespace between pairs */ + while (*p == ' ' || *p == '\t') + p++; + if (!*p) + break; + + /* Find '=' separator */ + keystart = p; + while (*p && *p != '=') + p++; + if (!*p) + break; + keyend = p; + /* Trim trailing whitespace from keyword */ + while (keyend > keystart && (keyend[-1] == ' ' || keyend[-1] == '\t')) + keyend--; + p++; /* skip '=' */ + /* Skip leading whitespace after '=' */ + while (*p == ' ' || *p == '\t') + p++; + + /* Find value end */ + if (*p == '\'') + { + p++; /* skip opening quote */ + valstart = p; + while (*p) + { + if (*p == '\'') + { + if (p[1] == '\'') + p += 2; /* escaped quote '' */ + else + break; /* closing quote */ + } + else + { + p++; + } + } + valend = p; + if (*p == '\'') + p++; /* skip closing quote */ + } + else + { + valstart = p; + while (*p && *p != ' ' && *p != '\t') + p++; + valend = p; + } + + /* Check if keyword is sensitive */ + klen = (size_t)(keyend - keystart); + if (klen < sizeof(kbuf)) + { + memcpy(kbuf, keystart, klen); + kbuf[klen] = '\0'; + is_sensitive = is_sensitive_conninfo_param(kbuf); + } + else + { + is_sensitive = FALSE; + } + + if (is_sensitive) + MYPRINTF(0, "%s%s=" REDACTED_CONNINFO_VALUE, first ? "" : " ", kbuf); + else + MYPRINTF(0, "%s%.*s=%.*s", first ? "" : " ", + (int)klen, keystart, + (int)(valend - valstart), valstart); + first = FALSE; + } +} + static int LIBPQ_connect(ConnectionClass *self) { @@ -2802,10 +2929,19 @@ LIBPQ_connect(ConnectionClass *self) char keepalive_idle_str[20]; char keepalive_interval_str[20]; char *errmsg = NULL; + const char *pqopt_str = SAFE_NAME(ci->pqopt); - MYLOG(0, "connecting to the database using %s as the server and pqopt={%s}\n", self->connInfo.server, SAFE_NAME(ci->pqopt)); + if (pqopt_str[0] != '\0') + { + MYLOG(0, "connecting to the database using %s as the server and pqopt={", self->connInfo.server); + log_redacted_pqopt(pqopt_str); + MYPRINTF(0, "}\n"); + } + else + MYLOG(0, "connecting to the database using %s as the server and pqopt={}\n", + self->connInfo.server); - if (NULL == (conninfoOption = PQconninfoParse(SAFE_NAME(ci->pqopt), &errmsg))) + if (NULL == (conninfoOption = PQconninfoParse(pqopt_str, &errmsg))) { char emsg[200]; @@ -2916,7 +3052,12 @@ LIBPQ_connect(ConnectionClass *self) QLOG(0, "PQconnectdbParams:"); for (popt = opts, pval = vals; *popt; popt++, pval++) - QPRINTF(0, " %s='%s'", *popt, *pval); + { + if (is_sensitive_conninfo_param(*popt)) + QPRINTF(0, " %s='%s'", *popt, REDACTED_CONNINFO_VALUE); + else + QPRINTF(0, " %s='%s'", *popt, *pval); + } QPRINTF(0, "\n"); } pqconn = PQconnectdbParams(opts, vals, FALSE);