Pull request 685: Added use of URI decoding for password

Squashed commit of the following: commit a3e56ba43f61f3f463752d67fcd5b88e4f49af78 Author: Yuriy Selivanov <[email protected]> Date: Thu Jan 30 01:56:18 2025 +0300 added test DoH credentials commit f0efa98ddbd0159e05026f2c77700c3aa604f792 Author: Yuriy Selivanov <[email protected]> Date: Wed Jan 29 15:17:46 2025 +0300 now using ada unicode commit a9ddf4d9251736bf21639527908ba9dab4f83836 Author: Yuriy Selivanov <[email protected]> Date: Tue Jan 28 21:56:13 2025 +0300 fix with new nlc commit 7e59cede051bd621241b4df98b37e40e6e9348c6 Author: Yuriy Selivanov <[email protected]> Date: Tue Jan 28 20:57:42 2025 +0300 Added use of URI decoding for password
AdguardTeam · Jan 31, 2025 · 5ee96a8 · 5ee96a8
1 parent f3522d2
commit 5ee96a8
Show file tree

Hide file tree

Showing 4 changed files with 84 additions and 1 deletion.
diff --git a/upstream/CMakeLists.txt b/upstream/CMakeLists.txt
@@ -86,3 +86,4 @@ if (APPLE)
 endif ()
 add_unit_test(test_dot_invalid_address "${TEST_DIR}" "${DNSLIBS_DIR}/upstream/src" TRUE TRUE)
 add_unit_test(test_bootstrapper "${TEST_DIR}" "${DNSLIBS_DIR}/upstream/src" TRUE TRUE)
+add_unit_test(test_doh_credentials "${TEST_DIR}" "${DNSLIBS_DIR}/upstream/src" TRUE TRUE)
diff --git a/upstream/test/test_doh_credentials.cpp b/upstream/test/test_doh_credentials.cpp
@@ -0,0 +1,75 @@
+#include <gtest/gtest.h>
+#include <memory>
+#include <string>
+
+#include "common/gtest_coro.h"
+#include "common/base64.h"
+#include "common/utils.h"
+#include "dns/common/event_loop.h"
+#include "dns/net/socket.h"
+#include "dns/upstream/upstream.h"
+
+#include "../upstream_doh.h"
+
+namespace ag::dns::upstream::test {
+
+struct DohCredentialTestParam {
+    std::string url;
+    std::string expected_username;
+    std::string expected_password;
+};
+
+class DohUpstreamParamTest : public ::testing::TestWithParam<DohCredentialTestParam> {
+public:
+    void SetUp() override {
+        m_loop = EventLoop::create();
+        m_loop->start();
+    }
+
+    void TearDown() override {
+        m_loop->stop();
+        m_loop->join();
+    }
+
+    EventLoopPtr m_loop;
+};
+
+TEST_P(DohUpstreamParamTest, ParsesCredentialsCorrectly) {
+    co_await m_loop->co_submit();
+
+    const auto &param = GetParam();
+
+    SocketFactory sf{{.loop = *m_loop}};
+    UpstreamFactory factory({.loop = *m_loop, .socket_factory = &sf});
+
+    UpstreamOptions options;
+    options.address = param.url;
+    options.bootstrap = {"8.8.8.8"};
+
+    auto upstream_res = factory.create_upstream(options);
+    ASSERT_FALSE(upstream_res.has_error()) << "Failed to create Upstream for " << param.url;
+
+    auto doh_upstream = std::dynamic_pointer_cast<DohUpstream>(upstream_res.value());
+    ASSERT_NE(doh_upstream, nullptr) << "Failed to cast to DohUpstream";
+
+    ASSERT_TRUE(doh_upstream->get_request_template().headers().get("Authorization").has_value());
+    std::string test_header(doh_upstream->get_request_template().headers().get("Authorization").value());
+    auto creds_expected = AG_FMT("{}:{}", param.expected_username, param.expected_password);
+    auto creds_expected_base64 = ag::encode_to_base64(as_u8v(creds_expected), false);
+    EXPECT_EQ(test_header, AG_FMT("Basic {}", creds_expected_base64));
+}
+
+static const DohCredentialTestParam doh_credential_test_cases[] = {
+        {"https://username:[email protected]/dns-query", "username", "password"},
+        {"https://user%20name:[email protected]/dns-query", "user name", "password"},
+        {"https://username:pass%[email protected]/dns-query", "username", "pass~word"},
+        {"https://user%7Cname:[email protected]/dns-query", "user|name", "pass~word"},
+        {"https://username:pass%[email protected]/dns-query", "username", "pass%word"},
+        {"https://username:%7E%%[email protected]/dns-query", "username", "~%|password"},
+        {"https://username:%[email protected]/dns-query", "username", std::string("\0password", 9)},
+};
+
+INSTANTIATE_TEST_SUITE_P(DohUpstreamParamTest, DohUpstreamParamTest,
+        ::testing::ValuesIn(doh_credential_test_cases));
+
+} // namespace ag::dns::upstream::test
diff --git a/upstream/upstream_doh.cpp b/upstream/upstream_doh.cpp
@@ -9,6 +9,7 @@
 #include <variant>
 
 #include <ada.h>
+#include <ada/unicode.h>
 #include <fmt/std.h>
 #include <magic_enum/magic_enum.hpp>
 #include <openssl/err.h>
@@ -329,7 +330,9 @@ ag::Error<ag::dns::Upstream::InitError> ag::dns::DohUpstream::init() {
 
     m_request_template.authority(std::string(m_url.get_hostname()));
     if (!m_url.get_username().empty() && !m_url.get_password().empty()) {
-        auto creds_fmt = AG_FMT("{}:{}", m_url.get_username(), m_url.get_password());
+        auto decode_username = ada::unicode::percent_decode(m_url.get_username(), m_url.get_username().find('%'));
+        auto decode_password = ada::unicode::percent_decode(m_url.get_password(), m_url.get_password().find('%'));
+        auto creds_fmt = AG_FMT("{}:{}", decode_username, decode_password);
         auto creds_base64 = ag::encode_to_base64(as_u8v(creds_fmt), false);
         m_request_template.headers().put("Authorization", AG_FMT("Basic {}", creds_base64));
     }

diff --git a/upstream/upstream_doh.h b/upstream/upstream_doh.h
@@ -37,6 +37,10 @@ class DohUpstream : public Upstream {
             std::vector<CertFingerprint> fingerprints);
     ~DohUpstream() override;
 
+    const http::Request get_request_template() const {
+        return m_request_template;
+    }
+
     DohUpstream() = delete;
     DohUpstream(const DohUpstream &) = delete;
     DohUpstream &operator=(const DohUpstream &) = delete;