From 04523b3f8153e8c35b80c30e43e385cce8c8c4a5 Mon Sep 17 00:00:00 2001
From: Andrew Mayorov <encube.ul@gmail.com>
Date: Wed, 19 Apr 2023 18:48:40 +0300
Subject: [PATCH 1/2] fix(ft): restrict max filename length in transfers

Reject transfers with too long filenames right away, during `init`
handling, to avoid having to deal with filesystem errors later.
---
 apps/emqx_ft/src/emqx_ft.erl        |  6 +++---
 apps/emqx_ft/src/emqx_ft_schema.erl | 15 ++++++++++++++-
 apps/emqx_ft/test/emqx_ft_SUITE.erl | 11 ++++++++++-
 3 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/apps/emqx_ft/src/emqx_ft.erl b/apps/emqx_ft/src/emqx_ft.erl
index 6ac6596ff..45207baa9 100644
--- a/apps/emqx_ft/src/emqx_ft.erl
+++ b/apps/emqx_ft/src/emqx_ft.erl
@@ -110,8 +110,8 @@ decode_filemeta(Map) when is_map(Map) ->
         Meta = hocon_tconf:check_plain(Schema, Map, #{atom_key => true, required => false}),
         {ok, Meta}
     catch
-        throw:Error ->
-            {error, {invalid_filemeta, Error}}
+        throw:{_Schema, Errors} ->
+            {error, {invalid_filemeta, Errors}}
     end.
 
 encode_filemeta(Meta = #{}) ->
@@ -381,7 +381,7 @@ do_validate([{filemeta, Payload} | Rest], Parsed) ->
         {ok, Meta} ->
             do_validate(Rest, [Meta | Parsed]);
         {error, Reason} ->
-            {error, {invalid_filemeta, Reason}}
+            {error, Reason}
     end;
 do_validate([{offset, Offset} | Rest], Parsed) ->
     case string:to_integer(Offset) of
diff --git a/apps/emqx_ft/src/emqx_ft_schema.erl b/apps/emqx_ft/src/emqx_ft_schema.erl
index bc780874a..ce3710cc1 100644
--- a/apps/emqx_ft/src/emqx_ft_schema.erl
+++ b/apps/emqx_ft/src/emqx_ft_schema.erl
@@ -35,6 +35,13 @@
 
 -reflect_type([json_value/0]).
 
+%% NOTE
+%% This is rather conservative limit, mostly dictated by the filename limitations
+%% on most filesystems. Even though, say, S3 does not have such limitations, it's
+%% still useful to have a limit on the filename length, to avoid having to deal with
+%% limits in the storage backends.
+-define(MAX_FILENAME_BYTELEN, 255).
+
 -import(hoconsc, [ref/2, mk/2]).
 
 namespace() -> file_transfer.
@@ -234,7 +241,13 @@ schema(filemeta) ->
     }.
 
 validator(filename) ->
-    fun emqx_ft_fs_util:is_filename_safe/1.
+    [
+        fun(Value) ->
+            Bin = unicode:characters_to_binary(Value),
+            byte_size(Bin) =< ?MAX_FILENAME_BYTELEN orelse {error, max_length_exceeded}
+        end,
+        fun emqx_ft_fs_util:is_filename_safe/1
+    ].
 
 converter(checksum) ->
     fun
diff --git a/apps/emqx_ft/test/emqx_ft_SUITE.erl b/apps/emqx_ft/test/emqx_ft_SUITE.erl
index c3ee7f271..3649d02f8 100644
--- a/apps/emqx_ft/test/emqx_ft_SUITE.erl
+++ b/apps/emqx_ft/test/emqx_ft_SUITE.erl
@@ -183,9 +183,18 @@ t_invalid_filename(Config) ->
         unspecified_error,
         emqtt:publish(C, mk_init_topic(<<"f3">>), encode_meta(meta("/etc/passwd", <<>>)), 1)
     ),
+    ?assertRCName(
+        unspecified_error,
+        emqtt:publish(
+            C,
+            mk_init_topic(<<"f4">>),
+            encode_meta(meta(lists:duplicate(1000, $A), <<>>)),
+            1
+        )
+    ),
     ?assertRCName(
         success,
-        emqtt:publish(C, mk_init_topic(<<"f4">>), encode_meta(meta("146%", <<>>)), 1)
+        emqtt:publish(C, mk_init_topic(<<"f5">>), encode_meta(meta("146%", <<>>)), 1)
     ).
 
 t_simple_transfer(Config) ->

From f06300cbed896c290c37b370026c6631cfb85a88 Mon Sep 17 00:00:00 2001
From: Andrew Mayorov <encube.ul@gmail.com>
Date: Wed, 19 Apr 2023 19:05:12 +0300
Subject: [PATCH 2/2] fix(s3): mark S3 secrets as `sensitive` in schema

---
 apps/emqx_s3/src/emqx_s3_schema.erl        |  9 +++++++--
 apps/emqx_s3/test/emqx_s3_schema_SUITE.erl | 19 +++++++++++++++++++
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/apps/emqx_s3/src/emqx_s3_schema.erl b/apps/emqx_s3/src/emqx_s3_schema.erl
index 2d714bb7d..23e69ec5d 100644
--- a/apps/emqx_s3/src/emqx_s3_schema.erl
+++ b/apps/emqx_s3/src/emqx_s3_schema.erl
@@ -12,6 +12,7 @@
 -export([roots/0, fields/1, namespace/0, tags/0, desc/1]).
 
 -export([translate/1]).
+-export([translate/2]).
 
 roots() ->
     [s3].
@@ -36,7 +37,8 @@ fields(s3) ->
                 string(),
                 #{
                     desc => ?DESC("secret_access_key"),
-                    required => false
+                    required => false,
+                    sensitive => true
                 }
             )},
         {bucket,
@@ -142,7 +144,10 @@ desc(transport_options) ->
     "Options for the HTTP transport layer used by the S3 client".
 
 translate(Conf) ->
-    Options = #{atom_key => true},
+    translate(Conf, #{}).
+
+translate(Conf, OptionsIn) ->
+    Options = maps:merge(#{atom_key => true}, OptionsIn),
     #{s3 := TranslatedConf} = hocon_tconf:check_plain(
         emqx_s3_schema, #{<<"s3">> => Conf}, Options, [s3]
     ),
diff --git a/apps/emqx_s3/test/emqx_s3_schema_SUITE.erl b/apps/emqx_s3/test/emqx_s3_schema_SUITE.erl
index bba1a5ba8..89ec8a958 100644
--- a/apps/emqx_s3/test/emqx_s3_schema_SUITE.erl
+++ b/apps/emqx_s3/test/emqx_s3_schema_SUITE.erl
@@ -108,6 +108,25 @@ t_full_config(_Config) ->
         })
     ).
 
+t_sensitive_config_hidden(_Config) ->
+    ?assertMatch(
+        #{
+            access_key_id := "access_key_id",
+            secret_access_key := <<"******">>
+        },
+        emqx_s3_schema:translate(
+            #{
+                <<"bucket">> => <<"bucket">>,
+                <<"host">> => <<"s3.us-east-1.endpoint.com">>,
+                <<"port">> => 443,
+                <<"access_key_id">> => <<"access_key_id">>,
+                <<"secret_access_key">> => <<"secret_access_key">>
+            },
+            % NOTE: this is what Config API handler is doing
+            #{obfuscate_sensitive_values => true}
+        )
+    ).
+
 t_invalid_limits(_Config) ->
     ?assertException(
         throw,