[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #5810 [Stem]: Implement verification of server descriptor
#5810: Implement verification of server descriptor
-------------------------+--------------------------------------------------
Reporter: reganeet | Owner: reganeet
Type: enhancement | Status: needs_revision
Priority: normal | Milestone:
Component: Stem | Version:
Keywords: descriptors | Parent:
Points: | Actualpoints:
-------------------------+--------------------------------------------------
Comment(by atagar):
Great. Please swap this ticket back to 'Needs code/patch review' when
you've pushed the revisions.
> I think I based this structure on the previous comments in the ticket.
If you prefer exceptions I can change the code to use them.
Gotcha. Since this is being used for validation it makes more sense for us
to raise a ValueError, like the other validation issues do.
> Do you guys mind custom exceptions ?
Custom exceptions are fine if there's a good reason for it (see
stem/__init__.py and stem/connection.py for some examples of places that
we have custom exceptions).
However, in this case the ServerDescriptor is documented as raising a
ValueError when the content is invalid. You can add a custom exception for
these issues if you want, but please have it subclass ValueError.
> I'd be inclined to leave the log messages in as well, but I'll stick
with whatever your preferred style is for the code..
Our other validation errors (... of which there's a ton) simply raise an
exception so we should probably do that here too. I can see some
advantages to logging the issue as well, but if we did that then it should
probably be done for all validation issues.
> I was thinking about adding some code to create valid signed
descriptors.
Ah ha. Agreed, that would definitely be better.
> I'd prefer not to return anything from the function. I'm not sure that
the digest needs to be publicly accessible?
As I understand it this function is chiefly so that we can determine if
the descriptor's digest matches what the network status says that it
should be...
https://gitweb.torproject.org/stem.git/blob/HEAD:/stem/descriptor/server_descriptor.py#l257
https://gitweb.torproject.org/stem.git/blob/HEAD:/stem/descriptor/router_status_entry.py#l248
However, in that case the ServerDescriptor's digest() method and
RouterStatusEntryV3's digest attribute would ideally have the same
encoding. It looks like ServerDescriptor was using a hex encoding and
RouterStatusEntryV3 was simply storing the base64 value. Here's a router
status entry example...
{{{
r caerSidi p1aag7VwarGxqctS7/fS0y5FU+s wyCIDl1crtM3lkLDEFFpR8+V8oM
2012-11-17 05:17:27 71.35.143. 230 9001 0
s Fast Named Running Stable Valid
v Tor 0.2.2.35
w Bandwidth=39
p reject 1-65535
}}}
My vote would be for both of them to provide the hex value, though I could
be persuaded otherwise.
Cheers! -Damian
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/5810#comment:22>
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