HMAC: deprecate default hash

[tiran]

BPO	17276
Nosy	@akuchling, @gpshead, @jcea, @pitrou, @vstinner, @tiran, @bitdancer
Files	17276.patch 17276-2.patch 17276-3.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/tiran'
closed_at = <Date 2013-11-20.16:23:46.549>
created_at = <Date 2013-02-22.12:16:42.941>
labels = ['type-bug', 'library']
title = 'HMAC: deprecate default hash'
updated_at = <Date 2014-03-09.19:17:42.572>
user = 'https://github.com/tiran'

bugs.python.org fields:

activity = <Date 2014-03-09.19:17:42.572>
actor = 'python-dev'
assignee = 'christian.heimes'
closed = True
closed_date = <Date 2013-11-20.16:23:46.549>
closer = 'christian.heimes'
components = ['Library (Lib)']
creation = <Date 2013-02-22.12:16:42.941>
creator = 'christian.heimes'
dependencies = []
files = ['31359', '31375', '32299']
hgrepos = []
issue_num = 17276
keywords = ['patch']
message_count = 19.0
messages = ['182662', '182663', '182666', '182668', '182669', '195569', '195645', '196662', '200729', '200801', '200931', '200934', '200937', '200939', '203165', '203228', '203498', '212950', '212974']
nosy_count = 8.0
nosy_names = ['akuchling', 'gregory.p.smith', 'jcea', 'pitrou', 'vstinner', 'christian.heimes', 'r.david.murray', 'python-dev']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue17276'
versions = ['Python 3.4']

[tiran]

As of now the hash algorithm for HMAC defaults to MD5. However MD5 is considered broken. HMAC-MD5 is still ok but shall not be used in new code. Applications should slowly migrate away from HMAC-MD5 and use a more modern algorithm like HMAC-SHA256.

Therefore I propose that default digestmod should be deprecated in Python 3.4 and removed in 3.5. Starting with Python 3.5 developer are forced to choose a hash algorithm like SHA256. Our documentation shall suggest it, too.

In addition I would like to enhance the meaning of the digestmod argument a bit. Right now it either must be a module or a callable. It should also support a name, e.g. hmac.new("secret", digestmod="sha256")

[jcea]

+1.

[pitrou]

I don't know how you intend to make digestmod mandatory given the current function signature.

Applications should slowly migrate away from HMAC-MD5 and use a more
modern algorithm like HMAC-SHA256.

Applications don't always choose their cipher. MD5 is needed for compatibility with existing protocols such as CRAM-MD5.

[tiran]

I don't know how you intend to make digestmod mandatory given the current function signature.

That's easy:

if digestmod is None:
    raise TypeError("HMAC needs argument 'digestmod'")

[tiran]

PS: I don't want to deprecate HMAC-MD5. I just want to deprecate that HMAC defaults to HMAC-MD5.

[tiran]

Here is a patch that deprecates MD5 has implicit default hashing algorithm. It also implements digestmod string support.

PEP-247 doesn't define the digestmod argument of keyed hashing algorithms. I'm going to define it in PEP-452.

[tiran]

assertWarns() is much easier than the block I have copied and pasted. Thanks. :)

[gpshead]

comments added to the review.

I don't think a DeprecationWarning should be raised as that'll infuriate users of python programs more than developers who can fix code.

[tiran]

GPS, what do you suggest instead? Do you want me to remove the deprecation warning?

[gpshead]

yes just remove the DeprecationWarning. Document it as deprecated with a release now+0.2 as the earliest it will be removed. (if you want a warning at all, use PendingDeprecationWarning as that one is filtered out by default so it won't bother users of tools written in Python but only developers actively looking for issues)

[tiran]

I've changed the deprecation warning to PendingDeprecationWarning. Please review my wording and grammar.

[vstinner]

I would prefer to directly raise an exception in Python 3.4. Developers will not notice a warning, warning are hidden by default. How many developers run their tests using -Werror?

Having to add a parameter to hmac() in applications to port them to Python 3.4 should not be so hard. And using MD5 is really a major security issue, don't you think so?

[pitrou]

Having to add a parameter to hmac() in applications to port them to
Python 3.4 should not be so hard. And using MD5 is really a major
security issue, don't you think so?

Some uses of md5 don't have anything to do with security. I'm -1
on removing the default value here.

[tiran]

HMAC-MD5 is still fine for legacy support. I wouldn't use it in new program, though

[tiran]

I'll commit the patch later.

[vstinner]

Well, if deprecating is not an option, it's probably better to add a red warning explaining why the default choice may not fit all use cases.

[python-dev]

New changeset 86107e7e6ee5 by Christian Heimes in branch 'default':
Issue bpo-17276: MD5 as default digestmod for HMAC is deprecated. The HMAC
http://hg.python.org/cpython/rev/86107e7e6ee5

[bitdancer]

I don't understand why PendingDeprecationWarning was used here. DeprecationWarnings are silent by default. I'm also not clear on why this is being delayed until 3.6, instead of being changed in 3.5 after a deprecation, given that the default is considered to be a bit of a security issue.

[python-dev]

New changeset c10ec51a2ce4 by R David Murray in branch 'default':
whatsnew: hmac *digestmod* accepts strings, and default is deprecated. (bpo-17276)
http://hg.python.org/cpython/rev/c10ec51a2ce4