From 93e6dc1c1d692b1511bca5594005ddf093a7f6a3 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 3 Jan 2019 05:10:55 -0800
Subject: [PATCH] Upgrade object reply addresses to SHA256 and remove
 "phabricator.mail-key"

Summary:
Ref T12509.

  - Upgrade an old SHA1 to SHA256.
  - Replace an old manually configurable HMAC key with an automatically generated one.

This is generally both simpler (less configuration) and more secure (you now get a unique value automatically).

This causes a one-time compatibility break that invalidates old "Reply-To" addresses. I'll note this in the changelog.

If you leaked a bunch of addresses, you could force a change here by mucking around with `phabricator_auth.auth_hmackey`, but AFAIK no one has ever used this value to react to any sort of security issue.

(I'll note the possibility that we might want to provide/document this "manually force HMAC keys to regenerate" stuff some day in T6994.)

Test Plan: Grepped for removed config. I'll vet this pathway more heavily in upcoming changes.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T12509

Differential Revision: https://secure.phabricator.com/D19945
---
 .../check/PhabricatorExtraConfigSetupCheck.php     |  3 +++
 .../option/PhabricatorSecurityConfigOptions.php    | 14 --------------
 .../receiver/PhabricatorObjectMailReceiver.php     |  6 +++---
 .../configuring_inbound_email.diviner              |  3 ---
 4 files changed, 6 insertions(+), 20 deletions(-)

diff --git a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php
index 708bbae0cb..19ced06efc 100644
--- a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php
+++ b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php
@@ -385,6 +385,9 @@ final class PhabricatorExtraConfigSetupCheck extends PhabricatorSetupCheck {
         'Mail thread IDs are now generated automatically.'),
       'metamta.placeholder-to-recipient' => pht(
         'Placeholder recipients are now generated automatically.'),
+
+      'metamta.mail-key' => pht(
+        'Mail object address hash keys are now generated automatically.'),
     );
 
     return $ancient_config;
diff --git a/src/applications/config/option/PhabricatorSecurityConfigOptions.php b/src/applications/config/option/PhabricatorSecurityConfigOptions.php
index 2b511ee023..2d82cf5f0f 100644
--- a/src/applications/config/option/PhabricatorSecurityConfigOptions.php
+++ b/src/applications/config/option/PhabricatorSecurityConfigOptions.php
@@ -169,20 +169,6 @@ EOTEXT
             'in a vague, mostly theoretical way. But it will take you like 3 '.
             'seconds of mashing on your keyboard to set it up so you might '.
             'as well.')),
-       $this->newOption(
-         'phabricator.mail-key',
-         'string',
-         '5ce3e7e8787f6e40dfae861da315a5cdf1018f12')
-        ->setHidden(true)
-        ->setSummary(
-          pht('Hashed with other inputs to generate mail tokens.'))
-        ->setDescription(
-          pht(
-            "This is hashed with other inputs to generate mail tokens. If ".
-            "you want, you can change it to some other string which is ".
-            "unique to your install. In particular, you will want to do ".
-            "this if you accidentally send a bunch of mail somewhere you ".
-            "shouldn't have, to invalidate all old reply-to addresses.")),
        $this->newOption(
         'uri.allowed-protocols',
         'set',
diff --git a/src/applications/metamta/receiver/PhabricatorObjectMailReceiver.php b/src/applications/metamta/receiver/PhabricatorObjectMailReceiver.php
index 801f7a8e6f..730ed6a2c5 100644
--- a/src/applications/metamta/receiver/PhabricatorObjectMailReceiver.php
+++ b/src/applications/metamta/receiver/PhabricatorObjectMailReceiver.php
@@ -200,9 +200,9 @@ abstract class PhabricatorObjectMailReceiver extends PhabricatorMailReceiver {
   }
 
   public static function computeMailHash($mail_key, $phid) {
-    $global_mail_key = PhabricatorEnv::getEnvConfig('phabricator.mail-key');
-
-    $hash = PhabricatorHash::weakDigest($mail_key.$global_mail_key.$phid);
+    $hash = PhabricatorHash::digestWithNamedKey(
+      $mail_key.$phid,
+      'mail.object-address-key');
     return substr($hash, 0, 16);
   }
 
diff --git a/src/docs/user/configuration/configuring_inbound_email.diviner b/src/docs/user/configuration/configuring_inbound_email.diviner
index 869196e9d8..84d4fa48d1 100644
--- a/src/docs/user/configuration/configuring_inbound_email.diviner
+++ b/src/docs/user/configuration/configuring_inbound_email.diviner
@@ -79,9 +79,6 @@ authenticating senders in the general case (e.g., where you are an open source
 project and need to interact with users whose email accounts you have no control
 over).
 
-If you leak a bunch of reply-to addresses by accident, you can change
-`phabricator.mail-key` in your configuration to invalidate all the old hashes.
-
 You can also set `metamta.public-replies`, which will change how Phabricator
 delivers email. Instead of sending each recipient a unique mail with a personal
 reply-to address, it will send a single email to everyone with a public reply-to