yuanbiao
/
emqx
1
0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #11780 from savonarola/1017-fix-pbkdf2-validation 

fix(authn): fix pbkdf2 option validation
This commit is contained in:
Ilya Averyanov 2023-10-30 16:37:37 +02:00 committed by GitHub
parent a4ab744d4e edde661da3
commit 3f6c09b195
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 239 additions and 83 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
apps/emqx/src/emqx_passwd.erl
View File

@ -83,7 +83,7 @@ do_check_pass({_SimpleHash, _Salt, _SaltPosition} = HashParams, PasswordHash, Pa
compare_secure(Hash, PasswordHash).
-spec hash(hash_params(), password()) -> password_hash().
hash({pbkdf2, MacFun, Salt, Iterations, DKLength}, Password) ->
hash({pbkdf2, MacFun, Salt, Iterations, DKLength}, Password) when Iterations > 0 ->
case pbkdf2(MacFun, Password, Salt, Iterations, DKLength) of
{ok, HashPasswd} ->
hex(HashPasswd);

2
apps/emqx_auth/src/emqx_authn/emqx_authn_password_hashing.erl
View File

@ -92,7 +92,7 @@ fields(pbkdf2) ->
)},
{iterations,
sc(
integer(),
pos_integer(),
#{required => true, desc => "Iteration count for PBKDF2 hashing algorithm."}
)},
{dk_length, fun dk_length/1}

26
apps/emqx_auth/test/emqx_authn/emqx_authn_password_hashing_SUITE.erl
View File

@ -185,3 +185,29 @@ hash_examples() ->
}
}
].
t_pbkdf2_schema(_Config) ->
Config = fun(Iterations) ->
#{
<<"pbkdf2">> => #{
<<"name">> => <<"pbkdf2">>,
<<"mac_fun">> => <<"sha">>,
<<"iterations">> => Iterations
}
}
end,
?assertException(
throw,
{emqx_authn_password_hashing, _},
hocon_tconf:check_plain(emqx_authn_password_hashing, Config(0), #{}, [pbkdf2])
),
?assertException(
throw,
{emqx_authn_password_hashing, _},
hocon_tconf:check_plain(emqx_authn_password_hashing, Config(-1), #{}, [pbkdf2])
),
?assertMatch(
#{<<"pbkdf2">> := _},
hocon_tconf:check_plain(emqx_authn_password_hashing, Config(1), #{}, [pbkdf2])
).

118
apps/emqx_auth_mnesia/src/emqx_authn_mnesia.erl
View File

@ -50,7 +50,7 @@
%% Internal exports (RPC)
-export([
do_destroy/1,
do_add_user/2,
do_add_user/1,
do_delete_user/2,
do_update_user/3,
import/2,
@ -187,24 +187,22 @@ import_users({Filename0, FileData}, State) ->
{error, {unsupported_file_format, Extension}}
end.
add_user(UserInfo, State) ->
trans(fun ?MODULE:do_add_user/2, [UserInfo, State]).
add_user(
UserInfo,
State
) ->
UserInfoRecord = user_info_record(UserInfo, State),
trans(fun ?MODULE:do_add_user/1, [UserInfoRecord]).
do_add_user(
#{
user_id := UserID,
password := Password
} = UserInfo,
#{
user_group := UserGroup,
password_hash_algorithm := Algorithm
}
#user_info{
user_id = {_UserGroup, UserID} = DBUserID,
is_superuser = IsSuperuser
} = UserInfoRecord
) ->
case mnesia:read(?TAB, {UserGroup, UserID}, write) of
case mnesia:read(?TAB, DBUserID, write) of
[] ->
{PasswordHash, Salt} = emqx_authn_password_hashing:hash(Algorithm, Password),
IsSuperuser = maps:get(is_superuser, UserInfo, false),
insert_user(UserGroup, UserID, PasswordHash, Salt, IsSuperuser),
insert_user(UserInfoRecord),
{ok, #{user_id => UserID, is_superuser => IsSuperuser}};
[_] ->
{error, already_exist}
@ -222,38 +220,30 @@ do_delete_user(UserID, #{user_group := UserGroup}) ->
end.
update_user(UserID, UserInfo, State) ->
trans(fun ?MODULE:do_update_user/3, [UserID, UserInfo, State]).
FieldsToUpdate = fields_to_update(
UserInfo,
[
hash_and_salt,
is_superuser
],
State
),
trans(fun ?MODULE:do_update_user/3, [UserID, FieldsToUpdate, State]).
do_update_user(
UserID,
UserInfo,
FieldsToUpdate,
#{
user_group := UserGroup,
password_hash_algorithm := Algorithm
user_group := UserGroup
}
) ->
case mnesia:read(?TAB, {UserGroup, UserID}, write) of
[] ->
{error, not_found};
[
#user_info{
password_hash = PasswordHash,
salt = Salt,
is_superuser = IsSuperuser
}
] ->
NSuperuser = maps:get(is_superuser, UserInfo, IsSuperuser),
{NPasswordHash, NSalt} =
case UserInfo of
#{password := Password} ->
emqx_authn_password_hashing:hash(
Algorithm, Password
);
#{} ->
{PasswordHash, Salt}
end,
insert_user(UserGroup, UserID, NPasswordHash, NSalt, NSuperuser),
{ok, #{user_id => UserID, is_superuser => NSuperuser}}
[#user_info{} = UserInfoRecord] ->
NUserInfoRecord = update_user_record(UserInfoRecord, FieldsToUpdate),
insert_user(NUserInfoRecord),
{ok, #{user_id => UserID, is_superuser => NUserInfoRecord#user_info.is_superuser}}
end.
lookup_user(UserID, #{user_group := UserGroup}) ->
@ -391,13 +381,59 @@ get_user_info_by_seq(_, _, _) ->
{error, bad_format}.
insert_user(UserGroup, UserID, PasswordHash, Salt, IsSuperuser) ->
UserInfo = #user_info{
UserInfoRecord = user_info_record(UserGroup, UserID, PasswordHash, Salt, IsSuperuser),
insert_user(UserInfoRecord).
insert_user(#user_info{} = UserInfoRecord) ->
mnesia:write(?TAB, UserInfoRecord, write).
user_info_record(UserGroup, UserID, PasswordHash, Salt, IsSuperuser) ->
#user_info{
user_id = {UserGroup, UserID},
password_hash = PasswordHash,
salt = Salt,
is_superuser = IsSuperuser
},
mnesia:write(?TAB, UserInfo, write).
}.
user_info_record(
#{
user_id := UserID,
password := Password
} = UserInfo,
#{
password_hash_algorithm := Algorithm,
user_group := UserGroup
} = _State
) ->
IsSuperuser = maps:get(is_superuser, UserInfo, false),
{PasswordHash, Salt} = emqx_authn_password_hashing:hash(Algorithm, Password),
user_info_record(UserGroup, UserID, PasswordHash, Salt, IsSuperuser).
fields_to_update(
#{password := Password} = UserInfo,
[hash_and_salt | Rest],
#{password_hash_algorithm := Algorithm} = State
) ->
[
{hash_and_salt,
emqx_authn_password_hashing:hash(
Algorithm, Password
)}
| fields_to_update(UserInfo, Rest, State)
];
fields_to_update(#{is_superuser := IsSuperuser} = UserInfo, [is_superuser | Rest], State) ->
[{is_superuser, IsSuperuser} | fields_to_update(UserInfo, Rest, State)];
fields_to_update(UserInfo, [_ | Rest], State) ->
fields_to_update(UserInfo, Rest, State);
fields_to_update(_UserInfo, [], _State) ->
[].
update_user_record(UserInfoRecord, []) ->
UserInfoRecord;
update_user_record(UserInfoRecord, [{hash_and_salt, {PasswordHash, Salt}} | Rest]) ->
update_user_record(UserInfoRecord#user_info{password_hash = PasswordHash, salt = Salt}, Rest);
update_user_record(UserInfoRecord, [{is_superuser, IsSuperuser} | Rest]) ->
update_user_record(UserInfoRecord#user_info{is_superuser = IsSuperuser}, Rest).
%% TODO: Support other type
get_user_identity(#{username := Username}, username) ->

105
apps/emqx_auth_mnesia/src/emqx_authn_scram_mnesia.erl
View File

@ -51,7 +51,7 @@
%% Internal exports (RPC)
-export([
do_destroy/1,
do_add_user/2,
do_add_user/1,
do_delete_user/2,
do_update_user/3
]).
@ -157,19 +157,15 @@ do_destroy(UserGroup) ->
).
add_user(UserInfo, State) ->
trans(fun ?MODULE:do_add_user/2, [UserInfo, State]).
UserInfoRecord = user_info_record(UserInfo, State),
trans(fun ?MODULE:do_add_user/1, [UserInfoRecord]).
do_add_user(
#{
user_id := UserID,
password := Password
} = UserInfo,
#{user_group := UserGroup} = State
#user_info{user_id = {UserID, _} = DBUserID, is_superuser = IsSuperuser} = UserInfoRecord
) ->
case mnesia:read(?TAB, {UserGroup, UserID}, write) of
case mnesia:read(?TAB, DBUserID, write) of
[] ->
IsSuperuser = maps:get(is_superuser, UserInfo, false),
add_user(UserGroup, UserID, Password, IsSuperuser, State),
mnesia:write(?TAB, UserInfoRecord, write),
{ok, #{user_id => UserID, is_superuser => IsSuperuser}};
[_] ->
{error, already_exist}
@ -187,36 +183,28 @@ do_delete_user(UserID, #{user_group := UserGroup}) ->
end.
update_user(UserID, User, State) ->
trans(fun ?MODULE:do_update_user/3, [UserID, User, State]).
FieldsToUpdate = fields_to_update(
User,
[
keys_and_salt,
is_superuser
],
State
),
trans(fun ?MODULE:do_update_user/3, [UserID, FieldsToUpdate, State]).
do_update_user(
UserID,
User,
#{user_group := UserGroup} = State
FieldsToUpdate,
#{user_group := UserGroup} = _State
) ->
case mnesia:read(?TAB, {UserGroup, UserID}, write) of
[] ->
{error, not_found};
[#user_info{is_superuser = IsSuperuser} = UserInfo] ->
UserInfo1 = UserInfo#user_info{
is_superuser = maps:get(is_superuser, User, IsSuperuser)
},
UserInfo2 =
case maps:get(password, User, undefined) of
undefined ->
UserInfo1;
Password ->
{StoredKey, ServerKey, Salt} = esasl_scram:generate_authentication_info(
Password, State
),
UserInfo1#user_info{
stored_key = StoredKey,
server_key = ServerKey,
salt = Salt
}
end,
mnesia:write(?TAB, UserInfo2, write),
{ok, format_user_info(UserInfo2)}
[#user_info{} = UserInfo0] ->
UserInfo1 = update_user_record(UserInfo0, FieldsToUpdate),
mnesia:write(?TAB, UserInfo1, write),
{ok, format_user_info(UserInfo1)}
end.
lookup_user(UserID, #{user_group := UserGroup}) ->
@ -315,19 +303,56 @@ check_client_final_message(Bin, #{is_superuser := IsSuperuser} = Cache, #{algori
{error, not_authorized}
end.
add_user(UserGroup, UserID, Password, IsSuperuser, State) ->
{StoredKey, ServerKey, Salt} = esasl_scram:generate_authentication_info(Password, State),
write_user(UserGroup, UserID, StoredKey, ServerKey, Salt, IsSuperuser).
user_info_record(
#{
user_id := UserID,
password := Password
} = UserInfo,
#{user_group := UserGroup} = State
) ->
IsSuperuser = maps:get(is_superuser, UserInfo, false),
user_info_record(UserGroup, UserID, Password, IsSuperuser, State).
write_user(UserGroup, UserID, StoredKey, ServerKey, Salt, IsSuperuser) ->
UserInfo = #user_info{
user_info_record(UserGroup, UserID, Password, IsSuperuser, State) ->
{StoredKey, ServerKey, Salt} = esasl_scram:generate_authentication_info(Password, State),
#user_info{
user_id = {UserGroup, UserID},
stored_key = StoredKey,
server_key = ServerKey,
salt = Salt,
is_superuser = IsSuperuser
},
mnesia:write(?TAB, UserInfo, write).
}.
fields_to_update(
#{password := Password} = UserInfo,
[keys_and_salt | Rest],
State
) ->
{StoredKey, ServerKey, Salt} = esasl_scram:generate_authentication_info(Password, State),
[
{keys_and_salt, {StoredKey, ServerKey, Salt}}
| fields_to_update(UserInfo, Rest, State)
];
fields_to_update(#{is_superuser := IsSuperuser} = UserInfo, [is_superuser | Rest], State) ->
[{is_superuser, IsSuperuser} | fields_to_update(UserInfo, Rest, State)];
fields_to_update(UserInfo, [_ | Rest], State) ->
fields_to_update(UserInfo, Rest, State);
fields_to_update(_UserInfo, [], _State) ->
[].
update_user_record(UserInfoRecord, []) ->
UserInfoRecord;
update_user_record(UserInfoRecord, [{keys_and_salt, {StoredKey, ServerKey, Salt}} | Rest]) ->
update_user_record(
UserInfoRecord#user_info{
stored_key = StoredKey,
server_key = ServerKey,
salt = Salt
},
Rest
);
update_user_record(UserInfoRecord, [{is_superuser, IsSuperuser} | Rest]) ->
update_user_record(UserInfoRecord#user_info{is_superuser = IsSuperuser}, Rest).
retrieve(UserID, #{user_group := UserGroup}) ->
case mnesia:dirty_read(?TAB, {UserGroup, UserID}) of

68
apps/emqx_auth_mnesia/test/emqx_authn_scram_mnesia_SUITE.erl
View File

@ -314,6 +314,74 @@ t_update_user(_) ->
{ok, #{is_superuser := true}} = emqx_authn_scram_mnesia:lookup_user(<<"u">>, State).
t_update_user_keys(_Config) ->
Algorithm = sha512,
Username = <<"u">>,
Password = <<"p">>,
init_auth(Username, <<"badpass">>, Algorithm),
{ok, [#{state := State}]} = emqx_authn_chains:list_authenticators(?GLOBAL),
emqx_authn_scram_mnesia:update_user(
Username,
#{password => Password},
State
),
ok = emqx_config:put([mqtt, idle_timeout], 500),
{ok, Pid} = emqx_authn_mqtt_test_client:start_link("127.0.0.1", 1883),
ClientFirstMessage = esasl_scram:client_first_message(Username),
ConnectPacket = ?CONNECT_PACKET(
#mqtt_packet_connect{
proto_ver = ?MQTT_PROTO_V5,
properties = #{
'Authentication-Method' => <<"SCRAM-SHA-512">>,
'Authentication-Data' => ClientFirstMessage
}
}
),
ok = emqx_authn_mqtt_test_client:send(Pid, ConnectPacket),
?AUTH_PACKET(
?RC_CONTINUE_AUTHENTICATION,
#{'Authentication-Data' := ServerFirstMessage}
) = receive_packet(),
{continue, ClientFinalMessage, ClientCache} =
esasl_scram:check_server_first_message(
ServerFirstMessage,
#{
client_first_message => ClientFirstMessage,
password => Password,
algorithm => Algorithm
}
),
AuthContinuePacket = ?AUTH_PACKET(
?RC_CONTINUE_AUTHENTICATION,
#{
'Authentication-Method' => <<"SCRAM-SHA-512">>,
'Authentication-Data' => ClientFinalMessage
}
),
ok = emqx_authn_mqtt_test_client:send(Pid, AuthContinuePacket),
?CONNACK_PACKET(
?RC_SUCCESS,
_,
#{'Authentication-Data' := ServerFinalMessage}
) = receive_packet(),
ok = esasl_scram:check_server_final_message(
ServerFinalMessage, ClientCache#{algorithm => Algorithm}
).
t_list_users(_) ->
Config = config(),
{ok, State} = emqx_authn_scram_mnesia:create(<<"id">>, Config),

1
changes/ce/fix-11780.en.md Normal file
View File

@ -0,0 +1 @@
Fixed validation of the `iterations` field of the `pbkdf2` password hashing algorithm. Now, `iterations` must be strictly positive. Previously, it could be set to 0, which led to a nonfunctional authenticator.