Merge branch 'backward_compatibility'

Do the HOTP slot counter conversion from ASCII to binary only for
Storage v0.53 and lower. Storage v0.54 returns binary counter.

6 files changed, 88 insertions, 8 deletions

diff --git a/NK_C_API.cc b/NK_C_API.cc
index ec7bc29..1d3fa3a 100644
--- a/NK_C_API.cc
+++ b/NK_C_API.cc
@@ -881,6 +881,29 @@ NK_C_API char* NK_get_SD_usage_data_as_string() {
   }
 
 
+  NK_C_API int NK_read_HOTP_slot(const uint8_t slot_num, struct ReadSlot_t* out){
+  if (out == nullptr)
+    return -1;
+  auto m = NitrokeyManager::instance();
+  auto result = get_with_status([&]() {
+    return m->get_HOTP_slot_data(slot_num);
+  }, stick10::ReadSlot::ResponsePayload() );
+  auto error_code = std::get<0>(result);
+  if (error_code != 0) {
+    return error_code;
+  }
+#define a(x) out->x = read_slot.x
+  stick10::ReadSlot::ResponsePayload read_slot = std::get<1>(result);
+  a(_slot_config);
+  a(slot_counter);
+#undef a
+#define m(x) memmove(out->x, read_slot.x, sizeof(read_slot.x))
+  m(slot_name);
+  m(slot_token_id);
+#undef m
+  return 0;
+}
+
 
 #ifdef __cplusplus
 }
diff --git a/NK_C_API.h b/NK_C_API.h
index 9383cd9..d5c54a3 100644
--- a/NK_C_API.h
+++ b/NK_C_API.h
@@ -995,6 +995,18 @@ extern "C" {
    */
   NK_C_API int NK_change_firmware_password_pro(const char *current_firmware_password, const char *new_firmware_password);
 
+
+// as in ReadSlot::ResponsePayload
+struct ReadSlot_t {
+  uint8_t slot_name[15];
+  uint8_t _slot_config;
+  uint8_t slot_token_id[13];
+  uint64_t slot_counter;
+};
+
+
+NK_C_API int NK_read_HOTP_slot(const uint8_t slot_num, struct ReadSlot_t* out);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/NitrokeyManager.cc b/NitrokeyManager.cc
index ea409ef..6c26a43 100644
--- a/NitrokeyManager.cc
+++ b/NitrokeyManager.cc
@@ -443,6 +443,7 @@ using nitrokey::misc::strcpyT;
       return "";
     }
 
+    bool NitrokeyManager::is_internal_hotp_slot_number(uint8_t slot_number) const { return slot_number < 0x20; }
     bool NitrokeyManager::is_valid_hotp_slot_number(uint8_t slot_number) const { return slot_number < 3; }
     bool NitrokeyManager::is_valid_totp_slot_number(uint8_t slot_number) const { return slot_number < 0x10-1; } //15
     uint8_t NitrokeyManager::get_internal_slot_number_for_totp(uint8_t slot_number) const { return (uint8_t) (0x20 + slot_number); }
@@ -1120,11 +1121,31 @@ using nitrokey::misc::strcpyT;
     return get_TOTP_code(slot_number, 0, 0, 0, user_temporary_password);
   }
 
+  /**
+   * Returns ReadSlot structure, describing OTP slot configuration. Always return binary counter -
+   * does the necessary conversion, if needed, to unify the behavior across Pro and Storage.
+   * @private For internal use only
+   * @param slot_number which OTP slot to use (usual format)
+   * @return ReadSlot structure
+   */
   stick10::ReadSlot::ResponsePayload NitrokeyManager::get_OTP_slot_data(const uint8_t slot_number) {
     auto p = get_payload<stick10::ReadSlot>();
     p.slot_number = slot_number;
+    p.data_format = stick10::ReadSlot::CounterFormat::BINARY; // ignored for devices other than Storage v0.54+
     auto data = stick10::ReadSlot::CommandTransaction::run(device, p);
-    return data.data();
+
+    auto &payload = data.data();
+
+    // if fw <=v0.53 and asked binary - do the conversion from ASCII
+    if (device->get_device_model() == DeviceModel::STORAGE && get_minor_firmware_version() <= 53
+         && is_internal_hotp_slot_number(slot_number))
+    {
+      //convert counter from string to ull
+      auto counter_s = std::string(payload.slot_counter_s, payload.slot_counter_s + sizeof(payload.slot_counter_s));
+      payload.slot_counter = std::stoull(counter_s);
+    }
+
+    return payload;
   }
 
   stick10::ReadSlot::ResponsePayload NitrokeyManager::get_TOTP_slot_data(const uint8_t slot_number) {
@@ -1132,13 +1153,7 @@ using nitrokey::misc::strcpyT;
   }
 
   stick10::ReadSlot::ResponsePayload NitrokeyManager::get_HOTP_slot_data(const uint8_t slot_number) {
-    auto slot_data = get_OTP_slot_data(get_internal_slot_number_for_hotp(slot_number));
-    if (device->get_device_model() == DeviceModel::STORAGE){
-      //convert counter from string to ull
-      auto counter_s = std::string(slot_data.slot_counter_s, slot_data.slot_counter_s+sizeof(slot_data.slot_counter_s));
-      slot_data.slot_counter = std::stoull(counter_s);
-    }
-    return slot_data;
+    return get_OTP_slot_data(get_internal_slot_number_for_hotp(slot_number));
   }
 
   void NitrokeyManager::lock_encrypted_volume() {
diff --git a/libnitrokey/NitrokeyManager.h b/libnitrokey/NitrokeyManager.h
index 0691035..33ede1b 100644
--- a/libnitrokey/NitrokeyManager.h
+++ b/libnitrokey/NitrokeyManager.h
@@ -299,6 +299,7 @@ char * strndup(const char* str, size_t maxlen);
       void enable_firmware_update_pro(const char *firmware_pin);
 
       void change_firmware_update_password_pro(const char *firmware_pin_current, const char *firmware_pin_new);
+      bool is_internal_hotp_slot_number(uint8_t slot_number) const;
     };
 }
 
diff --git a/libnitrokey/stick10_commands.h b/libnitrokey/stick10_commands.h
index 178b23f..5e8a5aa 100644
--- a/libnitrokey/stick10_commands.h
+++ b/libnitrokey/stick10_commands.h
@@ -304,8 +304,13 @@ class GetHOTP : Command<CommandID::GET_CODE> {
 
 class ReadSlot : Command<CommandID::READ_SLOT> {
  public:
+  enum class CounterFormat {
+    ASCII = 0,
+    BINARY = 1,
+  };
   struct CommandPayload {
     uint8_t slot_number;
+    CounterFormat data_format; //Storage v0.54+ only: slot_counter value format: 0 - in ascii, 1 - binary
 
     bool isValid() const { return !(slot_number & 0xF0); }
 
diff --git a/unittest/test_pro.py b/unittest/test_pro.py
index 685d0fa..b8ae290 100644
--- a/unittest/test_pro.py
+++ b/unittest/test_pro.py
@@ -979,3 +979,27 @@ def test_bootloader_password_change_pro_too_long(C):
     long_string = b'a' * 100
     assert C.NK_change_firmware_password_pro(long_string, long_string) == LibraryErrors.TOO_LONG_STRING
     assert C.NK_change_firmware_password_pro(DefaultPasswords.UPDATE, long_string) == LibraryErrors.TOO_LONG_STRING
+
+
+@pytest.mark.otp
+@pytest.mark.parametrize('counter_mid', [10**3-1, 10**4-1, 10**7-1, 10**8-10, 2**16, 2**31-1, 2**32-1, 2**33, 2**50, 2**60, 2**63])  # 2**64-1
+def test_HOTP_counter_getter(C, counter_mid: int):
+    if len(str(counter_mid)) > 8:
+        skip_if_device_version_lower_than({'S': 54, 'P': 7})
+    assert C.NK_first_authenticate(DefaultPasswords.ADMIN, DefaultPasswords.ADMIN_TEMP) == DeviceErrorCode.STATUS_OK
+    use_pin_protection = False
+    use_8_digits = False
+    assert C.NK_write_config(255, 255, 255, use_pin_protection, not use_pin_protection,
+                             DefaultPasswords.ADMIN_TEMP) == DeviceErrorCode.STATUS_OK
+    read_slot_st = ffi.new('struct ReadSlot_t *')
+    if not read_slot_st:
+        raise Exception("Could not allocate status")
+    slot_number = 1
+    for counter in range(counter_mid-3, counter_mid+3):
+        # assert C.NK_first_authenticate(DefaultPasswords.ADMIN, DefaultPasswords.ADMIN_TEMP) == DeviceErrorCode.STATUS_OK
+        assert C.NK_write_hotp_slot(slot_number, b'python_test', bbRFC_SECRET, counter, use_8_digits, False, False, b'',
+                                    DefaultPasswords.ADMIN_TEMP) == DeviceErrorCode.STATUS_OK
+        assert C.NK_read_HOTP_slot(slot_number, read_slot_st) == DeviceErrorCode.STATUS_OK
+        assert read_slot_st.slot_counter == counter
+
+