Use a custom type to avoid accidentially logging passwords.

Signed-off-by: Igor Mandrigin <i@mandrigin.ru>

doc/decisions/0007-masking-sensitive-data.md

@@ -0,0 +1,39 @@
+# 0007. Masking Sensitive Data
+
+| Date | Tags |
+|---|---|
+| 2018-05-22 | e.g: architecture, security |
+
+
+## Status
+
+Proposed
+
+## Context
+
+We have some data that we don't want to appear in the logs (user passwords are
+a good example). Currently, they are passed around as strings, that could be
+printed out by mistake in a log entry (see https://github.com/status-im/status-react/issues/4053)
+
+## Decision
+
+To minimize the risk of leaking passwords through logs, we should not pass
+passwords as strings in our codebase. We introduced a new type `MaskedData` in
+`status-im.utils.security`.
+We use `(security/mask-data <data to hide>` to wrap sensitive data into this
+type and then use `(security/unmask <masked-data>)` to get the plaintext back.
+
+It is important to keep that sensitive data masked as much as possible, until
+you need the plaintext to pass to the extenral APIs.
+
+Example:
+```clojure
+(println (security/mask-data "my-plaintext-password")) ;; Outputs "******"
+(println (security/unmask (security/mask-data "my-plaintext-password"))) ;; Outputs "my-plaintext-password"
+```
+
+## Consequences
+
+Tradeoffs:
+- developers need to be aware of this type and have a clear separation where do
+we use plaintext and where do we use masked datak

src/status_im/ui/screens/wallet/send/events.cljs

@@ -11,6 +11,7 @@
             [status-im.utils.handlers :as handlers]
             [status-im.utils.hex :as utils.hex]
             [status-im.utils.money :as money]
+            [status-im.utils.security :as security]
             [status-im.utils.types :as types]
             [status-im.utils.utils :as utils]
             [status-im.constants :as constants]
@@ -20,8 +21,9 @@
 
 (re-frame/reg-fx
  ::accept-transaction
- (fn [{:keys [password id on-completed]}]
-   (status/approve-sign-requests (list id) password on-completed)))
+ (fn [{:keys [masked-password id on-completed]}]
+   ;; unmasking the password as late as possible to avoid being exposed from app-db
+   (status/approve-sign-requests (list id) (security/unmask masked-password) on-completed)))
 
 (defn- send-ethers [{:keys [web3 from to value gas gas-price]}]
   (.sendTransaction (.-eth web3)
@@ -170,9 +172,9 @@
              (let [{:keys [password]} (get-in db [:wallet :send-transaction])
                    new-db'            (update-in new-db [:wallet :send-transaction] merge sending-db)] ; just update sending state as we are in wallet flow
                {:db                  new-db'
-                ::accept-transaction {:id           id
-                                      :password     password
-                                      :on-completed on-transactions-completed}})))
+                ::accept-transaction {:id              id
+                                      :masked-password password
+                                      :on-completed    on-transactions-completed}})))
           ;;SIGN MESSAGE
          (= method constants/web3-personal-sign)
 
@@ -259,9 +261,9 @@
          network (:network db)
          {:keys [amount id password to symbol method gas gas-price]} (get-in db [:wallet :send-transaction])]
      (if id
-       {::accept-transaction {:id           id
-                              :password     password
-                              :on-completed on-transactions-completed}
+       {::accept-transaction {:id              id
+                              :masked-password password
+                              :on-completed    on-transactions-completed}
         :db                  (assoc-in db' [:wallet :send-transaction :in-progress?] true)}
        {:db                (update-in db' [:wallet :send-transaction] assoc
                                       :waiting-signal? true
@@ -282,9 +284,9 @@
  (fn [{db :db} _]
    (let [{:keys [id password]} (get-in db [:wallet :send-transaction])]
      {:db                  (assoc-in db [:wallet :send-transaction :in-progress?] true)
-      ::accept-transaction {:id           id
-                            :password     password
-                            :on-completed on-transactions-modal-completed}})))
+      ::accept-transaction {:id              id
+                            :masked-password password
+                            :on-completed    on-transactions-modal-completed}})))
 
 (defn discard-transaction
   [{:keys [db]}]
@@ -315,8 +317,8 @@
 
 (handlers/register-handler-fx
  :wallet.send/set-password
- (fn [{:keys [db]} [_ password]]
-   {:db (assoc-in db [:wallet :send-transaction :password] password)}))
+ (fn [{:keys [db]} [_ masked-password]]
+   {:db (assoc-in db [:wallet :send-transaction :password] masked-password)}))
 
 (handlers/register-handler-fx
  :wallet.send/set-signing?

src/status_im/ui/screens/wallet/send/views.cljs

@@ -20,6 +20,7 @@
             [status-im.ui.screens.wallet.send.styles :as styles]
             [status-im.ui.screens.wallet.styles :as wallet.styles]
             [status-im.utils.money :as money]
+            [status-im.utils.security :as security]
             [status-im.utils.utils :as utils]
             [status-im.transport.utils :as transport.utils]
             [status-im.utils.ethereum.tokens :as tokens]
@@ -56,7 +57,7 @@
          :secure-text-entry      true
          :placeholder            (i18n/label :t/enter-password)
          :placeholder-text-color components.styles/color-gray4
-         :on-change-text         #(re-frame/dispatch [:wallet.send/set-password %])
+         :on-change-text         #(re-frame/dispatch [:wallet.send/set-password (security/mask-data %)])
          :style                  styles/password
          :accessibility-label    :enter-password-input}]
        (when wrong-password?

src/status_im/utils/handlers.cljs

@@ -22,12 +22,9 @@
 
 (defn- pretty-print-event [ctx]
   (let [[first second] (get-coeffect ctx :event)]
-    ;; TODO wrap passwords in a custom type so it won't be possible to print them occasionally
-    (if (= first :wallet.send/set-password)
-      (str first " " "******") ;; special case not to expose password to the logs
-      (if (or (string? second) (keyword? second) (boolean? second))
-        (str first " " second)
-        first))))
+    (if (or (string? second) (keyword? second) (boolean? second))
+      (str first " " second)
+      first)))
 
 (def debug-handlers-names
   "Interceptor which logs debug information to js/console for each event."

src/status_im/utils/security.cljs

@@ -0,0 +1,19 @@
+(ns status-im.utils.security)
+
+(defprotocol Unmaskable
+  ;; Retrieve the stored value.
+  (unmask [this]))
+
+;; MaskedData ensures that the object passed to it won't be occasionally printed
+;; via println or log functions. Useful for keeping sensitive data, such as passwords
+;; to avoid accidentally exposing them.
+(deftype MaskedData [data]
+  Object
+  (toString [_] "******")
+  Unmaskable
+  (unmask [this]
+    (.-data this)))
+
+;; Returns a MaskedData instance that stores the piece of data.
+(defn mask-data [data]
+  (MaskedData. data))