October 2019 - tor-commits - lists.torproject.org

[tor/master] dir: Remove connection_dir_is_anonymous() logging
by teor＠torproject.org 25 Oct '19

25 Oct '19

commit 985717675cf2c613ca88bae694695c5ee7db05d4 Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Oct 24 11:23:31 2019 -0400 dir: Remove connection_dir_is_anonymous() logging First, remove the HSDir mention which should not be in that generic function. Second, move them to debug() level since they are possible error case. Part of #31958 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/feature/dircommon/directory.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/feature/dircommon/directory.c b/src/feature/dircommon/directory.c index 1ac35dd8b..8e5b41332 100644 --- a/src/feature/dircommon/directory.c +++ b/src/feature/dircommon/directory.c @@ -212,7 +212,8 @@ connection_dir_is_anonymous(const dir_connection_t *dir_conn) * be closed or marked for close. */ if (linked_conn == NULL || linked_conn->magic != EDGE_CONNECTION_MAGIC || conn->linked_conn_is_closed || conn->linked_conn->marked_for_close) { - log_info(LD_DIR, "Rejected HSDir request: not linked to edge"); + log_debug(LD_DIR, "Directory connection is not anonymous: " + "not linked to edge"); return false; } @@ -221,7 +222,8 @@ connection_dir_is_anonymous(const dir_connection_t *dir_conn) /* Can't be a circuit we initiated and without a circuit, no channel. */ if (circ == NULL || CIRCUIT_IS_ORIGIN(circ)) { - log_info(LD_DIR, "Rejected HSDir request: not on OR circuit"); + log_debug(LD_DIR, "Directory connection is not anonymous: " + "not on OR circuit"); return false; } @@ -230,6 +232,8 @@ connection_dir_is_anonymous(const dir_connection_t *dir_conn) * not continue so return that it is not anonymous since we can not know for * sure if it is. */ if (circ->marked_for_close) { + log_debug(LD_DIR, "Directory connection is not anonymous: " + "circuit marked for close"); return false; } @@ -237,7 +241,8 @@ connection_dir_is_anonymous(const dir_connection_t *dir_conn) * BUG() because if the circuit is not mark for close, we ought to have a * p_chan else we have a code flow issue. */ if (BUG(CONST_TO_OR_CIRCUIT(circ)->p_chan == NULL)) { - log_info(LD_DIR, "Rejected HSDir request: no p_chan"); + log_debug(LD_DIR, "Directory connection is not anonymous: " + "no p_chan on circuit"); return false; }

1 0

[tor/master] Merge branch 'maint-0.4.2'
by teor＠torproject.org 25 Oct '19

25 Oct '19

commit a91f52a443e605a5b8efd4fe0d4032512c23b6cd Merge: b3d035d5c c8aa3cc17 Author: teor <teor(a)torproject.org> Date: Fri Oct 25 10:38:22 2019 +1000 Merge branch 'maint-0.4.2' changes/ticket31958 | 5 +++++ src/feature/dircache/dircache.c | 10 ++++++++-- src/feature/dircommon/directory.c | 23 +++++++++++++++++++---- 3 files changed, 32 insertions(+), 6 deletions(-)

1 0

[tor/master] Merge remote-tracking branch 'tor-github/pr/1466' into maint-0.4.2
by teor＠torproject.org 25 Oct '19

25 Oct '19

commit c8aa3cc17d9cf28ea98c4619984b3f9c0afa3f3a Merge: acd27f4b4 3867ca492 Author: teor <teor(a)torproject.org> Date: Fri Oct 25 10:38:01 2019 +1000 Merge remote-tracking branch 'tor-github/pr/1466' into maint-0.4.2 changes/ticket31958 | 5 +++++ src/feature/dircache/dircache.c | 10 ++++++++-- src/feature/dircommon/directory.c | 23 +++++++++++++++++++---- 3 files changed, 32 insertions(+), 6 deletions(-)

1 0

[tor/master] dir: Return 503 code when rejecting single hop request
by teor＠torproject.org 25 Oct '19

25 Oct '19

commit 3867ca4925a2e99b7bcc4526ccf4cfe602bef3bf Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Oct 24 11:25:05 2019 -0400 dir: Return 503 code when rejecting single hop request Single hop rejection (POST and GET) for HS v3 descriptor now return a 503 code which is more accurate code from dir-spec.txt and from other rejection case in the code. For instance if you are not a relay and you get a POST request, a 503 code is sent back with a rejection message. Part of #31958 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/feature/dircache/dircache.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/feature/dircache/dircache.c b/src/feature/dircache/dircache.c index 7c6af3582..d4d0ad993 100644 --- a/src/feature/dircache/dircache.c +++ b/src/feature/dircache/dircache.c @@ -1393,7 +1393,8 @@ handle_get_hs_descriptor_v3(dir_connection_t *conn, /* Reject non anonymous dir connections (which also tests if encrypted). We * do not allow single hop clients to query an HSDir. */ if (!connection_dir_is_anonymous(conn)) { - write_short_http_response(conn, 404, "Not found"); + write_short_http_response(conn, 503, + "Rejecting single hop HS v3 descriptor request"); goto done; } @@ -1636,7 +1637,12 @@ directory_handle_command_post,(dir_connection_t *conn, const char *headers, /* Handle HS descriptor publish request. We force an anonymous connection * (which also tests for encrypted). We do not allow single-hop client to * post a descriptor onto an HSDir. */ - if (connection_dir_is_anonymous(conn) && !strcmpstart(url, "/tor/hs/")) { + if (!strcmpstart(url, "/tor/hs/")) { + if (!connection_dir_is_anonymous(conn)) { + write_short_http_response(conn, 503, + "Rejecting single hop HS descriptor post"); + goto done; + } const char *msg = "HS descriptor stored successfully."; /* We most probably have a publish request for an HS descriptor. */

1 0

[tor/master] Add a script to rename C identifiers, and optionally add a commit message
by teor＠torproject.org 25 Oct '19

25 Oct '19

commit fcdb66acd3afd4068a3b200ffc71daeab72e0fcc Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Oct 23 13:26:08 2019 -0400 Add a script to rename C identifiers, and optionally add a commit message This script should "pay for itself" if it prevents one bug caused by careless search-replace-commit loops. --- changes/ticket32237 | 5 + scripts/maint/rename_c_identifier.py | 261 +++++++++++++++++++++++++++++++++++ 2 files changed, 266 insertions(+) diff --git a/changes/ticket32237 b/changes/ticket32237 new file mode 100644 index 000000000..dfa0b7840 --- /dev/null +++ b/changes/ticket32237 @@ -0,0 +1,5 @@ + o Minor features (developer tools): + - Add a rename_c_identifiers.py tool to rename a bunch of C + identifiers at once, and generate a well-formed commit message + describing the change. This should help with refactoring. Closes + ticket 32237. diff --git a/scripts/maint/rename_c_identifier.py b/scripts/maint/rename_c_identifier.py new file mode 100755 index 000000000..0e56dc48d --- /dev/null +++ b/scripts/maint/rename_c_identifier.py @@ -0,0 +1,261 @@ +#!/usr/bin/python3 +# +# Copyright (c) 2001 Matej Pfajfar. +# Copyright (c) 2001-2004, Roger Dingledine. +# Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. +# Copyright (c) 2007-2019, The Tor Project, Inc. +# See LICENSE for licensing information + +""" +Helpful script to replace one or more C identifiers, and optionally +generate a commit message explaining what happened. +""" + +import argparse +import fileinput +import os +import re +import shlex +import subprocess +import sys +import tempfile + +TOPDIR = "src" + + +def is_c_file(fn): + """ + Return true iff fn is the name of a C file. + + >>> is_c_file("a/b/module.c") + True + >>> is_c_file("a/b/module.h") + True + >>> is_c_file("a/b/module.c~") + False + >>> is_c_file("a/b/.module.c") + False + >>> is_c_file("a/b/module.cpp") + False + """ + fn = os.path.split(fn)[1] + if fn.startswith("."): + return False + ext = os.path.splitext(fn)[1] + return ext in {".c", ".h", ".i", ".inc"} + + +def list_c_files(topdir=TOPDIR): + """ + Use git to list all the C files under version control. + + >>> lst = list(list_c_files()) + >>> "src/core/mainloop/mainloop.c" in lst + True + >>> "src/core/mainloop/twiddledeedoo.c" in lst + False + >>> "micro-revision.i" in lst + False + """ + proc = subprocess.Popen( + ["git", "ls-tree", "--name-only", "-r", "HEAD", topdir], + stdout=subprocess.PIPE, + encoding="utf-8") + for line in proc.stdout.readlines(): + line = line.strip() + if is_c_file(line): + yield line + + +class Rewriter: + """ + A rewriter applies a series of word-by-word replacements, in + sequence. Replacements only happen at "word boundaries", + as determined by the \\b regular expression marker. + + ("A word is defined as a sequence of alphanumeric or underscore + characters", according to the documentation.) + + >>> R = Rewriter([("magic", "secret"), ("words", "codes")]) + >>> R.apply("The magic words are rambunctious bluejay") + 'The secret codes are rambunctious bluejay' + >>> R.apply("The magical words are rambunctious bluejay") + 'The magical codes are rambunctious bluejay' + >>> R.get_count() + 3 + + """ + + def __init__(self, replacements): + """Make a new Rewriter. Takes a sequence of pairs of + (from_id, to_id), where from_id is an identifier to replace, + and to_id is its replacement. + """ + self._patterns = [] + for id1, id2 in replacements: + pat = re.compile(r"\b{}\b".format(re.escape(id1))) + self._patterns.append((pat, id2)) + + self._count = 0 + + def apply(self, line): + """Return `line` as transformed by this rewriter.""" + for pat, ident in self._patterns: + line, count = pat.subn(ident, line) + self._count += count + return line + + def get_count(self): + """Return the number of identifiers that this rewriter has + rewritten.""" + return self._count + + +def rewrite_files(files, rewriter): + """ + Apply `rewriter` to every file in `files`, replacing those files + with their rewritten contents. + """ + for line in fileinput.input(files, inplace=True): + sys.stdout.write(rewriter.apply(line)) + + +def make_commit_msg(pairs, no_verify): + """Return a commit message to explain what was replaced by the provided + arguments. + """ + script = ["./scripts/maint/rename_c_identifier.py"] + for id1, id2 in pairs: + qid1 = shlex.quote(id1) + qid2 = shlex.quote(id2) + script.append(" {} {}".format(qid1, qid2)) + script = " \\\n".join(script) + + if len(pairs) == 1: + line1 = "Rename {} to {}".format(pairs[0]) + else: + line1 = "Replace several C identifiers." + + msg = """\ +{} + +This is an automated commit, generated by this command: + +{} +""".format(line1, script) + + if no_verify: + msg += """ +It was generated with --no-verify, so it probably breaks some commit hooks. +The commiter should be sure to fix them up in a subsequent commit. +""" + + return msg + + +def commit(pairs, no_verify=False): + """Try to commit the current git state, generating the commit message as + appropriate. If `no_verify` is True, pass the --no-verify argument to + git commit. + """ + args = [] + if no_verify: + args.append("--no-verify") + + # We have to use a try block to delete the temporary file here, since we + # are using tempfile with delete=False. We have to use delete=False, + # since otherwise we are not guaranteed to be able to give the file to + # git for it to open. + fname = None + try: + with tempfile.NamedTemporaryFile(mode="w", delete=False) as f: + fname = f.name + f.write(make_commit_msg(pairs, no_verify)) + s = subprocess.run(["git", "commit", "-a", "-F", fname, "--edit"]+args) + if s.returncode != 0 and not no_verify: + print('"git commit" failed. Maybe retry with --no-verify?', + file=sys.stderr) + revert_changes() + return False + finally: + os.unlink(fname) + + return True + + +def any_uncommitted_changes(): + """Return True if git says there are any uncommitted changes in the current + working tree; false otherwise. + """ + s = subprocess.run(["git", "diff-index", "--quiet", "HEAD"]) + return s.returncode != 0 + + +DESC = "Replace one identifier with another throughout our source." +EXAMPLES = """\ +Examples: + + rename_c_identifier.py set_ctrl_id set_controller_id + (Replaces every occurrence of "set_ctrl_id" with "set_controller_id".) + + rename_c_identifier.py --commit set_ctrl_id set_controller_id + (As above, but also generate a git commit with an appropriate message.) + + rename_c_identifier.py a b c d + (Replace "a" with "b", and "c" with "d".)""" + + +def revert_changes(): + """Tell git to revert all the changes in the current working tree. + """ + print('Reverting changes.', file=sys.stderr) + subprocess.run(["git", "checkout", "--quiet", TOPDIR]) + + +def main(argv): + import argparse + parser = argparse.ArgumentParser(description=DESC, epilog=EXAMPLES, + # prevent re-wrapping the examples + formatter_class=argparse.RawDescriptionHelpFormatter) + + parser.add_argument("--commit", action='store_true', + help="Generate a Git commit.") + parser.add_argument("--no-verify", action='store_true', + help="Tell Git not to run its pre-commit hooks.") + parser.add_argument("from_id", type=str, help="Original identifier") + parser.add_argument("to_id", type=str, help="New identifier") + parser.add_argument("more", type=str, nargs=argparse.REMAINDER, + help="Additional identifier pairs") + + args = parser.parse_args(argv[1:]) + + if len(args.more) % 2 != 0: + print("I require an even number of identifiers.", file=sys.stderr) + return 1 + + if any_uncommitted_changes(): + print("Uncommitted changes found. Not running.", file=sys.stderr) + return 1 + + pairs = [] + print("renaming {} to {}".format(args.from_id, args.to_id), file=sys.stderr) + pairs.append((args.from_id, args.to_id)) + for idx in range(0, len(args.more), 2): + id1 = args.more[idx] + id2 = args.more[idx+1] + print("renaming {} to {}".format(id1, id2)) + pairs.append((id1, id2)) + + rewriter = Rewriter(pairs) + + rewrite_files(list_c_files(), rewriter) + + print("Replaced {} identifiers".format(rewriter.get_count()), + file=sys.stderr) + + if args.commit: + commit(pairs, args.no_verify) + + +if __name__ == '__main__': + main(sys.argv)

1 0

[tor/master] Merge remote-tracking branch 'tor-github/pr/1463'
by teor＠torproject.org 25 Oct '19

25 Oct '19

commit b3d035d5c4ebe55a6793d80ade47e67ca514d4fd Merge: c00602cc3 fcdb66acd Author: teor <teor(a)torproject.org> Date: Fri Oct 25 10:27:41 2019 +1000 Merge remote-tracking branch 'tor-github/pr/1463' changes/ticket32237 | 5 + scripts/maint/rename_c_identifier.py | 261 +++++++++++++++++++++++++++++++++++ 2 files changed, 266 insertions(+)

1 0

[tor/release-0.4.2] Merge branch 'maint-0.4.2' into release-0.4.2
by teor＠torproject.org 24 Oct '19

24 Oct '19

commit 84e9060f4de1889fba8be99cff7e5bc05911a58f Merge: e73fb5b0e acd27f4b4 Author: teor <teor(a)torproject.org> Date: Fri Oct 25 09:42:10 2019 +1000 Merge branch 'maint-0.4.2' into release-0.4.2 changes/ticket28970 | 6 ++++++ src/feature/hs/hs_client.c | 8 ++++++-- 2 files changed, 12 insertions(+), 2 deletions(-)

1 0

[tor/release-0.4.2] hs-v3: Remove a BUG() caused by an acceptable race
by teor＠torproject.org 24 Oct '19

24 Oct '19

commit ed57a04a65a59ee744910a9db22a81359dac3491 Author: David Goulet <dgoulet(a)torproject.org> Date: Wed Oct 23 10:20:09 2019 -0400 hs-v3: Remove a BUG() caused by an acceptable race hs_client_purge_state() and hs_cache_clean_as_client() can remove a descriptor from the client cache with a NEWNYM or simply when the descriptor expires. Which means that for an INTRO circuit being established during that time, once it opens, we lookup the descriptor to get the IP object but hey surprised, no more descriptor. The approach here is minimalist that is accept the race and close the circuit since we can not continue. Before that, the circuit would stay opened and the client wait the SockTimeout. Fixers #28970. Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- changes/ticket28970 | 6 ++++++ src/feature/hs/hs_client.c | 8 ++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/changes/ticket28970 b/changes/ticket28970 new file mode 100644 index 000000000..138c575fc --- /dev/null +++ b/changes/ticket28970 @@ -0,0 +1,6 @@ + o Minor bugfixes (clietn, hidden service v3): + - Fix a BUG() assertion that occurs within a very small race window between + a client intro circuit opens and its descriptor that gets cleaned up from + the cache. The circuit is now closed which will trigger a re-fetch of the + descriptor and continue the HS connection. Fixes bug 28970; bugfix on + 0.3.2.1-alpha. diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c index 2a5765aec..fd2d26645 100644 --- a/src/feature/hs/hs_client.c +++ b/src/feature/hs/hs_client.c @@ -672,8 +672,12 @@ setup_intro_circ_auth_key(origin_circuit_t *circ) tor_assert(circ); desc = hs_cache_lookup_as_client(&circ->hs_ident->identity_pk); - if (BUG(desc == NULL)) { - /* Opening intro circuit without the descriptor is no good... */ + if (desc == NULL) { + /* There is a very small race window between the opening of this circuit + * and the client descriptor cache that gets purged (NEWNYM) or the + * cleaned up because it expired. Mark the circuit for close so a new + * descriptor fetch can occur. */ + circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_INTERNAL); goto end; }

1 0

[tor/release-0.4.2] Merge remote-tracking branch 'tor-github/pr/1464' into maint-0.4.2
by teor＠torproject.org 24 Oct '19

24 Oct '19

commit acd27f4b419907052d0677933fe5068a5c25953f Merge: 570d1367e ed57a04a6 Author: teor <teor(a)torproject.org> Date: Fri Oct 25 09:41:48 2019 +1000 Merge remote-tracking branch 'tor-github/pr/1464' into maint-0.4.2 changes/ticket28970 | 6 ++++++ src/feature/hs/hs_client.c | 8 ++++++-- 2 files changed, 12 insertions(+), 2 deletions(-)

1 0

[tor/maint-0.4.2] Merge remote-tracking branch 'tor-github/pr/1464' into maint-0.4.2
by teor＠torproject.org 24 Oct '19

24 Oct '19

commit acd27f4b419907052d0677933fe5068a5c25953f Merge: 570d1367e ed57a04a6 Author: teor <teor(a)torproject.org> Date: Fri Oct 25 09:41:48 2019 +1000 Merge remote-tracking branch 'tor-github/pr/1464' into maint-0.4.2 changes/ticket28970 | 6 ++++++ src/feature/hs/hs_client.c | 8 ++++++-- 2 files changed, 12 insertions(+), 2 deletions(-)

1 0