Hash cookie secret with user hashed password. #3009
Conversation
notebook/notebookapp.py
Outdated
| return secret | ||
| key = encodebytes(os.urandom(1024)) | ||
| self._write_cookie_secret_file(key) | ||
| h = sha256(digest_size=len(key)) |
There was a problem hiding this comment.
I was going to make a comment here about what happens if the cookie file happened to be empty (leading to a 0 length), then I noticed that at least on my systems I can't set this keyword argument:
In [1]: from hashlib import sha256
In [2]: sha256(digest_size=0)
---------------------------------------------------------------------------
TypeError Traceback (most recent call last)
<ipython-input-2-94e5dda9a83d> in <module>()
----> 1 sha256(digest_size=0)
TypeError: openssl_sha256() takes no keyword arguments
notebook/notebookapp.py
Outdated
| self._write_cookie_secret_file(key) | ||
| h = sha256(digest_size=len(key)) | ||
| h.update(key) | ||
| h.update(self.password.encode()) |
There was a problem hiding this comment.
IMO this should be a HMAC, with the secret key as the HMAC key & the password as the message. HMACs are built for this exact specific use case, and supported in the stdlib.
There was a problem hiding this comment.
I disagree that HMAC are built for that (no code is going to check hmac(key, pw) equal the value here). The result of this function will be used by HMAC as the key later on (in tornado), so we're basically doing HMAC(HMAC(key, pwd_sha), cookie_value); Though I agree that the returned value with HMAC will still be (pseudo-)random enough.
Currently changing the password does not revoke current session: - jupyter notebook password <password1> - jupyter notebook - Logging in - Kill server - jupyter notebook password <other password> - jupyter notebook - Oh ! I'm still logged in. With this, as the "effective" secret depends on the (hashed) password, changing it void any existing session (which I believe is the goal of most password change)
|
Should we also (or as an alternative) have the the |
Currently changing the password does not revoke current session:
With this, as the "effective" secret depends on the (hashed) password,
changing it void any existing session (which I believe is the goal of
most password change)
cc @minrk, @rgbkrk, and @yuvipanda for review.