diff --git a/resources/sql/autopatches/20180423.mail.01.properties.sql b/resources/sql/autopatches/20180423.mail.01.properties.sql new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20180423.mail.01.properties.sql @@ -0,0 +1,8 @@ +CREATE TABLE {$NAMESPACE}_metamta.metamta_mailproperties ( + id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, + objectPHID VARBINARY(64) NOT NULL, + mailProperties LONGTEXT NOT NULL COLLATE {$COLLATE_TEXT}, + dateCreated INT UNSIGNED NOT NULL, + dateModified INT UNSIGNED NOT NULL, + UNIQUE KEY `key_object` (objectPHID) +) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT}; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -3346,6 +3346,7 @@ 'PhabricatorMailOutboundRoutingSelfEmailHeraldAction' => 'applications/metamta/herald/PhabricatorMailOutboundRoutingSelfEmailHeraldAction.php', 'PhabricatorMailOutboundRoutingSelfNotificationHeraldAction' => 'applications/metamta/herald/PhabricatorMailOutboundRoutingSelfNotificationHeraldAction.php', 'PhabricatorMailOutboundStatus' => 'applications/metamta/constants/PhabricatorMailOutboundStatus.php', + 'PhabricatorMailPropertiesDestructionEngineExtension' => 'applications/metamta/engineextension/PhabricatorMailPropertiesDestructionEngineExtension.php', 'PhabricatorMailReceiver' => 'applications/metamta/receiver/PhabricatorMailReceiver.php', 'PhabricatorMailReceiverTestCase' => 'applications/metamta/receiver/__tests__/PhabricatorMailReceiverTestCase.php', 'PhabricatorMailReplyHandler' => 'applications/metamta/replyhandler/PhabricatorMailReplyHandler.php', @@ -3403,6 +3404,8 @@ 'PhabricatorMetaMTAMailHasRecipientEdgeType' => 'applications/metamta/edge/PhabricatorMetaMTAMailHasRecipientEdgeType.php', 'PhabricatorMetaMTAMailListController' => 'applications/metamta/controller/PhabricatorMetaMTAMailListController.php', 'PhabricatorMetaMTAMailPHIDType' => 'applications/metamta/phid/PhabricatorMetaMTAMailPHIDType.php', + 'PhabricatorMetaMTAMailProperties' => 'applications/metamta/storage/PhabricatorMetaMTAMailProperties.php', + 'PhabricatorMetaMTAMailPropertiesQuery' => 'applications/metamta/query/PhabricatorMetaMTAMailPropertiesQuery.php', 'PhabricatorMetaMTAMailQuery' => 'applications/metamta/query/PhabricatorMetaMTAMailQuery.php', 'PhabricatorMetaMTAMailSearchEngine' => 'applications/metamta/query/PhabricatorMetaMTAMailSearchEngine.php', 'PhabricatorMetaMTAMailSection' => 'applications/metamta/view/PhabricatorMetaMTAMailSection.php', @@ -9038,6 +9041,7 @@ 'PhabricatorMailOutboundRoutingSelfEmailHeraldAction' => 'PhabricatorMailOutboundRoutingHeraldAction', 'PhabricatorMailOutboundRoutingSelfNotificationHeraldAction' => 'PhabricatorMailOutboundRoutingHeraldAction', 'PhabricatorMailOutboundStatus' => 'Phobject', + 'PhabricatorMailPropertiesDestructionEngineExtension' => 'PhabricatorDestructionEngineExtension', 'PhabricatorMailReceiver' => 'Phobject', 'PhabricatorMailReceiverTestCase' => 'PhabricatorTestCase', 'PhabricatorMailReplyHandler' => 'Phobject', @@ -9106,6 +9110,11 @@ 'PhabricatorMetaMTAMailHasRecipientEdgeType' => 'PhabricatorEdgeType', 'PhabricatorMetaMTAMailListController' => 'PhabricatorMetaMTAController', 'PhabricatorMetaMTAMailPHIDType' => 'PhabricatorPHIDType', + 'PhabricatorMetaMTAMailProperties' => array( + 'PhabricatorMetaMTADAO', + 'PhabricatorPolicyInterface', + ), + 'PhabricatorMetaMTAMailPropertiesQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorMetaMTAMailQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorMetaMTAMailSearchEngine' => 'PhabricatorApplicationSearchEngine', 'PhabricatorMetaMTAMailSection' => 'Phobject', diff --git a/src/applications/auth/storage/PhabricatorAuthSSHKey.php b/src/applications/auth/storage/PhabricatorAuthSSHKey.php --- a/src/applications/auth/storage/PhabricatorAuthSSHKey.php +++ b/src/applications/auth/storage/PhabricatorAuthSSHKey.php @@ -70,12 +70,6 @@ return parent::save(); } - public function getMailKey() { - // NOTE: We don't actually receive mail for these objects. It's OK for - // the mail key to be predictable until we do. - return PhabricatorHash::digestForIndex($this->getPHID()); - } - public function toPublicKey() { return PhabricatorAuthSSHPublicKey::newFromStoredKey($this); } diff --git a/src/applications/metamta/engineextension/PhabricatorMailPropertiesDestructionEngineExtension.php b/src/applications/metamta/engineextension/PhabricatorMailPropertiesDestructionEngineExtension.php new file mode 100644 --- /dev/null +++ b/src/applications/metamta/engineextension/PhabricatorMailPropertiesDestructionEngineExtension.php @@ -0,0 +1,28 @@ +getPHID(); + $viewer = $engine->getViewer(); + + $properties = id(new PhabricatorMetaMTAMailPropertiesQuery()) + ->setViewer($viewer) + ->withObjectPHIDs(array($object_phid)) + ->executeOne(); + if ($properties) { + $properties->delete(); + } + } + +} diff --git a/src/applications/metamta/management/PhabricatorMailManagementReceiveTestWorkflow.php b/src/applications/metamta/management/PhabricatorMailManagementReceiveTestWorkflow.php --- a/src/applications/metamta/management/PhabricatorMailManagementReceiveTestWorkflow.php +++ b/src/applications/metamta/management/PhabricatorMailManagementReceiveTestWorkflow.php @@ -139,8 +139,10 @@ throw new Exception(pht("No such object '%s'!", $to)); } + $mail_key = PhabricatorMetaMTAMailProperties::loadMailKey($object); + $hash = PhabricatorObjectMailReceiver::computeMailHash( - $object->getMailKey(), + $mail_key, $user->getPHID()); $header_content['to'] = $to.'+'.$user->getID().'+'.$hash.'@test.com'; diff --git a/src/applications/metamta/query/PhabricatorMetaMTAMailPropertiesQuery.php b/src/applications/metamta/query/PhabricatorMetaMTAMailPropertiesQuery.php new file mode 100644 --- /dev/null +++ b/src/applications/metamta/query/PhabricatorMetaMTAMailPropertiesQuery.php @@ -0,0 +1,51 @@ +ids = $ids; + return $this; + } + + public function withObjectPHIDs(array $object_phids) { + $this->objectPHIDs = $object_phids; + return $this; + } + + public function newResultObject() { + return new PhabricatorMetaMTAMailProperties(); + } + + protected function loadPage() { + return $this->loadStandardPage($this->newResultObject()); + } + + protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) { + $where = parent::buildWhereClauseParts($conn); + + if ($this->ids !== null) { + $where[] = qsprintf( + $conn, + 'id IN (%Ld)', + $this->ids); + } + + if ($this->objectPHIDs !== null) { + $where[] = qsprintf( + $conn, + 'objectPHID IN (%Ls)', + $this->objectPHIDs); + } + + return $where; + } + + public function getQueryApplicationClass() { + return 'PhabricatorMetaMTAApplication'; + } + +} diff --git a/src/applications/metamta/receiver/PhabricatorObjectMailReceiver.php b/src/applications/metamta/receiver/PhabricatorObjectMailReceiver.php --- a/src/applications/metamta/receiver/PhabricatorObjectMailReceiver.php +++ b/src/applications/metamta/receiver/PhabricatorObjectMailReceiver.php @@ -124,7 +124,8 @@ $check_phid = $sender->getPHID(); } - $expect_hash = self::computeMailHash($object->getMailKey(), $check_phid); + $mail_key = PhabricatorMetaMTAMailProperties::loadMailKey($object); + $expect_hash = self::computeMailHash($mail_key, $check_phid); if (!phutil_hashes_are_identical($expect_hash, $parts['hash'])) { throw new PhabricatorMetaMTAReceivedMailProcessingException( diff --git a/src/applications/metamta/receiver/__tests__/PhabricatorObjectMailReceiverTestCase.php b/src/applications/metamta/receiver/__tests__/PhabricatorObjectMailReceiverTestCase.php --- a/src/applications/metamta/receiver/__tests__/PhabricatorObjectMailReceiverTestCase.php +++ b/src/applications/metamta/receiver/__tests__/PhabricatorObjectMailReceiverTestCase.php @@ -98,8 +98,11 @@ if ($is_bad_hash) { $hash = PhabricatorObjectMailReceiver::computeMailHash('x', 'y'); } else { + + $mail_key = PhabricatorMetaMTAMailProperties::loadMailKey($task); + $hash = PhabricatorObjectMailReceiver::computeMailHash( - $task->getMailKey(), + $mail_key, $is_public ? $task->getPHID() : $user->getPHID()); } diff --git a/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php b/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php --- a/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php +++ b/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php @@ -136,8 +136,11 @@ // 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... + + $mail_key = PhabricatorMetaMTAMailProperties::loadMailKey($receiver); + $hash = PhabricatorObjectMailReceiver::computeMailHash( - $receiver->getMailKey(), + $mail_key, $receiver->getPHID()); $address = "{$prefix}{$receiver_id}+public+{$hash}@{$domain}"; @@ -159,8 +162,11 @@ $receiver = $this->getMailReceiver(); $receiver_id = $receiver->getID(); $user_id = $user->getID(); + + $mail_key = PhabricatorMetaMTAMailProperties::loadMailKey($receiver); + $hash = PhabricatorObjectMailReceiver::computeMailHash( - $receiver->getMailKey(), + $mail_key, $user->getPHID()); $domain = $this->getReplyHandlerDomain(); diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAMailProperties.php b/src/applications/metamta/storage/PhabricatorMetaMTAMailProperties.php new file mode 100644 --- /dev/null +++ b/src/applications/metamta/storage/PhabricatorMetaMTAMailProperties.php @@ -0,0 +1,91 @@ + array( + 'mailProperties' => self::SERIALIZATION_JSON, + ), + self::CONFIG_COLUMN_SCHEMA => array(), + self::CONFIG_KEY_SCHEMA => array( + 'key_object' => array( + 'columns' => array('objectPHID'), + 'unique' => true, + ), + ), + ) + parent::getConfiguration(); + } + + public function getMailProperty($key, $default = null) { + return idx($this->mailProperties, $key, $default); + } + + public function setMailProperty($key, $value) { + $this->mailProperties[$key] = $value; + return $this; + } + + public static function loadMailKey($object) { + // If this is an older object with an onboard "mailKey" property, just + // use it. + // TODO: We should eventually get rid of these and get rid of this + // piece of code. + if ($object->hasProperty('mailKey')) { + return $object->getMailKey(); + } + + $viewer = PhabricatorUser::getOmnipotentUser(); + $object_phid = $object->getPHID(); + + $properties = id(new PhabricatorMetaMTAMailPropertiesQuery()) + ->setViewer($viewer) + ->withObjectPHIDs(array($object_phid)) + ->executeOne(); + if (!$properties) { + $properties = id(new self()) + ->setObjectPHID($object_phid); + } + + $mail_key = $properties->getMailProperty('mailKey'); + if ($mail_key !== null) { + return $mail_key; + } + + $mail_key = Filesystem::readRandomCharacters(20); + $properties->setMailProperty('mailKey', $mail_key); + + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); + $properties->save(); + unset($unguarded); + + return $mail_key; + } + + +/* -( PhabricatorPolicyInterface )----------------------------------------- */ + + + public function getCapabilities() { + return array( + PhabricatorPolicyCapability::CAN_VIEW, + ); + } + + public function getPolicy($capability) { + switch ($capability) { + case PhabricatorPolicyCapability::CAN_VIEW: + return PhabricatorPolicies::POLICY_NOONE; + } + } + + public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { + return false; + } + +}