QUERY · ISSUE
SHA512 and hashlib port
enhancement
At the moment, Micropython supports SHA1 and SHA256.
I would like to suggest addition of more algorithms, in specific SHA512.
It's already implemented in micropython-lib and of course at cpython itself.
CANDIDATE · PULL REQUEST
hashlib: Fix shadowing of sha256 and sha512 constructors.
hashlib.sha256() and hashlib.sha512() fails with:
TypeError: 'module' object is not callable
This fixes it by renaming the algorithm implementation modules.
Note that 'import hashlib.sha256' is not supported on CPython either,
so we don't need to support that.
I don't understand the need for this. The following works for me on uPy:
and all other variants of the sha.
I'm currently unsure. The above message was a cut and paste from yesterday.
I was trying to implement the hashlib.new() function, because my application uses this.
Without this patch it was not possible to do so, because from inside of hashlib's init.py accessing sha256 referenced the module instead of the function.
But I think it's a hell of confusing anyway, to have a subfile named the same as a function that is exported from the package.
See this commit: https://github.com/mbuesch/micropython-lib/commit/b86ca286b6fb65c4460e62850d45a3b9c502eb41
Without the file rename it will fail the test at line 6:
it seems that this is the issue:
So, the underlying issue appears to be ambiguity of what "hashlib.sha256" is. It can be hashlib/sha256.py , or "sha256" symbol from "hashlib". It should be the latter, but something glitches, exactly when "test" is imported.
With implementation like that, and test_hashlib.py appended with:
It prints:
So, doing it like that triggers some weird corner case. Anybody wants to get to the bottom of it? ;-)
Otherwise, hashlib.new() implementation is trivial, and if there's a separate test file for it, works as expected.
I think the problem is that the
from hashlib.sha256 import test as sha256_testimportshashliband setshashlib.sha256as the module, because it only imports from that submodule explicitly. A subsequentimport hashlib, which is supposed to trigger the__init__.pyto override thesha256attribute has no effect, becausehashlibalready is imported.Just a quick comment on that:
I was also thinking about implementing it like that. But I'm not so sure it's a good idea. Can the name string be trusted? What if it is a user/network supplied name?
CPython does it in a similar way, though. (But way more complicated). I'm not sure if that would be vulnerable, too.
So as to avoid calling random stuff, I hardcoded the dict of allowed functions in my
new().Apparently, importing any component of qualified module name should always process init.py if exists (this needs to be tested, actually, testcase for this added to uPy testsuite). If that doesn't happen, that's apparently a bug. If you can point to related code in the import implementation, then the issue is confirmed. Otherwise, someone else will need to confirm it.
You should validate anything like that yourself. Yet better, avoid using over-dynamic features like that.
Any Python app already can call random stuff. If the concern is app feeding random stuff to outside of it (like to stdlib function), then this vulnerability was covered in a more reliably way - by not providing function which may accept random stuff. CPython itself discourages using it: "The named constructors are much faster than new() and should be preferred." https://docs.python.org/3/library/hashlib.html#hashlib.new . Finally, the implementation was posted to show how it's trivial and that it doesn't have to be adhoc function in the module - it's just generic look up of an arbitrary class in a namespace. So, it's the same situation was with socket.error - it's not really needed. Of course, if people want it for compatibility with legacy code, it can be added. But doing "it in a similar way (But way more complicated)" doesn't buy much here IMHO, accepting that the best solution is just not using that function.
Ok, well. I'm not that interested in arguing whether this feature is needed or whether it should be implemented or not. The micropython-lib responsible developers will have to answer the question whether CPython compatibility is wanted here on their own. If new() is not implemented by micropython-lib, that is ok. I'm perfectly fine with either decision.
So I'll simply adapt my code regardless of the decision. That's pretty easy and works for both cases.
@mbuesch : Sounds good, please don't close this issue, sometimes more data/input, or even just time passed are needed to come to a decision.
Thanks, that was indeed needed, applied independently in https://github.com/pfalcon/micropython-lib/commit/f8ee045adbf7c3c44b2cc510283f8772a23c3cb3. Sorry for all the delays and lack of proper investigation in the first place (too many things to care about) :-(.