Replace the delimiter whitespace with '\0'.

+ Use '\0' as the delimiter.
+ Allow whitespace character for keystore password.

In previous implementation, we use space as the delimiter. That
will stop user from using passphrase with whitespace character.

5 files changed, 96 insertions, 87 deletions

diff --git a/cmds/keystore/certtool.h b/cmds/keystore/certtool.h
index aefad66..9b72bf7 100644
--- a/cmds/keystore/certtool.h
+++ b/cmds/keystore/certtool.h
@@ -38,9 +38,9 @@ static inline int get_cert(const char *certname, unsigned char *value, int *size
     int count, fd, ret = -1;
     LPC_MARSHAL cmd;
     char delimiter[] = "_";
-    char *namespace, *keyname;
+    char *p = NULL;
     char *context = NULL;
-    char cname[CERT_NAME_LEN];
+    char *cname = (char*)cmd.data;
 
     if ((certname == NULL) || (value == NULL)) {
         LOGE("get_cert: certname or value is null\n");
@@ -61,12 +61,10 @@ static inline int get_cert(const char *certname, unsigned char *value, int *size
     }
 
     cmd.opcode = GET;
-    if (((namespace = strtok_r(cname, delimiter, &context)) == NULL) ||
-        ((keyname = strtok_r(NULL, delimiter, &context)) == NULL)) {
-        goto err;
-    }
-    if ((cmd.len = snprintf((char*)cmd.data, BUFFER_MAX, "%s %s", namespace, keyname))
-        > (2 * MAX_KEY_NAME_LENGTH + 1)) goto err;
+    p = strstr(cname, delimiter);
+    cmd.len = strlen(certname) + 1;
+    if (p == NULL) goto err;
+    *p = 0; // replace the delimiter with \0 .
 
     if (write_marshal(fd, &cmd)) {
         LOGE("Incorrect command or command line is too long.\n");
diff --git a/cmds/keystore/keymgmt.c b/cmds/keystore/keymgmt.c
index 9a1f845..69e0380 100644
--- a/cmds/keystore/keymgmt.c
+++ b/cmds/keystore/keymgmt.c
@@ -192,21 +192,15 @@ static int create_master_key(char *upasswd)
     return ret;
 }
 
-static int change_passwd(char *data)
+int change_passwd(char *old_pass, char *new_pass)
 {
     unsigned char master_key[USER_KEY_LEN];
-    char *old_pass, *new_pass = NULL, *p, *delimiter=" ";
-    int ret, count = 0;
-    char *context = NULL;
-
-    old_pass = p = strtok_r(data, delimiter, &context);
-    while (p != NULL) {
-        count++;
-        new_pass = p;
-        p = strtok_r(NULL, delimiter, &context);
-    }
-    if (count != 2) return -1;
-    if (strlen(new_pass) < MIN_PASSWD_LENGTH) return -1;
+    int ret;
+
+    if (state == UNINITIALIZED) return -1;
+    if ((strlen(old_pass) < MIN_PASSWD_LENGTH) ||
+        (strlen(new_pass) < MIN_PASSWD_LENGTH)) return -1;
+
     if ((ret = get_master_key(old_pass, master_key)) == 0) {
         ret = store_master_key(new_pass, master_key);
         retry_count = 0;
@@ -336,14 +330,12 @@ int list_keys(const char *namespace, char reply[BUFFER_MAX])
     return 0;
 }
 
-int passwd(char *data)
+int new_passwd(char *password)
 {
-    if (state == UNINITIALIZED) {
-        if (strchr(data, ' ')) return -1;
-        if (strlen(data) < MIN_PASSWD_LENGTH) return -1;
-        return create_master_key(data);
-    }
-    return change_passwd(data);
+    int passwdlen = strlen(password);
+
+    if ((state != UNINITIALIZED) || (passwdlen < MIN_PASSWD_LENGTH)) return -1;
+    return create_master_key(password);
 }
 
 int lock()
diff --git a/cmds/keystore/keymgmt.h b/cmds/keystore/keymgmt.h
index 0e928db..116d7a3 100644
--- a/cmds/keystore/keymgmt.h
+++ b/cmds/keystore/keymgmt.h
@@ -72,7 +72,8 @@ int get_key(const char *namespace, const char *keyname,
             unsigned char *data, int *size);
 int remove_key(const char *namespace, const char *keyname);
 int list_keys(const char *namespace, char reply[BUFFER_MAX]);
-int passwd(char *data);
+int new_passwd(char *password);
+int change_passwd(char *old_pass, char *new_pass);
 int lock();
 int unlock(char *passwd);
 KEYSTORE_STATE get_state();
diff --git a/cmds/keystore/netkeystore.c b/cmds/keystore/netkeystore.c
index bdd5960..82a92c3 100644
--- a/cmds/keystore/netkeystore.c
+++ b/cmds/keystore/netkeystore.c
@@ -85,28 +85,44 @@ static int check_reset_perm(int uid)
     return -1;
 }
 
-static int parse_keyname(char *name, uint32_t len,
-                         char *namespace, char *keyname)
+/**
+ * The function parse_strings() only handle two or three tokens just for
+ * keystore's need.
+ */
+static int parse_strings(char *data, int data_len, int ntokens, ...)
 {
     int count = 0;
-    char *c = namespace, *p = namespace, *t = name;
-
-    if (!name || !namespace || !keyname) return -1;
-    while (t < name + len && (*t != 0)) {
-        if (*t == ' ') {
-            if (c == keyname) return -1;
-            *p = count = 0;
-            c = p = keyname;
-            t++;
-        } else {
-            if (!isalnum(*t)) return -1;
-            *p++ = *t++;
-            // also check if the keyname/namespace is too long.
-            if (count++ == MAX_KEY_NAME_LENGTH) return -1;
+    va_list args;
+    char *p = data, **q;
+
+    va_start(args, ntokens);
+    q = va_arg(args, char**);
+    *q = p;
+    while (p < (data + data_len)) {
+        if (*(p++) == 0) {
+            if (++count == ntokens) break;
+            if ((q = va_arg(args, char**)) == NULL) break;
+            *q = p;
         }
     }
-    *p = 0;
-    return 0;
+    va_end(args);
+    // the first two strings should be null-terminated and the third could
+    // ignore the delimiter.
+    if (count >= 2) {
+        if ((ntokens == 3) || ((ntokens == 2) && (p == (data + data_len)))) {
+            return 0;
+        }
+    }
+    return -1;
+}
+
+static int is_alnum_string(char *s)
+{
+    while (*s != 0) {
+        if (!isalnum(*s++)) return 0;
+    }
+    LOGE("The string %s is not an alphanumeric string\n", s);
+    return 1;
 }
 
 // args of passwd():
@@ -114,7 +130,17 @@ static int parse_keyname(char *name, uint32_t len,
 // oldPassword newPassword - for changing the password
 static void do_passwd(LPC_MARSHAL *cmd, LPC_MARSHAL *reply)
 {
-    reply->retcode = passwd((char*)cmd->data);
+    char *p1 = NULL, *p2 = NULL;
+
+    if (strlen((char*)cmd->data) == (cmd->len - 1)) {
+        reply->retcode = new_passwd((char*)cmd->data);
+    } else {
+        if (parse_strings((char *)cmd->data, cmd->len, 2, &p1, &p2) != 0) {
+            reply->retcode = -1;
+        } else {
+            reply->retcode = change_passwd(p1, p2);
+        }
+    }
 }
 
 // args of lock():
@@ -150,8 +176,7 @@ static void do_listkeys(LPC_MARSHAL *cmd, LPC_MARSHAL *reply)
 // namespace keyname
 static void do_get_key(LPC_MARSHAL *cmd, LPC_MARSHAL *reply)
 {
-    char namespace[MAX_KEY_NAME_LENGTH];
-    char keyname[MAX_KEY_NAME_LENGTH];
+    char *namespace = NULL, *keyname = NULL;
 
     if (check_get_perm(cr.uid)) {
         LOGE("uid %d doesn't have the permission to get key value\n", cr.uid);
@@ -159,7 +184,8 @@ static void do_get_key(LPC_MARSHAL *cmd, LPC_MARSHAL *reply)
         return;
     }
 
-    if (parse_keyname((char*)cmd->data, cmd->len, namespace, keyname)) {
+    if (parse_strings((char*)cmd->data, cmd->len, 2, &namespace, &keyname) ||
+        !is_alnum_string(namespace) || !is_alnum_string(keyname)) {
         reply->retcode = -1;
     } else {
         reply->retcode = get_key(namespace, keyname, reply->data,
@@ -182,31 +208,26 @@ static int get_value_index(LPC_MARSHAL *cmd)
 // namespace keyname keyvalue
 static void do_put_key(LPC_MARSHAL *cmd, LPC_MARSHAL *reply)
 {
-    char namespace[MAX_KEY_NAME_LENGTH];
-    char keyname[MAX_KEY_NAME_LENGTH];
+    char *namespace = NULL, *keyname = NULL;
+    char *value = NULL;
 
-    int p = get_value_index(cmd);
-    if (p == -1) {
+    if (parse_strings((char*)cmd->data, cmd->len, 3, &namespace, &keyname, &value) ||
+        !is_alnum_string(namespace) || !is_alnum_string(keyname)) {
         reply->retcode = -1;
-    } else {
-        unsigned char *value;
-        if (parse_keyname((char*)cmd->data, p - 1, namespace, keyname)) {
-            reply->retcode = -1;
-            return;
-        }
-        value = &cmd->data[p];
-        int len = cmd->len - p;
-        reply->retcode = put_key(namespace, keyname, value, len);
+        return;
     }
+    int len = cmd->len - (value - namespace);
+    reply->retcode = put_key(namespace, keyname, (unsigned char *)value, len);
 }
 
 // args of remove_key():
 // namespace keyname
 static void do_remove_key(LPC_MARSHAL *cmd, LPC_MARSHAL *reply)
 {
-    char namespace[MAX_KEY_NAME_LENGTH];
-    char keyname[MAX_KEY_NAME_LENGTH];
-    if (parse_keyname((char*)cmd->data, cmd->len, namespace, keyname)) {
+    char *namespace = NULL, *keyname = NULL;
+
+    if (parse_strings((char*)cmd->data, cmd->len, 2, &namespace, &keyname) ||
+        !is_alnum_string(namespace) || !is_alnum_string(keyname)) {
         reply->retcode = -1;
         return;
     }
@@ -260,8 +281,6 @@ static int append_input_from_file(const char *filename, LPC_MARSHAL *cmd)
         fprintf(stderr, "Can not open file %s\n", filename);
         return -1;
     }
-    cmd->data[cmd->len] = ' ';
-    cmd->len++;
     len = read(fd, cmd->data + cmd->len, BUFFER_MAX - cmd->len);
     if (len < 0 || (len == (int)(BUFFER_MAX - cmd->len))) {
         ret = -1;
@@ -278,14 +297,15 @@ static int flatten_str_args(int argc, const char **argv, LPC_MARSHAL *cmd)
     char *buf = (char*)cmd->data;
     buf[0] = 0;
     for (i = 0 ; i < argc ; ++i) {
+        // we also include the \0 character in the input.
         if (i == 0) {
-            len = strlcpy(buf, argv[i], BUFFER_MAX);
+            len = (strlcpy(buf, argv[i], BUFFER_MAX) + 1);
         } else {
-            len += snprintf(buf + len, BUFFER_MAX - len, " %s", argv[i]);
+            len += (snprintf(buf + len, BUFFER_MAX - len, "%s", argv[i]) + 1);
         }
         if (len >= BUFFER_MAX) return -1;
     }
-    if (len) cmd->len = len;
+    if (len) cmd->len = len ;
     return 0;
 }
 
diff --git a/cmds/keystore/tests/netkeystore_test.c b/cmds/keystore/tests/netkeystore_test.c
index e7e686b..00390e0 100644
--- a/cmds/keystore/tests/netkeystore_test.c
+++ b/cmds/keystore/tests/netkeystore_test.c
@@ -43,7 +43,7 @@ typedef struct {
 #define FUNC_BODY(x) int test_##x()
 
 #define TEST_PASSWD        "12345678"
-#define TEST_NPASSWD    "11111111"
+#define TEST_NPASSWD    "hello world"
 #define TEST_DIR        "/data/local/tmp/keystore"
 #define READONLY_DIR    "/proc/keystore"
 #define TEST_NAMESPACE    "test"
@@ -83,7 +83,7 @@ FUNC_BODY(reset_keystore)
 FUNC_BODY(get_state)
 {
     if (get_state() != UNINITIALIZED) return -1;
-    passwd(TEST_PASSWD);
+    new_passwd(TEST_PASSWD);
     if (get_state() != UNLOCKED) return -1;
     lock();
     if (get_state() != LOCKED) return -1;
@@ -96,19 +96,17 @@ FUNC_BODY(passwd)
 {
     char buf[512];
 
-    if (passwd(" 23432dsfsdf") == 0) return -1;
-    if (passwd("dsfsdf") == 0) return -1;
-    passwd(TEST_PASSWD);
+    if (new_passwd("2d fsdf") == 0) return -1;
+    if (new_passwd("dsfsdf") == 0) return -1;
+    new_passwd(TEST_PASSWD);
     lock();
     if (unlock("55555555") == 0) return -1;
     if (unlock(TEST_PASSWD) != 0) return -1;
 
     // change the password
-    sprintf(buf, "%s %s", "klfdjdsklfjg", "abcdefghi");
-    if (passwd(buf) == 0) return -1;
+    if (change_passwd("klfdjdsklfjg", "abcdefghi") == 0) return -1;
 
-    sprintf(buf, "%s %s", TEST_PASSWD, TEST_NPASSWD);
-    if (passwd(buf) != 0) return -1;
+    if (change_passwd(TEST_PASSWD, TEST_NPASSWD) != 0) return -1;
     lock();
 
     if (unlock(TEST_PASSWD) == 0) return -1;
@@ -120,7 +118,7 @@ FUNC_BODY(passwd)
 FUNC_BODY(lock)
 {
     if (lock() == 0) return -1;
-    passwd(TEST_PASSWD);
+    new_passwd(TEST_PASSWD);
     if (lock() != 0) return -1;
     if (lock() != 0) return -1;
     return EXIT_SUCCESS;
@@ -129,7 +127,7 @@ FUNC_BODY(lock)
 FUNC_BODY(unlock)
 {
     int i = MAX_RETRY_COUNT;
-    passwd(TEST_PASSWD);
+    new_passwd(TEST_PASSWD);
     lock();
     while (i > 1) {
         if (unlock(TEST_NPASSWD) != --i) return -1;
@@ -145,7 +143,7 @@ FUNC_BODY(put_key)
 
     if (put_key(TEST_NAMESPACE, TEST_KEYNAME, (unsigned char *)TEST_KEYVALUE,
                 strlen(TEST_KEYVALUE)) == 0) return -1;
-    passwd(TEST_PASSWD);
+    new_passwd(TEST_PASSWD);
     if (put_key(TEST_NAMESPACE, TEST_KEYNAME, (unsigned char *)TEST_KEYVALUE,
                 strlen(TEST_KEYVALUE)) != 0) return -1;
 
@@ -165,7 +163,7 @@ FUNC_BODY(get_key)
 
     if (get_key(TEST_NAMESPACE, TEST_KEYNAME, data, &size) == 0) return -1;
 
-    passwd(TEST_PASSWD);
+    new_passwd(TEST_PASSWD);
     put_key(TEST_NAMESPACE, TEST_KEYNAME, (unsigned char *)TEST_KEYVALUE,
             strlen(TEST_KEYVALUE));
     if (get_key(TEST_NAMESPACE, TEST_KEYNAME, data, &size) != 0) return -1;
@@ -178,7 +176,7 @@ FUNC_BODY(remove_key)
 {
     if (remove_key(TEST_NAMESPACE, TEST_KEYNAME) == 0) return -1;
 
-    passwd(TEST_PASSWD);
+    new_passwd(TEST_PASSWD);
     if (remove_key(TEST_NAMESPACE, TEST_KEYNAME) == 0) return -1;
 
     put_key(TEST_NAMESPACE, TEST_KEYNAME, (unsigned char *)TEST_KEYVALUE,
@@ -199,7 +197,7 @@ FUNC_BODY(list_keys)
 
     if (list_keys(TEST_NAMESPACE, reply) == 0) return -1;
 
-    passwd(TEST_PASSWD);
+    new_passwd(TEST_PASSWD);
     if (list_keys(buf, reply) == 0) return -1;
 
     if (list_keys(TEST_NAMESPACE, reply) != 0) return -1;