Re: [PR] mango: add $beginsWith operator [couchdb]
pgj commented on PR #4810: URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1789425187 > Another sentinel value might work, good idea. Sure but what should be that value specifically? My impression is that 0x was created for that exact purpose because it was missing before. It would be good to know how others have implemented this feature in lack of this symbol. -- 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: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] mango: add $beginsWith operator [couchdb]
nickva commented on PR #4810: URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1789416560 Another sentinel value might work, good idea. CentOS is EOL next year so then we can deprecate support for it and remove a few hacks we have for it. -- 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: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Decouple offline hash strength from online [couchdb]
rnewson commented on code in PR #4814:
URL: https://github.com/apache/couchdb/pull/4814#discussion_r1379094457
##
src/couch/src/couch_passwords_cache.erl:
##
@@ -0,0 +1,65 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+%
+% password hashes on disk can take a long time to verify. This is by design, to
+% guard against offline attacks. This module adds an in-memory cache for
password
+% verification to speed up verification after a successful slow verification
from
+% the hash stored on disk, in order not to transfer the deliberate offline
attack
+% protection to database requests.
+%
+% In memory we record a PBKDF_SHA256 derivation using a low number of
iterations
+% and check against this if present. Entries in couch_passwords_cache expire
automatically
+% and the maximum number of cached entries is configurable.
+
+-module(couch_passwords_cache).
+
+-define(CACHE, couch_passwords_cache_lru).
+
+-export([start_link/0]).
+
+% public api
+-export([authenticate/4, insert/4]).
+
+start_link() ->
+gen_server:start_link({local, ?MODULE}, ?MODULE, [], []).
Review Comment:
removed
--
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: notifications-unsubscr...@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Decouple offline hash strength from online [couchdb]
rnewson commented on code in PR #4814:
URL: https://github.com/apache/couchdb/pull/4814#discussion_r1379093773
##
src/couch/src/couch_passwords_cache.erl:
##
@@ -0,0 +1,65 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+%
+% password hashes on disk can take a long time to verify. This is by design, to
+% guard against offline attacks. This module adds an in-memory cache for
password
+% verification to speed up verification after a successful slow verification
from
+% the hash stored on disk, in order not to transfer the deliberate offline
attack
+% protection to database requests.
+%
+% In memory we record a PBKDF_SHA256 derivation using a low number of
iterations
+% and check against this if present. Entries in couch_passwords_cache expire
automatically
+% and the maximum number of cached entries is configurable.
+
+-module(couch_passwords_cache).
+
+-define(CACHE, couch_passwords_cache_lru).
+
+-export([start_link/0]).
+
+% public api
+-export([authenticate/4, insert/4]).
+
+start_link() ->
+gen_server:start_link({local, ?MODULE}, ?MODULE, [], []).
+
+% public functions
+-spec authenticate(
+AuthModule :: atom(), UserName :: binary(), Password :: binary(), Salt ::
binary()
+) ->
+not_found | boolean().
+authenticate(AuthModule, UserName, Password, Salt) ->
Review Comment:
done
--
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: notifications-unsubscr...@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Decouple offline hash strength from online [couchdb]
rnewson commented on code in PR #4814:
URL: https://github.com/apache/couchdb/pull/4814#discussion_r1379093552
##
src/couch/src/couch_passwords.erl:
##
@@ -69,106 +68,74 @@ get_unhashed_admins() ->
({_User, "-pbkdf2-" ++ _}) ->
% already hashed
false;
+({_User, "-pbkdf2:sha-" ++ _}) ->
+% already hashed
+false;
+({_User, "-pbkdf2:sha224-" ++ _}) ->
+% already hashed
+false;
+({_User, "-pbkdf2:sha256-" ++ _}) ->
+% already hashed
+false;
+({_User, "-pbkdf2:sha384-" ++ _}) ->
+% already hashed
+false;
+({_User, "-pbkdf2:sha512-" ++ _}) ->
+% already hashed
+false;
({_User, _ClearPassword}) ->
true
end,
config:get("admins")
).
-%% Current scheme, much stronger.
--spec pbkdf2(binary(), binary(), integer()) -> binary().
-pbkdf2(Password, Salt, Iterations) when
+pbkdf2(Password, Salt, Iterations) ->
+pbkdf2(sha, Password, Salt, Iterations).
+
+pbkdf2(PRF, Password, Salt, Iterations) when is_atom(PRF) ->
+#{size := Size} = crypto:hash_info(PRF),
+pbkdf2(PRF, Password, Salt, Iterations, Size);
+pbkdf2(Password, Salt, Iterations, KeyLen) ->
+pbkdf2(sha, Password, Salt, Iterations, KeyLen).
+
+pbkdf2(PRF, Password, Salt, Iterations, KeyLen) when
+is_atom(PRF),
is_binary(Password),
is_binary(Salt),
is_integer(Iterations),
-Iterations > 0
+Iterations > 0,
+KeyLen > 0
Review Comment:
done
##
src/couch/src/couch_passwords.erl:
##
@@ -69,106 +68,74 @@ get_unhashed_admins() ->
({_User, "-pbkdf2-" ++ _}) ->
% already hashed
false;
+({_User, "-pbkdf2:sha-" ++ _}) ->
+% already hashed
+false;
+({_User, "-pbkdf2:sha224-" ++ _}) ->
+% already hashed
+false;
+({_User, "-pbkdf2:sha256-" ++ _}) ->
+% already hashed
+false;
+({_User, "-pbkdf2:sha384-" ++ _}) ->
+% already hashed
+false;
+({_User, "-pbkdf2:sha512-" ++ _}) ->
+% already hashed
+false;
({_User, _ClearPassword}) ->
true
end,
config:get("admins")
).
-%% Current scheme, much stronger.
--spec pbkdf2(binary(), binary(), integer()) -> binary().
-pbkdf2(Password, Salt, Iterations) when
+pbkdf2(Password, Salt, Iterations) ->
+pbkdf2(sha, Password, Salt, Iterations).
+
+pbkdf2(PRF, Password, Salt, Iterations) when is_atom(PRF) ->
+#{size := Size} = crypto:hash_info(PRF),
+pbkdf2(PRF, Password, Salt, Iterations, Size);
+pbkdf2(Password, Salt, Iterations, KeyLen) ->
+pbkdf2(sha, Password, Salt, Iterations, KeyLen).
+
+pbkdf2(PRF, Password, Salt, Iterations, KeyLen) when
+is_atom(PRF),
is_binary(Password),
is_binary(Salt),
is_integer(Iterations),
-Iterations > 0
+Iterations > 0,
+KeyLen > 0
->
-{ok, Result} = pbkdf2(Password, Salt, Iterations, ?SHA1_OUTPUT_LENGTH),
-Result;
-pbkdf2(Password, Salt, Iterations) when
+DerivedKey = fast_pbkdf2:pbkdf2(PRF, Password, Salt, Iterations, KeyLen),
+couch_util:to_hex_bin(DerivedKey);
+pbkdf2(PRF, Password, Salt, Iterations, KeyLen) when
+is_atom(PRF),
is_binary(Salt),
is_integer(Iterations),
-Iterations > 0
+Iterations > 0,
+KeyLen > 0
->
Msg = io_lib:format("Password value of '~p' is invalid.", [Password]),
throw({forbidden, Msg});
-pbkdf2(Password, Salt, Iterations) when
+pbkdf2(PRF, Password, Salt, Iterations, KeyLen) when
+is_atom(PRF),
is_binary(Password),
is_integer(Iterations),
-Iterations > 0
+Iterations > 0,
+KeyLen > 0
Review Comment:
done
##
src/couch/src/couch_passwords.erl:
##
@@ -69,106 +68,74 @@ get_unhashed_admins() ->
({_User, "-pbkdf2-" ++ _}) ->
% already hashed
false;
+({_User, "-pbkdf2:sha-" ++ _}) ->
+% already hashed
+false;
+({_User, "-pbkdf2:sha224-" ++ _}) ->
+% already hashed
+false;
+({_User, "-pbkdf2:sha256-" ++ _}) ->
+% already hashed
+false;
+({_User, "-pbkdf2:sha384-" ++ _}) ->
+% already hashed
+false;
+({_User, "-pbkdf2:sha512-" ++ _}) ->
+% already hashed
+false;
({_User, _ClearPassword}) ->
true
end,
config:get("admins")
).
-%% Current scheme, much stronger.
--spec pbkdf2(binary(), binary(), integer()) -> binary().
Re: [PR] Decouple offline hash strength from online [couchdb]
rnewson commented on code in PR #4814:
URL: https://github.com/apache/couchdb/pull/4814#discussion_r1379093251
##
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 might try the admin upgrade to see what it looks like, now that I added
the piece to preserve salt for upgrades I think I can mitigate any problems
with the upgrade, it's just whether the fake admin doc makes the update harder,
etc. but I would happily proceed with merging without it.
##
src/couch/src/couch_password_hasher.erl:
##
@@ -45,29 +66,95 @@ start_link() ->
init(_Args) ->
hash_admin_passwords(true),
-{ok, #state{}}.
+?IN_PROGRESS_ETS = ets:new(?IN_PROGRESS_ETS, [named_table,
{read_concurrency, true}]),
+{ok, start_worker_loop(#state{})}.
handle_call(Msg, _From, #state{} = State) ->
{stop, {invalid_call, Msg}, {invalid_call, Msg}, State}.
-handle_cast({hash_passwords, Persist}, State) ->
-hash_admin_passwords(Persist),
+handle_cast({upgrade_password_hash, AuthModule, UserName, Password,
UserProps}, State) ->
+case ets:insert_new(?IN_PROGRESS_ETS, {{AuthModule, UserName}}) of
+true ->
+State#state.worker_pid !
+{upgrade_password_hash, AuthModule, UserName, Password,
UserProps};
+false ->
+ok
+end,
+{noreply, State};
+handle_cast({hash_admin_passwords, Persist}, State) ->
+hash_admin_passwords_int(Persist),
{noreply, State};
handle_cast(Msg, State) ->
{stop, {invalid_cast, Msg}, State}.
+handle_info({done, AuthModule, UserName}, State) ->
+ets:delete(?IN_PROGRESS_ETS, {AuthModule, UserName}),
+{noreply, State};
+handle_info({'DOWN', MonRef, _, _, normal}, #state{worker_mon = MonRef} =
State) ->
+ets:delete_all_objects(?IN_PROGRESS_ETS),
+{noreply, start_worker_loop(State)};
+handle_info(Msg, State) ->
+{stop, {invalid_info, Msg}, State}.
+
code_change(_OldVsn, #state{} = State, _Extra) ->
{ok, State}.
%%%===
%%% Internal functions
%%%===
-hash_admin_passwords(Persist) ->
+hash_admin_passwords_int(Persist) ->
lists:foreach(
fun({User, ClearPassword}) ->
HashedPassword =
couch_passwords:hash_admin_password(ClearPassword),
config:set("admins", User, ?b2l(HashedPassword), Persist)
end,
couch_passwords:get_unhashed_admins()
).
+
+is_admin(UserProps) ->
+Roles = couch_util:get_value(<<"roles">>, UserProps, []),
+lists:member(<<"_admin">>, Roles).
+
+needs_upgrade(UserProps) ->
+CurrentScheme = couch_util:get_value(<<"password_scheme">>, UserProps,
<<"simple">>),
+TargetScheme = ?l2b(chttpd_util:get_chttpd_auth_config("password_scheme",
"pbkdf2")),
+CurrentPRF = couch_util:get_value(<<"pbkdf2_prf">>, UserProps, <<"sha">>),
+TargetPRF = ?l2b(chttpd_util:get_chttpd_auth_config("pbkdf2_prf",
"sha256")),
+CurrentIterations = couch_util:to_integer(
+couch_util:get_value(<<"iterations">>, UserProps, "10")
+),
+TargetIterations = chttpd_util:get_chttpd_auth_config_integer(
+"iterations", 10
+),
+if
+CurrentScheme == TargetScheme andalso TargetScheme == <<"simple">>
andalso
+CurrentIterations == TargetIterations ->
+false;
+CurrentScheme == TargetScheme andalso TargetScheme == <<"pbkdf2">>
andalso
+CurrentPRF == TargetPRF andalso CurrentIterations ==
TargetIterations ->
+false;
+true ->
+true
+end.
+
+in_progress(AuthModule, UserName) ->
+ets:member(?IN_PROGRESS_ETS, {AuthModule, UserName}).
+
+start_worker_loop(State) ->
+{WorkerPid, WorkerMon} = spawn_monitor(?MODULE, worker_loop, [self()]),
Review Comment:
updated
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL
Re: [PR] Decouple offline hash strength from online [couchdb]
nickva commented on code in PR #4814:
URL: https://github.com/apache/couchdb/pull/4814#discussion_r1378989463
##
src/couch/src/couch_passwords_cache.erl:
##
@@ -0,0 +1,65 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+%
+% password hashes on disk can take a long time to verify. This is by design, to
+% guard against offline attacks. This module adds an in-memory cache for
password
+% verification to speed up verification after a successful slow verification
from
+% the hash stored on disk, in order not to transfer the deliberate offline
attack
+% protection to database requests.
+%
+% In memory we record a PBKDF_SHA256 derivation using a low number of
iterations
+% and check against this if present. Entries in couch_passwords_cache expire
automatically
+% and the maximum number of cached entries is configurable.
+
+-module(couch_passwords_cache).
+
+-define(CACHE, couch_passwords_cache_lru).
+
+-export([start_link/0]).
+
+% public api
+-export([authenticate/4, insert/4]).
+
+start_link() ->
+gen_server:start_link({local, ?MODULE}, ?MODULE, [], []).
+
+% public functions
+-spec authenticate(
+AuthModule :: atom(), UserName :: binary(), Password :: binary(), Salt ::
binary()
+) ->
+not_found | boolean().
+authenticate(AuthModule, UserName, Password, Salt) ->
+case config:get_boolean("couch_passwords_cache", "enable", true) of
+true ->
+authenticate_int(AuthModule, UserName, Password, Salt);
+false ->
+not_found
+end.
+
+authenticate_int(AuthModule, UserName, Password, Salt) ->
+% salt in key to ensure entry is not a match if user changes their password
+% (as salt is re-randomised in that event)
+case ets_lru:lookup_d(?CACHE, {AuthModule, UserName, Salt}) of
+not_found ->
+not_found;
+{ok, ExpectedHash} ->
+ActualHash = hash(Password, Salt),
+couch_passwords:verify(ExpectedHash, ActualHash)
+end.
+
+-spec insert(AuthModule :: atom(), UserName :: binary(), Password :: binary(),
Salt :: binary()) ->
+ok.
+insert(AuthModule, UserName, Password, Salt) ->
+ets_lru:insert(?CACHE, {AuthModule, UserName, Salt}, hash(Password, Salt)).
+
+hash(Password, Salt) ->
+fast_pbkdf2:pbkdf2(sha256, Password, Salt, _Iterations = 5, _KeyLen = 32).
Review Comment:
Well, it wouldn't be configurable anyway, but the idea is to make them more
visible as they are parameters for this cache module. Perhaps with a comment
about why iterations is 5 and maybe what threat model we're assuming. If I were
auditing the code ` _Iterations = 5` would jump out immediately as a red flag.
Since fast_pbkdf2 is an external library let's isolate all the calls to it
in `couch_passwords` or other hash utility module if we have one. If my otp
pbdkf2 unlocking pr merges, we'd have less places to replace.
--
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: notifications-unsubscr...@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Decouple offline hash strength from online [couchdb]
nickva commented on code in PR #4814:
URL: https://github.com/apache/couchdb/pull/4814#discussion_r1378976731
##
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:
That makes sense. We could put it in a comment in the code perhaps right
above `maybe_upgrade_password_hash`, just so we if we change the code it
wouldn't get out of sync with a readme.md file somewhere.
--
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: notifications-unsubscr...@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Decouple offline hash strength from online [couchdb]
rnewson commented on code in PR #4814:
URL: https://github.com/apache/couchdb/pull/4814#discussion_r1378872713
##
src/couch/src/couch_passwords_cache.erl:
##
@@ -0,0 +1,65 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+%
+% password hashes on disk can take a long time to verify. This is by design, to
+% guard against offline attacks. This module adds an in-memory cache for
password
+% verification to speed up verification after a successful slow verification
from
+% the hash stored on disk, in order not to transfer the deliberate offline
attack
+% protection to database requests.
+%
+% In memory we record a PBKDF_SHA256 derivation using a low number of
iterations
+% and check against this if present. Entries in couch_passwords_cache expire
automatically
+% and the maximum number of cached entries is configurable.
+
+-module(couch_passwords_cache).
+
+-define(CACHE, couch_passwords_cache_lru).
+
+-export([start_link/0]).
+
+% public api
+-export([authenticate/4, insert/4]).
+
+start_link() ->
+gen_server:start_link({local, ?MODULE}, ?MODULE, [], []).
+
+% public functions
+-spec authenticate(
+AuthModule :: atom(), UserName :: binary(), Password :: binary(), Salt ::
binary()
+) ->
+not_found | boolean().
+authenticate(AuthModule, UserName, Password, Salt) ->
+case config:get_boolean("couch_passwords_cache", "enable", true) of
+true ->
+authenticate_int(AuthModule, UserName, Password, Salt);
+false ->
+not_found
+end.
+
+authenticate_int(AuthModule, UserName, Password, Salt) ->
+% salt in key to ensure entry is not a match if user changes their password
+% (as salt is re-randomised in that event)
+case ets_lru:lookup_d(?CACHE, {AuthModule, UserName, Salt}) of
+not_found ->
+not_found;
+{ok, ExpectedHash} ->
+ActualHash = hash(Password, Salt),
+couch_passwords:verify(ExpectedHash, ActualHash)
+end.
+
+-spec insert(AuthModule :: atom(), UserName :: binary(), Password :: binary(),
Salt :: binary()) ->
+ok.
+insert(AuthModule, UserName, Password, Salt) ->
+ets_lru:insert(?CACHE, {AuthModule, UserName, Salt}, hash(Password, Salt)).
+
+hash(Password, Salt) ->
+fast_pbkdf2:pbkdf2(sha256, Password, Salt, _Iterations = 5, _KeyLen = 32).
Review Comment:
This was my way of showing that the in-memory hashing is not configurable by
design.
--
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: notifications-unsubscr...@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Decouple offline hash strength from online [couchdb]
rnewson commented on code in PR #4814: URL: https://github.com/apache/couchdb/pull/4814#discussion_r1378861581 ## src/couch/src/couch_passwords_cache.erl: ## @@ -0,0 +1,65 @@ +% Licensed under the Apache License, Version 2.0 (the "License"); you may not +% use this file except in compliance with the License. You may obtain a copy of +% the License at +% +% http://www.apache.org/licenses/LICENSE-2.0 +% +% Unless required by applicable law or agreed to in writing, software +% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +% License for the specific language governing permissions and limitations under +% the License. +% +% password hashes on disk can take a long time to verify. This is by design, to +% guard against offline attacks. This module adds an in-memory cache for password +% verification to speed up verification after a successful slow verification from +% the hash stored on disk, in order not to transfer the deliberate offline attack +% protection to database requests. +% +% In memory we record a PBKDF_SHA256 derivation using a low number of iterations +% and check against this if present. Entries in couch_passwords_cache expire automatically +% and the maximum number of cached entries is configurable. + +-module(couch_passwords_cache). + +-define(CACHE, couch_passwords_cache_lru). Review Comment: good catch, this existed before I switched to ets_lru -- 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: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Decouple offline hash strength from online [couchdb]
rnewson commented on code in PR #4814:
URL: https://github.com/apache/couchdb/pull/4814#discussion_r1378853926
##
src/couch/src/couch_password_hasher.erl:
##
@@ -45,29 +66,95 @@ start_link() ->
init(_Args) ->
hash_admin_passwords(true),
-{ok, #state{}}.
+?IN_PROGRESS_ETS = ets:new(?IN_PROGRESS_ETS, [named_table,
{read_concurrency, true}]),
+{ok, start_worker_loop(#state{})}.
handle_call(Msg, _From, #state{} = State) ->
{stop, {invalid_call, Msg}, {invalid_call, Msg}, State}.
-handle_cast({hash_passwords, Persist}, State) ->
-hash_admin_passwords(Persist),
+handle_cast({upgrade_password_hash, AuthModule, UserName, Password,
UserProps}, State) ->
+case ets:insert_new(?IN_PROGRESS_ETS, {{AuthModule, UserName}}) of
+true ->
+State#state.worker_pid !
+{upgrade_password_hash, AuthModule, UserName, Password,
UserProps};
+false ->
+ok
+end,
+{noreply, State};
+handle_cast({hash_admin_passwords, Persist}, State) ->
+hash_admin_passwords_int(Persist),
{noreply, State};
handle_cast(Msg, State) ->
{stop, {invalid_cast, Msg}, State}.
+handle_info({done, AuthModule, UserName}, State) ->
+ets:delete(?IN_PROGRESS_ETS, {AuthModule, UserName}),
+{noreply, State};
+handle_info({'DOWN', MonRef, _, _, normal}, #state{worker_mon = MonRef} =
State) ->
+ets:delete_all_objects(?IN_PROGRESS_ETS),
+{noreply, start_worker_loop(State)};
+handle_info(Msg, State) ->
+{stop, {invalid_info, Msg}, State}.
+
code_change(_OldVsn, #state{} = State, _Extra) ->
{ok, State}.
%%%===
%%% Internal functions
%%%===
-hash_admin_passwords(Persist) ->
+hash_admin_passwords_int(Persist) ->
lists:foreach(
fun({User, ClearPassword}) ->
HashedPassword =
couch_passwords:hash_admin_password(ClearPassword),
config:set("admins", User, ?b2l(HashedPassword), Persist)
end,
couch_passwords:get_unhashed_admins()
).
+
+is_admin(UserProps) ->
+Roles = couch_util:get_value(<<"roles">>, UserProps, []),
+lists:member(<<"_admin">>, Roles).
+
+needs_upgrade(UserProps) ->
+CurrentScheme = couch_util:get_value(<<"password_scheme">>, UserProps,
<<"simple">>),
+TargetScheme = ?l2b(chttpd_util:get_chttpd_auth_config("password_scheme",
"pbkdf2")),
+CurrentPRF = couch_util:get_value(<<"pbkdf2_prf">>, UserProps, <<"sha">>),
+TargetPRF = ?l2b(chttpd_util:get_chttpd_auth_config("pbkdf2_prf",
"sha256")),
+CurrentIterations = couch_util:to_integer(
+couch_util:get_value(<<"iterations">>, UserProps, "10")
+),
+TargetIterations = chttpd_util:get_chttpd_auth_config_integer(
+"iterations", 10
+),
+if
+CurrentScheme == TargetScheme andalso TargetScheme == <<"simple">>
andalso
+CurrentIterations == TargetIterations ->
+false;
+CurrentScheme == TargetScheme andalso TargetScheme == <<"pbkdf2">>
andalso
+CurrentPRF == TargetPRF andalso CurrentIterations ==
TargetIterations ->
+false;
+true ->
+true
+end.
+
+in_progress(AuthModule, UserName) ->
+ets:member(?IN_PROGRESS_ETS, {AuthModule, UserName}).
+
+start_worker_loop(State) ->
+{WorkerPid, WorkerMon} = spawn_monitor(?MODULE, worker_loop, [self()]),
Review Comment:
I think spawn_link and just letting worker_loop's death kill
couch_password_hasher seems best / least surprising. will change.
--
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: notifications-unsubscr...@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[PR] Add Debian Bookworm for packaging [couchdb-pkg]
big-r81 opened a new pull request, #115: URL: https://github.com/apache/couchdb-pkg/pull/115 (no comment) -- 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: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] Add Debian Bookworm [couchdb-ci]
big-r81 opened a new pull request, #58: URL: https://github.com/apache/couchdb-ci/pull/58 Bumped Erlang, Elixir, Node versions ... -- 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: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Decouple offline hash strength from online [couchdb]
rnewson commented on code in PR #4814:
URL: https://github.com/apache/couchdb/pull/4814#discussion_r1378759270
##
src/couch/src/couch_password_hasher.erl:
##
@@ -45,29 +66,95 @@ start_link() ->
init(_Args) ->
hash_admin_passwords(true),
-{ok, #state{}}.
+?IN_PROGRESS_ETS = ets:new(?IN_PROGRESS_ETS, [named_table,
{read_concurrency, true}]),
+{ok, start_worker_loop(#state{})}.
handle_call(Msg, _From, #state{} = State) ->
{stop, {invalid_call, Msg}, {invalid_call, Msg}, State}.
-handle_cast({hash_passwords, Persist}, State) ->
-hash_admin_passwords(Persist),
+handle_cast({upgrade_password_hash, AuthModule, UserName, Password,
UserProps}, State) ->
+case ets:insert_new(?IN_PROGRESS_ETS, {{AuthModule, UserName}}) of
+true ->
+State#state.worker_pid !
+{upgrade_password_hash, AuthModule, UserName, Password,
UserProps};
+false ->
+ok
+end,
+{noreply, State};
+handle_cast({hash_admin_passwords, Persist}, State) ->
+hash_admin_passwords_int(Persist),
{noreply, State};
handle_cast(Msg, State) ->
{stop, {invalid_cast, Msg}, State}.
+handle_info({done, AuthModule, UserName}, State) ->
+ets:delete(?IN_PROGRESS_ETS, {AuthModule, UserName}),
+{noreply, State};
+handle_info({'DOWN', MonRef, _, _, normal}, #state{worker_mon = MonRef} =
State) ->
+ets:delete_all_objects(?IN_PROGRESS_ETS),
+{noreply, start_worker_loop(State)};
+handle_info(Msg, State) ->
+{stop, {invalid_info, Msg}, State}.
+
code_change(_OldVsn, #state{} = State, _Extra) ->
{ok, State}.
%%%===
%%% Internal functions
%%%===
-hash_admin_passwords(Persist) ->
+hash_admin_passwords_int(Persist) ->
lists:foreach(
fun({User, ClearPassword}) ->
HashedPassword =
couch_passwords:hash_admin_password(ClearPassword),
config:set("admins", User, ?b2l(HashedPassword), Persist)
end,
couch_passwords:get_unhashed_admins()
).
+
+is_admin(UserProps) ->
+Roles = couch_util:get_value(<<"roles">>, UserProps, []),
+lists:member(<<"_admin">>, Roles).
+
+needs_upgrade(UserProps) ->
+CurrentScheme = couch_util:get_value(<<"password_scheme">>, UserProps,
<<"simple">>),
+TargetScheme = ?l2b(chttpd_util:get_chttpd_auth_config("password_scheme",
"pbkdf2")),
+CurrentPRF = couch_util:get_value(<<"pbkdf2_prf">>, UserProps, <<"sha">>),
+TargetPRF = ?l2b(chttpd_util:get_chttpd_auth_config("pbkdf2_prf",
"sha256")),
+CurrentIterations = couch_util:to_integer(
+couch_util:get_value(<<"iterations">>, UserProps, "10")
+),
+TargetIterations = chttpd_util:get_chttpd_auth_config_integer(
+"iterations", 10
+),
+if
+CurrentScheme == TargetScheme andalso TargetScheme == <<"simple">>
andalso
+CurrentIterations == TargetIterations ->
+false;
+CurrentScheme == TargetScheme andalso TargetScheme == <<"pbkdf2">>
andalso
+CurrentPRF == TargetPRF andalso CurrentIterations ==
TargetIterations ->
+false;
+true ->
+true
+end.
Review Comment:
I'll try it but I struggled here to be clear and found the `if` clauses the
clearest option.
--
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: notifications-unsubscr...@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Decouple offline hash strength from online [couchdb]
rnewson commented on code in PR #4814:
URL: https://github.com/apache/couchdb/pull/4814#discussion_r1378758404
##
src/couch/src/couch_password_hasher.erl:
##
@@ -45,29 +66,95 @@ start_link() ->
init(_Args) ->
hash_admin_passwords(true),
-{ok, #state{}}.
+?IN_PROGRESS_ETS = ets:new(?IN_PROGRESS_ETS, [named_table,
{read_concurrency, true}]),
+{ok, start_worker_loop(#state{})}.
handle_call(Msg, _From, #state{} = State) ->
{stop, {invalid_call, Msg}, {invalid_call, Msg}, State}.
-handle_cast({hash_passwords, Persist}, State) ->
-hash_admin_passwords(Persist),
+handle_cast({upgrade_password_hash, AuthModule, UserName, Password,
UserProps}, State) ->
+case ets:insert_new(?IN_PROGRESS_ETS, {{AuthModule, UserName}}) of
+true ->
+State#state.worker_pid !
+{upgrade_password_hash, AuthModule, UserName, Password,
UserProps};
+false ->
+ok
+end,
+{noreply, State};
+handle_cast({hash_admin_passwords, Persist}, State) ->
+hash_admin_passwords_int(Persist),
{noreply, State};
handle_cast(Msg, State) ->
{stop, {invalid_cast, Msg}, State}.
+handle_info({done, AuthModule, UserName}, State) ->
+ets:delete(?IN_PROGRESS_ETS, {AuthModule, UserName}),
+{noreply, State};
+handle_info({'DOWN', MonRef, _, _, normal}, #state{worker_mon = MonRef} =
State) ->
+ets:delete_all_objects(?IN_PROGRESS_ETS),
+{noreply, start_worker_loop(State)};
Review Comment:
good catch, I will `_` the reason.
--
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: notifications-unsubscr...@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Decouple offline hash strength from online [couchdb]
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 view into the docs, or some internal couchdb doc on this,
as this is clearly a deliberate choice I made.
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: notifications-unsubscr...@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Decouple offline hash strength from online [couchdb]
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: notifications-unsubscr...@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Decouple offline hash strength from online [couchdb]
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 too. 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: notifications-unsubscr...@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] mango: add $beginsWith operator [couchdb]
pgj commented on PR #4810: URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1788849130 > whether there's a pragmatically high code point we could use as the high bound I was thinking about the same. Unfortunately I am not that experienced with Unicode collation and ICU and I do not have any idea. I will keep studying the implementation for some inspiration and work on the shim-based approach so we could at least have something as a fix. -- 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: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] Need to limit access to Fauxton UI to localhost, but maintain remote API access. [couchdb]
estoT1 closed issue #4827: Need to limit access to Fauxton UI to localhost, but maintain remote API access. URL: https://github.com/apache/couchdb/issues/4827 -- 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: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] Need to limit access to Fauxton UI to localhost, but maintain remote API access. [couchdb]
estoT1 commented on issue #4827: URL: https://github.com/apache/couchdb/issues/4827#issuecomment-1788763323 Thank you, this fully answers my question! Have a great day! -- 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: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] Need to limit access to Fauxton UI to localhost, but maintain remote API access. [couchdb]
rnewson commented on issue #4827: URL: https://github.com/apache/couchdb/issues/4827#issuecomment-1788722544 Got it. What you can do is remove all the files under the `share/www` directory, and then `/_utils` won't serve up those files. You can then place those files elsewhere and use haproxy/nginx/anything to serve them, putting whatever additional access restrictions you'd like over them. If you're building from source you can use the `--disable-fauxton` configure option which skips all the build steps for fauxton and skips the crucial final step of copying the output into the `share` directory in the artifact. -- 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: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] mango: add $beginsWith operator [couchdb]
willholley commented on PR #4810: URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1788606504 I wonder whether there's a pragmatically high code point we could use as the high bound in `$beginsWith` until we drop support for Centos 7 (it goes out of support next year)? -- 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: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] Need to limit access to Fauxton UI to localhost, but maintain remote API access. [couchdb]
estoT1 commented on issue #4827: URL: https://github.com/apache/couchdb/issues/4827#issuecomment-1788590244 Thank you for your prompt reply, all feedback is appreciated. From your reply I infer the Fauxton UI cannot be limited to local host bind address from the APIs? Short answer: Reducing the attack surface. Everything has its issue and must be patched. Long answer: To minimize the impact of UI vulnerabilities and assign appropriate maintenance plan/risk response plan for the CouchDB asset group. Thanking you in advance! -- 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: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] mango: add $beginsWith operator [couchdb]
willholley commented on PR #4810: URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1788538868 Looking at https://icu.unicode.org/download/, it looks like there was a [rewrite of the collator](https://icu.unicode.org/design/collation/v2) for ICU 53 which includes better support for code points. For `$beginsWith` we would need to use a different unicode character for the upper bound I think, so there's a question around whether we make that conditional on the ICU version or not. -- 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: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
