Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Timing leak in RPC authentication #2838

Closed
pakt opened this issue Jul 20, 2013 · 4 comments
Closed

Timing leak in RPC authentication #2838

pakt opened this issue Jul 20, 2013 · 4 comments
Labels

Comments

@pakt
Copy link

pakt commented Jul 20, 2013

bitcoinrpc.cpp

bool HTTPAuthorized(map<string, string>& mapHeaders)
{
    string strAuth = mapHeaders["authorization"];
    if (strAuth.substr(0,6) != "Basic ")
        return false;
    string strUserPass64 = strAuth.substr(6); boost::trim(strUserPass64);
    string strUserPass = DecodeBase64(strUserPass64);
    return strUserPass == strRPCUserColonPass;
}

Last string comparision strUserPass == strRPCUserColonPass gets compiled to:

    do
    {
      if ( !len )
        break;
      less = *(_BYTE *)strUserPass < *(_BYTE *)strRPCUserColonPass;
      eq = *(_BYTE *)strUserPass++ == *(_BYTE *)strRPCUserColonPass++;
      --len;
    }
    while ( eq );

which is a byte-by-byte compare. Attacker with precise clock (being in the same LAN, for example) might learn the RPC password letter by letter, by trying passwords like 'a...", "b...", "c..." and observing which took the longest time to verify.

This code in bitcoinrpc.cpp:

            if (mapArgs["-rpcpassword"].size() < 20)
                MilliSleep(250);

protects from bruteforce for short password. Unfortunately, when run, bitcoind suggest a 32 chars password. Paradoxically short passwords are safer to use than longer ones, wrt to the timing leak.

Here's an example of time independent array comparison: http://rdist.root.org/2010/01/07/timing-independent-array-comparison/

@jgarzik
Copy link
Contributor

jgarzik commented Jul 20, 2013

It's Basic authentication; easier just to decode the base64 ;p

@gmaxwell
Copy link
Contributor

@jgarzik he doesn't mean timing an actual authentication session, he means that even absent a user who knows the password an attacker who could reach the rpc port could analyze the timing of the rejections to derive the password one character at a time because the comparison shortcuts and thus returns faster when the error is at the front.

If you've exposed your rpc port to an attacker— life is probably not good for you to begin with. But this should probably using a constant time comparison and also delay all failures. I'll look into it.

grayleonard added a commit to grayleonard/bitcoin that referenced this issue Jul 23, 2013
As per bitcoin#2838 . Eliminates the possibility of timing attacks by changing the way the two passwords are compared. See http://rdist.root.org/2010/01/07/timing-independent-array-comparison/ for reference.

It iterates through each char in the strings, and if the two chars it is comparing aren't the same, then it adds 1 to nReturn and the function returns false. Previously, the function would return false on the first char that didn't match, allowing a possible attacker to run a timing attack.
@fgeek
Copy link

fgeek commented Jul 25, 2013

Please use CVE-2013-4165 for this issue as per http://openwall.com/lists/oss-security/2013/07/24/7

@laanwj
Copy link
Member

laanwj commented Nov 12, 2013

Solved by #2886

@laanwj laanwj closed this as completed Nov 12, 2013
IntegralTeam pushed a commit to IntegralTeam/bitcoin that referenced this issue Jun 4, 2019
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
uvhw referenced this issue in uvhw/conchimgiangnang Jun 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

6 participants
@laanwj @fgeek @jgarzik @gmaxwell @pakt and others