From 6d0a76805acdeb3f77a6b0f747c53fb8162dec66 Mon Sep 17 00:00:00 2001
From: William Yang <mscame@gmail.com>
Date: Fri, 5 May 2023 10:04:20 +0200
Subject: [PATCH] feat(tls-partial-chains): error handling for invalid
 cacertfile

---
 src/emqx_tls_lib.erl                              | 15 +++++++++++++--
 ...qx_listener_tls_verify_partial_chain_SUITE.erl | 11 +++++++++++
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/src/emqx_tls_lib.erl b/src/emqx_tls_lib.erl
index eaa46d3f5..11299787c 100644
--- a/src/emqx_tls_lib.erl
+++ b/src/emqx_tls_lib.erl
@@ -26,6 +26,8 @@
         , opt_partial_chain/1
         ]).
 
+-include("logger.hrl").
+
 %% non-empty string
 -define(IS_STRING(L), (is_list(L) andalso L =/= [] andalso is_integer(hd(L)))).
 %% non-empty list of strings
@@ -189,14 +191,23 @@ opt_partial_chain(SslOpts) ->
         false ->
             SslOpts;
         V when V =:= cacert_from_cacertfile orelse V == true ->
-            replace(SslOpts, partial_chain, cacert_from_cacertfile(SslOpts))
+            replace(SslOpts, partial_chain, rootfun_trusted_ca_from_cacertfile(SslOpts))
     end.
 
 replace(Opts, Key, Value) -> [{Key, Value} | proplists:delete(Key, Opts)].
 
 %% @doc Helper, make TLS root_fun
-cacert_from_cacertfile(SslOpts) ->
+rootfun_trusted_ca_from_cacertfile(SslOpts) ->
     Cacertfile = proplists:get_value(cacertfile, SslOpts, undefined),
+    try do_rootfun_trusted_ca_from_cacertfile(Cacertfile)
+    catch _Error:_ ->
+            %% The cacertfile will be checked by OTP SSL as well and OTP choice to be silent on this.
+            %% We are touching security sutffs, don't leak extra info..
+            ?LOG(error, "Failed to look for trusted cacert from cacertfile. loc: ~p:~p",
+                 [?MODULE, ?FUNCTION_NAME]),
+            throw({error, ?FUNCTION_NAME})
+    end.
+do_rootfun_trusted_ca_from_cacertfile(Cacertfile) ->
     {ok, PemBin} = file:read_file(Cacertfile),
     %% The last one should be the top parent in the chain if it is a chain
     {'Certificate', CADer, _} = lists:last(public_key:pem_decode(PemBin)),
diff --git a/test/emqx_listener_tls_verify_partial_chain_SUITE.erl b/test/emqx_listener_tls_verify_partial_chain_SUITE.erl
index c38039cf1..4b90b3a31 100644
--- a/test/emqx_listener_tls_verify_partial_chain_SUITE.erl
+++ b/test/emqx_listener_tls_verify_partial_chain_SUITE.erl
@@ -281,6 +281,17 @@ t_conn_success_with_server_intermediate_and_client_root_chain(Config) ->
   fail_when_ssl_error(Socket),
   ok = ssl:close(Socket).
 
+t_error_handling_invalid_cacertfile(Config) ->
+  Port = emqx_test_tls_certs_helper:select_free_port(ssl),
+  DataDir = ?config(data_dir, Config),
+  Options = [{ssl_options, [ {cacertfile, filename:join(DataDir, "server2.key")} %% trigger error
+                           , {certfile, filename:join(DataDir, "server2.pem")}
+                           , {keyfile, filename:join(DataDir, "server2.key")}
+                           | ?config(ssl_config, Config)
+                           ]}],
+  ?assertException(throw, {error, rootfun_trusted_ca_from_cacertfile}, emqx_listeners:start_listener(ssl, Port, Options)).
+
+
 ssl_config_verify_partial_chain() ->
   [ {verify, verify_peer}
   , {fail_if_no_peer_cert, true}