[Django] #36546: Deprecate django.utils.crypto.constant_time_compare()

[Django]

#36546: Deprecate django.utils.crypto.constant_time_compare()
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Type:
| Cleanup/optimization
Status: new | Component: Utilities
Version: dev | Severity: Normal
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
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
[https://github.com/python/cpython/blob/dd079db4b96fa474b8e6d71ae9db662c4ce28caf/Lib/secrets.py#L17
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/36546>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36546: Deprecate django.utils.crypto.constant_time_compare()
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner: (none)
Type: | Status: new
Cleanup/optimization |
Component: Utilities | Version: dev
Severity: Normal | Resolution:

Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by 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))
}}}
`
--
Ticket URL: <https://code.djangoproject.com/ticket/36546#comment:1>

[Django]

#36546: Deprecate django.utils.crypto.constant_time_compare()
--------------------------------------+------------------------------------

Reporter: Tim Graham | Owner: (none)

Type: Cleanup/optimization | Status: new

Component: Utilities | Version: dev
Severity: Normal | Resolution:

Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

--------------------------------------+------------------------------------
Changes (by Sarah Boyce):

* stage: Unreviewed => Accepted

--
Ticket URL: <https://code.djangoproject.com/ticket/36546#comment:2>

[Django]

#36546: Deprecate django.utils.crypto.constant_time_compare()
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner:

Type: | JaeHyuckSa
Cleanup/optimization | Status: assigned

Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by JaeHyuckSa):

* owner: (none) => JaeHyuckSa
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/36546#comment:3>

[Django]

#36546: Deprecate django.utils.crypto.constant_time_compare()
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner:
Type: | JaeHyuckSa
Cleanup/optimization | Status: assigned
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by JaeHyuckSa):

* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/36546#comment:4>

[Django]

#36546: Deprecate django.utils.crypto.constant_time_compare()
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner:
Type: | JaeHyuckSa
Cleanup/optimization | Status: assigned
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by JaeHyuckSa):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/36546#comment:5>

[Django]

#36546: Deprecate django.utils.crypto.constant_time_compare()
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner:
Type: | JaeHyuckSa
Cleanup/optimization | Status: assigned
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by JaeHyuckSa):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/36546#comment:6>

[Django]

#36546: Deprecate django.utils.crypto.constant_time_compare()
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner:
Type: | JaeHyuckSa
Cleanup/optimization | Status: assigned
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/36546#comment:7>

[Django]

#36546: Deprecate django.utils.crypto.constant_time_compare()
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner:
Type: | JaeHyuckSa
Cleanup/optimization | Status: assigned
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by JaeHyuckSa):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/36546#comment:8>

[Django]

#36546: Deprecate django.utils.crypto.constant_time_compare()
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner:
Type: | JaeHyuckSa
Cleanup/optimization | Status: assigned
Component: Utilities | Version: dev
Severity: Normal | Resolution:

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/36546#comment:9>

[Django]

#36546: Deprecate django.utils.crypto.constant_time_compare()
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner:
Type: | JaeHyuckSa

Cleanup/optimization | Status: closed
Component: Utilities | Version: dev
Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce <42296566+sarahboyce@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"0246f478882c26bc1fe293224653074cd46a90d0" 0246f47]:
{{{#!CommitTicketReference repository=""
revision="0246f478882c26bc1fe293224653074cd46a90d0"
Fixed #36546 -- Deprecated django.utils.crypto.constant_time_compare() in
favor of hmac.compare_digest().

Signed-off-by: SaJH <wogur...@gmail.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36546#comment:10>

[Django]

#36546: Deprecate django.utils.crypto.constant_time_compare()
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner:
Type: | JaeHyuckSa
Cleanup/optimization | Status: closed
Component: Utilities | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Sarah Boyce <42296566+sarahboyce@…>):

In [changeset:"d0e4dd5cdd743a5c43c4ccc2c8fa29d3982eaa71" d0e4dd5c]:
{{{#!CommitTicketReference repository=""
revision="d0e4dd5cdd743a5c43c4ccc2c8fa29d3982eaa71"
Fixed #36572 -- Revert "Fixed #36546 -- Deprecated

django.utils.crypto.constant_time_compare() in favor of
hmac.compare_digest()."

This reverts commit 0246f478882c26bc1fe293224653074cd46a90d0.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36546#comment:11>

[Django]

#36546: Deprecate django.utils.crypto.constant_time_compare()
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner:
Type: | JaeHyuckSa

Cleanup/optimization | Status: new

Component: Utilities | Version: dev
Severity: Normal | Resolution:

Keywords: | Triage Stage:
| Someday/Maybe
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce):

* has_patch: 1 => 0
* resolution: fixed =>
* stage: Ready for checkin => Someday/Maybe
* status: closed => new

Comment:

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
--
Ticket URL: <https://code.djangoproject.com/ticket/36546#comment:12>

[Django]

#36546: Deprecate django.utils.crypto.constant_time_compare()
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner:
Type: | JaeHyuckSa
Cleanup/optimization | Status: new
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Someday/Maybe
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by 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
--
Ticket URL: <https://code.djangoproject.com/ticket/36546#comment:13>

[Django]

#36546: Deprecate django.utils.crypto.constant_time_compare()
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner: (none)
Type: | Status: assigned
Cleanup/optimization |

Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Someday/Maybe
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by JaeHyuckSa):

* owner: JaeHyuckSa => (none)

* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/36546#comment:14>

[Django]

#36546: Deprecate django.utils.crypto.constant_time_compare()
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner: (none)
Type: | Status: assigned
Cleanup/optimization |
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Someday/Maybe
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by JaeHyuckSa):

* cc: JaeHyuckSa (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/36546#comment:15>