commit bce18a764252c66e605680e29a27ea30375a6db1 Author: George Kadianakis desnacked@riseup.net Date: Thu Jun 1 13:21:03 2017 +0300

prop224: Refactor parse_extended_hostname() to parse v3 addrs.

We need this func so that we recognize SOCKS conns to v3 addresses.

- Also rename rend_valid_service_id() to rend_valid_v2_service_id()

- Also move parse_extended_hostname() tests to their own unittest, and add a v3 address to the test as well.

Signed-off-by: David Goulet dgoulet@torproject.org --- src/or/connection_edge.c | 21 +++++++++++++-------- src/or/connection_edge.h | 3 ++- src/or/control.c | 4 ++-- src/or/rendcache.c | 4 ++-- src/or/rendclient.c | 2 +- src/or/rendcommon.c | 2 +- src/or/rendcommon.h | 2 +- src/or/rendservice.c | 2 +- src/test/test.c | 17 ----------------- src/test/test_hs_common.c | 38 +++++++++++++++++++++++++++++++++++++- 10 files changed, 60 insertions(+), 35 deletions(-)

diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 12ddc7e82..941b273cc 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -1558,7 +1558,7 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, }

/* Now, we handle everything that isn't a .onion address. */ - if (addresstype != ONION_HOSTNAME) { + if (addresstype != ONION_V2_HOSTNAME) { /* Not a hidden-service request. It's either a hostname or an IP, * possibly with a .exit that we stripped off. We're going to check * if we're allowed to connect/resolve there, and then launch the @@ -3679,10 +3679,12 @@ connection_ap_can_use_exit(const entry_connection_t *conn, }

/** If address is of the form "y.onion" with a well-formed handle y: - * Put a NUL after y, lower-case it, and return ONION_HOSTNAME. + * Put a NUL after y, lower-case it, and return ONION_V2_HOSTNAME or + * ONION_V3_HOSTNAME depending on the HS version. * * If address is of the form "x.y.onion" with a well-formed handle x: - * Drop "x.", put a NUL after y, lower-case it, and return ONION_HOSTNAME. + * Drop "x.", put a NUL after y, lower-case it, and return + * ONION_V2_HOSTNAME or ONION_V3_HOSTNAME depending on the HS version. * * If address is of the form "y.onion" with a badly-formed handle y: * Return BAD_HOSTNAME and log a message. @@ -3698,7 +3700,7 @@ parse_extended_hostname(char *address) { char *s; char *q; - char query[REND_SERVICE_ID_LEN_BASE32+1]; + char query[HS_SERVICE_ADDR_LEN_BASE32+1];

s = strrchr(address,'.'); if (!s) @@ -3718,14 +3720,17 @@ parse_extended_hostname(char *address) goto failed; /* reject sub-domain, as DNS does */ } q = (NULL == q) ? address : q + 1; - if (strlcpy(query, q, REND_SERVICE_ID_LEN_BASE32+1) >= - REND_SERVICE_ID_LEN_BASE32+1) + if (strlcpy(query, q, HS_SERVICE_ADDR_LEN_BASE32+1) >= + HS_SERVICE_ADDR_LEN_BASE32+1) goto failed; if (q != address) { memmove(address, q, strlen(q) + 1 /* also get \0 */); } - if (rend_valid_service_id(query)) { - return ONION_HOSTNAME; /* success */ + if (rend_valid_v2_service_id(query)) { + return ONION_V2_HOSTNAME; /* success */ + } + if (hs_address_is_valid(query)) { + return ONION_V3_HOSTNAME; } failed: /* otherwise, return to previous state and return 0 */ diff --git a/src/or/connection_edge.h b/src/or/connection_edge.h index 9987f88b8..914238fc1 100644 --- a/src/or/connection_edge.h +++ b/src/or/connection_edge.h @@ -98,7 +98,8 @@ int connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn,

/** Possible return values for parse_extended_hostname. */ typedef enum hostname_type_t { - NORMAL_HOSTNAME, ONION_HOSTNAME, EXIT_HOSTNAME, BAD_HOSTNAME + NORMAL_HOSTNAME, ONION_V2_HOSTNAME, ONION_V3_HOSTNAME, + EXIT_HOSTNAME, BAD_HOSTNAME } hostname_type_t; hostname_type_t parse_extended_hostname(char *address);

diff --git a/src/or/control.c b/src/or/control.c index 724d4b35c..e8cbd046d 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -4132,7 +4132,7 @@ handle_control_hsfetch(control_connection_t *conn, uint32_t len, /* Extract the first argument (either HSAddress or DescID). */ arg1 = smartlist_get(args, 0); /* Test if it's an HS address without the .onion part. */ - if (rend_valid_service_id(arg1)) { + if (rend_valid_v2_service_id(arg1)) { hsaddress = arg1; } else if (strcmpstart(arg1, v2_str) == 0 && rend_valid_descriptor_id(arg1 + v2_str_len) && @@ -4771,7 +4771,7 @@ handle_control_del_onion(control_connection_t *conn, return 0;

const char *service_id = smartlist_get(args, 0); - if (!rend_valid_service_id(service_id)) { + if (!rend_valid_v2_service_id(service_id)) { connection_printf_to_buf(conn, "512 Malformed Onion Service id\r\n"); goto out; } diff --git a/src/or/rendcache.c b/src/or/rendcache.c index 11b60b36a..792f6a130 100644 --- a/src/or/rendcache.c +++ b/src/or/rendcache.c @@ -512,7 +512,7 @@ rend_cache_lookup_entry(const char *query, int version, rend_cache_entry_t **e) tor_assert(rend_cache); tor_assert(query);

- if (!rend_valid_service_id(query)) { + if (!rend_valid_v2_service_id(query)) { ret = -EINVAL; goto end; } @@ -558,7 +558,7 @@ rend_cache_lookup_v2_desc_as_service(const char *query, rend_cache_entry_t **e) tor_assert(rend_cache_local_service); tor_assert(query);

- if (!rend_valid_service_id(query)) { + if (!rend_valid_v2_service_id(query)) { ret = -EINVAL; goto end; } diff --git a/src/or/rendclient.c b/src/or/rendclient.c index e47e1ef63..fbfba27a2 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -1466,7 +1466,7 @@ rend_parse_service_authorization(const or_options_t *options, goto err; } strlcpy(auth->onion_address, onion_address, REND_SERVICE_ID_LEN_BASE32+1); - if (!rend_valid_service_id(auth->onion_address)) { + if (!rend_valid_v2_service_id(auth->onion_address)) { log_warn(LD_CONFIG, "Onion address has wrong format: '%s'", onion_address); goto err; diff --git a/src/or/rendcommon.c b/src/or/rendcommon.c index 8b555a316..8829ede96 100644 --- a/src/or/rendcommon.c +++ b/src/or/rendcommon.c @@ -695,7 +695,7 @@ rend_get_service_id(crypto_pk_t *pk, char *out) /** Return true iff <b>query</b> is a syntactically valid service ID (as * generated by rend_get_service_id). */ int -rend_valid_service_id(const char *query) +rend_valid_v2_service_id(const char *query) { if (strlen(query) != REND_SERVICE_ID_LEN_BASE32) return 0; diff --git a/src/or/rendcommon.h b/src/or/rendcommon.h index 292f9277e..f03a57f2e 100644 --- a/src/or/rendcommon.h +++ b/src/or/rendcommon.h @@ -30,7 +30,7 @@ void rend_encoded_v2_service_descriptor_free( rend_encoded_v2_service_descriptor_t *desc); void rend_intro_point_free(rend_intro_point_t *intro);

-int rend_valid_service_id(const char *query); +int rend_valid_v2_service_id(const char *query); int rend_valid_descriptor_id(const char *query); int rend_valid_client_name(const char *client_name); int rend_encode_v2_descriptors(smartlist_t *descs_out, diff --git a/src/or/rendservice.c b/src/or/rendservice.c index a205b00c6..8dbab2e2a 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -890,7 +890,7 @@ int rend_service_del_ephemeral(const char *service_id) { rend_service_t *s; - if (!rend_valid_service_id(service_id)) { + if (!rend_valid_v2_service_id(service_id)) { log_warn(LD_CONFIG, "Requested malformed Onion Service id for removal."); return -1; } diff --git a/src/test/test.c b/src/test/test.c index 994a97ab0..f775102ba 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -534,25 +534,8 @@ test_rend_fns(void *arg) size_t intro_points_size; size_t encoded_size; int i; - char address1[] = "fooaddress.onion"; - char address2[] = "aaaaaaaaaaaaaaaa.onion"; - char address3[] = "fooaddress.exit"; - char address4[] = "www.torproject.org"; - char address5[] = "foo.abcdefghijklmnop.onion"; - char address6[] = "foo.bar.abcdefghijklmnop.onion"; - char address7[] = ".abcdefghijklmnop.onion";

(void)arg; - tt_assert(BAD_HOSTNAME == parse_extended_hostname(address1)); - tt_assert(ONION_HOSTNAME == parse_extended_hostname(address2)); - tt_str_op(address2,OP_EQ, "aaaaaaaaaaaaaaaa"); - tt_assert(EXIT_HOSTNAME == parse_extended_hostname(address3)); - tt_assert(NORMAL_HOSTNAME == parse_extended_hostname(address4)); - tt_assert(ONION_HOSTNAME == parse_extended_hostname(address5)); - tt_str_op(address5,OP_EQ, "abcdefghijklmnop"); - tt_assert(ONION_HOSTNAME == parse_extended_hostname(address6)); - tt_str_op(address6,OP_EQ, "abcdefghijklmnop"); - tt_assert(BAD_HOSTNAME == parse_extended_hostname(address7));

/* Initialize the service cache. */ rend_cache_init(); diff --git a/src/test/test_hs_common.c b/src/test/test_hs_common.c index c60deb2ab..f7e8e2811 100644 --- a/src/test/test_hs_common.c +++ b/src/test/test_hs_common.c @@ -14,6 +14,7 @@ #include "log_test_helpers.h" #include "hs_test_helpers.h"

+#include "connection_edge.h" #include "hs_common.h" #include "hs_service.h" #include "config.h" @@ -696,6 +697,38 @@ test_disaster_srv(void *arg) ; }

+static void +test_parse_extended_hostname(void *arg) +{ + (void) arg; + + char address1[] = "fooaddress.onion"; + char address2[] = "aaaaaaaaaaaaaaaa.onion"; + char address3[] = "fooaddress.exit"; + char address4[] = "www.torproject.org"; + char address5[] = "foo.abcdefghijklmnop.onion"; + char address6[] = "foo.bar.abcdefghijklmnop.onion"; + char address7[] = ".abcdefghijklmnop.onion"; + char address8[] = + "www.p3xnclpu4mu22dwaurjtsybyqk4xfjmcfz6z62yl24uwmhjatiwnlnad.onion"; + + tt_assert(BAD_HOSTNAME == parse_extended_hostname(address1)); + tt_assert(ONION_V2_HOSTNAME == parse_extended_hostname(address2)); + tt_str_op(address2,OP_EQ, "aaaaaaaaaaaaaaaa"); + tt_assert(EXIT_HOSTNAME == parse_extended_hostname(address3)); + tt_assert(NORMAL_HOSTNAME == parse_extended_hostname(address4)); + tt_assert(ONION_V2_HOSTNAME == parse_extended_hostname(address5)); + tt_str_op(address5,OP_EQ, "abcdefghijklmnop"); + tt_assert(ONION_V2_HOSTNAME == parse_extended_hostname(address6)); + tt_str_op(address6,OP_EQ, "abcdefghijklmnop"); + tt_assert(BAD_HOSTNAME == parse_extended_hostname(address7)); + tt_assert(ONION_V3_HOSTNAME == parse_extended_hostname(address8)); + tt_str_op(address8, OP_EQ, + "p3xnclpu4mu22dwaurjtsybyqk4xfjmcfz6z62yl24uwmhjatiwnlnad"); + + done: ; +} + struct testcase_t hs_common_tests[] = { { "build_address", test_build_address, TT_FORK, NULL, NULL }, @@ -713,7 +746,10 @@ struct testcase_t hs_common_tests[] = { NULL, NULL }, { "desc_reupload_logic", test_desc_reupload_logic, TT_FORK, NULL, NULL }, - { "disaster_srv", test_disaster_srv, TT_FORK, NULL, NULL }, + { "disaster_srv", test_disaster_srv, TT_FORK, + NULL, NULL }, + { "parse_extended_hostname", test_parse_extended_hostname, TT_FORK, + NULL, NULL },

END_OF_TESTCASES };