mifare_desfire_key: fix get/set_version for AES keys #70
Conversation
There was a problem hiding this comment.
The current behavior looks odd indeed.
May I ask you add some simple unit-test that breaks without this change (and prove the code is wrong) and pass with the proposed patch? Extra point if you push the tests, Travis-CI complains that the code is broken, and then push this commit on top and Travis-CI is happy again 😉
|
No, but feel free to take my contribution as is. The defect is obvious. |
|
Is it really better to leave this bug in the code base than to take this simple fix for an obvious defect without a unit test? |
|
If this isn't resolved by the time I get a pull request for #77 submitted, I could also throw in a test for this as a part of that pull request. |
@darconeous please don't mix fixes in a single Pull-Request 😨 . Feel free to open a new PR that fix this issue, and reference this issue in the new Pull-Request… that way we do not forget to close one PR when merging the other one, and have a more readable code history 👍 . Thanks! |
|
No problem. |
In addition to adding tests for the bugs addressed via nfc-tools#70, this commit also addresses a key corruption bug that would occur on 3DES keys when `mifare_desfire_key_set_version()` was called.
|
#78 includes tests which validate this PR. |
In addition to adding tests for the bugs addressed via nfc-tools#70, this commit also addresses a key corruption bug that would occur on 3DES keys when `mifare_desfire_key_set_version()` was called.
In addition to adding tests for the bugs addressed via nfc-tools#70, this commit also addresses a key corruption bug that would occur on 3DES keys when `mifare_desfire_key_set_version()` was called.
In addition to adding tests for the bugs addressed via nfc-tools#70, this commit also addresses a key corruption bug that would occur on 3DES keys when `mifare_desfire_key_set_version()` was called.
In addition to adding tests for the bugs addressed via nfc-tools#70, this commit also addresses a key corruption bug that would occur on 3DES keys when `mifare_desfire_key_set_version()` was called.
In addition to adding tests for the bugs addressed via nfc-tools#70, this commit also addresses a key corruption bug that would occur on 3DES keys when `mifare_desfire_key_set_version()` was called.
In addition to adding tests for the bugs addressed via nfc-tools#70, this commit also addresses a key corruption bug that would occur on 3DES keys when `mifare_desfire_key_set_version()` was called.
|
Closing since #78 will be merged soon-ish! |
In addition to adding tests for the bugs addressed via nfc-tools#70, this commit also addresses a key corruption bug that would occur on 3DES keys when `mifare_desfire_key_set_version()` was called.
mifare_desfire_key_set_version incorrectly modified AES key bytes, while mifare_desfire_key_get_version returned some bits of the AES key.