Differential D18908 Diff 45326 src/applications/people/storage/PhabricatorUser.php

Changeset View

Standalone View

src/applications/people/storage/PhabricatorUser.php

Show First 20 Lines • Show All 1,584 Lines • ▼ Show 20 Lines	public function newPasswordDigest(
// machine for the next century. By digesting passwords to a standard		// machine for the next century. By digesting passwords to a standard
// length first, the length of the raw input does not impact the runtime		// length first, the length of the raw input does not impact the runtime
// of the hashing algorithm.		// of the hashing algorithm.

// Some hashers like bcrypt are self-salting, while other hashers are not.		// Some hashers like bcrypt are self-salting, while other hashers are not.
// Applying salt while digesting passwords ensures that hashes are salted		// Applying salt while digesting passwords ensures that hashes are salted
// whether we ultimately select a self-salting hasher or not.		// whether we ultimately select a self-salting hasher or not.

// For legacy compatibility reasons, the VCS and Account password digest		// For legacy compatibility reasons, old VCS and Account password digest
// algorithms are significantly more complicated than necessary to achieve		// algorithms are significantly more complicated than necessary to achieve
// these goals. This is because they once used a different hashing and		// these goals. This is because they once used a different hashing and
// salting process. When we upgraded to the modern modular hasher		// salting process. When we upgraded to the modern modular hasher
// infrastructure, we just bolted it onto the end of the existing pipelines		// infrastructure, we just bolted it onto the end of the existing pipelines
// so that upgrading didn't break all users' credentials.		// so that upgrading didn't break all users' credentials.

// New implementations can (and, generally, should) safely select the		// New implementations can (and, generally, should) safely select the
// simple HMAC SHA256 digest at the bottom of the function, which does		// simple HMAC SHA256 digest at the bottom of the function, which does
// everything that a digest callback should without any needless legacy		// everything that a digest callback should without any needless legacy
// baggage on top.		// baggage on top.

		if ($password->getLegacyDigestFormat() == 'v1') {
switch ($password->getPasswordType()) {		switch ($password->getPasswordType()) {
case PhabricatorAuthPassword::PASSWORD_TYPE_VCS:		case PhabricatorAuthPassword::PASSWORD_TYPE_VCS:
// VCS passwords use an iterated HMAC SHA1 as a digest algorithm. They		// Old VCS passwords use an iterated HMAC SHA1 as a digest algorithm.
// originally used this as a hasher, but it became a digest alorithm		// They originally used this as a hasher, but it became a digest
// once hashing was upgraded to include bcrypt.		// algorithm once hashing was upgraded to include bcrypt.
$digest = $envelope->openEnvelope();		$digest = $envelope->openEnvelope();
$salt = $this->getPHID();		$salt = $this->getPHID();
for ($ii = 0; $ii < 1000; $ii++) {		for ($ii = 0; $ii < 1000; $ii++) {
$digest = PhabricatorHash::weakDigest($digest, $salt);		$digest = PhabricatorHash::weakDigest($digest, $salt);
}		}
return new PhutilOpaqueEnvelope($digest);		return new PhutilOpaqueEnvelope($digest);
case PhabricatorAuthPassword::PASSWORD_TYPE_ACCOUNT:		case PhabricatorAuthPassword::PASSWORD_TYPE_ACCOUNT:
// Account passwords use this weird mess of salt and do not digest		// Account passwords previously used this weird mess of salt and did
// the input to a standard length.		// not digest the input to a standard length.

// TODO: We should build a migration pathway forward from this which		// Beyond this being a weird special case, there are two actual
// uses a better (HMAC SHA256) digest algorithm. Beyond this being		// problems with this, although neither are particularly severe:
// a weird special case, there are two actual problems with this,
// although neither are particularly severe:

// First, because we do not normalize the length of passwords, this		// First, because we do not normalize the length of passwords, this
// algorithm may make us vulnerable to DOS attacks where attacker		// algorithm may make us vulnerable to DOS attacks where an attacker
// attempt to use very long inputs to slow down hashers.		// attempts to use a very long input to slow down hashers.

// Second, because the username is part of the hash algorithm, renaming		// Second, because the username is part of the hash algorithm,
// a user breaks their password. This isn't a huge deal but it's pretty		// renaming a user breaks their password. This isn't a huge deal but
// silly. There's no security justification for this behavior, I just		// it's pretty silly. There's no security justification for this
// didn't think about the implication when I wrote it originally.		// behavior, I just didn't think about the implication when I wrote
		// it originally.

$parts = array(		$parts = array(
$this->getUsername(),		$this->getUsername(),
$envelope->openEnvelope(),		$envelope->openEnvelope(),
$this->getPHID(),		$this->getPHID(),
$password->getPasswordSalt(),		$password->getPasswordSalt(),
);		);
		amckinleyUnsubmitted Not Done Inline Actions Is the actual business-end of this getting updated in a different revision? Looks like it's going to be out of sync with the comment block above it. amckinley: Is the actual business-end of this getting updated in a different revision? Looks like it's…
		epriestleyAuthorUnsubmitted Not Done Inline Actions Out of sync in the sense of the comment saying stuff like "previously used" even though the code still exists and is reachable? The block is like "we used to do this nonsense, which is why we have this weird piece of code here for old passwords", if that makes sense? epriestley: Out of sync in the sense of the comment saying stuff like "previously used" even though the…
		amckinleyUnsubmitted Not Done Inline Actions Oh, that makes more sense. amckinley: Oh, that makes more sense.

return new PhutilOpaqueEnvelope(implode('', $parts));		return new PhutilOpaqueEnvelope(implode('', $parts));
}		}
		}

// For passwords which do not have some crazy legacy reason to use some		// For passwords which do not have some crazy legacy reason to use some
// other digest algorithm, HMAC SHA256 is an excellent choice. It satisfies		// other digest algorithm, HMAC SHA256 is an excellent choice. It satisfies
// the digest requirements and is simple.		// the digest requirements and is simple.

$digest = PhabricatorHash::digestHMACSHA256(		$digest = PhabricatorHash::digestHMACSHA256(
$envelope->openEnvelope(),		$envelope->openEnvelope(),
$password->getPasswordSalt());		$password->getPasswordSalt());

return new PhutilOpaqueEnvelope($digest);		return new PhutilOpaqueEnvelope($digest);
}		}


}		}