Merge #10408: Net: Improvements to Tor control port parser

49a199b torcontrol: Handle escapes in Tor QuotedStrings (Jack Grigg)
0182a11 torcontrol: Log invalid parameters in Tor reply strings where meaningful (Jack Grigg)
0b6f40d torcontrol: Check for reading errors in ReadBinaryFile (Jack Grigg)
d63677b torcontrol: Fix ParseTorReplyMapping (Jack Grigg)
29f3c20 torcontrol: Add unit tests for Tor reply parsers (Jack Grigg)
d8e03c0 torcontrol: Improve comments (Jack Grigg)

Tree-SHA512: aa3ce8072d20299b38c4ba9471af7fab1f5df096c237bf40a96ee9274a357f7366f95ced0cc80f8da1f22f6455a1a8e68bad9a5ff71817eef3397b6aefcbc7ae
This commit is contained in:
Wladimir J. van der Laan
2017-05-18 19:58:34 +02:00
3 changed files with 275 additions and 7 deletions

View File

@@ -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.
@@ -249,6 +250,8 @@ bool TorControlConnection::Command(const std::string &cmd, const ReplyHandlerCB&
/* 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<std::string,std::string> SplitTorReplyLine(const std::string &s)
{
@@ -264,6 +267,10 @@ static std::pair<std::string,std::string> SplitTorReplyLine(const std::string &s
}
/** 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<std::string,std::string> ParseTorReplyMapping(const std::string &s)
{
@@ -271,28 +278,74 @@ static std::map<std::string,std::string> ParseTorReplyMapping(const std::string
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;
}
if (ptr == s.size()) // unexpected end of line
return std::map<std::string,std::string>();
if (s[ptr] == ' ') // The remaining string is an OptArguments
break;
++ptr; // skip '='
if (ptr < s.size() && s[ptr] == '"') { // Quoted string
++ptr; // skip '='
++ptr; // skip opening '"'
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;
}
if (ptr == s.size()) // unexpected end of line
return std::map<std::string,std::string>();
++ptr; // skip closing '"'
/* 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
while (ptr < s.size() && s[ptr] != ' ') {
value.push_back(s[ptr]);
@@ -322,6 +375,10 @@ static std::pair<bool,std::string> ReadBinaryFile(const fs::path &filename, size
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;
@@ -438,6 +495,13 @@ void TorController::add_onion_cb(TorControlConnection& _conn, const TorControlRe
if ((i = m.find("PrivateKey")) != m.end())
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", service_id, service.ToString());
if (WriteBinaryFile(GetPrivateKeyFile(), private_key)) {
@@ -515,6 +579,10 @@ void TorController::authchallenge_cb(TorControlConnection& _conn, const TorContr
std::pair<std::string,std::string> l = SplitTorReplyLine(reply.lines[0]);
if (l.first == "AUTHCHALLENGE") {
std::map<std::string,std::string> m = ParseTorReplyMapping(l.second);
if (m.empty()) {
LogPrintf("tor: Error parsing AUTHCHALLENGE parameters: %s\n", SanitizeString(l.second));
return;
}
std::vector<uint8_t> serverHash = ParseHex(m["SERVERHASH"]);
std::vector<uint8_t> serverNonce = ParseHex(m["SERVERNONCE"]);
LogPrint(BCLog::TOR, "tor: AUTHCHALLENGE ServerHash %s ServerNonce %s\n", HexStr(serverHash), HexStr(serverNonce));