Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WRONG_PASSWORD after accessing Nitrokey with scdrand #81

Open
robinkrahl opened this issue May 2, 2021 · 10 comments
Open

WRONG_PASSWORD after accessing Nitrokey with scdrand #81

robinkrahl opened this issue May 2, 2021 · 10 comments

Comments

@robinkrahl
Copy link

First reported by @bircoph against nitrocli, see d-e-s-o/nitrocli#175.

Steps to reproduce:

  1. Set 20-chars users password and use it (e.g. nitrocli pws get).
  2. Disconnect and reconnect device.
  3. Run scdrand 4096
  4. Using nitrocli again (e.g. nitrocli pws get) causes a wrong password error
  5. After the initial failure, subsequent calls succeed.

This could not be reproduced with the default passphrase 123456. Instead of calling nitrocli, the issue could also be reproduced by executing this minimal example:

#include <stdio.h>
#include <stdlib.h>
#include <libnitrokey/NK_C_API.h>

static const char *USER_PIN = "12345678901234567890";

int main(void)
{
	NK_set_debug(true);
	if (NK_login_auto() != 1) {
		fprintf(stderr, "No Nitrokey device found.\n");
		return 1;
	}

	int result = NK_enable_password_safe(USER_PIN);
	if (result) {
		fprintf(stderr, "Failed to enable password safe: %d\n", result);
		return 1;
	}

	char *slot_name = NK_get_password_safe_slot_name(1);
	if (!slot_name) {
		fprintf(stderr, "Failed to query PWS slot: %d\n",
				NK_get_last_command_status());
		return 1;
	}
	printf("Slot 1: '%s'\n", slot_name);
	free(slot_name);

	NK_logout();
	return 0;
}

Log for step 4:

$ ./nitrokey
[Sun May  2 22:06:32 2021][DEBUG_L1]    Connection success: 1 ()
[Sun May  2 22:06:32 2021][DEBUG_L1]    Connection success: 0 ()
[Sun May  2 22:06:32 2021][DEBUG_L1]    Connection success: 0 ()
[Sun May  2 22:06:32 2021][DEBUG_L1]    Disconnection: handle already freed: 1 ()
[Sun May  2 22:06:32 2021][DEBUG_L1]    Disconnection: handle already freed: 1 ()
[Sun May  2 22:06:32 2021][DEBUG]       -------------------
[Sun May  2 22:06:32 2021][DEBUG]       Outgoing HID packet:
[Sun May  2 22:06:32 2021][DEBUG]       Contents:
Command ID:     DETECT_SC_AES
CRC:    26769a05
Payload:
 user_password: ***********

[Sun May  2 22:06:32 2021][DEBUG_L1]    => DETECT_SC_AES
[Sun May  2 22:06:32 2021][DEBUG_L1]    <= DETECT_SC_AES 0 0
[Sun May  2 22:06:32 2021][DEBUG]       Incoming HID packet:
[Sun May  2 22:06:32 2021][DEBUG]       Device status:  0 OK
Command ID:     DETECT_SC_AES hex: 6a
Last command CRC:       26769a05
Last command status:    4 STICK10::COMMAND_STATUS::WRONG_PASSWORD
CRC:    3e98932b
Payload:
Empty Payload.
[Sun May  2 22:06:32 2021][DEBUG_L1]    Throw: CommandFailedException 4
[Sun May  2 22:06:32 2021][DEBUG]       CommandFailedException, status: 4
Failed to enable password safe: 4

Log for step 5:

./nitrokey
[Sun May  2 22:06:35 2021][DEBUG_L1]    Connection success: 1 ()
[Sun May  2 22:06:35 2021][DEBUG_L1]    Connection success: 0 ()
[Sun May  2 22:06:35 2021][DEBUG_L1]    Connection success: 0 ()
[Sun May  2 22:06:35 2021][DEBUG_L1]    Disconnection: handle already freed: 1 ()
[Sun May  2 22:06:35 2021][DEBUG_L1]    Disconnection: handle already freed: 1 ()
[Sun May  2 22:06:35 2021][DEBUG]       -------------------
[Sun May  2 22:06:35 2021][DEBUG]       Outgoing HID packet:
[Sun May  2 22:06:35 2021][DEBUG]       Contents:
Command ID:     DETECT_SC_AES
CRC:    26769a05
Payload:
 user_password: ***********

[Sun May  2 22:06:35 2021][DEBUG_L1]    => DETECT_SC_AES
.
[Sun May  2 22:06:36 2021][DEBUG_L1]    <= DETECT_SC_AES 0 0
[Sun May  2 22:06:36 2021][DEBUG]       Incoming HID packet:
[Sun May  2 22:06:36 2021][DEBUG]       Device status:  0 OK
Command ID:     DETECT_SC_AES hex: 6a
Last command CRC:       26769a05
Last command status:    0 STICK10::COMMAND_STATUS::OK
CRC:    5f7699f2
Payload:
Empty Payload.
[Sun May  2 22:06:36 2021][DEBUG]       -------------------
[Sun May  2 22:06:36 2021][DEBUG]       Outgoing HID packet:
[Sun May  2 22:06:36 2021][DEBUG]       Contents:
Command ID:     PW_SAFE_ENABLE
CRC:    3520f28d
Payload:
 user_password: ***********

Sun May  2 22:06:36 2021][DEBUG_L1]    => PW_SAFE_ENABLE
.
[Sun May  2 22:06:36 2021][DEBUG_L1]    <= PW_SAFE_ENABLE 0 0
[Sun May  2 22:06:36 2021][DEBUG]       Incoming HID packet:
[Sun May  2 22:06:36 2021][DEBUG]       Device status:  0 OK
Command ID:     PW_SAFE_ENABLE hex: 67
Last command CRC:       3520f28d
Last command status:    0 STICK10::COMMAND_STATUS::OK
CRC:    75bb48a7
Payload:
Empty Payload.
[Sun May  2 22:06:36 2021][DEBUG]       -------------------
[Sun May  2 22:06:36 2021][DEBUG]       Outgoing HID packet:
[Sun May  2 22:06:36 2021][DEBUG]       Contents:
Command ID:     GET_PW_SAFE_SLOT_NAME
CRC:    6a1f2ce2
Payload:
slot_number     1

[Sun May  2 22:06:36 2021][DEBUG_L1]    => GET_PW_SAFE_SLOT_NAME
[Sun May  2 22:06:36 2021][DEBUG_L1]    <= GET_PW_SAFE_SLOT_NAME 0 0
[Sun May  2 22:06:36 2021][DEBUG]       Incoming HID packet:
[Sun May  2 22:06:36 2021][DEBUG]       Device status:  0 OK
Command ID:     GET_PW_SAFE_SLOT_NAME hex: 61
Last command CRC:       6a1f2ce2
Last command status:    0 STICK10::COMMAND_STATUS::OK
CRC:    37eebaf4
Payload:
 slot_name:     ***********

Slot 1: 'xxx'

Device information:

$ nitrocli status
Status:
  model:             Nitrokey Pro
  serial number:     XXXXXXXX
  firmware version:  v0.10
  user retry count:  3
  admin retry count: 3
@szszszsz
Copy link
Member

szszszsz commented May 3, 2021

Thank you for the detailed report!
Looking at the details of the DETECT_AES it seems that it accesses the smart card locally (to verify AES support; see below excerpt), and seems to be happening while the scdrand is still using it. This causes the familiar issue with the concurrent smart card access, which was not solved yet in this project.
Potentially the DETECT_AES call could be removed, as all models were sold with the AES support and this is rather for historic reason, but even so later calls using smart card internally would fail similarly, hence that would not improve the situation.
This has to be solved in the firmware.

Connected: #54

uint8_t isAesSupported (void)
{
InitSCTStruct (&tSCT);
CcidSelectOpenPGPApp ();
unsigned short cRet;
tSCT.cAPDU[CCID_CLA] = 0x00;
tSCT.cAPDU[CCID_INS] = 0x2A;
tSCT.cAPDU[CCID_P1] = 0x80;
tSCT.cAPDU[CCID_P2] = 0x86;
tSCT.cAPDU[CCID_LC] = 17;
char test_data[17] = { 0x02, // Use AES to DECIPHER
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
};
memcpy ((char *) &tSCT.cAPDU[CCID_DATA], test_data, 17);
tSCT.cAPDU[CCID_DATA + 17] = 0;
tSCT.cAPDU[CCID_DATA + 18] = 0;
tSCT.cAPDULength = 5 + 17;
cRet = SendAPDU (&tSCT);
/* Possible answers and intepretations: * * APDU_ANSWER_COMMAND_CORRECT (SW1=90, SW2=00) * Successfully used AES module to decipher => AES module
exists * * APDU_ANSWER_REF_DATA_NOT_FOUND (SW1=6A, SW2=88) * AES key not found => AES module exists, and the error is at the non existing key *
* APDU_ANSWER_USE_CONDIT_NOT_SATISFIED (SW1=69, SW2=85) * AES module doen't exist * */
// Determine if AES module exists
if ((APDU_ANSWER_COMMAND_CORRECT == cRet) || (APDU_ANSWER_REF_DATA_NOT_FOUND == cRet))
return TRUE;
else
return FALSE;
}

@szszszsz
Copy link
Member

szszszsz commented May 3, 2021

Moving this ticket to Nitrokey Pro firmware project.

@szszszsz szszszsz transferred this issue from Nitrokey/libnitrokey May 3, 2021
@robinkrahl
Copy link
Author

Thanks for looking into this! I think this is a different issue than the GnuPG problem because:

  • The GnuPG problem occurs because GnuPG makes assumptions about the smart card state – the Nitrokey side is working fine. In this case, the problem occurs only on the Nitrokey.
  • The GnuPG problem results in a hanging Nitrokey device, while here only the first command invocation after scdrand is affected. (The Nitrokey device does not have to be reconnected to fix the issue.)
  • This issue only occurs with a 20-character password.

Also, scdrand terminated before I tried to access the Nitrokey. I can test if increasing the delay fixes the issue.

@szszszsz
Copy link
Member

szszszsz commented May 3, 2021

I agree, this is not exactly the same - let me elaborate on my understanding of the cause.
After a brief reading (without looking into the debug log, so I can be mistaken) I have assumed here that firmware is not resetting the smart card, which is in an unknown state once the execution starts. So I believe this time Nitrokey Pro does some assumptions. It seems to run the proper reset procedure at the end though, since consecutive call is working.
Not handling the initial smart card reset falls for me into the concurrent access problems basket, which is why I have linked the mentioned ticket. This project needs to improve on generic handling of the context reset / switching of the smart card, as well as proper communication orchestration and interleaving / mutexing.
Saying this, it's not an easy problem. Ideal solution would be probably to handle the smart card sessions, interpret and organize incoming CCID calls into a queue using a command pattern, and synchronize it all with the local access.

Perhaps it would be best to workaround it by signalizing to host the smart card ejection before running any local access, then restoring it back afterwards. This should clear the cache on the host as well. It would be nice to finish the current CCID host-sourced tasks first before doing so, so the exclusion would not be forced. That should fix the current issues, but may introduce some side-effects.

The PIN length being a factor is not exactly fitting in my theory though. The 14-bytes difference should not make the call divided into multiple parts.

Let me know what you think.

@robinkrahl
Copy link
Author

Makes sense, thanks for the explanation!

@robinkrahl
Copy link
Author

The 14-bytes difference should not make the call divided into multiple parts.

By the way, even a 19-character passphrase did not trigger the issue.

@szszszsz
Copy link
Member

szszszsz commented May 3, 2021

The 14-bytes difference should not make the call divided into multiple parts.

By the way, even a 19-character passphrase did not trigger the issue.

This starts to sound like a buffer overflow or missing a \0 character in a C string. I saw though firmware side it handles that up to 25+1 characters. Perhaps worth rechecking.

@szszszsz
Copy link
Member

szszszsz commented May 3, 2021

@robinkrahl do attempt counters change after executing the test?

@robinkrahl
Copy link
Author

@robinkrahl do attempt counters change after executing the test?

According to the initial bug report, no. I haven’t checked that myself.

@szszszsz
Copy link
Member

szszszsz commented May 3, 2021

I do not see anything in the firmware regarding the PIN validation so far. To check with the actual debugging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants