1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-20 20:40:56 +01:00

Replace callsites to sha1() that use it to asciify entropy with

Filesystem::readRandomCharacters()

Summary: See T547. To improve auditability of use of crypto-sensitive hash
functions, use Filesystem::readRandomCharacters() in place of
sha1(Filesystem::readRandomBytes()) when we're just generating random ASCII
strings.

Test Plan:
  - Generated a new PHID.
  - Logged out and logged back in (to test sessions).
  - Regenerated Conduit certificate.
  - Created a new task, verified mail key generated sensibly.
  - Created a new revision, verified mail key generated sensibly.
  - Ran "arc list", got blocked, installed new certificate, ran "arc list"
again.

Reviewers: jungejason, nh, tuomaspelkonen, aran, benmathews

Reviewed By: jungejason

CC: aran, epriestley, jungejason

Differential Revision: 1000
This commit is contained in:
epriestley 2011-10-10 19:22:30 -07:00
parent abb39d06a2
commit 661f077bf7
6 changed files with 7 additions and 14 deletions

View file

@ -40,7 +40,7 @@ class PhabricatorConduitTokenController extends PhabricatorConduitController {
$token = id(new PhabricatorConduitCertificateToken()) $token = id(new PhabricatorConduitCertificateToken())
->setUserPHID($user->getPHID()) ->setUserPHID($user->getPHID())
->setToken(sha1(Filesystem::readRandomBytes(128))) ->setToken(Filesystem::readRandomCharacters(40))
->save(); ->save();
$panel = new AphrontPanelView(); $panel = new AphrontPanelView();

View file

@ -117,7 +117,7 @@ class DifferentialRevision extends DifferentialDAO {
public function save() { public function save() {
if (!$this->getMailKey()) { if (!$this->getMailKey()) {
$this->mailKey = sha1(Filesystem::readRandomBytes(20)); $this->mailKey = Filesystem::readRandomCharacters(40);
} }
return parent::save(); return parent::save();
} }

View file

@ -47,7 +47,7 @@ final class PhabricatorS3FileStorageEngine
public function writeFile($data, array $params) { public function writeFile($data, array $params) {
$s3 = $this->newS3API(); $s3 = $this->newS3API();
$name = 'phabricator/'.sha1(Filesystem::readRandomBytes(20)); $name = 'phabricator/'.Filesystem::readRandomCharacters(20);
AphrontWriteGuard::willWrite(); AphrontWriteGuard::willWrite();
$s3->putObject( $s3->putObject(

View file

@ -123,7 +123,7 @@ class ManiphestTask extends ManiphestDAO {
public function save() { public function save() {
if (!$this->mailKey) { if (!$this->mailKey) {
$this->mailKey = sha1(Filesystem::readRandomBytes(20)); $this->mailKey = Filesystem::readRandomCharacters(20);
} }
$result = parent::save(); $result = parent::save();

View file

@ -97,10 +97,7 @@ class PhabricatorUser extends PhabricatorUserDAO {
} }
private function generateConduitCertificate() { private function generateConduitCertificate() {
$entropy = Filesystem::readRandomBytes(256); return Filesystem::readRandomCharacters(255);
$entropy = base64_encode($entropy);
$entropy = substr($entropy, 0, 255);
return $entropy;
} }
public function comparePassword($password) { public function comparePassword($password) {
@ -259,8 +256,7 @@ class PhabricatorUser extends PhabricatorUserDAO {
// Consume entropy to generate a new session key, forestalling the eventual // Consume entropy to generate a new session key, forestalling the eventual
// heat death of the universe. // heat death of the universe.
$entropy = Filesystem::readRandomBytes(20); $session_key = Filesystem::readRandomCharacters(40);
$session_key = sha1($entropy);
// UNGUARDED WRITES: Logging-in users don't have CSRF stuff yet. // UNGUARDED WRITES: Logging-in users don't have CSRF stuff yet.
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();

View file

@ -31,10 +31,7 @@ class PhabricatorPHID extends PhabricatorPHIDDAO {
throw new Exception("Can not generate PHID with no type."); throw new Exception("Can not generate PHID with no type.");
} }
$entropy = Filesystem::readRandomBytes(20); $uniq = Filesystem::readRandomCharacters(20);
$uniq = sha1($entropy);
$uniq = substr($uniq, 0, 20);
$phid = 'PHID-'.$type.'-'.$uniq; $phid = 'PHID-'.$type.'-'.$uniq;
$phid_rec = new PhabricatorPHID(); $phid_rec = new PhabricatorPHID();