Add __eq__ to HashAlgorithm and padding instances by mathiasertl · Pull Request #13271 · pyca/cryptography

mathiasertl · 2025-08-09T09:06:11Z

This PR:

Adds __eq__() to all subclasses of HashAlgorithm.
Adds __eq__() to all padding classes.

My primary use case is testing. First, it's really un-pythonic to me (obviously very subjective, but it feels just not right to me) to write assert isinstance(cert.cert.signature_hash_algorithm, hashes.SHA256()), when I could write assert cert.cert.signature_hash_algorithm == hashes.SHA256(). But once you start comparing to other data structures, it gets even uglier:

def test_something() -> None:
    ...
    assert isinstance(cert.signature_hash_algorithm, type(other.signature_hash_algorithm))

I think this just looks a lot better and readable:

def test_something() -> None:
    ...
    assert cert.signature_hash_algorithm == other.signature_hash_algorithm

reaperhulk · 2025-08-15T01:16:43Z

+  instances of classes in
+  :mod:`~cryptography.hazmat.primitives.asymmetric.padding`
+  comparable.
+* Added `salt_length` property to


Let's put these in a separate PR. Whether or not we want to do __eq__ is possibly debateable, but if you want these properties to be public that's a separate discussion.

Hi @reaperhulk

Thanks for the feedback - I've updated this PR to only include the __eq__ instances. I'll make a separate PR for the extra properties.

kr, Mat

reaperhulk · 2025-09-14T19:44:42Z

Looking at the test failures here the issue is that scapy creates its own HashAlgorithm, which now fails because they don't implement __eq__. We could avoid this by making __eq__ a concrete method that raises NotImplementedError, but we'd need to cover those lines in a test as well.

mathiasertl · 2025-09-15T08:44:19Z

Or it could be a breaking change and that lib would need to implement the method.

Open to implement any solution, just let me know what you prefer!

reaperhulk · 2025-09-16T02:21:19Z

@alex do you have an opinion here? I'm reluctant to spend breakage budget on this when we could avoid it with a concrete implementation, but I'm interested in other opinions.

alex · 2025-09-16T02:30:25Z

I'm pretty ambivalent about this. We have a few choices:

Simply implement __eq__ for our own hashes, don't change the contract (all objects always have __eq__).
Add __eq__ to the ABC and require people to implement it: breaking change for people defining their own hashes
Add __eq__ to the ABC and provide a default impl: works for some users with their own hashes, but is wrong for anyone with their own hash which has any attributes (e.g. blake2).

Of these I think (1) is probably best.

reaperhulk · 2025-09-16T02:56:59Z

For 3 the default impl could raise NotImplementedError so you wouldn’t need to have a potentially incorrect impl

alex · 2025-09-16T03:00:26Z

That can break existing applications, to the extent they're relying on the default object.__eq__ behavior!

mathiasertl · 2025-09-16T12:56:55Z

I'm also ambivalent, I defer to your call and will adapt as requested.

Just option three (default Implementation) seems like a bad idea to me.

Mat

reaperhulk · 2025-10-04T13:10:57Z

After thinking for too long, I'm okay with option 1 here.

reaperhulk · 2025-11-04T19:48:32Z

Are you still interested in this PR? Happy to review/land with option 1 + a rebase to fix the changelog conflict

alex

I think a question is whether we should be returning NotImplemented or False for diffefrent types

alex · 2025-11-16T16:23:10Z

+            eq_salt_length = self._salt_length is other._salt_length
+
+        return (
+            isinstance(other, PSS)


We need to do this check first (otherwise you get an AttributeError) and return NotImplemented

Right, on it!

alex · 2025-11-16T16:23:16Z


+    def __eq__(self, other: typing.Any) -> bool:
+        return (
+            isinstance(other, OAEP)


Same issue here.

Actually not correct in this case:

>>> h = hashes.SHA256() >>> mgf = MGF1(h) >>> OAEP(mgf, h, None) == h False

alex · 2025-11-16T16:24:43Z

+* Make instances of
+  :class:`~cryptography.hazmat.primitives.hashes.HashAlgorithm` as well as
+  instances of classes in
+  :mod:`~cryptography.hazmat.primitives.asymmetric.padding`
+  comparable.


This isn't quite right for the behavior we implemented. I'd probably document it as "builtin hash classes can now be compared with =="

agreed. will be updated.

mathiasertl · 2025-11-16T16:53:20Z

I think a question is whether we should be returning NotImplemented or False for diffefrent types

I don't know of any classes that do that. Standard python certainly doesn't:

>>> {} == 0
False

alex · 2025-11-16T17:00:40Z

>>> {}.__eq__(0)
NotImplemented

:-)

mathiasertl · 2025-11-16T17:07:53Z

>>> {}.__eq__(0)
NotImplemented

:-)

To quote official documentation - emphasis mine:

A rich comparison method may return the singleton NotImplemented if it does not implement the operation for a given pair of arguments. By convention, False and True are returned for a successful comparison.

So I can implement it, it's just lots of extra code with no benefit that I'm aware of.

mathiasertl · 2025-11-16T17:52:36Z


+    def __eq__(self, other: typing.Any) -> bool:
+        return isinstance(other, DummyMGF)
+


This class is more-or-less duplicated in test_openssl as well!

I would offer to move it to tests.doubles.py for shorter code - if you approve.

Sounds great, thanks

reaperhulk · 2026-06-06T21:32:14Z

Sorry to let this drop off our radar. After discussion @alex and I still think there's value in supporting __eq__, but we do want the NotImplemented behavior to match our other __eq__ impls across the codebase.

mathiasertl · 2026-06-07T14:18:10Z

Hi there!

This also dropped of my radar, so no worries! I'll try to follow up soon, but busy in June. I hope July is still okay.

Mat

* Add __eq__ and __hash__ to hash algorithm and padding classes This incorporates the work from #13271, implementing __eq__ on all concrete HashAlgorithm subclasses and on the classes in the asymmetric padding module (PKCS1v15, PSS, OAEP, MGF1). Per review discussion on that PR, the implementations follow the pattern used by other __eq__ impls across the codebase: they return NotImplemented for non-matching types rather than False, and the ABCs are left untouched so third-party subclasses are unaffected. __hash__ is defined alongside __eq__ so instances remain hashable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WsrSAAABRsHFpHtQxZ1sjs * Remove __hash__ from hash algorithm and padding classes Keep only the __eq__ implementations, matching the scope of the original PR. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WsrSAAABRsHFpHtQxZ1sjs * Test hash algorithm inequality x509-ext style Replace the direct __eq__ NotImplemented assertion with a test_ne matching the pattern used in test_x509_ext.py. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WsrSAAABRsHFpHtQxZ1sjs * Restructure equality tests to match test_x509_ext.py conventions Split == and != assertions into separate test_eq/test_ne methods, rename test_eq_different_* to test_ne_different_*, and assert inequality against object() in test_ne. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WsrSAAABRsHFpHtQxZ1sjs * Collapse redundant test_eq/test_ne methods Merge test_ne assertions into test_eq where both were parameterized over the same values or trivially small. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WsrSAAABRsHFpHtQxZ1sjs --------- Co-authored-by: Claude <noreply@anthropic.com>

reaperhulk · 2026-07-04T13:37:22Z

#15153 takes this over, thanks!

mathiasertl force-pushed the add-eq-functions branch from 631283d to 2be27b4 Compare August 9, 2025 09:16

reaperhulk reviewed Aug 15, 2025

View reviewed changes

mathiasertl force-pushed the add-eq-functions branch from 2be27b4 to a68beff Compare August 16, 2025 12:03

mathiasertl changed the title ~~Add __eq__ and missing properties~~ Add __eq__ to HashAlgorithm and padding instances Aug 16, 2025

mathiasertl force-pushed the add-eq-functions branch from a68beff to 7dd838f Compare November 16, 2025 16:20

alex reviewed Nov 16, 2025

View reviewed changes

mathiasertl force-pushed the add-eq-functions branch from 7dd838f to 8fd3f3b Compare November 16, 2025 16:45

mathiasertl force-pushed the add-eq-functions branch from 8fd3f3b to c0e6409 Compare November 16, 2025 16:58

mathiasertl force-pushed the add-eq-functions branch 3 times, most recently from d8d41e4 to 45d3bac Compare November 16, 2025 17:43

mathiasertl commented Nov 16, 2025

View reviewed changes

add __eq__ for HashAlgorithm and padding instances

d71c7ac

mathiasertl force-pushed the add-eq-functions branch from 45d3bac to d71c7ac Compare November 16, 2025 17:55

reaperhulk closed this Jul 4, 2026


		def __eq__(self, other: typing.Any) -> bool:
		return isinstance(other, DummyMGF)

Uh oh!

Conversation

mathiasertl commented Aug 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mathiasertl Aug 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

reaperhulk commented Sep 14, 2025

Uh oh!

mathiasertl commented Sep 15, 2025

Uh oh!

reaperhulk commented Sep 16, 2025

Uh oh!

alex commented Sep 16, 2025

Uh oh!

reaperhulk commented Sep 16, 2025

Uh oh!

alex commented Sep 16, 2025

Uh oh!

mathiasertl commented Sep 16, 2025

Uh oh!

reaperhulk commented Oct 4, 2025

Uh oh!

reaperhulk commented Nov 4, 2025

Uh oh!

alex left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mathiasertl Nov 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mathiasertl commented Nov 16, 2025

Uh oh!

alex commented Nov 16, 2025

Uh oh!

mathiasertl commented Nov 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

reaperhulk commented Jun 6, 2026

Uh oh!

mathiasertl commented Jun 7, 2026

Uh oh!

reaperhulk commented Jul 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

mathiasertl commented Aug 9, 2025 •

edited

Loading

mathiasertl Aug 16, 2025 •

edited

Loading

mathiasertl Nov 16, 2025 •

edited

Loading