Use a proper entropy source to generate file keys

Summary:
See T549. Under configurations where files are served from an alternate domain
which does not have cookie credentials, we use random keys to prevent browsing,
similar to how Facebook relies on pseudorandom information in image URIs (we
could some day go farther than this and generate file sessions on the alternate
domain or something, I guess).

Currently, we generate these random keys in a roundabout manner. Instead, use a
real entropy source and store the key on the object. This reduces the number of
sha1() calls in the codebase as per T547.

Test Plan: Ran upgrade scripts, verified database was populated correctly.
Configured alternate file domain, uploaded file, verified secret generated and
worked properly. Changed secret, was given 404.

Reviewers: jungejason, benmathews, nh, tuomaspelkonen, aran

Reviewed By: aran

CC: aran, epriestley

Differential Revision: 1036

conf/default.conf.php

@@ -406,12 +406,6 @@ return array(
   // addresses.
   'phabricator.mail-key'        => '5ce3e7e8787f6e40dfae861da315a5cdf1018f12',
 
-
-  // This is hashed with other inputs to generate file secret keys. Changing
-  // it will invalidate all file URIs if you have an alternate file domain
-  // configured (see 'security.alternate-file-domain').
-  'phabricator.file-key'        => 'ade8dadc8b4382067069a4d4798112191af8a190',
-
   // Version string displayed in the footer. You probably should leave this
   // alone.
   'phabricator.version'         => 'UNSTABLE',

resources/sql/patches/080.filekeys.sql

@@ -0,0 +1,2 @@
+ALTER TABLE phabricator_file.file
+  ADD secretKey VARCHAR(20) BINARY;

resources/sql/patches/081.filekeys.php

@@ -0,0 +1,31 @@
+<?php
+
+/*
+ * Copyright 2011 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+echo "Generating file keys...\n";
+$files = id(new PhabricatorFile())->loadAllWhere('secretKey IS NULL');
+echo count($files).' files to generate keys for';
+foreach ($files as $file) {
+  queryfx(
+    $file->establishConnection('r'),
+    'UPDATE %T SET secretKey = %s WHERE id = %d',
+    $file->getTableName(),
+    $file->generateSecretKey(),
+    $file->getID());
+  echo '.';
+}
+echo "\nDone.\n";

src/applications/files/controller/altview/PhabricatorFileAltViewController.php

@@ -61,6 +61,7 @@ class PhabricatorFileAltViewController extends PhabricatorFileController {
     $data = $file->loadFileData();
     $response = new AphrontFileResponse();
     $response->setContent($data);
+    $response->setMimeType($file->getMimeType());
     $response->setCacheDurationInSeconds(60 * 60 * 24 * 30);
 
     return $response;

src/applications/files/storage/file/PhabricatorFile.php

@@ -25,6 +25,7 @@ class PhabricatorFile extends PhabricatorFileDAO {
   protected $mimeType;
   protected $byteSize;
   protected $authorPHID;
+  protected $secretKey;
 
   protected $storageEngine;
   protected $storageFormat;
@@ -222,9 +223,14 @@ class PhabricatorFile extends PhabricatorFileDAO {
   }
 
   public function getViewURI() {
+    if (!$this->getPHID()) {
+      throw new Exception(
+        "You must save a file before you can generate a view URI.");
+    }
+
     $alt = PhabricatorEnv::getEnvConfig('security.alternate-file-domain');
     if ($alt) {
-      $path = '/file/alt/'.$this->generateSecretKey().'/'.$this->getPHID().'/';
+      $path = '/file/alt/'.$this->getSecretKey().'/'.$this->getPHID().'/';
       $uri = new PhutilURI($alt);
       $uri->setPath($path);
 
@@ -318,13 +324,17 @@ class PhabricatorFile extends PhabricatorFileDAO {
   }
 
   public function validateSecretKey($key) {
-    return ($key == $this->generateSecretKey());
+    return ($key == $this->getSecretKey());
   }
 
-  private function generateSecretKey() {
-    $file_key = PhabricatorEnv::getEnvConfig('phabricator.file-key');
-    $hash = sha1($this->phid.$this->storageHandle.$file_key);
-    return substr($hash, 0, 20);
+  public function save() {
+    if (!$this->getSecretKey()) {
+      $this->setSecretKey($this->generateSecretKey());
+    }
+    return parent::save();
   }
 
+  public function generateSecretKey() {
+    return Filesystem::readRandomCharacters(20);
+  }
 }