diff --git a/arcanist/linter/LocaleDependenceLinter.php b/arcanist/linter/LocaleDependenceLinter.php --- a/arcanist/linter/LocaleDependenceLinter.php +++ b/arcanist/linter/LocaleDependenceLinter.php @@ -29,7 +29,10 @@ "split", "is_space", ], - "src/torcontrol.cpp" => ["atoi"], + "src/torcontrol.cpp" => [ + "atoi", + "strtol", + ], "src/uint256.cpp" => ["tolower"], "src/util.cpp" => ["atoi", "tolower"], "src/utilmoneystr.cpp" => ["isdigit"], diff --git a/src/Makefile.test.include b/src/Makefile.test.include --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -116,6 +116,7 @@ test/test_bitcoin.h \ test/test_bitcoin_main.cpp \ test/timedata_tests.cpp \ + test/torcontrol_tests.cpp \ test/transaction_tests.cpp \ test/txindex_tests.cpp \ test/txvalidationcache_tests.cpp \ diff --git a/src/test/CMakeLists.txt b/src/test/CMakeLists.txt --- a/src/test/CMakeLists.txt +++ b/src/test/CMakeLists.txt @@ -129,6 +129,7 @@ test_bitcoin.cpp test_bitcoin_main.cpp timedata_tests.cpp + torcontrol_tests.cpp transaction_tests.cpp txindex_tests.cpp txvalidationcache_tests.cpp diff --git a/src/test/torcontrol_tests.cpp b/src/test/torcontrol_tests.cpp new file mode 100644 --- /dev/null +++ b/src/test/torcontrol_tests.cpp @@ -0,0 +1,192 @@ +// Copyright (c) 2017 The Zcash developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. +// +#include "test/test_bitcoin.h" +#include "torcontrol.cpp" + +#include + +BOOST_FIXTURE_TEST_SUITE(torcontrol_tests, BasicTestingSetup) + +void CheckSplitTorReplyLine(std::string input, std::string command, + std::string args) { + BOOST_TEST_MESSAGE(std::string("CheckSplitTorReplyLine(") + input + ")"); + auto ret = SplitTorReplyLine(input); + BOOST_CHECK_EQUAL(ret.first, command); + BOOST_CHECK_EQUAL(ret.second, args); +} + +BOOST_AUTO_TEST_CASE(util_SplitTorReplyLine) { + // Data we should receive during normal usage + CheckSplitTorReplyLine("PROTOCOLINFO PIVERSION", "PROTOCOLINFO", + "PIVERSION"); + CheckSplitTorReplyLine("AUTH METHODS=COOKIE,SAFECOOKIE " + "COOKIEFILE=\"/home/x/.tor/control_auth_cookie\"", + "AUTH", + "METHODS=COOKIE,SAFECOOKIE " + "COOKIEFILE=\"/home/x/.tor/control_auth_cookie\""); + CheckSplitTorReplyLine("AUTH METHODS=NULL", "AUTH", "METHODS=NULL"); + CheckSplitTorReplyLine("AUTH METHODS=HASHEDPASSWORD", "AUTH", + "METHODS=HASHEDPASSWORD"); + CheckSplitTorReplyLine("VERSION Tor=\"0.2.9.8 (git-a0df013ea241b026)\"", + "VERSION", "Tor=\"0.2.9.8 (git-a0df013ea241b026)\""); + CheckSplitTorReplyLine("AUTHCHALLENGE SERVERHASH=aaaa SERVERNONCE=bbbb", + "AUTHCHALLENGE", "SERVERHASH=aaaa SERVERNONCE=bbbb"); + + // Other valid inputs + CheckSplitTorReplyLine("COMMAND", "COMMAND", ""); + CheckSplitTorReplyLine("COMMAND SOME ARGS", "COMMAND", "SOME ARGS"); + + // These inputs are valid because PROTOCOLINFO accepts an OtherLine that is + // just an OptArguments, which enables multiple spaces to be present + // between the command and arguments. + CheckSplitTorReplyLine("COMMAND ARGS", "COMMAND", " ARGS"); + CheckSplitTorReplyLine("COMMAND EVEN+more ARGS", "COMMAND", + " EVEN+more ARGS"); +} + +void CheckParseTorReplyMapping(std::string input, + std::map expected) { + BOOST_TEST_MESSAGE(std::string("CheckParseTorReplyMapping(") + input + ")"); + auto ret = ParseTorReplyMapping(input); + BOOST_CHECK_EQUAL(ret.size(), expected.size()); + auto r_it = ret.begin(); + auto e_it = expected.begin(); + while (r_it != ret.end() && e_it != expected.end()) { + BOOST_CHECK_EQUAL(r_it->first, e_it->first); + BOOST_CHECK_EQUAL(r_it->second, e_it->second); + r_it++; + e_it++; + } +} + +BOOST_AUTO_TEST_CASE(util_ParseTorReplyMapping) { + // Data we should receive during normal usage + CheckParseTorReplyMapping( + "METHODS=COOKIE,SAFECOOKIE " + "COOKIEFILE=\"/home/x/.tor/control_auth_cookie\"", + { + {"METHODS", "COOKIE,SAFECOOKIE"}, + {"COOKIEFILE", "/home/x/.tor/control_auth_cookie"}, + }); + CheckParseTorReplyMapping("METHODS=NULL", { + {"METHODS", "NULL"}, + }); + CheckParseTorReplyMapping("METHODS=HASHEDPASSWORD", + { + {"METHODS", "HASHEDPASSWORD"}, + }); + CheckParseTorReplyMapping("Tor=\"0.2.9.8 (git-a0df013ea241b026)\"", + { + {"Tor", "0.2.9.8 (git-a0df013ea241b026)"}, + }); + CheckParseTorReplyMapping("SERVERHASH=aaaa SERVERNONCE=bbbb", + { + {"SERVERHASH", "aaaa"}, + {"SERVERNONCE", "bbbb"}, + }); + CheckParseTorReplyMapping("ServiceID=exampleonion1234", + { + {"ServiceID", "exampleonion1234"}, + }); + CheckParseTorReplyMapping("PrivateKey=RSA1024:BLOB", + { + {"PrivateKey", "RSA1024:BLOB"}, + }); + CheckParseTorReplyMapping("ClientAuth=bob:BLOB", + { + {"ClientAuth", "bob:BLOB"}, + }); + + // Other valid inputs + CheckParseTorReplyMapping("Foo=Bar=Baz Spam=Eggs", { + {"Foo", "Bar=Baz"}, + {"Spam", "Eggs"}, + }); + CheckParseTorReplyMapping("Foo=\"Bar=Baz\"", { + {"Foo", "Bar=Baz"}, + }); + CheckParseTorReplyMapping("Foo=\"Bar Baz\"", { + {"Foo", "Bar Baz"}, + }); + + // Escapes + CheckParseTorReplyMapping("Foo=\"Bar\\ Baz\"", { + {"Foo", "Bar Baz"}, + }); + CheckParseTorReplyMapping("Foo=\"Bar\\Baz\"", { + {"Foo", "BarBaz"}, + }); + CheckParseTorReplyMapping("Foo=\"Bar\\@Baz\"", { + {"Foo", "Bar@Baz"}, + }); + CheckParseTorReplyMapping("Foo=\"Bar\\\"Baz\" Spam=\"\\\"Eggs\\\"\"", + { + {"Foo", "Bar\"Baz"}, + {"Spam", "\"Eggs\""}, + }); + CheckParseTorReplyMapping("Foo=\"Bar\\\\Baz\"", { + {"Foo", "Bar\\Baz"}, + }); + + // C escapes + CheckParseTorReplyMapping( + "Foo=\"Bar\\nBaz\\t\" Spam=\"\\rEggs\" " + "Octals=\"\\1a\\11\\17\\18\\81\\377\\378\\400\\2222\" Final=Check", + { + {"Foo", "Bar\nBaz\t"}, + {"Spam", "\rEggs"}, + {"Octals", "\1a\11\17\1" + "881\377\37" + "8\40" + "0\222" + "2"}, + {"Final", "Check"}, + }); + CheckParseTorReplyMapping("Valid=Mapping Escaped=\"Escape\\\\\"", + { + {"Valid", "Mapping"}, + {"Escaped", "Escape\\"}, + }); + CheckParseTorReplyMapping("Valid=Mapping Bare=\"Escape\\\"", {}); + CheckParseTorReplyMapping("OneOctal=\"OneEnd\\1\" TwoOctal=\"TwoEnd\\11\"", + { + {"OneOctal", "OneEnd\1"}, + {"TwoOctal", "TwoEnd\11"}, + }); + + // Special handling for null case + // (needed because string comparison reads the null as end-of-string) + BOOST_TEST_MESSAGE(std::string("CheckParseTorReplyMapping(Null=\"\\0\")")); + auto ret = ParseTorReplyMapping("Null=\"\\0\""); + BOOST_CHECK_EQUAL(ret.size(), 1); + auto r_it = ret.begin(); + BOOST_CHECK_EQUAL(r_it->first, "Null"); + BOOST_CHECK_EQUAL(r_it->second.size(), 1); + BOOST_CHECK_EQUAL(r_it->second[0], '\0'); + + // A more complex valid grammar. PROTOCOLINFO accepts a VersionLine that + // takes a key=value pair followed by an OptArguments, making this valid. + // Because an OptArguments contains no semantic data, there is no point in + // parsing it. + CheckParseTorReplyMapping("SOME=args,here MORE optional=arguments here", + { + {"SOME", "args,here"}, + }); + + // Inputs that are effectively invalid under the target grammar. + // PROTOCOLINFO accepts an OtherLine that is just an OptArguments, which + // would make these inputs valid. However, + // - This parser is never used in that situation, because the + // SplitTorReplyLine parser enables OtherLine to be skipped. + // - Even if these were valid, an OptArguments contains no semantic data, + // so there is no point in parsing it. + CheckParseTorReplyMapping("ARGS", {}); + CheckParseTorReplyMapping("MORE ARGS", {}); + CheckParseTorReplyMapping("MORE ARGS", {}); + CheckParseTorReplyMapping("EVEN more=ARGS", {}); + CheckParseTorReplyMapping("EVEN+more ARGS", {}); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp --- a/src/torcontrol.cpp +++ b/src/torcontrol.cpp @@ -1,4 +1,5 @@ // Copyright (c) 2015-2016 The Bitcoin Core developers +// Copyright (c) 2017 The Zcash developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -270,6 +271,8 @@ /* Split reply line in the form 'AUTH METHODS=...' into a type * 'AUTH' and arguments 'METHODS=...'. + * Grammar is implicitly defined in https://spec.torproject.org/control-spec by + * the server reply formats for PROTOCOLINFO (S3.21) and AUTHCHALLENGE (S3.24). */ static std::pair SplitTorReplyLine(const std::string &s) { @@ -289,6 +292,10 @@ /** * Parse reply arguments in the form 'METHODS=COOKIE,SAFECOOKIE * COOKIEFILE=".../control_auth_cookie"'. + * Returns a map of keys to values, or an empty map if there was an error. + * Grammar is implicitly defined in https://spec.torproject.org/control-spec by + * the server reply formats for PROTOCOLINFO (S3.21), AUTHCHALLENGE (S3.24), + * and ADD_ONION (S3.27). See also sections 2.1 and 2.3. */ static std::map ParseTorReplyMapping(const std::string &s) { @@ -296,7 +303,7 @@ size_t ptr = 0; while (ptr < s.size()) { std::string key, value; - while (ptr < s.size() && s[ptr] != '=') { + while (ptr < s.size() && s[ptr] != '=' && s[ptr] != ' ') { key.push_back(s[ptr]); ++ptr; } @@ -304,15 +311,20 @@ if (ptr == s.size()) { return std::map(); } + // The remaining string is an OptArguments + if (s[ptr] == ' ') { + break; + } // skip '=' ++ptr; // Quoted string if (ptr < s.size() && s[ptr] == '"') { - // skip '=' + // skip opening '"' ++ptr; bool escape_next = false; - while (ptr < s.size() && (!escape_next && s[ptr] != '"')) { - escape_next = (s[ptr] == '\\'); + while (ptr < s.size() && (escape_next || s[ptr] != '"')) { + // Repeated backslashes must be interpreted as pairs + escape_next = (s[ptr] == '\\' && !escape_next); value.push_back(s[ptr]); ++ptr; } @@ -322,10 +334,59 @@ } // skip closing '"' ++ptr; - /* TODO: unescape value - according to the spec this depends on the - * context, some strings use C-LogPrintf style escape codes, some - * don't. So may be better handled at the call site. + /** + * Unescape value. Per https://spec.torproject.org/control-spec + * section 2.1.1: + * + * For future-proofing, controller implementors MAY use the + * following rules to be compatible with buggy Tor implementations + * and with future ones that implement the spec as intended: + * + * Read \n \t \r and \0 ... \377 as C escapes. + * Treat a backslash followed by any other character as that + * character. */ + std::string escaped_value; + for (size_t i = 0; i < value.size(); ++i) { + if (value[i] == '\\') { + // This will always be valid, because if the QuotedString + // ended in an odd number of backslashes, then the parser + // would already have returned above, due to a missing + // terminating double-quote. + ++i; + if (value[i] == 'n') { + escaped_value.push_back('\n'); + } else if (value[i] == 't') { + escaped_value.push_back('\t'); + } else if (value[i] == 'r') { + escaped_value.push_back('\r'); + } else if ('0' <= value[i] && value[i] <= '7') { + size_t j; + // Octal escape sequences have a limit of three octal + // digits, but terminate at the first character that is + // not a valid octal digit if encountered sooner. + for (j = 1; j < 3 && (i + j) < value.size() && + '0' <= value[i + j] && value[i + j] <= '7'; + ++j) { + } + // Tor restricts first digit to 0-3 for three-digit + // octals. A leading digit of 4-7 would therefore be + // interpreted as a two-digit octal. + if (j == 3 && value[i] > '3') { + j--; + } + escaped_value.push_back( + strtol(value.substr(i, j).c_str(), NULL, 8)); + // Account for automatic incrementing at loop end + i += j - 1; + } else { + escaped_value.push_back(value[i]); + } + } else { + escaped_value.push_back(value[i]); + } + } + value = escaped_value; } else { // Unquoted value. Note that values can contain '=' at will, just no // spaces @@ -364,6 +425,11 @@ char buffer[128]; size_t n; while ((n = fread(buffer, 1, sizeof(buffer), f)) > 0) { + // Check for reading errors so we don't return any data if we couldn't + // read the entire file (or up to maxsize) + if (ferror(f)) { + return std::make_pair(false, ""); + } retval.append(buffer, buffer + n); if (retval.size() > maxsize) { break; @@ -490,6 +556,13 @@ private_key = i->second; } } + if (service_id.empty()) { + LogPrintf("tor: Error parsing ADD_ONION parameters:\n"); + for (const std::string &s : reply.lines) { + LogPrintf(" %s\n", SanitizeString(s)); + } + return; + } service = LookupNumeric(std::string(service_id + ".onion").c_str(), GetListenPort()); LogPrintf("tor: Got service ID %s, advertising service %s\n", @@ -582,6 +655,11 @@ if (l.first == "AUTHCHALLENGE") { std::map m = ParseTorReplyMapping(l.second); + if (m.empty()) { + LogPrintf("tor: Error parsing AUTHCHALLENGE parameters: %s\n", + SanitizeString(l.second)); + return; + } std::vector serverHash = ParseHex(m["SERVERHASH"]); std::vector serverNonce = ParseHex(m["SERVERNONCE"]); LogPrint(BCLog::TOR,