Merge pull request #11348 from thalesmg/fix-ocsp-path-encoding-v44-20230725

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

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)

changes/v4.4.20-zh.md

@@ -0,0 +1,5 @@
+# v4.4.20
+
+## 修复
+
+- 确保 OCSP 请求路径已正确进行 URL 编码。[#11348](https://github.com/emqx/emqx/pull/11348)

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,[]},

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.

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),