From 7b2b5cd91e018f3479a20056da79a82dd1008d74 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 1 Feb 2018 06:16:32 -0800 Subject: [PATCH] Add basic support for a "Must Encrypt" mail flag which prevents unsecured content transmission Summary: Ref T13053. See PHI291. For particularly sensitive objects (like security issues), installs may reasonably wish to prevent details from being sent in plaintext over email. This adds a "Must Encrypt" mail behavior, which discards mail content and all identifying details, replacing it with a link to the `/mail/` application. Users can follow the link to view the message over HTTPS. The flag discards body content, attachments, and headers which imply things about the content of the object. It retains threading headers and headers which may uniquely identify the object as long as they don't disclose anyting about the content. The `bin/mail list-outbound` command now flags these messages with a `#` mark. The `bin/mail show-outbound` command now shows sent/suppressed headers and the body content as delivered (if it differs from the original body content). The `/mail/` web UI now shows a tag for messages marked with this flag. For now, there is no way to actually set this flag on mail. Test Plan: - Forced this flag on, made comments and took actions to send mail. - Reviewed mail with `bin/mail` and `/mail/` in the web UI, saw all content information omitted. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13053 Differential Revision: https://secure.phabricator.com/D18983 --- .../PhabricatorMetaMTAMailViewController.php | 17 ++ ...atorMailManagementListOutboundWorkflow.php | 2 + ...atorMailManagementShowOutboundWorkflow.php | 57 +++++-- .../storage/PhabricatorMetaMTAMail.php | 146 ++++++++++++++++-- 4 files changed, 197 insertions(+), 25 deletions(-) diff --git a/src/applications/metamta/controller/PhabricatorMetaMTAMailViewController.php b/src/applications/metamta/controller/PhabricatorMetaMTAMailViewController.php index 80da535a9e..20bbc425b5 100644 --- a/src/applications/metamta/controller/PhabricatorMetaMTAMailViewController.php +++ b/src/applications/metamta/controller/PhabricatorMetaMTAMailViewController.php @@ -32,6 +32,23 @@ final class PhabricatorMetaMTAMailViewController $color = PhabricatorMailOutboundStatus::getStatusColor($status); $header->setStatus($icon, $color, $name); + if ($mail->getMustEncrypt()) { + Javelin::initBehavior('phabricator-tooltips'); + $header->addTag( + id(new PHUITagView()) + ->setType(PHUITagView::TYPE_SHADE) + ->setColor('blue') + ->setName(pht('Must Encrypt')) + ->setIcon('fa-shield blue') + ->addSigil('has-tooltip') + ->setMetadata( + array( + 'tip' => pht( + 'Message content can only be transmitted over secure '. + 'channels.'), + ))); + } + $crumbs = $this->buildApplicationCrumbs() ->addTextCrumb(pht('Mail %d', $mail->getID())) ->setBorder(true); diff --git a/src/applications/metamta/management/PhabricatorMailManagementListOutboundWorkflow.php b/src/applications/metamta/management/PhabricatorMailManagementListOutboundWorkflow.php index b4aa76379e..a83dafb0a8 100644 --- a/src/applications/metamta/management/PhabricatorMailManagementListOutboundWorkflow.php +++ b/src/applications/metamta/management/PhabricatorMailManagementListOutboundWorkflow.php @@ -37,6 +37,7 @@ final class PhabricatorMailManagementListOutboundWorkflow $table = id(new PhutilConsoleTable()) ->setShowHeader(false) ->addColumn('id', array('title' => pht('ID'))) + ->addColumn('encrypt', array('title' => pht('#'))) ->addColumn('status', array('title' => pht('Status'))) ->addColumn('subject', array('title' => pht('Subject'))); @@ -45,6 +46,7 @@ final class PhabricatorMailManagementListOutboundWorkflow $table->addRow(array( 'id' => $mail->getID(), + 'encrypt' => ($mail->getMustEncrypt() ? '#' : ' '), 'status' => PhabricatorMailOutboundStatus::getStatusName($status), 'subject' => $mail->getSubject(), )); diff --git a/src/applications/metamta/management/PhabricatorMailManagementShowOutboundWorkflow.php b/src/applications/metamta/management/PhabricatorMailManagementShowOutboundWorkflow.php index 54a91861ae..0fc7dd14b9 100644 --- a/src/applications/metamta/management/PhabricatorMailManagementShowOutboundWorkflow.php +++ b/src/applications/metamta/management/PhabricatorMailManagementShowOutboundWorkflow.php @@ -79,7 +79,7 @@ final class PhabricatorMailManagementShowOutboundWorkflow $info = array(); - $info[] = pht('PROPERTIES'); + $info[] = $this->newSectionHeader(pht('PROPERTIES')); $info[] = pht('ID: %d', $message->getID()); $info[] = pht('Status: %s', $message->getStatus()); $info[] = pht('Related PHID: %s', $message->getRelatedPHID()); @@ -87,15 +87,17 @@ final class PhabricatorMailManagementShowOutboundWorkflow $ignore = array( 'body' => true, + 'body.sent' => true, 'html-body' => true, 'headers' => true, 'attachments' => true, 'headers.sent' => true, + 'headers.unfiltered' => true, 'authors.sent' => true, ); $info[] = null; - $info[] = pht('PARAMETERS'); + $info[] = $this->newSectionHeader(pht('PARAMETERS')); $parameters = $message->getParameters(); foreach ($parameters as $key => $value) { if (isset($ignore[$key])) { @@ -110,22 +112,40 @@ final class PhabricatorMailManagementShowOutboundWorkflow } $info[] = null; - $info[] = pht('HEADERS'); + $info[] = $this->newSectionHeader(pht('HEADERS')); $headers = $message->getDeliveredHeaders(); - if (!$headers) { + $unfiltered = $message->getUnfilteredHeaders(); + if (!$unfiltered) { $headers = $message->generateHeaders(); + $unfiltered = $headers; } + $header_map = array(); foreach ($headers as $header) { list($name, $value) = $header; - $info[] = "{$name}: {$value}"; + $header_map[$name.':'.$value] = true; + } + + foreach ($unfiltered as $header) { + list($name, $value) = $header; + $was_sent = isset($header_map[$name.':'.$value]); + + if ($was_sent) { + $marker = ' '; + } else { + $marker = '#'; + } + + $info[] = "{$marker} {$name}: {$value}"; } $attachments = idx($parameters, 'attachments'); if ($attachments) { $info[] = null; - $info[] = pht('ATTACHMENTS'); + + $info[] = $this->newSectionHeader(pht('ATTACHMENTS')); + foreach ($attachments as $attachment) { $info[] = idx($attachment, 'filename', pht('Unnamed File')); } @@ -136,7 +156,9 @@ final class PhabricatorMailManagementShowOutboundWorkflow $actors = $message->getDeliveredActors(); if ($actors) { $info[] = null; - $info[] = pht('RECIPIENTS'); + + $info[] = $this->newSectionHeader(pht('RECIPIENTS')); + foreach ($actors as $actor_phid => $actor_info) { $actor = idx($all_actors, $actor_phid); if ($actor) { @@ -162,15 +184,22 @@ final class PhabricatorMailManagementShowOutboundWorkflow } $info[] = null; - $info[] = pht('TEXT BODY'); + $info[] = $this->newSectionHeader(pht('TEXT BODY')); if (strlen($message->getBody())) { - $info[] = $message->getBody(); + $info[] = tsprintf('%B', $message->getBody()); } else { $info[] = pht('(This message has no text body.)'); } + $delivered_body = $message->getDeliveredBody(); + if ($delivered_body !== null) { + $info[] = null; + $info[] = $this->newSectionHeader(pht('BODY AS DELIVERED'), true); + $info[] = tsprintf('%B', $delivered_body); + } + $info[] = null; - $info[] = pht('HTML BODY'); + $info[] = $this->newSectionHeader(pht('HTML BODY')); if (strlen($message->getHTMLBody())) { $info[] = $message->getHTMLBody(); $info[] = null; @@ -186,4 +215,12 @@ final class PhabricatorMailManagementShowOutboundWorkflow } } + private function newSectionHeader($label, $emphasize = false) { + if ($emphasize) { + return tsprintf('** %s **', $label); + } else { + return tsprintf('** %s **', $label); + } + } + } diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php index 20b1482036..c203e86530 100644 --- a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php +++ b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php @@ -21,7 +21,10 @@ final class PhabricatorMetaMTAMail public function __construct() { $this->status = PhabricatorMailOutboundStatus::STATUS_QUEUE; - $this->parameters = array('sensitive' => true); + $this->parameters = array( + 'sensitive' => true, + 'mustEncrypt' => false, + ); parent::__construct(); } @@ -247,6 +250,15 @@ final class PhabricatorMetaMTAMail return $this->getParam('sensitive', true); } + public function setMustEncrypt($bool) { + $this->setParam('mustEncrypt', $bool); + return $this; + } + + public function getMustEncrypt() { + return $this->getParam('mustEncrypt', false); + } + public function setHTMLBody($html) { $this->setParam('html-body', $html); return $this; @@ -431,6 +443,7 @@ final class PhabricatorMetaMTAMail unset($params['is-first-message']); $is_threaded = (bool)idx($params, 'thread-id'); + $must_encrypt = $this->getMustEncrypt(); $reply_to_name = idx($params, 'reply-to-name', ''); unset($params['reply-to-name']); @@ -502,6 +515,11 @@ final class PhabricatorMetaMTAMail mpull($cc_actors, 'getEmailAddress')); break; case 'attachments': + // If the mail content must be encrypted, don't add attachments. + if ($must_encrypt) { + break; + } + $value = $this->getAttachments(); foreach ($value as $attachment) { $mailer->addAttachment( @@ -521,14 +539,20 @@ final class PhabricatorMetaMTAMail $subject[] = trim(idx($params, 'subject-prefix')); - $vary_prefix = idx($params, 'vary-subject-prefix'); - if ($vary_prefix != '') { - if ($this->shouldVarySubject($preferences)) { - $subject[] = $vary_prefix; + // If mail content must be encrypted, we replace the subject with + // a generic one. + if ($must_encrypt) { + $subject[] = pht('Object Updated'); + } else { + $vary_prefix = idx($params, 'vary-subject-prefix'); + if ($vary_prefix != '') { + if ($this->shouldVarySubject($preferences)) { + $subject[] = $vary_prefix; + } } - } - $subject[] = $value; + $subject[] = $value; + } $mailer->setSubject(implode(' ', array_filter($subject))); break; @@ -567,7 +591,22 @@ final class PhabricatorMetaMTAMail } } - $body = idx($params, 'body', ''); + $raw_body = idx($params, 'body', ''); + $body = $raw_body; + if ($must_encrypt) { + $parts = array(); + $parts[] = pht( + 'The content for this message can only be transmitted over a '. + 'secure channel. To view the message content, follow this '. + 'link:'); + + $parts[] = PhabricatorEnv::getProductionURI($this->getURI()); + + $body = implode("\n\n", $parts); + } else { + $body = $raw_body; + } + $max = PhabricatorEnv::getEnvConfig('metamta.email-body-limit'); if (strlen($body) > $max) { $body = id(new PhutilUTF8StringTruncator()) @@ -578,18 +617,32 @@ final class PhabricatorMetaMTAMail } $mailer->setBody($body); - $html_emails = $this->shouldSendHTML($preferences); - if ($html_emails && isset($params['html-body'])) { + // If we sent a different message body than we were asked to, record + // what we actually sent to make debugging and diagnostics easier. + if ($body !== $raw_body) { + $this->setParam('body.sent', $body); + } + + if ($must_encrypt) { + $send_html = false; + } else { + $send_html = $this->shouldSendHTML($preferences); + } + + if ($send_html && isset($params['html-body'])) { $mailer->setHTMLBody($params['html-body']); } // Pass the headers to the mailer, then save the state so we can show - // them in the web UI. - foreach ($headers as $header) { + // them in the web UI. If the mail must be encrypted, we remove headers + // which are not on a strict whitelist to avoid disclosing information. + $filtered_headers = $this->filterHeaders($headers, $must_encrypt); + foreach ($filtered_headers as $header) { list($header_key, $header_value) = $header; $mailer->addHeader($header_key, $header_value); } - $this->setParam('headers.sent', $headers); + $this->setParam('headers.unfiltered', $headers); + $this->setParam('headers.sent', $filtered_headers); // Save the final deliverability outcomes and reasoning so we can // explain why things happened the way they did. @@ -1002,8 +1055,6 @@ final class PhabricatorMetaMTAMail // Some clients respect this to suppress OOF and other auto-responses. $headers[] = array('X-Auto-Response-Suppress', 'All'); - // If the message has mailtags, filter out any recipients who don't want - // to receive this type of mail. $mailtags = $this->getParam('mailtags'); if ($mailtags) { $tag_header = array(); @@ -1028,6 +1079,10 @@ final class PhabricatorMetaMTAMail $headers[] = array('Precedence', 'bulk'); } + if ($this->getMustEncrypt()) { + $headers[] = array('X-Phabricator-Must-Encrypt', 'Yes'); + } + return $headers; } @@ -1035,6 +1090,19 @@ final class PhabricatorMetaMTAMail return $this->getParam('headers.sent'); } + public function getUnfilteredHeaders() { + $unfiltered = $this->getParam('headers.unfiltered'); + + if ($unfiltered === null) { + // Older versions of Phabricator did not filter headers, and thus did + // not record unfiltered headers. If we don't have unfiltered header + // data just return the delivered headers for compatibility. + return $this->getDeliveredHeaders(); + } + + return $unfiltered; + } + public function getDeliveredActors() { return $this->getParam('actors.sent'); } @@ -1047,6 +1115,54 @@ final class PhabricatorMetaMTAMail return $this->getParam('routingmap.sent'); } + public function getDeliveredBody() { + return $this->getParam('body.sent'); + } + + private function filterHeaders(array $headers, $must_encrypt) { + if (!$must_encrypt) { + return $headers; + } + + $whitelist = array( + 'In-Reply-To', + 'Message-ID', + 'Precedence', + 'References', + 'Thread-Index', + + 'X-Mail-Transport-Agent', + 'X-Auto-Response-Suppress', + + 'X-Phabricator-Sent-This-Message', + 'X-Phabricator-Must-Encrypt', + ); + + // NOTE: The major header we want to drop is "X-Phabricator-Mail-Tags". + // This header contains a significant amount of meaningful information + // about the object. + + $whitelist_map = array(); + foreach ($whitelist as $term) { + $whitelist_map[phutil_utf8_strtolower($term)] = true; + } + + foreach ($headers as $key => $header) { + list($name, $value) = $header; + $name = phutil_utf8_strtolower($name); + + if (!isset($whitelist_map[$name])) { + unset($headers[$key]); + } + } + + return $headers; + } + + public function getURI() { + return '/mail/detail/'.$this->getID().'/'; + } + /* -( Routing )------------------------------------------------------------ */