From eea8b7b51ecb91652bb53c2c875ea6c503ed5328 Mon Sep 17 00:00:00 2001 From: Andreas Oberritter Date: Wed, 30 Aug 2017 23:06:58 +0200 Subject: [PATCH 1/2] mifare_desfire_key: fix get/set_version for AES keys --- libfreefare/mifare_desfire_key.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/libfreefare/mifare_desfire_key.c b/libfreefare/mifare_desfire_key.c index 00e542c..3968cd9 100644 --- a/libfreefare/mifare_desfire_key.c +++ b/libfreefare/mifare_desfire_key.c @@ -114,6 +114,9 @@ mifare_desfire_key_get_version(MifareDESFireKey key) { uint8_t version = 0; + if (key->type == T_AES) + return key->aes_version; + for (int n = 0; n < 8; n++) { version |= ((key->data[n] & 1) << (7 - n)); } @@ -124,6 +127,11 @@ mifare_desfire_key_get_version(MifareDESFireKey key) void mifare_desfire_key_set_version(MifareDESFireKey key, uint8_t version) { + if (key->type == T_AES) { + key->aes_version = version; + return; + } + for (int n = 0; n < 8; n++) { uint8_t version_bit = ((version & (1 << (7 - n))) >> (7 - n)); key->data[n] &= 0xfe; From 67e7186d2b826bcf8542530c2bcd0e9c1f94e94f Mon Sep 17 00:00:00 2001 From: Robert Quattlebaum Date: Wed, 3 Jan 2018 21:00:09 -0800 Subject: [PATCH 2/2] Additional MifareDesfireKey fixes + tests In addition to adding tests for the bugs addressed via #70, this commit also addresses a key corruption bug that would occur on 3DES keys when `mifare_desfire_key_set_version()` was called. --- libfreefare/mifare_desfire_key.c | 29 ++++++++++++++--- test/test_mifare_desfire_key.c | 54 ++++++++++++++++++++++++++++++-- 2 files changed, 76 insertions(+), 7 deletions(-) diff --git a/libfreefare/mifare_desfire_key.c b/libfreefare/mifare_desfire_key.c index 3968cd9..7a0a9be 100644 --- a/libfreefare/mifare_desfire_key.c +++ b/libfreefare/mifare_desfire_key.c @@ -136,12 +136,33 @@ mifare_desfire_key_set_version(MifareDESFireKey key, uint8_t version) uint8_t version_bit = ((version & (1 << (7 - n))) >> (7 - n)); key->data[n] &= 0xfe; key->data[n] |= version_bit; - if (key->type == T_DES) { + switch (key->type) { + case T_DES: + // DESFire cards always treat DES keys as special cases of 2K3DES + // keys. The DESFire functional specification explicitly points + // out that if the subkeys of a 2K3DES key are exactly identical + // (including parity bits), then (and only then) is the key treated + // as a DES key for authentication purposes. Specifically, the + // version/parity bits must be idential, as well as the rest of the + // key, otherwise the PICC will treat it as a 2K3DES key. This + // next line ensure that. key->data[n + 8] = key->data[n]; - } else { - // Write ~version to avoid turning a 3DES key into a DES key + break; + case T_3DES: + // But what if we really did want the PICC to treat the key as a + // real 2K3DES key, even if the actual 56 bits of the subkeys did + // match? To ensure that such as case still works (largely because + // the datasheet implies authentication would behave differently + // otherwise), we need to ensure that the parity bits on the subkeys + // explicitly do not match. The easiest way to ensure that is to + // always write the bits of `~version` to the parity bits of the + // second subkey. Note that this would only have an effect at the + // PICC level if the subkeys were otherwise identical. key->data[n + 8] &= 0xfe; - key->data[n + 8] |= ~version_bit; + key->data[n + 8] |= !version_bit; + break; + default: + break; } } } diff --git a/test/test_mifare_desfire_key.c b/test/test_mifare_desfire_key.c index 5c428e4..3c00cf7 100644 --- a/test/test_mifare_desfire_key.c +++ b/test/test_mifare_desfire_key.c @@ -1,6 +1,7 @@ #include #include +#include "freefare_internal.h" void @@ -14,6 +15,8 @@ test_mifare_desfire_key(void) key = mifare_desfire_des_key_new(key1_des_data); version = mifare_desfire_key_get_version(key); cut_assert_equal_int(0x00, version, cut_message("Wrong MifareDESFireKey version")); + mifare_desfire_key_set_version(key, 0x55); + cut_assert_equal_memory(key1_des_data, sizeof(key1_des_data), key->data, sizeof(key1_des_data), cut_message("Version change corrupted key")); mifare_desfire_key_free(key); key = mifare_desfire_des_key_new_with_version(key1_des_data); @@ -22,6 +25,8 @@ test_mifare_desfire_key(void) mifare_desfire_key_set_version(key, 0xaa); version = mifare_desfire_key_get_version(key); cut_assert_equal_int(0xaa, version, cut_message("Wrong MifareDESFireKey version")); + mifare_desfire_key_set_version(key, 0x55); + cut_assert_equal_memory(key1_des_data, sizeof(key1_des_data), key->data, sizeof(key1_des_data), cut_message("Version change corrupted key")); mifare_desfire_key_free(key); @@ -38,11 +43,13 @@ test_mifare_desfire_key(void) mifare_desfire_key_free(key); - uint8_t key1_3des_data[16] = { 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0xAA, 0xBB, 0xCC, 0xDD, 0XEE, 0xFF }; + uint8_t key1_3des_data[16] = { 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x89, 0x98, 0xAB, 0xBA, 0xCD, 0xDC, 0xEF, 0xFE }; key = mifare_desfire_3des_key_new(key1_3des_data); version = mifare_desfire_key_get_version(key); cut_assert_equal_int(0x00, version, cut_message("Wrong MifareDESFireKey version")); + mifare_desfire_key_set_version(key, 0x55); + cut_assert_equal_memory(key1_3des_data, sizeof(key1_3des_data), key->data, sizeof(key1_3des_data), cut_message("Version change corrupted key")); mifare_desfire_key_free(key); key = mifare_desfire_3des_key_new_with_version(key1_3des_data); @@ -51,9 +58,11 @@ test_mifare_desfire_key(void) mifare_desfire_key_set_version(key, 0xaa); version = mifare_desfire_key_get_version(key); cut_assert_equal_int(0xaa, version, cut_message("Wrong MifareDESFireKey version")); + mifare_desfire_key_set_version(key, 0x55); + cut_assert_equal_memory(key1_3des_data, sizeof(key1_3des_data), key->data, sizeof(key1_3des_data), cut_message("Version change corrupted key")); mifare_desfire_key_free(key); - uint8_t key2_3des_data[16] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0X01, 0x00, 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77 }; + uint8_t key2_3des_data[16] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77 }; key = mifare_desfire_3des_key_new(key2_3des_data); version = mifare_desfire_key_get_version(key); @@ -65,7 +74,7 @@ test_mifare_desfire_key(void) cut_assert_equal_int(0x02, version, cut_message("Wrong MifareDESFireKey version")); mifare_desfire_key_free(key); - uint8_t key3_3des_data[16] = { 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0X00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x77 }; + uint8_t key3_3des_data[16] = { 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x77 }; key = mifare_desfire_3des_key_new(key3_3des_data); version = mifare_desfire_key_get_version(key); @@ -76,4 +85,43 @@ test_mifare_desfire_key(void) version = mifare_desfire_key_get_version(key); cut_assert_equal_int(0x10, version, cut_message("Wrong MifareDESFireKey version")); mifare_desfire_key_free(key); + + uint8_t key1_3k3des_data[24] = { 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08 }; + + key = mifare_desfire_3k3des_key_new(key1_3k3des_data); + version = mifare_desfire_key_get_version(key); + cut_assert_equal_int(0x00, version, cut_message("Wrong MifareDESFireKey version")); + mifare_desfire_key_set_version(key, 0x55); + cut_assert_equal_memory(key1_3k3des_data, sizeof(key1_3k3des_data), key->data, sizeof(key1_3k3des_data), cut_message("Version change corrupted key")); + mifare_desfire_key_free(key); + + key = mifare_desfire_3k3des_key_new_with_version(key1_3k3des_data); + version = mifare_desfire_key_get_version(key); + cut_assert_equal_int(0x55, version, cut_message("Wrong MifareDESFireKey version")); + mifare_desfire_key_set_version(key, 0xaa); + version = mifare_desfire_key_get_version(key); + cut_assert_equal_int(0xaa, version, cut_message("Wrong MifareDESFireKey version")); + mifare_desfire_key_set_version(key, 0x55); + cut_assert_equal_memory(key1_3k3des_data, sizeof(key1_3k3des_data), key->data, sizeof(key1_3k3des_data), cut_message("Version change corrupted key")); + mifare_desfire_key_free(key); + + + uint8_t key1_aes_data[16] = { 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF }; + + key = mifare_desfire_aes_key_new(key1_aes_data); + version = mifare_desfire_key_get_version(key); + cut_assert_equal_int(0x00, version, cut_message("Wrong MifareDESFireKey version")); + mifare_desfire_key_set_version(key, 0x55); + cut_assert_equal_memory(key1_aes_data, sizeof(key1_aes_data), key->data, sizeof(key1_aes_data), cut_message("Version change corrupted key")); + mifare_desfire_key_free(key); + + key = mifare_desfire_aes_key_new_with_version(key1_aes_data, 0x33); + version = mifare_desfire_key_get_version(key); + cut_assert_equal_int(0x33, version, cut_message("Wrong MifareDESFireKey version")); + mifare_desfire_key_set_version(key, 0xaa); + version = mifare_desfire_key_get_version(key); + cut_assert_equal_int(0xaa, version, cut_message("Wrong MifareDESFireKey version")); + mifare_desfire_key_set_version(key, 0x33); + cut_assert_equal_memory(key1_aes_data, sizeof(key1_aes_data), key->data, sizeof(key1_aes_data), cut_message("Version change corrupted key")); + mifare_desfire_key_free(key); }