security(configs): mark credential configs secret and withhold them on read (24.05)

api/api.js

@@ -90,6 +90,43 @@ plugins.connectToAllDatabases().then(function() {
         trim_trailing_ending_spaces: false
     });
 
+    //What a non global admin may read from the api namespace. The first group is what
+    //App Management renders: the dashboard infers each input's widget from the type of
+    //the value here, so a key missing from this list means that setting disappears from
+    //the screen. Keep it in step with showInAppManagment in the plugins frontend.
+    //
+    //domain is separate: crash symbolication builds its return URL from it, and falls
+    //back to the browser's origin when it is absent, which is wrong behind a custom
+    //domain rather than merely cosmetic.
+    plugins.setReadableConfigs("api", {
+        safe: true,
+        session_duration_limit: true,
+        country_data: true,
+        city_data: true,
+        event_limit: true,
+        event_segmentation_limit: true,
+        event_segmentation_value_limit: true,
+        metric_limit: true,
+        session_cooldown: true,
+        total_users: true,
+        prevent_duplicate_requests: true,
+        metric_changes: true,
+        trim_trailing_ending_spaces: true,
+        domain: true
+    });
+
+    //the dashboard reads the password policy to validate a new password before sending
+    //it. Nothing else from this namespace is readable, which is what keeps the proxy
+    //credentials out of the response.
+    plugins.setReadableConfigs("security", {
+        password_min: true,
+        password_char: true,
+        password_number: true,
+        password_symbol: true,
+        password_expiration: true,
+        password_autocomplete: true
+    });
+
     /**
     * Set Plugins APPs Config
     */
@@ -124,6 +161,15 @@ plugins.connectToAllDatabases().then(function() {
         proxy_type: "https"
     });
 
+    //the outbound proxy credentials. The whole security namespace used to be
+    //serialized into the dashboard page and returned to any app admin, so these were
+    //readable well below the operator who set them. The dashboard only uses the
+    //password policy keys from this namespace, so nothing needs them.
+    plugins.setSecretConfigs("security", {
+        proxy_username: true,
+        proxy_password: true
+    });
+
     /**
     * Set tracking config
     */

frontend/express/app.js

@@ -992,9 +992,15 @@ Promise.all([plugins.dbConnection(countlyConfig), plugins.dbConnection("countly_
                     csrf_token: req.csrfToken(),
                     auth_token: req.session.auth_token,
                     member: member,
-                    config: req.config,
-                    security: plugins.getConfig("security"),
-                    tracking: plugins.getConfig("tracking"),
+                    //these namespaces are serialized into the page, so every logged in
+                    //user can read them in the source regardless of their permissions.
+                    //Secret values are dropped rather than masked: the page has no use
+                    //for a placeholder. Anything the browser genuinely needs must not
+                    //be marked secret, since masking what is already in page source
+                    //would achieve nothing.
+                    config: plugins.omitSecretConfigs("frontend", req.config),
+                    security: plugins.omitSecretConfigs("security", plugins.getConfig("security")),
+                    tracking: plugins.omitSecretConfigs("tracking", plugins.getConfig("tracking")),
                     plugins: plugins.getPlugins(),
                     pluginsFull: plugins.getPlugins(true),
                     path: countlyConfig.path || "",

plugins/alerts/api/api.js

@@ -10,6 +10,14 @@ const FEATURE_NAME = 'alerts';
 const commonLib = require("./parts/common-lib.js");
 const moment = require('moment-timezone');
 
+//the alert drawer reads concurrent_users.alert_interval to bound the interval it
+//offers, falling back to a hardcoded 3 minutes when it cannot. The namespace belongs
+//to an enterprise plugin, so it is declared here by the consumer that needs it rather
+//than by its owner.
+plugins.setReadableConfigs("concurrent_users", {
+    alert_interval: true
+});
+
 /**
  * Alerts that can be triggered when an event is received.
  * see module file for details.

plugins/pluginManager.js

@@ -26,6 +26,11 @@ var pluginDependencies = require('./pluginDependencies.js'),
     configextender = require('../api/configextender');
 var pluginConfig = {};
 
+//stands in for a secret configuration value when configuration is read by someone
+//who may know the value is set but not what it is. Deliberately not a plausible
+//value, so it is obvious in a UI and in a bug report.
+const SECRET_CONFIG_MASK = "********";
+
 /**
 * This module handles communicaton with plugins
 * @module "plugins/pluginManager"
@@ -41,6 +46,12 @@ var pluginManager = function pluginManager() {
     var defaultConfigs = {};
     var configsOnchanges = {};
     var excludeFromUI = {plugins: true};
+    //configuration values that hold credentials rather than settings, by namespace.
+    //see setSecretConfigs()
+    var secretConfigs = {};
+    //configuration values a caller who is not a global admin may read, by namespace.
+    //Nothing is readable unless it appears here. See setReadableConfigs()
+    var readableConfigs = {};
     var finishedSyncing = true;
     var expireList = [];
     var masking = {};
@@ -388,6 +399,163 @@ var pluginManager = function pluginManager() {
         }
     };
 
+    /**
+    * Declare which configuration values a caller who is not a global admin may read.
+    *
+    * Nothing is readable unless declared here. That direction is deliberate: marking
+    * secrets instead would mean every new setting is exposed until somebody remembers
+    * to mark it, so one forgotten annotation is a leaked credential. Declaring what is
+    * needed fails the other way, and a missing entry costs a missing input in the UI,
+    * which someone notices and fixes.
+    *
+    * Declare the keys your own code reads. A plugin may declare a key from another
+    * namespace when it is the consumer: the reader knows what it needs, and the
+    * namespace owner may live in a different repository.
+    *
+    * This governs reads by other people. It has nothing to do with what the server
+    * itself can read, which is always the real configuration through getConfig().
+    *
+    * @param {string} namespace - namespace of configuration, usually plugin name
+    * @param {object} conf - object whose keys are readable when the value is true
+    **/
+    this.setReadableConfigs = function(namespace, conf) {
+        if (!readableConfigs[namespace]) {
+            readableConfigs[namespace] = {};
+        }
+        for (let i in conf) {
+            readableConfigs[namespace][i] = conf[i];
+        }
+    };
+
+    /**
+    * Whether a caller who is not a global admin may read a configuration value.
+    * @param {string} namespace - namespace of configuration
+    * @param {string} key - configuration key
+    * @returns {boolean} true when the value may be handed out
+    **/
+    this.isReadableConfig = function(namespace, key) {
+        return !!(Object.prototype.hasOwnProperty.call(readableConfigs, namespace)
+            && readableConfigs[namespace][key] === true);
+    };
+
+    /**
+    * Copy of a full configuration object reduced to the values a caller who is not a
+    * global admin may read. A namespace with nothing declared is dropped entirely.
+    *
+    * The input is not modified.
+    *
+    * @param {object} confs - object of namespace -> configuration object
+    * @returns {object} copy containing only declared values
+    **/
+    this.filterReadableConfigs = function(confs) {
+        var out = {};
+        for (let namespace in confs) {
+            var conf = confs[namespace];
+            if (!conf || typeof conf !== "object" || Array.isArray(conf)) {
+                continue;
+            }
+            for (let key in conf) {
+                if (this.isReadableConfig(namespace, key)) {
+                    if (!out[namespace]) {
+                        out[namespace] = {};
+                    }
+                    out[namespace][key] = conf[key];
+                }
+            }
+        }
+        return out;
+    };
+
+    /**
+    * Mark configuration values as secrets, so they are not handed out on read.
+    *
+    * Some configuration holds credentials rather than settings: a proxy password, an
+    * API key, the key report subscribe tokens are signed with. Those are readable by
+    * anyone who can read configuration at all, which is a lower bar than the
+    * operator who set them. Marking them here masks the value everywhere
+    * configuration is returned to a caller who is not a global admin, and keeps it
+    * out of the values exposed to the dashboard page.
+    *
+    * A value the browser needs is not a secret and must not be marked: once it is in
+    * page source, masking is theatre. tracking.self_tracking_app_key and
+    * recaptcha.site_key are deliberately not marked for that reason.
+    *
+    * getConfig() is unaffected, so server code keeps reading the real value.
+    *
+    * @param {string} namespace - namespace of configuration, usually plugin name
+    * @param {object} conf - object whose keys are secret when the value is true
+    **/
+    this.setSecretConfigs = function(namespace, conf) {
+        if (!secretConfigs[namespace]) {
+            secretConfigs[namespace] = {};
+        }
+        for (let i in conf) {
+            secretConfigs[namespace][i] = conf[i];
+        }
+    };
+
+    /**
+    * Whether a configuration value is marked secret.
+    * @param {string} namespace - namespace of configuration
+    * @param {string} key - configuration key
+    * @returns {boolean} true when the value must not be handed out
+    **/
+    this.isSecretConfig = function(namespace, key) {
+        return !!(secretConfigs[namespace] && secretConfigs[namespace][key] === true);
+    };
+
+    /**
+    * Copy of a full configuration object with every secret value replaced by a
+    * placeholder. Used for callers who may see that configuration exists but not what
+    * it is set to.
+    *
+    * The input is not modified, so the caller cannot accidentally hand a masked
+    * object to code that needs the real values.
+    *
+    * @param {object} confs - object of namespace -> configuration object
+    * @returns {object} copy with secret values masked
+    **/
+    this.maskSecretConfigs = function(confs) {
+        var masked = {};
+        for (let namespace in confs) {
+            var conf = confs[namespace];
+            if (!conf || typeof conf !== "object" || Array.isArray(conf)) {
+                masked[namespace] = conf;
+                continue;
+            }
+            masked[namespace] = {};
+            for (let key in conf) {
+                masked[namespace][key] = this.isSecretConfig(namespace, key) && conf[key] !== "" && conf[key] !== null && typeof conf[key] !== "undefined"
+                    ? SECRET_CONFIG_MASK
+                    : conf[key];
+            }
+        }
+        return masked;
+    };
+
+    /**
+    * Copy of one namespace's configuration with secret keys removed entirely, for
+    * values that are serialized into the dashboard page. Removed rather than masked:
+    * the page has no use for a placeholder, and an absent key cannot be mistaken for
+    * a real one.
+    *
+    * @param {string} namespace - namespace of configuration
+    * @param {object} conf - configuration object for that namespace
+    * @returns {object} copy without the secret keys
+    **/
+    this.omitSecretConfigs = function(namespace, conf) {
+        if (!conf || typeof conf !== "object" || Array.isArray(conf)) {
+            return conf;
+        }
+        var out = {};
+        for (let key in conf) {
+            if (!this.isSecretConfig(namespace, key)) {
+                out[key] = conf[key];
+            }
+        }
+        return out;
+    };
+
     /**
     * Get configuration from specific namespace and populate empty values with provided defaults
     * @param {string} namespace - namespace of configuration, usually plugin name

plugins/plugins/api/api.js

@@ -282,6 +282,30 @@ var plugin = {},
             plugins.loadConfigs(common.db, function() {
                 var confs = plugins.getAllConfigs();
                 delete confs.services;
+                //A caller who is not a global admin gets only the values some part of
+                //the dashboard needs in order to work: the app management settings and
+                //a handful of display values. Nothing else, whether or not anybody
+                //remembered it was sensitive.
+                //
+                //Two mechanisms on purpose, in this order. The allow-list is the
+                //control: a value nobody declared is not returned, so a new setting is
+                //private by default and forgetting costs a missing input rather than a
+                //leak. Masking is the backstop: if a credential is ever declared
+                //readable by mistake, its value is still withheld.
+                if (!params.member.global_admin) {
+                    var before = Object.keys(confs);
+                    confs = plugins.maskSecretConfigs(plugins.filterReadableConfigs(confs));
+                    //a namespace disappearing here is how an undeclared value looks
+                    //from the outside, and the UI degrades silently when it happens, so
+                    //leave a trail rather than nothing
+                    var dropped = before.filter(function(ns) {
+                        return !confs[ns];
+                    });
+                    if (dropped.length) {
+                        log.d("Withheld configuration namespaces from a non global admin"
+                            + common.reqInfo(params) + ": " + dropped.join(", "));
+                    }
+                }
                 common.returnOutput(params, confs);
             });
         });