From 66f9a6b51f4c3b60bc8d48bec25213d08db549d5 Mon Sep 17 00:00:00 2001 From: iceman1001 Date: Sun, 13 Sep 2026 12:58:46 +0200 Subject: [PATCH] hf mfdes: report a select/auth failure once, not twice DesfireSelectAndAuthenticate*() and its callers both printed the same error at different severities, so every failure came out as two lines. The core helper now owns the message and names the AID and the step; the 23 restatements in cmdhfmfdes.c are gone. Same duplicate pair fixed in cmdhfgallagher.c. Also: DesfireAuthErrorToStr() falls back to DesfireGetErrorString() for the PM3_E* codes the select paths pass through, which used to print an empty reason, and auth error 7 no longer reuses the text of error 1. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 1 + client/src/cmdhfgallagher.c | 4 ++-- client/src/cmdhfmfdes.c | 28 ----------------------- client/src/mifare/desfirecore.c | 39 ++++++++++++++++++++------------- 4 files changed, 27 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fed806f56..ac65259f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ All notable changes to this project will be documented in this file. This project uses the changelog in accordance with [keepchangelog](http://keepachangelog.com/). Please use this to write notable changes, which is not the same as git commit log... ## [unreleased][unreleased] +- Fixed `hf mfdes` - select / authenticate failures are now reported once, naming the AID and the failing step (@iceman1001) - Fixed `-f ` - a filename given with its own extension is now used as-is on both load and save (@iceman1001) - Fixed `hf mf view` - VIGIK signature verification now recovers the ISO 9796-1 message and checks it against the card (@iceman1001) - Added `hf mf view --selftest` - recovers and checks the signature of a known good expired VIGIK card (@iceman1001) diff --git a/client/src/cmdhfgallagher.c b/client/src/cmdhfgallagher.c index 7a94c6e06..c965d8acc 100644 --- a/client/src/cmdhfgallagher.c +++ b/client/src/cmdhfgallagher.c @@ -161,7 +161,7 @@ static int authenticate(DesfireContext_t *ctx, bool verbose) { int res = DesfireAuthenticate(ctx, DACEV1, false); if (res != PM3_SUCCESS) { - PrintAndLogEx(ERR, "Desfire authenticate " _RED_("error") ". Result: [%d] %s", + PrintAndLogEx(FAILED, "Desfire authentication " _RED_("failed") ". Result [%d] %s", res, DesfireAuthErrorToStr(res) ); @@ -426,7 +426,7 @@ static int hfgal_create_creds_app(DesfireContext_t *ctx, uint8_t *site_key, uint DesfireSetKeyNoClear(ctx, 0, T_AES, blank_key); DesfireSetKdf(ctx, MFDES_KDF_ALGO_NONE, NULL, 0); res = authenticate(ctx, verbose); - PM3_RET_IF_ERR_WITH_MSG(res, "Desfire authenticate error. Result: [%d] %s", res, DesfireAuthErrorToStr(res)); + PM3_RET_IF_ERR(res); // authenticate() already reported the reason // Change key DesfireSetCommMode(ctx, DCMEncryptedPlain); diff --git a/client/src/cmdhfmfdes.c b/client/src/cmdhfmfdes.c index 97df7ea5e..95aa11d35 100644 --- a/client/src/cmdhfmfdes.c +++ b/client/src/cmdhfmfdes.c @@ -2051,7 +2051,6 @@ static int CmdHF14aDesDetect(const char *Cmd) { res = DesfireSelectAndAuthenticateAppW(&dctx, securechann, selectway, id, true, verbose); if (res != PM3_SUCCESS) { DropField(); - PrintAndLogEx(FAILED, "Select or authentication %s " _RED_("failed") ". Result [%d] %s", DesfireWayIDStr(selectway, id), res, DesfireAuthErrorToStr(res)); return res; } @@ -2121,7 +2120,6 @@ static int CmdHF14aDesDetect(const char *Cmd) { res = DesfireSelectAndAuthenticateAppW(&dctx, securechann, selectway, id, true, verbose); if (res != PM3_SUCCESS) { DropField(); - PrintAndLogEx(FAILED, "Select or authentication %s " _RED_("failed") ". Result [%d] %s", DesfireWayIDStr(selectway, id), res, DesfireAuthErrorToStr(res)); return res; } } @@ -2141,7 +2139,6 @@ static int CmdHF14aDesDetect(const char *Cmd) { res = DesfireSelectAndAuthenticateAppW(&dctx, securechann, selectway, id, true, verbose); if (res != PM3_SUCCESS) { DropField(); - PrintAndLogEx(FAILED, "Select or authentication %s " _RED_("failed") " after the LRP probe. Result [%d] %s", DesfireWayIDStr(selectway, id), res, DesfireAuthErrorToStr(res)); return res; } } @@ -3385,7 +3382,6 @@ static int CmdHF14ADesBruteISOFIDs(const char *Cmd) { res = DesfireSelectAndAuthenticateW(&dctx, securechann, selectway, appid, false, 0, true, verbose); if (res != PM3_SUCCESS) { DropField(); - PrintAndLogEx(FAILED, "Select or authentication %s " _RED_("failed") ". Result [%d] %s", DesfireWayIDStr(selectway, appid), res, DesfireAuthErrorToStr(res)); return res; } @@ -3723,7 +3719,6 @@ static int CmdHF14ADesAuth(const char *Cmd) { res = DesfireSelectAndAuthenticateAppW(&dctx, securechann, selectway, id, false, verbose); if (res != PM3_SUCCESS) { DropField(); - PrintAndLogEx(FAILED, "Select or authentication %s " _RED_("failed") ". Result [%d] %s", DesfireWayIDStr(selectway, id), res, DesfireAuthErrorToStr(res)); return res; } @@ -3858,12 +3853,6 @@ static int CmdHF14ADesSetConfiguration(const char *Cmd) { res = DesfireSelectAndAuthenticateAppW(&dctx, securechann, selectway, id, false, verbose); if (res != PM3_SUCCESS) { DropField(); - PrintAndLogEx(FAILED, "Select or authentication ( %s ) Result [%d] %s %s", - DesfireWayIDStr(selectway, id), - res, - DesfireAuthErrorToStr(res), - _RED_("failed") - ); return res; } @@ -4000,7 +3989,6 @@ static int CmdHF14ADesChangeKey(const char *Cmd) { res = DesfireSelectAndAuthenticateAppW(&dctx, securechann, selectway, id, false, verbose); if (res != PM3_SUCCESS) { DropField(); - PrintAndLogEx(FAILED, "Select or authentication %s " _RED_("failed") ". Result [%d] %s", DesfireWayIDStr(selectway, id), res, DesfireAuthErrorToStr(res)); return res; } @@ -4742,7 +4730,6 @@ static int CmdHF14ADesGetUID(const char *Cmd) { res = DesfireSelectAndAuthenticateAppW(&dctx, securechann, selectway, id, false, verbose); if (res != PM3_SUCCESS) { DropField(); - PrintAndLogEx(FAILED, "Select or authentication %s " _RED_("failed") ". Result [%d] %s", DesfireWayIDStr(selectway, id), res, DesfireAuthErrorToStr(res)); return res; } @@ -5124,7 +5111,6 @@ static int CmdHF14ADesGetKeyVersions(const char *Cmd) { res = DesfireSelectAndAuthenticateAppW(&dctx, securechann, selectway, id, noauth, verbose); if (res != PM3_SUCCESS) { DropField(); - PrintAndLogEx(FAILED, "Select or authentication %s " _RED_("failed") ". Result [%d] %s", DesfireWayIDStr(selectway, id), res, DesfireAuthErrorToStr(res)); return res; } @@ -5434,7 +5420,6 @@ static int CmdHF14ADesGetFileIDs(const char *Cmd) { res = DesfireSelectAndAuthenticateAppW(&dctx, securechann, selectway, id, noauth, verbose); if (res != PM3_SUCCESS) { DropField(); - PrintAndLogEx(FAILED, "Select or authentication %s " _RED_("failed") ". Result [%d] %s", DesfireWayIDStr(selectway, id), res, DesfireAuthErrorToStr(res)); return res; } @@ -5511,7 +5496,6 @@ static int CmdHF14ADesGetFileISOIDs(const char *Cmd) { res = DesfireSelectAndAuthenticateAppW(&dctx, securechann, selectway, id, noauth, verbose); if (res != PM3_SUCCESS) { DropField(); - PrintAndLogEx(FAILED, "Select or authentication %s " _RED_("failed") ". Result [%d] %s", DesfireWayIDStr(selectway, id), res, DesfireAuthErrorToStr(res)); return res; } @@ -5593,7 +5577,6 @@ static int CmdHF14ADesGetFileSettings(const char *Cmd) { res = DesfireSelectAndAuthenticateAppW(&dctx, securechann, selectway, id, noauth, verbose); if (res != PM3_SUCCESS) { DropField(); - PrintAndLogEx(FAILED, "Select or authentication %s " _RED_("failed") ". Result [%d] %s", DesfireWayIDStr(selectway, id), res, DesfireAuthErrorToStr(res)); return res; } @@ -5796,7 +5779,6 @@ static int CmdHF14ADesChFileSettings(const char *Cmd) { res = DesfireSelectAndAuthenticateAppW(&dctx, securechann, selectway, id, noauth, verbose); if (res != PM3_SUCCESS) { DropField(); - PrintAndLogEx(FAILED, "Select or authentication %s " _RED_("failed") ". Result [%d] %s", DesfireWayIDStr(selectway, id), res, DesfireAuthErrorToStr(res)); return res; } @@ -6331,7 +6313,6 @@ static int CmdHF14ADesCreateTrMACFile(const char *Cmd) { res = DesfireSelectAndAuthenticateAppW(&dctx, securechann, selectway, id, noauth, verbose); if (res != PM3_SUCCESS) { DropField(); - PrintAndLogEx(FAILED, "Select or authentication %s " _RED_("failed") ". Result [%d] %s", DesfireWayIDStr(selectway, id), res, DesfireAuthErrorToStr(res)); return res; } @@ -6411,7 +6392,6 @@ static int CmdHF14ADesDeleteFile(const char *Cmd) { res = DesfireSelectAndAuthenticateAppW(&dctx, securechann, selectway, id, noauth, verbose); if (res != PM3_SUCCESS) { DropField(); - PrintAndLogEx(FAILED, "Select or authentication %s " _RED_("failed") ". Result [%d] %s", DesfireWayIDStr(selectway, id), res, DesfireAuthErrorToStr(res)); return res; } @@ -6519,7 +6499,6 @@ static int CmdHF14ADesValueOperations(const char *Cmd) { res = DesfireSelectAndAuthenticateAppW(&dctx, securechann, selectway, id, noauth, verbose); if (res != PM3_SUCCESS) { DropField(); - PrintAndLogEx(FAILED, "Select or authentication ( %s )" _RED_("failed") " Result [%d] %s", DesfireWayIDStr(selectway, id), res, DesfireAuthErrorToStr(res)); return res; } @@ -6724,7 +6703,6 @@ static int CmdHF14ADesClearRecordFile(const char *Cmd) { res = DesfireSelectAndAuthenticateAppW(&dctx, securechann, selectway, id, noauth, verbose); if (res != PM3_SUCCESS) { DropField(); - PrintAndLogEx(FAILED, "Select or authentication %s " _RED_("failed") ". Result [%d] %s", DesfireWayIDStr(selectway, id), res, DesfireAuthErrorToStr(res)); return res; } @@ -7256,7 +7234,6 @@ static int CmdHF14ADesCreateMFCMapping(const char *Cmd) { res = DesfireSelectAndAuthenticateAppW(&dctx, securechann, selectway, id, false, verbose); if (res != PM3_SUCCESS) { DropField(); - PrintAndLogEx(FAILED, "Select or authentication %s " _RED_("failed") ". Result [%d] %s", DesfireWayIDStr(selectway, id), res, DesfireAuthErrorToStr(res)); return res; } @@ -7684,7 +7661,6 @@ static int CmdHF14ADesReadData(const char *Cmd) { res = DesfireSelectAndAuthenticateW(&dctx, securechann, selectway, id, fileisoidpresent, fileisoid, noauth, verbose); if (res != PM3_SUCCESS) { DropField(); - PrintAndLogEx(FAILED, "Select or authentication %s " _RED_("failed") ". Result [%d] %s", DesfireWayIDStr(selectway, id), res, DesfireAuthErrorToStr(res)); return res; } @@ -7887,7 +7863,6 @@ static int CmdHF14ADesWriteData(const char *Cmd) { res = DesfireSelectAndAuthenticateW(&dctx, securechann, selectway, id, fileisoidpresent, fileisoid, noauth, verbose); if (res != PM3_SUCCESS) { DropField(); - PrintAndLogEx(FAILED, "Select or authentication %s " _RED_("failed") ". Result [%d] %s", DesfireWayIDStr(selectway, id), res, DesfireAuthErrorToStr(res)); return res; } @@ -8161,7 +8136,6 @@ static int CmdHF14ADesLsFiles(const char *Cmd) { res = DesfireSelectAndAuthenticateAppW(&dctx, securechann, selectway, id, noauth, verbose); if (res != PM3_SUCCESS) { DropField(); - PrintAndLogEx(FAILED, "Select or authentication %s " _RED_("failed") ". Result [%d] %s", DesfireWayIDStr(selectway, id), res, DesfireAuthErrorToStr(res)); return res; } @@ -8309,7 +8283,6 @@ static int CmdHF14ADesDump(const char *Cmd) { res = DesfireSelectAndAuthenticateAppW(&dctx, securechann, selectway, id, noauth, verbose); if (res != PM3_SUCCESS) { DropField(); - PrintAndLogEx(FAILED, "Select or authentication %s " _RED_("failed") ". Result [%d] %s", DesfireWayIDStr(selectway, id), res, DesfireAuthErrorToStr(res)); return res; } @@ -8329,7 +8302,6 @@ static int CmdHF14ADesDump(const char *Cmd) { res = DesfireSelectAndAuthenticateAppW(&dctx, securechann, selectway, id, noauth, verbose); if (res != PM3_SUCCESS) { DropField(); - PrintAndLogEx(FAILED, "Select or authentication %s " _RED_("failed") ". Result [%d] %s", DesfireWayIDStr(selectway, id), res, DesfireAuthErrorToStr(res)); return res; } } diff --git a/client/src/mifare/desfirecore.c b/client/src/mifare/desfirecore.c index 14ec978a4..23921a9af 100644 --- a/client/src/mifare/desfirecore.c +++ b/client/src/mifare/desfirecore.c @@ -258,7 +258,7 @@ const char *DesfireAuthErrorToStr(int error) { case 6: return "mbedtls_aes_setkey_enc failed"; case 7: - return "Sending auth command failed"; + return "Sending auth response (2nd frame) failed"; case 8: return "Authentication failed. Card timeout."; case 9: @@ -302,6 +302,13 @@ const char *DesfireAuthErrorToStr(int error) { default: break; } + + // the select / auth helpers hand PM3_E* codes straight back to the caller, + // so translate those here instead of returning an empty string + if (error < 0) { + return DesfireGetErrorString(error, NULL); + } + return ""; } @@ -327,7 +334,7 @@ char *DesfireWayIDStr(DesfireISOSelectWay way, uint32_t id) { if (way == ISWMF || way == ISWDFName) snprintf(str, sizeof(str), "%s", DesfireSelectWayToStr(way)); else - snprintf(str, sizeof(str), "%s %0*x", DesfireSelectWayToStr(way), (way == ISW6bAID) ? 6 : 4, id); + snprintf(str, sizeof(str), "%s %0*X", DesfireSelectWayToStr(way), (way == ISW6bAID) ? 6 : 4, id); return str; } @@ -1132,7 +1139,7 @@ int DesfireSelectAndAuthenticateEx(DesfireContext_t *dctx, DesfireSecureChannel if (aid == 0x000000) { res = DesfireAnticollision(verbose); if (res != PM3_SUCCESS) { - PrintAndLogEx(ERR, "Desfire anticollision " _RED_("fail")); + PrintAndLogEx(FAILED, "Desfire anticollision " _RED_("failed")); return 200; } @@ -1143,7 +1150,7 @@ int DesfireSelectAndAuthenticateEx(DesfireContext_t *dctx, DesfireSecureChannel } else { res = DesfireSelectAIDHex(dctx, aid, false, 0); if (res != PM3_SUCCESS) { - PrintAndLogEx(ERR, "Desfire select " _RED_("fail")); + PrintAndLogEx(FAILED, "Desfire %s select " _RED_("failed"), DesfireWayIDStr(ISW6bAID, aid)); return 200; } @@ -1160,15 +1167,16 @@ int DesfireSelectAndAuthenticateEx(DesfireContext_t *dctx, DesfireSecureChannel res = DesfireAuthenticate(dctx, secureChannel, verbose); if (res != PM3_SUCCESS) { - PrintAndLogEx(ERR, "Desfire authenticate " _RED_("fail") ". Result: [%d] %s", res, DesfireAuthErrorToStr(res)); + PrintAndLogEx(FAILED, "Desfire %s authentication " _RED_("failed") ". Result [%d] %s", DesfireWayIDStr(ISW6bAID, aid), res, DesfireAuthErrorToStr(res)); return res; } if (DesfireIsAuthenticated(dctx)) { if (verbose) { - PrintAndLogEx(INFO, "Desfire " _GREEN_("authenticated")); + PrintAndLogEx(INFO, "Desfire " _GREEN_("authenticated")); } } else { + PrintAndLogEx(FAILED, "Desfire %s authentication " _RED_("failed") ". Result [%d] %s", DesfireWayIDStr(ISW6bAID, aid), 201, DesfireAuthErrorToStr(201)); return 201; } } @@ -1195,7 +1203,7 @@ int DesfireSelectAndAuthenticateW(DesfireContext_t *dctx, DesfireSecureChannel s res = DesfireISOSelect(dctx, ISSDFName, dctx->selectedDFName, dctx->selectedDFNameLen, resp, &resplen); if (res != PM3_SUCCESS) { - PrintAndLogEx(ERR, "Desfire DF name select " _RED_("error")); + PrintAndLogEx(FAILED, "Desfire DF name select " _RED_("failed")); if (res == PM3_ECARDEXCHANGE || res == PM3_ETIMEOUT || res == PM3_ERFTRANS) { return res; } @@ -1216,7 +1224,7 @@ int DesfireSelectAndAuthenticateW(DesfireContext_t *dctx, DesfireSecureChannel s res = DesfireSelectAIDHex(dctx, id, false, 0); if (res != PM3_SUCCESS) { - PrintAndLogEx(ERR, "Desfire select " _RED_("error")); + PrintAndLogEx(FAILED, "Desfire %s select " _RED_("failed"), DesfireWayIDStr(ISW6bAID, id)); if (res == PM3_ECARDEXCHANGE || res == PM3_ETIMEOUT || res == PM3_ERFTRANS) { return res; } @@ -1232,7 +1240,7 @@ int DesfireSelectAndAuthenticateW(DesfireContext_t *dctx, DesfireSecureChannel s } else { res = DesfireSelectEx(dctx, false, way, id, NULL); if (res != PM3_SUCCESS) { - PrintAndLogEx(ERR, "Desfire %s select " _RED_("error"), DesfireSelectWayToStr(way)); + PrintAndLogEx(FAILED, "Desfire %s select " _RED_("failed"), DesfireWayIDStr(way, id)); if (res == PM3_ECARDEXCHANGE || res == PM3_ETIMEOUT || res == PM3_ERFTRANS) { return res; } @@ -1247,7 +1255,7 @@ int DesfireSelectAndAuthenticateW(DesfireContext_t *dctx, DesfireSecureChannel s // Also select by ISO ID if specified res = DesfireSelectEx(dctx, false, way, id, NULL); if (res != PM3_SUCCESS) { - PrintAndLogEx(ERR, "Desfire %s select " _RED_("error"), DesfireSelectWayToStr(way)); + PrintAndLogEx(FAILED, "Desfire %s select " _RED_("failed"), DesfireWayIDStr(way, id)); if (res == PM3_ECARDEXCHANGE || res == PM3_ETIMEOUT || res == PM3_ERFTRANS) { return res; } @@ -1266,7 +1274,7 @@ int DesfireSelectAndAuthenticateW(DesfireContext_t *dctx, DesfireSecureChannel s res = DesfireSelectAIDHex(dctx, id, false, 0); if (res != PM3_SUCCESS) { - PrintAndLogEx(ERR, "Desfire select " _RED_("error")); + PrintAndLogEx(FAILED, "Desfire %s select " _RED_("failed"), DesfireWayIDStr(ISW6bAID, id)); if (res == PM3_ECARDEXCHANGE || res == PM3_ETIMEOUT || res == PM3_ERFTRANS) { return res; } @@ -1282,7 +1290,7 @@ int DesfireSelectAndAuthenticateW(DesfireContext_t *dctx, DesfireSecureChannel s } else { res = DesfireSelectEx(dctx, true, way, id, NULL); if (res != PM3_SUCCESS) { - PrintAndLogEx(ERR, "Desfire %s select " _RED_("error"), DesfireSelectWayToStr(way)); + PrintAndLogEx(FAILED, "Desfire %s select " _RED_("failed"), DesfireWayIDStr(way, id)); if (res == PM3_ECARDEXCHANGE || res == PM3_ETIMEOUT || res == PM3_ERFTRANS) { return res; } @@ -1296,7 +1304,7 @@ int DesfireSelectAndAuthenticateW(DesfireContext_t *dctx, DesfireSecureChannel s if (selectfile) { res = DesfireSelectEx(dctx, false, ISWIsoID, isofileid, NULL); if (res != PM3_SUCCESS) { - PrintAndLogEx(ERR, "Desfire iso file select " _RED_("error")); + PrintAndLogEx(FAILED, "Desfire %s iso file %04x select " _RED_("failed"), DesfireWayIDStr(way, id), isofileid); if (res == PM3_ECARDEXCHANGE || res == PM3_ETIMEOUT || res == PM3_ERFTRANS) { return res; } @@ -1311,15 +1319,16 @@ int DesfireSelectAndAuthenticateW(DesfireContext_t *dctx, DesfireSecureChannel s if (noauth == false) { res = DesfireAuthenticate(dctx, secureChannel, verbose); if (res != PM3_SUCCESS) { - PrintAndLogEx(ERR, "Desfire authenticate " _RED_("error") ". Result: [%d] %s", res, DesfireAuthErrorToStr(res)); + PrintAndLogEx(FAILED, "Desfire %s authentication " _RED_("failed") ". Result [%d] %s", DesfireWayIDStr(way, id), res, DesfireAuthErrorToStr(res)); return res; } if (DesfireIsAuthenticated(dctx)) { if (verbose) { - PrintAndLogEx(INFO, "Desfire " _GREEN_("authenticated")); + PrintAndLogEx(INFO, "Desfire " _GREEN_("authenticated")); } } else { + PrintAndLogEx(FAILED, "Desfire %s authentication " _RED_("failed") ". Result [%d] %s", DesfireWayIDStr(way, id), 201, DesfireAuthErrorToStr(201)); return 201; } }