[SQL-125] Redact sensitive information from traces

src/main/logback.xml

@@ -14,9 +14,16 @@
       <pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern>
     </encoder>
   </appender>
-  <root level="info">
+  <root level="debug">
     <appender-ref ref="STDOUT" />
     <appender-ref ref="FILE" />
   </root>
   <logger name="io.pedestal.http" level="warn" />
+  <!--IMPORTANT: We need to turn Pedestal logging off because on an
+      error, Pedestal will spew out the entire request/response map,
+      which may contain sensitive info.-->
+  <!--TODO: Perhaps we can create a custom error logger to replace
+      Pedestal's logger that applies redaction to the request/response
+      map before logging?-->
+  <logger name="io.pedestal.http.impl.servlet-interceptor" level="off" />
 </configuration>

src/main/lrsql/input/statement.clj

@@ -340,8 +340,8 @@
 (defn- warn-on-dupe-shas
   [attachments shas]
   (when (not= (count attachments) (count shas))
-    (log/warnf "%s\nAttachments: %s"
-               duplicate-sha-emsg
+    (log/warn duplicate-sha-emsg)
+    (log/debug "Attachments: %s"
                (mapv #(dissoc % :content) attachments))))
 
 (defn add-insert-attachment-inputs

src/main/lrsql/system/database.clj

@@ -3,7 +3,7 @@
             [com.stuartsierra.component :as component]
             [lrsql.backend.protocol :as bp]
             [lrsql.spec.config :as cs]
-            [lrsql.system.util :refer [assert-config make-jdbc-url]])
+            [lrsql.system.util :as cu :refer [assert-config make-jdbc-url]])
   (:import [com.zaxxer.hikari HikariConfig HikariDataSource]
            [com.codahale.metrics MetricRegistry]
            [com.codahale.metrics.jmx JmxReporter]))
@@ -97,7 +97,7 @@
       (if-not ?conn-pool
         (let [conn-pool (make-conn-pool backend config)]
           (log/infof "Starting new connection for %s database..." db-type)
-          (log/debugf "Config: %s" config)
+          (log/debugf "Config: %s" (cu/redact-config-vars config))
           (assoc conn :conn-pool conn-pool))
         (do
           (log/info "Connection already started; do nothing.")

src/main/lrsql/system/util.clj

@@ -1,16 +1,44 @@
 (ns lrsql.system.util
   (:require [clojure.spec.alpha :as s]
+            [clojure.walk :as w]
             [clojure.tools.logging :as log]
             [next.jdbc.connection :as jdbc-conn]
             [ring.util.codec :refer [form-encode form-decode]]))
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
-;; Macros
+;; Helpers and Macros
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
+(def redactable-config-vars
+  #{:db-password
+    :admin-pass-default
+    :api-secret-default
+    :key-password})
+
+(defn redact-config-vars
+  "Given a `config` map, replace redactable values with \"[REDACTED]\"
+   (if it is a string; keywords and symbols are treated similarly).
+   See `redactable-config-vars` for which vars are redactable."
+  [config]
+  (w/postwalk (fn [node]
+                (if (map-entry? node)
+                  (let [[k v] node]
+                    (if (contains? redactable-config-vars k)
+                      ;; We only consider strings and similar because
+                      ;; other vals should immediately cause an assertion
+                      ;; error and never be meaningful passwords.
+                      (cond
+                        (string? v)  [k "[REDACTED]"]
+                        (keyword? v) [k :redacted]
+                        (symbol? v)  [k 'redacted]
+                        :else        [k v])
+                      [k v]))
+                  node))
+              config))
+
 (defmacro assert-config
   [spec component-name config]
-  `(when-some [err# (s/explain-data ~spec ~config)]
+  `(when-some [err# (s/explain-data ~spec ~(redact-config-vars config))]
      (do
        (log/errorf "Configuration errors:\n%s"
                    (with-out-str (s/explain-out err#)))
@@ -19,6 +47,17 @@
                        {:type ::invalid-config
                         :error-data err#})))))
 
+(comment ; TODO: Turn these into tests
+  (s/explain :lrsql.spec.config/database
+             (redact-config-vars {:db-type     "h2:mem"
+                             :db-name     "foo"
+                             :db-password 100}))
+  (assert-config :lrsql.spec.config/database
+                 "database"
+                 {:db-type     "h2:mem"
+                  :db-name     "foo"
+                  :db-password "swordfish"}))
+
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;; JDBC Config
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

src/main/lrsql/system/webserver.clj

@@ -7,7 +7,7 @@
             [com.yetanalytics.lrs.pedestal.interceptor :as i]
             [lrsql.admin.routes :refer [add-admin-routes]]
             [lrsql.spec.config :as cs]
-            [lrsql.system.util :refer [assert-config]]
+            [lrsql.system.util :refer [assert-config redact-config-vars]]
             [lrsql.util.cert :as cu]))
 
 (defn- service-map
@@ -70,7 +70,7 @@
    (assert-config ::cs/webserver "webserver" config)
    (if server
      (do (log/info "Webserver already started; do nothing.")
-         (log/debugf "Server map: %s" server)
+         (log/debugf "Server map: %s" (redact-config-vars server))
          this)
      (if lrs
        (let [service (or service ;; accept passed in
@@ -92,7 +92,7 @@
                         host
                         ssl-port)))
          (log/info logo)
-         (log/debugf "Server map: %s" server)
+         (log/debugf "Server map: %s" (redact-config-vars server))
          ;; Return new webserver
          (assoc this
                 :service service