Skip to content

Commit

Permalink
Merge pull request #2223 (Configurable CN usage in SASL EXTERNAL)
Browse files Browse the repository at this point in the history
Add option to configure whether to use cn in SASL EXTERNAL authentication
  • Loading branch information
fenek committed Feb 26, 2019
2 parents 7f233c0 + ffc5385 commit 65e5bae
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 11 deletions.
33 changes: 32 additions & 1 deletion big_tests/tests/sasl_external_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ all() ->
groups() ->
G = [{fast_tls, [parallel], common_test_cases() ++ no_allowed_self_signed_test_cases()},
{just_tls, [parallel], common_test_cases() ++ no_allowed_self_signed_test_cases()},
{fast_tls_no_common_name, no_cn_test_cases()},
{just_tls_no_common_name, no_cn_test_cases()},
{fast_tls_allow_self_signed, [parallel], common_test_cases() ++ self_signed_test_cases()},
{just_tls_allow_self_signed, [parallel], common_test_cases() ++ self_signed_test_cases()}],
ct_helper:repeat_all_until_all_ok(G).
Expand All @@ -35,6 +37,12 @@ common_test_cases() ->
no_cert_fails_to_authenticate
].

no_cn_test_cases() ->
[
cert_with_cn_no_xmpp_addrs_requested_correct_user,
cert_no_cn_no_xmpp_addrs_request_name_empty
].

self_signed_test_cases() ->
[self_signed_cert_is_allowed_with_tls,
self_signed_cert_is_allowed_with_ws,
Expand Down Expand Up @@ -69,17 +77,28 @@ init_per_group(GroupName, Config) ->
"{keyfile, \"priv/ssl/fake_key.pem\"}, {password, \"\"},"
"{verify, verify_peer}," ++ verify_mode_by_group_name(GroupName) ++
"{cacertfile, \"" ++ CACertFile ++ "\"}]},"},
{authenticate_with_cn, "{authenticate_with_cn," ++ cn_by_group_name(GroupName) ++ "}"},
{auth_method, "pki"},
{sasl_mechanisms, "{sasl_mechanisms, [cyrsasl_external]}."}],
ejabberd_node_utils:modify_config_file(NewConfigValues, Config),
ejabberd_node_utils:restart_application(mongooseim),

Config.

cn_by_group_name(just_tls_no_common_name) ->
"false";
cn_by_group_name(fast_tls_no_common_name) ->
"false";
cn_by_group_name(_GroupName) ->
"true".

tls_module_by_group_name(fast_tls) ->
fast_tls;
tls_module_by_group_name(just_tls) ->
just_tls;
tls_module_by_group_name(fast_tls_no_common_name) ->
fast_tls;
tls_module_by_group_name(just_tls_no_common_name) ->
just_tls;
tls_module_by_group_name(just_tls_allow_self_signed) ->
just_tls;
tls_module_by_group_name(fast_tls_allow_self_signed) ->
Expand All @@ -89,6 +108,10 @@ ssl_options_by_group_name(fast_tls) ->
"";
ssl_options_by_group_name(just_tls) ->
"{ssl_options, [{verify_fun, {peer, false}}]},";
ssl_options_by_group_name(fast_tls_no_common_name) ->
"";
ssl_options_by_group_name(just_tls_no_common_name) ->
"{ssl_options, [{verify_fun, {peer, false}}]},";
ssl_options_by_group_name(just_tls_allow_self_signed) ->
"{ssl_options, [{verify_fun, {selfsigned_peer, true}}]},";
ssl_options_by_group_name(fast_tls_allow_self_signed) ->
Expand All @@ -98,6 +121,10 @@ verify_mode_by_group_name(fast_tls) ->
"";
verify_mode_by_group_name(just_tls) ->
"";
verify_mode_by_group_name(fast_tls_no_common_name) ->
"";
verify_mode_by_group_name(just_tls_no_common_name) ->
"";
verify_mode_by_group_name(just_tls_allow_self_signed) ->
"{verify_mode, selfsigned_peer},";
verify_mode_by_group_name(fast_tls_allow_self_signed) ->
Expand Down Expand Up @@ -143,6 +170,10 @@ cert_with_cn_no_xmpp_addrs_request_name_empty(C) ->

escalus_connection:stop(Client).

cert_no_cn_no_xmpp_addrs_request_name_empty(C) ->
UserSpec = generate_user_tcp(C, "john"),
cert_fails_to_authenticate(UserSpec).

cert_with_cn_no_xmpp_addrs_request_wrong_name(C) ->
UserSpec = [{requested_name, <<"mike@localhost">>} |
generate_user_tcp(C, "not-mike-name")],
Expand Down
4 changes: 3 additions & 1 deletion doc/authentication-methods/client-certificate.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ certificates. If the `xmpp_addrs` fields are included in the certificate, they a
When no `xmpp_addrs` are specified, the `cn` (_common name_) field might be
used, but it is optional. If there is more than one `xmpp_addrs` or both the
list and `cn` field are empty, the client must include
authorization entity.
authorization entity. By default _common name_ is used, however it can be
disabled by adding `{authenticate_with_cn, false}` tuple to the list of
`auth_opts` in MongooseIM config file.
For the details please refer to [XEP-0178 Best Practices for Use of SASL EXTERNAL with Certificates](https://xmpp.org/extensions/xep-0178.html).

### Enable compatible authentication backend
Expand Down
3 changes: 3 additions & 0 deletions rel/files/mongooseim.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,9 @@
%% {jwt_secret_source, "/path/to/file"},
%% {jwt_algorithm, "RS256"},
%% {jwt_username_key, user}
%% For cyrsasl_external
%% {authenticate_with_cn, false}
{{{authenticate_with_cn}}}
]}.

%%
Expand Down
1 change: 1 addition & 0 deletions rel/vars.config.in
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@

{sm_backend, "{mnesia, []}"}.
{auth_method, "internal"}.
{authenticate_with_cn, "{authenticate_with_cn, true}"}.
{ext_auth_script, "%%{extauth_program, \"/path/to/authentication/script\"}."}.
{tls_config, "{certfile, \"priv/ssl/fake_server.pem\"}, starttls,"}.
{tls_module, ""}.
Expand Down
24 changes: 15 additions & 9 deletions src/sasl/cyrsasl_external.erl
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,9 @@ do_mech_step(Creds, User) ->
ejabberd_auth:authorize(NewCreds)
end.

check_auth_req([], CommonName, <<"">>, _) ->
case is_binary(CommonName) of
true ->
check_auth_req([], CommonName, <<"">>, Server) ->
case ejabberd_auth:get_opt(Server, authenticate_with_cn, true) of
true when is_binary(CommonName) ->
{ok, CommonName};
_ ->
{error, <<"not-authorized">>}
Expand All @@ -94,12 +94,8 @@ check_auth_req(_, _, <<"">>, _) ->
check_auth_req([], undefined, User, Server) ->
verify_server(User, Server);
check_auth_req([], CommonName, User, Server) ->
case verify_server(User, Server) of
{ok, CommonName} ->
{ok, CommonName};
_ ->
{error, <<"not-authorized">>}
end;
CNOption = ejabberd_auth:get_opt(Server, authenticate_with_cn, true),
maybe_use_common_name(CommonName, User, Server, CNOption);
check_auth_req([_], _, _, _) ->
{error, <<"invalid-authzid">>};
check_auth_req(XmppAddrs, _, User, Server) ->
Expand All @@ -110,6 +106,16 @@ check_auth_req(XmppAddrs, _, User, Server) ->
{error, <<"not-authorized">>}
end.

maybe_use_common_name(_, User, Server, false) ->
verify_server(User, Server);
maybe_use_common_name(CommonName, User, Server, true) ->
case verify_server(User, Server) of
{ok, CommonName} ->
{ok, CommonName};
_ ->
{error, <<"not-authorized">>}
end.

get_common_name(Cert) ->
case cert_utils:get_common_name(Cert) of
error -> [];
Expand Down

0 comments on commit 65e5bae

Please sign in to comment.