- tor-commits - lists.torproject.org

[tor/master] test/cell_formats: Expand the IPv6-only EXTEND2 test
by nickm＠torproject.org 29 Apr '20

29 Apr '20

commit f6c8a8c5387eaf35470f08e9cd9b35f05bdc1057 Author: teor <teor(a)torproject.org> Date: Tue Apr 14 18:44:06 2020 +1000 test/cell_formats: Expand the IPv6-only EXTEND2 test Part of 33901. --- src/test/test_cell_formats.c | 41 ++++++++++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/src/test/test_cell_formats.c b/src/test/test_cell_formats.c index 3d3f151fc..f9ff101c9 100644 --- a/src/test/test_cell_formats.c +++ b/src/test/test_cell_formats.c @@ -769,6 +769,40 @@ test_cfmt_extend_cells(void *arg) tt_mem_op("brownshoesdontmakeit/brownshoesd", OP_EQ, ec.ed_pubkey.pubkey, 32); + /* Now try IPv6 without IPv4 */ + memset(p, 0, sizeof(p)); + memcpy(p, "\x02", 1); + memcpy(p+1, "\x02\x14" "anthropomorphization", 22); + memcpy(p+23, "\x01\x12" "xxxxxxxxxxxxxxxxYY", 20); + memcpy(p+43, "\xff\xff\x00\x20", 4); + tt_int_op(0, OP_EQ, extend_cell_parse(&ec, RELAY_COMMAND_EXTEND2, + p, sizeof(p))); + tt_int_op(RELAY_COMMAND_EXTEND2, OP_EQ, ec.cell_type); + tt_assert(fast_mem_is_zero((const char *)&ec.orport_ipv4.addr, + sizeof(tor_addr_t))); + tt_int_op(0, OP_EQ, ec.orport_ipv4.port); + tt_str_op("7878:7878:7878:7878:7878:7878:7878:7878", + OP_EQ, fmt_addr(&ec.orport_ipv6.addr)); + tt_int_op(22873, OP_EQ, ec.orport_ipv6.port); + tt_assert(ed25519_public_key_is_zero(&ec.ed_pubkey)); + tt_mem_op(ec.node_id,OP_EQ, "anthropomorphization", 20); + tt_int_op(cc->cell_type, OP_EQ, CELL_CREATE2); + tt_int_op(cc->handshake_type, OP_EQ, 0xffff); + tt_int_op(cc->handshake_len, OP_EQ, 32); + tt_int_op(0, OP_EQ, extend_cell_format(&p2_cmd, &p2_len, p2, &ec)); + tt_int_op(p2_cmd, OP_EQ, RELAY_COMMAND_EXTEND2); + tt_int_op(p2_len, OP_EQ, 47+32); + test_memeq_hex(p2, + /* Two items */ + "02" + /* The next is an RSA digest: anthropomorphization */ + "0214" "616e7468726f706f6d6f727068697a6174696f6e" + /*IPv6 address */ + "0112" "78787878787878787878787878787878" "5959" + /* Now the handshake prologue */ + "ffff0020"); + tt_int_op(0, OP_EQ, create_cell_format_relayed(&cell, cc)); + /* == Now try parsing some junk */ /* Try a too-long handshake */ @@ -824,13 +858,6 @@ test_cfmt_extend_cells(void *arg) memcpy(p+48, "\xff\xff\x00\x20", 4); tt_int_op(-1, OP_EQ, extend_cell_parse(&ec, RELAY_COMMAND_EXTEND2, p, sizeof(p))); - memset(p, 0, sizeof(p)); - memcpy(p, "\x02", 1); - memcpy(p+1, "\x02\x14" "anarchoindividualist", 22); - memcpy(p+23, "\x01\x12" "xxxxxxxxxxxxxxxxYY", 18); - memcpy(p+41, "\xff\xff\x00\x20", 4); - tt_int_op(0, OP_EQ, extend_cell_parse(&ec, RELAY_COMMAND_EXTEND2, - p, sizeof(p))); /* Running out of space in specifiers */ memset(p,0,sizeof(p));

1 0

[tor/master] core/or: Support IPv6 EXTEND2 cells
by nickm＠torproject.org 29 Apr '20

29 Apr '20

commit bd6ab90ad4b40a64c604c1a4b6b37da6991fad9e Author: teor <teor(a)torproject.org> Date: Tue Apr 14 15:08:42 2020 +1000 core/or: Support IPv6 EXTEND2 cells Allow clients and relays to send dual-stack and IPv6-only EXTEND2 cells. Parse dual-stack and IPv6-only EXTEND2 cells on relays. Relays do not make connections or extend circuits via IPv6: that's the next step. Closes ticket 33901. --- changes/ticket33901 | 4 ++++ src/core/or/onion.c | 41 ++++++++++++++++++++++++++++++++++------- src/lib/net/address.c | 14 ++++++++++++++ src/lib/net/address.h | 1 + src/test/test_cell_formats.c | 39 ++++++++++++++++++++++++++------------- 5 files changed, 79 insertions(+), 20 deletions(-) diff --git a/changes/ticket33901 b/changes/ticket33901 new file mode 100644 index 000000000..b824cc5b0 --- /dev/null +++ b/changes/ticket33901 @@ -0,0 +1,4 @@ + o Minor features (IPv6, relay): + - Allow clients and relays to send dual-stack and IPv6-only EXTEND2 cells. + Parse dual-stack and IPv6-only EXTEND2 cells on relays. + Closes ticket 33901. diff --git a/src/core/or/onion.c b/src/core/or/onion.c index 45144b5e6..543d9f3e4 100644 --- a/src/core/or/onion.c +++ b/src/core/or/onion.c @@ -240,11 +240,21 @@ created_cell_parse(created_cell_t *cell_out, const cell_t *cell_in) static int check_extend_cell(const extend_cell_t *cell) { + const bool is_extend2 = (cell->cell_type == RELAY_COMMAND_EXTEND2); + if (tor_digest_is_zero((const char*)cell->node_id)) return -1; - /* We don't currently allow EXTEND2 cells without an IPv4 address */ - if (tor_addr_family(&cell->orport_ipv4.addr) == AF_UNSPEC) - return -1; + if (tor_addr_family(&cell->orport_ipv4.addr) == AF_UNSPEC) { + /* EXTEND cells must have an IPv4 address. */ + if (!is_extend2) { + return -1; + } + /* EXTEND2 cells must have at least one IP address. + * It can be IPv4 or IPv6. */ + if (tor_addr_family(&cell->orport_ipv6.addr) == AF_UNSPEC) { + return -1; + } + } if (cell->create_cell.cell_type == CELL_CREATE) { if (cell->cell_type != RELAY_COMMAND_EXTEND) return -1; @@ -364,7 +374,12 @@ extend_cell_from_extend2_cell_body(extend_cell_t *cell_out, } } - if (!found_rsa_id || !found_ipv4) /* These are mandatory */ + /* EXTEND2 cells must have an RSA ID */ + if (!found_rsa_id) + return -1; + + /* EXTEND2 cells must have at least one IP address */ + if (!found_ipv4 && !found_ipv6) return -1; return create_cell_from_create2_cell_body(&cell_out->create_cell, @@ -620,12 +635,13 @@ extend_cell_format(uint8_t *command_out, uint16_t *len_out, break; case RELAY_COMMAND_EXTEND2: { - uint8_t n_specifiers = 2; + uint8_t n_specifiers = 1; *command_out = RELAY_COMMAND_EXTEND2; extend2_cell_body_t *cell = extend2_cell_body_new(); link_specifier_t *ls; - { - /* IPv4 specifier first. */ + if (tor_addr_port_is_valid_ap(&cell_in->orport_ipv4, 0)) { + /* Maybe IPv4 specifier first. */ + ++n_specifiers; ls = link_specifier_new(); extend2_cell_body_add_ls(cell, ls); ls->ls_type = LS_IPV4; @@ -651,6 +667,17 @@ extend_cell_format(uint8_t *command_out, uint16_t *len_out, ls->ls_len = 32; memcpy(ls->un_ed25519_id, cell_in->ed_pubkey.pubkey, 32); } + if (tor_addr_port_is_valid_ap(&cell_in->orport_ipv6, 0)) { + /* Then maybe IPv6 specifier. */ + ++n_specifiers; + ls = link_specifier_new(); + extend2_cell_body_add_ls(cell, ls); + ls->ls_type = LS_IPV6; + ls->ls_len = 18; + tor_addr_get_ipv6_bytes((char *)ls->un_ipv6_addr, + &cell_in->orport_ipv6.addr); + ls->un_ipv6_port = cell_in->orport_ipv6.port; + } cell->n_spec = n_specifiers; /* Now, the handshake */ diff --git a/src/lib/net/address.c b/src/lib/net/address.c index 5dbef6a79..5fe9abca1 100644 --- a/src/lib/net/address.c +++ b/src/lib/net/address.c @@ -903,6 +903,20 @@ tor_addr_from_in6(tor_addr_t *dest, const struct in6_addr *in6) tor_addr_from_ipv6_bytes(dest, (const char*)in6->s6_addr); } +/** Set the 16 bytes at dest to equal the IPv6 address src. + * src must be an IPv6 address, if it is not, log a warning, and clear + * dest. */ +void +tor_addr_get_ipv6_bytes(char *dest, const tor_addr_t *src) +{ + tor_assert(dest); + tor_assert(src); + memset(dest, 0, 16); + IF_BUG_ONCE(src->family != AF_INET6) + return; + memcpy(dest, src->addr.in6_addr.s6_addr, 16); +} + /** Copy a tor_addr_t from src to dest. */ void diff --git a/src/lib/net/address.h b/src/lib/net/address.h index 498449493..f1c223310 100644 --- a/src/lib/net/address.h +++ b/src/lib/net/address.h @@ -303,6 +303,7 @@ void tor_addr_from_ipv6_bytes(tor_addr_t *dest, const char *bytes); #define tor_addr_from_in(dest, in) \ tor_addr_from_ipv4n((dest), (in)->s_addr); void tor_addr_from_in6(tor_addr_t *dest, const struct in6_addr *in6); +void tor_addr_get_ipv6_bytes(char *dest, const tor_addr_t *src); int tor_addr_is_null(const tor_addr_t *addr); int tor_addr_is_loopback(const tor_addr_t *addr); diff --git a/src/test/test_cell_formats.c b/src/test/test_cell_formats.c index 8d6d1940f..3d3f151fc 100644 --- a/src/test/test_cell_formats.c +++ b/src/test/test_cell_formats.c @@ -713,16 +713,20 @@ test_cfmt_extend_cells(void *arg) tt_mem_op(cc->onionskin,OP_EQ, b, 99+20); tt_int_op(0, OP_EQ, extend_cell_format(&p2_cmd, &p2_len, p2, &ec)); tt_int_op(p2_cmd, OP_EQ, RELAY_COMMAND_EXTEND2); - /* We'll generate it minus the IPv6 address and minus the konami code */ - tt_int_op(p2_len, OP_EQ, 89+99-34-20); + /* We'll generate it minus the konami code */ + tt_int_op(p2_len, OP_EQ, 89+99-34); test_memeq_hex(p2, - /* Two items: one that same darn IP address. */ - "02000612F40001F0F1" - /* The next is a digest : anthropomorphization */ - "0214616e7468726f706f6d6f727068697a6174696f6e" + /* Three items */ + "03" + /* IPv4 address */ + "0006" "12F40001" "F0F1" + /* The next is an RSA digest: anthropomorphization */ + "0214" "616e7468726f706f6d6f727068697a6174696f6e" + /*IPv6 address */ + "0112" "20020000000000000000000000f0c51e" "1112" /* Now the handshake prologue */ "01050063"); - tt_mem_op(p2+1+8+22+4,OP_EQ, b, 99+20); + tt_mem_op(p2+1+8+22+20+4, OP_EQ, b, 99+20); tt_int_op(0, OP_EQ, create_cell_format_relayed(&cell, cc)); /* Now let's add an ed25519 key to that extend2 cell. */ @@ -732,22 +736,31 @@ test_cfmt_extend_cells(void *arg) /* As before, since we aren't extending by ed25519. */ get_options_mutable()->ExtendByEd25519ID = 0; tt_int_op(0, OP_EQ, extend_cell_format(&p2_cmd, &p2_len, p2, &ec)); - tt_int_op(p2_len, OP_EQ, 89+99-34-20); + tt_int_op(p2_len, OP_EQ, 89+99-34); test_memeq_hex(p2, - "02000612F40001F0F1" + "03" + "000612F40001F0F1" "0214616e7468726f706f6d6f727068697a6174696f6e" + "011220020000000000000000000000f0c51e1112" "01050063"); /* Now try with the ed25519 ID. */ get_options_mutable()->ExtendByEd25519ID = 1; tt_int_op(0, OP_EQ, extend_cell_format(&p2_cmd, &p2_len, p2, &ec)); - tt_int_op(p2_len, OP_EQ, 89+99-34-20 + 34); + tt_int_op(p2_len, OP_EQ, 89+99); test_memeq_hex(p2, - "03000612F40001F0F1" + /* Four items */ + "04" + /* IPv4 address */ + "0006" "12F40001" "F0F1" + /* The next is an RSA digest: anthropomorphization */ "0214616e7468726f706f6d6f727068697a6174696f6e" - // ed digest follows: + /* Then an ed public key: brownshoesdontmakeit/brownshoesd */ "0320" "62726f776e73686f6573646f6e746d616b656" "9742f62726f776e73686f657364" + /*IPv6 address */ + "0112" "20020000000000000000000000f0c51e" "1112" + /* Now the handshake prologue */ "01050063"); /* Can we parse that? Did the key come through right? */ memset(&ec, 0, sizeof(ec)); @@ -816,7 +829,7 @@ test_cfmt_extend_cells(void *arg) memcpy(p+1, "\x02\x14" "anarchoindividualist", 22); memcpy(p+23, "\x01\x12" "xxxxxxxxxxxxxxxxYY", 18); memcpy(p+41, "\xff\xff\x00\x20", 4); - tt_int_op(-1, OP_EQ, extend_cell_parse(&ec, RELAY_COMMAND_EXTEND2, + tt_int_op(0, OP_EQ, extend_cell_parse(&ec, RELAY_COMMAND_EXTEND2, p, sizeof(p))); /* Running out of space in specifiers */

1 0

[tor/master] relay: Refactor address and port checks
by nickm＠torproject.org 29 Apr '20

29 Apr '20

commit 07c008c6723644553827140f2f8a863a1e2d071a Author: teor <teor(a)torproject.org> Date: Wed Apr 15 08:58:32 2020 +1000 relay: Refactor address and port checks tor_addr_port_is_valid_ap(ap, 0) checks if the address or port are zero, exactly like the previous code. Preparation for 33817. --- src/feature/relay/circuitbuild_relay.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/feature/relay/circuitbuild_relay.c b/src/feature/relay/circuitbuild_relay.c index 2fa92eeac..96b46bb65 100644 --- a/src/feature/relay/circuitbuild_relay.c +++ b/src/feature/relay/circuitbuild_relay.c @@ -139,7 +139,7 @@ circuit_extend_lspec_valid_helper(const struct extend_cell_t *ec, return -1; } - if (!ec->orport_ipv4.port || tor_addr_is_null(&ec->orport_ipv4.addr)) { + if (!tor_addr_port_is_valid_ap(&ec->orport_ipv4, 0)) { log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, "Client asked me to extend to zero destination port or addr."); return -1;

1 0

[tor/master] relay: Refactor address checks into a function
by nickm＠torproject.org 29 Apr '20

29 Apr '20

commit ffc2fd001a306894bb8082d81de9b136937ef124 Author: teor <teor(a)torproject.org> Date: Wed Apr 15 09:13:16 2020 +1000 relay: Refactor address checks into a function No behaviour change. Part of 33817. --- src/feature/relay/circuitbuild_relay.c | 39 +++++++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/src/feature/relay/circuitbuild_relay.c b/src/feature/relay/circuitbuild_relay.c index 96b46bb65..dd38a2825 100644 --- a/src/feature/relay/circuitbuild_relay.c +++ b/src/feature/relay/circuitbuild_relay.c @@ -119,6 +119,33 @@ circuit_extend_add_ed25519_helper(struct extend_cell_t *ec) return 0; } +/* Check if the address and port in the tor_addr_port_t ap are valid, + * and are allowed by the current ExtendAllowPrivateAddresses config. + * + * If they are valid, return 0. + * Otherwise, if they are invalid, log a warning at log_level, + * and return -1. + */ +static int +circuit_extend_addr_port_helper(const struct tor_addr_port_t *ap, + int log_level) +{ + if (!tor_addr_port_is_valid_ap(ap, 0)) { + log_fn(log_level, LD_PROTOCOL, + "Client asked me to extend to zero destination port or addr."); + return -1; + } + + if (tor_addr_is_internal(&ap->addr, 0) && + !get_options()->ExtendAllowPrivateAddresses) { + log_fn(log_level, LD_PROTOCOL, + "Client asked me to extend to a private address."); + return -1; + } + + return 0; +} + /* Before replying to an extend cell, check the link specifiers in the extend * cell ec, which was received on the circuit circ. * @@ -139,16 +166,8 @@ circuit_extend_lspec_valid_helper(const struct extend_cell_t *ec, return -1; } - if (!tor_addr_port_is_valid_ap(&ec->orport_ipv4, 0)) { - log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, - "Client asked me to extend to zero destination port or addr."); - return -1; - } - - if (tor_addr_is_internal(&ec->orport_ipv4.addr, 0) && - !get_options()->ExtendAllowPrivateAddresses) { - log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, - "Client asked me to extend to a private address."); + if (circuit_extend_addr_port_helper(&ec->orport_ipv4, + LOG_PROTOCOL_WARN) < 0) { return -1; }

1 0

[tor/master] core/or: Check extends for zero addresses and ports
by nickm＠torproject.org 29 Apr '20

29 Apr '20

commit 587a7fbcf62447c7ade05c57220f1bfe2f3acdc1 Author: teor <teor(a)torproject.org> Date: Tue Apr 14 15:53:17 2020 +1000 core/or: Check extends for zero addresses and ports Check for invalid zero IPv4 addresses and ports, when sending and receiving extend cells. Fixes bug 33900; bugfix on 0.2.4.8-alpha. --- changes/bug33900 | 3 +++ src/core/or/onion.c | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/changes/bug33900 b/changes/bug33900 new file mode 100644 index 000000000..c1649d228 --- /dev/null +++ b/changes/bug33900 @@ -0,0 +1,3 @@ + o Minor bugfixes (IPv4, relay): + - Check for invalid zero IPv4 addresses and ports, when sending and + receiving extend cells. Fixes bug 33900; bugfix on 0.2.4.8-alpha. diff --git a/src/core/or/onion.c b/src/core/or/onion.c index 543d9f3e4..d73f981d2 100644 --- a/src/core/or/onion.c +++ b/src/core/or/onion.c @@ -244,14 +244,14 @@ check_extend_cell(const extend_cell_t *cell) if (tor_digest_is_zero((const char*)cell->node_id)) return -1; - if (tor_addr_family(&cell->orport_ipv4.addr) == AF_UNSPEC) { + if (!tor_addr_port_is_valid_ap(&cell->orport_ipv4, 0)) { /* EXTEND cells must have an IPv4 address. */ if (!is_extend2) { return -1; } /* EXTEND2 cells must have at least one IP address. * It can be IPv4 or IPv6. */ - if (tor_addr_family(&cell->orport_ipv6.addr) == AF_UNSPEC) { + if (!tor_addr_port_is_valid_ap(&cell->orport_ipv6, 0)) { return -1; } }

1 0

[tor/master] relay/circuitbuild: Consider IPv6-only extends valid
by nickm＠torproject.org 29 Apr '20

29 Apr '20

commit bad1181b5d1bef55e060d64a9d4ef9278619495b Author: teor <teor(a)torproject.org> Date: Wed Apr 15 11:34:12 2020 +1000 relay/circuitbuild: Consider IPv6-only extends valid Allow extend cells with IPv6-only link specifiers. Warn and fail if both IPv4 and IPv6 are invalid. Also warn if the IPv4 or IPv6 addresses are unexpectedly internal, but continue with the valid address. Part of 33817. --- src/feature/relay/circuitbuild_relay.c | 50 +++++++++---- src/test/test_circuitbuild.c | 125 ++++++++++++++++++++++++++++++++- 2 files changed, 161 insertions(+), 14 deletions(-) diff --git a/src/feature/relay/circuitbuild_relay.c b/src/feature/relay/circuitbuild_relay.c index 05146f1b6..080781f71 100644 --- a/src/feature/relay/circuitbuild_relay.c +++ b/src/feature/relay/circuitbuild_relay.c @@ -123,11 +123,14 @@ circuit_extend_add_ed25519_helper(struct extend_cell_t *ec) * and are allowed by the current ExtendAllowPrivateAddresses config. * * If they are valid, return 0. - * Otherwise, if they are invalid, log a warning at log_level, - * and return -1. + * Otherwise, if they are invalid, return -1. + * If log_zero_addrs is true, log warnings about zero addresses at + * log_level. If log_internal_addrs is true, log warnings about + * internal addresses at log_level. */ static int circuit_extend_addr_port_helper(const struct tor_addr_port_t *ap, + bool log_zero_addrs, bool log_internal_addrs, int log_level) { /* It's safe to print the family. But we don't want to print the address, @@ -135,19 +138,23 @@ circuit_extend_addr_port_helper(const struct tor_addr_port_t *ap, * But some internal addresses might be.)*/ if (!tor_addr_port_is_valid_ap(ap, 0)) { - log_fn(log_level, LD_PROTOCOL, - "Client asked me to extend to a zero destination port or " - "%s address '%s'.", - fmt_addr_family(&ap->addr), safe_str(fmt_addrport_ap(ap))); + if (log_zero_addrs) { + log_fn(log_level, LD_PROTOCOL, + "Client asked me to extend to a zero destination port or " + "%s address '%s'.", + fmt_addr_family(&ap->addr), safe_str(fmt_addrport_ap(ap))); + } return -1; } if (tor_addr_is_internal(&ap->addr, 0) && !get_options()->ExtendAllowPrivateAddresses) { - log_fn(log_level, LD_PROTOCOL, - "Client asked me to extend to a private %s address '%s'.", - fmt_addr_family(&ap->addr), - safe_str(fmt_and_decorate_addr(&ap->addr))); + if (log_internal_addrs) { + log_fn(log_level, LD_PROTOCOL, + "Client asked me to extend to a private %s address '%s'.", + fmt_addr_family(&ap->addr), + safe_str(fmt_and_decorate_addr(&ap->addr))); + } return -1; } @@ -174,9 +181,28 @@ circuit_extend_lspec_valid_helper(const struct extend_cell_t *ec, return -1; } - if (circuit_extend_addr_port_helper(&ec->orport_ipv4, - LOG_PROTOCOL_WARN) < 0) { + /* Check the addresses, without logging */ + const int ipv4_valid = + (circuit_extend_addr_port_helper(&ec->orport_ipv4, false, false, 0) == 0); + const int ipv6_valid = + (circuit_extend_addr_port_helper(&ec->orport_ipv6, false, false, 0) == 0); + /* We need at least one valid address */ + if (!ipv4_valid && !ipv6_valid) { + /* Now, log the invalid addresses at protocol warning level */ + circuit_extend_addr_port_helper(&ec->orport_ipv4, true, true, + LOG_PROTOCOL_WARN); + circuit_extend_addr_port_helper(&ec->orport_ipv6, true, true, + LOG_PROTOCOL_WARN); + /* And fail */ return -1; + } else if (!ipv4_valid) { + /* Always log unexpected internal addresses, but go on to use the other + * valid address */ + circuit_extend_addr_port_helper(&ec->orport_ipv4, false, true, + LOG_PROTOCOL_WARN); + } else if (!ipv6_valid) { + circuit_extend_addr_port_helper(&ec->orport_ipv6, false, true, + LOG_PROTOCOL_WARN); } IF_BUG_ONCE(circ->magic != OR_CIRCUIT_MAGIC) { diff --git a/src/test/test_circuitbuild.c b/src/test/test_circuitbuild.c index 932248084..a8d6323e4 100644 --- a/src/test/test_circuitbuild.c +++ b/src/test/test_circuitbuild.c @@ -474,6 +474,9 @@ mock_get_options(void) #define PUBLIC_IPV4 "1.2.3.4" #define INTERNAL_IPV4 "0.0.0.1" +#define PUBLIC_IPV6 "1234::cdef" +#define INTERNAL_IPV6 "::1" + #define VALID_PORT 0x1234 /* Test the different cases in circuit_extend_lspec_valid_helper(). */ @@ -519,7 +522,7 @@ test_circuit_extend_lspec_valid(void *arg) tor_end_capture_bugs_(); mock_clean_saved_logs(); - /* IPv4 addr or port are 0, these should fail */ + /* IPv4 and IPv6 addr and port are all zero, this should fail */ tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); expect_log_msg("Client asked me to extend to a zero destination port " "or unspecified address '[scrubbed]'.\n"); @@ -527,35 +530,103 @@ test_circuit_extend_lspec_valid(void *arg) /* Now ask for the actual address in the logs */ fake_options->SafeLogging_ = SAFELOG_SCRUB_NONE; + + /* IPv4 port is 0, IPv6 addr and port are both zero, this should fail */ tor_addr_parse(&ec->orport_ipv4.addr, PUBLIC_IPV4); tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); expect_log_msg("Client asked me to extend to a zero destination port " "or IPv4 address '1.2.3.4:0'.\n"); mock_clean_saved_logs(); - tor_addr_make_null(&ec->orport_ipv4.addr, AF_INET); + tor_addr_port_make_null_ap(&ec->orport_ipv4, AF_INET); + tor_addr_port_make_null_ap(&ec->orport_ipv6, AF_INET6); + /* IPv4 addr is 0, IPv6 addr and port are both zero, this should fail */ ec->orport_ipv4.port = VALID_PORT; tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); expect_log_msg("Client asked me to extend to a zero destination port " "or IPv4 address '0.0.0.0:4660'.\n"); mock_clean_saved_logs(); ec->orport_ipv4.port = 0; + tor_addr_port_make_null_ap(&ec->orport_ipv4, AF_INET); + tor_addr_port_make_null_ap(&ec->orport_ipv6, AF_INET6); /* IPv4 addr is internal, and port is valid. + * (IPv6 addr and port are both zero.) + * Result depends on ExtendAllowPrivateAddresses. */ + tor_addr_parse(&ec->orport_ipv4.addr, INTERNAL_IPV4); + ec->orport_ipv4.port = VALID_PORT; + + fake_options->ExtendAllowPrivateAddresses = 0; + tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); + expect_log_msg("Client asked me to extend " + "to a private IPv4 address '0.0.0.1'.\n"); + mock_clean_saved_logs(); + fake_options->ExtendAllowPrivateAddresses = 0; + tor_addr_port_make_null_ap(&ec->orport_ipv4, AF_INET); + tor_addr_port_make_null_ap(&ec->orport_ipv6, AF_INET6); + + /* Now do the same tests, but for IPv6 */ + + /* IPv6 port is 0, IPv4 addr and port are both zero, this should fail */ + tor_addr_parse(&ec->orport_ipv6.addr, PUBLIC_IPV6); + tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); + expect_log_msg("Client asked me to extend to a zero destination port " + "or IPv6 address '[1234::cdef]:0'.\n"); + mock_clean_saved_logs(); + tor_addr_port_make_null_ap(&ec->orport_ipv4, AF_INET); + tor_addr_port_make_null_ap(&ec->orport_ipv6, AF_INET6); + + /* IPv6 addr is 0, IPv4 addr and port are both zero, this should fail */ + ec->orport_ipv6.port = VALID_PORT; + tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); + expect_log_msg("Client asked me to extend to a zero destination port " + "or IPv6 address '[::]:4660'.\n"); + mock_clean_saved_logs(); + ec->orport_ipv4.port = 0; + tor_addr_port_make_null_ap(&ec->orport_ipv4, AF_INET); + tor_addr_port_make_null_ap(&ec->orport_ipv6, AF_INET6); + + /* IPv6 addr is internal, and port is valid. + * (IPv4 addr and port are both zero.) + * Result depends on ExtendAllowPrivateAddresses. */ + tor_addr_parse(&ec->orport_ipv6.addr, INTERNAL_IPV6); + ec->orport_ipv6.port = VALID_PORT; + + fake_options->ExtendAllowPrivateAddresses = 0; + tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); + expect_log_msg("Client asked me to extend " + "to a private IPv6 address '[::1]'.\n"); + mock_clean_saved_logs(); + fake_options->ExtendAllowPrivateAddresses = 0; + tor_addr_port_make_null_ap(&ec->orport_ipv4, AF_INET); + tor_addr_port_make_null_ap(&ec->orport_ipv6, AF_INET6); + + /* Both addresses are internal. * Result depends on ExtendAllowPrivateAddresses. */ tor_addr_parse(&ec->orport_ipv4.addr, INTERNAL_IPV4); ec->orport_ipv4.port = VALID_PORT; + tor_addr_parse(&ec->orport_ipv6.addr, INTERNAL_IPV6); + ec->orport_ipv6.port = VALID_PORT; fake_options->ExtendAllowPrivateAddresses = 0; tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); expect_log_msg("Client asked me to extend " "to a private IPv4 address '0.0.0.1'.\n"); + expect_log_msg("Client asked me to extend " + "to a private IPv6 address '[::1]'.\n"); mock_clean_saved_logs(); fake_options->ExtendAllowPrivateAddresses = 0; + tor_addr_port_make_null_ap(&ec->orport_ipv4, AF_INET); + tor_addr_port_make_null_ap(&ec->orport_ipv6, AF_INET6); /* If we pass the private address check, but don't have the right * OR circuit magic number, we trigger another bug */ + tor_addr_parse(&ec->orport_ipv4.addr, INTERNAL_IPV4); + ec->orport_ipv4.port = VALID_PORT; + tor_addr_parse(&ec->orport_ipv6.addr, INTERNAL_IPV6); + ec->orport_ipv6.port = VALID_PORT; fake_options->ExtendAllowPrivateAddresses = 1; + tor_capture_bugs_(1); tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); tt_int_op(smartlist_len(tor_get_captured_bug_log_()), OP_EQ, 1); @@ -564,6 +635,21 @@ test_circuit_extend_lspec_valid(void *arg) tor_end_capture_bugs_(); mock_clean_saved_logs(); fake_options->ExtendAllowPrivateAddresses = 0; + tor_addr_port_make_null_ap(&ec->orport_ipv4, AF_INET); + tor_addr_port_make_null_ap(&ec->orport_ipv6, AF_INET6); + + /* Fail again, but this time only set an IPv4 address. */ + tor_addr_parse(&ec->orport_ipv4.addr, INTERNAL_IPV4); + ec->orport_ipv4.port = VALID_PORT; + fake_options->ExtendAllowPrivateAddresses = 1; + tor_capture_bugs_(1); + tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); + /* Since we're using IF_BUG_ONCE(), expect 0-1 bug logs */ + tt_int_op(smartlist_len(tor_get_captured_bug_log_()), OP_GE, 0); + tt_int_op(smartlist_len(tor_get_captured_bug_log_()), OP_LE, 1); + tor_end_capture_bugs_(); + mock_clean_saved_logs(); + fake_options->ExtendAllowPrivateAddresses = 0; /* Now set the right magic */ or_circ->base_.magic = OR_CIRCUIT_MAGIC; @@ -625,6 +711,41 @@ test_circuit_extend_lspec_valid(void *arg) tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, 0); mock_clean_saved_logs(); + /* Now let's check that we warn, but succeed, when only one address is + * private */ + tor_addr_parse(&ec->orport_ipv4.addr, INTERNAL_IPV4); + ec->orport_ipv4.port = VALID_PORT; + tor_addr_parse(&ec->orport_ipv6.addr, PUBLIC_IPV6); + ec->orport_ipv6.port = VALID_PORT; + fake_options->ExtendAllowPrivateAddresses = 0; + + tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, 0); + expect_log_msg("Client asked me to extend " + "to a private IPv4 address '0.0.0.1'.\n"); + mock_clean_saved_logs(); + tor_addr_port_make_null_ap(&ec->orport_ipv4, AF_INET); + tor_addr_port_make_null_ap(&ec->orport_ipv6, AF_INET6); + + /* Now with private IPv6 */ + tor_addr_parse(&ec->orport_ipv4.addr, PUBLIC_IPV4); + ec->orport_ipv4.port = VALID_PORT; + tor_addr_parse(&ec->orport_ipv6.addr, INTERNAL_IPV6); + ec->orport_ipv6.port = VALID_PORT; + fake_options->ExtendAllowPrivateAddresses = 0; + + tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, 0); + expect_log_msg("Client asked me to extend " + "to a private IPv6 address '[::1]'.\n"); + mock_clean_saved_logs(); + tor_addr_port_make_null_ap(&ec->orport_ipv4, AF_INET); + tor_addr_port_make_null_ap(&ec->orport_ipv6, AF_INET6); + + /* Now reset to public IPv4 and IPv6 */ + tor_addr_parse(&ec->orport_ipv4.addr, PUBLIC_IPV4); + ec->orport_ipv4.port = VALID_PORT; + tor_addr_parse(&ec->orport_ipv6.addr, PUBLIC_IPV6); + ec->orport_ipv6.port = VALID_PORT; + /* Fail on matching non-zero identities */ memset(&ec->ed_pubkey, 0xEE, sizeof(ec->ed_pubkey)); memset(&p_chan->ed25519_identity, 0xEE, sizeof(p_chan->ed25519_identity));

1 0

[tor/master] test/circuitbuild: Show bad addresses in some logs
by nickm＠torproject.org 29 Apr '20

29 Apr '20

commit 7cef02ec1fa76f593dfb9bd53f76922225696934 Author: teor <teor(a)torproject.org> Date: Wed Apr 15 10:07:26 2020 +1000 test/circuitbuild: Show bad addresses in some logs Disable SafeLogging for some extend tests, so we can check the actual addresses. Part of 33817. --- src/test/test_circuitbuild.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/test/test_circuitbuild.c b/src/test/test_circuitbuild.c index ab5c9c993..932248084 100644 --- a/src/test/test_circuitbuild.c +++ b/src/test/test_circuitbuild.c @@ -525,17 +525,19 @@ test_circuit_extend_lspec_valid(void *arg) "or unspecified address '[scrubbed]'.\n"); mock_clean_saved_logs(); + /* Now ask for the actual address in the logs */ + fake_options->SafeLogging_ = SAFELOG_SCRUB_NONE; tor_addr_parse(&ec->orport_ipv4.addr, PUBLIC_IPV4); tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); expect_log_msg("Client asked me to extend to a zero destination port " - "or IPv4 address '[scrubbed]'.\n"); + "or IPv4 address '1.2.3.4:0'.\n"); mock_clean_saved_logs(); tor_addr_make_null(&ec->orport_ipv4.addr, AF_INET); ec->orport_ipv4.port = VALID_PORT; tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); expect_log_msg("Client asked me to extend to a zero destination port " - "or IPv4 address '[scrubbed]'.\n"); + "or IPv4 address '0.0.0.0:4660'.\n"); mock_clean_saved_logs(); ec->orport_ipv4.port = 0; @@ -547,7 +549,7 @@ test_circuit_extend_lspec_valid(void *arg) fake_options->ExtendAllowPrivateAddresses = 0; tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); expect_log_msg("Client asked me to extend " - "to a private IPv4 address '[scrubbed]'.\n"); + "to a private IPv4 address '0.0.0.1'.\n"); mock_clean_saved_logs(); fake_options->ExtendAllowPrivateAddresses = 0;

1 0

[tor/master] net: Remove an extra space in address.h
by nickm＠torproject.org 29 Apr '20

29 Apr '20

commit e9d04b05c6ba3930d55dc6c7e512e082ff67ebfb Author: teor <teor(a)torproject.org> Date: Wed Apr 15 09:52:25 2020 +1000 net: Remove an extra space in address.h --- src/lib/net/address.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/net/address.h b/src/lib/net/address.h index f1c223310..186f1fe86 100644 --- a/src/lib/net/address.h +++ b/src/lib/net/address.h @@ -221,7 +221,7 @@ char *tor_addr_to_str_dup(const tor_addr_t *addr) ATTR_MALLOC; const char *fmt_addr_impl(const tor_addr_t *addr, int decorate); const char *fmt_addrport(const tor_addr_t *addr, uint16_t port); -const char * fmt_addr32(uint32_t addr); +const char *fmt_addr32(uint32_t addr); MOCK_DECL(int,get_interface_address6,(int severity, sa_family_t family, tor_addr_t *addr));

1 0

[tor/master] relay: Log the address in circuit protocol warnings
by nickm＠torproject.org 29 Apr '20

29 Apr '20

commit 44f71e08c414f6c7aad6304e24be90d5d320c95b Author: teor <teor(a)torproject.org> Date: Wed Apr 15 09:55:09 2020 +1000 relay: Log the address in circuit protocol warnings Always log the address family in extend protocol warnings. If SafeLogging is 0, also log the address and port. Diagnostics for 33817. --- src/feature/relay/circuitbuild_relay.c | 12 ++++++++++-- src/test/test_circuitbuild.c | 19 ++++++++++--------- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/src/feature/relay/circuitbuild_relay.c b/src/feature/relay/circuitbuild_relay.c index dd38a2825..05146f1b6 100644 --- a/src/feature/relay/circuitbuild_relay.c +++ b/src/feature/relay/circuitbuild_relay.c @@ -130,16 +130,24 @@ static int circuit_extend_addr_port_helper(const struct tor_addr_port_t *ap, int log_level) { + /* It's safe to print the family. But we don't want to print the address, + * unless specifically configured to do so. (Zero addresses aren't sensitive, + * But some internal addresses might be.)*/ + if (!tor_addr_port_is_valid_ap(ap, 0)) { log_fn(log_level, LD_PROTOCOL, - "Client asked me to extend to zero destination port or addr."); + "Client asked me to extend to a zero destination port or " + "%s address '%s'.", + fmt_addr_family(&ap->addr), safe_str(fmt_addrport_ap(ap))); return -1; } if (tor_addr_is_internal(&ap->addr, 0) && !get_options()->ExtendAllowPrivateAddresses) { log_fn(log_level, LD_PROTOCOL, - "Client asked me to extend to a private address."); + "Client asked me to extend to a private %s address '%s'.", + fmt_addr_family(&ap->addr), + safe_str(fmt_and_decorate_addr(&ap->addr))); return -1; } diff --git a/src/test/test_circuitbuild.c b/src/test/test_circuitbuild.c index 061f39937..ab5c9c993 100644 --- a/src/test/test_circuitbuild.c +++ b/src/test/test_circuitbuild.c @@ -521,21 +521,21 @@ test_circuit_extend_lspec_valid(void *arg) /* IPv4 addr or port are 0, these should fail */ tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); - expect_log_msg("Client asked me to extend to " - "zero destination port or addr.\n"); + expect_log_msg("Client asked me to extend to a zero destination port " + "or unspecified address '[scrubbed]'.\n"); mock_clean_saved_logs(); tor_addr_parse(&ec->orport_ipv4.addr, PUBLIC_IPV4); tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); - expect_log_msg("Client asked me to extend to " - "zero destination port or addr.\n"); + expect_log_msg("Client asked me to extend to a zero destination port " + "or IPv4 address '[scrubbed]'.\n"); mock_clean_saved_logs(); tor_addr_make_null(&ec->orport_ipv4.addr, AF_INET); ec->orport_ipv4.port = VALID_PORT; tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); - expect_log_msg("Client asked me to extend to " - "zero destination port or addr.\n"); + expect_log_msg("Client asked me to extend to a zero destination port " + "or IPv4 address '[scrubbed]'.\n"); mock_clean_saved_logs(); ec->orport_ipv4.port = 0; @@ -546,7 +546,8 @@ test_circuit_extend_lspec_valid(void *arg) fake_options->ExtendAllowPrivateAddresses = 0; tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); - expect_log_msg("Client asked me to extend to a private address.\n"); + expect_log_msg("Client asked me to extend " + "to a private IPv4 address '[scrubbed]'.\n"); mock_clean_saved_logs(); fake_options->ExtendAllowPrivateAddresses = 0; @@ -1002,8 +1003,8 @@ test_circuit_extend(void *arg) tt_int_op(circuit_extend(cell, circ), OP_EQ, -1); tt_int_op(mock_extend_cell_parse_calls, OP_EQ, 1); - expect_log_msg("Client asked me to extend to " - "zero destination port or addr.\n"); + expect_log_msg("Client asked me to extend to a zero destination port " + "or unspecified address '[scrubbed]'.\n"); mock_clean_saved_logs(); mock_extend_cell_parse_calls = 0;

1 0

[tor/master] net: Add fmt_addrport_ap() and fmt_addr_family()
by nickm＠torproject.org 29 Apr '20

29 Apr '20

commit a72e017e7f69581ceb005d05ce8033a6fd05626e Author: teor <teor(a)torproject.org> Date: Wed Apr 15 09:52:57 2020 +1000 net: Add fmt_addrport_ap() and fmt_addr_family() Add fmt_addrport_ap(), a macro that takes a tor_addr_port_t, and uses it to call fmt_addrport(). Add fmt_addr_family(), a function that returns a string constant describing the address family. Utility functions for 33817. --- src/lib/net/address.c | 33 +++++++++++++++++++++++++++++++++ src/lib/net/address.h | 2 ++ 2 files changed, 35 insertions(+) diff --git a/src/lib/net/address.c b/src/lib/net/address.c index 5fe9abca1..9a90093ae 100644 --- a/src/lib/net/address.c +++ b/src/lib/net/address.c @@ -1206,6 +1206,39 @@ fmt_addr32(uint32_t addr) return buf; } +/** Return a string representing the family of addr. + * + * This string is a string constant, and must not be freed. + * This function is thread-safe. + */ +const char * +fmt_addr_family(const tor_addr_t *addr) +{ + static int default_bug_once = 0; + + IF_BUG_ONCE(!addr) + return "NULL pointer"; + + switch (tor_addr_family(addr)) { + case AF_INET6: + return "IPv6"; + case AF_INET: + return "IPv4"; + case AF_UNIX: + return "UNIX socket"; + case AF_UNSPEC: + return "unspecified"; + default: + if (!default_bug_once) { + log_warn(LD_BUG, "Called with unknown address family %d", + (int)tor_addr_family(addr)); + default_bug_once = 1; + } + return "unknown"; + } + //return "(unreachable code)"; +} + /** Convert the string in src to a tor_addr_t addr. The string * may be an IPv4 address, or an IPv6 address surrounded by square brackets. * diff --git a/src/lib/net/address.h b/src/lib/net/address.h index 186f1fe86..611d1ca1e 100644 --- a/src/lib/net/address.h +++ b/src/lib/net/address.h @@ -221,7 +221,9 @@ char *tor_addr_to_str_dup(const tor_addr_t *addr) ATTR_MALLOC; const char *fmt_addr_impl(const tor_addr_t *addr, int decorate); const char *fmt_addrport(const tor_addr_t *addr, uint16_t port); +#define fmt_addrport_ap(ap) fmt_addrport(&(ap)->addr, (ap)->port) const char *fmt_addr32(uint32_t addr); +const char *fmt_addr_family(const tor_addr_t *addr); MOCK_DECL(int,get_interface_address6,(int severity, sa_family_t family, tor_addr_t *addr));

1 0