diff --git a/conf/default.conf.php b/conf/default.conf.php index a90b9738a8..fcd09e4898 100644 --- a/conf/default.conf.php +++ b/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 ------------------------------------------------------------------ // diff --git a/src/applications/differential/replyhandler/DifferentialReplyHandler.php b/src/applications/differential/replyhandler/DifferentialReplyHandler.php index ae7be54e1b..516ebd453c 100644 --- a/src/applications/differential/replyhandler/DifferentialReplyHandler.php +++ b/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'); diff --git a/src/applications/maniphest/replyhandler/ManiphestReplyHandler.php b/src/applications/maniphest/replyhandler/ManiphestReplyHandler.php index b86f5685f7..c3f8cfe1c9 100644 --- a/src/applications/maniphest/replyhandler/ManiphestReplyHandler.php +++ b/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'); diff --git a/src/applications/metamta/replyhandler/base/PhabricatorMailReplyHandler.php b/src/applications/metamta/replyhandler/base/PhabricatorMailReplyHandler.php index f8ab23fa3a..8b2d932bb4 100644 --- a/src/applications/metamta/replyhandler/base/PhabricatorMailReplyHandler.php +++ b/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) { diff --git a/src/applications/metamta/replyhandler/base/__init__.php b/src/applications/metamta/replyhandler/base/__init__.php index 5ff3e91560..145c5f6fd3 100644 --- a/src/applications/metamta/replyhandler/base/__init__.php +++ b/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'); diff --git a/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php b/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php index cdd978feef..1ddc36c6e5 100644 --- a/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php +++ b/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php @@ -58,7 +58,7 @@ class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO { // "some display name" $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" '. + $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(); } diff --git a/src/docs/configuration/configuring_inbound_email.diviner b/src/docs/configuration/configuring_inbound_email.diviner index ebc524b738..981b6a9512 100644 --- a/src/docs/configuration/configuring_inbound_email.diviner +++ b/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):