net: cleanup SOCKS5 auth logging #35116

pull takeshikurosawaa wants to merge 2 commits into bitcoin:master from takeshikurosawaa:cleanup-socks5-auth-logging changing 1 files +1 −1

takeshikurosawaa commented at 6:16 PM on April 19, 2026: none

Socks5() logs the supplied username and password in BCLog::PROXY when the server selects USER_PASS.

Keep the log entry for the auth path, but omit the credentials. Log the message before sending the authentication data.

No functional behavior change intended.
DrahtBot added the label P2P on Apr 19, 2026
DrahtBot commented at 6:17 PM on April 19, 2026: contributor



The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Reviews

See the guideline for information on the review process. A summary of reviews will appear here.



Conflicts

No conflicts as of last run.
takeshikurosawaa force-pushed on Apr 19, 2026
sedited commented at 8:26 PM on April 19, 2026: contributor

I don't think a unit test is required for this. Matching that way for a log string seems brittle. I'd be fine with just the log line change.
takeshikurosawaa force-pushed on Apr 19, 2026
takeshikurosawaa commented at 8:57 PM on April 19, 2026: none

Removed the unit test.

This only changes the log message.

net: cleanup SOCKS5 auth logging

Do not log SOCKS5 auth credentials.

Keep the log entry for the auth path, but omit the
username and password.

No behavior change intended.

b2debc9276

takeshikurosawaa force-pushed on Apr 19, 2026
net: log SOCKS5 auth before sending 3bf3b6d59a

in src/netbase.cpp:435 in b2debc9276 outdated

 431 | @@ -432,7 +432,7 @@ bool Socks5(const std::string& strDest, uint16_t port, const ProxyCredentials* a
 432 |              vAuth.push_back(auth->password.size());
 433 |              vAuth.insert(vAuth.end(), auth->password.begin(), auth->password.end());
 434 |              sock.SendComplete(vAuth, g_socks5_recv_timeout, g_socks5_interrupt);
 435 | -            LogDebug(BCLog::PROXY, "SOCKS5 sending proxy authentication %s:%s\n", auth->username, auth->password);
 436 | +            LogDebug(BCLog::PROXY, "SOCKS5 sending username/password authentication\n");

furszy commented at 10:01 PM on April 19, 2026:

Isn't this occurring after the send happened?. Could move it to the line above to actually log prior to sending the data.

takeshikurosawaa commented at 10:45 PM on April 19, 2026:

Fixed, moved it above send.

DrahtBot added the label CI failed on Apr 20, 2026
DrahtBot removed the label CI failed on Apr 20, 2026

Contributors

Labels

net: cleanup SOCKS5 auth logging #35116

Reviews

Conflicts