Bug #5060
closedmandatory Crypto module: pycrypto or pycryptodome?
100%
Description
Hi,
With last master version, I have an issue regarding Crypto.AES when writing an USIM (with Ki and Op), using pycrypto or pycryptodome:
./pySim-prog.py -p 0 -a xxxx -x xxx -y xxx -i xxxx -s xxx --op=xxx -k xxxx --acc=xxx -n xxx
- With pycrypto, the error is:
Card programming failed with an execption:
---------------------8<---------------------
Traceback (most recent call last):
File "/tmp/pysim/pySim-prog.py", line 750, in <module>
rc = process_card(opts, first, card_handler)
File "/tmp/pysim/pySim-prog.py", line 673, in process_card
cp = gen_parameters(opts)
File "/tmp/pysim/pySim-prog.py", line 445, in gen_parameters
opc = derive_milenage_opc(ki, opts.op)
File "/tmp/pysim/pySim/utils.py", line 247, in derive_milenage_opc
aes = AES.new(h2b(ki_hex))
File "/home/vagrant/.virtualenvs/py3-ansible2.9/lib/python3.8/site-packages/Crypto/Cipher/AES.py", line 95, in new
return AESCipher(key, *args, **kwargs)
File "/home/vagrant/.virtualenvs/py3-ansible2.9/lib/python3.8/site-packages/Crypto/Cipher/AES.py", line 59, in __init__
blockalgo.BlockAlgo.__init__(self, _AES, key, *args, **kwargs)
File "/home/vagrant/.virtualenvs/py3-ansible2.9/lib/python3.8/site-packages/Crypto/Cipher/blockalgo.py", line 141, in __init__
self._cipher = factory.new(key, *args, **kwargs)
TypeError: argument 1 must be read-only bytes-like object, not bytearray
- With pycryptodome, the error is:
Ready for Programming: Insert card now (or CTRL-C to cancel)
Autodetected card type: sysmoUSIM-SJS1
Card programming failed with an execption:
---------------------8<---------------------
Traceback (most recent call last):
File "/tmp/pysim/pySim-prog.py", line 750, in <module>
rc = process_card(opts, first, card_handler)
File "/tmp/pysim/pySim-prog.py", line 673, in process_card
cp = gen_parameters(opts)
File "/tmp/pysim/pySim-prog.py", line 445, in gen_parameters
opc = derive_milenage_opc(ki, opts.op)
File "/tmp/pysim/pySim/utils.py", line 247, in derive_milenage_opc
aes = AES.new(h2b(ki_hex))
TypeError: new() missing 1 required positional argument: 'mode'
---------------------8<---------------------
Programming failed: Remove card from reader
In both case, it doesnt work !
Updated by laforge about 3 years ago
- Status changed from New to In Progress
- Assignee set to laforge
- Priority changed from Normal to High
- % Done changed from 0 to 10
Updated by laforge about 3 years ago
On Fri, Mar 05, 2021 at 04:35:20PM +0000, chertault [REDMINE] wrote:
With last master version, I have an issue regarding Crypto.AES when writing an USIM (with Ki and Op), using pycrypto or pycryptodome:
Thanks, I guess nobody has tested that part yet with recent commits.
I suppose if you go back to 85484a977d46c9fa27c05b7a81ee02c5c1dffa35 or revert
4f6ca43e1f6726f00cfc91ff6d17db6878316c4d it works again?
Thanks for your help.
Updated by laforge about 3 years ago
Note to dexter: We need to include a test that executes the OPc-derivation from OP in our test suite, that way we'd have catched this bug before a merge.
I'm not much of a python programmer, and this dualism of bytearray vs. bytes confuses the hell out of me. bytes() is read-only, and byte-array is writeable. So if I pass a bytearray in a place where bytes is expected, why on earth does a dynamically typed language not simply peform the conversion from writable to read-only type?
Updated by laforge about 3 years ago
- % Done changed from 10 to 70
- https://gerrit.osmocom.org/c/pysim/+/23258 fix TypeError in derive_milenage_opc() [NEW]
- https://gerrit.osmocom.org/c/pysim/+/23259 pySim/utils.py: Attempt to support pycryptodpme
Please let me know if both of those work for you, especially testing with pycryptodome is appreciated.
Updated by laforge about 3 years ago
- Status changed from In Progress to Feedback
- Assignee changed from laforge to chertault
- % Done changed from 70 to 90
patches merged to master now. waiting for feedback from reporter.
Updated by laforge about 3 years ago
- Status changed from Feedback to Resolved
- % Done changed from 90 to 100
Applied in changeset pysim|eab8d2adf772e0da8c363ec0f68c155797675eca.
Updated by chertault about 3 years ago
Hi,
I succeed to prog SIM this morning with last commits, with pycrytodome or pycrypto, thank you !
Updated by chertault about 3 years ago
Just a remark, could you use tags to define stable version? as we use an automation program to read/write SIMs, we get pysim program each time, think we need to be sure that pointing on tag X.X, pysim will work. Thanks.
Updated by laforge about 3 years ago
On Mon, Mar 08, 2021 at 08:31:48AM +0000, chertault [REDMINE] wrote:
Just a remark, could you use tags to define stable version?
In general, we have quite extensive automatic testing with all supported
sim card types in place that ensures no commit can be merged without
passing the tests. It's just that one (for us rather exotic feature) of
deriving OPC from OP+K is not covered by the tests. I've already asked
dexter to add that, so we improve the coverage.
Generally, it should be safe as we use an automation program to
read/write SIMs, we get pysim program each time, think we need to be
sure that pointing on tag X.X, pysim will work. Thanks.
I've just pushed an '1.0' tag for a version that predates some of the
higher-risk changes of recent weeks where we are introducing support
for pysim-shell.
Having said that, there is of course no guarantee that this tag of a
random version has less bugs than master. It has not undergone more
testing than every commit in master.
As a community-run collaborative open source project We're always happy
to receive contributions. I'd be very happy to hear if you'd be
interested in improving test coverage, or somebody volunteered to
maintain "stable releases".
