chore: address review comments from 17 dec

packages/atauth/include/atauth/atactivate_arg_parser.h

@@ -11,17 +11,18 @@
  *
  * @param argc The number of arguments
  * @param argv The array of arguments
- * @param atsign pointer to store the atsign value
- * @param cram_secret pointer to store the cram_secret value
- * @param otp OTP fetched from otp verb handler
- * @param atkeys_fp pointer to store the file path of the atkeys
- * @param app_name
- * @param device_name
- * @param namespaces
- * @param root_host pointer to store the root host server address
+ * @param atsign pointer to store the parsed atsign value
+ * @param cram_secret pointer to store the parsed cram_secret value
+ * @param otp OTP pointer to store the parsed OTP/SPP value (accepts OTP fetched from OTP verb)
+ * @param atkeys_fp pointer to store the parsed file path of the atkeys
+ * @param app_name pointer to store the parsed app_name for current enrollment
+ * @param device_name pointer to store the parsed device_name for current enrollment
+ * @param namespaces pointert to store the parsed namespaces list for current enrollment
+ * @param root_host pointer to store the parsed root server host
+ * @param root_port pointer to store the parsed root server port
  * @return int 0 on success, non-zero on error
  */
 int atactivate_parse_args(int argc, char *argv[], char **atsign, char **cram_secret, char **otp, char **atkeys_fp,
-                          char **app_name, char **device_name, char **namespaces, char **root_host);
+                          char **app_name, char **device_name, char **namespaces, char **root_host, int *root_port);
 
 #endif // ATACTIVATE_ARG_PARSER_H

packages/atauth/src/atactivate.c

@@ -21,6 +21,7 @@
 int main(int argc, char *argv[]) {
   int ret = 0;
   char *atsign_temp = NULL, *cram_secret = NULL, *root_host = NULL, *atkeys_fp = NULL, *otp = NULL;
+  int root_port = 0;
   char enrollment_id[ENROLL_ID_MAX_LEN];
   char status[ATCOMMONS_ENROLL_STATUS_STRING_MAX_LEN];
 
@@ -137,7 +138,7 @@ int main(int argc, char *argv[]) {
    * 1. Parse args
    */
   if ((ret = atactivate_parse_args(argc, argv, &atsign_temp, &cram_secret, &otp, &atkeys_fp, NULL, NULL, NULL,
-                                   &root_host)) != 0) {
+                                   &root_host, &root_port)) != 0) {
     goto exit;
   }
 

packages/atauth/src/atactivate_arg_parser.c

@@ -1,29 +1,23 @@
 #include "atauth/atactivate_arg_parser.h"
+#include <atclient/constants.h>
 #include <atlogger/atlogger.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
 
-#define DEFAULT_ROOT_SERVER "root.atsign.org"
-#define DEFAULT_ROOT_PORT 64
+int parse_root_domain(const char *root_domain_string, char **root_host, int *root_port);
 
-/// ToDO: add impl to read the root server FQDN then parse it. Currently only accepts root host, cannot parse root port
 int atactivate_parse_args(const int argc, char *argv[], char **atsign, char **cram_secret, char **otp, char **atkeys_fp,
-                          char **app_name, char **device_name, char **namespaces, char **root_host) {
-  int ret = 0;
-  int opt;
-
-  // Initialize defaults
-  *root_host = malloc(sizeof(char) * strlen(DEFAULT_ROOT_SERVER) + 1);
-  if (*root_host == NULL) {
-    fprintf(stderr, "Memory allocation failed for root_host\n");
-    return -1;
-  }
-  strcpy(*root_host, DEFAULT_ROOT_SERVER);
+                          char **app_name, char **device_name, char **namespaces, char **root_host, int *root_port) {
+  int ret = 0, opt = 0;
+  char *root_fqdn = NULL;
+  const char *usage = "Usage: \n\tActivate: \t./atactivate -a atsign -c cram-secret [-k path_to_store_keysfile] [-r root-domain]"
+                      "\n\n\tNew enrollment: ./at_auth_cli -a atsign -s otp/spp -p app_name -d device_name -n "
+                      "namespaces(\"wavi:rw,buzz:r\") [-k path_to_store_keysfile] [-r root-domain]\n";
 
   // Parse command-line arguments
-  while ((opt = getopt(argc, argv, "a:c:k:o:p:d:n:r:vh")) != -1) {
+  while ((opt = getopt(argc, argv, "a:c:k:s:p:d:n:r:vh")) != -1) {
     switch (opt) {
     case 'a':
       *atsign = malloc(sizeof(char) * strlen(optarg) + 1);
@@ -56,12 +50,12 @@ int atactivate_parse_args(const int argc, char *argv[], char **atsign, char **cr
       }
       strcpy(*atkeys_fp, optarg);
       break;
-    case 'o':
+    case 's':
       if (otp == NULL)
         break;
       *otp = malloc(sizeof(char) * strlen(optarg));
       if (*otp == NULL) {
-        fprintf(stderr, "Memory allocation failed for atkeys file path\n");
+        fprintf(stderr, "Memory allocation failed for OTP\n");
         ret = -1;
         goto exit;
       }
@@ -70,7 +64,7 @@ int atactivate_parse_args(const int argc, char *argv[], char **atsign, char **cr
     case 'p':
       if (app_name == NULL)
         break;
-      *app_name = realloc(*root_host, sizeof(char) * strlen(optarg) + 1);
+      *app_name = malloc(sizeof(char) * strlen(optarg) + 1);
       if (*app_name == NULL) {
         fprintf(stderr, "Memory reallocation failed for app_name\n");
         ret = -1;
@@ -81,7 +75,7 @@ int atactivate_parse_args(const int argc, char *argv[], char **atsign, char **cr
     case 'd':
       if (device_name == NULL)
         break;
-      *device_name = realloc(*device_name, sizeof(char) * strlen(optarg) + 1);
+      *device_name = malloc(sizeof(char) * strlen(optarg) + 1);
       if (*device_name == NULL) {
         fprintf(stderr, "Memory reallocation failed for device_name\n");
         ret = -1;
@@ -92,7 +86,7 @@ int atactivate_parse_args(const int argc, char *argv[], char **atsign, char **cr
     case 'n':
       if (namespaces == NULL)
         break;
-      *namespaces = realloc(*namespaces, sizeof(char) * strlen(optarg) + 1);
+      *namespaces = malloc(sizeof(char) * strlen(optarg) + 1);
       if (*namespaces == NULL) {
         fprintf(stderr, "Memory reallocation failed for namespaces\n");
         ret = -1;
@@ -101,39 +95,58 @@ int atactivate_parse_args(const int argc, char *argv[], char **atsign, char **cr
       strcpy(*namespaces, optarg);
       break;
     case 'r':
-      *root_host = realloc(*root_host, sizeof(char) * strlen(optarg) + 1);
-      if (*root_host == NULL) {
-        fprintf(stderr, "Memory reallocation failed for root_host\n");
+      root_fqdn = malloc(sizeof(char) * strlen(optarg) + 1);
+      if (root_fqdn == NULL) {
+        fprintf(stderr, "Memory allocation failed for root_host\n");
         ret = -1;
         goto exit;
       }
-      strcpy(*root_host, optarg);
+      strcpy(root_fqdn, optarg);
       break;
     case 'v':
       atlogger_set_logging_level(ATLOGGER_LOGGING_LEVEL_DEBUG);
       break;
     case 'h':
-      fprintf(stderr, "Usage: %s -a atsign -c cram-secret -o otp [-r root-server] [-p port]\n", argv[0]);
-      exit(0); // force exit to display usage
+      fprintf(stdout, usage);
+      ret = 0;
+      goto exit;
     default:
-      fprintf(stderr, "Usage: %s -a atsign -c cram-secret -o otp [-r root-server] [-p port]\n", argv[0]);
+      fprintf(stderr, usage);
       ret = -1;
       goto exit;
     }
   }
 
+  // set default root server address if not provided through CLI
+  if (root_fqdn == NULL || parse_root_domain(root_fqdn, root_host, root_port) != 0) {
+    *root_host = strdup(ATCLIENT_ATDIRECTORY_PRODUCTION_HOST);
+    *root_port = ATCLIENT_ATDIRECTORY_PRODUCTION_PORT;
+  }
+
   if (atsign == NULL) {
     fprintf(stderr, "Error: -a (atsign) is mandatory.\n");
-    fprintf(stderr, "Usage: %s -a atsign -c cram-secret -o otp [-r root-server] [-p port]\n", argv[0]);
+    fprintf(stderr, usage);
     ret = 1;
   }
 
   if (cram_secret == NULL && otp == NULL) {
     fprintf(stderr, "Cannot proceed without either of CRAM secret or enroll OTP.\n");
-    fprintf(stderr, "Usage: %s -a atsign -c cram-secret -o otp [-r root-server] [-p port]\n", argv[0]);
+    fprintf(stderr, usage);
     ret = 1;
   }
 
 exit:
   return ret;
 }
+
+int parse_root_domain(const char *root_domain_string, char **root_host, int *root_port) {
+  if(root_domain_string == NULL) {
+    return 1;
+  }
+  *root_host = strdup(strtok((char *)root_domain_string, ":"));
+  *root_port = atoi(strtok(NULL, ":"));
+  if(*root_host == NULL || root_port == NULL) {
+    return 1;
+  }
+  return 0;
+}

packages/atauth/src/auth_cli.c

@@ -27,7 +27,8 @@ int create_new_atserver_connection(atclient *ctx, const char *atsign, const atcl
 int atauth_validate_args(const char *otp, const char *app_name, const char *device_name, const char *namespaces_str);
 
 int main(int argc, char *argv[]) {
-  int ret = 0;
+  atlogger_set_logging_level(ATLOGGER_LOGGING_LEVEL_INFO);
+  int ret = 0, root_port = 0;
   char *atsign_temp = NULL, *root_host = NULL, *atkeys_fp = NULL, *otp = NULL, *app_name = NULL, *device_name = NULL,
        *namespaces_str = NULL;
 
@@ -66,7 +67,7 @@ int main(int argc, char *argv[]) {
    * 1. Parse + validate command-line arguments
    */
   if ((ret = atactivate_parse_args(argc, argv, &atsign_temp, NULL, &otp, &atkeys_fp, &app_name, &device_name,
-                                   &namespaces_str, &root_host)) != 0) {
+                                   &namespaces_str, &root_host, &root_port)) != 0) {
     goto exit;
   }
 
@@ -110,7 +111,7 @@ int main(int argc, char *argv[]) {
   atclient_atkeys_set_pkam_private_key_base64(&atkeys, (const char *)pkam_private_key_base64,
                                               strlen((const char *)pkam_private_key_base64));
 
-  // 2.1.2 populate the pkam public/private key bytes in the atkeys struct by parsing the base64 formats
+  // 2.1.2 populate the pkam public/private key bytes in the atkeys struct by parsing the base64 encoded keys
   atclient_atkeys_populate_pkam_public_key(&atkeys, (const char *)pkam_public_key_base64,
                                            strlen((const char *)pkam_public_key_base64));
   atclient_atkeys_populate_pkam_private_key(&atkeys, (const char *)pkam_private_key_base64,
@@ -120,14 +121,14 @@ int main(int argc, char *argv[]) {
   atclient_authenticate_options opts;
   atclient_authenticate_options_init(&opts);
   atclient_authenticate_options_set_atdirectory_host(&opts, root_host);
+  atclient_authenticate_options_set_atdirectory_port(&opts, root_port);
 
   atclient at_client;
   atclient_init(&at_client);
-  atclient_set_atsign(&at_client, atsign);
 
   // 2.2.1 Start new connection
   if ((ret = create_new_atserver_connection(&at_client, atsign, &opts)) != 0) {
-    atlogger_log(TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "create_new_atserver_connection: %d\n", ret);
+    atlogger_log(TAG, ATLOGGER_LOGGING_LEVEL_DEBUG, "create_new_atserver_connection: %d\n", ret);
     goto pkam_pub_keys_exit;
   }
 
@@ -190,10 +191,11 @@ int main(int argc, char *argv[]) {
 
   // 2.5.1 base64 encode the encrypted APKAM symmetric key
   size_t encrypted_apkam_symmetric_key_base64_len = 0;
-  if ((ret = atchops_base64_encode(
-           (unsigned char *)encrypted_apkam_symmetric_key_bytes, sizeof(unsigned char) * rsa_2048_ciphertext_size,
-           (unsigned char *)encrypted_apkam_symmetric_key_base64, sizeof(unsigned char) * base64_encoded_rsa2048_ciphertext_size,
-           &encrypted_apkam_symmetric_key_base64_len)) != 0) {
+  if ((ret = atchops_base64_encode((unsigned char *)encrypted_apkam_symmetric_key_bytes,
+                                   sizeof(unsigned char) * rsa_2048_ciphertext_size,
+                                   (unsigned char *)encrypted_apkam_symmetric_key_base64,
+                                   sizeof(unsigned char) * base64_encoded_rsa2048_ciphertext_size,
+                                   &encrypted_apkam_symmetric_key_base64_len)) != 0) {
     atlogger_log(TAG, ATLOGGER_LOGGING_LEVEL_ERROR,
                  "Failed base64 encoding encrypted_apkam_symmetric_key | atchops_base64_encode: %d\n", ret);
     goto enc_pub_key_exit;
@@ -204,6 +206,10 @@ int main(int argc, char *argv[]) {
    */
   // 3.1 Initialize and populate enrollment params structs
   atcommons_enroll_namespace_list_t *ns_list = malloc(sizeof(atcommons_enroll_namespace_list_t));
+  if (ns_list == NULL) {
+    atlogger_log(TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Could not allocate memory for namespace list\n");
+    goto enc_pub_key_exit;
+  }
 
   // 3.1.1 parse namespace list string passed through command-line args
   if ((ret = atcommons_enroll_namespace_list_from_string(&ns_list, namespaces_str)) != 0) {
@@ -504,27 +510,23 @@ int is_enrollment_denied(const char *err_msg) {
 int create_new_atserver_connection(atclient *ctx, const char *atsign, const atclient_authenticate_options *options) {
   char *atserver_host = NULL;
   int atserver_port = 0, ret = 0;
-  if (options != NULL) {
-    if (atclient_authenticate_options_is_atdirectory_host_initialized(options) &&
-        atclient_authenticate_options_is_atdirectory_port_initialized(options)) {
-      atserver_host = options->atdirectory_host;
-      atserver_port = options->atdirectory_port;
-    }
-  }
 
   if (atserver_host == NULL || atserver_port == 0) {
-    atlogger_log(TAG, ATLOGGER_LOGGING_LEVEL_INFO,
-                 "Missing atServer host or port. Using production atDirectory to look up atServer host and port\n");
-    if ((ret = atclient_utils_find_atserver_address(ATCLIENT_ATDIRECTORY_PRODUCTION_HOST,
-                                                    ATCLIENT_ATDIRECTORY_PRODUCTION_PORT, atsign, &atserver_host,
-                                                    &atserver_port)) != 0) {
-      atlogger_log(TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "atclient_utils_find_atserver_address: %d\n", ret);
+    atlogger_log(TAG, ATLOGGER_LOGGING_LEVEL_INFO, "Fetching secondary server address for atsign: %s\n", atsign);
+    if ((ret = atclient_utils_find_atserver_address(options->atdirectory_host, options->atdirectory_port, atsign,
+                                                    &atserver_host, &atserver_port)) != 0) {
+      atlogger_log(TAG, ATLOGGER_LOGGING_LEVEL_DEBUG, "atclient_utils_find_atserver_address: %d\n", ret);
+      atlogger_log(TAG, ATLOGGER_LOGGING_LEVEL_ERROR,
+                   "Could not fetch secondary address for atsign: %s on root directory: %s:%d\n", atsign,
+                   options->atdirectory_host, options->atdirectory_port);
       goto exit;
     }
   }
 
   if ((ret = atclient_start_atserver_connection(ctx, atserver_host, atserver_port)) != 0) {
-    atlogger_log(TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "atclient_start_atserver_connection: %d\n", ret);
+    atlogger_log(TAG, ATLOGGER_LOGGING_LEVEL_DEBUG, "atclient_start_atserver_connection: %d\n", ret);
+    atlogger_log(TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Could not connect to secondary server at %s:%d\n", atserver_host,
+                 atserver_port);
   }
 
 exit: { return ret; }

packages/atauth/src/send_enroll_request.c

@@ -19,7 +19,7 @@ int atauth_validate_send_enroll_request_arguments(const atclient *client, const
 int atauth_send_enroll_request(atclient *client, const atcommons_enroll_params_t *ep, char *enroll_id,
                                char *enroll_status) {
   int ret = 0;
-  const size_t recv_size = 100; // to hold the response for enroll request
+  const size_t recv_size = 300; // to hold the response for enroll request
   unsigned char recv[recv_size];
   char *recv_trimmed = NULL;
   size_t recv_len;

packages/atclient/include/atclient/constants.h

@@ -20,10 +20,6 @@ extern "C" {
 
 #define ATCLIENT_ERR_AT0015_KEY_NOT_FOUND -0x1980
 
-// default param values for ATCLIENT PKAM AUTHENTICATE OPTIONS
-#define ATCLIENT_DEFAULT_AT_DIRECTORY_HOST "root.atsign.org"
-#define ATCLIENT_DEFAULT_AT_DIRECTORY_PORT 64
-
 #define ATCLIENT_DATA_TOKEN "data:"
 
 #define ATCLIENT_CRAM_PREFIX "cram"

packages/atclient/include/atclient/request_options.h

@@ -32,10 +32,8 @@ extern "C" {
 
 #define ATCLIENT_GET_PUBLIC_KEY_REQUEST_OPTIONS_BYPASS_CACHE_INDEX 0
 #define ATCLIENT_GET_PUBLIC_KEY_REQUEST_OPTIONS_STORE_ATKEY_METADATA_INDEX 0
-#define ATCLIENT_GET_PUBLIC_KEY_REQUEST_OPTIONS_SHOULD_AUTH_INDEX 0
 #define ATCLIENT_GET_PUBLIC_KEY_REQUEST_OPTIONS_BYPASS_CACHE_INITIALIZED (VALUE_INITIALIZED << 0)
 #define ATCLIENT_GET_PUBLIC_KEY_REQUEST_OPTIONS_STORE_ATKEY_METADATA_INITIALIZED (VALUE_INITIALIZED << 1)
-#define ATCLIENT_GET_PUBLIC_KEY_REQUEST_OPTIONS_SHOULD_AUTH_INITIALIZED (VALUE_INITIALIZED << 2)
 
 #define ATCLIENT_GET_ATKEYS_REQUEST_OPTIONS_REGEX_INDEX 0
 #define ATCLIENT_GET_ATKEYS_REQUEST_OPTIONS_SHOW_HIDDEN_INDEX 0
@@ -108,7 +106,6 @@ typedef struct atclient_get_shared_key_request_options {
 typedef struct atclient_get_public_key_request_options {
   bool bypass_cache;
   bool store_atkey_metadata;
-  bool should_auth;
   uint8_t _initialized_fields[1];
 } atclient_get_public_key_request_options;
 
@@ -233,12 +230,6 @@ int atclient_get_public_key_request_options_set_store_atkey_metadata(atclient_ge
 void atclient_get_public_key_request_options_unset_store_atkey_metadata(
     atclient_get_public_key_request_options *options);
 
- bool atclient_get_public_key_request_options_is_should_auth_initialized(
-    const atclient_get_public_key_request_options *options);
- int atclient_get_public_key_request_options_set_should_auth(atclient_get_public_key_request_options *options,
-                                                              const bool should_auth);
- void atclient_get_public_key_request_options_unset_should_auth(atclient_get_public_key_request_options *options);
-
 /*
  * 3. Delete
  */