feat: APKAM enrollmentId support and atkeys_file refactor #400

JeremyTubongbanua · 2024-09-08T00:19:04Z

closes #287
closes #387
closes #393
closes #375
closes #155

- What I did

Added enrollment_id support to atkeys_file.h/.c and atkeys.h/.c
atclient now builds pkam verb with enrollmentId:, if it exists
Refactored atkeysfile to atkeys_file to be consistent with conventions
Refactored atclient_pkam_authenticate_options usage in atclient_pkam_authenticate and atclient_monitor_pkam_authenticate
- Added functional test to ensure both options = NULL and predefined options work with pkam authentication

- How to verify it

The PKAM Authenticate example using keys that were APKAM'd

[INFO] 2024-09-08 00:15:39.234159 | pkam_authenticate | atclient_atkeys_file_read: 0
[INFO] 2024-09-08 00:15:39.235201 | pkam_authenticate | atclient_atkeys_populate_from_atkeys_file: 0
[INFO] 2024-09-08 00:15:39.235218 | atclient | options is NULL. Using production atDirectory to find atServer host and port
[DEBG] 2024-09-08 00:15:39.693942 | connection |        SENT: "smoothalligator"
[DEBG] 2024-09-08 00:15:39.797274 | connection |        RECV: "245b44d4-a4bd-5f33-b077-c559f956486a.swarm0001.atsign.zone:1722"
[INFO] 2024-09-08 00:15:39.797353 | atclient | atserver_host: 245b44d4-a4bd-5f33-b077-c559f956486a.swarm0001.atsign.zone
[INFO] 2024-09-08 00:15:39.797358 | atclient | atserver_port: 1722
[DEBG] 2024-09-08 00:15:40.457876 | connection |        SENT: "from:smoothalligator"
[DEBG] 2024-09-08 00:15:40.536135 | connection |        RECV: "data:_7ffed872-c543-43d9-8601-b134a6dbd6e2@smoothalligator:bfa908a0-14dd-4203-8879-28425ddd2fa9"
[DEBG] 2024-09-08 00:15:40.545910 | connection |        SENT: "pkam:enrollmentId:f4bdc0a1-2f76-479f-ae41-84275638ee95:XfWj9N2WbIqXBqA4vZxdSjnaBbDDddVCrAVZwWpsnv2kk9lwvlEUE7gDLzDPfqGYSrmwoswPDkQcs40t3Wp+Yh6HRFiUrefArB2wP5Q3l7rcc+A/hV9KL41JSBTkJXd9I93P1UkgM90y1l2WM/0/QIqkl14IvU0mqTlJw1ClyZ413qZG7iKBdYSqlzGOw1k4pfLQLLnDPW7GidnnSdC3A9Gtjwa74UzBYIZEipvApU+Hu9gnZ8PV0ygN4BCBJ/CCdGVwVh6V2Tq7a62bxEx6MKKhtdzGII9/NpNDiTAP42k3kkzdUr6CG2++ICQWag6AdLrQR7IWgKtiCQoBkutFng=="
[DEBG] 2024-09-08 00:15:40.634723 | connection |        RECV: "data:success"
[DEBG] 2024-09-08 00:15:40.634739 | pkam_authenticate | Authenticated

- Description for the changelog
feat: APKAM enrollment support

JeremyTubongbanua · 2024-09-08T01:22:01Z

@gkc @srieteja Can I get your confirmation this is how the PKAM command with an enrollmentId looks like?

[DEBG] 2024-09-08 00:15:40.545910 | connection |        SENT: "pkam:enrollmentId:f4bdc0a1-2f76-479f-ae41-84275638ee95:XfWj9N2WbIqXBqA4vZxdSjnaBbDDddVCrAVZwWpsnv2kk9lwvlEUE7gDLzDPfqGYSrmwoswPDkQcs40t3Wp+Yh6HRFiUrefArB2wP5Q3l7rcc+A/hV9KL41JSBTkJXd9I93P1UkgM90y1l2WM/0/QIqkl14IvU0mqTlJw1ClyZ413qZG7iKBdYSqlzGOw1k4pfLQLLnDPW7GidnnSdC3A9Gtjwa74UzBYIZEipvApU+Hu9gnZ8PV0ygN4BCBJ/CCdGVwVh6V2Tq7a62bxEx6MKKhtdzGII9/NpNDiTAP42k3kkzdUr6CG2++ICQWag6AdLrQR7IWgKtiCQoBkutFng=="

gkc · 2024-09-09T11:58:12Z

@sitaram-kalluri @murali-shris see above question from @JeremyTubongbanua

sitaram-kalluri · 2024-09-09T12:00:31Z

@JeremyTubongbanua : Yes, It is correct. Also, PKAM command optionally accepts signing algo and hashing algo

@gkc @srieteja Can I get your confirmation this is how the PKAM command with an enrollmentId looks like?

[DEBG] 2024-09-08 00:15:40.545910 | connection |        SENT: "pkam:enrollmentId:f4bdc0a1-2f76-479f-ae41-84275638ee95:XfWj9N2WbIqXBqA4vZxdSjnaBbDDddVCrAVZwWpsnv2kk9lwvlEUE7gDLzDPfqGYSrmwoswPDkQcs40t3Wp+Yh6HRFiUrefArB2wP5Q3l7rcc+A/hV9KL41JSBTkJXd9I93P1UkgM90y1l2WM/0/QIqkl14IvU0mqTlJw1ClyZ413qZG7iKBdYSqlzGOw1k4pfLQLLnDPW7GidnnSdC3A9Gtjwa74UzBYIZEipvApU+Hu9gnZ8PV0ygN4BCBJ/CCdGVwVh6V2Tq7a62bxEx6MKKhtdzGII9/NpNDiTAP42k3kkzdUr6CG2++ICQWag6AdLrQR7IWgKtiCQoBkutFng=="

gkc

@JeremyTubongbanua there are changes in atclient.c regarding host and port which seem to be unrelated to the main purpose of this PR ... can you add explanation of those changes to the PR description please?

gkc

@JeremyTubongbanua also monitor's message parsing has changed, can you add description of those changes also please?

XavierChanth · 2024-09-17T20:09:29Z

packages/atclient/include/atclient/monitor.h

-int atclient_monitor_pkam_authenticate(atclient *monitor_conn, const char *atserver_host, const int atserver_port,
-                                       const atclient_atkeys *atkeys, const char *atsign);
+int atclient_monitor_pkam_authenticate(atclient *monitor_conn, const char *atsign, const atclient_atkeys *atkeys,
+                                       atclient_pkam_authenticate_options *options);


Same introduction of options to monitor specific pkam authenticate implementation

XavierChanth · 2024-09-17T20:10:10Z

packages/atclient/src/atclient.c

-  if(options == NULL){
-    atclient_pkam_authenticate_options options;
-    atclient_pkam_authenticate_options_init(&options);
-  }


options is allowed to be NULL now

XavierChanth · 2024-09-17T20:10:28Z

packages/atclient/src/atclient.c

-  /*
-   * 1.1 If atserver_host and atserver_port are null and 0 respectively, then fetch the server host and port.
-   */
-  if (options->atserver_host == NULL && options->atserver_port == 0) {
-    if (atclient_utils_find_atserver_address(options->at_directory_host, options->at_directory_port, atsign,
-                                             &(options->atserver_host), &(options->atserver_port)) != 0) {
-      return ret;
-    }
-  }
-


This scenario is readdressed below

XavierChanth · 2024-09-17T20:14:07Z

packages/atclient/src/atclient.c

+  if (options != NULL) {
+    if (atclient_pkam_authenticate_options_is_atdirectory_host_initialized(options) &&
+        atclient_pkam_authenticate_options_is_atdirectory_port_initialized(options)) {
+      atserver_host = options->atdirectory_host;
+      atserver_port = options->atdirectory_port;
+    }
+  }


Try to populate host and port if provided.

XavierChanth · 2024-09-17T20:14:32Z

packages/atclient/src/atclient.c

+  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);
+      goto exit;
+    }
+    // only free this memory if it was allocated internally (by atclient_utils_find_atserver_address)
+    should_free_atserver_host = true;
  }


If host and port weren't populated from options, look them up.

XavierChanth · 2024-09-17T20:15:10Z

packages/atclient/src/atclient.c

-  const size_t pkam_cmd_size = strlen("pkam:") + signature_base64_len + strlen("\r\n") + 1;
+  size_t pkam_cmd_size = strlen("pkam:");
+  if (atclient_atkeys_is_enrollment_id_initialized((atclient_atkeys *)atkeys) && atkeys->enrollment_id != NULL) {
+    pkam_cmd_size += strlen("enrollmentId:") + strlen(atkeys->enrollment_id) + strlen(":");
+  }
+  pkam_cmd_size += signature_base64_len + strlen("\r\n") + 1;
  if ((pkam_cmd = malloc(sizeof(char) * pkam_cmd_size)) == NULL) {
    atlogger_log(TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to allocate memory for pkam_cmd\n");
    goto exit;
  }
-  snprintf(pkam_cmd, pkam_cmd_size, "pkam:%s\r\n", signature_base64);
+  size_t pos = 0;
+  pos += snprintf(pkam_cmd + pos, pkam_cmd_size - pos, "pkam:");
+
+  if (atclient_atkeys_is_enrollment_id_initialized((atclient_atkeys *)atkeys) && atkeys->enrollment_id != NULL) {
+    pos += snprintf(pkam_cmd + pos, pkam_cmd_size - pos, "enrollmentId:%s:", atkeys->enrollment_id);
+  }
+
+  pos += snprintf(pkam_cmd + pos, pkam_cmd_size - pos, "%s\r\n", signature_base64);


modify pkam verb to conditionally include the enrollment id

XavierChanth · 2024-09-17T20:15:45Z

packages/atclient/src/atclient.c

+  if (should_free_atserver_host) {
+    free(atserver_host);
+  }


free the atserver_host if it was allocated during automatic lookup (wasn't provided by options)

XavierChanth · 2024-09-17T20:16:08Z

packages/atclient/src/atkey.c

formatting and compiler warnings

XavierChanth · 2024-09-17T20:17:14Z

packages/atclient/src/atkeys.c

added enrollment id to atkeys struct and appropriate accessors/mutators (just following the same patterns we have been)

XavierChanth · 2024-09-17T20:18:27Z

packages/atclient/src/atkeys.c

+  // 6. enrollment id, if it exists
+  if (enrollment_id_str != NULL && enrollment_id_str_len > 0) {
+    if ((ret = atclient_atkeys_set_enrollment_id(atkeys, enrollment_id_str, enrollment_id_str_len)) != 0) {
+      atlogger_log(TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "atclient_atkeys_set_enrollment_id: %d\n", ret);
+      goto exit;
+    }
+  }
+


populate from strings if enrollment_id_str isn't NULL or empty

XavierChanth · 2024-09-17T20:19:34Z

packages/atclient/src/atkeys.c

+  if (atclient_atkeysfile_is_enrollment_id_str_initialized((atclient_atkeysfile *)atkeys_file)) {
+    if ((ret = atclient_atkeys_populate_from_strings(
+             atkeys, atkeys_file->aes_pkam_public_key_str, strlen(atkeys_file->aes_pkam_public_key_str),
+             atkeys_file->aes_pkam_private_key_str, strlen(atkeys_file->aes_pkam_private_key_str),
+             atkeys_file->aes_encrypt_public_key_str, strlen(atkeys_file->aes_encrypt_public_key_str),
+             atkeys_file->aes_encrypt_private_key_str, strlen(atkeys_file->aes_encrypt_private_key_str),
+             atkeys_file->self_encryption_key_str, strlen(atkeys_file->self_encryption_key_str),
+             atkeys_file->enrollment_id_str, strlen(atkeys_file->enrollment_id_str))) != 0) {
+      atlogger_log(TAG, ATLOGGER_LOGGING_LEVEL_ERROR,
+                   "atclient_atkeys_populate_from_strings: %d | failed to populate from strings\n", ret);
+      goto exit;
+    }


conditionally add the enrollment id if it is initialized when populating from atkeys file

XavierChanth · 2024-09-17T20:21:49Z

packages/atclient/src/atkeys.c

+}
+}
+
+int atclient_atkeys_populate_from_string(atclient_atkeys *atkeys, const char *file_string) {


new populate variant, similar to populate from path, but is canonical to the atclient_atkeys_file_from_string variant for atkeys_file

XavierChanth · 2024-09-17T20:22:24Z

packages/atclient/src/atkeys.c

+static void unset_enrollment_id(atclient_atkeys *atkeys) {
+  if (is_enrollment_id_initialized(atkeys)) {
+    free(atkeys->enrollment_id);
+  }
+  atkeys->enrollment_id = NULL;
+  set_enrollment_id_initialized(atkeys, false);
+}
+


unset enrollment id, wiping value and removing from initialization vector

XavierChanth · 2024-09-17T20:22:36Z

packages/atclient/src/atkeys.c

+  return ret;
+}
+
+static int set_enrollment_id(atclient_atkeys *atkeys, const char *enrollment_id, const size_t enrollment_id_len) {


enrollment id setter

XavierChanth · 2024-09-17T20:22:53Z

packages/atclient/src/atkeys_file.c

renamed from atkeysfile.c

XavierChanth · 2024-09-17T20:24:22Z

packages/atclient/src/atkeys_file.c

+int atclient_atkeysfile_from_string(atclient_atkeysfile *atkeys_file, const char *file_string) {
+  int ret = 1;
+
+  cJSON *root = cJSON_Parse(file_string);


atclient_atkeysfile_from_string variant: we cJSON_Parse the file at file_string, then do all the same, using cjson to extract the atkeys file contents.

XavierChanth · 2024-09-17T20:26:39Z

packages/atclient/src/atnotification.c

formatting and compiler warnings

XavierChanth · 2024-09-17T20:27:00Z

packages/atclient/src/connection.c

formatting and compiler warnings

XavierChanth · 2024-09-17T20:27:21Z

packages/atclient/src/monitor.c

-int atclient_monitor_pkam_authenticate(atclient *monitor_conn, const char *atserver_host, const int atserver_port,
-                                       const atclient_atkeys *atkeys, const char *atsign) {
+int atclient_monitor_pkam_authenticate(atclient *monitor_conn, const char *atsign, const atclient_atkeys *atkeys,
+                                       atclient_pkam_authenticate_options *options) {


same signature change as mentioned before

XavierChanth · 2024-09-17T20:27:48Z

packages/atclient/src/monitor.c

-  atclient_pkam_authenticate_options_init(&options);
-  ret = atclient_pkam_authenticate(monitor_conn, atsign, atkeys, &options);
-  if (ret != 0) {
+  if ((ret = atclient_pkam_authenticate(monitor_conn, atsign, atkeys, options)) != 0) {


defer to the other pkam_authenticate method - monitor's variant is just a wrapper around the normal pkam auth

XavierChanth · 2024-09-17T20:29:26Z

packages/atclient/src/monitor.c

-  int i = 0;
-  while (buffer[i] != ':') {
-    i++;
-  }


this was a hack to align the monitor messages, which should be resolved by the parse_message changes below

XavierChanth · 2024-09-18T02:14:48Z

tests/functional_tests/tests/test_atclient_pkam_authenticate.c

+  return ret;
+}
+
+static int test1_pkam_no_options() {


Test pkam when options = NULL

XavierChanth · 2024-09-18T02:15:09Z

tests/functional_tests/tests/test_atclient_pkam_authenticate.c

+  return ret;
+}
+
+static int test2_pkam_with_options() {


Test pkam auth when we've cached atServer connection info in options

XavierChanth · 2024-09-18T02:26:21Z

Tested here using C sshnpd: https://asciinema.org/a/n5MWtuUxF2lAGD0oSk7E0Wsc3

gkc

LGTM

feat: APKAM enrollment support

5cbb247

JeremyTubongbanua self-assigned this Sep 8, 2024

JeremyTubongbanua added 2 commits September 7, 2024 20:23

fix: helper functioanl test

5338eeb

fix:

39e0738

JeremyTubongbanua changed the title ~~feat: APKAM enrollment support~~ feat: APKAM enrollment support and atkeys_file refactor Sep 8, 2024

JeremyTubongbanua changed the title ~~feat: APKAM enrollment support and atkeys_file refactor~~ feat: APKAM enrollmentId support and atkeys_file refactor Sep 8, 2024

JeremyTubongbanua marked this pull request as ready for review September 8, 2024 00:33

JeremyTubongbanua requested review from XavierChanth, sitaram-kalluri, cconstab, realvarx and srieteja September 8, 2024 00:34

JeremyTubongbanua added 5 commits September 7, 2024 20:37

fix: atclient_monitor_pkam

d16c0bf

fix: examples

93b0120

fix: buildl examples

3da35a3

fix: parse msg

857f3e2

fix: parse message header

1f58601

JeremyTubongbanua marked this pull request as draft September 8, 2024 00:56

JeremyTubongbanua added 3 commits September 7, 2024 21:00

fix:

9caea86

chore:

28b8159

fix:

60955e5

JeremyTubongbanua marked this pull request as ready for review September 8, 2024 01:13

This was referenced Sep 8, 2024

feat: pkam enrollment id support atsign-foundation/noports#1328

Merged

C daemon does not work with APKAM atKeys atsign-foundation/noports#1326

Closed

JeremyTubongbanua requested a review from gkc September 8, 2024 01:21

gkc reviewed Sep 10, 2024

View reviewed changes