[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #2748 [Tor Hidden Services]: Multiple bogosities in rend_cache_lookup_v2_desc_as_dir (was: v2 HS dirs reply incorrectly to requests for descriptors they are not responsible for)
#2748: Multiple bogosities in rend_cache_lookup_v2_desc_as_dir
---------------------------------+------------------------------------------
Reporter: rransom | Owner: rransom
Type: defect | Status: needs_review
Priority: minor | Milestone: Tor: 0.2.1.x-final
Component: Tor Hidden Services | Version:
Keywords: | Parent:
Points: | Actualpoints:
---------------------------------+------------------------------------------
Comment(by rransom):
Replying to [comment:2 Sebastian]:
> Hrm. The documentation for rend_cache_lookup_v2_desc_as_dir() says that
the function returns 0 if we couldn't look up the descriptor but the
descriptor request was well-formed, so your patch violates the
documentation here. The caller also relies on this behaviour, see comments
in directory_handle_command_get().
When `rend_cache_lookup_v2_desc_as_dir` was added in
[https://gitweb.torproject.org/tor.git/commitdiff/e136f00ca8a955afc7507ba5e5e91374a7b59401
commit e136f00c], it was clearly intended to return -1 (indicating
failure) if the request was rejected for any reason other than 'descriptor
not found' (as was `rend_cache_store_v2_dir`, introduced in the same
commit). The comment on `rend_cache_lookup_v2_desc_as_dir` was changed to
document its return values in
[https://gitweb.torproject.org/tor.git/commitdiff/024798ee4c6d504f7aec3ddcb3dfc07d96e475f1
the following commit (024798ee)]; that documentation appears to have been
based on the comments in `directory_handle_command_get`, and only
accidentally matched the function's behaviour because it misinterpreted
the return value of `hid_serv_responsible_for_desc_id`.
Then again, that function also logs at level `LOG_WARN` (not
`LOG_PROTOCOL_WARN`) if it receives a malformed request, so its original
behaviour wasn't especially good.
> Also, reading the spec, I don't see why a 400 response would be the
right thing to send back if alice doesn't have the descriptor but the
request was ok. Please clarify?
I still think it would be appropriate to return error code 400 if a
directory server refused to try to process a request for any reason, but
I'm no longer convinced that refusing to process requests for HS
descriptors that a relay is not responsible for is worthwhile.
See [http://repo.or.cz/w/tor/rransom.git/shortlog/refs/heads/bug2748-v2
bug2748-v2] ( !ssh://mob@xxxxxxxxxx/srv/git/tor/rransom.git bug2748-v2 )
for patches to remove this call to `hid_serv_responsible_for_desc_id`
entirely and use the correct log level to report malformed requests.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/2748#comment:3>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs