Opened 6 weeks ago

Closed 3 weeks ago

#27995 closed defect (fixed)

hs v3 auth descriptor cookie validation: tor crash when parsing .auth file after SIGHUP

Reported by: madage Owned by: dgoulet
Priority: High Milestone: Tor: 0.3.5.x-final
Component: Core Tor/Tor Version: Tor: 0.3.5.2-alpha
Severity: Normal Keywords: hs onion service v3 descriptor cookie validation regression high
Cc: Actual Points:
Parent ID: Points:
Reviewer: asn Sponsor:

Description (last modified by arma)

Hello devs,

While running tor onion service v3 with client auth disabled, if a new client .auth file is put under the authorized_clients subdir and a SIGHUP is sent to tor, the main process crashes after a bad assertion.

######

Oct 10 16:29:42.000 [info] load_client_keys(): Loading a client authorization key file a.auth...
Oct 10 16:29:42.000 [info] load_client_keys(): Loaded a client authorization key file a.auth.
Oct 10 16:29:42.000 [err] tor_assertion_failed_(): Bug: ../tor-0.3.5.2-alpha/src/feature/hs/hs_descriptor.c:2883: hs_desc_build_authorized_client: Assertion !tor_mem_is_zero((char *) descriptor_cookie, HS_DESC_DESCRIPTOR_COOKIE_LEN) failed; aborting. (on Tor 0.3.5.2-alpha )
Oct 10 16:29:42.000 [err] Bug: Assertion !tor_mem_is_zero((char *) descriptor_cookie, HS_DESC_DESCRIPTOR_COOKIE_LEN) failed in hs_desc_build_authorized_client at ../tor-0.3.5.2-alpha/src/feature/hs/hs_descriptor.c:2883. Stack trace: (on Tor 0.3.5.2-alpha )
Oct 10 16:29:42.000 [err] Bug: tor(log_backtrace_impl+0x5a) [0x781307] (on Tor 0.3.5.2-alpha )
Oct 10 16:29:42.000 [err] Bug: tor(tor_assertion_failed_+0x105) [0x77bc33] (on Tor 0.3.5.2-alpha )
Oct 10 16:29:42.000 [err] Bug: tor(hs_desc_build_authorized_client+0x255) [0x58cfa8] (on Tor 0.3.5.2-alpha )
Oct 10 16:29:42.000 [err] Bug: tor(+0x1216c9) [0x5936c9] (on Tor 0.3.5.2-alpha )
Oct 10 16:29:42.000 [err] Bug: tor(+0x1207f5) [0x5927f5] (on Tor 0.3.5.2-alpha )
Oct 10 16:29:42.000 [err] Bug: tor(+0x11ed28) [0x590d28] (on Tor 0.3.5.2-alpha )
Oct 10 16:29:42.000 [err] Bug: tor(hs_service_load_all_keys+0xdc) [0x598fb8] (on Tor 0.3.5.2-alpha )
Oct 10 16:29:42.000 [err] Bug: tor(+0x1bf5ea) [0x6315ea] (on Tor 0.3.5.2-alpha )
Oct 10 16:29:42.000 [err] Bug: tor(set_options+0xb0) [0x62e9f5] (on Tor 0.3.5.2-alpha )
Oct 10 16:29:42.000 [err] Bug: tor(options_init_from_string+0x63d) [0x63b6d6] (on Tor 0.3.5.2-alpha )
Oct 10 16:29:42.000 [err] Bug: tor(options_init_from_torrc+0x4f8) [0x63ad2c] (on Tor 0.3.5.2-alpha )
Oct 10 16:29:42.000 [err] Bug: tor(+0x43a82) [0x4b5a82] (on Tor 0.3.5.2-alpha )
Oct 10 16:29:42.000 [err] Bug: tor(+0x44666) [0x4b6666] (on Tor 0.3.5.2-alpha )
Oct 10 16:29:42.000 [err] Bug: tor(+0x4443a) [0x4b643a] (on Tor 0.3.5.2-alpha )
Oct 10 16:29:42.000 [err] Bug: /usr/lib/i386-linux-gnu/libevent-2.1.so.6(+0x209db) [0xb7dfa9db] (on Tor 0.3.5.2-alpha )
Oct 10 16:29:42.000 [err] Bug: /usr/lib/i386-linux-gnu/libevent-2.1.so.6(event_base_loop+0x4d1) [0xb7dfb3b1] (on Tor 0.3.5.2-alpha )
Oct 10 16:29:42.000 [err] Bug: tor(tor_libevent_run_event_loop+0x4b) [0x67047e] (on Tor 0.3.5.2-alpha )
Oct 10 16:29:42.000 [err] Bug: tor(+0x4418c) [0x4b618c] (on Tor 0.3.5.2-alpha )
Oct 10 16:29:42.000 [err] Bug: tor(+0x443a2) [0x4b63a2] (on Tor 0.3.5.2-alpha )
Oct 10 16:29:42.000 [err] Bug: tor(do_main_loop+0x372) [0x4b60e6] (on Tor 0.3.5.2-alpha )
Oct 10 16:29:42.000 [err] Bug: tor(tor_run_main+0x256) [0x4bb6ac] (on Tor 0.3.5.2-alpha )
Oct 10 16:29:42.000 [err] Bug: tor(tor_main+0x8a) [0x4b00d5] (on Tor 0.3.5.2-alpha )
Oct 10 16:29:42.000 [err] Bug: tor(main+0x46) [0x4afc8f] (on Tor 0.3.5.2-alpha )
Oct 10 16:29:42.000 [err] Bug: /lib/i386-linux-gnu/libc.so.6(libc_start_main+0xf1) [0xb791c9a1] (on Tor 0.3.5.2-alpha )
Oct 10 16:29:42.000 [err] Bug: tor(+0x3db41) [0x4afb41] (on Tor 0.3.5.2-alpha )

######

This crash happened while running a non-optimized tor version and this is the backtrace from gdb:

#0  0xb7f75b91 in __kernel_vsyscall ()
#1  0xb7931112 in __libc_signal_restore_set (set=0xbfe7a9fc) at ../sysdeps/unix/sysv/linux/nptl-signals.h:80
#2  __GI_raise (sig=6) at ../sysdeps/unix/sysv/linux/raise.c:48
#3  0xb7932531 in __GI_abort () at abort.c:79
#4  0x0058cfad in hs_desc_build_authorized_client ()
#5  0x005936c9 in build_service_desc_superencrypted ()
#6  0x005927f5 in move_descriptors ()
#7  0x00590d28 in register_all_services ()
#8  0x00598fb8 in hs_service_load_all_keys ()
#9  0x006315ea in options_act ()
#10 0x0062e9f5 in set_options ()
#11 0x0063b6d6 in options_init_from_string ()
#12 0x0063ad2c in options_init_from_torrc ()
#13 0x004b5a82 in do_hup ()
#14 0x004b6666 in process_signal ()
#15 0x004b643a in signal_callback ()
#16 0xb7dfa9db in ?? () from /usr/lib/i386-linux-gnu/libevent-2.1.so.6
#17 0xb7dfb3b1 in event_base_loop () from /usr/lib/i386-linux-gnu/libevent-2.1.so.6
#18 0x0067047e in tor_libevent_run_event_loop ()
#19 0x004b618c in run_main_loop_once ()
#20 0x004b63a2 in run_main_loop_until_done ()
#21 0x004b60e6 in do_main_loop ()
#22 0x004bb6ac in tor_run_main ()
#23 0x004b00d5 in tor_main ()
#24 0x004afc8f in main ()

######

If the process is restarted, there is no problem setting up the descriptor cookie.

I've coded a dirty patch that solves this problem:

On tor 0.3.5.2-alpha/src/feature/hs/hs_service.c

1764a1765,1771
>     /* Test that descriptor_cookie is not zero because we will use it
>      * bellow */
>     if (tor_mem_is_zero((char*)desc->descriptor_cookie,
>                                   HS_DESC_DESCRIPTOR_COOKIE_LEN)) {
>               crypto_strongest_rand(desc->descriptor_cookie,
>                               sizeof(desc->descriptor_cookie));
>     }

I don't know if this is the best course of action or if it would be wiser to check it elsewhere.

TODO: elaborate a unit test.

Child Tickets

Change History (7)

comment:1 Changed 6 weeks ago by arma

Description: modified (diff)
Milestone: Tor: 0.3.5.x-final

comment:2 Changed 6 weeks ago by nickm

Keywords: regression high added
Priority: MediumHigh
Status: newneeds_review

comment:3 Changed 5 weeks ago by dgoulet

Reviewer: dgoulet

comment:4 Changed 5 weeks ago by dgoulet

Owner: set to dgoulet
Reviewer: dgoulet
Status: needs_reviewaccepted

Hmmm this is reproducible as explained in the ticket...

The reason is that the service descriptor_cookie is created when we generate the service keys in build_service_desc_keys() meaning that on HUP, that does NOT get called again ending up with an empty descriptor cookie but with authorized clients.

To be honest, in order to minimize complexity, we should probably _always_ generate the cookie and only use it when we have authorized client enabled. That way, we don't have to worry about configuration changes and that cookie value. Would be a one liner fix.

comment:5 Changed 5 weeks ago by dgoulet

Reviewer: asn
Status: acceptedneeds_review

Branch: ticket27995_035_01.
PR: https://github.com/torproject/tor/pull/415

comment:6 Changed 5 weeks ago by asn

Status: needs_reviewmerge_ready

LGTM!

comment:7 Changed 3 weeks ago by dgoulet

Resolution: fixed
Status: merge_readyclosed

Merged forward!

Note: See TracTickets for help on using tickets.