Add support for SRP authentication to the manager#312
Conversation
|
|
||
| def int_to_bytes(n): | ||
| """Convert the given int to a byte string in big-endian byte order.""" | ||
| bs = [] |
| untouched. | ||
| """ | ||
| m = hash_class() | ||
| for a in args: |
| converted to bytes using int_to_bytes, while byte strings are left | ||
| untouched. | ||
| """ | ||
| m = hash_class() |
| m.update(challenge) | ||
| if isinstance(credential.password, bytes): | ||
| m.update(credential.password) | ||
| if 'srp' in self.manager_features: |
There was a problem hiding this comment.
@jayich Secure Random Password -- see https://tools.ietf.org/html/rfc5054
|
|
||
| if tls_mode == 'on': | ||
| tls_options = crypto.tls_options(host) | ||
| p = yield _factory.connectSSL(host, port, tls_options, timeout=C.TIMEOUT) |
kunalq
left a comment
kunalq
left a comment
There was a problem hiding this comment.
Aside from some comments re: exceptions and constants, this LGTM :)
| """Convert the given int to a byte string in big-endian byte order.""" | ||
| bs = [] | ||
| while n > 0: | ||
| b, n = n & 0xFF, n >> 8 |
There was a problem hiding this comment.
For readability, I'd break apart the parallel assignment here--
b = n & 0xFF
n = n >> 8| 0846851D F9AB4819 5DED7EA1 B1D510BD 7EE74D73 FAF36BC3 1ECFA268 | ||
| 359046F4 EB879F92 4009438B 481C6CD7 889A002E D5EE382B C9190DA6 | ||
| FC026E47 9558E447 5677E9AA 9E3050E2 765694DF C81F56E8 80B96E71 | ||
| 60C980DD 98EDD3DF FFFFFFFF FFFFFFFF |
There was a problem hiding this comment.
Likewise here, I think we can have a common file that can be read in to store these numbers. Then both Scala & python can read them in.
There was a problem hiding this comment.
And, actually, we can use MoultingYAML in Scala and Python's YAML format to stuff these into a YAML file keyed on the bit size.
| return hash_all(hash_class, *args) | ||
|
|
||
| def PAD(x): | ||
| return int_to_bytes(x).rjust(len(N_bytes), chr(0)) |
There was a problem hiding this comment.
I would suggest renaming PAD -> pad to keep naming consistent (even though this looks like it's mimicking a C macro)
| u = bytes_to_int(H(PAD(A), PAD(B))) | ||
| if u == 0: | ||
| raise Exception("auth failed: incompatible server and client " | ||
| "challenges") |
There was a problem hiding this comment.
I recommend making an AuthException class and raising that instead, in case we need it in the future (or using the existing LoginFailedError exception)...
| m.update(challenge) | ||
| if isinstance(credential.password, bytes): | ||
| m.update(credential.password) | ||
| if 'srp' in self.manager_features: |
There was a problem hiding this comment.
@jayich Secure Random Password -- see https://tools.ietf.org/html/rfc5054
| A_bytes, M1 = srp.process_server_challenge(salt, B_bytes) | ||
| try: | ||
| resp, M2 = yield self._sendManagerRequest(11, (A_bytes, M1)) | ||
| except Exception: |
There was a problem hiding this comment.
I realize that labrad will propagate the stack trace to the client, which should hopefully explain where the exception is being raised, but I'm still hesitant to use this outside of generic code. Any chance we can be more specific in this exception? My experience with except Exception is that it can cause serious debugging issues trying to isolate root causes.
| m.update(credential.password.encode('UTF-8')) | ||
| try: | ||
| resp = yield self._sendManagerRequest(0, m.digest()) | ||
| except Exception: |
3 participants
No description provided.