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>
This commit is contained in:
Guinness 2020-07-08 17:46:16 +02:00 committed by David Goulet
parent 9603d8af0b
commit 562957e0db
3 changed files with 15 additions and 2 deletions

4
changes/ticket33873 Normal file
View File

@ -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.

View File

@ -1663,6 +1663,9 @@ parse_extended_hostname(char *address, hostname_type_t *type_out)
log_warn(LD_APP, "Invalid %shostname %s; rejecting", log_warn(LD_APP, "Invalid %shostname %s; rejecting",
is_onion ? "onion " : "", is_onion ? "onion " : "",
safe_str_client(address)); safe_str_client(address));
if (*type_out == ONION_V3_HOSTNAME) {
*type_out = BAD_HOSTNAME;
}
return false; return false;
} }
@ -2139,7 +2142,7 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn,
if (!parse_extended_hostname(socks->address, &addresstype)) { if (!parse_extended_hostname(socks->address, &addresstype)) {
control_event_client_status(LOG_WARN, "SOCKS_BAD_HOSTNAME HOSTNAME=%s", control_event_client_status(LOG_WARN, "SOCKS_BAD_HOSTNAME HOSTNAME=%s",
escaped(socks->address)); escaped(socks->address));
if (addresstype == ONION_V3_HOSTNAME) { if (addresstype == BAD_HOSTNAME) {
conn->socks_request->socks_extended_error_code = SOCKS5_HS_BAD_ADDRESS; conn->socks_request->socks_extended_error_code = SOCKS5_HS_BAD_ADDRESS;
} }
connection_mark_unattached_ap(conn, END_STREAM_REASON_TORPROTOCOL); connection_mark_unattached_ap(conn, END_STREAM_REASON_TORPROTOCOL);

View File

@ -792,6 +792,8 @@ test_parse_extended_hostname(void *arg)
"www.25njqamcweflpvkl73j4szahhihoc4xt3ktcgjnpaingr5yhkenl5sid.onion"; "www.25njqamcweflpvkl73j4szahhihoc4xt3ktcgjnpaingr5yhkenl5sid.onion";
char address9[] = char address9[] =
"www.15njqamcweflpvkl73j4szahhihoc4xt3ktcgjnpaingr5yhkenl5sid.onion"; "www.15njqamcweflpvkl73j4szahhihoc4xt3ktcgjnpaingr5yhkenl5sid.onion";
char address10[] =
"15njqamcweflpvkl73j4szahhihoc4xt3ktcgjnpaingr5yhkenl5sid7jdl.onion";
tt_assert(!parse_extended_hostname(address1, &type)); tt_assert(!parse_extended_hostname(address1, &type));
tt_int_op(type, OP_EQ, BAD_HOSTNAME); tt_int_op(type, OP_EQ, BAD_HOSTNAME);
@ -824,7 +826,11 @@ test_parse_extended_hostname(void *arg)
/* Invalid v3 address. */ /* Invalid v3 address. */
tt_assert(!parse_extended_hostname(address9, &type)); 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: ; done: ;
} }