commit 562957e0db200ea58fcf878d61314d3a2b0c297c Author: Guinness <guinness@crans.org> Date: Wed Jul 8 17:46:16 2020 +0200 socks: Returns 0xF6 only if BAD_HOSTNAME This commit modifies the behavior of `parse_extended_address` in such a way that if it fails, it will always return a `BAD_HOSTNAME` value, which is then used to return the 0xF6 extended error code. This way, in any case that is not a valid v2 address, we return the 0xF6 error code, which is the expected behavior. Signed-off-by: David Goulet <dgoulet@torproject.org> --- changes/ticket33873 | 4 ++++ src/core/or/connection_edge.c | 5 ++++- src/test/test_hs_common.c | 8 +++++++- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/changes/ticket33873 b/changes/ticket33873 new file mode 100644 index 000000000..c45191181 --- /dev/null +++ b/changes/ticket33873 @@ -0,0 +1,4 @@ + o Minor bugfix (SOCKS, onion service client): + - Also detect bad v3 onion service address of the wrong length when + returning the F6 ExtendedErrors code. Fixes bug 33873; bugfix on + 0.4.3.1-alpha. diff --git a/src/core/or/connection_edge.c b/src/core/or/connection_edge.c index fc77db833..26595003c 100644 --- a/src/core/or/connection_edge.c +++ b/src/core/or/connection_edge.c @@ -1663,6 +1663,9 @@ parse_extended_hostname(char *address, hostname_type_t *type_out) log_warn(LD_APP, "Invalid %shostname %s; rejecting", is_onion ? "onion " : "", safe_str_client(address)); + if (*type_out == ONION_V3_HOSTNAME) { + *type_out = BAD_HOSTNAME; + } return false; } @@ -2139,7 +2142,7 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, if (!parse_extended_hostname(socks->address, &addresstype)) { control_event_client_status(LOG_WARN, "SOCKS_BAD_HOSTNAME HOSTNAME=%s", escaped(socks->address)); - if (addresstype == ONION_V3_HOSTNAME) { + if (addresstype == BAD_HOSTNAME) { conn->socks_request->socks_extended_error_code = SOCKS5_HS_BAD_ADDRESS; } connection_mark_unattached_ap(conn, END_STREAM_REASON_TORPROTOCOL); diff --git a/src/test/test_hs_common.c b/src/test/test_hs_common.c index 4a161db33..9202074e2 100644 --- a/src/test/test_hs_common.c +++ b/src/test/test_hs_common.c @@ -792,6 +792,8 @@ test_parse_extended_hostname(void *arg) "www.25njqamcweflpvkl73j4szahhihoc4xt3ktcgjnpaingr5yhkenl5sid.onion"; char address9[] = "www.15njqamcweflpvkl73j4szahhihoc4xt3ktcgjnpaingr5yhkenl5sid.onion"; + char address10[] = + "15njqamcweflpvkl73j4szahhihoc4xt3ktcgjnpaingr5yhkenl5sid7jdl.onion"; tt_assert(!parse_extended_hostname(address1, &type)); tt_int_op(type, OP_EQ, BAD_HOSTNAME); @@ -824,7 +826,11 @@ test_parse_extended_hostname(void *arg) /* Invalid v3 address. */ tt_assert(!parse_extended_hostname(address9, &type)); - tt_int_op(type, OP_EQ, ONION_V3_HOSTNAME); + tt_int_op(type, OP_EQ, BAD_HOSTNAME); + + /* Invalid v3 address: too long */ + tt_assert(!parse_extended_hostname(address10, &type)); + tt_int_op(type, OP_EQ, BAD_HOSTNAME); done: ; }