fix(ocsp): ensure request path is URL encoded (v4.4)

Fixes https://emqx.atlassian.net/browse/EMQX-10624
2023-07-25 17:57:03 -03:00 · 2023-07-25 17:57:03 -03:00 · a92a68c1e0
parent 12c5595381
commit a92a68c1e0
5 changed files with 84 additions and 20 deletions
--- a/changes/v4.4.20-en.md
+++ b/changes/v4.4.20-en.md
@ -0,0 +1,5 @@
+# v4.4.20
+
+## Bug fixes
+
+- Ensure that OCSP request path is properly URL encoded. [#11348](https://github.com/emqx/emqx/pull/11348)
--- a/changes/v4.4.20-zh.md
+++ b/changes/v4.4.20-zh.md
@ -0,0 +1,5 @@
+# v4.4.20
+
+## 修复
+
+- 确保 OCSP 请求路径已正确进行 URL 编码。[#11348](https://github.com/emqx/emqx/pull/11348)
--- a/src/emqx.appup.src
+++ b/src/emqx.appup.src
@ -2,10 +2,12 @@
 %% Unless you know what you are doing, DO NOT edit manually!!
 {VSN,
  [{"4.4.19",
-    [{load_module,emqx_relup,brutal_purge,soft_purge,[]},
+    [{load_module,emqx_ocsp_cache,brutal_purge,soft_purge,[]},
+     {load_module,emqx_relup,brutal_purge,soft_purge,[]},
     {load_module,emqx_app,brutal_purge,soft_purge,[]}]},
   {"4.4.18",
-    [{load_module,emqx_plugins,brutal_purge,soft_purge,[]},
+    [{load_module,emqx_ocsp_cache,brutal_purge,soft_purge,[]},
+     {load_module,emqx_plugins,brutal_purge,soft_purge,[]},
     {load_module,emqx_hooks,brutal_purge,soft_purge,[]},
     {load_module,emqx_zone,brutal_purge,soft_purge,[]},
     {load_module,emqx_connection,brutal_purge,soft_purge,[]},
@ -18,7 +20,8 @@
     {load_module,emqx_relup,brutal_purge,soft_purge,[]},
     {load_module,emqx_app,brutal_purge,soft_purge,[]}]},
   {"4.4.17",
-    [{load_module,emqx_hooks,brutal_purge,soft_purge,[]},
+    [{load_module,emqx_ocsp_cache,brutal_purge,soft_purge,[]},
+     {load_module,emqx_hooks,brutal_purge,soft_purge,[]},
     {load_module,emqx_zone,brutal_purge,soft_purge,[]},
     {load_module,emqx_connection,brutal_purge,soft_purge,[]},
     {load_module,emqx_cm,brutal_purge,soft_purge,[]},
@ -31,7 +34,8 @@
     {load_module,emqx_app,brutal_purge,soft_purge,[]},
     {load_module,emqx_plugins,brutal_purge,soft_purge,[]}]},
   {"4.4.16",
-    [{load_module,emqx_hooks,brutal_purge,soft_purge,[]},
+    [{load_module,emqx_ocsp_cache,brutal_purge,soft_purge,[]},
+     {load_module,emqx_hooks,brutal_purge,soft_purge,[]},
     {load_module,emqx_zone,brutal_purge,soft_purge,[]},
     {load_module,emqx_connection,brutal_purge,soft_purge,[]},
     {load_module,emqx_cm,brutal_purge,soft_purge,[]},
@ -47,7 +51,8 @@
     {load_module,emqx_plugins,brutal_purge,soft_purge,[]},
     {load_module,emqx_app,brutal_purge,soft_purge,[]}]},
   {"4.4.15",
-    [{load_module,emqx_hooks,brutal_purge,soft_purge,[]},
+    [{load_module,emqx_ocsp_cache,brutal_purge,soft_purge,[]},
+     {load_module,emqx_hooks,brutal_purge,soft_purge,[]},
     {load_module,emqx_zone,brutal_purge,soft_purge,[]},
     {load_module,emqx_connection,brutal_purge,soft_purge,[]},
     {load_module,emqx_cm,brutal_purge,soft_purge,[]},
@ -65,7 +70,8 @@
     {load_module,emqx_relup,brutal_purge,soft_purge,[]},
     {load_module,emqx_app,brutal_purge,soft_purge,[]}]},
   {"4.4.14",
-    [{load_module,emqx_hooks,brutal_purge,soft_purge,[]},
+    [{load_module,emqx_ocsp_cache,brutal_purge,soft_purge,[]},
+     {load_module,emqx_hooks,brutal_purge,soft_purge,[]},
     {load_module,emqx_zone,brutal_purge,soft_purge,[]},
     {load_module,emqx_cm_locker,brutal_purge,soft_purge,[]},
     {load_module,emqx_cm,brutal_purge,soft_purge,[]},
@ -88,7 +94,8 @@
     {load_module,emqx_app,brutal_purge,soft_purge,[]},
     {load_module,emqx_rule_actions_trans,brutal_purge,soft_purge,[]}]},
   {"4.4.13",
-    [{load_module,emqx_hooks,brutal_purge,soft_purge,[]},
+    [{load_module,emqx_ocsp_cache,brutal_purge,soft_purge,[]},
+     {load_module,emqx_hooks,brutal_purge,soft_purge,[]},
     {load_module,emqx_zone,brutal_purge,soft_purge,[]},
     {load_module,emqx_cm_locker,brutal_purge,soft_purge,[]},
     {load_module,emqx_listeners,brutal_purge,soft_purge,[]},
@ -111,7 +118,8 @@
     {load_module,emqx_relup,brutal_purge,soft_purge,[]},
     {load_module,emqx_app,brutal_purge,soft_purge,[]}]},
   {"4.4.12",
-    [{load_module,emqx_hooks,brutal_purge,soft_purge,[]},
+    [{load_module,emqx_ocsp_cache,brutal_purge,soft_purge,[]},
+     {load_module,emqx_hooks,brutal_purge,soft_purge,[]},
     {load_module,emqx_zone,brutal_purge,soft_purge,[]},
     {load_module,emqx_cm_locker,brutal_purge,soft_purge,[]},
     {load_module,emqx_listeners,brutal_purge,soft_purge,[]},
@ -134,7 +142,8 @@
     {load_module,emqx_relup,brutal_purge,soft_purge,[]},
     {load_module,emqx_app,brutal_purge,soft_purge,[]}]},
   {"4.4.11",
-    [{load_module,emqx_hooks,brutal_purge,soft_purge,[]},
+    [{load_module,emqx_ocsp_cache,brutal_purge,soft_purge,[]},
+     {load_module,emqx_hooks,brutal_purge,soft_purge,[]},
     {load_module,emqx_zone,brutal_purge,soft_purge,[]},
     {load_module,emqx_cm_locker,brutal_purge,soft_purge,[]},
     {load_module,emqx_listeners,brutal_purge,soft_purge,[]},
@ -650,10 +659,12 @@
                         [gen_rpc,insecure_auth_fallback_allowed,true]}}]},
   {<<".*">>,[]}],
  [{"4.4.19",
-    [{load_module,emqx_relup,brutal_purge,soft_purge,[]},
+    [{load_module,emqx_ocsp_cache,brutal_purge,soft_purge,[]},
+     {load_module,emqx_relup,brutal_purge,soft_purge,[]},
     {load_module,emqx_app,brutal_purge,soft_purge,[]}]},
   {"4.4.18",
-    [{load_module,emqx_plugins,brutal_purge,soft_purge,[]},
+    [{load_module,emqx_ocsp_cache,brutal_purge,soft_purge,[]},
+     {load_module,emqx_plugins,brutal_purge,soft_purge,[]},
     {load_module,emqx_hooks,brutal_purge,soft_purge,[]},
     {load_module,emqx_zone,brutal_purge,soft_purge,[]},
     {load_module,emqx_connection,brutal_purge,soft_purge,[]},
@ -665,7 +676,8 @@
     {load_module,emqx_relup,brutal_purge,soft_purge,[]},
     {load_module,emqx_app,brutal_purge,soft_purge,[]}]},
   {"4.4.17",
-    [{load_module,emqx_hooks,brutal_purge,soft_purge,[]},
+    [{load_module,emqx_ocsp_cache,brutal_purge,soft_purge,[]},
+     {load_module,emqx_hooks,brutal_purge,soft_purge,[]},
     {load_module,emqx_zone,brutal_purge,soft_purge,[]},
     {load_module,emqx_connection,brutal_purge,soft_purge,[]},
     {load_module,emqx_cm,brutal_purge,soft_purge,[]},
@ -677,7 +689,8 @@
     {load_module,emqx_app,brutal_purge,soft_purge,[]},
     {load_module,emqx_plugins,brutal_purge,soft_purge,[]}]},
   {"4.4.16",
-    [{load_module,emqx_hooks,brutal_purge,soft_purge,[]},
+    [{load_module,emqx_ocsp_cache,brutal_purge,soft_purge,[]},
+     {load_module,emqx_hooks,brutal_purge,soft_purge,[]},
     {load_module,emqx_zone,brutal_purge,soft_purge,[]},
     {load_module,emqx_connection,brutal_purge,soft_purge,[]},
     {load_module,emqx_cm,brutal_purge,soft_purge,[]},
@ -692,7 +705,8 @@
     {load_module,emqx_relup,brutal_purge,soft_purge,[]},
     {load_module,emqx_app,brutal_purge,soft_purge,[]}]},
   {"4.4.15",
-    [{load_module,emqx_hooks,brutal_purge,soft_purge,[]},
+    [{load_module,emqx_ocsp_cache,brutal_purge,soft_purge,[]},
+     {load_module,emqx_hooks,brutal_purge,soft_purge,[]},
     {load_module,emqx_zone,brutal_purge,soft_purge,[]},
     {load_module,emqx_connection,brutal_purge,soft_purge,[]},
     {load_module,emqx_cm,brutal_purge,soft_purge,[]},
@ -709,7 +723,8 @@
     {load_module,emqx_relup,brutal_purge,soft_purge,[]},
     {load_module,emqx_app,brutal_purge,soft_purge,[]}]},
   {"4.4.14",
-    [{load_module,emqx_hooks,brutal_purge,soft_purge,[]},
+    [{load_module,emqx_ocsp_cache,brutal_purge,soft_purge,[]},
+     {load_module,emqx_hooks,brutal_purge,soft_purge,[]},
     {load_module,emqx_zone,brutal_purge,soft_purge,[]},
     {load_module,emqx_cm_locker,brutal_purge,soft_purge,[]},
     {load_module,emqx_cm,brutal_purge,soft_purge,[]},
@ -731,7 +746,8 @@
     {load_module,emqx_app,brutal_purge,soft_purge,[]},
     {load_module,emqx_rule_actions_trans,brutal_purge,soft_purge,[]}]},
   {"4.4.13",
-    [{load_module,emqx_hooks,brutal_purge,soft_purge,[]},
+    [{load_module,emqx_ocsp_cache,brutal_purge,soft_purge,[]},
+     {load_module,emqx_hooks,brutal_purge,soft_purge,[]},
     {load_module,emqx_zone,brutal_purge,soft_purge,[]},
     {load_module,emqx_cm_locker,brutal_purge,soft_purge,[]},
     {load_module,emqx_listeners,brutal_purge,soft_purge,[]},
@ -753,7 +769,8 @@
     {load_module,emqx_relup,brutal_purge,soft_purge,[]},
     {load_module,emqx_app,brutal_purge,soft_purge,[]}]},
   {"4.4.12",
-    [{load_module,emqx_hooks,brutal_purge,soft_purge,[]},
+    [{load_module,emqx_ocsp_cache,brutal_purge,soft_purge,[]},
+     {load_module,emqx_hooks,brutal_purge,soft_purge,[]},
     {load_module,emqx_zone,brutal_purge,soft_purge,[]},
     {load_module,emqx_cm_locker,brutal_purge,soft_purge,[]},
     {load_module,emqx_listeners,brutal_purge,soft_purge,[]},
@ -775,7 +792,8 @@
     {load_module,emqx_relup,brutal_purge,soft_purge,[]},
     {load_module,emqx_app,brutal_purge,soft_purge,[]}]},
   {"4.4.11",
-    [{load_module,emqx_hooks,brutal_purge,soft_purge,[]},
+    [{load_module,emqx_ocsp_cache,brutal_purge,soft_purge,[]},
+     {load_module,emqx_hooks,brutal_purge,soft_purge,[]},
     {load_module,emqx_zone,brutal_purge,soft_purge,[]},
     {load_module,emqx_cm_locker,brutal_purge,soft_purge,[]},
     {load_module,emqx_listeners,brutal_purge,soft_purge,[]},
--- a/src/emqx_ocsp_cache.erl
+++ b/src/emqx_ocsp_cache.erl
@ -313,7 +313,8 @@ build_ocsp_request(IssuerPem, ServerCert) ->
                   }
            },
    ReqDer = public_key:der_encode('OCSPRequest', Req),
-    base64:encode_to_string(ReqDer).
+    B64Encoded = base64:encode_to_string(ReqDer),
+    emqx_http_lib:uri_encode(B64Encoded).

 to_bin(Str) when is_list(Str) -> list_to_binary(Str);
 to_bin(Bin) when is_binary(Bin) -> Bin.
--- a/test/emqx_ocsp_cache_SUITE.erl
+++ b/test/emqx_ocsp_cache_SUITE.erl
@ -143,8 +143,9 @@ init_per_testcase(_TestCase, Config) ->
                end),
    {ok, CachePid} = emqx_ocsp_cache:start_link(),
    DataDir = ?config(data_dir, Config),
+    ResponderURL = "http://localhost:9877",
    OCSPOpts = [ {ocsp_stapling_enabled, true}
-               , {ocsp_responder_url, "http://localhost:9877"}
+               , {ocsp_responder_url, ResponderURL}
               , {ocsp_issuer_pem,
                  filename:join(DataDir, "ocsp-issuer.pem")}
               , {ocsp_refresh_http_timeout, 15_000}
@ -161,6 +162,7 @@ init_per_testcase(_TestCase, Config) ->
        }]),
    snabbkaffe:start_trace(),
    [ {cache_pid, CachePid}
+    , {responder_url, ResponderURL}
    | Config].

 end_per_testcase(t_openssl_client, Config) ->
@ -487,6 +489,39 @@ t_sni_fun_http_error(_Config) ->
      emqx_ocsp_cache:sni_fun(ServerName, ListenerID)),
    ok.

+t_path_encoding(Config) ->
+    ResponderURL = ?config(responder_url, Config) ++ "/",
+    ListenerID = <<"mqtt:ssl:test_ocsp">>,
+    TestPid = self(),
+    ok = meck:expect(
+        emqx_ocsp_cache,
+        http_get,
+        fun(RequestURI, _HTTPTimeout) ->
+            TestPid ! {request_uri, RequestURI},
+            {ok, {{"HTTP/1.0", 200, 'OK'}, [], <<"ocsp response">>}}
+        end
+    ),
+    ?check_trace(
+        begin
+            ?assertMatch({ok, _}, emqx_ocsp_cache:fetch_response(ListenerID)),
+            receive
+                {request_uri, RequestURI} ->
+                    Path = string:prefix(RequestURI, ResponderURL),
+                    ?assertEqual(nomatch, string:find(Path, "/"), #{path => Path}),
+                    ok
+            after 100 ->
+                ct:pal(
+                    "responder url: ~p\nmailbox: ~p",
+                    [ResponderURL, process_info(self(), messages)]
+                ),
+                ct:fail("request not made")
+            end,
+            ok
+        end,
+        []
+    ),
+    ok.
+
 t_openssl_client(Config) ->
    TLSVsn = ?config(tls_vsn, Config),
    WithStatusRequest = ?config(status_request, Config),