rnewson commented on code in PR #4814:
URL: https://github.com/apache/couchdb/pull/4814#discussion_r1378756370
##########
src/couch/src/couch_password_hasher.erl:
##########
@@ -21,20 +21,41 @@
init/1,
handle_call/3,
handle_cast/2,
+ handle_info/2,
code_change/3
]).
--export([hash/1]).
+-export([maybe_upgrade_password_hash/4, hash_admin_passwords/1]).
--record(state, {}).
+-export([worker_loop/1]).
+
+-define(IN_PROGRESS_ETS, couch_password_hasher_in_progress).
+
+-record(state, {
+ worker_pid,
+ worker_mon
+}).
%%%===================================================================
%%% Public functions
%%%===================================================================
--spec hash(Persist :: boolean()) -> Reply :: term().
-hash(Persist) ->
- gen_server:cast(?MODULE, {hash_passwords, Persist}).
+maybe_upgrade_password_hash(AuthModule, UserName, Password, UserProps) ->
+ IsAdmin = is_admin(UserProps),
+ NeedsUpgrade = needs_upgrade(UserProps),
+ InProgress = in_progress(AuthModule, UserName),
+ if
+ not IsAdmin andalso NeedsUpgrade andalso not InProgress ->
Review Comment:
I should write my design choices into the docs, or some internal couchdb doc
on this;
Firstly, there's the complication that admin hashes are written to .ini
files (across all nodes of the cluster) whereas user hashes are written to
documents (with fabric quorum). At various points in the code the admins are
processed as if they had documents (make_admin_doc) to ease handling, but those
documents cannot be updated, as they don't exist (and, for security reasons,
must not be created).
Secondly, there's an aspect of caution. Messing up the password hash
properties of a user doc is bad, but correctable (by an admin, if necessary).
Messing up the admin hashes is harder to recover from (remsh if you can, but
likely you have to get on the nodes, edit files, and restart services).
Thirdly, there is a slight security issue to consider. Merely upgrading the
hash without changing either the password or salt is an improvement, but it's
nowhere near as good as changing the password and the salt. The old hashes
might have been leaked, and will yield to brute force far quicker. So, the
optimistic hashing upgrade is helpful, but a password change is better. Hence,
I think admin hashes should change only when you change the password, and
perhaps (like at Cloudant) the admin hashes are externally managed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
