Responses for certain commands contain wrong CRC #32

d-e-s-o · 2017-03-31T04:22:09Z

Certain command responses have a CRC value in them that does not match the actual content. See for instance here where this has been mentioned for the send password case. I have my own library and see the same issue happening for the response of the unlock encrypted volume command.

Can this please be looked into? That is no state to work with. For that reason libnitrokey essentially cuts down on CRC checks completely which circumvents the entire concept of providing such check codes in the first place.

szszszsz · 2017-03-31T07:51:33Z

Just to keep things straight - you have linked an outdated fork, here is the latest version: link. It is the same regarding CRC though.
Invalid CRCs are returned while in BUSY status (not so much important as with OK status, but still an inconsistency) and for SENDPASSWORD command (this should be fixed indeed). I do not remember more faulty CRC cases. In case you have found any other please mention here.

FACTORY_RESET seems to not set the CRC code too.

d-e-s-o · 2017-04-08T15:23:32Z

Thanks for the correction, Szczepan. What about DISABLE_CRYPTED_PARI, have you tried it?

The nitrokey embeds a wrong CRC code into the response for certain commands. When closing the encrypted volume, for example, the CRC code never matches the actual report data. This problem is tracked by issue 32: Nitrokey/nitrokey-storage-firmware#32 To work around this problem for now we unfortunately have to ignore the result of the CRC check. This change adjusts the code to do that.

szszszsz · 2017-04-10T17:52:00Z

For DISABLE_CRYPTED_PARI: it sets the CRC sum and it seems to be correct (log below, libnitrokey, Windows 8.1). Do you have other experiences?

[DEBUG] [04/10/17 19:43:04 Central European Summer Time]        Outgoing HID packet:
[DEBUG] [04/10/17 19:43:04 Central European Summer Time]        Raw HID packet:
0000    00 21 50 00 00 00 00 00 00 00 00 00 00 00 00 00         .!P.............
0010    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00         ................
0020    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00         ................
0030    00 00 00 00 00 00 00 00 00 00 00 00 00 2b 88 3a         .............+.:
0040    8b -- -- -- -- -- -- -- -- -- -- -- -- -- -- --         .
Contents:
Command ID:     DISABLE_CRYPTED_PARI
CRC:    8b3a882b
Payload:
 kind:  P
 password:

(...)
[DEBUG_L2] [04/10/17 19:43:04 Central European Summer Time]     Detected storage device cmd, status: 1
[DEBUG] [04/10/17 19:43:04 Central European Summer Time]        Incoming HID packet:
[DEBUG] [04/10/17 19:43:04 Central European Summer Time]        Raw HID packet:
0000    00 00 21 2b 88 3a 8b 00 00 00 00 00 00 00 00 00         ..!+.:..........
0010    00 00 00 00 00 0c 21 01 00 00 00 00 00 00 00 00         ......!.........
0020    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00         ................
0030    00 00 00 00 00 00 00 00 00 00 00 00 00 9e 26 f5         ..............&.
0040    2e -- -- -- -- -- -- -- -- -- -- -- -- -- -- --         .
Device status:  0 OK
Command ID:     DISABLE_CRYPTED_PARI hex: 21
Last command CRC:       8b3a882b
Last command status:    0 STICK10::COMMAND_STATUS::OK
CRC:    2ef5269e
Storage stick status (where applicable):
 pod.storage_status.command_counter:    0c
 pod.storage_status.command_id:         21
 pod.storage_status.device_status:      01
 pod.storage_status.progress_bar_value:         00
Payload:
Empty Payload.
[DEBUG] [04/10/17 19:43:04 Central European Summer Time]        receiving_retry_counter count: 19

d-e-s-o · 2017-04-13T03:50:39Z

The CRC may seem correct but it is not.

[DEBUG] Outgoing HID packet:
[DEBUG] Raw HID packet:
0000    00 21 50 00 00 00 00 00 00 00 00 00 00 00 00 00         .!P.............
0010    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00         ................
0020    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00         ................
0030    00 00 00 00 00 00 00 00 00 00 00 00 00 2b 88 3a         .............+.:
0040    8b -- -- -- -- -- -- -- -- -- -- -- -- -- -- --         .
Contents:
Command ID:     DISABLE_CRYPTED_PARI
CRC:    8b3a882b
Payload:
 kind:  P
 password:

[DEBUG] Incoming HID packet:
[DEBUG] Raw HID packet:
0000    00 00 21 2b 88 3a 8b 00 00 00 00 00 00 00 00 00         ..!+.:..........
0010    00 00 00 00 00 07 21 01 00 00 00 00 00 00 00 00         ......!.........
0020    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00         ................
0030    00 00 00 00 00 00 00 00 00 00 00 00 00 f7 1b 76         ...............v
0040    68 -- -- -- -- -- -- -- -- -- -- -- -- -- -- --         h
Device status:  0 OK
Command ID:     DISABLE_CRYPTED_PARI hex: 21
Last command CRC:       8b3a882b
Last command status:    0 STICK10::COMMAND_STATUS::OK
CRC:    68761bf7
Storage stick status:
 pod.storage_status.command_counter:    07
 pod.storage_status.command_id:         21
 pod.storage_status.device_status:      01
 pod.storage_status.progress_bar_value:         00
Payload:
Empty Payload.

Take the following program:

static uint32_t _crc32(uint32_t crc, uint32_t data) {
  int i;
  crc = crc ^ data;

  for (i = 0; i < 32; i++) {
    if (crc & 0x80000000)
      crc = (crc << 1) ^ 0x04C11DB7;  // polynomial used in STM32
    else
      crc = (crc << 1);
  }

  return crc;
}

uint32_t stm_crc32(const uint8_t *data, size_t size) {
  uint32_t crc = 0xffffffff;
  const uint32_t *pend = (const uint32_t *)(data + size);
  for (const uint32_t *p = (const uint32_t *)(data); p < pend; p++)
    crc = _crc32(crc, *p);
  return crc;
}


int main()
{
  // Disable encrypted part.
  unsigned char const data[] = {0x00, 0x21, 0x2b, 0x88, 0x3a, 0x8b,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x07, 0x21, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 };

  std::cout << "size: " << sizeof(data) << std::endl;
  std::cout << std::hex << "0x" << stm_crc32((const uint8_t*)data, sizeof(data)) << std::endl;
}

$ ./test
size: 60
0xfd3563e6 // that's the correct CRC and it does not match the reported one.

Now let's double check that with a command producing the correct CRC:

[DEBUG] Outgoing HID packet:
[DEBUG] Raw HID packet:
0000    00 0f 00 00 00 00 00 00 00 00 00 00 00 00 00 00         ................
0010    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00         ................
0020    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00         ................
0030    00 00 00 00 00 00 00 00 00 00 00 00 00 6e 5f 59         .............n_Y
0040    5e -- -- -- -- -- -- -- -- -- -- -- -- -- -- --         ^
Contents:
Command ID:     GET_USER_PASSWORD_RETRY_COUNT
CRC:    5e595f6e
Payload:
Empty Payload.
[DEBUG] Incoming HID packet:
[DEBUG] Raw HID packet:
0000    00 00 0f 6e 5f 59 5e 00 03 00 00 00 00 00 00 00         ...n_Y^.........
0010    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00         ................
0020    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00         ................
0030    00 00 00 00 00 00 00 00 00 00 00 00 00 95 a8 86         ................
0040    db -- -- -- -- -- -- -- -- -- -- -- -- -- -- --         .
Device status:  0 OK
Command ID:     GET_USER_PASSWORD_RETRY_COUNT hex: f
Last command CRC:       5e595f6e
Last command status:    0 STICK10::COMMAND_STATUS::OK
CRC:    db86a895
Storage stick status:
 pod.storage_status.command_counter:    00
 pod.storage_status.command_id:         00
 pod.storage_status.device_status:      00
 pod.storage_status.progress_bar_value:         00
Payload:
 password_retry_count   3

int main()
{
  // Get user pin.
  unsigned char const data[] = {0x00, 0x0f, 0x6e, 0x5f, 0x59, 0x5e,
    0x00, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};

  std::cout << "size: " << sizeof(data) << std::endl;
  std::cout << std::hex << "0x" << stm_crc32((const uint8_t*)data, sizeof(data)) << std::endl;
}

./test
size: 60
0xdb86a895 // That's exactly the CRC that is reported above.

Now please tell me my logic is wrong.

d-e-s-o · 2017-04-13T03:56:29Z

My claim also holds for the data you pasted.

int main()
{
  unsigned char const data[] = {0x00, 0x21, 0x2b, 0x88, 0x3a, 0x8b,                                     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,                                   0x00, 0x00, 0x00, 0x0c, 0x21, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00,                                   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};

  std::cout << "size: " << sizeof(data) << std::endl;
  std::cout << std::hex << "0x" << stm_crc32((const uint8_t*)data, sizeof(data)) << std::endl;
}

results in:

./test
size: 60
0xbbb65e8f // that is the correct CRC and not 0x2ef5269e as claimed, I believe

d-e-s-o · 2017-04-23T02:04:42Z

Can you confirm the problem? Or do you see a flaw in the logic?

d-e-s-o · 2017-05-21T19:28:40Z

Is there any update on the reported issue?

szszszsz · 2017-05-23T11:23:58Z

Sorry for delay! I think you are correct. I might have checked wrong value/condition back then and falsely reported here. Recently a WARNING level message about wrong CRC was added to log (I do not remember it was merged). I plan to list all commands with wrong CRC this week and fix at least most of them. However I have to take after some higher priority tasks first.

szszszsz · 2017-05-24T14:44:59Z

This is the list of commands with responses containing invalid CRC. I have compiled it after running py.test tests (both _pro and _storage) with libnitrokey (modified with unpublished yet patch):

CHANGE_UPDATE_PIN
CLEAR_NEW_SD_CARD_FOUND
DISABLE_CRYPTED_PARI
DISABLE_HIDDEN_CRYPTED_PARI
ENABLE_CRYPTED_PARI
ENABLE_HIDDEN_CRYPTED_PARI
ENABLE_READONLY_UNCRYPTED_LUN
ENABLE_READWRITE_UNCRYPTED_LUN
EXPORT_FIRMWARE_TO_FILE
GENERATE_NEW_KEYS
SEND_HIDDEN_VOLUME_SETUP
SEND_NEW_PASSWORD
SEND_PASSWORD

Edit: Storage v0.45

d-e-s-o · 2017-05-25T01:14:52Z

Great! Thanks for compiling this list. I am afraid I won't be able to help much more from here. I only have one nitrokey and have actual data on it, so I won't use it for trying out custom firmware blobs. I looked into the firmware code but it seems rather intricate and I was unable to figure out why DISABLE_CRYPTED_PARI in particular does not set the correct CRC.

szszszsz · 2017-05-25T05:51:12Z

Sure! I will test all modifications by myself first (I have one already). If you would have any idea later please let me know.

Signed-off-by: Szczepan Zalega <[email protected]>

Fixes #32

szszszsz · 2017-05-30T10:02:42Z

Confirmed this issue is not occurring on NK Pro 0.7 and 0.8.

d-e-s-o · 2017-06-25T21:08:44Z

Thanks for working on this issue, Szczepan. I have updated my nitrokey storage firmware to version 0.46. I still see wrong CRCs being reported. Is this to be expected? Did the final fix go into any of 0.45 or 0.46? Or is it not final yet?

d-e-s-o · 2017-06-25T21:10:25Z

Oh, I see it hasn't been merged to master yet. Is there an ETA? Or did you hit a problem during testing?

szszszsz · 2017-06-26T08:08:06Z

It looks like it is still waiting for review. I will check the details and let you know.

szszszsz · 2017-06-27T06:53:55Z

It is merged!

d-e-s-o · 2017-06-27T14:08:47Z

Awesome! Many thanks!

The nitrokey embeds a wrong CRC code into the response for certain commands. When closing the encrypted volume, for example, the CRC code never matches the actual report data. This problem is tracked by issue 32: Nitrokey/nitrokey-storage-firmware#32 To work around this problem for now we unfortunately have to ignore the result of the CRC check. This change adjusts the code to do that.

In the past the Nitrokey Storage device reported wrong CRC values for certain commands, e.g., the closing of an encrypted volume. This problem was tracked by issue 32: Nitrokey/nitrokey-storage-firmware#32 Firmware version 0.includes a fix for the problem. With this change we remove the workaround for the incorrect CRC from nitrocli. By doing so, we require the Nitrokey Storage to be running firmware 0.47 or higher to be properly usable.