ircmaxell

True, but in this case if you can write an invalid hash into a database, you can likewise write a valid one, and as such this doesn't really enable anything.

The one thing this does get you is that the original password would still work (technically any password would still work) so it may make it harder to detect since the user wouldn't "suddenly be locked out"...

Heck, a very popular Java book (Android programming specifically) has examples of database queries using concatenation (and hence SQLi).

Except that in PHP type declaration mismatches are actually errors... They aren't just "documentation" to be used by static analyzers.

PHP. This is a known weakness in PHP's bcrypt implementation. From Wikipedia, "Many implementations of bcrypt truncate the password to the first 72 bytes." I would hope that they're using a competent implementation that either supports longer passwords or throws an error if it's asked to hash a longer password.

Actually, it's a known weakness in BCRYPT. PHP did not implement bcrypt, it was ported in via crypt(3). Meaning that ALL versions of bcrypt have this issue.

Some implementations error on > 72 bytes, but NONE of them accept longer passwords.

I don't think we know enough to conclude that they were definitely doing it wrong, but it would be nice to know more details about the algorithm, though.

Given what has been shared so far, there's enough signs pointing that the chances are pretty high they did something wrong. 40 byte salt? Bcrypt only supports a 128 bit salt. So either they did something silly custom (at which point it's no longer bcrypt), they aren't actually using bcrypt, or they did something silly like concatenate the salt + pepper + password and pass it to the password field.

The only implementations of bcrypt (and crypt(3)) that I've seen that's NOT vulnerable to this are Golang and Python.

It has nothing to do with PHP...

I would recommend that the author read up on NFAs and DFAs -- they are a formalism better suited to lexers than tries.

Author here. The actual regex implementation uses a NFA. The start of it used a Trie, but it moved away.

The majority of what I wanted to get across here was the use of a minimal structure (single-character).

The next step was using a maximal radix implementation (as long of a prefix as possible). Then finally, throwing all of it away and going straight to parsing using a state machine.

Well, let's look at it realistically: http://arstechnica.com/security/2015/01/yes-123456-is-the-mo...

The #1 password out of 3.3 million was 123456, which was used 20,000 times.

So extrapolating that for your 2 million hashes, we'd expect the top password to appear roughly 12,000 times.

Running those numbers, we'd expect each guess to have a 1/12000 chance of matching. Or more specifically, a 1988000/2000000 of not matching.

With some quick running of those numbers, we'd expect a 50% chance of finding a match after trying just 115 random usernames.

I'm not saying it isn't an interesting approach, I just don't think it's nearly as effective as if you encrypt the hash directly (which has no attack vector unless you can get the key).

Well, let's imagine an attack scenario.

As an attacker, I get SQL access to your DB (meaning no access to the encryption key). I then download the user names, and the hashes. I then attack the hashes offline. I recover only the weakest few percent (since you're using bcrypt). But since the weakest few are those most likely to be re-used (both by different users and by a single user across sites), they are going to be both more valuable to me and easier for the next steps:

Then, I take the highest frequency passwords and the user table, and I start validating them online in your system. Now if I do that too quickly, you'll notice and I'll be shut down. And if I do that all from the same IP, I'll be shut down.

But what if I had a botnet that I could distribute the load across. What if I kept my request rate small enough to stay under the radar of even a moderate scale system.

I would expect to start seeing meaningful results within days.

If you had 1000 users, then I could surmise that you don't have much traffic, and hence keep the request rate down to perhaps 100 per day. In 10 days I'd have at least a few u/p combinations that I know for a fact worked.

If you had 1000000 users, I could ramp it up quite a bit higher, to perhaps 1000 or 10000 per day.

And since they all came from separate IP addresses, it could be rather difficult for you to tell an attack was going on unless you were looking specifically for it.

Does that mean you should stop immediately? No. It's not that bad of a scheme. But be aware that it doesn't give you (or your users) the level of protection that it may look like on the surface.

A properly implemented, simple pepper can only help password security and can't hurt it.

Well, yes. But what is the definition of "properly"? There are definitely constructions of "pepper" that look simple, but drastically hurt overall security:

    bcrypt(hmac(password, key), salt)

It's sort of like the difference between birth control and counting based contraceptive methods (Standard Days Method). Executed perfectly, they are equally as effective. But with a slight error, one stays roughly as effective (losing maybe 5 to 10% effectiveness overall) while the other drops drastically (down to 10 to 20% effectiveness).

Considering using encryption is as effective as using a pepper, and it's less prone to weakening the core password hash, I suggest using encryption instead of peppers.

We looked into using it. After much thought, we decided not to, here's why: https://wiki.php.net/rfc/scalar_type_hints_v5#why_not_use_us...

I break down all of the proposed alternatives and the cases against them.

Having the caller do it seems quite pointless when the recipient is anyway doing it.

Actually, I disagree. The caller is the only one who has semantic information about what the variable (and hence its value) means. All the callee (recipient) can do is blind cast it. The caller on the other hand can interpret it because it knows the meaning (talking about the developer, not the engine).

You don't hire 10x just people who make other people 10x? Then that person you just hired is 10x.

The point was more that I value a person's ability to lift those around them much higher than their abilities to outperform them. It's hard to scale one person. But one person who can help scale an entire team is infinitely more valuable.

In simple business terms you have a guy who'll get you sued for 1 million but will make you 10 million.

He's not a liability because of potential lawsuits. He's a liability because he brings the team down (antics like this do have a huge impact on the rest of the team).

I don't hire nor value 10x engineers. So no problem there.

I hire and value engineers that elevate those around them to 10x.

And this behavior lowers people, not elevates. So no, that person is not a valuable asset. He's the very definition of a liability.

If PHP is the slowest part of your application, you're probably in good shape.

Or doing something epicly wrong. One of the two...

Actually, the PECL compiler he mentions generates C, and then compiles it using GCC with -O3.

So in a way, yes it is measuring the speed of a C program. Not a hand-optimized one, but it's compiled the same.

Why did we tolerate it? Because we weren't given the tools to do anything about it. We couldn't kick or ban people. We couldn't moderate our own room. All we (as owners) could do was move messages or flag. We got yelled at every time we flagged something, so we learned to live with it. The only other option we had was to leave (which many users did, even leaders).

Today, we have the ability to kick-ban. Awesome. But he's also calmed down a lot. And is seen as a resource.

It's gotten a lot better over the years, but there is still work to be done in there. But now we're starting to get the tools to handle it. Which is awesome.

That's definitely valid. However, in the vast majority of cases where someone complains about CV-PLS in my experience, it's because they had their question closed.

For example: http://meta.stackexchange.com/q/158712/147370

Look in this very thread. You have people saying it's a horrible practice. Yet nobody really saying what should be done instead.

The fact of the matter is that there is a huge problem on SO of under-moderation. Over 11,000 questions have >=1 closevote right now. CV-PLS is one technique that the community has found effective in keeping the site searchable and with good content.

The people who are against it, I'd love to hear ideas on other effective methods. But to say it should be forbidden is a bit short-sighted.

That's absolutely a valid experience.

There are plenty of others who've had positive experiences as well.

And those "caustic individuals" have a LOT to share if you're willing to engage in an open and healthy way.

How do you solve the torrent of duplicates and low quality questions then? The review queue is backed up all the time (currently 11900 questions with 1+ close-vote).

Ignoring the problem does not make it go away. If you think one of the ways people are trying to solve it should be forbidden, that's fine. Please share an effective alternative.

This has been discussed time and time again:

- http://meta.stackoverflow.com/q/256501/338665 - http://meta.stackoverflow.com/q/271899/338665 - http://meta.stackoverflow.com/q/278092/338665 - http://meta.stackoverflow.com/q/285471/338665

Etc...

And especially: http://meta.stackexchange.com/questions/120275/is-asking-oth...

Basically, there's nothing wrong with the concept, unless it's used incorrectly. So if someone closes something incorrectly (you can point to a definite reason it's incorrectly closed), then re-open it or raise a meta post.

If you want to remove the ability for the people helping moderate a community to moderate, then how do you expect it to be moderated?

The big issue is that there are a LOT of low quality questions being asked. Duplicates. Many times literally copy/pasting the the question title into Google will give you the answer. Should these questions remain open because you want to repwhore? Should they remain open and further reduce the ability for Google to take you to a good canonical answer?

Or should they be closed and point to the good canonical answer? That way people can find their way to good content, rather than littering the site with duplication and poor copies of other answers.

The meaning of the CV reasons has changed over time as the community matures and figures out what works and what doesn't.

I do disagree with closing questions about particular framework (unless there's a dedicated SE site for it).

But bitching doesn't help. Raise a question on Meta. Step into the chat rooms and have a discussion. Get involved and help us fix things.

All bitching does is make the people who are putting time and effort into the community feel like they are doing something bad. Which is the fastest way to kill a community.

Something seems horribly wrong here, no?

Yes. The distributions are effectively forking these versions, and keeping the same version numbers. Leading to some REALLY awkward problems: https://github.com/ircmaxell/password_compat/issues/10

However, it's a tradeoff. It hurts some, helps others. After all, I'd rather have someone get security fixes than nothing...

I counted any point release since the latest security release as secure.

So for PHP's 5.6 line, only 5.6.4 is secure, since 5.6.4 is a security release.

I'm currently crunching numbers for other platforms. For example, Nginx's last security release (for 1.7) was 1.7.5, so 1.7.5 -> 1.7.9 are all considered security.

Well, it won't idle if there is another process ready to execute (load is greater than 1). If there is no process wanting to execute, it will idle.

Again, I'm not saying this is practical. I'm saying it might be possible (even if improbable).

And don't get me wrong, I'm not saying "OMG YOU ARE BAD IF YOU DON"T PROTECT THIS RIGHT". I'm more leaning on the side of "if there's a chance, I assume someone could possibly figure out a way".

So, I tried this in the past.

The problem you'll run into with PHP specifically is that reading an undefined string offset (past the end) will result in a notice: http://3v4l.org/nIkf5

Which means that errors are triggered. So you can increase the length of the user string and note a linear increase in runtime until you increase it past the length of the string, at which point it becomes MUCH slower on a per-character basis (even if you don't do anything with the notice, the error mechanism is still triggered internally, which isn't cheap).

Actually, my original code was more robust as it never read past the end of the string, preventing the notice:

    /**
     * A timing safe equals comparison
     *
     * To prevent leaking length information, it is important
     * that user input is always used as the second parameter.
     *
     * @param string $safe The internal (safe) value to be checked
     * @param string $user The user submitted (unsafe) value
     *
     * @return boolean True if the two strings are identical.
     */
    function timingSafeEquals($safe, $user) {
        // Prevent issues if string length is 0
        $safe .= chr(0);
        $user .= chr(0);

        $safeLen = strlen($safe);
        $userLen = strlen($user);

        // Set the result to the difference between the lengths
        $result = $safeLen - $userLen;

        // Note that we ALWAYS iterate over the user-supplied length
        // This is to prevent leaking length information
        for ($i = 0; $i < $userLen; $i++) {
            // Using % here is a trick to prevent notices
            // It's safe, since if the lengths are different
            // $result is already non-0
            $result |= (ord($safe[$i % $safeLen]) ^ ord($user[$i]));
        }

        // They are only identical strings if $result is exactly 0...
        return $result === 0;
    }

Basically, while it may keep the length %64 safe (since cache lines are 64 bites wide), it doesn't keep the length safe in general. Some length information will be leaked on larger strings. And considering it's impossible to protect the length in the general case, making a function which says it protects length is a lie. Therefore I don't even try and hence save the complexity.

But let me ask this: what cases would you have where are you trying to protect the length? Anything with variable length input (like a password) should likely be one-way hashed anyway. So you'd be comparing fixed-length hashes. So where's the possible leak?

The far better approach is to just make the operations not depend on the secret.

You only really need to worry about timing attacks for values that the attacker doesn't know, and you don't want them to know.

So it's only things like encryption keys, passwords, session identifiers, reset tokens, etc that you need to worry about.

And programmers who touch sensitive code can easily forget the requirement for constant-time behaviour.

And that's why I support the discussion we were having on PHP's internals list where we talked about making functions which are commonly used with secrets timing safe by default. As long as there isn't a non-trivial performance penalty to it at least.

As far as worrying about it, I'd rather people understand SQLi and XSS better. They are both FAR bigger surface areas than a timing attack ever will be. And likely going to be the bigger threat to 99.99% of applications.

How can you actually make sensitive operations take constant time? This sounds impossibly hard. For example, your operating system could be context switching thousands of times per second.

Sorry, it appears that I didn't actually define constant time anywhere. What I really mean is that:

    Runtime does not depend in any way on the *value* of secret data.

So it's not about keeping "absolute" time constant, but only the impact of the secret on runtime.

Well, access to the CPU happens with every user (since you can see the current mode of every core as an unprivileged user - idle, wait or running).

Accessing RAM requires system level access (privileged users, super user really) or running as the same user as the other process.

So unless the server is horribly misconfigured, or you exploit another vulnerability, reading from RAM isn't as likely as monitoring the CPU.

OP here. Seeing as this question is getting asked a lot, I'll edit something into the post, but I wanted to answer you here as well.

So, there are a few problems with this technique.

1. It ignores the local timing leak

An attacker who can get code running on the server (shared hosts for example), can carefully monitor the CPU usage to see when the process is actually doing work, vs when it sleeps. So really, its not hiding anything.

2. The resolution of the sleep call is WAY too high. We're talking about detecting differences down to 15 nanoseconds. Sleeping for blocks of microseconds or even milliseconds will be far to granular. It will introduce block-like patterns in the requests that should be pretty easy to detect with statistical means.

3. It's basically identical to a random delay. Considering it depends on the system clock, and the original request comes in at a random point, it's functionally identical to calling sleep(random(1, 100)). And over time (many requests), that will average out.

Now, what if we took a different approach. What if we made the operation fixed-time?

    execstart = utime()
    // whatever code
    // clamp to always take 500 microseconds
    sleep( 500 - utime() - execduration)

However, there are two reasons I wouldn't recommend it: It requires guesswork and idle CPU.

You would either need to actively guess every single operation (and remember to clamp it) or clamp the overall application.

If you do it for every operation, that sleep time can become expensive (if you have a lot of them).

If you do it on the application level, and if you do too little, an attacker can use other expensive control (like larger input introducing memory allocation latency) to increase the runtime past the sleep clamp (hence allowing them to attack the vulnerability anyway). If you do too much, the attacker can leverage it to DOS your site (since even a sleeping process is non-trivially expensive).

There are two valid ways of protection IMHO:

1. Make sensitive operations actually constant time.

2. Implement strong IP based protections to prevent the large amount of requests that would be needed to collect enough data to analyze noisy environments. (I need to add this to the post now that I write it).

Personally, you should be doing #2 anyway. But since I also believe in defense-in-depth, I'd do #1 as well.

Please look at the test script again. It does indeed warm up the JIT.

The timing results come from within HHVM. Basically, the test is dispatched within HHVM, isolating just the function call itself. It's then executed 5 times (timed independently) and then the bottom 2 executions are thrown away. See: https://gist.github.com/ircmaxell/8859d7f1bbee7b2f6e16#file-...

So the warm-up period happens. It's not just "benchmark `hhvm file.php`". The benchmark happens inside of HHVM.

So yes, JIT warmup is accounted for 100%. In fact, PHP, HHVM and Recki-CT all execute with the exact same pre-compile steps (parsing and any pre-compiling are ignored in the benchmark).

And their AOT approach that they tried was all-or-nothing. Meaning the entire code base had to be converted. This is a "compile what you want, and run the rest normally". In fact, this is 100% compatible with Hack and HHVM...

To be 100% fair, this is a non-official 20% project. So "Google backed" is a bit of a misnomer.