Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

extmod/moduhashlib: Add members to sha256 to make it usable by the hmac module. #4539

Closed
wants to merge 1 commit into from

Conversation

Jongy
Copy link
Contributor

@Jongy Jongy commented Feb 21, 2019

micropython-hmac reuiqres extra attributes currently not implemented by uhashlib;
This patch enables them (and fixes #4500) if demanded (only for SHA256 though).

…`hmac` module.

`micropython-hmac` reuiqres extra attributes currently not implemented by `uhashlib`;
This patch enables them (and fixes micropython#4500) if demanded (only for SHA256 though).
Copy link
Contributor

@pfalcon pfalcon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-1 to the whole logic. If some "micropython-hmac" needs extra members, then it should be fixed, not uhashlib.

@Jongy
Copy link
Contributor Author

Jongy commented Feb 23, 2019

-1 to the whole logic. If some "micropython-hmac" needs extra members, then it should be fixed, not uhashlib.

Fair enough. However, this is basically done to enable the same functionality as done in #4488, and at least the copy method has to be implemented in C, while the 2 constants indeed don't have to (or we can just get rid of them.

@pfalcon
Copy link
Contributor

pfalcon commented Feb 24, 2019

However, this is basically done to enable the same functionality as done in #4488

That's why you don't see me praising it. And the issue is not a specific change, but meta-process - people take adding bloat as an indulgence to add even more bloat. Whereas adding anything should be seen as an exception. If a change grows code size, it's by definition bad, and the question is whether it adds order of magnitude more goodness than the bad it inherently brings.

and at least the copy method has to be implemented in C

This assumes that copy() is needed, but you can check e.g. Wikipedia that HMAC has nothing to do with copying a hasher object.

Note that I'm not saying that copy() isn't needed in general - maybe it's needed (configurable and disabled by default), but it's bad, bad change, because it doesn't really enable anything (reasonable and conventional) which wasn't available without it, but turns MicroPython into bloat.

tannewt added a commit to tannewt/circuitpython that referenced this pull request Apr 8, 2021
…support-matrix

add alias boards and modules to the support matrix
@dpgeorge
Copy link
Member

The hmac module is still broken, but I don't think this is the right way to fix it. Would be better to rewrite the hmac module to not require `copy().

But thanks anyway!

@dpgeorge dpgeorge closed this Jun 24, 2022
@Jongy Jongy deleted the uhashlib-sha256-hmac-compat branch June 26, 2022 17:30
@jimmo
Copy link
Member

jimmo commented Aug 8, 2022

The hmac module is still broken, but I don't think this is the right way to fix it. Would be better to rewrite the hmac module to not require `copy().

@Jongy I did this in micropython/micropython-lib#515

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

uhashlib not compatible with micropython-hmac
4 participants