fixup! refactor: rpc-server.cc (#2152) (#2164)

fixup: rpc-server auth regression; fix tests

libtransmission/crypto-utils.cc

@@ -268,6 +268,15 @@ void* tr_base64_decode_str(char const* input, size_t* output_length)
     return tr_base64_decode(input, input == nullptr ? 0 : strlen(input), output_length);
 }
 
+std::string tr_base64_decode_str(std::string_view input)
+{
+    auto len = size_t{};
+    auto* buf = tr_base64_decode(std::data(input), std::size(input), &len);
+    auto str = std::string{ reinterpret_cast<char const*>(buf), len };
+    tr_free(buf);
+    return str;
+}
+
 /***
 ****
 ***/

libtransmission/crypto-utils.h

@@ -176,6 +176,12 @@ void* tr_base64_decode(void const* input, size_t input_length, size_t* output_le
  */
 void* tr_base64_decode_str(char const* input, size_t* output_length) TR_GNUC_MALLOC;
 
+/**
+ * @brief Translate a character range from base64 into raw form.
+ * @return a new std::string with the decoded contents.
+ */
+std::string tr_base64_decode_str(std::string_view input);
+
 /**
  * @brief Wrapper around tr_binary_to_hex() for SHA_DIGEST_LENGTH.
  */

libtransmission/rpc-server.cc

@@ -564,6 +564,31 @@ static bool test_session_id(tr_rpc_server* server, struct evhttp_request* req)
     return success;
 }
 
+static bool isAuthorized(tr_rpc_server const* server, char const* auth_header)
+{
+    if (!server->isPasswordEnabled)
+    {
+        return true;
+    }
+
+    // https://datatracker.ietf.org/doc/html/rfc7617
+    // `Basic ${base64(username)}:${base64(password)}`
+
+    auto constexpr Prefix = "Basic "sv;
+    auto auth = std::string_view{ auth_header != nullptr ? auth_header : "" };
+    if (!tr_strvStartsWith(auth, Prefix))
+    {
+        return false;
+    }
+
+    auth.remove_prefix(std::size(Prefix));
+    auto const decoded_str = tr_base64_decode_str(auth);
+    auto decoded = std::string_view{ decoded_str };
+    auto const username = tr_strvSep(&decoded, ':');
+    auto const password = decoded;
+    return server->username == username && tr_ssha1_matches(server->salted_password, password);
+}
+
 static void handle_request(struct evhttp_request* req, void* arg)
 {
     auto* server = static_cast<tr_rpc_server*>(arg);
@@ -605,35 +630,12 @@ static void handle_request(struct evhttp_request* req, void* arg)
             return;
         }
 
-        char const* const auth = evhttp_find_header(req->input_headers, "Authorization");
-        char* user = nullptr;
-        char* pass = nullptr;
-
-        if (auth != nullptr && evutil_ascii_strncasecmp(auth, "basic ", 6) == 0)
-        {
-            auto* p = static_cast<char*>(tr_base64_decode_str(auth + 6, nullptr));
-
-            if (p != nullptr)
-            {
-                if ((pass = strchr(p, ':')) != nullptr)
-                {
-                    user = p;
-                    *pass++ = '\0';
-                }
-                else
-                {
-                    tr_free(p);
-                }
-            }
-        }
-
-        if (server->isPasswordEnabled &&
-            (pass == nullptr || user == nullptr || server->username != user || !tr_ssha1_matches(server->password, pass)))
+        if (!isAuthorized(server, evhttp_find_header(req->input_headers, "Authorization")))
         {
             evhttp_add_header(req->output_headers, "WWW-Authenticate", "Basic realm=\"" MY_REALM "\"");
             if (server->isAntiBruteForceEnabled)
             {
-                server->loginattempts++;
+                ++server->loginattempts;
             }
 
             char* unauthuser = tr_strdup_printf(
@@ -641,7 +643,6 @@ static void handle_request(struct evhttp_request* req, void* arg)
                 server->loginattempts);
             send_simple_response(req, 401, unauthuser);
             tr_free(unauthuser);
-            tr_free(user);
             return;
         }
 
@@ -703,7 +704,7 @@ static void handle_request(struct evhttp_request* req, void* arg)
             tr_free(tmp);
         }
 #endif
-        else if (location == "rpc"sv)
+        else if (tr_strvStartsWith(location, "rpc"sv))
         {
             handle_rpc(req, server);
         }
@@ -711,8 +712,6 @@ static void handle_request(struct evhttp_request* req, void* arg)
         {
             send_simple_response(req, HTTP_NOTFOUND, req->uri);
         }
-
-        tr_free(user);
     }
 }
 
@@ -964,16 +963,21 @@ std::string const& tr_rpcGetUsername(tr_rpc_server const* server)
     return server->username;
 }
 
+static constexpr bool isSalted(std::string_view password)
+{
+    return !std::empty(password) && password.front() == '{';
+}
+
 void tr_rpcSetPassword(tr_rpc_server* server, std::string_view password)
 {
-    server->password = !std::empty(password) && password.front() == '{' ? tr_ssha1(password) : password;
+    server->salted_password = isSalted(password) ? password : tr_ssha1(password);
 
-    dbgmsg("setting our Password to [%s]", server->password.c_str());
+    dbgmsg("setting our salted password to [%s]", server->salted_password.c_str());
 }
 
 std::string const& tr_rpcGetPassword(tr_rpc_server const* server)
 {
-    return server->password;
+    return server->salted_password;
 }
 
 void tr_rpcSetPasswordEnabled(tr_rpc_server* server, bool isEnabled)

libtransmission/rpc-server.h

@@ -39,7 +39,7 @@ public:
 
     std::list<std::string> hostWhitelist;
     std::list<std::string> whitelist;
-    std::string password;
+    std::string salted_password;
     std::string username;
     std::string whitelistStr;
     std::string url;

libtransmission/transmission.h

@@ -357,7 +357,8 @@ void tr_sessionSetRPCPassword(tr_session* session, char const* password);
 
 void tr_sessionSetRPCUsername(tr_session* session, char const* username);
 
-/** @brief get the password used to restrict RPC requests.
+// TODO(ckerr): rename function to indicate it returns the salted value
+/** @brief get the salted version of the password used to restrict RPC requests.
     @return the password string.
     @see tr_sessionInit()
     @see tr_sessionSetRPCPassword() */

tests/libtransmission/session-test.cc

@@ -146,22 +146,33 @@ TEST_F(SessionTest, propertiesApi)
     tr_sessionSetRPCUrl(session, nullptr);
     EXPECT_EQ(""sv, tr_sessionGetRPCUrl(session));
 
-    // rpc username, password
+    // rpc username
 
     for (auto const& value : { "foo"sv, "bar"sv, ""sv })
     {
         tr_sessionSetRPCUsername(session, std::string{ value }.c_str());
         EXPECT_EQ(value, tr_sessionGetRPCUsername(session));
-
-        tr_sessionSetRPCPassword(session, std::string{ value }.c_str());
-        EXPECT_EQ(value, tr_sessionGetRPCPassword(session));
     }
 
     tr_sessionSetRPCUsername(session, nullptr);
     EXPECT_EQ(""sv, tr_sessionGetRPCUsername(session));
 
-    tr_sessionSetRPCPassword(session, nullptr);
-    EXPECT_EQ(""sv, tr_sessionGetRPCPassword(session));
+    // rpc password (unsalted)
+
+    {
+        auto const value = "foo"sv;
+        tr_sessionSetRPCPassword(session, std::string{ value }.c_str());
+        EXPECT_NE(value, tr_sessionGetRPCPassword(session));
+        EXPECT_EQ('{', tr_sessionGetRPCPassword(session)[0]);
+    }
+
+    // rpc password (salted)
+
+    {
+        auto const value = "{foo"sv;
+        tr_sessionSetRPCPassword(session, std::string{ value }.c_str());
+        EXPECT_EQ(value, tr_sessionGetRPCPassword(session));
+    }
 
     // blocklist enabled