Allow Phabricator to be configured to use a public Reply-To address

Summary:
We already support this (and Facebook uses it) but it is difficult to configure
and you have to write a bunch of code. Instead, provide a simple flag.

See the documentation changes for details, but when this flag is enabled we send
one email with a reply-to like "D2+public+23hf91fh19fh@phabricator.example.com".
Anyone can reply to this, and we figure out who they are based on their "From"
address instead of a unique hash. This is less secure, but a reasonable tradeoff
in many cases.

This also has the advantage over a naive implementation of at least doing object
hash validation.

@jungejason: I don't think this affects Facebook's implementation but this is an
area where we've had problems in the past, so watch out for it when you deploy.
Also note that you must set "metamta.public-replies" to true since Maniphest now
looks for that key specifically before going into public reply mode; it no
longer just tests for a public reply address being generateable (since it can
always generate one now).

Test Plan:
Swapped my local install in and out of public reply mode and commented on
objects. Got expected email behavior. Replied to public and private email
addresses.

Attacked public addresses by using them when the install was configured to
disallow them and by altering the hash and the from address. All this stuff was
rejected.

Reviewed By: jungejason
Reviewers: moskov, jungejason, tuomaspelkonen, aran
CC: aran, epriestley, moskov, jungejason
Differential Revision: 563

conf/default.conf.php

@@ -190,6 +190,17 @@ return array(
   // Prefix prepended to mail sent by Differential.
   'metamta.differential.subject-prefix' => '[Differential]',
 
+  // By default, Phabricator generates unique reply-to addresses and sends a
+  // separate email to each recipient when you enable reply handling. This is
+  // more secure than using "From" to establish user identity, but can mean
+  // users may receive multiple emails when they are on mailing lists. Instead,
+  // you can use a single, non-unique reply to address and authenticate users
+  // based on the "From" address by setting this to 'true'. This trades away
+  // a little bit of security for convenience, but it's reasonable in many
+  // installs. Object interactions are still protected using hashes in the
+  // single public email address, so objects can not be replied to blindly.
+  'metamta.public-replies' => false,
+
 
 // -- Auth ------------------------------------------------------------------ //
 

src/applications/differential/replyhandler/DifferentialReplyHandler.php

@@ -31,6 +31,10 @@ class DifferentialReplyHandler extends PhabricatorMailReplyHandler {
     return $this->getDefaultPrivateReplyHandlerEmailAddress($handle, 'D');
   }
 
+  public function getPublicReplyHandlerEmailAddress() {
+    return $this->getDefaultPublicReplyHandlerEmailAddress('D');
+  }
+
   public function getReplyHandlerDomain() {
     return PhabricatorEnv::getEnvConfig(
       'metamta.differential.reply-handler-domain');

src/applications/maniphest/replyhandler/ManiphestReplyHandler.php

@@ -29,6 +29,10 @@ class ManiphestReplyHandler extends PhabricatorMailReplyHandler {
     return $this->getDefaultPrivateReplyHandlerEmailAddress($handle, 'T');
   }
 
+  public function getPublicReplyHandlerEmailAddress() {
+    return $this->getDefaultPublicReplyHandlerEmailAddress('T');
+  }
+
   public function getReplyHandlerDomain() {
     return PhabricatorEnv::getEnvConfig(
       'metamta.maniphest.reply-handler-domain');

src/applications/metamta/replyhandler/base/PhabricatorMailReplyHandler.php

@@ -48,12 +48,20 @@ abstract class PhabricatorMailReplyHandler {
   abstract public function receiveEmail(PhabricatorMetaMTAReceivedMail $mail);
 
   public function supportsPrivateReplies() {
-    return (bool)$this->getReplyHandlerDomain();
+    return (bool)$this->getReplyHandlerDomain() &&
+           !$this->supportsPublicReplies();
+  }
+
+  public function supportsPublicReplies() {
+    if (!PhabricatorEnv::getEnvConfig('metamta.public-replies')) {
+      return false;
+    }
+    return (bool)$this->getPublicReplyHandlerEmailAddress();
   }
 
   final public function supportsReplies() {
     return $this->supportsPrivateReplies() ||
-           (bool)$this->getPublicReplyHandlerEmailAddress();
+           $this->supportsPublicReplies();
   }
 
   public function getPublicReplyHandlerEmailAddress() {
@@ -145,6 +153,22 @@ abstract class PhabricatorMailReplyHandler {
     return implode(', ', $list);
   }
 
+  protected function getDefaultPublicReplyHandlerEmailAddress($prefix) {
+
+    $receiver = $this->getMailReceiver();
+    $receiver_id = $receiver->getID();
+    $domain = $this->getReplyHandlerDomain();
+
+    // We compute a hash using the object's own PHID to prevent an attacker
+    // from blindly interacting with objects that they haven't ever received
+    // mail about by just sending to D1@, D2@, etc...
+    $hash = PhabricatorMetaMTAReceivedMail::computeMailHash(
+      $receiver->getMailKey(),
+      $receiver->getPHID());
+
+    return "{$prefix}{$receiver_id}+public+{$hash}@{$domain}";
+  }
+
   protected function getDefaultPrivateReplyHandlerEmailAddress(
     PhabricatorObjectHandle $handle,
     $prefix) {

src/applications/metamta/replyhandler/base/__init__.php

@@ -8,6 +8,7 @@
 
 phutil_require_module('phabricator', 'applications/metamta/storage/receivedmail');
 phutil_require_module('phabricator', 'applications/phid/constants');
+phutil_require_module('phabricator', 'infrastructure/env');
 
 phutil_require_module('phutil', 'utils');
 

src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php

@@ -58,7 +58,7 @@ class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO {
     //    "some display name" <D1+xyz+asdf@example.com>
     $matches = null;
     $ok = preg_match(
-      '/(?:^|<)((?:D|T)\d+)\+(\d+)\+([a-f0-9]{16})@/U',
+      '/(?:^|<)((?:D|T)\d+)\+([\w]+)\+([a-f0-9]{16})@/U',
       $to,
       $matches);
 
@@ -70,9 +70,35 @@ class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO {
     $user_id = $matches[2];
     $hash = $matches[3];
 
-    $user = id(new PhabricatorUser())->load($user_id);
-    if (!$user) {
-      return $this->setMessage("Invalid user '{$user_id}'")->save();
+    if ($user_id == 'public') {
+      if (!PhabricatorEnv::getEnvConfig('metamta.public-replies')) {
+        return $this->setMessage("Public replies not enabled.")->save();
+      }
+
+      // Strip the email address out of the 'from' if necessary, since it might
+      // have some formatting like '"Abraham Lincoln" <alincoln@logcab.in>'.
+      $from = idx($this->headers, 'from');
+      $matches = null;
+      $ok = preg_match('/<(.*)>/', $from, $matches);
+      if ($ok) {
+        $from = $matches[1];
+      }
+
+      $user = id(new PhabricatorUser())->loadOneWhere(
+        'email = %s',
+        $from);
+      if (!$user) {
+        return $this->setMessage("Invalid public user '{$from}'.")->save();
+      }
+
+      $use_user_hash = false;
+    } else {
+      $user = id(new PhabricatorUser())->load($user_id);
+      if (!$user) {
+        return $this->setMessage("Invalid private user '{$user_id}'.")->save();
+      }
+
+      $use_user_hash = true;
     }
 
     if ($user->getIsDisabled()) {
@@ -88,9 +114,17 @@ class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO {
 
     $this->setRelatedPHID($receiver->getPHID());
 
-    $expect_hash = self::computeMailHash(
-      $receiver->getMailKey(),
-      $user->getPHID());
+    if ($use_user_hash) {
+      // This is a private reply-to address, check that the user hash is
+      // correct.
+      $check_phid = $user->getPHID();
+    } else {
+      // This is a public reply-to address, check that the object hash is
+      // correct.
+      $check_phid = $receiver->getPHID();
+    }
+
+    $expect_hash = self::computeMailHash($receiver->getMailKey(), $check_phid);
     if ($expect_hash != $hash) {
       return $this->setMessage("Invalid mail hash!")->save();
     }

src/docs/configuration/configuring_inbound_email.diviner

@@ -65,6 +65,15 @@ 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
+address. This decreases security because anyone who can spoof a "From" address
+can act as another user, but increases convenience if you use mailing lists and,
+practically, is a reasonable setting for many installs. The reply-to address
+will still contain a hash unique to the object it represents, so users who have
+not received an email about an object can not blindly interact with it.
+
 NOTE: Phabricator does not currently attempt to verify "From" addresses because
 this is technically complex, seems unreasonably difficult in the general case,
 and no installs have had a need for it yet. If you have a specific case where a
@@ -190,11 +199,11 @@ content of the email to Phabricator:
   import logging, subprocess
   from lamson.routing import route, stateless
   from lamson import view
-  
+
   PHABRICATOR_ROOT = "/path/to/phabricator"
   PHABRICATOR_ENV = "custom/myconf"
   LOGGING_ENABLED = True
-  
+
   @route("(address)@(host)", address=".+")
   @stateless
   def START(message, address=None, host=None):