Skip to content

Instantly share code, notes, and snippets.

@elliotchance
Created October 8, 2019 06:18
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save elliotchance/05421b7a090af7e598ab3b5714408082 to your computer and use it in GitHub Desktop.
Save elliotchance/05421b7a090af7e598ab3b5714408082 to your computer and use it in GitHub Desktop.
import "golang.org/x/crypto/bcrypt"
// CheckPassword is used for Basic authentication to check the secret. The logic
// has been copied from the existing implementation in:
//
// protected/models/traits/HasPassword.php
//
// The PHP source code will be copied verbatim here, just in case something
// changes it can be compared:
//
// private static function digestPassword($password, $salt = null)
// {
// static $cryptPrefix = '$2y$10$';
// if ($salt === null) {
// $salt = str_replace(
// '+',
// '.',
// substr(base64_encode(md5(uniqid('', true), true)), 0, 21)
// );
// }
//
// return substr(crypt($password, $cryptPrefix . $salt . '$'), 7);
// }
//
// Turns out this is not a trivial problem. In fact it took an entire day to
// find out how and why salt was being handled incorrectly to produce the wrong
// hashes.
//
// Well, the hashes aren't wrong, per se, but if you fed the same password and
// salt into the equivalent Go built-in (bcrypt) package it would not work. Let
// me explain.
//
// The crypt() function in PHP, like many functions in PHP is a wrapper for the
// C libraries underneath. That can make it subject to underlying
// implementations. This is important to highlight because while PHP "wrapper"
// does have known and documented bugs with its crypt() function (specifically
// around Blowfish, which we are using). None of those are actually occurring
// (as far as I could tell, I will explain more later) with the difference in
// implementation (since the bcrypt package is pure Go).
//
// The crypt() function works by taking a password, encryption options
// (including the type of encryption) and an optional salt. The salt is optional
// because one will be generated for you if not provided. However, as you can
// see from the Kounta PHP implementation above we generate our own salt. This
// is what has caused the issue. Or at least, triggered it.
//
// It is a little confusing at first, but the reason why the salt is generated
// automatically for you but not returned or stored independently is because the
// salt is actually baked into (excuse the pun) the output hash. The result of
// using the crypt() function with Blowfish looks like this:
//
// crypt('foobar', '$2a$10$1234567890123456789012')
//
// => $2a$10$123456789012345678901udiU8wKV.famJggjCUFdUGtiTNmkIXYW
// ^-----^--------------------^-------------------------------^
// 1 2 3
//
// 1. Information about the encryption options used. The "2a" (mainly the "2")
// describes that we are using Blowfish. The "10" is the cost, which can range
// from 04 to 31. This in a nutshell is about how much CPU time you want to
// spend to generate the hash. Presumably higher is better, but 10 seems to be
// default in existing implementations I've seen.
//
// 2. This looks familiar right? Well, it is. It is most of the salt. "Most of"
// is the extremely confusing part that is the cause of this very long
// explanation. I'll save the in-depth for below.
//
// 3. This is the actual calculated Blowfish hash. It is not used by itself, but
// in conjunction with the salt part to verify the password again.
//
// So how is this hash used to actually test the password in the future,
// exactly? The oversimplified answer is we extract the encryption options and
// salt from the existing known hash. Running the password we wish to test with
// these back through the crypt() function should generate the same hash. When
// the entire hashes (all three parts) are equal the password must be correct.
//
// This is absolutely true, but it's also true that when the hashes are not
// equal under just the right cases the password is still valid.
//
// How is this possible if hashes are supposed to be deterministic? The same
// input(s) should always reveal the same outputs, right? It comes down the the
// salt. Remember how I said that the "most" of the salt makes it into the hash?
//
// The salt (for Blowfish) must be exactly 22 characters and encoded with base
// 64. However, it turns out that it doesn't use all of the salt. In fact it
// only uses 128 bits of it. Here is an example:
//
// // Notice only the last digit of the sale is different.
// crypt('foobar', '$2a$10$1234567890123456789012')
// crypt('foobar', '$2a$10$1234567890123456789013')
//
// Both of these calls produce the exact same hash, despite the different salt:
//
// $2a$10$123456789012345678901udiU8wKV.famJggjCUFdUGtiTNmkIXYW
//
// Great, so what's the last digit for if it's not needed? Well here's another
// example:
//
// // Once again only the last digit is different, but this time by a slightly
// // larger amount.
// crypt('foobar', '$2a$10$1234567890123456789012')
// crypt('foobar', '$2a$10$123456789012345678901A')
//
// This however generated two very different hashes:
//
// $2a$10$123456789012345678901udiU8wKV.famJggjCUFdUGtiTNmkIXYW
// $2a$10$123456789012345678901./ft8TTkhYi0llXsqn8E23vNOOhf0eM.
//
// What's going on and how does this relate to the PHP version? Look at the PHP
// version again and you will see that it is generating a key that is 21
// characters (not the required 22) but then appending a '$' below. The '$'
// could be misconstrued as a terminator since it's the same character we wrap
// the encryption options in. It's not.
//
// It actually being used as part of the salt. The not so illustrious 22nd
// required character. There one obvious problem with that. A '$' is not an
// acceptable character in base 64. So we cannot pass the same character through
// to the equivalent Go library without incurring a very reasonable error.
//
// Those familiar with base 64 may know that encoded data must always be a
// multiple of 4 bytes. If the data length is not a modulo of 4, then it's
// padded with '=' to the correct length. The '=' itself is not part of the base
// 64 alphabet so cannot appear anywhere else in the stream. However, on Unix
// '$' is considered to be part of the alphabet AND a padding character. Either
// by coincidence or not the '$' in this case is being treated as a padding
// character, like a '='.
//
// Let's go back to an example. Here is a real password and hash:
//
// $password = 'WB5i4u8TNQvJIbYfW2nQC3o6W1dLR0R08iuo3e6b';
// $hash = '$2a$10$37OBUteY4OKy.T9lB0cvJ.7s0Desj76NjmmcShDPF77vGgd2zWUA.'
//
// When testing this same combination in Go the CompareHashAndPassword() returns
// false. That is because internally it generates it's own hash that is expects
// to be equal:
//
// $2a$10$37OBUteY4OKy.T9lB0cvJ.UD/NmqeYcboloNeVVcvExyrCjFMzC5u
//
// The hash is completely different, but the password is still correct. The
// amount of significant bits from the salt is different.
//
// My only option was to brute force generate all combinations with the last two
// characters of the salt so see if I could generate the same Blowfish hash with
// a very similar salt because I knew similar salts produce the same hash.
//
// Fortunately I found a match, actually several. The salt could be modified to
// adjust for the right amount of significant bits by resetting both of the last
// characters to 0. In base 64 this is "..".
//
// When applying this "hack" to more examples (10 randomly generated passwords
// and hashes) it worked all cases. So it looks like this solves for the general
// case. These have been put into unit tests.
//
// The moral of this story is you should not generate your own salt.
//
// [1] https://stackoverflow.com/a/2237009/1470961
func CheckPassword(password, hash string) bool {
realExpectedHash := []byte("$2a$10$" + hash[:20] + ".." + hash[22:])
return bcrypt.CompareHashAndPassword(realExpectedHash, []byte(password)) == nil
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment