RPC: Avoid cleartext passwords by default #1986
Conversation
|
I agree this is useful, even with encrypted transport over SSL it's a safe precaution to not use cleartext passwords. It's a pity though that we need to invent our own authentication scheme for this, but it looks like you evaluated the other options already. One question: taking a quick look at the code I don't see any challenge response code in there. How well does this withstand replay attacks? Does it check the embedded date/time? |
| string strUser = vWords[1]; | ||
| if (strUser != mapArgs["-rpcuser"]) | ||
| return false; | ||
| string strDate = vWords[2]; |
There was a problem hiding this comment.
What is the format of strDate? And shouldn't there be a check here to make sure strDate is reasonably close to the current time, to prevent replay attacks?
There was a problem hiding this comment.
-
Format: Unix time (number of seconds since epoch)
-
Yes, there should be a check. See main pull req discussion, querying the best time window.
|
Your basic impression is sound. HTTP Digest was the preferred solution, but that introduces an additional round-trip to obtain a server nonce value, mitigating replay attacks. Lacking that round trip to gain additional protection, the best one can do is make sure the embedded time is within a certain window. Currently there are no time checks, because I wanted to get some feedback as to best time window. The server could demand that the HTTP request time be +/- five (5) minutes of its own system clock... or one hour? Which one takes into account broken clients etc. the best? What is a good time window? The smaller the window, the more secure. |
|
Just added a commit to clamp the timestamp to +/- 60 seconds. However, it is still open for discussion, because this is a usability issue: users with a drifting clock will get an unfriendly, vague "not authorized" error message. The window should be as small as possible... while still being wide enough to avoid most users hitting this issue. (of course, I think all users should be running or sync'ing with NTP, but that's an unrealistic opinion.....) |
|
So why not use digest auth instead of something custom (and fragile) and solve the latency concerns by persisting the connection where you're concerned with that. The session bring up tear down has more of those nasty round trips in any case. |
|
"custom and fragile" is oversold, considering this is intentionally highly similar to Amazon S3's HMAC authentication scheme. S3 must even deal with the same issue: picking a time window encompassing average client clock drift, without shutting out too many. I think they chose 5 minutes. |
|
RE: times: default should be 'as secure as possible.' Users should sync their clocks (but an if (fDebug) printf("rpc timestamp out of sync...") would be nice to make it easier to debug but not open up non -debug usage to flood-of-bad-authorization DoS attack). RE: custom and fragile: This seems less fragile than the old, maybe-soon-to-be-broken, MD5-based HTTP digest authorization. Jeff: any reason this is highly similar to Amazon's scheme and not exactly the same? |
|
I'm just concerned that we're solving things that weren't obvious a problem with novel cryptographic authentication protocols. Even with authentication it is still unacceptable to run RPC across an insecure network because an active attacker can just intercept and replace RPC calls (not to mention sniffing the content)... and without connection persistence anything latency sensitive will still suffer. And this will insert really confusing failure modes. I don't normally expect my client's clock to have to be in the right timezone. |
|
The DKIM RFC 6376 : http://www.ietf.org/rfc/rfc6376.txt ... also seems very similar. It uses an explicit expiration time, which seems like the right thing to do. It is overly complicated for this (e.g. we don't need multiple signature algorithms, don't need to specify which headers are part of the signature hash, etc), but I always like adopting or adapting an existing standard that experts have blessed instead of inventing our own. |
|
Needs rebase |
Although in theory RPC API access should be locked down, there are occasions where cleartext passwords have been used anyway. HTTP Basic authentication remains, but a new default "Bitcoin" HTTP Authorization header is used. HTTP Digest authentication was considered initially, but that may require additional HTTP round-trips. The standard HMAC-SHA256 algorithm pair was chosen instead, with some additional stirring factors (random nonce, time). The HTTP server will accept Basic or Bitcoin authentication now. The HTTP client will attempt Bitcoin authentication, and fall back to Basic if that fails.
|
Rebased. Still need to address valid @gavinandresen etc. comments. |
|
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/010cc65142f83b1045bc21b64cfe7cb46b8eff78 for binaries and test log. |
|
Closing. Will reopen with closer integration with Amazon S3 spec, as per comments. |
|
Found a bug. The initial request sends "Connection: close" header, but then if the server requests Basic auth instead, it sends the retry over the same socket (which is now closed). |
d061387 [Build] Replicate TravisCI tests in Github Actions (Fuzzbawls) Pull request description: This is a near 1:1 re-creation of our TravisCI workflow implemented in Github Actions. TravisCI recently rolled out un-announced changes that have made it extremely difficult for open source projects to continue using the free build jobs they are supposed to able to use. No artifacts are made available here due to the need to use a specific zk-params path at configure time so our unit/functional tests can run properly. The resulting binaries would not be easily portable to other systems. See the workflow results in my repo to view the build logs for this: https://github.com/Fuzzbawls/PIVX/actions/runs/378462901 ACKs for top commit: random-zebra: Big ACK d061387 furszy: Big ACK d061387 as well 🚀 and merging.. Tree-SHA512: 75afbf87648dd35da596774150a79ad2fd2aad0f16cec5f247455ecc7161c9fb2842edd30ab967ded8365654aeddf5571d6902e56aa371f2c3e24f3e924956e4
Although in theory RPC API access should be locked down, there are
occasions where cleartext passwords have been used anyway.
HTTP Basic authentication remains, but a new default "Bitcoin" HTTP
Authorization header is used. HTTP Digest authentication was considered
initially, but that may require additional HTTP round-trips. The standard
HMAC-SHA256 algorithm pair was chosen instead, with some additional stirring
factors (random nonce, time).
The HTTP server will accept Basic or Bitcoin authentication now.
The HTTP client will attempt Bitcoin authentication, and fall back to
Basic if that fails.