From 6733556582355f9c884c3e5099e272eeca5d49d7 Mon Sep 17 00:00:00 2001
From: Mike Gerwitz <mike.gerwitz@rtspecialty.com>
Date: Tue, 3 Apr 2018 15:06:49 -0400
Subject: [PATCH] Remove hard-coded skey

This wasn't intended to make its way into a public repo. :)

The existing key was a long-forgotten kluge that was supposed to be
temporary, allowing internal services to create quotes without
authentication.  The chances of this being practically exploited are minimal
in our environment, and it's auditable using webserver logs.

This moves the skey into a configuration file, which allows it to vary by
server and be rotated until a better solution is made available.  skey is
disabled by default (empty string), and when used by us internally, the keys
are now generated using a CSPRNG rather than a brute-forcable 5-byte key
that was hard-coded.

The fact that this appears in webserver logs is a big issue as well.  I
added a task to address that.

* conf/vanilla-server.json (skey): New key.  Default empty.
* src/server/daemon/Daemon.js (start): Provide skey to `#getRouters'.
  (getRouters): Provide skey to `#getProgramController'.
  (getProgramController): Set skey on `controller'.
* src/server/daemon/controller.js (skey): New mutable export (unideal; quick
    change).
  (has_skey): Use it.
---
 conf/vanilla-server.json        |  2 ++
 src/server/daemon/Daemon.js     | 16 +++++++++++-----
 src/server/daemon/controller.js | 21 +++++++++++++++++----
 3 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/conf/vanilla-server.json b/conf/vanilla-server.json
index 5e5da12..41e90b8 100644
--- a/conf/vanilla-server.json
+++ b/conf/vanilla-server.json
@@ -18,6 +18,8 @@
     }
   },
 
+  "skey": "",
+
   "user": {
     "session": {
       "handler": {
diff --git a/src/server/daemon/Daemon.js b/src/server/daemon/Daemon.js
index 4b80e43..282197f 100644
--- a/src/server/daemon/Daemon.js
+++ b/src/server/daemon/Daemon.js
@@ -112,7 +112,8 @@ module.exports = AbstractClass( 'Daemon',
         return Promise.all( [
             this._createDebugLog(),
             this._createAccessLog(),
-        ] ).then( ([ debug_log, access_log ]) =>
+            this._conf.get( 'skey' ),
+        ] ).then( ([ debug_log, access_log, skey ]) =>
         {
             this._debugLog   = debug_log;
             this._accessLog  = access_log;
@@ -121,7 +122,7 @@ module.exports = AbstractClass( 'Daemon',
             this._rater      = liza.server.rater.ProcessManager();
             this._encService = this.getEncryptionService();
             this._memcache   = this.getMemcacheClient();
-            this._routers    = this.getRouters();
+            this._routers    = this.getRouters( skey );
         } )
             .then( () => this._startDaemon() );
     },
@@ -181,11 +182,16 @@ module.exports = AbstractClass( 'Daemon',
     },
 
 
-    'protected getProgramController': function()
+    'protected getProgramController': function( skey )
     {
         var controller = require( './controller' );
         controller.rater = this._rater;
 
+        if ( skey )
+        {
+            controller.skey  = skey;
+        }
+
         return controller;
     },
 
@@ -270,10 +276,10 @@ module.exports = AbstractClass( 'Daemon',
     'abstract protected getEncryptionService': [],
 
 
-    'protected getRouters': function()
+    'protected getRouters': function( skey )
     {
         return [
-            this.getProgramController(),
+            this.getProgramController( skey ),
             this.getScriptsController(),
             this.getClientErrorController(),
         ];
diff --git a/src/server/daemon/controller.js b/src/server/daemon/controller.js
index 1cca831..e6571f2 100644
--- a/src/server/daemon/controller.js
+++ b/src/server/daemon/controller.js
@@ -94,6 +94,7 @@ var sflag = {};
 
 // TODO: kluge to get liza somewhat decoupled from lovullo (rating module)
 exports.rater = {};
+exports.skey  = "";
 
 
 exports.init = function( logger, enc_service, conf )
@@ -619,12 +620,24 @@ function createQuoteQuick( id )
 }
 
 
+/**
+ * Check whether the proper skey (session key) was provided
+ *
+ * This is a basic authentication token that allows bypassing authentication
+ * for internal tasks (like creating quotes).
+ *
+ * XXX: A single shared secret is a terrible idea; this was intended to
+ * be a temporary solution.  Fix this crap in favor of proper authentication
+ * between services.
+ */
 function has_skey( user_request )
 {
-    // a basic authentication token that allows our systems to bypass
-    // authentication...this isn't really secure, but it doesn't need to be,
-    // because for our uses, they really cannot do any damage
-    return ( user_request.getGetData().skey === 'fd29d02ac1' )
+    if ( !exports.skey )
+    {
+        return false;
+    }
+
+    return ( user_request.getGetData().skey === exports.skey );
 }