From 07fc8f17ccae9d50e33d8e38c4104b3c697ae7f1 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 14 Apr 2016 05:44:04 -0700
Subject: [PATCH] Support "ssl.chain" in Aphlict configuration

Summary: Fixes T10806. Although browsers don't seem to care about this, it's more correct to support it, and the new test console uses normal `cURL` and does care.

Test Plan:
  - Hit the error case for providing a chain but no key/cert.
  - Used `openssl s_client -connect localhost:22280` to connect to local Aphlict servers.
  - With SSL but no chain, saw `openssl` fail to verify the remote.
  - With SSL and a chain, saw `openssl` verify the identify of the remote.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10806

Differential Revision: https://secure.phabricator.com/D15709
---
 conf/aphlict/aphlict.default.json                |  6 ++++--
 .../PhabricatorAphlictManagementWorkflow.php     | 16 ++++++++++++++++
 .../user/configuration/notifications.diviner     |  6 ++++--
 support/aphlict/server/aphlict_server.js         | 10 +++++++++-
 4 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/conf/aphlict/aphlict.default.json b/conf/aphlict/aphlict.default.json
index 1f1bafc3ea..7afdf7e8ff 100644
--- a/conf/aphlict/aphlict.default.json
+++ b/conf/aphlict/aphlict.default.json
@@ -5,14 +5,16 @@
       "port": 22280,
       "listen": "0.0.0.0",
       "ssl.key": null,
-      "ssl.cert": null
+      "ssl.cert": null,
+      "ssl.chain": null
     },
     {
       "type": "admin",
       "port": 22281,
       "listen": "127.0.0.1",
       "ssl.key": null,
-      "ssl.cert": null
+      "ssl.cert": null,
+      "ssl.chain": null
     }
   ],
   "logs": [
diff --git a/src/applications/aphlict/management/PhabricatorAphlictManagementWorkflow.php b/src/applications/aphlict/management/PhabricatorAphlictManagementWorkflow.php
index 60d34b89eb..8d8e1edf77 100644
--- a/src/applications/aphlict/management/PhabricatorAphlictManagementWorkflow.php
+++ b/src/applications/aphlict/management/PhabricatorAphlictManagementWorkflow.php
@@ -99,6 +99,7 @@ abstract class PhabricatorAphlictManagementWorkflow
           'listen' => 'optional string|null',
           'ssl.key' => 'optional string|null',
           'ssl.cert' => 'optional string|null',
+          'ssl.chain' => 'optional string|null',
         ));
 
       $port = $server['port'];
@@ -145,6 +146,21 @@ abstract class PhabricatorAphlictManagementWorkflow
             'ssl.key',
             'ssl.cert'));
       }
+
+      $ssl_chain = idx($server, 'ssl.chain');
+      if ($ssl_chain && (!$ssl_key && !$ssl_cert)) {
+        throw new PhutilArgumentUsageException(
+          pht(
+            'A specified server (at index "%s", on port "%s") specifies '.
+            'a value for "%s", but no value for "%s" or "%s". Servers '.
+            'should only provide an SSL chain if they also provide an SSL '.
+            'key and SSL certificate.',
+            $index,
+            $port,
+            'ssl.chain',
+            'ssl.key',
+            'ssl.cert'));
+      }
     }
 
     if (!$servers) {
diff --git a/src/docs/user/configuration/notifications.diviner b/src/docs/user/configuration/notifications.diviner
index 960b931b66..6a669eaef1 100644
--- a/src/docs/user/configuration/notifications.diviner
+++ b/src/docs/user/configuration/notifications.diviner
@@ -85,13 +85,15 @@ Each server in the `servers` list should be an object with these keys:
     `admin` or `client`. Normally, you should run one of each.
   - `port`: //Required int.// The port this server should listen on.
   - `listen`: //Optional string.// Which interface to bind to. By default,
-    the `admin` server is bound to localhost (so only other services on the
+    the `admin` server is bound to `127.0.0.1` (so only other services on the
     local machine can connect to it), while the `client` server is bound
-    to `0.0.0.0` (so any client can connect.
+    to `0.0.0.0` (so any client can connect).
   - `ssl.key`: //Optional string.// If you want to use SSL on this port,
     the path to an SSL key.
   - `ssl.cert`: //Optional string.// If you want to use SSL on this port,
     the path to an SSL certificate.
+  - `ssl.chain`: //Optional string.// If you have configured SSL on this
+    port, an optional path to a certificate chain file.
 
 Each log in the `logs` list should be an object with these keys:
 
diff --git a/support/aphlict/server/aphlict_server.js b/support/aphlict/server/aphlict_server.js
index 263a355542..2c03875c7d 100644
--- a/support/aphlict/server/aphlict_server.js
+++ b/support/aphlict/server/aphlict_server.js
@@ -104,6 +104,10 @@ for (ii = 0; ii < config.servers.length; ii++) {
     spec['ssl.cert'] = fs.readFileSync(spec['ssl.cert']);
   }
 
+  if (spec['ssl.chain']){
+    spec['ssl.chain'] = fs.readFileSync(spec['ssl.chain']);
+  }
+
   servers.push(spec);
 }
 
@@ -132,9 +136,13 @@ for (ii = 0; ii < servers.length; ii++) {
   if (server['ssl.key']) {
     var https_config = {
       key: server['ssl.key'],
-      cert: server['ssl.cert']
+      cert: server['ssl.cert'],
     };
 
+    if (server['ssl.chain']) {
+      https_config.ca = server['ssl.chain'];
+    }
+
     http_server = https.createServer(https_config);
   } else {
     http_server = http.createServer();