[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #5262 [Stem]: Implement Safe Cookie in Stem

#5262: Implement Safe Cookie in Stem
-------------------------+--------------------------------------------------
 Reporter:  atagar       |          Owner:  neena       
     Type:  enhancement  |         Status:  needs_review
 Priority:  normal       |      Milestone:              
Component:  Stem         |        Version:              
 Keywords:               |         Parent:              
   Points:               |   Actualpoints:              
-------------------------+--------------------------------------------------
Changes (by neena):

  * status:  assigned => needs_review


Comment:

 I am yet to complete writing the tests, though, that might change before
 you review this.

 I have two branches, one with a seperate tree of exceptions, and one which
 shares the cookie authentication exceptions with an additional attribute
 added to it for storing the authentication method used.

 http://repo.or.cz/w/stem/neena.git/shortlog/refs/heads/safe-cookie
 and
 http://repo.or.cz/w/stem/neena.git/shortlog/refs/heads/safe-cookie-alt

 Replying to [comment:4 atagar]:
 > > +  |  |- InvalidClientNonce - The client nonce is invalid.
 >
 > Are you sure that we want to reuse CookieAuthFailed exceptions for
 safecookie authentication? If it works then that's certainly preferable
 (since it simplifies the exception hierarchy), but I wonder if it'll
 confuse authenticate() callers since they then won't know if an exception
 is from a cookie or safecookie authentication attempt.
 >
 > One option would be for CookieAuthFailed to have an attribute that says
 if it was from a cookie or safecookie failure, but I'm not sure if that's
 cleaner or more confusing than having a separate branch of exceptions. At
 present I'm favoring it a bit since normal cookie authentication is slated
 for deprecation...
 >

 I thought having seperate exceptions would be a good idea. But, now, after
 implementing both, I think having an attribute with the authentication
 method is quite neat.

 > Hmm, I keep going back and forth about if it would be better as
 'authenticate_safecookie' or 'authenticate_safe_cookie' - thoughts?
 I don't really mind either. Shall I change it?

 Replying to [comment:5 atagar]:
 > > +  parsed_ac_response = re.match("^AUTHCHALLENGE
 SERVERHASH=([A-Fa-f0-9]*) \
 > > +SERVERNONCE=([A-Fa-f0-9]*)$", challenge_response.content()[0][2])
 >
 > No, no, no, please don't parse responses like this via regexes. This is
 the path to the dark side.
 Yeah, I had a feeling I was doing something terribly wrong here.

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/5262#comment:7>
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