From 4a7e74f5d6874210ce67a6c6fcb249ab32948cbd Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Sun, 15 Jan 2023 11:09:33 +0100 Subject: [PATCH 1/6] fix(schema): add password converter to ensure its binary() type --- apps/emqx/src/emqx_schema.erl | 20 +++++++++++++++++-- apps/emqx_conf/src/emqx_conf_schema.erl | 3 ++- .../src/emqx_connector_schema_lib.erl | 1 + .../src/mqtt/emqx_connector_mqtt_schema.erl | 3 ++- .../src/emqx_dashboard_schema.erl | 1 + apps/emqx_gateway/src/emqx_gateway_schema.erl | 8 +++++++- .../src/emqx_ee_bridge_kafka.erl | 7 ++++++- .../src/emqx_ee_connector_influxdb.erl | 8 +++++++- 8 files changed, 44 insertions(+), 7 deletions(-) diff --git a/apps/emqx/src/emqx_schema.erl b/apps/emqx/src/emqx_schema.erl index b6390081b..e4e02add7 100644 --- a/apps/emqx/src/emqx_schema.erl +++ b/apps/emqx/src/emqx_schema.erl @@ -114,6 +114,7 @@ -export([namespace/0, roots/0, roots/1, fields/1, desc/1, tags/0]). -export([conf_get/2, conf_get/3, keys/2, filter/1]). -export([server_ssl_opts_schema/2, client_ssl_opts_schema/1, ciphers_schema/1]). +-export([password_converter/2]). -export([authz_fields/0]). -export([sc/2, map/2]). @@ -1510,7 +1511,9 @@ fields("sysmon_top") -> #{ mapping => "system_monitor.db_password", default => "system_monitor_password", - desc => ?DESC(sysmon_top_db_password) + desc => ?DESC(sysmon_top_db_password), + converter => fun password_converter/2, + sensitive => true } )}, {"db_name", @@ -1900,7 +1903,8 @@ common_ssl_opts_schema(Defaults) -> required => false, example => <<"">>, format => <<"password">>, - desc => ?DESC(common_ssl_opts_schema_password) + desc => ?DESC(common_ssl_opts_schema_password), + converter => fun password_converter/2 } )}, {"versions", @@ -2068,6 +2072,18 @@ do_default_ciphers(_) -> %% otherwise resolve default ciphers list at runtime []. +password_converter(undefined, _) -> + undefined; +password_converter(I, _) when is_integer(I) -> + integer_to_binary(I); +password_converter(X, _) -> + try + iolist_to_binary(X) + catch + _:_ -> + throw("must_quote") + end. + authz_fields() -> [ {"no_match", diff --git a/apps/emqx_conf/src/emqx_conf_schema.erl b/apps/emqx_conf/src/emqx_conf_schema.erl index 7a20a88dc..5cdbc5480 100644 --- a/apps/emqx_conf/src/emqx_conf_schema.erl +++ b/apps/emqx_conf/src/emqx_conf_schema.erl @@ -408,7 +408,8 @@ fields("node") -> required => true, 'readOnly' => true, sensitive => true, - desc => ?DESC(node_cookie) + desc => ?DESC(node_cookie), + converter => fun emqx_schema:password_converter/2 } )}, {"process_limit", diff --git a/apps/emqx_connector/src/emqx_connector_schema_lib.erl b/apps/emqx_connector/src/emqx_connector_schema_lib.erl index 5d8f6941c..f64208311 100644 --- a/apps/emqx_connector/src/emqx_connector_schema_lib.erl +++ b/apps/emqx_connector/src/emqx_connector_schema_lib.erl @@ -101,6 +101,7 @@ password(desc) -> ?DESC("password"); password(required) -> false; password(format) -> <<"password">>; password(sensitive) -> true; +password(converter) -> fun emqx_schema:password_converter/2; password(_) -> undefined. auto_reconnect(type) -> boolean(); diff --git a/apps/emqx_connector/src/mqtt/emqx_connector_mqtt_schema.erl b/apps/emqx_connector/src/mqtt/emqx_connector_mqtt_schema.erl index 5e833aa99..be462fcc1 100644 --- a/apps/emqx_connector/src/mqtt/emqx_connector_mqtt_schema.erl +++ b/apps/emqx_connector/src/mqtt/emqx_connector_mqtt_schema.erl @@ -107,7 +107,8 @@ fields("server_configs") -> #{ format => <<"password">>, sensitive => true, - desc => ?DESC("password") + desc => ?DESC("password"), + converter => fun emqx_schema:password_converter/2 } )}, {clean_start, diff --git a/apps/emqx_dashboard/src/emqx_dashboard_schema.erl b/apps/emqx_dashboard/src/emqx_dashboard_schema.erl index 6742032d5..09dbaf78c 100644 --- a/apps/emqx_dashboard/src/emqx_dashboard_schema.erl +++ b/apps/emqx_dashboard/src/emqx_dashboard_schema.erl @@ -209,6 +209,7 @@ default_password(default) -> "public"; default_password(required) -> true; default_password('readOnly') -> true; default_password(sensitive) -> true; +default_password(converter) -> fun emqx_schema:password_converter/2; default_password(desc) -> ?DESC(default_password); default_password(_) -> undefined. diff --git a/apps/emqx_gateway/src/emqx_gateway_schema.erl b/apps/emqx_gateway/src/emqx_gateway_schema.erl index 804e1f862..4ea845ea1 100644 --- a/apps/emqx_gateway/src/emqx_gateway_schema.erl +++ b/apps/emqx_gateway/src/emqx_gateway_schema.erl @@ -380,7 +380,13 @@ fields(ssl_server_opts) -> fields(clientinfo_override) -> [ {username, sc(binary(), #{desc => ?DESC(gateway_common_clientinfo_override_username)})}, - {password, sc(binary(), #{desc => ?DESC(gateway_common_clientinfo_override_password)})}, + {password, + sc(binary(), #{ + desc => ?DESC(gateway_common_clientinfo_override_password), + sensitive => true, + format => <<"password">>, + converter => fun emqx_schema:password_converter/2 + })}, {clientid, sc(binary(), #{desc => ?DESC(gateway_common_clientinfo_override_clientid)})} ]; fields(lwm2m_translators) -> diff --git a/lib-ee/emqx_ee_bridge/src/emqx_ee_bridge_kafka.erl b/lib-ee/emqx_ee_bridge/src/emqx_ee_bridge_kafka.erl index 4a6c1411c..e694f6c15 100644 --- a/lib-ee/emqx_ee_bridge/src/emqx_ee_bridge_kafka.erl +++ b/lib-ee/emqx_ee_bridge/src/emqx_ee_bridge_kafka.erl @@ -116,7 +116,12 @@ fields(auth_username_password) -> })}, {username, mk(binary(), #{required => true, desc => ?DESC(auth_sasl_username)})}, {password, - mk(binary(), #{required => true, sensitive => true, desc => ?DESC(auth_sasl_password)})} + mk(binary(), #{ + required => true, + sensitive => true, + desc => ?DESC(auth_sasl_password), + converter => fun emqx_schema:password_converter/2 + })} ]; fields(auth_gssapi_kerberos) -> [ diff --git a/lib-ee/emqx_ee_connector/src/emqx_ee_connector_influxdb.erl b/lib-ee/emqx_ee_connector/src/emqx_ee_connector_influxdb.erl index 553e5369f..ff8c31997 100644 --- a/lib-ee/emqx_ee_connector/src/emqx_ee_connector_influxdb.erl +++ b/lib-ee/emqx_ee_connector/src/emqx_ee_connector_influxdb.erl @@ -157,7 +157,13 @@ fields(influxdb_api_v1) -> [ {database, mk(binary(), #{required => true, desc => ?DESC("database")})}, {username, mk(binary(), #{desc => ?DESC("username")})}, - {password, mk(binary(), #{desc => ?DESC("password"), format => <<"password">>})} + {password, + mk(binary(), #{ + desc => ?DESC("password"), + format => <<"password">>, + sensitive => true, + converter => fun emqx_schema:password_converter/2 + })} ] ++ emqx_connector_schema_lib:ssl_fields(); fields(influxdb_api_v2) -> fields(common) ++ From 2afbf6a406b6a5a1af2ac4c7eeeff2ff12aadc4b Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Sun, 15 Jan 2023 12:43:25 +0100 Subject: [PATCH 2/6] test: unlink process which is getting shutdown --- apps/emqx_authn/test/emqx_authn_SUITE.erl | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/emqx_authn/test/emqx_authn_SUITE.erl b/apps/emqx_authn/test/emqx_authn_SUITE.erl index 6b24f7231..b30d2fcff 100644 --- a/apps/emqx_authn/test/emqx_authn_SUITE.erl +++ b/apps/emqx_authn/test/emqx_authn_SUITE.erl @@ -98,6 +98,7 @@ t_will_message_connection_denied(Config) when is_list(Config) -> {will_payload, <<"should not be published">>} ]), Ref = monitor(process, Publisher), + _ = unlink(Publisher), {error, _} = emqtt:connect(Publisher), receive {'DOWN', Ref, process, Publisher, Reason} -> From 263deae1f3250e5a7a05a4071dc65b7849e39177 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Mon, 16 Jan 2023 10:59:37 +0100 Subject: [PATCH 3/6] refactor: add a more generic name for password_converter --- apps/emqx/src/emqx_schema.erl | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/apps/emqx/src/emqx_schema.erl b/apps/emqx/src/emqx_schema.erl index e4e02add7..8f016f684 100644 --- a/apps/emqx/src/emqx_schema.erl +++ b/apps/emqx/src/emqx_schema.erl @@ -114,7 +114,7 @@ -export([namespace/0, roots/0, roots/1, fields/1, desc/1, tags/0]). -export([conf_get/2, conf_get/3, keys/2, filter/1]). -export([server_ssl_opts_schema/2, client_ssl_opts_schema/1, ciphers_schema/1]). --export([password_converter/2]). +-export([password_converter/2, bin_str_converter/2]). -export([authz_fields/0]). -export([sc/2, map/2]). @@ -2072,11 +2072,14 @@ do_default_ciphers(_) -> %% otherwise resolve default ciphers list at runtime []. -password_converter(undefined, _) -> +password_converter(X, Opts) -> + bin_str_converter(X, Opts). + +bin_str_converter(undefined, _) -> undefined; -password_converter(I, _) when is_integer(I) -> +bin_str_converter(I, _) when is_integer(I) -> integer_to_binary(I); -password_converter(X, _) -> +bin_str_converter(X, _) -> try iolist_to_binary(X) catch From b793aad34496e35eec137a972143827f2bbe5ce1 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Tue, 17 Jan 2023 11:21:01 +0100 Subject: [PATCH 4/6] docs: add changelog --- changes/v5.0.15/fix-9765.en.md | 6 ++++++ changes/v5.0.15/fix-9765.zh.md | 7 +++++++ 2 files changed, 13 insertions(+) create mode 100644 changes/v5.0.15/fix-9765.en.md create mode 100644 changes/v5.0.15/fix-9765.zh.md diff --git a/changes/v5.0.15/fix-9765.en.md b/changes/v5.0.15/fix-9765.en.md new file mode 100644 index 000000000..8c1ed4ad1 --- /dev/null +++ b/changes/v5.0.15/fix-9765.en.md @@ -0,0 +1,6 @@ +Parse decimals as password from environment variable overrides correctly. +Prior to this change, config values for passwords are not allowed to be decimals. +e.g. `EMQX_FOOBAR__PASSWORD=12344` or `emqx.foobar.password=1234` +would result in a type check error, unless quoted as: +`EMQX_FOOBAR__PASSWORD='"12344"'` or `emqx.foobar.password="1234"`. +After this fix, the value does not have to be auoted. diff --git a/changes/v5.0.15/fix-9765.zh.md b/changes/v5.0.15/fix-9765.zh.md new file mode 100644 index 000000000..dd0b6a79c --- /dev/null +++ b/changes/v5.0.15/fix-9765.zh.md @@ -0,0 +1,7 @@ +允许使用纯数字作为密码配置。 +在此修复前，密码的配置必须是字符串，使用纯数字时，会报类型检查错误。 +例如，`EMQX_FOOBAR__PASSWORD=12344` 或 `emqx.foobar.password=1234` 会出错， +必须用引把值括起来才行： +`EMQX_FOOBAR__PASSWORD='"12344"'` 或 `emqx.foobar.password="1234"`。 +修复后可以不使用引号。在环境变量重载中使用更加方便。 + From fc992f28bc78a743239ec1eafea221fc754c95aa Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Tue, 17 Jan 2023 20:13:01 +0100 Subject: [PATCH 5/6] test: add test coverage --- apps/emqx/test/emqx_schema_tests.erl | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/apps/emqx/test/emqx_schema_tests.erl b/apps/emqx/test/emqx_schema_tests.erl index b249dea92..e1ac1874f 100644 --- a/apps/emqx/test/emqx_schema_tests.erl +++ b/apps/emqx/test/emqx_schema_tests.erl @@ -465,3 +465,10 @@ converter_invalid_input_test() -> ?assertEqual(undefined, emqx_schema:convert_servers(undefined)), %% 'foo: bar' is a valid HOCON value, but 'bar' is not a port number ?assertThrow("bad_host_port", emqx_schema:convert_servers(#{foo => bar})). + +password_converter_test() -> + ?assertEqual(undefined, emqx_schema:password_converter(undefined, #{})), + ?assertEqual(<<"123">>, emqx_schema:password_converter(123, #{})), + ?assertEqual(<<"123">>, emqx_schema:password_converter(<<"123">>, #{})), + ?assertThrow("must_quote", emqx_schema:password_converter(foobar, #{})), + ok. From 5f5f34bd94e1aed47760ee16f4fecfac587ba6a0 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Wed, 18 Jan 2023 14:47:23 +0100 Subject: [PATCH 6/6] docs: fix a typo in changelog --- changes/v5.0.15/fix-9765.en.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changes/v5.0.15/fix-9765.en.md b/changes/v5.0.15/fix-9765.en.md index 8c1ed4ad1..7de7e55f3 100644 --- a/changes/v5.0.15/fix-9765.en.md +++ b/changes/v5.0.15/fix-9765.en.md @@ -3,4 +3,4 @@ Prior to this change, config values for passwords are not allowed to be decimals e.g. `EMQX_FOOBAR__PASSWORD=12344` or `emqx.foobar.password=1234` would result in a type check error, unless quoted as: `EMQX_FOOBAR__PASSWORD='"12344"'` or `emqx.foobar.password="1234"`. -After this fix, the value does not have to be auoted. +After this fix, the value does not have to be quoted.