Handle the case when we can't generate descriptors

Relays that fail to generate a server descriptor or extra-info descriptor also fail to generate a server descriptor. They just print out a warning and retry a couple of milliseconds later.

rransom wrote a patch in his mitigate2183 branch that at least generates a server descriptor in case we cannot generate an extra-info descriptor.

Sebastian mentioned that failing may also be an option if we can't generate an extra-info descriptor, or we'll never learn about this bug.

We should decide what the best behavior in this case is and write a patch.

changed milestone to %Tor: 0.2.2.x-final

added component::core tor/tor milestone::Tor: 0.2.2.x-final priority::medium resolution::fixed status::closed tor-relay type::defect version::tor 0.2.2.17-alpha labels

Trac:
Milestone: Tor: 0.2.3.x-final to Tor: 0.2.2.x-final
Cc: N/A to rransom
Status: new to needs_review

Reviewing mitigate2183 :

• You should use tor_digest_is_zero() when setting has_extra_info_digest.

Other than that, looks okay to me. Is it tested? It should be trivial enough to insert a "return -1" to the extrainfo_dump_to_string function to make sure the the routerinfo gets generated okay.

See also my comments and rransom's reply in https://trac.torproject.org/projects/tor/ticket/2183#comment:18 .

Also, did we decide that not generating an extra-info descriptor is the desired behavior? See Sebastian's idea in the ticket description. I don't feel strongly here, but want to avoid that this gets lost.

Replying to karsten:

Also, did we decide that not generating an extra-info descriptor is the desired behavior? See Sebastian's idea in the ticket description. I don't feel strongly here, but want to avoid that this gets lost.

Crashing a relay (thereby ending all circuits passing through it, losing its Stable and Guard flags, etc.) merely to draw attention to the fact that it could not generate an extra-info descriptor is a bad idea. Uploading a router descriptor without an extra-info-digest line will still be quite noticeable, and will be far less disruptive to the network.

Agreed that crashing is not right.

Replying to nickm:

Reviewing mitigate2183 :

• You should use tor_digest_is_zero() when setting has_extra_info_digest.

Thanks! I didn't know there was a utility function for that.

Other than that, looks okay to me. Is it tested? It should be trivial enough to insert a "return -1" to the extrainfo_dump_to_string function to make sure the the routerinfo gets generated okay.

It's working for me now as a bridge (sanitized fingerprint 07DC 31AB 1422 1482 77C7 3BB3 7C2C 5A3B 7FF5 DBC9).

My revised branch is pushed as fix2195-v2 ( !git://repo.or.cz/tor/rransom.git fix2195-v2 ).

+  has_extra_info_digest =
+    tor_digest_is_zero(router->cache_info.extra_info_digest);

Shouldn't there be a ! in there?

Replying to nickm:

{{{

• has_extra_info_digest =
• tor_digest_is_zero(router->cache_info.extra_info_digest); }}}

Shouldn't there be a ! in there?

Thanks! Fix pushed in 69472ca4210183ec11a632abd952b6060a086258.

thanks for the fix on that; merging.

Trac:
Status: needs_review to closed
Resolution: N/A to fixed

You left alone ei->cache_info.send_unencrypted = 1 with a possible ei == NULL.

Replying to cypherpunks:

You left alone ei->cache_info.send_unencrypted = 1 with a possible ei == NULL. Oops! Thanks for reporting that!

Trac:
Resolution: fixed to N/A
Status: closed to reopened

See fix2195-fix ( !ssh://mob@repo.or.cz/srv/git/tor/rransom.git fix2195-fix ).

Trac:
Status: reopened to needs_review

Merged; thanks to cypherpunks for the review and rransom for the quick fix.

Trac:
Resolution: N/A to fixed
Status: needs_review to closed

Do not dereference NULL if a bridge fails to build It was about a public relay not a bridge: !options->BridgeRelay Bridge wasn't affect.

Trac:
Keywords: N/A deleted, tor-relay added

Trac:
Component: Tor Relay to Tor

closed

moved to tpo/core/tor#2195 (closed)