Hide "notification.servers" configuration and don't follow redirects from Aphlict

Summary:
See <https://hackerone.com/reports/850114>.

An attacker with administrator privileges can configure "notification.servers" to connect to internal services, either directly or with chosen parameters by selecting an attacker-controlled service and having it issue a "Location" redirect.

Generally, we allow this attack to occur. The same administrator can use an authentication provider or a VCS repository to perform the same attack, and we can't reasonably harden these workflows without breaking things that users expect to be able to do.

There's no reason this particular variation of the attack needs to be allowable, though, and the current behavior isn't consistent with how other similar things work.

  - Hide the "notification.servers" configuration, which also locks it. This is similar to other modern service/server configuration.
  - Don't follow redirects on these requests. Aphlict should never issue a "Location" header, so if we encounter one something is misconfigured. Declining to follow this header likely makes the issue easier to debug.

Test Plan:
  - Viewed configuration in web UI.
  - Configured a server that "Location: ..." redirects, got a followed redirect before and a failure afterward.

{F7365973}

Differential Revision: https://secure.phabricator.com/D21123

src/applications/config/option/PhabricatorNotificationConfigOptions.php

@@ -52,6 +52,7 @@ EOTEXT
 
     return array(
       $this->newOption('notification.servers', $servers_type, array())
+        ->setHidden(true)
         ->setSummary(pht('Configure real-time notifications.'))
         ->setDescription($servers_help)
         ->addExample(

src/applications/notification/client/PhabricatorNotificationServerRef.php

@@ -209,8 +209,7 @@ final class PhabricatorNotificationServerRef
 
     $server_uri = $this->getURI('/status/');
 
-    list($body) = id(new HTTPSFuture($server_uri))
-      ->setTimeout(2)
+    list($body) = $this->newFuture($server_uri)
       ->resolvex();
 
     return phutil_json_decode($body);
@@ -225,10 +224,32 @@ final class PhabricatorNotificationServerRef
     $server_uri = $this->getURI('/');
     $payload = phutil_json_encode($data);
 
-    id(new HTTPSFuture($server_uri, $payload))
+    $this->newFuture($server_uri, $payload)
       ->setMethod('POST')
-      ->setTimeout(2)
       ->resolvex();
   }
 
+  private function newFuture($uri, $data = null) {
+    if ($data === null) {
+      $future = new HTTPSFuture($uri);
+    } else {
+      $future = new HTTPSFuture($uri, $data);
+    }
+
+    $future->setTimeout(2);
+
+    // At one point, a HackerOne researcher reported a "Location:" redirect
+    // attack here (if the attacker can gain control of the notification
+    // server or the configuration).
+
+    // Although this attack is not particularly concerning, we don't expect
+    // Aphlict to ever issue a "Location:" header, so receiving one indicates
+    // something is wrong and declining to follow the header may make debugging
+    // easier.
+
+    $future->setFollowLocation(false);
+
+    return $future;
+  }
+
 }

src/docs/user/configuration/configuration_locked.diviner

@@ -146,6 +146,12 @@ To clear this setup warning and avoid surprise behavioral changes in the future,
 you should move these configuration values from the database to a local config
 file. Usually, you'll do this by first copying the value from the database:
 
+```
+phabricator/ $ ./bin/config get <key>
+```
+
+...into local configuration:
+
 ```
 phabricator/ $ ./bin/config set <key> <value>
 ```

src/docs/user/configuration/notifications.diviner

@@ -156,7 +156,8 @@ Verifying Server Status
 =======================
 
 After configuring `notification.servers`, navigate to
-{nav Config > Notification Servers} to verify that things are operational.
+{nav Config > Services > Notification Servers} to verify that things are
+operational.
 
 
 Troubleshooting