diff --git a/.ci/docker-compose-file/docker-compose-ldap.yaml b/.ci/docker-compose-file/docker-compose-ldap.yaml index f92df47a0..03627c763 100644 --- a/.ci/docker-compose-file/docker-compose-ldap.yaml +++ b/.ci/docker-compose-file/docker-compose-ldap.yaml @@ -6,8 +6,6 @@ services: build: context: ../.. dockerfile: .ci/docker-compose-file/openldap/Dockerfile - args: - LDAP_TAG: ${LDAP_TAG} image: openldap #ports: # - 389:389 diff --git a/.ci/docker-compose-file/openldap/Dockerfile b/.ci/docker-compose-file/openldap/Dockerfile index dd0114b64..9eba7b3a5 100644 --- a/.ci/docker-compose-file/openldap/Dockerfile +++ b/.ci/docker-compose-file/openldap/Dockerfile @@ -1,13 +1,4 @@ -FROM buildpack-deps:bookworm - -ARG LDAP_TAG=2.5.16 - -RUN apt-get update && apt-get install -y groff groff-base -RUN wget https://www.openldap.org/software/download/OpenLDAP/openldap-release/openldap-${LDAP_TAG}.tgz \ - && tar xvzf openldap-${LDAP_TAG}.tgz \ - && cd openldap-${LDAP_TAG} \ - && ./configure && make depend && make && make install \ - && cd .. && rm -rf openldap-${LDAP_TAG} +FROM docker.io/zmstone/openldap:2.5.16 COPY .ci/docker-compose-file/openldap/slapd.conf /usr/local/etc/openldap/slapd.conf COPY apps/emqx_ldap/test/data/emqx.io.ldif /usr/local/etc/openldap/schema/emqx.io.ldif diff --git a/.ci/docker-compose-file/openldap/slapd.conf b/.ci/docker-compose-file/openldap/slapd.conf index d6ba20caa..984cf3b4c 100644 --- a/.ci/docker-compose-file/openldap/slapd.conf +++ b/.ci/docker-compose-file/openldap/slapd.conf @@ -1,14 +1,13 @@ include /usr/local/etc/openldap/schema/core.schema include /usr/local/etc/openldap/schema/cosine.schema include /usr/local/etc/openldap/schema/inetorgperson.schema -include /usr/local/etc/openldap/schema/ppolicy.schema include /usr/local/etc/openldap/schema/emqx.schema TLSCACertificateFile /usr/local/etc/openldap/cacert.pem TLSCertificateFile /usr/local/etc/openldap/cert.pem TLSCertificateKeyFile /usr/local/etc/openldap/key.pem -database bdb +database mdb suffix "dc=emqx,dc=io" rootdn "cn=root,dc=emqx,dc=io" rootpw {SSHA}eoF7NhNrejVYYyGHqnt+MdKNBh4r1w3W diff --git a/apps/emqx/include/emqx_release.hrl b/apps/emqx/include/emqx_release.hrl index 801773663..381c5fa13 100644 --- a/apps/emqx/include/emqx_release.hrl +++ b/apps/emqx/include/emqx_release.hrl @@ -32,10 +32,10 @@ %% `apps/emqx/src/bpapi/README.md' %% Opensource edition --define(EMQX_RELEASE_CE, "5.2.1"). +-define(EMQX_RELEASE_CE, "5.3.0"). %% Enterprise edition --define(EMQX_RELEASE_EE, "5.3.0-alpha.2"). +-define(EMQX_RELEASE_EE, "5.3.0-rc.1"). %% The HTTP API version -define(EMQX_API_VERSION, "5.0"). diff --git a/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_ldap.erl b/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_ldap.erl index dd354a007..de789df0b 100644 --- a/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_ldap.erl +++ b/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_ldap.erl @@ -124,15 +124,17 @@ login( of {ok, []} -> {error, user_not_found}; - {ok, [Entry | _]} -> + {ok, [Entry]} -> case emqx_resource:simple_sync_query( ResourceId, {bind, Entry#eldap_entry.object_name, Sign} ) of - ok -> + {ok, #{result := ok}} -> ensure_user_exists(Username); + {ok, #{result := 'invalidCredentials'} = Reason} -> + {error, Reason}; {error, _} = Error -> Error end; diff --git a/apps/emqx_dashboard_sso/test/emqx_dashboard_sso_ldap_SUITE.erl b/apps/emqx_dashboard_sso/test/emqx_dashboard_sso_ldap_SUITE.erl index 048a4923a..8966ffca9 100644 --- a/apps/emqx_dashboard_sso/test/emqx_dashboard_sso_ldap_SUITE.erl +++ b/apps/emqx_dashboard_sso/test/emqx_dashboard_sso_ldap_SUITE.erl @@ -13,8 +13,12 @@ -define(LDAP_HOST, "ldap"). -define(LDAP_DEFAULT_PORT, 389). --define(LDAP_USER, <<"mqttuser0001">>). --define(LDAP_USER_PASSWORD, <<"mqttuser0001">>). +-define(LDAP_USER, <<"viewer1">>). +-define(LDAP_USER_PASSWORD, <<"viewer1">>). +-define(LDAP_BASE_DN, <<"ou=dashboard,dc=emqx,dc=io">>). +-define(LDAP_FILTER_WITH_UID, <<"(uid=${username})">>). +%% there are more than one users in this group +-define(LDAP_FILTER_WITH_GROUP, <<"(ugroup=group1)">>). -define(MOD_TAB, emqx_dashboard_sso). -define(MOD_KEY_PATH, [dashboard, sso, ldap]). @@ -22,6 +26,7 @@ -import(emqx_mgmt_api_test_util, [request/2, request/3, uri/1, request_api/3]). +%% order matters all() -> [ t_bad_create, @@ -31,6 +36,7 @@ all() -> t_login_with_bad, t_first_login, t_next_login, + t_more_than_one_user_matched, t_bad_update, t_delete ]. @@ -46,12 +52,13 @@ end_per_suite(_Config) -> [emqx_dashboard_admin:remove_user(Name) || #{username := Name} <- All], emqx_mgmt_api_test_util:end_suite([emqx_conf, emqx_dashboard_sso]). -init_per_testcase(_, Config) -> +init_per_testcase(Case, Config) -> {ok, _} = emqx_cluster_rpc:start_link(), + ?MODULE:Case({init, Config}), Config. -end_per_testcase(Case, _) -> - Case =:= t_delete_backend andalso emqx_dashboard_sso_manager:delete(ldap), +end_per_testcase(Case, Config) -> + ?MODULE:Case({'end', Config}), case erlang:whereis(node()) of undefined -> ok; @@ -61,6 +68,10 @@ end_per_testcase(Case, _) -> end, ok. +t_bad_create({init, Config}) -> + Config; +t_bad_create({'end', _}) -> + ok; t_bad_create(_) -> Path = uri(["sso", "ldap"]), ?assertMatch( @@ -90,6 +101,10 @@ t_bad_create(_) -> ), ok. +t_create({init, Config}) -> + Config; +t_create({'end', _Config}) -> + ok; t_create(_) -> check_running([]), Path = uri(["sso", "ldap"]), @@ -109,6 +124,10 @@ t_create(_) -> ?assertNotEqual(undefined, emqx_dashboard_sso_manager:lookup_state(ldap)), ok. +t_update({init, Config}) -> + Config; +t_update({'end', _Config}) -> + ok; t_update(_) -> Path = uri(["sso", "ldap"]), {ok, 200, Result} = request(put, Path, ldap_config(#{<<"enable">> => <<"true">>})), @@ -118,6 +137,10 @@ t_update(_) -> ?assertNotEqual(undefined, emqx_dashboard_sso_manager:lookup_state(ldap)), ok. +t_get({init, Config}) -> + Config; +t_get({'end', _Config}) -> + ok; t_get(_) -> Path = uri(["sso", "ldap"]), {ok, 200, Result} = request(get, Path), @@ -127,6 +150,10 @@ t_get(_) -> {ok, 400, _} = request(get, NotExists), ok. +t_login_with_bad({init, Config}) -> + Config; +t_login_with_bad({'end', _Config}) -> + ok; t_login_with_bad(_) -> Path = uri(["sso", "login", "ldap"]), Req = #{ @@ -138,6 +165,10 @@ t_login_with_bad(_) -> ?assertMatch(#{code := <<"BAD_USERNAME_OR_PWD">>}, decode_json(Result)), ok. +t_first_login({init, Config}) -> + Config; +t_first_login({'end', _Config}) -> + ok; t_first_login(_) -> Path = uri(["sso", "login", "ldap"]), Req = #{ @@ -154,6 +185,10 @@ t_first_login(_) -> ), ok. +t_next_login({init, Config}) -> + Config; +t_next_login({'end', _Config}) -> + ok; t_next_login(_) -> Path = uri(["sso", "login", "ldap"]), Req = #{ @@ -165,6 +200,38 @@ t_next_login(_) -> ?assertMatch(#{license := _, token := _}, decode_json(Result)), ok. +t_more_than_one_user_matched({init, Config}) -> + emqx_logger:set_primary_log_level(error), + Config; +t_more_than_one_user_matched({'end', _Config}) -> + %% restore default config + Path = uri(["sso", "ldap"]), + {ok, 200, _} = request(put, Path, ldap_config(#{<<"enable">> => true})), + ok; +t_more_than_one_user_matched(_) -> + Path = uri(["sso", "ldap"]), + %% change to query with ugroup=group1 + NewConfig = ldap_config(#{ + <<"enable">> => true, + <<"base_dn">> => ?LDAP_BASE_DN, + <<"filter">> => ?LDAP_FILTER_WITH_GROUP + }), + ?assertMatch({ok, 200, _}, request(put, Path, NewConfig)), + check_running([<<"ldap">>]), + Path1 = uri(["sso", "login", "ldap"]), + Req = #{ + <<"backend">> => <<"ldap">>, + <<"username">> => ?LDAP_USER, + <<"password">> => ?LDAP_USER_PASSWORD + }, + {ok, 401, Result} = request(post, Path1, Req), + ?assertMatch(#{code := <<"BAD_USERNAME_OR_PWD">>}, decode_json(Result)), + ok. + +t_bad_update({init, Config}) -> + Config; +t_bad_update({'end', _Config}) -> + ok; t_bad_update(_) -> Path = uri(["sso", "ldap"]), ?assertMatch( @@ -184,9 +251,12 @@ t_bad_update(_) -> ?assertMatch( [#{backend := <<"ldap">>, enable := true, running := false, last_error := _}], get_sso() ), - ok. +t_delete({init, Config}) -> + Config; +t_delete({'end', _Config}) -> + ok; t_delete(_) -> Path = uri(["sso", "ldap"]), ?assertMatch({ok, 204, _}, request(delete, Path)), @@ -214,8 +284,8 @@ ldap_config(Override) -> <<"backend">> => <<"ldap">>, <<"enable">> => <<"false">>, <<"server">> => ldap_server(), - <<"base_dn">> => <<"uid=${username},ou=testdevice,dc=emqx,dc=io">>, - <<"filter">> => <<"(objectClass=mqttUser)">>, + <<"base_dn">> => ?LDAP_BASE_DN, + <<"filter">> => ?LDAP_FILTER_WITH_UID, <<"username">> => <<"cn=root,dc=emqx,dc=io">>, <<"password">> => <<"public">>, <<"pool_size">> => 8 diff --git a/apps/emqx_ldap/src/emqx_ldap.erl b/apps/emqx_ldap/src/emqx_ldap.erl index 66ce11e20..628397ff3 100644 --- a/apps/emqx_ldap/src/emqx_ldap.erl +++ b/apps/emqx_ldap/src/emqx_ldap.erl @@ -249,7 +249,7 @@ do_ldap_query( #{pool_name := PoolName} = State ) -> LogMeta = #{connector => InstId, search => SearchOptions, state => emqx_utils:redact(State)}, - ?TRACE("QUERY", "ldap_connector_received", LogMeta), + ?TRACE("QUERY", "ldap_connector_received_query", LogMeta), case ecpool:pick_and_do( PoolName, @@ -262,7 +262,25 @@ do_ldap_query( ldap_connector_query_return, #{result => Result} ), - {ok, Result#eldap_search_result.entries}; + case Result#eldap_search_result.entries of + [_] = Entry -> + {ok, Entry}; + [_ | _] = L -> + %% Accept only a single exact match. + %% Multiple matches likely indicate: + %% 1. A misconfiguration in EMQX, allowing overly broad query conditions. + %% 2. Indistinguishable entries in the LDAP database. + %% Neither scenario should be allowed to proceed. + Msg = "ldap_query_found_more_than_one_match", + ?SLOG( + error, + LogMeta#{ + msg => "ldap_query_found_more_than_one_match", + count => length(L) + } + ), + {error, {unrecoverable_error, Msg}} + end; {error, 'noSuchObject'} -> {ok, []}; {error, Reason} -> diff --git a/apps/emqx_ldap/src/emqx_ldap_authn.erl b/apps/emqx_ldap/src/emqx_ldap_authn.erl index cbe62dfb2..16540bec8 100644 --- a/apps/emqx_ldap/src/emqx_ldap_authn.erl +++ b/apps/emqx_ldap/src/emqx_ldap_authn.erl @@ -131,7 +131,7 @@ authenticate( of {ok, []} -> ignore; - {ok, [Entry | _]} -> + {ok, [Entry]} -> is_enabled(Password, Entry, State); {error, Reason} -> ?TRACE_AUTHN_PROVIDER(error, "ldap_query_failed", #{ diff --git a/apps/emqx_ldap/src/emqx_ldap_authn_bind.erl b/apps/emqx_ldap/src/emqx_ldap_authn_bind.erl index fe192bf88..fce78312c 100644 --- a/apps/emqx_ldap/src/emqx_ldap_authn_bind.erl +++ b/apps/emqx_ldap/src/emqx_ldap_authn_bind.erl @@ -95,15 +95,21 @@ authenticate( of {ok, []} -> ignore; - {ok, [Entry | _]} -> + {ok, [Entry]} -> case emqx_resource:simple_sync_query( ResourceId, {bind, Entry#eldap_entry.object_name, Credential} ) of - ok -> + {ok, #{result := ok}} -> {ok, #{is_superuser => false}}; + {ok, #{result := 'invalidCredentials'}} -> + ?TRACE_AUTHN_PROVIDER(error, "ldap_bind_failed", #{ + resource => ResourceId, + reason => 'invalidCredentials' + }), + {error, bad_username_or_password}; {error, Reason} -> ?TRACE_AUTHN_PROVIDER(error, "ldap_bind_failed", #{ resource => ResourceId, diff --git a/apps/emqx_ldap/src/emqx_ldap_authz.erl b/apps/emqx_ldap/src/emqx_ldap_authz.erl index b48b3a48b..bfff9149f 100644 --- a/apps/emqx_ldap/src/emqx_ldap_authz.erl +++ b/apps/emqx_ldap/src/emqx_ldap_authz.erl @@ -111,11 +111,11 @@ authorize( case emqx_resource:simple_sync_query(ResourceID, {query, Client, Attrs, QueryTimeout}) of {ok, []} -> nomatch; - {ok, [Entry | _]} -> + {ok, [Entry]} -> do_authorize(Action, Topic, Attrs, Entry); {error, Reason} -> ?SLOG(error, #{ - msg => "query_ldap_error", + msg => "ldap_query_failed", reason => emqx_utils:redact(Reason), resource_id => ResourceID }), diff --git a/apps/emqx_ldap/src/emqx_ldap_bind_worker.erl b/apps/emqx_ldap/src/emqx_ldap_bind_worker.erl index 27310c7ff..1b1bd3ce9 100644 --- a/apps/emqx_ldap/src/emqx_ldap_bind_worker.erl +++ b/apps/emqx_ldap/src/emqx_ldap_bind_worker.erl @@ -80,7 +80,9 @@ on_query( ldap_connector_query_return, #{result => ok} ), - ok; + {ok, #{result => ok}}; + {error, 'invalidCredentials'} -> + {ok, #{result => 'invalidCredentials'}}; {error, Reason} -> ?SLOG( error, diff --git a/apps/emqx_ldap/test/data/emqx.io.ldif b/apps/emqx_ldap/test/data/emqx.io.ldif index 138651958..71a1bb3fc 100644 --- a/apps/emqx_ldap/test/data/emqx.io.ldif +++ b/apps/emqx_ldap/test/data/emqx.io.ldif @@ -13,6 +13,12 @@ objectClass: top objectclass:organizationalUnit ou:testdevice +# create dashboard.emqx.io +dn:ou=dashboard,dc=emqx,dc=io +objectClass: top +objectclass:organizationalUnit +ou:dashboard + # create user admin dn:uid=admin,ou=testdevice,dc=emqx,dc=io objectClass: top @@ -150,3 +156,23 @@ objectClass: mqttSecurity uid: mqttuser0007 isSuperuser: TRUE userPassword: {SHA}axpQGbl00j3jvOG058y313ocnBk= + +## Try to test with base DN 'ou=dashboard,dc=emqx,dc=io' +## with a filter ugroup=group1 +## this should return 2 users in the query and fail the test + +## echo -n "viewer1" | sha1sum | cut -d' ' -f1 | xxd -r -p | base64 +dn:uid=viewer1,ou=dashboard,dc=emqx,dc=io +objectClass: top +objectClass: dashboardUser +uid: viewer1 +ugroup: group1 +userPassword: {SHA}I/LgVpQ6joiHifK7pZEQ1+0AUlg= + +## echo -n "viewer2" | sha1sum | cut -d' ' -f1 | xxd -r -p | base64 +dn:uid=viewer2,ou=dashboard,dc=emqx,dc=io +objectClass: top +objectClass: dashboardUser +uid: viewer2 +ugroup: group1 +userPassword: {SHA}SR0qZpf8pYKKAbn6ILFvX91JuQg= diff --git a/apps/emqx_ldap/test/data/emqx.schema b/apps/emqx_ldap/test/data/emqx.schema index d08548272..4ecc37bb7 100644 --- a/apps/emqx_ldap/test/data/emqx.schema +++ b/apps/emqx_ldap/test/data/emqx.schema @@ -35,10 +35,11 @@ attributetype ( 1.3.6.1.4.1.11.2.53.2.2.3.1.2.3.4.4 NAME ( 'mqttAccountName' 'ma SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 USAGE userApplications ) - -objectclass ( 1.3.6.1.4.1.11.2.53.2.2.3.1.2.3.4 NAME 'mqttUser' - AUXILIARY - MAY ( mqttPublishTopic $ mqttSubscriptionTopic $ mqttPubSubTopic $ mqttAccountName $ isSuperuser) ) +attributetype ( 1.3.6.1.4.1.11.2.53.2.2.3.1.2.3.5.1 NAME 'ugroup' + EQUALITY caseIgnoreMatch + SUBSTR caseIgnoreSubstringsMatch + SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 + USAGE userApplications ) objectclass ( 1.3.6.1.4.1.11.2.53.2.2.3.1.2.3.2 NAME 'mqttDevice' SUP top @@ -50,3 +51,13 @@ objectclass ( 1.3.6.1.4.1.11.2.53.2.2.3.1.2.3.3 NAME 'mqttSecurity' SUP top AUXILIARY MUST ( userPassword ) ) + +objectclass ( 1.3.6.1.4.1.11.2.53.2.2.3.1.2.3.4 NAME 'mqttUser' + AUXILIARY + MAY ( mqttPublishTopic $ mqttSubscriptionTopic $ mqttPubSubTopic $ mqttAccountName $ isSuperuser ) ) + +objectclass (1.3.6.1.4.1.11.2.53.2.2.3.1.2.3.5 NAME 'dashboardUser' + SUP top + STRUCTURAL + MUST ( uid $ userPassword ) + MAY ( ugroup )) diff --git a/apps/emqx_management/test/emqx_mgmt_api_test_util.erl b/apps/emqx_management/test/emqx_mgmt_api_test_util.erl index e6c3bb3d6..cd768a529 100644 --- a/apps/emqx_management/test/emqx_mgmt_api_test_util.erl +++ b/apps/emqx_management/test/emqx_mgmt_api_test_util.erl @@ -119,7 +119,7 @@ do_request_api(Method, Request, Opts) -> ReturnAll = maps:get(return_all, Opts, false), CompatibleMode = maps:get(compatible_mode, Opts, false), HttpcReqOpts = maps:get(httpc_req_opts, Opts, []), - ct:pal("Method: ~p, Request: ~p, Opts: ~p", [Method, Request, Opts]), + ct:pal("~p: ~p~nOpts: ~p", [Method, Request, Opts]), case httpc:request(Method, Request, [], HttpcReqOpts) of {error, socket_closed_remotely} -> {error, socket_closed_remotely}; diff --git a/deploy/charts/emqx-enterprise/Chart.yaml b/deploy/charts/emqx-enterprise/Chart.yaml index c89a7f90d..e0720bad9 100644 --- a/deploy/charts/emqx-enterprise/Chart.yaml +++ b/deploy/charts/emqx-enterprise/Chart.yaml @@ -14,8 +14,8 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. -version: 5.3.0-alpha.2 +version: 5.3.0-rc.1 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. -appVersion: 5.3.0-alpha.2 +appVersion: 5.3.0-rc.1 diff --git a/deploy/charts/emqx/Chart.yaml b/deploy/charts/emqx/Chart.yaml index d32268ee2..f1c26cc8d 100644 --- a/deploy/charts/emqx/Chart.yaml +++ b/deploy/charts/emqx/Chart.yaml @@ -14,8 +14,8 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. -version: 5.2.1 +version: 5.3.0 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. -appVersion: 5.2.1 +appVersion: 5.3.0