diff --git a/apps/emqx/src/emqx_tls_certfile_gc.erl b/apps/emqx/src/emqx_tls_certfile_gc.erl index 5e617a272..78dfdbaca 100644 --- a/apps/emqx/src/emqx_tls_certfile_gc.erl +++ b/apps/emqx/src/emqx_tls_certfile_gc.erl @@ -63,10 +63,13 @@ -define(HAS_OWNER_WRITE(Mode), ((Mode band 8#00200) > 0)). -type filename() :: string(). +-type basename() :: string(). -type fileinfo() :: #file_info{}. +-type fileref() :: {basename(), fileinfo()}. +-type orphans() :: #{fileref() => [filename()]}. -type st() :: #{ - orphans => #{filename() => fileinfo()}, + orphans => orphans(), gc_interval => pos_integer(), next_gc_timer => reference() }. @@ -149,7 +152,7 @@ restart_timer(St = #{next_gc_timer := TRef}) -> start_timer(St). log_event({collect, Filename, ok}, _) -> - ?tp(debug, "tls_certfile_gc_collected", #{filename => Filename}); + ?tp(info, "tls_certfile_gc_collected", #{filename => Filename}); log_event({collect, Filename, {error, Reason}}, _) -> ?tp(warning, "tls_certfile_gc_collect_error", #{filename => Filename, reason => Reason}). @@ -163,30 +166,45 @@ collect(St, Opts) -> OrphansLast = maps:get(orphans, St, #{}), Convicts = convicts(Orphans, OrphansLast), Result = collect_files(Convicts, RootDir, Opts), - {ok, Result, St#{orphans => maps:without(Convicts, Orphans)}}. + {ok, Result, St#{orphans => Orphans}}. +-spec orphans() -> + orphans(). orphans() -> Dir = emqx_utils_fs:canonicalize(emqx:mutable_certs_dir()), orphans(Dir). +-spec orphans(_Dir :: filename()) -> + orphans(). orphans(Dir) -> + % NOTE + % Orphans are files located under directory `Dir` that are not referenced by any + % configuration fragment defining TLS-related options. Certfiles = find_managed_files(fun is_managed_file/2, Dir), lists:foldl( fun(Root, Acc) -> - References = find_references(Root), + % NOTE + % In situations where there's an ambiguity in `Certfiles` (e.g. a fileref + % pointing to more than one file), we'll spare _all of them_ from marking + % as orphans. + References = find_config_references(Root), maps:without(References, Acc) end, Certfiles, emqx_config:get_root_names() ). +-spec convicts(orphans(), orphans()) -> + [filename()]. convicts(Orphans, OrphansLast) -> + % NOTE + % Convicts are files that are orphans both in `Orphans` and `OrphansLast`. maps:fold( - fun(AbsPath, #file_info{mtime = MTime, inode = Inode}, Acc) -> - case maps:get(AbsPath, Orphans, undefined) of - #file_info{mtime = MTime, inode = Inode} -> + fun(FileRef, Filenames, Acc) -> + case maps:get(FileRef, Orphans, []) of + Filenames -> % Certfile was not changed / recreated in the meantime - [AbsPath | Acc]; + Filenames ++ Acc; _ -> % Certfile was changed / created / recreated in the meantime Acc @@ -196,13 +214,18 @@ convicts(Orphans, OrphansLast) -> OrphansLast ). +-spec find_managed_files(Filter, _Dir :: filename()) -> orphans() when + Filter :: fun((_Abs :: filename(), fileinfo()) -> boolean()). find_managed_files(Filter, Dir) -> emqx_utils_fs:traverse_dir( fun (AbsPath, Info = #file_info{}, Acc) -> case Filter(AbsPath, Info) of - true -> Acc#{AbsPath => Info}; - false -> Acc + true -> + FileRef = mk_fileref(AbsPath, Info), + Acc#{FileRef => [AbsPath | maps:get(FileRef, Acc, [])]}; + false -> + Acc end; (AbsPath, {error, Reason}, Acc) -> ?SLOG(notice, "filesystem_object_inaccessible", #{ @@ -224,14 +247,16 @@ is_managed_file(AbsPath, #file_info{type = Type, mode = Mode}) -> ?HAS_OWNER_WRITE(Mode) andalso emqx_tls_lib:is_managed_ssl_file(AbsPath). -find_references(Root) -> +-spec find_config_references(Root :: binary()) -> + [fileref() | {basename(), {error, _}}]. +find_config_references(Root) -> Config = emqx_config:get_raw([Root]), fold_config( fun(Stack, Value, Acc) -> case is_file_reference(Stack) andalso is_binary(Value) of true -> Filename = emqx_schema:naive_env_interpolation(Value), - {stop, [emqx_utils_fs:canonicalize(Filename) | Acc]}; + {stop, [mk_fileref(Filename) | Acc]}; false -> {cont, Acc} end @@ -246,6 +271,27 @@ is_file_reference(Stack) -> emqx_tls_lib:ssl_file_conf_keypaths() ). +mk_fileref(AbsPath) -> + case emqx_utils_fs:read_info(AbsPath) of + {ok, Info} -> + mk_fileref(AbsPath, Info); + {error, _} = Error -> + % NOTE + % Such broken fileref will not be regarded as live reference. However, there + % are some edge cases where this _might be wrong_, e.g. one of the `AbsPath` + % components is a symlink w/o read permission. + {filename:basename(AbsPath), Error} + end. + +mk_fileref(AbsPath, Info = #file_info{}) -> + % NOTE + % This structure helps to tell if two file paths refer to the same file, without the + % need to reimplement `realpath` or something similar. It is not perfect because false + % positives are always possible, due to: + % * On Windows, inode is always 0. + % * On Unix, files can be hardlinked and have the same basename. + {filename:basename(AbsPath), Info#file_info{atime = undefined}}. + %% fold_config(FoldFun, AccIn, Config) -> diff --git a/apps/emqx/test/emqx_tls_certfile_gc_SUITE.erl b/apps/emqx/test/emqx_tls_certfile_gc_SUITE.erl index 854319b03..4d53a9413 100644 --- a/apps/emqx/test/emqx_tls_certfile_gc_SUITE.erl +++ b/apps/emqx/test/emqx_tls_certfile_gc_SUITE.erl @@ -44,7 +44,7 @@ init_per_testcase(TC, Config) -> TCAbsDir = filename:join(?config(priv_dir, Config), TC), ok = application:set_env(emqx, data_dir, TCAbsDir), ok = snabbkaffe:start_trace(), - [{tc_name, TC}, {tc_absdir, TCAbsDir} | Config]. + [{tc_name, atom_to_list(TC)}, {tc_absdir, TCAbsDir} | Config]. end_per_testcase(_TC, Config) -> ok = snabbkaffe:stop(), @@ -317,6 +317,60 @@ t_gc_spares_recreated_certfiles(_Config) -> ok = proc_lib:stop(Pid). +t_gc_spares_symlinked_datadir(Config) -> + {ok, Pid} = emqx_tls_certfile_gc:start_link(), + + % Create a certfiles set and a server that references it + SSL = #{ + <<"keyfile">> => key(), + <<"certfile">> => cert(), + <<"ocsp">> => #{<<"issuer_pem">> => cert()} + }, + {ok, SSL1} = emqx_tls_lib:ensure_ssl_files("srv", SSL), + SSL1Keyfile = emqx_utils_fs:canonicalize(maps:get(<<"keyfile">>, SSL1)), + + ok = load_config(#{ + <<"servers">> => #{<<"srv">> => #{<<"ssl">> => SSL1}} + }), + + % Change the emqx data_dir to a symlink + TCAbsDir = ?config(tc_absdir, Config), + TCAbsLink = filename:join(?config(priv_dir, Config), ?config(tc_name, Config) ++ ".symlink"), + ok = file:make_symlink(TCAbsDir, TCAbsLink), + ok = application:set_env(emqx, data_dir, TCAbsLink), + + % Make a hardlink to the SSL1 keyfile, that looks like a managed SSL file + SSL1KeyfileHardlink = filename:join([ + filename:dirname(SSL1Keyfile), + "hardlink", + filename:basename(SSL1Keyfile) + ]), + ok = filelib:ensure_dir(SSL1KeyfileHardlink), + ok = file:make_link(SSL1Keyfile, SSL1KeyfileHardlink), + + % Nothing should have been collected + ?assertMatch( + {ok, []}, + emqx_tls_certfile_gc:force() + ), + + ok = put_config([<<"servers">>, <<"srv">>, <<"ssl">>], #{}), + + % Everything should have been collected, including the hardlink + ?assertMatch( + {ok, [ + {collect, _SSL1Dir, ok}, + {collect, _SSL1Certfile, ok}, + {collect, _SSL1KeyfileHardlinkDir, ok}, + {collect, _SSL1KeyfileHardlink, ok}, + {collect, _SSL1Keyfile, ok}, + {collect, _SSL1IssuerPEM, ok} + ]}, + emqx_tls_certfile_gc:force() + ), + + ok = proc_lib:stop(Pid). + t_gc_active(_Config) -> ok = emqx_common_test_helpers:boot_modules([]), ok = emqx_common_test_helpers:start_apps([]), diff --git a/apps/emqx_utils/src/emqx_utils_fs.erl b/apps/emqx_utils/src/emqx_utils_fs.erl index 57d9d4329..ad50a409e 100644 --- a/apps/emqx_utils/src/emqx_utils_fs.erl +++ b/apps/emqx_utils/src/emqx_utils_fs.erl @@ -19,6 +19,7 @@ -include_lib("kernel/include/file.hrl"). -export([traverse_dir/3]). +-export([read_info/1]). -export([canonicalize/1]). -type fileinfo() :: #file_info{}. @@ -56,6 +57,8 @@ traverse_dir(FoldFun, Acc, AbsPath, {ok, Info}) -> traverse_dir(FoldFun, Acc, AbsPath, {error, Reason}) -> FoldFun(AbsPath, {error, Reason}, Acc). +-spec read_info(file:name()) -> + {ok, fileinfo()} | {error, file:posix() | badarg}. read_info(AbsPath) -> file:read_link_info(AbsPath, [{time, posix}, raw]).