Merge pull request #7427 from HJianBo/fix-psk-cipher

Fix psk cipher suites

CHANGES-4.3.md

@@ -19,6 +19,9 @@ File format:
 * add api: PUT /rules/{id}/reset_metrics.
   This api reset the metrics of the rule engine of a rule, and reset the metrics of the action related to this rule. [#7474]
 * Enhanced rule engine error handling when json parsing error.
+* Add support for `RSA-PSK-AES256-GCM-SHA384`, `RSA-PSK-AES256-CBC-SHA384`,
+ `RSA-PSK-AES128-GCM-SHA256`, `RSA-PSK-AES128-CBC-SHA256` PSK ciphers, and remove `PSK-3DES-EDE-CBC-SHA`,
+ `PSK-RC4-SHA` from the default configuration. [#7427]
 
 ### Bug fixes
 

apps/emqx_bridge_mqtt/etc/emqx_bridge_mqtt.conf

@@ -129,7 +129,7 @@ bridge.mqtt.aws.ciphers = TLS_AES_256_GCM_SHA384,TLS_AES_128_GCM_SHA256,TLS_CHAC
 ## Note that 'bridge.${BridgeName}.ciphers' and 'bridge.${BridgeName}.psk_ciphers' cannot
 ## be configured at the same time.
 ## See 'https://tools.ietf.org/html/rfc4279#section-2'.
-#bridge.mqtt.aws.psk_ciphers = PSK-AES128-CBC-SHA,PSK-AES256-CBC-SHA,PSK-3DES-EDE-CBC-SHA,PSK-RC4-SHA
+#bridge.mqtt.aws.psk_ciphers = RSA-PSK-AES256-GCM-SHA384,RSA-PSK-AES256-CBC-SHA384,RSA-PSK-AES128-GCM-SHA256,RSA-PSK-AES128-CBC-SHA256,RSA-PSK-AES256-CBC-SHA,RSA-PSK-AES128-CBC-SHA
 
 ## Ping interval of a down bridge.
 ##

apps/emqx_bridge_mqtt/priv/emqx_bridge_mqtt.schema

@@ -134,14 +134,30 @@
 ]}.
 
 {translation, "emqx_bridge_mqtt.bridges", fun(Conf) ->
-
+    AvaiableCiphers = ["RSA-PSK-AES256-GCM-SHA384","RSA-PSK-AES256-CBC-SHA384",
+                       "RSA-PSK-AES128-GCM-SHA256","RSA-PSK-AES128-CBC-SHA256",
+                       "RSA-PSK-AES256-CBC-SHA","RSA-PSK-AES128-CBC-SHA"
+                      ],
+    %% Compatible with legacy PSK Cipher strings
+    PskMapping = fun("PSK-AES128-CBC-SHA") -> {true, "RSA-PSK-AES128-CBC-SHA"};
+                    ("PSK-AES256-CBC-SHA") -> {true, "RSA-PSK-AES256-CBC-SHA"};
+                    ("PSK-3DES-EDE-CBC-SHA") -> {true, "PSK-3DES-EDE-CBC-SHA"};
+                    ("PSK-RC4-SHA") -> {true, "PSK-RC4-SHA"};
+                    (C) -> case lists:member(C, AvaiableCiphers) of
+                               true -> {true, C};
+                               false -> false
+                           end
+                 end,
     MapPSKCiphers = fun(PSKCiphers) ->
-                        lists:map(
+                      lists:filtermap(fun(C0) ->
-                          fun("PSK-AES128-CBC-SHA") -> {psk, aes_128_cbc, sha};
+                        case PskMapping(C0) of
-                             ("PSK-AES256-CBC-SHA") -> {psk, aes_256_cbc, sha};
+                            false ->
-                             ("PSK-3DES-EDE-CBC-SHA") -> {psk, '3des_ede_cbc', sha};
+                              cuttlefish:invalid(
-                             ("PSK-RC4-SHA") -> {psk, rc4_128, sha}
+                                io_lib:format("psk_ciphers: not support ~s", [C0]));
-                          end, PSKCiphers)
+                            {true, C} ->
+                                {true, C}
+                        end
+                      end, PSKCiphers)
                     end,
 
     Split = fun(undefined) -> undefined; (S) -> string:tokens(S, ",") end,

apps/emqx_bridge_mqtt/src/emqx_bridge_mqtt.app.src

@@ -1,6 +1,6 @@
 {application, emqx_bridge_mqtt,
  [{description, "EMQ X Bridge to MQTT Broker"},
-  {vsn, "4.3.4"}, % strict semver, bump manually!
+  {vsn, "4.3.5"}, % strict semver, bump manually!
   {modules, []},
   {registered, []},
   {applications, [kernel,stdlib,replayq,emqtt]},

apps/emqx_bridge_mqtt/src/emqx_bridge_mqtt.appup.src

@@ -1,6 +1,10 @@
 %% -*-: erlang -*-
 {VSN,
   [
+    {"4.3.4", [
+      %% There are only changes to the schema file, so we don't need
+      %% any commands here.
+    ]},
     {"4.3.3", [
       {load_module, emqx_bridge_mqtt, brutal_purge, soft_purge, []}
     ]},
@@ -16,6 +20,8 @@
     {<<".*">>, []}
   ],
   [
+    {"4.3.4", [
+    ]},
     {"4.3.3", [
       {load_module, emqx_bridge_mqtt, brutal_purge, soft_purge, []}
     ]},

apps/emqx_exproto/etc/emqx_exproto.conf

@@ -224,7 +224,7 @@ exproto.listener.protoname.reuseaddr = true
 ## Note that 'listener.ssl.external.ciphers' and 'listener.ssl.external.psk_ciphers' cannot
 ## be configured at the same time.
 ## See 'https://tools.ietf.org/html/rfc4279#section-2'.
-#exproto.listener.protoname.psk_ciphers = PSK-AES128-CBC-SHA,PSK-AES256-CBC-SHA,PSK-3DES-EDE-CBC-SHA,PSK-RC4-SHA
+#exproto.listener.protoname.psk_ciphers = RSA-PSK-AES256-GCM-SHA384,RSA-PSK-AES256-CBC-SHA384,RSA-PSK-AES128-GCM-SHA256,RSA-PSK-AES128-CBC-SHA256,RSA-PSK-AES256-CBC-SHA,RSA-PSK-AES128-CBC-SHA
 
 ## SSL parameter renegotiation is a feature that allows a client and a server
 ## to renegotiate the parameters of the SSL connection on the fly.

apps/emqx_exproto/priv/emqx_exproto.schema

@@ -274,13 +274,30 @@ end}.
                           {reuseaddr, cuttlefish:conf_get(Prefix ++ ".reuseaddr", Conf, undefined)}])
               end,
     SplitFun = fun(undefined) -> undefined; (S) -> string:tokens(S, ",") end,
+    AvaiableCiphers = ["RSA-PSK-AES256-GCM-SHA384","RSA-PSK-AES256-CBC-SHA384",
+                       "RSA-PSK-AES128-GCM-SHA256","RSA-PSK-AES128-CBC-SHA256",
+                       "RSA-PSK-AES256-CBC-SHA","RSA-PSK-AES128-CBC-SHA"
+                      ],
+    %% Compatible with legacy PSK Cipher strings
+    PskMapping = fun("PSK-AES128-CBC-SHA") -> {true, "RSA-PSK-AES128-CBC-SHA"};
+                    ("PSK-AES256-CBC-SHA") -> {true, "RSA-PSK-AES256-CBC-SHA"};
+                    ("PSK-3DES-EDE-CBC-SHA") -> {true, "PSK-3DES-EDE-CBC-SHA"};
+                    ("PSK-RC4-SHA") -> {true, "PSK-RC4-SHA"};
+                    (C) -> case lists:member(C, AvaiableCiphers) of
+                               true -> {true, C};
+                               false -> false
+                           end
+                 end,
     MapPSKCiphers = fun(PSKCiphers) ->
-                      lists:map(
+                      lists:filtermap(fun(C0) ->
-                          fun("PSK-AES128-CBC-SHA") -> {psk, aes_128_cbc, sha};
+                        case PskMapping(C0) of
-                             ("PSK-AES256-CBC-SHA") -> {psk, aes_256_cbc, sha};
+                            false ->
-                             ("PSK-3DES-EDE-CBC-SHA") -> {psk, '3des_ede_cbc', sha};
+                              cuttlefish:invalid(
-                             ("PSK-RC4-SHA") -> {psk, rc4_128, sha}
+                                io_lib:format("psk_ciphers: not support ~s", [C0]));
-                          end, PSKCiphers)
+                            {true, C} ->
+                                {true, C}
+                        end
+                      end, PSKCiphers)
                     end,
     SslOpts = fun(Prefix) ->
                   Versions = case SplitFun(cuttlefish:conf_get(Prefix ++ ".tls_versions", Conf, undefined)) of

apps/emqx_exproto/src/emqx_exproto.app.src

@@ -1,6 +1,6 @@
 {application, emqx_exproto,
  [{description, "EMQ X Extension for Protocol"},
-  {vsn, "4.3.6"}, %% 4.3.3 is used by ee
+  {vsn, "4.3.7"}, %% 4.3.3 is used by ee
   {modules, []},
   {registered, []},
   {mod, {emqx_exproto_app, []}},

apps/emqx_exproto/src/emqx_exproto.appup.src

@@ -1,6 +1,11 @@
 %% -*- mode: erlang -*-
 {VSN,
-  [{<<"4\\.3\\.[4-5]">>,
+  [
+   {"4.3.6",
+    [ %% There are only changes to the schema file, so we don't need any
+      %% commands here
+    ]},
+   {<<"4\\.3\\.[4-5]">>,
     [{load_module,emqx_exproto_conn,brutal_purge,soft_purge,[]},
      {load_module,emqx_exproto_channel,brutal_purge,soft_purge,[]}]},
    {<<"4\\.3\\.[2-3]">>,
@@ -12,7 +17,8 @@
      {load_module,emqx_exproto_conn,brutal_purge,soft_purge,[]},
      {load_module,emqx_exproto_channel,brutal_purge,soft_purge,[]}]},
    {<<".*">>,[]}],
-  [{<<"4\\.3\\.[4-5]">>,
+  [{"4.3.6", []},
+   {<<"4\\.3\\.[4-5]">>,
     [{load_module,emqx_exproto_conn,brutal_purge,soft_purge,[]},
      {load_module,emqx_exproto_channel,brutal_purge,soft_purge,[]}]},
    {<<"4\\.3\\.[2-3]">>,

apps/emqx_lwm2m/priv/emqx_lwm2m.schema

@@ -190,21 +190,41 @@ end}.
       case cuttlefish:conf_get("lwm2m.dtls.ciphers", Conf, undefined) of
           undefined ->
               [];
-          C ->
+          Ciphers0 ->
-              [{ciphers, SplitFun(C)}]
+              [{ciphers, SplitFun(Ciphers0)}]
       end,
   PskCiphers =
       case cuttlefish:conf_get("lwm2m.dtls.psk_ciphers", Conf, undefined) of
           undefined ->
               [];
           C2 ->
-              Psk = lists:map(fun("PSK-AES128-CBC-SHA") -> "RSA-PSK-AES128-CBC-SHA";
+              AvaiableCiphers = ["RSA-PSK-AES256-GCM-SHA384","RSA-PSK-AES256-CBC-SHA384",
-                                 ("PSK-AES256-CBC-SHA") -> "RSA-PSK-AES256-CBC-SHA";
+                                 "RSA-PSK-AES128-GCM-SHA256","RSA-PSK-AES128-CBC-SHA256",
-                                 ("PSK-3DES-EDE-CBC-SHA") -> "RSA-PSK-3DES-EDE-CBC-SHA";
+                                 "RSA-PSK-AES256-CBC-SHA","RSA-PSK-AES128-CBC-SHA"
-                                 ("PSK-RC4-SHA") -> "RSA-PSK-RC4-SHA";
+                                ],
-                                 (Suite) -> Suite
+              %% Compatible with legacy PSK Cipher strings
-                              end, SplitFun(C2)),
+              PskMapping = fun("PSK-AES128-CBC-SHA") -> {true, "RSA-PSK-AES128-CBC-SHA"};
-              [{ciphers, Psk}, {user_lookup_fun, {fun emqx_psk:lookup/3, <<>>}}]
+                              ("PSK-AES256-CBC-SHA") -> {true, "RSA-PSK-AES256-CBC-SHA"};
+                              ("PSK-3DES-EDE-CBC-SHA") -> {true, "PSK-3DES-EDE-CBC-SHA"};
+                              ("PSK-RC4-SHA") -> {true, "PSK-RC4-SHA"};
+                              (C) -> case lists:member(C, AvaiableCiphers) of
+                                         true -> {true, C};
+                                         false -> false
+                                     end
+                           end,
+              MapPSKCiphers = fun(PSKCiphers) ->
+                                lists:filtermap(fun(C0) ->
+                                  case PskMapping(C0) of
+                                      false ->
+                                        cuttlefish:invalid(
+                                          io_lib:format("psk_ciphers: not support ~s", [C0]));
+                                      {true, C} ->
+                                          {true, C}
+                                  end
+                                end, PSKCiphers)
+                              end,
+              [{ciphers, MapPSKCiphers(SplitFun(C2))},
+               {user_lookup_fun, {fun emqx_psk:lookup/3, <<>>}}]
       end,
   Ciphers /= []
   andalso PskCiphers /= []

apps/emqx_lwm2m/src/emqx_lwm2m.app.src

@@ -1,6 +1,6 @@
 {application,emqx_lwm2m,
  [{description,"EMQ X LwM2M Gateway"},
-  {vsn, "4.3.6"}, % strict semver, bump manually!
+  {vsn, "4.3.7"}, % strict semver, bump manually!
   {modules,[]},
   {registered,[emqx_lwm2m_sup]},
   {applications,[kernel,stdlib,lwm2m_coap]},

apps/emqx_lwm2m/src/emqx_lwm2m.appup.src

@@ -10,7 +10,11 @@
     [{load_module,emqx_lwm2m_protocol,brutal_purge,soft_purge,[]},
      {load_module,emqx_lwm2m_api,brutal_purge,soft_purge,[]}]},
    {"4.3.5",
-    [{load_module,emqx_lwm2m_api,brutal_purge,soft_purge,[]}]}],
+    [{load_module,emqx_lwm2m_api,brutal_purge,soft_purge,[]}]},
+   {"4.3.6",
+    [ %% There are only changes to the schema file, so we don't need any
+      %% commands here
+    ]}],
   [{<<"4\\.3\\.[0-1]">>,
     [{restart_application,emqx_lwm2m}]},
    {"4.3.2",
@@ -21,4 +25,5 @@
     [{load_module,emqx_lwm2m_protocol,brutal_purge,soft_purge,[]},
      {load_module,emqx_lwm2m_api,brutal_purge,soft_purge,[]}]},
    {"4.3.5",
-    [{load_module,emqx_lwm2m_api,brutal_purge,soft_purge,[]}]}]}.
+    [{load_module,emqx_lwm2m_api,brutal_purge,soft_purge,[]}]},
+   {"4.3.6", []}]}.

apps/emqx_psk_file/test/emqx_psk_file_SUITE.erl

@@ -15,10 +15,85 @@
 %%--------------------------------------------------------------------
 
 -module(emqx_psk_file_SUITE).
+
 -compile(nowarn_export_all).
 -compile(export_all).
 
-all() -> [].
+-include_lib("eunit/include/eunit.hrl").
 
-groups() ->
+all() ->
-    [].
+    emqx_ct:all(?MODULE).
+
+init_per_suite(Config) ->
+    emqx_ct_helpers:start_apps([emqx_psk_file], fun set_special_confs/1),
+    Config.
+
+end_per_suite(_) ->
+    emqx_ct_helpers:stop_apps([emqx_psk_file]).
+
+set_special_confs(emqx) ->
+    emqx_ct_helpers:change_emqx_opts(
+      ssl_twoway, [{ssl_options,
+                    [{versions, ['tlsv1.2','tlsv1.1', tlsv1]},
+                     {ciphers, psk_ciphers()},
+                     {user_lookup_fun,{fun emqx_psk:lookup/3,<<>>}}
+                    ]
+                   }]),
+    application:set_env(emqx, plugins_loaded_file,
+                        emqx_ct_helpers:deps_path(emqx, "test/emqx_SUITE_data/loaded_plugins"));
+set_special_confs(emqx_psk_file) ->
+    Path = emqx_ct_helpers:deps_path(emqx_psk_file, "etc/psk.txt"),
+    application:set_env(emqx_psk_file, path, Path),
+    application:set_env(emqx_psk_file, delimiter, ":");
+set_special_confs(_App) ->
+    ok.
+
+%%--------------------------------------------------------------------
+%% cases
+
+t_psk_loaded(_) ->
+    ?assertEqual({stop, <<"1234">>},
+                  emqx_psk_file:on_psk_lookup(<<"client1">>, undefined)).
+
+t_psk_ciphers(_) ->
+    lists:foreach(fun(Cipher) ->
+        {ok, C} = do_emqtt_connect(Cipher),
+        emqtt:disconnect(C)
+    end, psk_ciphers()).
+
+do_emqtt_connect(Cipher) ->
+    {ok, C} = emqtt:start_link(
+                [{proto_ver, v5},
+                 {port, 8883},
+                 {ssl, true},
+                 {ssl_opts, ssl_opts(Cipher)}
+                ]),
+    {ok, _} = emqtt:connect(C),
+    {ok, C}.
+
+psk_ciphers() ->
+    ["RSA-PSK-AES256-GCM-SHA384","RSA-PSK-AES256-CBC-SHA384",
+     "RSA-PSK-AES128-GCM-SHA256","RSA-PSK-AES128-CBC-SHA256",
+     "RSA-PSK-AES256-CBC-SHA","RSA-PSK-AES128-CBC-SHA"].
+
+ssl_opts(Cipher) ->
+    TlsFile = fun(Name) ->
+        emqx_ct_helpers:app_path(
+          emqx,
+          filename:join(["etc", "certs", Name]))
+    end,
+    [{cacertfile, TlsFile("cacert.pem")},
+     {certfile, TlsFile("client-cert.pem")},
+     {keyfile, TlsFile("client-key.pem")},
+     {verify, verify_peer},
+     {server_name_indication, disable},
+     {protocol, tls},
+     {versions, ['tlsv1.2', 'tlsv1.1']},
+     {psk_identity, "client1"},
+     {user_lookup_fun, {fun ?MODULE:on_psk_client_lookup/3, #{}}},
+     {ciphers, [Cipher]}
+    ].
+
+on_psk_client_lookup(psk, _PSKId, _UserState) ->
+    {stop, Psk} = emqx_psk_file:on_psk_lookup(<<"client1">>, undefined),
+    {ok, Psk}.

etc/emqx.conf

@@ -1468,7 +1468,7 @@ listener.ssl.external.ciphers = TLS_AES_256_GCM_SHA384,TLS_AES_128_GCM_SHA256,TL
 ## Note that 'listener.ssl.external.ciphers' and 'listener.ssl.external.psk_ciphers' cannot
 ## be configured at the same time.
 ## See 'https://tools.ietf.org/html/rfc4279#section-2'.
-#listener.ssl.external.psk_ciphers = PSK-AES128-CBC-SHA,PSK-AES256-CBC-SHA,PSK-3DES-EDE-CBC-SHA,PSK-RC4-SHA
+#listener.ssl.external.psk_ciphers = RSA-PSK-AES256-GCM-SHA384,RSA-PSK-AES256-CBC-SHA384,RSA-PSK-AES128-GCM-SHA256,RSA-PSK-AES128-CBC-SHA256,RSA-PSK-AES256-CBC-SHA,RSA-PSK-AES128-CBC-SHA
 
 ## SSL parameter renegotiation is a feature that allows a client and a server
 ## to renegotiate the parameters of the SSL connection on the fly.
@@ -1993,7 +1993,7 @@ listener.wss.external.ciphers = TLS_AES_256_GCM_SHA384,TLS_AES_128_GCM_SHA256,TL
 ## Note that 'listener.wss.external.ciphers' and 'listener.wss.external.psk_ciphers' cannot
 ## be configured at the same time.
 ## See 'https://tools.ietf.org/html/rfc4279#section-2'.
-## listener.wss.external.psk_ciphers = PSK-AES128-CBC-SHA,PSK-AES256-CBC-SHA,PSK-3DES-EDE-CBC-SHA,PSK-RC4-SHA
+## listener.wss.external.psk_ciphers = RSA-PSK-AES256-GCM-SHA384,RSA-PSK-AES256-CBC-SHA384,RSA-PSK-AES128-GCM-SHA256,RSA-PSK-AES128-CBC-SHA256,RSA-PSK-AES256-CBC-SHA,RSA-PSK-AES128-CBC-SHA
 
 ## See: listener.ssl.$name.secure_renegotiate
 ##

priv/emqx.schema

@@ -2060,13 +2060,31 @@ end}.
                           {reuseaddr, cuttlefish:conf_get(Prefix ++ ".reuseaddr", Conf, undefined)}])
               end,
     SplitFun = fun(undefined) -> undefined; (S) -> string:tokens(S, ",") end,
+    %% In erlang, we only support the following PSK ciphers (ssl_cipher:psk_suites(3))
+    AvaiableCiphers = ["RSA-PSK-AES256-GCM-SHA384","RSA-PSK-AES256-CBC-SHA384",
+                       "RSA-PSK-AES128-GCM-SHA256","RSA-PSK-AES128-CBC-SHA256",
+                       "RSA-PSK-AES256-CBC-SHA","RSA-PSK-AES128-CBC-SHA"
+                      ],
+    %% Compatible with legacy PSK Cipher strings
+    PskMapping = fun("PSK-AES128-CBC-SHA") -> {true, "RSA-PSK-AES128-CBC-SHA"};
+                    ("PSK-AES256-CBC-SHA") -> {true, "RSA-PSK-AES256-CBC-SHA"};
+                    ("PSK-3DES-EDE-CBC-SHA") -> {true, "PSK-3DES-EDE-CBC-SHA"};
+                    ("PSK-RC4-SHA") -> {true, "PSK-RC4-SHA"};
+                    (C) -> case lists:member(C, AvaiableCiphers) of
+                               true -> {true, C};
+                               false -> false
+                           end
+                 end,
     MapPSKCiphers = fun(PSKCiphers) ->
-                      lists:map(
+                      lists:filtermap(fun(C0) ->
-                          fun("PSK-AES128-CBC-SHA") -> {psk, aes_128_cbc, sha};
+                        case PskMapping(C0) of
-                             ("PSK-AES256-CBC-SHA") -> {psk, aes_256_cbc, sha};
+                            false ->
-                             ("PSK-3DES-EDE-CBC-SHA") -> {psk, '3des_ede_cbc', sha};
+                              cuttlefish:invalid(
-                             ("PSK-RC4-SHA") -> {psk, rc4_128, sha}
+                                io_lib:format("psk_ciphers: not support ~s", [C0]));
-                          end, PSKCiphers)
+                            {true, C} ->
+                                {true, C}
+                        end
+                      end, PSKCiphers)
                     end,
     SslOpts = fun(Prefix) ->
                   Versions = case SplitFun(cuttlefish:conf_get(Prefix ++ ".tls_versions", Conf, undefined)) of
@@ -2082,7 +2100,14 @@ end}.
                       {TLSCiphers, undefined} ->
                         SplitFun(TLSCiphers);
                       {undefined, PSKCiphers} ->
-                        MapPSKCiphers(SplitFun(PSKCiphers));
+                        case Versions == undefined orelse lists:member('tlsv1.3', Versions) of
+                            true ->
+                                cuttlefish:invalid(
+                                  Prefix++".tls_versions cannot contain tlsv1.3 "
+                                  "if "++Prefix++".psk_ciphers is configured");
+                            _ ->
+                                MapPSKCiphers(SplitFun(PSKCiphers))
+                        end;
                       {_TLSCiphers, _PSKCiphers} ->
                         cuttlefish:invalid(Prefix++".ciphers and "++Prefix++".psk_ciphers cannot be configured at the same time")
                     end,

rebar.config

@@ -6,6 +6,7 @@
 %% with rebar.config.erl module. Final result is written to
 %% rebar.config.rendered if environment DEBUG is set.
 
+{minimum_otp_vsn, "23"}.
 {edoc_opts, [{preprocess,true}]}.
 {erl_opts, [warn_unused_vars,warn_shadow_vars,warn_unused_import,
             warn_obsolete_guard,compressed,