Opened 5 months ago

Closed 4 months ago

#26932 closed defect (fixed)

Soft assert in HS3 with vanguards ([warn] Invalid signature for service descriptor signing key: expired)

Reported by: asn Owned by:
Priority: Medium Milestone: Tor: 0.3.5.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-hs possible-regression bug easy 034-backport 033-backport-maybe 032-backport-maybe
Cc: mikeperry Actual Points:
Parent ID: Points:
Reviewer: teor Sponsor:

Description

I recently got the following soft assert on my HSv3 with latest master:

Jul 22 09:05:41.000 [warn] Invalid signature for service descriptor signing key: expired
Jul 22 09:05:41.000 [warn] tor_bug_occurred_(): Bug: src/feature/hs/hs_descriptor.c:2367: hs_desc_encode_descriptor: Non-fatal assertion !(ret < 0) failed. (on Tor 0.3.5.0-alpha-dev e2b744ce38edb890)
Jul 22 09:05:41.000 [warn] Bug: Non-fatal assertion !(ret < 0) failed in hs_desc_encode_descriptor at src/feature/hs/hs_descriptor.c:2367. Stack trace: (on Tor 0.3.5.0-alpha-dev e2b744ce38edb890)
Jul 22 09:05:41.000 [warn] Bug:     ./tor/src/app/tor(log_backtrace_impl+0x47) [0x7f9979f5b437] (on Tor 0.3.5.0-alpha-dev e2b744ce38edb890)
Jul 22 09:05:41.000 [warn] Bug:     ./tor/src/app/tor(tor_bug_occurred_+0xbe) [0x7f9979f56d0e] (on Tor 0.3.5.0-alpha-dev e2b744ce38edb890)
Jul 22 09:05:41.000 [warn] Bug:     ./tor/src/app/tor(hs_desc_encode_descriptor+0xb2) [0x7f9979e5dba2] (on Tor 0.3.5.0-alpha-dev e2b744ce38edb890)
Jul 22 09:05:41.000 [warn] Bug:     ./tor/src/app/tor(hs_service_run_scheduled_events+0x1a75) [0x7f9979e64ac5] (on Tor 0.3.5.0-alpha-dev e2b744ce38edb890)
Jul 22 09:05:41.000 [warn] Bug:     ./tor/src/app/tor(+0x51fb1) [0x7f9979dc4fb1] (on Tor 0.3.5.0-alpha-dev e2b744ce38edb890)
Jul 22 09:05:41.000 [warn] Bug:     ./tor/src/app/tor(+0x593b1) [0x7f9979dcc3b1] (on Tor 0.3.5.0-alpha-dev e2b744ce38edb890)
Jul 22 09:05:41.000 [warn] Bug:     /usr/lib/x86_64-linux-gnu/libevent-2.0.so.5(event_base_loop+0x7fc) [0x7f99793f63dc] (on Tor 0.3.5.0-alpha-dev e2b744ce38edb890)
Jul 22 09:05:41.000 [warn] Bug:     ./tor/src/app/tor(do_main_loop+0x203) [0x7f9979dc8e13] (on Tor 0.3.5.0-alpha-dev e2b744ce38edb890)
Jul 22 09:05:41.000 [warn] Bug:     ./tor/src/app/tor(tor_run_main+0x2a5) [0x7f9979dca325] (on Tor 0.3.5.0-alpha-dev e2b744ce38edb890)
Jul 22 09:05:41.000 [warn] Bug:     ./tor/src/app/tor(tor_main+0x3a) [0x7f9979dc364a] (on Tor 0.3.5.0-alpha-dev e2b744ce38edb890)
Jul 22 09:05:41.000 [warn] Bug:     ./tor/src/app/tor(main+0x19) [0x7f9979dc33b9] (on Tor 0.3.5.0-alpha-dev e2b744ce38edb890)
Jul 22 09:05:41.000 [warn] Bug:     /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf1) [0x7f99785e92b1] (on Tor 0.3.5.0-alpha-dev e2b744ce38edb890)
Jul 22 09:05:41.000 [warn] Bug:     ./tor/src/app/tor(_start+0x2a) [0x7f9979dc340a] (on Tor 0.3.5.0-alpha-dev e2b744ce38edb890)

I'm suspecting it might have to do with the #25552 fix because the code changed there is relevant to this bug, altho I don't see how it can cause this bug.

Based on the logs, it seems like my HSv3 kept that descriptor around for about 56 hours when the descriptor signing key cert lifetime is 54 hours (see HS_DESC_CERT_LIFETIME).

Child Tickets

Change History (9)

comment:1 Changed 5 months ago by asn

Cc: mikeperry added
Keywords: easy added
Priority: HighMedium
Summary: Soft assert with [warn] Invalid signature for service descriptor signing key: expiredSoft assert in HS3 with vanguards ([warn] Invalid signature for service descriptor signing key: expired)

I further debugged this. It seems to be related to my use of the vanguards script, and not related to #25552 fortunately.

Basically, it seems like service->state.next_rotation_time was getting reset very often, because the vanguards script was HUPing Tor and hence keep calling move_hs_state() which does not update next_rotation_time. This was causing the HS descriptors to stick around for ever, instead of getting rotated.

I think the fix here is to patch move_hs_state() to update next_rotation_time.

Last edited 5 months ago by asn (previous) (diff)

comment:2 Changed 5 months ago by mikeperry

Milestone: Tor: 0.3.5.x-finalTor: 0.3.4.x-final

I also got this on my service side on 0.3.4.4-rc. I can try a patch. Should move_hs_state() just call set_rotation_time(dest_service, time(NULL))? Or is there more to it?

Also we want this to be for at least 0.3.4.x. Maybe also 0.3.3.x?

comment:3 Changed 5 months ago by asn

IMO, the patch is to add dst->next_rotation_time = src->next_rotation_time; to move_hs_state().

comment:4 Changed 5 months ago by teor

Keywords: 034-backport 033-backport-maybe 032-backport-maybe added
Milestone: Tor: 0.3.4.x-finalTor: 0.3.5.x-final

If it's a simple patch, let's consider backporting it to 0.3.2 and later.

comment:5 Changed 5 months ago by asn

Yep, it's a simple patch.

I've been considering whether it's possible changing the logic of move_hs_state() so that instead of doing the shallow copy by naively copying elements around, we just do a memcpy() between the two structs. Might be more future-proof to avoid similar bugs in the future.

comment:6 Changed 5 months ago by asn

Status: newneeds_review

Please see bug26932 in my repo for the naive fix here:
https://github.com/torproject/tor/pull/251

comment:7 Changed 5 months ago by asn

Reviewer: teor

comment:8 Changed 4 months ago by teor

Status: needs_reviewmerge_ready

Seems like a simple fix, I think we should probably backport it.

comment:9 Changed 4 months ago by nickm

Resolution: fixed
Status: merge_readyclosed

Cherry-picked to 0.3.2 as c798957b5925b6; merged forward.

Note: See TracTickets for help on using tickets.