commit be6db23d1d4ce1185a7263f8554978e0fb9ea821 Author: Neel Chauhan neel@neelc.org Date: Tue Nov 24 19:05:27 2020 -0800
Some test and logic corrections --- src/feature/control/control_cmd.c | 4 +- src/feature/hs/hs_service.c | 32 +++++++------- src/feature/hs/hs_service.h | 2 +- src/feature/rend/rendservice.c | 2 +- src/test/test_hs_control.c | 88 ++++++++++++++------------------------- 5 files changed, 54 insertions(+), 74 deletions(-)
diff --git a/src/feature/control/control_cmd.c b/src/feature/control/control_cmd.c index 739577c506..8df9598c9f 100644 --- a/src/feature/control/control_cmd.c +++ b/src/feature/control/control_cmd.c @@ -1831,8 +1831,9 @@ handle_control_add_onion(control_connection_t *conn, } } else if (!strcasecmp(arg->key, "ClientAuthV3")) { hs_service_authorized_client_t *client_v3 = - parse_authorized_client_key(arg->value); + parse_authorized_client_key(arg->value, false); if (!client_v3) { + control_write_endreply(conn, 512, "Cannot decode v3 client auth key"); goto out; }
@@ -1925,7 +1926,6 @@ handle_control_add_onion(control_connection_t *conn, auth_clients, auth_clients_v3, &service_id); port_cfgs = NULL; /* port_cfgs is now owned by the rendservice code. */ auth_clients = NULL; /* so is auth_clients */ - auth_clients_v3 = NULL; /* so is auth_clients_v3 */ switch (ret) { case RSAE_OKAY: { diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c index 53b90ce374..c173dbcbfe 100644 --- a/src/feature/hs/hs_service.c +++ b/src/feature/hs/hs_service.c @@ -1119,17 +1119,19 @@ client_filename_is_valid(const char *filename) * * Return the key on success, return NULL, otherwise. */ hs_service_authorized_client_t * -parse_authorized_client_key(const char *key_str) +parse_authorized_client_key(const char *key_str, bool log) { hs_service_authorized_client_t *client = NULL;
- /* We expect a specific length of the base32 encoded key so make sure we + /* We expect a specific length of the base64 encoded key so make sure we * have that so we don't successfully decode a value with a different length * and end up in trouble when copying the decoded key into a fixed length * buffer. */ if (strlen(key_str) != BASE32_NOPAD_LEN(CURVE25519_PUBKEY_LEN)) { - log_warn(LD_REND, "Client authorization encoded base32 public key " - "length is invalid: %s", key_str); + if (log) { + log_warn(LD_REND, "Client authorization encoded base32 public key " + "length is invalid: %s", key_str); + } goto err; }
@@ -1138,8 +1140,10 @@ parse_authorized_client_key(const char *key_str) sizeof(client->client_pk.public_key), key_str, strlen(key_str)) != sizeof(client->client_pk.public_key)) { - log_warn(LD_REND, "Client authorization public key cannot be decoded: %s", - key_str); + if (log) { + log_warn(LD_REND, "Client authorization public key cannot be decoded: " + "%s", key_str); + } goto err; }
@@ -1198,7 +1202,7 @@ parse_authorized_client(const char *client_key_str) goto err; }
- if ((client = parse_authorized_client_key(pubkey_b32)) == NULL) { + if ((client = parse_authorized_client_key(pubkey_b32, true)) == NULL) { goto err; }
@@ -3753,14 +3757,14 @@ hs_service_add_ephemeral(ed25519_secret_key_t *sk, smartlist_t *ports, goto err; }
- if (service->config.clients == NULL) { - service->config.clients = smartlist_new(); - } - SMARTLIST_FOREACH(auth_clients_v3, hs_service_authorized_client_t *, c, { - if (c != NULL) { - smartlist_add(service->config.clients, c); + if (auth_clients_v3) { + if (service->config.clients == NULL) { + service->config.clients = smartlist_new(); } - }); + SMARTLIST_FOREACH(auth_clients_v3, hs_service_authorized_client_t *, c, { + smartlist_add(service->config.clients, c); + }); + }
/* Build the onion address for logging purposes but also the control port * uses it for the HS_DESC event. */ diff --git a/src/feature/hs/hs_service.h b/src/feature/hs/hs_service.h index 4d49929127..12698a483c 100644 --- a/src/feature/hs/hs_service.h +++ b/src/feature/hs/hs_service.h @@ -390,7 +390,7 @@ void hs_service_dump_stats(int severity); void hs_service_circuit_cleanup_on_close(const circuit_t *circ);
hs_service_authorized_client_t * -parse_authorized_client_key(const char *key_str); +parse_authorized_client_key(const char *key_str, bool log);
void service_authorized_client_free_(hs_service_authorized_client_t *client); diff --git a/src/feature/rend/rendservice.c b/src/feature/rend/rendservice.c index 45b1d3d822..add25579b3 100644 --- a/src/feature/rend/rendservice.c +++ b/src/feature/rend/rendservice.c @@ -3818,7 +3818,7 @@ upload_service_descriptor(rend_service_t *service) smartlist_clear(client_cookies); switch (service->auth_type) { case REND_NO_AUTH: - case REND_V3_AUTH: + case REND_V3_AUTH: /* Do nothing here. */ break; case REND_BASIC_AUTH: diff --git a/src/test/test_hs_control.c b/src/test/test_hs_control.c index e1a5ab4841..e5401b4ce7 100644 --- a/src/test/test_hs_control.c +++ b/src/test/test_hs_control.c @@ -743,70 +743,46 @@ test_hs_control_add_onion_with_bad_pubkey(void *arg) static void test_hs_add_onion_helper_add_service(void *arg) { - int hs_version_good, hs_version_bad; - add_onion_secret_key_t sk_good, sk_bad; - ed25519_public_key_t pk_good, pk_bad; - char *key_new_blob_good = NULL, *key_new_blob_bad = NULL; - const char *key_new_alg_good = NULL, *key_new_alg_bad = NULL; - hs_service_authorized_client_t *client_good, *client_bad; - smartlist_t *list_v2, *list_good, *list_bad; - hs_service_ht *global_map; - rend_service_port_config_t *portcfg; - smartlist_t *portcfgs; - char *address_out_good, *address_out_bad; + control_connection_t conn; + char *args = NULL, *cp1 = NULL; + size_t sz;
(void) arg;
hs_init(); - global_map = get_hs_service_map(); - - portcfg = rend_service_parse_port_config("8080", ",", NULL); - portcfgs = smartlist_new(); - smartlist_add(portcfgs, portcfg); - - memset(&sk_good, 0, sizeof(sk_good)); - memset(&sk_bad, 0, sizeof(sk_bad)); - - add_onion_helper_keyarg("NEW:ED25519-V3", 0, &key_new_alg_good, - &key_new_blob_good, &sk_good, &hs_version_good, NULL); - add_onion_helper_keyarg("NEW:ED25519-V3", 0, &key_new_alg_bad, - &key_new_blob_bad, &sk_bad, &hs_version_bad, NULL);
- ed25519_public_key_generate(&pk_good, sk_good.v3); - ed25519_public_key_generate(&pk_bad, sk_bad.v3); - - client_good = parse_authorized_client_key( - "N2NU7BSRL6YODZCYPN4CREB54TYLKGIE2KYOQWLFYC23ZJVCE5DQ"); - client_bad = parse_authorized_client_key("dummy"); - - list_v2 = smartlist_new(); - list_good = smartlist_new(); - smartlist_add(list_good, client_good); - list_bad = smartlist_new(); - smartlist_add(list_bad, client_bad); - - add_onion_helper_add_service(HS_VERSION_THREE, &sk_good, portcfgs, 1, 1, - REND_V3_AUTH, list_v2, list_good, &address_out_good); - add_onion_helper_add_service(HS_VERSION_THREE, &sk_bad, portcfgs, 1, 1, - REND_V3_AUTH, list_v2, list_bad, &address_out_bad); - - hs_service_t *srv_good = find_service(global_map, &pk_good); - hs_service_t *srv_bad = find_service(global_map, &pk_bad); + memset(&conn, 0, sizeof(control_connection_t)); + TO_CONN(&conn)->outbuf = buf_new(); + conn.current_cmd = tor_strdup("ADD_ONION"); + args = tor_strdup("ED25519-V3:KLMQ4CLKwlDCHuMPn8j3od33cU5LhnrLNoZh7CWChl3VkY" + "pNAkeP5dGW8xeKR9HxQBWQ/w7Kr12lA/U8Pd/oxw== " + "ClientAuthV3=dz4q5xqlb4ldnbs72iarrml4ephk3du4i7o2cgiva5lwr6wkquja " + "Flags=V3Auth Port=9735,127.0.0.1"); + handle_control_command(&conn, (uint32_t) strlen(args), args); + cp1 = buf_get_contents(TO_CONN(&conn)->outbuf, &sz); + tt_str_op(cp1, OP_EQ, + "250-ServiceID=n35etu3yjxrqjpntmfziom5sjwspoydchmelc4xleoy4jk2u4lziz2yd\r\n" + "250-ClientAuthV3=dz4q5xqlb4ldnbs72iarrml4ephk3du4i7o2cgiva5lwr6wkquja\r\n" + "250 OK\r\n"); + tor_free(args); + tor_free(cp1);
- tt_int_op(smartlist_len(srv_good->config.clients), OP_EQ, 1); - tt_int_op(smartlist_len(srv_bad->config.clients), OP_EQ, 0); + args = tor_strdup("ED25519-V3:iIU8EBi71qE7G6UTsROU1kWN0JMrRP/YukC0Xk5WLGyil3" + "gm4u3wEBXr+/TaCpXS+65Pcdqz+PG+4+oWHLN05A== " + "ClientAuthV3=dummy Flags=V3Auth Port=9735,127.0.0.1"); + handle_control_command(&conn, (uint32_t) strlen(args), args); + cp1 = buf_get_contents(TO_CONN(&conn)->outbuf, &sz); + tt_str_op(cp1, OP_EQ, "512 Cannot decode v3 client auth key\r\n");
done: - tor_free(key_new_blob_good); - tor_free(key_new_blob_bad); - tor_free(address_out_good); - tor_free(address_out_bad); - - service_authorized_client_free(client_good); - - smartlist_free(list_v2); - smartlist_free(list_good); - smartlist_free(list_bad); + tor_free(args); + tor_free(cp1); + tor_free(conn.current_cmd); + buf_free(TO_CONN(&conn)->outbuf); + SMARTLIST_FOREACH(conn.ephemeral_onion_services, char *, + service, tor_free(service)); + smartlist_free(conn.ephemeral_onion_services); + hs_client_free_all(); }
struct testcase_t hs_control_tests[] = {