Reconsider password-cache policy

[akater]

Consider the following snippet from =tramp.el=:

#+begin_example elisp
;; Try the password cache.
(progn
  (setq auth-passwd (password-read pw-prompt key)
        tramp-password-save-function
        (lambda () (password-cache-add key auth-passwd)))
  auth-passwd)
#+end_example

I think that if the intent of the progn form is to “try the password
cache” then such trial should fail whenever password caching is turned
off, i.e. when ~password-cache~ variable set to nil.

This suggestion will likely be questioned but in any case, there seems
to be no way at all to prevent TRAMP from caching passwords.
Cf. ~auth-source-do-cache~ variable; there does not seem to be any
equivalent in TRAMP.  So there should be ~tramp-do-password-cache~
variable at least.

However, I have no doubts that if ~password-cache~ is set to nil then
~password-data~ must be kept empty.  Authors of third party libraries
should not attempt to use password cache when ~password-cache~ is nil.
Thus, ~password-cache-add~ should always check the value of
~password-cache~ before doing anything and if yes, it should emit a
warning that there was an attempt to add a password to cache but
~password-cache~ is nil.

So, at the very least in ~tramp-read-passwd~ there should be
#+begin_example elisp
;; Try the password cache.
(when tramp-do-password-cache
  (setq auth-passwd (password-read pw-prompt key)
        tramp-password-save-function
        (lambda () (password-cache-add key auth-passwd)))
  auth-passwd)
#+end_example

with newly defined ~tramp-do-password-cache~ variable.

but I certainly suggest to go further, explicitly warn third party
library authors against using password cache when ~password-cache~ is
nil, and alter (at least) the definitions of

- ~tramp-read-passwd~:

#+begin_example elisp
;; Try the password cache.
(when password-cache
  (setq auth-passwd (password-read pw-prompt key)
        tramp-password-save-function
        (lambda () (password-cache-add key auth-passwd)))
  auth-passwd)
#+end_example

- ~password-cache-add~:

#+begin_example elisp
(defun password-cache-add (key password)
  "Add password to cache.
The password is removed by a timer after `password-cache-expiry' seconds."
  (if (not password-cache)
      (warn
       (format
        "There was an attempt to add a password to cache while `%s' is nil"
        'password-cache))
    (when (and password-cache-expiry
               (eq (gethash key password-data 'password-cache-no-data)
                   'password-cache-no-data))
      (run-at-time password-cache-expiry nil
                   #'password-cache-remove
                   key))
    (puthash key password password-data))
  nil)
#+end_example

- ~auth-source-search~:

#+begin_example elisp
(when password-cache
  (auth-source-remember spec found))
#+end_example

and alias ~auth-source-do-cache~ variable to ~password-cache~, marking
it as obsolete.

There is no point in allowing one library to use the cache but
disallowing another to do it.  It does not help with security as any
Elisp code can access that data anyway, any time, while added complexity
is always bad for security.  In contrast, there certainly must be a
clear way to turn caching off once and for all.  Given the current
policy, it can not possibly exist.  Multiplying ~..-do-cache~ variables
across elisp libraries will not do users any good.

I also think that password caching should be turned off by default.

signature.asc
Description: PGP signature