feat: save uploaded OCSP issuer pem like other ssl certs

apps/emqx/src/emqx_tls_lib.erl

@@ -47,8 +47,18 @@
 -define(IS_TRUE(Val), ((Val =:= true) orelse (Val =:= <<"true">>))).
 -define(IS_FALSE(Val), ((Val =:= false) orelse (Val =:= <<"false">>))).
 
--define(SSL_FILE_OPT_NAMES, [<<"keyfile">>, <<"certfile">>, <<"cacertfile">>]).
--define(SSL_FILE_OPT_NAMES_A, [keyfile, certfile, cacertfile]).
+-define(SSL_FILE_OPT_NAMES, [
+    [<<"keyfile">>],
+    [<<"certfile">>],
+    [<<"cacertfile">>],
+    [<<"ocsp">>, <<"issuer_pem">>]
+]).
+-define(SSL_FILE_OPT_NAMES_A, [
+    [keyfile],
+    [certfile],
+    [cacertfile],
+    [ocsp, issuer_pem]
+]).
 
 %% non-empty string
 -define(IS_STRING(L), (is_list(L) andalso L =/= [] andalso is_integer(hd(L)))).
@@ -298,20 +308,20 @@ ensure_ssl_files(Dir, SSL, Opts) ->
     RequiredKeys = maps:get(required_keys, Opts, []),
     case ensure_ssl_file_key(SSL, RequiredKeys) of
         ok ->
-            Keys = ?SSL_FILE_OPT_NAMES ++ ?SSL_FILE_OPT_NAMES_A,
-            ensure_ssl_files(Dir, SSL, Keys, Opts);
+            KeyPaths = ?SSL_FILE_OPT_NAMES ++ ?SSL_FILE_OPT_NAMES_A,
+            ensure_ssl_files(Dir, SSL, KeyPaths, Opts);
         {error, _} = Error ->
             Error
     end.
 
 ensure_ssl_files(_Dir, SSL, [], _Opts) ->
     {ok, SSL};
-ensure_ssl_files(Dir, SSL, [Key | Keys], Opts) ->
-    case ensure_ssl_file(Dir, Key, SSL, maps:get(Key, SSL, undefined), Opts) of
+ensure_ssl_files(Dir, SSL, [KeyPath | KeyPaths], Opts) ->
+    case ensure_ssl_file(Dir, KeyPath, SSL, emqx_map_lib:deep_get(KeyPath, SSL, undefined), Opts) of
         {ok, NewSSL} ->
-            ensure_ssl_files(Dir, NewSSL, Keys, Opts);
+            ensure_ssl_files(Dir, NewSSL, KeyPaths, Opts);
         {error, Reason} ->
-            {error, Reason#{which_options => [Key]}}
+            {error, Reason#{which_options => [KeyPath]}}
     end.
 
 %% @doc Compare old and new config, delete the ones in old but not in new.
@@ -321,11 +331,11 @@ delete_ssl_files(Dir, NewOpts0, OldOpts0) ->
     {ok, NewOpts} = ensure_ssl_files(Dir, NewOpts0, #{dry_run => DryRun}),
     {ok, OldOpts} = ensure_ssl_files(Dir, OldOpts0, #{dry_run => DryRun}),
     Get = fun
-        (_K, undefined) -> undefined;
-        (K, Opts) -> maps:get(K, Opts, undefined)
+        (_KP, undefined) -> undefined;
+        (KP, Opts) -> emqx_map_lib:deep_get(KP, Opts, undefined)
     end,
     lists:foreach(
-        fun(Key) -> delete_old_file(Get(Key, NewOpts), Get(Key, OldOpts)) end,
+        fun(KeyPath) -> delete_old_file(Get(KeyPath, NewOpts), Get(KeyPath, OldOpts)) end,
         ?SSL_FILE_OPT_NAMES ++ ?SSL_FILE_OPT_NAMES_A
     ),
     %% try to delete the dir if it is empty
@@ -346,29 +356,33 @@ delete_old_file(_New, Old) ->
             ?SLOG(error, #{msg => "failed_to_delete_ssl_file", file_path => Old, reason => Reason})
     end.
 
-ensure_ssl_file(_Dir, _Key, SSL, undefined, _Opts) ->
+ensure_ssl_file(_Dir, _KeyPath, SSL, undefined, _Opts) ->
     {ok, SSL};
-ensure_ssl_file(Dir, Key, SSL, MaybePem, Opts) ->
+ensure_ssl_file(Dir, KeyPath, SSL, MaybePem, Opts) ->
     case is_valid_string(MaybePem) of
         true ->
             DryRun = maps:get(dry_run, Opts, false),
-            do_ensure_ssl_file(Dir, Key, SSL, MaybePem, DryRun);
+            do_ensure_ssl_file(Dir, KeyPath, SSL, MaybePem, DryRun);
         false ->
             {error, #{reason => invalid_file_path_or_pem_string}}
     end.
 
-do_ensure_ssl_file(Dir, Key, SSL, MaybePem, DryRun) ->
+do_ensure_ssl_file(Dir, KeyPath, SSL, MaybePem, DryRun) ->
     case is_pem(MaybePem) of
         true ->
-            case save_pem_file(Dir, Key, MaybePem, DryRun) of
-                {ok, Path} -> {ok, SSL#{Key => Path}};
-                {error, Reason} -> {error, Reason}
+            case save_pem_file(Dir, KeyPath, MaybePem, DryRun) of
+                {ok, Path} ->
+                    NewSSL = emqx_map_lib:deep_put(KeyPath, SSL, Path),
+                    {ok, NewSSL};
+                {error, Reason} ->
+                    {error, Reason}
             end;
         false ->
             case is_valid_pem_file(MaybePem) of
                 true ->
                     {ok, SSL};
-                {error, enoent} when DryRun -> {ok, SSL};
+                {error, enoent} when DryRun ->
+                    {ok, SSL};
                 {error, Reason} ->
                     {error, #{
                         pem_check => invalid_pem,
@@ -398,8 +412,8 @@ is_pem(MaybePem) ->
 %% To make it simple, the file is always overwritten.
 %% Also a potentially half-written PEM file (e.g. due to power outage)
 %% can be corrected with an overwrite.
-save_pem_file(Dir, Key, Pem, DryRun) ->
-    Path = pem_file_name(Dir, Key, Pem),
+save_pem_file(Dir, KeyPath, Pem, DryRun) ->
+    Path = pem_file_name(Dir, KeyPath, Pem),
     case filelib:ensure_dir(Path) of
         ok when DryRun ->
             {ok, Path};
@@ -422,11 +436,14 @@ is_generated_file(Filename) ->
         _ -> false
     end.
 
-pem_file_name(Dir, Key, Pem) ->
+pem_file_name(Dir, KeyPath, Pem) ->
     <<CK:8/binary, _/binary>> = crypto:hash(md5, Pem),
     Suffix = hex_str(CK),
-    FileName = binary:replace(ensure_bin(Key), <<"file">>, <<"-", Suffix/binary>>),
-    filename:join([pem_dir(Dir), FileName]).
+    Segments = lists:map(fun ensure_bin/1, KeyPath),
+    Filename0 = iolist_to_binary(lists:join(<<"_">>, Segments)),
+    Filename1 = binary:replace(Filename0, <<"file">>, <<>>),
+    Filename = <<Filename1/binary, "-", Suffix/binary>>,
+    filename:join([pem_dir(Dir), Filename]).
 
 pem_dir(Dir) ->
     filename:join([emqx:mutable_certs_dir(), Dir]).
@@ -465,9 +482,9 @@ is_valid_pem_file(Path) ->
 %% so they are forced to upload a cert file, or use an existing file path.
 -spec drop_invalid_certs(map()) -> map().
 drop_invalid_certs(#{enable := False} = SSL) when ?IS_FALSE(False) ->
-    maps:without(?SSL_FILE_OPT_NAMES_A, SSL);
+    lists:foldl(fun emqx_map_lib:deep_remove/2, SSL, ?SSL_FILE_OPT_NAMES_A);
 drop_invalid_certs(#{<<"enable">> := False} = SSL) when ?IS_FALSE(False) ->
-    maps:without(?SSL_FILE_OPT_NAMES, SSL);
+    lists:foldl(fun emqx_map_lib:deep_remove/2, SSL, ?SSL_FILE_OPT_NAMES);
 drop_invalid_certs(#{enable := True} = SSL) when ?IS_TRUE(True) ->
     do_drop_invalid_certs(?SSL_FILE_OPT_NAMES_A, SSL);
 drop_invalid_certs(#{<<"enable">> := True} = SSL) when ?IS_TRUE(True) ->
@@ -475,14 +492,16 @@ drop_invalid_certs(#{<<"enable">> := True} = SSL) when ?IS_TRUE(True) ->
 
 do_drop_invalid_certs([], SSL) ->
     SSL;
-do_drop_invalid_certs([Key | Keys], SSL) ->
-    case maps:get(Key, SSL, undefined) of
+do_drop_invalid_certs([KeyPath | KeyPaths], SSL) ->
+    case emqx_map_lib:deep_get(KeyPath, SSL, undefined) of
         undefined ->
-            do_drop_invalid_certs(Keys, SSL);
+            do_drop_invalid_certs(KeyPaths, SSL);
         PemOrPath ->
             case is_pem(PemOrPath) orelse is_valid_pem_file(PemOrPath) of
-                true -> do_drop_invalid_certs(Keys, SSL);
-                {error, _} -> do_drop_invalid_certs(Keys, maps:without([Key], SSL))
+                true ->
+                    do_drop_invalid_certs(KeyPaths, SSL);
+                {error, _} ->
+                    do_drop_invalid_certs(KeyPaths, emqx_map_lib:deep_remove(KeyPath, SSL))
             end
     end.
 
@@ -565,9 +584,10 @@ ensure_bin(A) when is_atom(A) -> atom_to_binary(A, utf8).
 
 ensure_ssl_file_key(_SSL, []) ->
     ok;
-ensure_ssl_file_key(SSL, RequiredKeys) ->
-    Filter = fun(Key) -> not maps:is_key(Key, SSL) end,
-    case lists:filter(Filter, RequiredKeys) of
+ensure_ssl_file_key(SSL, RequiredKeyPaths) ->
+    NotFoundRef = make_ref(),
+    Filter = fun(KeyPath) -> NotFoundRef =:= emqx_map_lib:deep_get(KeyPath, SSL, NotFoundRef) end,
+    case lists:filter(Filter, RequiredKeyPaths) of
         [] -> ok;
         Miss -> {error, #{reason => ssl_file_option_not_found, which_options => Miss}}
     end.

apps/emqx/test/emqx_ocsp_cache_SUITE.erl

@@ -673,7 +673,8 @@ do_t_update_listener(Config) ->
     Keyfile = filename:join([DataDir, "server.key"]),
     Certfile = filename:join([DataDir, "server.pem"]),
     Cacertfile = filename:join([DataDir, "ca.pem"]),
-    IssuerPem = filename:join([DataDir, "ocsp-issuer.pem"]),
+    IssuerPemPath = filename:join([DataDir, "ocsp-issuer.pem"]),
+    {ok, IssuerPem} = file:read_file(IssuerPemPath),
 
     %% no ocsp at first
     ListenerId = "ssl:default",
@@ -701,6 +702,9 @@ do_t_update_listener(Config) ->
                     <<"ocsp">> =>
                         #{
                             <<"enable_ocsp_stapling">> => true,
+                            %% we use the file contents to check that
+                            %% the API converts that to an internally
+                            %% managed file
                             <<"issuer_pem">> => IssuerPem,
                             <<"responder_url">> => <<"http://localhost:9877">>
                         }
@@ -722,6 +726,22 @@ do_t_update_listener(Config) ->
         },
         ListenerData2
     ),
+    %% issuer pem should have been uploaded and saved to a new
+    %% location
+    ?assertNotEqual(
+        IssuerPemPath,
+        emqx_map_lib:deep_get(
+            [<<"ssl_options">>, <<"ocsp">>, <<"issuer_pem">>],
+            ListenerData2
+        )
+    ),
+    ?assertNotEqual(
+        IssuerPem,
+        emqx_map_lib:deep_get(
+            [<<"ssl_options">>, <<"ocsp">>, <<"issuer_pem">>],
+            ListenerData2
+        )
+    ),
     assert_http_get(1, 5_000),
     ok.
 

apps/emqx/test/emqx_tls_lib_tests.erl

@@ -117,7 +117,7 @@ ssl_files_failure_test_() ->
             %% empty string
             ?assertMatch(
                 {error, #{
-                    reason := invalid_file_path_or_pem_string, which_options := [<<"keyfile">>]
+                    reason := invalid_file_path_or_pem_string, which_options := [[<<"keyfile">>]]
                 }},
                 emqx_tls_lib:ensure_ssl_files("/tmp", #{
                     <<"keyfile">> => <<>>,
@@ -128,7 +128,7 @@ ssl_files_failure_test_() ->
             %% not valid unicode
             ?assertMatch(
                 {error, #{
-                    reason := invalid_file_path_or_pem_string, which_options := [<<"keyfile">>]
+                    reason := invalid_file_path_or_pem_string, which_options := [[<<"keyfile">>]]
                 }},
                 emqx_tls_lib:ensure_ssl_files("/tmp", #{
                     <<"keyfile">> => <<255, 255>>,
@@ -136,6 +136,18 @@ ssl_files_failure_test_() ->
                     <<"cacertfile">> => bin(test_key())
                 })
             ),
+            ?assertMatch(
+                {error, #{
+                    reason := invalid_file_path_or_pem_string,
+                    which_options := [[<<"ocsp">>, <<"issuer_pem">>]]
+                }},
+                emqx_tls_lib:ensure_ssl_files("/tmp", #{
+                    <<"keyfile">> => bin(test_key()),
+                    <<"certfile">> => bin(test_key()),
+                    <<"cacertfile">> => bin(test_key()),
+                    <<"ocsp">> => #{<<"issuer_pem">> => <<255, 255>>}
+                })
+            ),
             %% not printable
             ?assertMatch(
                 {error, #{reason := invalid_file_path_or_pem_string}},
@@ -155,7 +167,8 @@ ssl_files_failure_test_() ->
                         #{
                             <<"cacertfile">> => bin(TmpFile),
                             <<"keyfile">> => bin(TmpFile),
-                            <<"certfile">> => bin(TmpFile)
+                            <<"certfile">> => bin(TmpFile),
+                            <<"ocsp">> => #{<<"issuer_pem">> => bin(TmpFile)}
                         }
                     )
                 )
@@ -170,22 +183,29 @@ ssl_files_save_delete_test() ->
     SSL0 = #{
         <<"keyfile">> => Key,
         <<"certfile">> => Key,
-        <<"cacertfile">> => Key
+        <<"cacertfile">> => Key,
+        <<"ocsp">> => #{<<"issuer_pem">> => Key}
     },
     Dir = filename:join(["/tmp", "ssl-test-dir"]),
     {ok, SSL} = emqx_tls_lib:ensure_ssl_files(Dir, SSL0),
-    File = maps:get(<<"keyfile">>, SSL),
-    ?assertMatch(<<"/tmp/ssl-test-dir/key-", _:16/binary>>, File),
-    ?assertEqual({ok, bin(test_key())}, file:read_file(File)),
+    FileKey = maps:get(<<"keyfile">>, SSL),
+    ?assertMatch(<<"/tmp/ssl-test-dir/key-", _:16/binary>>, FileKey),
+    ?assertEqual({ok, bin(test_key())}, file:read_file(FileKey)),
+    FileIssuerPem = emqx_map_lib:deep_get([<<"ocsp">>, <<"issuer_pem">>], SSL),
+    ?assertMatch(<<"/tmp/ssl-test-dir/ocsp_issuer_pem-", _:16/binary>>, FileIssuerPem),
+    ?assertEqual({ok, bin(test_key())}, file:read_file(FileIssuerPem)),
     %% no old file to delete
     ok = emqx_tls_lib:delete_ssl_files(Dir, SSL, undefined),
-    ?assertEqual({ok, bin(test_key())}, file:read_file(File)),
+    ?assertEqual({ok, bin(test_key())}, file:read_file(FileKey)),
+    ?assertEqual({ok, bin(test_key())}, file:read_file(FileIssuerPem)),
     %% old and new identical, no delete
     ok = emqx_tls_lib:delete_ssl_files(Dir, SSL, SSL),
-    ?assertEqual({ok, bin(test_key())}, file:read_file(File)),
+    ?assertEqual({ok, bin(test_key())}, file:read_file(FileKey)),
+    ?assertEqual({ok, bin(test_key())}, file:read_file(FileIssuerPem)),
     %% new is gone, delete old
     ok = emqx_tls_lib:delete_ssl_files(Dir, undefined, SSL),
-    ?assertEqual({error, enoent}, file:read_file(File)),
+    ?assertEqual({error, enoent}, file:read_file(FileKey)),
+    ?assertEqual({error, enoent}, file:read_file(FileIssuerPem)),
     %% test idempotence
     ok = emqx_tls_lib:delete_ssl_files(Dir, undefined, SSL),
     ok.
@@ -198,7 +218,8 @@ ssl_files_handle_non_generated_file_test() ->
     SSL0 = #{
         <<"keyfile">> => TmpKeyFile,
         <<"certfile">> => TmpKeyFile,
-        <<"cacertfile">> => TmpKeyFile
+        <<"cacertfile">> => TmpKeyFile,
+        <<"ocsp">> => #{<<"issuer_pem">> => TmpKeyFile}
     },
     Dir = filename:join(["/tmp", "ssl-test-dir-00"]),
     {ok, SSL2} = emqx_tls_lib:ensure_ssl_files(Dir, SSL0),
@@ -216,24 +237,32 @@ ssl_file_replace_test() ->
     SSL0 = #{
         <<"keyfile">> => Key1,
         <<"certfile">> => Key1,
-        <<"cacertfile">> => Key1
+        <<"cacertfile">> => Key1,
+        <<"ocsp">> => #{<<"issuer_pem">> => Key1}
     },
     SSL1 = #{
         <<"keyfile">> => Key2,
         <<"certfile">> => Key2,
-        <<"cacertfile">> => Key2
+        <<"cacertfile">> => Key2,
+        <<"ocsp">> => #{<<"issuer_pem">> => Key2}
     },
     Dir = filename:join(["/tmp", "ssl-test-dir2"]),
     {ok, SSL2} = emqx_tls_lib:ensure_ssl_files(Dir, SSL0),
     {ok, SSL3} = emqx_tls_lib:ensure_ssl_files(Dir, SSL1),
     File1 = maps:get(<<"keyfile">>, SSL2),
     File2 = maps:get(<<"keyfile">>, SSL3),
+    IssuerPem1 = emqx_map_lib:deep_get([<<"ocsp">>, <<"issuer_pem">>], SSL2),
+    IssuerPem2 = emqx_map_lib:deep_get([<<"ocsp">>, <<"issuer_pem">>], SSL3),
     ?assert(filelib:is_regular(File1)),
     ?assert(filelib:is_regular(File2)),
+    ?assert(filelib:is_regular(IssuerPem1)),
+    ?assert(filelib:is_regular(IssuerPem2)),
     %% delete old file (File1, in SSL2)
     ok = emqx_tls_lib:delete_ssl_files(Dir, SSL3, SSL2),
     ?assertNot(filelib:is_regular(File1)),
     ?assert(filelib:is_regular(File2)),
+    ?assertNot(filelib:is_regular(IssuerPem1)),
+    ?assert(filelib:is_regular(IssuerPem2)),
     ok.
 
 bin(X) -> iolist_to_binary(X).

apps/emqx_dashboard/src/emqx_dashboard_listener.erl

@@ -163,7 +163,7 @@ diff_listeners(Type, Stop, Start) -> {#{Type => Stop}, #{Type => Start}}.
 
 ensure_ssl_cert(#{<<"listeners">> := #{<<"https">> := #{<<"enable">> := true}}} = Conf) ->
     Https = emqx_map_lib:deep_get([<<"listeners">>, <<"https">>], Conf, undefined),
-    Opts = #{required_keys => [<<"keyfile">>, <<"certfile">>, <<"cacertfile">>]},
+    Opts = #{required_keys => [[<<"keyfile">>], [<<"certfile">>], [<<"cacertfile">>]]},
     case emqx_tls_lib:ensure_ssl_files(?DIR, Https, Opts) of
         {ok, undefined} ->
             {error, <<"ssl_cert_not_found">>};