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

[tor/release-0.4.2] dir: Look if circuit is closed in connection_dir_is_anonymous()
by teor＠torproject.org 25 Oct '19

25 Oct '19

commit 09468cc58b52132af1232e2cd3925c273382bba6 Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Oct 24 11:08:25 2019 -0400 dir: Look if circuit is closed in connection_dir_is_anonymous() Before inspecting the p_chan, we must check if the circuit is marked for close because if it is the case, the channels are nullified from the circuit. Several valid cases can mark the circuit for close of the directory connection. Fixes #31958 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- changes/ticket31958 | 5 +++++ src/feature/dircommon/directory.c | 12 +++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/changes/ticket31958 b/changes/ticket31958 new file mode 100644 index 000000000..8206064df --- /dev/null +++ b/changes/ticket31958 @@ -0,0 +1,5 @@ + o Minor bugfixes (directory): + - When checking if a directory connection is anonymous, test if the circuit + was marked for close before looking at its channel. This avoids a BUG() + stacktrace in case it was previously closed. Fixes bug 31958; bugfix on + 0.4.2.1-alpha. diff --git a/src/feature/dircommon/directory.c b/src/feature/dircommon/directory.c index b3db0aa10..1ac35dd8b 100644 --- a/src/feature/dircommon/directory.c +++ b/src/feature/dircommon/directory.c @@ -225,7 +225,17 @@ connection_dir_is_anonymous(const dir_connection_t *dir_conn) return false; } - /* Get the previous channel to learn if it is a client or relay link. */ + /* It is possible that the circuit was closed because one of the channel was + * closed or a DESTROY cell was received. Either way, this connection can + * not continue so return that it is not anonymous since we can not know for + * sure if it is. */ + if (circ->marked_for_close) { + return false; + } + + /* Get the previous channel to learn if it is a client or relay link. We + * 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"); return false;

1 0

[tor/release-0.4.2] 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/release-0.4.2] 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/release-0.4.2] 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/maint-0.4.2] 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/maint-0.4.2] dir: Look if circuit is closed in connection_dir_is_anonymous()
by teor＠torproject.org 25 Oct '19

25 Oct '19

commit 09468cc58b52132af1232e2cd3925c273382bba6 Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Oct 24 11:08:25 2019 -0400 dir: Look if circuit is closed in connection_dir_is_anonymous() Before inspecting the p_chan, we must check if the circuit is marked for close because if it is the case, the channels are nullified from the circuit. Several valid cases can mark the circuit for close of the directory connection. Fixes #31958 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- changes/ticket31958 | 5 +++++ src/feature/dircommon/directory.c | 12 +++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/changes/ticket31958 b/changes/ticket31958 new file mode 100644 index 000000000..8206064df --- /dev/null +++ b/changes/ticket31958 @@ -0,0 +1,5 @@ + o Minor bugfixes (directory): + - When checking if a directory connection is anonymous, test if the circuit + was marked for close before looking at its channel. This avoids a BUG() + stacktrace in case it was previously closed. Fixes bug 31958; bugfix on + 0.4.2.1-alpha. diff --git a/src/feature/dircommon/directory.c b/src/feature/dircommon/directory.c index b3db0aa10..1ac35dd8b 100644 --- a/src/feature/dircommon/directory.c +++ b/src/feature/dircommon/directory.c @@ -225,7 +225,17 @@ connection_dir_is_anonymous(const dir_connection_t *dir_conn) return false; } - /* Get the previous channel to learn if it is a client or relay link. */ + /* It is possible that the circuit was closed because one of the channel was + * closed or a DESTROY cell was received. Either way, this connection can + * not continue so return that it is not anonymous since we can not know for + * sure if it is. */ + if (circ->marked_for_close) { + return false; + } + + /* Get the previous channel to learn if it is a client or relay link. We + * 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"); return false;

1 0

[tor/maint-0.4.2] 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/maint-0.4.2] 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] dir: Look if circuit is closed in connection_dir_is_anonymous()
by teor＠torproject.org 25 Oct '19

25 Oct '19

commit 09468cc58b52132af1232e2cd3925c273382bba6 Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Oct 24 11:08:25 2019 -0400 dir: Look if circuit is closed in connection_dir_is_anonymous() Before inspecting the p_chan, we must check if the circuit is marked for close because if it is the case, the channels are nullified from the circuit. Several valid cases can mark the circuit for close of the directory connection. Fixes #31958 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- changes/ticket31958 | 5 +++++ src/feature/dircommon/directory.c | 12 +++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/changes/ticket31958 b/changes/ticket31958 new file mode 100644 index 000000000..8206064df --- /dev/null +++ b/changes/ticket31958 @@ -0,0 +1,5 @@ + o Minor bugfixes (directory): + - When checking if a directory connection is anonymous, test if the circuit + was marked for close before looking at its channel. This avoids a BUG() + stacktrace in case it was previously closed. Fixes bug 31958; bugfix on + 0.4.2.1-alpha. diff --git a/src/feature/dircommon/directory.c b/src/feature/dircommon/directory.c index b3db0aa10..1ac35dd8b 100644 --- a/src/feature/dircommon/directory.c +++ b/src/feature/dircommon/directory.c @@ -225,7 +225,17 @@ connection_dir_is_anonymous(const dir_connection_t *dir_conn) return false; } - /* Get the previous channel to learn if it is a client or relay link. */ + /* It is possible that the circuit was closed because one of the channel was + * closed or a DESTROY cell was received. Either way, this connection can + * not continue so return that it is not anonymous since we can not know for + * sure if it is. */ + if (circ->marked_for_close) { + return false; + } + + /* Get the previous channel to learn if it is a client or relay link. We + * 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"); return false;

1 0

[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