Return error on invalid slot

Signed-off-by: Szczepan Zalega <szczepan@nitrokey.com>

7 files changed, 71 insertions, 37 deletions

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 3abcb0e..265e737 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -22,6 +22,6 @@ set(SOURCE_FILES
     log.cc
     misc.cc
     NitrokeyManager.cc
-        NK_C_API.cc CommandFailedException.cpp include/CommandFailedException.h TooLongStringException.cpp include/TooLongStringException.h InvalidSlotException.h)
+        NK_C_API.cc CommandFailedException.cpp include/CommandFailedException.h TooLongStringException.cpp include/TooLongStringException.h include/InvalidSlotException.h include/LibraryException.h)
 
 add_executable(libnitrokey ${SOURCE_FILES})
\ No newline at end of file
diff --git a/NK_C_API.cc b/NK_C_API.cc
index 4ba29ab..ea63f36 100644
--- a/NK_C_API.cc
+++ b/NK_C_API.cc
@@ -23,8 +23,8 @@ uint8_t * get_with_array_result(T func){
     catch (CommandFailedException & commandFailedException){
         NK_last_command_status = commandFailedException.last_command_status;
     }
-    catch (TooLongStringException & longStringException){
-        NK_last_command_status = TooLongStringException::exception_id;
+    catch (LibraryException & libraryException){
+        NK_last_command_status = libraryException.exception_id();
     }
     return nullptr;
 }
@@ -38,8 +38,8 @@ const char* get_with_string_result(T func){
     catch (CommandFailedException & commandFailedException){
         NK_last_command_status = commandFailedException.last_command_status;
     }
-    catch (TooLongStringException & longStringException){
-        NK_last_command_status = TooLongStringException::exception_id;
+    catch (LibraryException & libraryException){
+        NK_last_command_status = libraryException.exception_id();
     }
     return "";
 }
@@ -53,8 +53,8 @@ auto get_with_result(T func){
     catch (CommandFailedException & commandFailedException){
         NK_last_command_status = commandFailedException.last_command_status;
     }
-    catch (TooLongStringException & longStringException){
-        NK_last_command_status = TooLongStringException::exception_id;
+    catch (LibraryException & libraryException){
+        NK_last_command_status = libraryException.exception_id();
     }
     return static_cast<decltype(func())>(0);
 }
@@ -68,12 +68,11 @@ uint8_t get_without_result(T func){
     }
     catch (CommandFailedException & commandFailedException){
         NK_last_command_status = commandFailedException.last_command_status;
-        return commandFailedException.last_command_status;
     }
-    catch (TooLongStringException & longStringException){
-        NK_last_command_status = TooLongStringException::exception_id;
-        return NK_last_command_status;
+    catch (LibraryException & libraryException){
+        NK_last_command_status = libraryException.exception_id();
     }
+    return NK_last_command_status;
 }
 
 extern "C"
diff --git a/NitrokeyManager.cc b/NitrokeyManager.cc
index c27de2b..81d27aa 100644
--- a/NitrokeyManager.cc
+++ b/NitrokeyManager.cc
@@ -1,14 +1,16 @@
-#include <cassert>
 #include <cstring>
 #include <iostream>
 #include "include/NitrokeyManager.h"
 #include "include/TooLongStringException.h"
+#include "include/InvalidSlotException.h"
 
 namespace nitrokey{
 
     template <typename T>
     void strcpyT(T& dest, const char* src){
-        assert(src != nullptr);
+        if (src == nullptr)
+//            throw EmptySourceStringException(slot_number);
+            return;
         const size_t s_dest = sizeof dest;
         if (strlen(src) > s_dest){
             throw TooLongStringException(strlen(src), s_dest, src);
@@ -93,7 +95,7 @@ namespace nitrokey{
     }
 
     uint32_t NitrokeyManager::get_HOTP_code(uint8_t slot_number, const char *user_temporary_password) {
-        assert(is_valid_hotp_slot_number(slot_number));
+        if (!is_valid_hotp_slot_number(slot_number)) throw InvalidSlotException(slot_number);
         auto gh = get_payload<GetHOTP>();
         gh.slot_number = get_internal_slot_number_for_hotp(slot_number);
 
@@ -114,7 +116,7 @@ namespace nitrokey{
     uint32_t NitrokeyManager::get_TOTP_code(uint8_t slot_number, uint64_t challenge, uint64_t last_totp_time,
                                             uint8_t last_interval,
                                             const char *user_temporary_password) {
-        assert(is_valid_totp_slot_number(slot_number));
+        if(!is_valid_totp_slot_number(slot_number)) throw InvalidSlotException(slot_number);
         slot_number = get_internal_slot_number_for_totp(slot_number);
         auto gt = get_payload<GetTOTP>();
         gt.slot_number = slot_number;
@@ -140,13 +142,13 @@ namespace nitrokey{
     }
 
     bool NitrokeyManager::erase_hotp_slot(uint8_t slot_number, const char *temporary_password) {
-        assert(is_valid_hotp_slot_number(slot_number));
+        if (!is_valid_hotp_slot_number(slot_number)) throw InvalidSlotException(slot_number);
         slot_number = get_internal_slot_number_for_hotp(slot_number);
         return erase_slot(slot_number, temporary_password);
     }
 
     bool NitrokeyManager::erase_totp_slot(uint8_t slot_number, const char *temporary_password) {
-        assert(is_valid_totp_slot_number(slot_number));
+        if (!is_valid_totp_slot_number(slot_number)) throw InvalidSlotException(slot_number);
         slot_number = get_internal_slot_number_for_totp(slot_number);
         return erase_slot(slot_number, temporary_password);
     }
@@ -155,7 +157,7 @@ namespace nitrokey{
     bool NitrokeyManager::write_HOTP_slot(uint8_t slot_number, const char *slot_name, const char *secret, uint8_t hotp_counter,
                                               bool use_8_digits, bool use_enter, bool use_tokenID, const char *token_ID,
                                               const char *temporary_password) {
-        assert(is_valid_hotp_slot_number(slot_number));
+        if (!is_valid_hotp_slot_number(slot_number)) throw InvalidSlotException(slot_number);
 
         slot_number = get_internal_slot_number_for_hotp(slot_number);
         auto payload = get_payload<WriteToHOTPSlot>();
@@ -178,7 +180,7 @@ namespace nitrokey{
                                               bool use_8_digits, bool use_enter, bool use_tokenID, const char *token_ID,
                                               const char *temporary_password) {
         auto payload = get_payload<WriteToTOTPSlot>();
-        assert(is_valid_totp_slot_number(slot_number));
+        if (!is_valid_totp_slot_number(slot_number)) throw InvalidSlotException(slot_number);
 
         slot_number = get_internal_slot_number_for_totp(slot_number);
         payload.slot_number = slot_number;
@@ -197,12 +199,12 @@ namespace nitrokey{
     }
 
     const char * NitrokeyManager::get_totp_slot_name(uint8_t slot_number) {
-        assert(is_valid_totp_slot_number(slot_number));
+        if (!is_valid_totp_slot_number(slot_number)) throw InvalidSlotException(slot_number);
         slot_number = get_internal_slot_number_for_totp(slot_number);
         return get_slot_name(slot_number);
     }
     const char * NitrokeyManager::get_hotp_slot_name(uint8_t slot_number) {
-        assert(is_valid_hotp_slot_number(slot_number));
+        if (!is_valid_hotp_slot_number(slot_number)) throw InvalidSlotException(slot_number);
         slot_number = get_internal_slot_number_for_hotp(slot_number);
         return get_slot_name(slot_number);
     }
@@ -307,7 +309,7 @@ namespace nitrokey{
     }
 
     const char *NitrokeyManager::get_password_safe_slot_name(uint8_t slot_number) {
-        assert (is_valid_password_safe_slot_number(slot_number));
+        if (!is_valid_password_safe_slot_number(slot_number)) throw InvalidSlotException(slot_number);
         auto p = get_payload<GetPasswordSafeSlotName>();
         p.slot_number = slot_number;
         auto response = GetPasswordSafeSlotName::CommandTransaction::run(*device, p);
@@ -317,7 +319,7 @@ namespace nitrokey{
     bool NitrokeyManager::is_valid_password_safe_slot_number(uint8_t slot_number) const { return slot_number < 16; }
 
     const char *NitrokeyManager::get_password_safe_slot_login(uint8_t slot_number) {
-        assert (is_valid_password_safe_slot_number(slot_number));
+        if (!is_valid_password_safe_slot_number(slot_number)) throw InvalidSlotException(slot_number);
         auto p = get_payload<GetPasswordSafeSlotLogin>();
         p.slot_number = slot_number;
         auto response = GetPasswordSafeSlotLogin::CommandTransaction::run(*device, p);
@@ -325,7 +327,7 @@ namespace nitrokey{
     }
 
     const char *NitrokeyManager::get_password_safe_slot_password(uint8_t slot_number) {
-        assert (is_valid_password_safe_slot_number(slot_number));
+        if (!is_valid_password_safe_slot_number(slot_number)) throw InvalidSlotException(slot_number);
         auto p = get_payload<GetPasswordSafeSlotPassword>();
         p.slot_number = slot_number;
         auto response = GetPasswordSafeSlotPassword::CommandTransaction::run(*device, p);
@@ -334,7 +336,7 @@ namespace nitrokey{
 
     void NitrokeyManager::write_password_safe_slot(uint8_t slot_number, const char *slot_name, const char *slot_login,
                                                        const char *slot_password) {
-        assert (is_valid_password_safe_slot_number(slot_number));
+        if (!is_valid_password_safe_slot_number(slot_number)) throw InvalidSlotException(slot_number);
         auto p = get_payload<SetPasswordSafeSlotData>();
         p.slot_number = slot_number;
         strcpyT(p.slot_name, slot_name);
@@ -348,7 +350,7 @@ namespace nitrokey{
     }
 
     void NitrokeyManager::erase_password_safe_slot(uint8_t slot_number) {
-        assert (is_valid_password_safe_slot_number(slot_number));
+        if (!is_valid_password_safe_slot_number(slot_number)) throw InvalidSlotException(slot_number);
         auto p = get_payload<ErasePasswordSafeSlot>();
         p.slot_number = slot_number;
         ErasePasswordSafeSlot::CommandTransaction::run(*device, p);
diff --git a/InvalidSlotException.h b/include/InvalidSlotException.h
index 741d53e..e97d993 100644
--- a/InvalidSlotException.h
+++ b/include/InvalidSlotException.h
@@ -8,17 +8,21 @@
 
 #include <cstdint>
 #include <string>
-#include <exception>
+#include "LibraryException.h"
 
-class InvalidSlotException : public std::exception {
+
+class InvalidSlotException : public LibraryException {
 public:
-    static const std::uint8_t exception_id = 201;
+    virtual uint8_t exception_id() override {
+        return 201;
+    }
 
+public:
     uint8_t slot_selected;
 
     InvalidSlotException(uint8_t slot_selected) : slot_selected(slot_selected) {}
 
-    virtual const char *what() const throw() {
+    virtual const char *what() const throw() override {
         return "Wrong slot selected";
     }
 
diff --git a/include/LibraryException.h b/include/LibraryException.h
new file mode 100644
index 0000000..ef81624
--- /dev/null
+++ b/include/LibraryException.h
@@ -0,0 +1,17 @@
+//
+// Created by sz on 09.08.16.
+//
+
+#ifndef LIBNITROKEY_LIBRARYEXCEPTION_H
+#define LIBNITROKEY_LIBRARYEXCEPTION_H
+
+#include <exception>
+#include <cstdint>
+
+class LibraryException: std::exception {
+public:
+    virtual uint8_t exception_id()= 0;
+};
+
+
+#endif //LIBNITROKEY_LIBRARYEXCEPTION_H
diff --git a/include/TooLongStringException.h b/include/TooLongStringException.h
index 3a9244b..58f5801 100644
--- a/include/TooLongStringException.h
+++ b/include/TooLongStringException.h
@@ -8,11 +8,13 @@
 
 #include <cstdint>
 #include <string>
-#include <exception>
+#include "LibraryException.h"
 
-class TooLongStringException : public std::exception {
+class TooLongStringException : public LibraryException {
 public:
-    static const std::uint8_t exception_id = 200;
+    virtual uint8_t exception_id() override {
+        return 200;
+    }
 
     std::size_t size_source;
     std::size_t size_destination;
@@ -21,7 +23,7 @@ public:
     TooLongStringException(size_t size_source, size_t size_destination, const std::string &message = "") : size_source(
             size_source), size_destination(size_destination), message(message) {}
 
-    virtual const char *what() const throw() {
+    virtual const char *what() const throw() override {
         //TODO add sizes and message data to final message
         return "Too long string has been supplied as an argument";
     }
diff --git a/unittest/test_bindings.py b/unittest/test_bindings.py
index 9eebcb3..7b16a46 100644
--- a/unittest/test_bindings.py
+++ b/unittest/test_bindings.py
@@ -210,9 +210,19 @@ def test_too_long_strings(C):
     assert C.NK_get_last_command_status() == LibraryErrors.TOO_LONG_STRING
 
 
-# def test_invalid_slot(C):
-#     invalid_slot = 255
-#     assert C.NK_erase_totp_slot(invalid_slot, 'some password') == LibraryErrors.INVALID_SLOT
+def test_invalid_slot(C):
+    invalid_slot = 255
+    assert C.NK_erase_totp_slot(invalid_slot, 'some password') == LibraryErrors.INVALID_SLOT
+    assert C.NK_write_hotp_slot(invalid_slot, 'long_test', RFC_SECRET, 0, False, False, False, "",
+                            'aaa') == LibraryErrors.INVALID_SLOT
+    assert C.NK_get_hotp_code_PIN(invalid_slot, 'some password') == 0
+    assert C.NK_get_last_command_status() == LibraryErrors.INVALID_SLOT
+    assert C.NK_erase_password_safe_slot(invalid_slot) == LibraryErrors.INVALID_SLOT
+    assert C.NK_enable_password_safe(DefaultPasswords.USER) == DeviceErrorCode.STATUS_OK
+    assert gs(C.NK_get_password_safe_slot_name(invalid_slot)) == ''
+    assert C.NK_get_last_command_status() == LibraryErrors.INVALID_SLOT
+    assert gs(C.NK_get_password_safe_slot_login(invalid_slot)) == ''
+    assert C.NK_get_last_command_status() == LibraryErrors.INVALID_SLOT
 
 
 def test_admin_retry_counts(C):