Deprecate django.utils.crypto.constant_time_compare()

[Tim Graham]

Historically it was a bespoke implementation (added in 45c7f427ce830dd1b2f636fb9c244fda9201cadb) before the stdlib implementation was preferred (58176dee88ac7c1038c7f685af023e634b143d02). Now it's just alias of secrets.compare_digest (which itself is ​an alias of hmac.compare_digest, which was used before 1d0bab0bfd77edcf1228d45bf654457a8ff1890d).

constant_time_compare() does call force_bytes() on its arguments but this was a workaround for Python 2.7 (7e3cf3cfd27e53ced0a1fc65a02849f78a292d3d) and no tests in Django's test suite fail with those calls removed.

[suhail vs]

what about adding a warning in function constant_time_compare in file django.utils.crypto.py

import warnings
from django.utils.deprecation import RemovedInNextVersionWarning

def constant_time_compare(val1, val2):
    """Return True if the two strings are equal, False otherwise."""
    warnings.warn(
        "constant_time_compare(val1, val2) is deprecated and will be removed in Django 6.0. "
        "`import secrets` from python standard library, then run secrets.compare_digest(val1,val2) instead.",
        RemovedInNextVersionWarning,
        stacklevel=2
    )
    return secrets.compare_digest(force_bytes(val1), force_bytes(val2))

`

[Sarah Boyce <42296566+sarahboyce@…>]

In 0246f47:

Fixed #36546 -- Deprecated django.utils.crypto.constant_time_compare() in favor of hmac.compare_digest().

Signed-off-by: SaJH <wogur981208@…>

[Sarah Boyce <42296566+sarahboyce@…>]

In d0e4dd5c:

Fixed #36572 -- Revert "Fixed #36546 -- Deprecated django.utils.crypto.constant_time_compare() in favor of hmac.compare_digest()."

This reverts commit 0246f478882c26bc1fe293224653074cd46a90d0.

[Sarah Boyce]

Reopening following revert. See discussion in #36572

A later first step could be to replace internal usage of constant_time_compare() with compare_digest()

The deprecation itself needs more consideration

[JaeHyuckSa]

There are quite a few points that need further discussion, so it’s difficult to move this forward right now. I’ll unassign myself for the time being

[Pravin]

What is current state, Can i work on this ?

[Simon Charette]

Pravin, refer to the discussion on the revert commit in comment:12. Looks like we first want to stop using it internally.

[Pravin]

@Simon Thanks for the more context.

I'm going to start working on this , First I will remove internal dependency on constant_time_compare.

[Pravin]

just confirming the technical approach for the internal dependency removal:

1. I will be using secrets.compare_digest() for the replacement.
2. The implementation at the call site will include force_bytes() on both arguments to ensure type safety: secrets.compare_digest(force_bytes(val1), force_bytes(val2)).
3. The definition of constant_time_compare() itself will not be modified to avoid breaking third-party applications that rely on its current type-coercion logic.

Example of the change:

Old internal Django code:constant_time_compare(val1, val2)
New internal Django code: secrets.compare_digest(force_bytes(val1), force_bytes(val2))

I am proceeding with the internal refactoring now.

[Tim Graham]

No, the force_bytes() is not needed in Django's use of constant_time_compare(), see ticket:36572#comment:4. The problem is only with third-party users of the function.

[Pravin]

Hi Tim,
I was in that dilema and I was not so confident in removing force_bytes. But Since you confirmed it I can safely proceed ( i will no use force_bytes ). Thanks.