classification
Title: Add advice on best practices for hashing secrets
Type: enhancement Stage: patch review
Components: Documentation Versions: Python 3.4, Python 3.5, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: docs@python Nosy List: Ramchandra Apte, alex, asvetlov, christian.heimes, docs@python, ezio.melotti, giampaolo.rodola, hynek, jab, pitrou, r.david.murray, rhettinger, vstinner, yating.huang
Priority: normal Keywords: patch

Created on 2013-01-21 08:48 by christian.heimes, last changed 2016-01-04 16:51 by Ramchandra Apte.

Files
File name Uploaded Description Edit
hashlib.patch priyapappachan, 2014-03-11 18:32 Need more explanation in 'Security Considerations' section. review
hashlib.patch yating.huang, 2014-03-13 21:31 review
Messages (19)
msg180330 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-01-21 08:48
Lot's of people still think that something like sha512(secret + message), sha1(password + salt) or even sha1(password) is secure. Except it isn't. Most crypto hash functions like md5, sha1, sha2 family (sha256, sha384, sha512) use a Merkle–Damgård construction [1]. The construction is vulnerable to several attack vectors like length extension attacks. Passwords needs special care, too.

I propose we add a warning to the documentation of hashlib. It's not the right place to teach cryptographics but it's a good place to raise attention. The warning should explain that you shouldn't solely hash secrets or messages containing a secret. For messages a MAC algorithm like HMAC should be used. For passwords a key stretching and key derivation function like PBKDF2, bcrypt or scrypt is much more secure.

[1] http://en.wikipedia.org/wiki/Merkle%E2%80%93Damg%C3%A5rd_construction
msg180331 - (view) Author: Hynek Schlawack (hynek) * (Python committer) Date: 2013-01-21 09:06
I think since we ship cryptographic functions, we should take responsibility and warn against the most common mistakes people do.
msg180341 - (view) Author: Ramchandra Apte (Ramchandra Apte) * Date: 2013-01-21 13:30
+1
"Better to be safe than sorry"
msg200764 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-10-21 12:25
Who likes to step in? Alex?
msg203161 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-11-17 14:11
ping :)
msg203207 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-11-17 18:51
This might be useful to whoever attempts to write a patch: http://docs.python.org/devguide/documenting.html#security-considerations-and-other-concerns
msg203225 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013-11-17 23:05
> For passwords a key stretching and key derivation function like PBKDF2, bcrypt or scrypt is much more secure.

It looks like Python 3.4 now provides something for pbkdf2, so it may be interested to mention it on the top of the hashlib in your warning.

http://docs.python.org/dev/library/hashlib.html#hashlib.pbkdf2_hmac
msg213171 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-03-11 19:46
Priya: I see that there is already a 'warning' box at the top of the hashib page.  If Christian concurs, I suggest we add your note to that warning, with the link to the security considerations section as you have it, and removing the reference to the 'see also' section.  Then we move the security related 'see also' link into the new security section, with a sentence that says that it explains the weaknesses.

A note about the patch: you should wrap all lines to less than 80 characters.
msg213478 - (view) Author: Ya-Ting Huang (yating.huang) * Date: 2014-03-13 21:31
Hi. this is my first patch. I tried to follow the instruction by David to add Christian's notes into a new security section.
msg213480 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-03-13 21:41
Please note what I said about wrapping lines to less than 80 characters.

Also, my thought was to move the 'see also' entry that is referenced in the existing warning text (the wikipedia link) into the 'security considerations' section, probably as a separate paragraph that consists of the current text that follows that reference, with 'wikipedia article' turned into a link to the actual article.  (See http://docutils.sourceforge.net/docs/user/rst/quickref.html#hyperlink-targets for information on how to make links to external urls in .rst files).
msg213533 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2014-03-14 10:44
If something gets added, please follow the dev-guide and word it affirmatively (here the recommended practices) instead of continuing to fill the docs with warnings and danger signs.
msg213541 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-03-14 12:55
Good point.  There is an existing warning for hash weaknesses...the whole thing could be rephrased as "Please see the security considerations section for important information on the considerations involved in using the various hashing algorithms, and notes on best practices for message and password hashing."

The whole thing could then be turned into a note...except that there is a reason that it is a warning, because some of the hashes have known weaknesses yet still must be used for certain things.  So probably it should stay a warning in this particular case.
msg213545 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2014-03-14 13:23
> So probably it should stay a warning in this particular case.

Please don't.  Python's docs have become cluttered with warning and danger signs.  This stands in marked contrast with the docs for other languages which are much cleaner.  Our docs have also started to become "preachy", telling people how we think they should write programs rather than documenting what the language does.
msg213548 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-03-14 13:36
Raymond: I'm not talking about *adding* a warning.

Is it your opinion that the existing warning should be removed?
msg213558 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2014-03-14 14:50
Raymond makes a good point. We mustn't clutter the docs with warnings. People are going to skip warning boxes if they occur too often. The documentation of the hashlib module contains three "note" boxes and one "warning box". That's far too many.

The first "note" box could be moved to "see also". The other two "note" could be removed and their content added to the documentation of update(). The warning box should follow the example of the ssl module and all further security considerations should be moved into a new section.

The Python stdlib documentation is the wrong place to teach users about crypto and security stuff. But in my opinion good documentation should point out that something is dangerous or may lure a user into false sense of security.

Perhaps I should start a howto with common security-related issues in Python software for 3.5.
msg213563 - (view) Author: A.M. Kuchling (akuchling) * (Python committer) Date: 2014-03-14 15:19
+1 to reducing the number of notes, and to a security HOWTO.  (Christian: if you need writing help, please let me know; I'd be happy to help.)
msg213575 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-03-14 17:38
Note boxes have nothing to do with warnings, we should discuss them separately if needed.
(I see nothing wrong with multiple notes, given that a note is generally something ancillary and optional)
msg257267 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2016-01-01 03:24
> People are going to skip warning boxes if they occur too often.

I'm not sure I agree.  This would be true if they were abused for trivial things ("Warnings: using .pop() on a empty list will return an IndexError!"), but I don't think they are.

I think warnings are ignored only by people that are already familiar with the module and its limitation/issues, and that know what they are doing.  If the warning is not evident, people are going to miss it [0].

If warnings are used correctly, people will spot them easily and read them (or ignore them if they already know what they are warning against).

[0]: I know I missed it in e.g. https://api.jquery.com/die/ -- the function is deprecated, but (currently) this is only written in the top right corner and in small in the category at the top -- two places that are easily overlooked.  https://api.jquery.com/toggle-event/ on the other hand has a clearly visible yellow box at the top that immediately says that the method is deprecated.
msg257468 - (view) Author: Ramchandra Apte (Ramchandra Apte) * Date: 2016-01-04 16:51
It is good to add warnings; if they are ignored it is little worse than the
status quo.

On 1 January 2016 at 08:54, Ezio Melotti <report@bugs.python.org> wrote:

>
> Ezio Melotti added the comment:
>
> > People are going to skip warning boxes if they occur too often.
>
> I'm not sure I agree.  This would be true if they were abused for trivial
> things ("Warnings: using .pop() on a empty list will return an
> IndexError!"), but I don't think they are.
>
> I think warnings are ignored only by people that are already familiar with
> the module and its limitation/issues, and that know what they are doing.
> If the warning is not evident, people are going to miss it [0].
>
> If warnings are used correctly, people will spot them easily and read them
> (or ignore them if they already know what they are warning against).
>
> [0]: I know I missed it in e.g. https://api.jquery.com/die/ -- the
> function is deprecated, but (currently) this is only written in the top
> right corner and in small in the category at the top -- two places that are
> easily overlooked.  https://api.jquery.com/toggle-event/ on the other
> hand has a clearly visible yellow box at the top that immediately says that
> the method is deprecated.
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue17006>
> _______________________________________
>
History
Date User Action Args
2016-01-04 16:51:53Ramchandra Aptesetmessages: + msg257468
2016-01-01 03:24:17ezio.melottisetmessages: + msg257267
2015-10-29 16:27:13jabsetnosy: + jab
2014-12-31 16:22:31akuchlingsetnosy: - akuchling
2014-06-19 22:53:34ezio.melottisetstage: needs patch -> patch review
versions: + Python 3.5, - Python 3.3
2014-03-14 17:38:17pitrousetnosy: + pitrou
messages: + msg213575
2014-03-14 15:19:36akuchlingsetnosy: + akuchling
messages: + msg213563
2014-03-14 14:50:53christian.heimessetmessages: + msg213558
2014-03-14 13:36:42r.david.murraysetmessages: + msg213548
2014-03-14 13:23:47rhettingersetmessages: + msg213545
2014-03-14 12:55:49r.david.murraysetmessages: + msg213541
2014-03-14 10:45:14rhettingersettitle: Warn users about hashing secrets? -> Add advice on best practices for hashing secrets
2014-03-14 10:44:06rhettingersetnosy: + rhettinger
messages: + msg213533
2014-03-13 21:41:13r.david.murraysetmessages: + msg213480
2014-03-13 21:31:49yating.huangsetfiles: + hashlib.patch
nosy: + yating.huang
messages: + msg213478

2014-03-11 19:46:32r.david.murraysetnosy: + r.david.murray
messages: + msg213171
2014-03-11 18:32:46priyapappachansetfiles: + hashlib.patch
keywords: + patch
2013-11-17 23:05:47vstinnersetnosy: + vstinner
messages: + msg203225
2013-11-17 18:51:00ezio.melottisetmessages: + msg203207
2013-11-17 14:11:16christian.heimessetmessages: + msg203161
2013-10-21 12:25:45christian.heimessetnosy: + alex
messages: + msg200764
2013-01-21 13:51:49asvetlovsetnosy: + asvetlov
2013-01-21 13:30:58Ramchandra Aptesetnosy: + Ramchandra Apte
messages: + msg180341
2013-01-21 11:16:04giampaolo.rodolasetnosy: + giampaolo.rodola
2013-01-21 09:06:53ezio.melottisetnosy: + ezio.melotti

stage: needs patch
2013-01-21 09:06:15hyneksetnosy: + hynek
messages: + msg180331
2013-01-21 08:48:02christian.heimescreate