fix otp

Change-Id: I2c0ad61d2b671911f0a11024206dcacd56d4b659

1 files changed, 61 insertions, 20 deletions

diff --git a/apps/plugins/otp.c b/apps/plugins/otp.c
index 1297981..0384a75 100644
--- a/apps/plugins/otp.c
+++ b/apps/plugins/otp.c
@@ -40,6 +40,8 @@
 
 #define MAX(a, b) (((a)>(b))?(a):(b))
 
+#define assert(x) (!(x)?assert_fail():0)
+
 struct account_t {
     char name[MAX_NAME];
 
@@ -72,6 +74,12 @@ static char enc_password[PASS_MAX + 1];
 
 static void acct_menu(const char *title, void (*cb)(int acct));
 
+static void assert_fail(void)
+{
+    rb->splashf(HZ * 2, "Assertion failed! REPORT ME!");
+    exit(0);
+}
+
 static int HOTP(unsigned char *secret, size_t sec_len, uint64_t ctr, int digits)
 {
     ctr = htobe64(ctr);
@@ -262,7 +270,7 @@ static void aes_ctr_nextblock(struct aes_ctr_ctx *ctx)
     ctx->bytes_left = 16;
 }
 
-static void aes_ctr_process(struct aes_ctr_ctx *ctx, const char *in, char *out, size_t len)
+static void aes_ctr_process(struct aes_ctr_ctx *ctx, const unsigned char *in, unsigned char *out, size_t len)
 {
     while(len--)
     {
@@ -282,13 +290,15 @@ static void aes_ctr_destroy(struct aes_ctr_ctx *ctx)
 #endif
 }
 
+static unsigned char in_buf[sizeof(struct account_t)];
+
 static bool read_accts(void)
 {
     int fd = rb->open(ACCT_FILE, O_RDONLY);
     if(fd < 0)
         return false;
 
-    char buf[4];
+    unsigned char buf[4];
     /* two versions to maintain backwards-compatibility */
     char magic_old[] = "OTP1";
     char magic[] = "OTP2";
@@ -311,12 +321,11 @@ static bool read_accts(void)
             uint64_t nonce;
             rb->read(fd, &nonce, sizeof(nonce));
 
-            char in[sizeof(struct account_t)];
             char buf[4] = {0};
             char magic[] = "OTP2";
 
             /* read the encrypted header */
-            rb->read(fd, in, 4);
+            rb->read(fd, in_buf, 4);
 
             for(int i = 0; i < 3; ++i)
             {
@@ -338,7 +347,7 @@ static bool read_accts(void)
 
                 aes_ctr_init(&aes_ctx, key, nonce);
 
-                aes_ctr_process(&aes_ctx, in, buf, 4);
+                aes_ctr_process(&aes_ctx, in_buf, buf, 4);
                 if(rb->memcmp(buf, magic, 4))
                 {
                     rb->splashf(HZ * 2, "Wrong password!");
@@ -348,9 +357,9 @@ static bool read_accts(void)
 
                 while(next_slot < max_accts)
                 {
-                    if(rb->read(fd, in, sizeof(in)) != sizeof(struct account_t))
+                    if(rb->read(fd, in_buf, sizeof(in_buf)) != sizeof(struct account_t))
                         break;
-                    aes_ctr_process(&aes_ctx, in, (char*)accounts + next_slot, sizeof(struct account_t));
+                    aes_ctr_process(&aes_ctx, in_buf, (unsigned char*)(accounts + next_slot), sizeof(struct account_t));
                     ++next_slot;
                 }
 
@@ -381,6 +390,8 @@ static bool read_accts(void)
     return true;
 }
 
+static char out_buf[sizeof(struct account_t)];
+
 static void save_accts(void)
 {
     int fd = rb->open(ACCT_FILE, O_WRONLY | O_CREAT | O_TRUNC, 0600);
@@ -390,20 +401,22 @@ static void save_accts(void)
     rb->write(fd, &time_offs, sizeof(time_offs));
     rb->write(fd, &encrypted, sizeof(encrypted));
 
+    assert(sizeof(out_buf) == sizeof(struct account_t));
+
     if(encrypted)
     {
         /* encrypt the data with AES128-CTR */
 
         /* generate/write the nonce */
-        //uint64_t nonce = *rb->current_tick;
+        uint64_t nonce = *rb->current_tick;
 #if CONFIG_RTC
-        //nonce += get_utc();
+        nonce += get_utc();
 #endif
-        uint64_t nonce = 42;
 
         rb->write(fd, &nonce, sizeof(nonce));
 
-        /* the SHA-1 of the password is truncated to 128 bits to form the key */
+        /* the HMAC-SHA-1 of the password and salt is truncated to 128
+         * bits to form the key */
         char key[20];
 
         hmac_sha1(&nonce, sizeof(nonce), enc_password, rb->strlen(enc_password), key);
@@ -411,16 +424,15 @@ static void save_accts(void)
         struct aes_ctr_ctx aes_ctx;
         aes_ctr_init(&aes_ctx, key, nonce);
 
-        char out[sizeof(struct account_t)];
         const char *magic = "OTP2";
 
-        aes_ctr_process(&aes_ctx, magic, out, 4);
-        rb->write(fd, out, 4);
+        aes_ctr_process(&aes_ctx, magic, out_buf, 4);
+        rb->write(fd, out_buf , 4);
 
         for(int i = 0; i < next_slot; ++i)
         {
-            aes_ctr_process(&aes_ctx, (char*)accounts + i, out, sizeof(struct account_t));
-            rb->write(fd, out, sizeof(out));
+            aes_ctr_process(&aes_ctx, (unsigned char*)(accounts + i), out_buf, sizeof(struct account_t));
+            rb->write(fd, out_buf, sizeof(out_buf));
         }
 
         aes_ctr_destroy(&aes_ctx);
@@ -432,6 +444,17 @@ static void save_accts(void)
     rb->close(fd);
 }
 
+static int compare_acct(const void *a, const void *b)
+{
+    const struct account_t *a1 = a, *b1 = b;
+    return rb->strcmp(a1->name, b1->name);
+}
+
+static void sort_accts(void)
+{
+    rb->qsort(accounts, next_slot, sizeof(struct account_t), compare_acct);
+}
+
 static void add_acct_file(void)
 {
     char fname[MAX_PATH];
@@ -566,6 +589,7 @@ static void add_acct_file(void)
     else
     {
         rb->splashf(HZ * 2, "Added %d account(s).", next_slot - before);
+        sort_accts();
         save_accts();
     }
 }
@@ -652,6 +676,7 @@ static void add_acct_manual(void)
 
     ++next_slot;
 
+    sort_accts();
     save_accts();
 
     rb->splashf(HZ * 2, "Success.");
@@ -824,8 +849,10 @@ static void edit_menu(int acct)
                 break;
             }
             rb->strlcpy(accounts[acct].name, data_buf, sizeof(accounts[acct].name));
+            sort_accts();
             save_accts();
-            break;
+            rb->splash(HZ, "Success.");
+            return;
         case 1: // delete
             if(danger_confirm())
             {
@@ -867,8 +894,11 @@ static void edit_menu(int acct)
             rb->splash(HZ, "Success.");
             break;
         case 4: // secret
+            /* save the old secret */
             old_len = accounts[acct].sec_len;
             rb->memcpy(temp_sec, accounts[acct].secret, accounts[acct].sec_len);
+
+            /* encode */
             base32_encode(accounts[acct].secret, accounts[acct].sec_len, data_buf, sizeof(data_buf));
 
             if(rb->kbd_input(data_buf, sizeof(data_buf)) < 0)
@@ -885,7 +915,9 @@ static void edit_menu(int acct)
             accounts[acct].sec_len = ret;
 
             save_accts();
+
             rb->splash(HZ, "Success.");
+
             break;
 #ifdef CONFIG_RTC
         case 5:
@@ -1015,7 +1047,8 @@ static int get_time_offs(void)
     return get_time_seconds(label);
 
 #if 0
-    /* provided in case menu internals change */
+    /* kept just in case menu internals change and the above code
+     * breaks */
     switch(rb->do_menu(&menu, &sel, NULL, false))
     {
     case 0: case 1: case 2:
@@ -1186,7 +1219,7 @@ static void encrypt_menu(void)
         if(rb->kbd_input(temp_pass2, sizeof(temp_pass2)) < 0)
             break;
 
-        if(rb->strcmp(temp_pass, enc_password))
+        if(rb->strcmp(temp_pass, temp_pass2))
         {
             rb->splash(HZ * 2, "Passwords do not match!");
             break;
@@ -1379,6 +1412,8 @@ static void type_code(int acct)
         rb->sleep(TYPE_DELAY);
     }
 
+    rb->usb_hid_send(HID_USAGE_PAGE_KEYBOARD_KEYPAD, HID_KEYBOARD_RETURN);
+
     if(change_numlock)
         rb->usb_hid_send(HID_USAGE_PAGE_KEYBOARD_KEYPAD, HID_KEYPAD_NUM_LOCK_AND_CLEAR);
 
@@ -1401,7 +1436,7 @@ static void type_codes(void)
 {
     if(!rb->global_settings->usb_hid)
     {
-        rb->splashf(HZ * 4, "Please enable USB HID.");
+        rb->splashf(HZ * 4, "Please enable USB HID in the system settings.");
     }
     acct_menu("Type Code", type_code);
 }
@@ -1442,6 +1477,10 @@ static void acct_menu(const char *title, void (*cb)(int acct))
             break;
         case PLA_SELECT:
             cb(idx);
+            if(idx >= next_slot)
+                idx = 0;
+            if(next_slot == 0)
+                return;
             break;
         case PLA_UP:
         case PLA_CANCEL:
@@ -1469,6 +1508,7 @@ enum plugin_status plugin_start(const void* parameter)
     /* self-test with RFC 4226 values */
     if(HOTP("12345678901234567890", rb->strlen("12345678901234567890"), 1, 6) != 287082)
     {
+        rb->splashf(HZ * 4, "ERROR: self-test failed! Report me!");
         return PLUGIN_ERROR;
     }
 
@@ -1479,6 +1519,7 @@ enum plugin_status plugin_start(const void* parameter)
     if(!read_accts())
 #if CONFIG_RTC
     {
+        /* first-run config */
         time_offs = get_time_offs();
     }
 #else