From f6a5f06b43dca32bc72309aca83422c3049eef31 Mon Sep 17 00:00:00 2001 From: xavierchanth Date: Fri, 27 Sep 2024 21:16:38 -0400 Subject: [PATCH 1/7] feat: permit-open --- packages/c/sshnpd/CMakeLists.txt | 1 + packages/c/sshnpd/include/sshnpd/params.h | 5 +- packages/c/sshnpd/include/sshnpd/permitopen.h | 25 ++++ packages/c/sshnpd/src/handle_npt_request.c | 63 +++++--- packages/c/sshnpd/src/main.c | 42 +++++- packages/c/sshnpd/src/params.c | 137 +++++++++--------- packages/c/sshnpd/src/permitopen.c | 101 +++++++++++++ 7 files changed, 278 insertions(+), 96 deletions(-) create mode 100644 packages/c/sshnpd/include/sshnpd/permitopen.h create mode 100644 packages/c/sshnpd/src/permitopen.c diff --git a/packages/c/sshnpd/CMakeLists.txt b/packages/c/sshnpd/CMakeLists.txt index 498eabadc..fdf243c2d 100644 --- a/packages/c/sshnpd/CMakeLists.txt +++ b/packages/c/sshnpd/CMakeLists.txt @@ -18,6 +18,7 @@ set( ${CMAKE_CURRENT_LIST_DIR}/src/handler_commons.c ${CMAKE_CURRENT_LIST_DIR}/src/main.c ${CMAKE_CURRENT_LIST_DIR}/src/params.c + ${CMAKE_CURRENT_LIST_DIR}/src/permitopen.c ${CMAKE_CURRENT_LIST_DIR}/src/run_srv_process.c ) diff --git a/packages/c/sshnpd/include/sshnpd/params.h b/packages/c/sshnpd/include/sshnpd/params.h index 9d814f493..d87fec7d3 100644 --- a/packages/c/sshnpd/include/sshnpd/params.h +++ b/packages/c/sshnpd/include/sshnpd/params.h @@ -21,8 +21,11 @@ struct _sshnpd_params { size_t manager_list_len; char **manager_list; + char *policy; + size_t permitopen_len; - char **permitopen; + char **permitopen_hosts; + uint16_t *permitopen_ports; // 0 = '*' char *permitopen_str; bool should_free_permitopen_str; diff --git a/packages/c/sshnpd/include/sshnpd/permitopen.h b/packages/c/sshnpd/include/sshnpd/permitopen.h new file mode 100644 index 000000000..eb5bcca4b --- /dev/null +++ b/packages/c/sshnpd/include/sshnpd/permitopen.h @@ -0,0 +1,25 @@ +#ifndef SSHNPD_PERMITOPEN_H +#define SSHNPD_PERMITOPEN_H +#include +#include +#include +#include + +// atlogger won't be available during the initial parsing of the parameters +// (since we are waiting for the verbose flag to be set) +int parse_permitopen(char *input, char ***permitopen_hosts, uint16_t **permitopen_ports, size_t *permitopen_len, + bool is_logger_available); + +struct _permitopen_params { + char *requested_host; + uint16_t requested_port; + + char **permitopen_hosts; + uint16_t *permitopen_ports; + size_t permitopen_len; +}; + +typedef struct _permitopen_params permitopen_params; + +bool should_permitopen(struct _permitopen_params *params); +#endif diff --git a/packages/c/sshnpd/src/handle_npt_request.c b/packages/c/sshnpd/src/handle_npt_request.c index 3b2c2f81c..53534e506 100644 --- a/packages/c/sshnpd/src/handle_npt_request.c +++ b/packages/c/sshnpd/src/handle_npt_request.c @@ -1,5 +1,6 @@ #include "atclient/request_options.h" #include "sshnpd/params.h" +#include "sshnpd/permitopen.h" #include "sshnpd/sshnpd.h" #include #include @@ -106,6 +107,24 @@ void handle_npt_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn return; } + // NPT ONLY + // Don't try optimizing this to reuse the permitopen struct from main.c. + // none of the memory duplication here is expensive, and it's a surface for bugs + permitopen_params permitopen; + permitopen.permitopen_len = params->permitopen_len; + permitopen.permitopen_hosts = params->permitopen_hosts; + permitopen.permitopen_ports = params->permitopen_ports; + permitopen.requested_host = cJSON_GetStringValue(requested_host); + permitopen.requested_port = cJSON_GetNumberValue(requested_port); + + if (!should_permitopen(&permitopen)) { + atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_DEBUG, "Ignoring request to localhost:%d\n", + permitopen.requested_port); + cJSON_Delete(envelope); + return; + } + // END NPT ONLY + // These values do not need to be asserted for v4 compatibility, only for v5 cJSON *auth_to_rvd = cJSON_GetObjectItem(payload, "authenticateToRvd"); @@ -138,6 +157,7 @@ void handle_npt_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn char *buffer = NULL; res = atclient_get_public_key(atclient, &atkey, &buffer, NULL); + atclient_atkey_free(&atkey); if (res != 0) { atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to get public key\n"); atclient_atkey_free(&atkey); @@ -145,8 +165,6 @@ void handle_npt_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn return; } - atclient_atkey_free(&atkey); - atchops_rsa_key_public_key requesting_atsign_publickey; atchops_rsa_key_public_key_init(&requesting_atsign_publickey); @@ -272,7 +290,7 @@ void handle_npt_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn if (!encrypt_rvd_traffic) { atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "encryptRvdTraffic=false is not supported by this daemon\n"); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } cJSON_Delete(envelope); return; @@ -284,7 +302,7 @@ void handle_npt_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn "encryptRvdTraffic was requested, but no client ephemeral public key / key type was provided\n"); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } cJSON_Delete(envelope); return; @@ -294,7 +312,7 @@ void handle_npt_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn if ((res = atchops_aes_generate_key(key, ATCHOPS_AES_256)) != 0) { atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to generate session aes key\n"); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } cJSON_Delete(envelope); return; @@ -305,7 +323,7 @@ void handle_npt_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn if (res != 0) { atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to generate session aes key\n"); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } cJSON_Delete(envelope); return; @@ -315,7 +333,7 @@ void handle_npt_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn if ((res = atchops_iv_generate(iv)) != 0) { atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to generate session iv\n"); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } cJSON_Delete(envelope); return; @@ -326,7 +344,7 @@ void handle_npt_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn if (res != 0) { atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to generate session iv\n"); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } cJSON_Delete(envelope); return; @@ -348,7 +366,7 @@ void handle_npt_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to populate client ephemeral pk\n"); atchops_rsa_key_public_key_free(&ac); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } cJSON_Delete(envelope); return; @@ -360,7 +378,7 @@ void handle_npt_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn "Failed to allocate memory to encrypt the session aes key\n"); atchops_rsa_key_public_key_free(&ac); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } cJSON_Delete(envelope); return; @@ -371,7 +389,7 @@ void handle_npt_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to encrypt the session aes key\n"); atchops_rsa_key_public_key_free(&ac); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } free(session_aes_key_encrypted); cJSON_Delete(envelope); @@ -387,7 +405,7 @@ void handle_npt_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn "Failed to allocate memory to base64 encode the session aes key\n"); atchops_rsa_key_public_key_free(&ac); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } free(session_aes_key_encrypted); cJSON_Delete(envelope); @@ -402,7 +420,7 @@ void handle_npt_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to base64 encode the session aes key\n"); atchops_rsa_key_public_key_free(&ac); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } free(session_aes_key_base64); free(session_aes_key_encrypted); @@ -418,7 +436,7 @@ void handle_npt_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to allocate memory to encrypt the session iv\n"); atchops_rsa_key_public_key_free(&ac); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } free(session_aes_key_base64); cJSON_Delete(envelope); @@ -431,7 +449,7 @@ void handle_npt_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn if (res != 0) { atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to encrypt the session iv\n"); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } free(session_iv_encrypted); free(session_aes_key_base64); @@ -446,7 +464,7 @@ void handle_npt_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to allocate memory to base64 encode the session iv\n"); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } free(session_iv_encrypted); free(session_aes_key_base64); @@ -461,7 +479,7 @@ void handle_npt_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn if (res != 0) { atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to base64 encode the session iv\n"); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } free(session_iv_base64); free(session_iv_encrypted); @@ -480,7 +498,7 @@ void handle_npt_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "%s is not an accepted key type for encrypting the aes key\n", pk_type); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } cJSON_Delete(envelope); return; @@ -517,13 +535,10 @@ void handle_npt_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn int res = run_srv_process(rvd_host_str, rvd_port_int, requested_host_str, requested_port_int, authenticate_to_rvd, rvd_auth_string, encrypt_rvd_traffic, multi, session_aes_key, session_iv); - free(rvd_host_str); - free(requested_host_str); - *is_child_process = true; if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } cJSON_Delete(envelope); exit(res); @@ -652,7 +667,7 @@ void handle_npt_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn clean_res: { free(keyname); } clean_final_res_value: { atclient_atkey_free(&final_res_atkey); - free(final_res_value); + cJSON_free(final_res_value); } clean_json: { cJSON_Delete(final_res_envelope); @@ -665,7 +680,7 @@ void handle_npt_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn } cancel: if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } if (free_session_base64) { free(session_iv_base64); diff --git a/packages/c/sshnpd/src/main.c b/packages/c/sshnpd/src/main.c index 8751d3f78..5d08271a6 100644 --- a/packages/c/sshnpd/src/main.c +++ b/packages/c/sshnpd/src/main.c @@ -3,6 +3,7 @@ #include "sshnpd/handle_ping.h" #include "sshnpd/handle_ssh_request.h" #include "sshnpd/handle_sshpublickey.h" +#include "sshnpd/permitopen.h" #include "sshnpd/sshnpd.h" #include "sshnpd/version.h" #include @@ -209,6 +210,11 @@ int main(int argc, char **argv) { // atclient_get_public_encryption_key(&atclient, params.manager_list[i], &public_encryption_key); // TODO: finish caching } + if (params.policy == NULL) { + atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_DEBUG, "Policy Manager: NULL"); + } else { + atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_DEBUG, "Policy Manager: %s", params.policy); + } printf("\n"); if (!should_run) { @@ -231,9 +237,13 @@ int main(int argc, char **argv) { cJSON_AddItemToObject(ping_response_json, "supportedFeatures", supported_features); cJSON *allowed_services = cJSON_CreateArray(); + char *buf = malloc(sizeof(char) * 1024); for (int i = 0; i < params.permitopen_len; i++) { - cJSON_AddItemToArray(allowed_services, cJSON_CreateString(params.permitopen[i])); + sprintf(buf, "%s:%u", params.permitopen_hosts[i], (unsigned int)params.permitopen_ports[i]); + cJSON_AddItemToArray(allowed_services, cJSON_CreateString(buf)); } + free(buf); + cJSON_AddItemToObject(ping_response_json, "allowedServices", allowed_services); // @@ -383,9 +393,9 @@ int main(int argc, char **argv) { exit: free(params.manager_list); - free(params.permitopen); + free(params.permitopen_hosts); + free(params.permitopen_ports); free(params.permitopen_str); - exit(exit_res); } @@ -399,6 +409,13 @@ void main_loop() { atclient_monitor_response message; + permitopen_params permitopen; + permitopen.permitopen_len = params.permitopen_len; + permitopen.permitopen_hosts = params.permitopen_hosts; + permitopen.permitopen_ports = params.permitopen_ports; + + permitopen_params npa_permitopen; + while (should_run) { atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_DEBUG, "Waiting for next monitor thread message\n"); atclient_monitor_response_init(&message); @@ -408,7 +425,6 @@ void main_loop() { atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_DEBUG, "Received message of type: %d\n", message.type); - // in code -> clang-format -> out code switch (message.type) { case ATCLIENT_MONITOR_ERROR_READ: if (!atclient_monitor_is_connected(&monitor_ctx)) { @@ -502,7 +518,14 @@ void main_loop() { break; } + if (params.policy != NULL) { + // TODO: implement a separate permitopen check for npa checks + // DO NOT USE permitopen, use npa_permitopen + } + // TODO: maybe multithread these handlers + char *requested_host = NULL; + uint16_t requested_port = 0; switch (notification_key) { case NK_SSHPUBLICKEY: atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_DEBUG, "Executing handle_sshpublickey\n"); @@ -514,6 +537,15 @@ void main_loop() { break; case NK_SSH_REQUEST: atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_DEBUG, "Executing handle_ssh_request\n"); + // permitopen happens first for ssh so we can avoid a bunch of unnecessary tasks + permitopen.requested_host = "localhost"; + permitopen.requested_port = params.local_sshd_port; + if (!should_permitopen(&permitopen)) { + atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_DEBUG, "Ignoring request to localhost:%d\n", + params.local_sshd_port); + // TODO notify daemon doesn't permit connections to $requested_host:$requested_port + break; + } handle_ssh_request(&worker, &atclient_lock, ¶ms, &is_child_process, &message, home_dir, authkeys_file, authkeys_filename, signingkey); if (is_child_process) { @@ -524,6 +556,8 @@ void main_loop() { break; case NK_NPT_REQUEST: atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_DEBUG, "Executing handle_npt_request\n"); + // No permitopen here... since we need to parse the json first in order to check, it happens inside + // handle_npt_request handle_npt_request(&worker, &atclient_lock, ¶ms, &is_child_process, &message, home_dir, authkeys_file, authkeys_filename, signingkey); break; diff --git a/packages/c/sshnpd/src/params.c b/packages/c/sshnpd/src/params.c index dde20d48c..e95235078 100644 --- a/packages/c/sshnpd/src/params.c +++ b/packages/c/sshnpd/src/params.c @@ -1,13 +1,17 @@ #include +#include #include #include #include #include +#include #define default_permitopen "localhost:22,localhost:3389" void apply_default_values_to_sshnpd_params(sshnpd_params *params) { params->key_file = NULL; params->atsign = NULL; + // manager is handled at parse time + params->policy = NULL; params->device = "default"; params->sshpublickey = 0; params->hide = 0; @@ -16,17 +20,24 @@ void apply_default_values_to_sshnpd_params(sshnpd_params *params) { params->ephemeral_permission = ""; params->root_domain = "root.atsign.org"; params->local_sshd_port = 22; + params->storage_path = NULL; } int parse_sshnpd_params(sshnpd_params *params, int argc, const char **argv) { char *ssh_algorithm_input = ""; char *manager = NULL; char *permitopen = NULL; + ArgparseOption options[] = { OPT_HELP(), OPT_STRING('k', "key-file", ¶ms->key_file, "Path to the key file"), OPT_STRING('a', "atsign", ¶ms->atsign, "Atsign to use (mandatory)"), - OPT_STRING('m', "manager", &manager, "Manager to use (mandatory)"), + OPT_STRING('m', "manager", &manager, + "atSign or list of atSigns (comma separated) that this device will accept requests from. At least one " + "of --manager and --policy-manager must be supplied."), + // OPT_STRING('p', "policy-manager", ¶ms->policy, + // "The atSign which this device will use to decide whether or not to accept request from some client " + // "atSignAt least one of --manager and --policy-manager must be supplied."), OPT_STRING('d', "device", ¶ms->device, "Device to use"), OPT_BOOLEAN('s', "sshpublickey", ¶ms->sshpublickey, "Generate ssh public key"), OPT_BOOLEAN('h', "hide", ¶ms->hide, "Hide device from device entry (still responds to pings)"), @@ -39,6 +50,8 @@ int parse_sshnpd_params(sshnpd_params *params, int argc, const char **argv) { OPT_STRING(0, "root-domain", ¶ms->root_domain, "Root domain to use"), OPT_INTEGER(0, "local-sshd-port", ¶ms->local_sshd_port, "Local sshd port to use"), OPT_STRING(0, "storage-path", ¶ms->storage_path, NULL), + + // Doesn't do anything more, added in case old config would cause a parsing issue OPT_BOOLEAN('u', "un-hide", NULL, NULL), OPT_END(), }; @@ -56,14 +69,16 @@ int parse_sshnpd_params(sshnpd_params *params, int argc, const char **argv) { argparse_usage(&argparse); printf("Invalid Argument(s): Option atsign is mandatory\n"); return 1; - } else if (manager == NULL) { + } else if (manager == NULL && params->policy == NULL) { argparse_usage(&argparse); - printf("Invalid Argument(s) Option manager is mandatory\n"); + // TODO: enable this message when enabling policy + // printf("Invalid Argument(s) One of --manager or --policy-manager must be provided"); + printf("Invalid Argument(s) --manager must be provided"); return 1; } if (permitopen == NULL) { - params->permitopen_str = malloc(sizeof(char) * (strlen(default_permitopen) + 1)); // FIXME: leaks + params->permitopen_str = malloc(sizeof(char) * (strlen(default_permitopen) + 1)); if (params->permitopen_str == NULL) { printf("Failed to allocate memory for default permitopen string\n"); return 1; @@ -71,9 +86,21 @@ int parse_sshnpd_params(sshnpd_params *params, int argc, const char **argv) { strcpy(params->permitopen_str, default_permitopen); permitopen = params->permitopen_str; } + if ((parse_permitopen(permitopen, ¶ms->permitopen_hosts, ¶ms->permitopen_ports, ¶ms->permitopen_len, + false) != 0)) { + printf("Failed to parse permit-open string\n"); + free(params->permitopen_str); + return 1; + } - int manager_end = strlen(manager); - int permitopen_end = strlen(permitopen); + printf("permitting open:\n"); + for (int i = 0; i < params->permitopen_len; i++) { + if (params->permitopen_ports[i] == 0) { + printf("%s:*\n", params->permitopen_hosts[i]); + } else { + printf("%s:%d\n", params->permitopen_hosts[i], params->permitopen_ports[i]); + } + } if (strlen(ssh_algorithm_input) != 0) { // Parse ssh_algorithm_input to its enum value @@ -97,6 +124,11 @@ int parse_sshnpd_params(sshnpd_params *params, int argc, const char **argv) { return 1; } + int manager_end = 0; + if (manager != NULL) { + manager_end = strlen(manager); + } + // Validation and type inference for manager list int sep_count = 0; // first counter the number of seperators @@ -106,71 +138,42 @@ int parse_sshnpd_params(sshnpd_params *params, int argc, const char **argv) { } } - // malloc pointers to each string, but don't malloc any more memory for individual char storage - params->manager_list = malloc((sep_count + 1) * sizeof(char *)); // FIXME: leak - if (params->manager_list == NULL) { - printf("Failed to allocate memory for manager list\n"); - free(params->permitopen_str); - return 1; - } - params->manager_list[0] = manager; - int pos = 1; // Starts at 1 since we already added the first item to the list - for (int i = 0; i < manager_end; i++) { - if (manager[i] == ',') { - // Set this comma to a null terminator - manager[i] = '\0'; - if (manager[i + 1] == '\0') { - // Trailing comma, so we over counted by one - sep_count--; - // The allocated memory has a double trailing null seperator, but that's fine - break; - } - if (manager[i + 1] != '@') { - printf("Invalid Argument(s): Expected a list of atSigns: \"%s\"\n", manager); - free(params->manager_list); - free(params->permitopen_str); - return 1; - } - // Keep track of the start of the next item - params->manager_list[pos++] = manager + i + 1; - } - } - params->manager_list_len = sep_count + 1; - - // Repeat for permit-open - sep_count = 0; - for (int i = 0; i < permitopen_end - 1; i++) { - if (permitopen[i] == ',') { - sep_count++; + int pos; // position counter + if (manager != NULL) { + // malloc pointers to each string, but don't malloc any more memory for individual char storage + params->manager_list = malloc((sep_count + 1) * sizeof(char *)); // FIXME: leak + if (params->manager_list == NULL) { + printf("Failed to allocate memory for manager list\n"); + free(params->permitopen_str); + return 1; } - } - - // malloc pointers to each string, but don't malloc any more memory for individual char storage - params->permitopen = malloc((sep_count + 1) * sizeof(char *)); // FIXME leak - if (params->permitopen == NULL) { - printf("Failed to allocate memory for permitopen\n"); - free(params->manager_list); - free(params->permitopen_str); - return 1; - } - - params->permitopen[0] = permitopen; - pos = 1; // Starts at 1 since we already added the first item to the list - for (int i = 0; i < permitopen_end; i++) { - if (permitopen[i] == ',') { - // Set this comma to a null terminator - permitopen[i] = '\0'; - if (permitopen[i + 1] == '\0') { - // Trailing comma, so we over counted by one - sep_count--; - // The allocated memory has a double trailing null seperator, but that's fine - break; + params->manager_list[0] = manager; + pos = 1; // Starts at 1 since we already added the first item to the list + for (int i = 0; i < manager_end; i++) { + if (manager[i] == ',') { + // Set this comma to a null terminator + manager[i] = '\0'; + if (manager[i + 1] == '\0') { + // Trailing comma, so we over counted by one + sep_count--; + // The allocated memory has a double trailing null seperator, but that's fine + break; + } + if (manager[i + 1] != '@') { + printf("Invalid Argument(s): Expected a list of atSigns: \"%s\"\n", manager); + free(params->manager_list); + free(params->permitopen_str); + return 1; + } + // Keep track of the start of the next item + params->manager_list[pos++] = manager + i + 1; } - // Keep track of the start of the next item - params->permitopen[pos++] = permitopen + i + 1; } + params->manager_list_len = sep_count + 1; + } else { + params->manager_list_len = 0; } - params->permitopen_len = sep_count + 1; + // Repeat for permit-open return 0; } diff --git a/packages/c/sshnpd/src/permitopen.c b/packages/c/sshnpd/src/permitopen.c new file mode 100644 index 000000000..087003a48 --- /dev/null +++ b/packages/c/sshnpd/src/permitopen.c @@ -0,0 +1,101 @@ +#include "sshnpd/permitopen.h" +#include +#include +#include +#include + +int parse_permitopen(char *input, char ***permitopen_hosts, uint16_t **permitopen_ports, size_t *permitopen_len, + bool is_logger_available) { + const char *TAG = "parse_permitopen"; + int sep_count = 0; + int permitopen_end = strlen(input); + + for (int i = 0; i < permitopen_end; i++) { + if (input[i] == ':') { + sep_count++; + input[i] = '\0'; + } + + if (input[i] == ',') { + input[i] = '\0'; + } + } + + // malloc pointers to each string, but don't malloc any more memory for individual char storage + *permitopen_hosts = malloc((sep_count) * sizeof(char *)); + *permitopen_ports = malloc((sep_count) * sizeof(uint16_t)); + if (*permitopen_hosts == NULL) { + if (is_logger_available) { + atlogger_log(TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to allocate memory for permitopen\n"); + } else { + printf("Failed to allocate memory for permitopen\n"); + } + free(*permitopen_hosts); + free(*permitopen_ports); + return 1; + } + + int pos = 0; + for (int i = 0; i < sep_count; i++) { + // Add the host to the host list + (*permitopen_hosts)[i] = input + pos; + // Jump to the port string + pos += strlen(input + pos) + 1; + if (input[pos] == '*') { + (*permitopen_ports)[i] = 0; + if (input[pos + 1] != '\0') { + // error received a string other than '*' for port + if (is_logger_available) { + atlogger_log(TAG, ATLOGGER_LOGGING_LEVEL_ERROR, + "Argument error, received %s for port, must be a number 1-65535 or '*'", input + pos); + } else { + printf("Argument error, received %s for port, must be a number 1-65535 or '*'", input + pos); + } + free(*permitopen_hosts); + free(*permitopen_ports); + return 1; + } + } else { + char *end; + long num = strtol(input + pos, &end, 10); + if (end == input + pos || *end != '\0' || errno == ERANGE) { + if (is_logger_available) { + atlogger_log(TAG, ATLOGGER_LOGGING_LEVEL_ERROR, + "Argument error, received %s for port, must be a number 1-65535 or '*'", input + pos); + } else { + printf("Argument error, received %s for port, must be a number 1-65535 or '*'", input + pos); + } + free(*permitopen_hosts); + free(*permitopen_ports); + return 1; + } + + (*permitopen_ports)[i] = (uint16_t)num; + } + + // Jump to the host string + // + pos = pos + strlen(input + pos) + 1; + } + + *permitopen_len = sep_count; + return 0; +} + +bool should_permitopen(permitopen_params *params) { + const char *TAG = "should_permitopen"; + + for (int i = 0; i < params->permitopen_len; i++) { + // permitopen_port[i] = 0 is equivalent to '*' + if (params->permitopen_ports[i] == 0 || params->permitopen_ports[i] == params->requested_port) { + if (params->permitopen_hosts[i][0] == '*' && strlen(params->permitopen_hosts[i]) == 1) { + return true; + } + if (strcmp(params->permitopen_hosts[i], params->requested_host) == 0) { + return true; + } + } + } + + return false; +} From 2c4d59fa728d7e8aa1387a6f89d0486d3118c5fa Mon Sep 17 00:00:00 2001 From: xavierchanth Date: Fri, 27 Sep 2024 21:22:53 -0400 Subject: [PATCH 2/7] chore: clean up --- packages/c/sshnpd/src/handle_npt_request.c | 2 - packages/c/sshnpd/src/handle_ssh_request.c | 43 ++++++++++------------ 2 files changed, 20 insertions(+), 25 deletions(-) diff --git a/packages/c/sshnpd/src/handle_npt_request.c b/packages/c/sshnpd/src/handle_npt_request.c index 53534e506..6e42f3a66 100644 --- a/packages/c/sshnpd/src/handle_npt_request.c +++ b/packages/c/sshnpd/src/handle_npt_request.c @@ -1,4 +1,3 @@ -#include "atclient/request_options.h" #include "sshnpd/params.h" #include "sshnpd/permitopen.h" #include "sshnpd/sshnpd.h" @@ -160,7 +159,6 @@ void handle_npt_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn atclient_atkey_free(&atkey); if (res != 0) { atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to get public key\n"); - atclient_atkey_free(&atkey); cJSON_Delete(envelope); return; } diff --git a/packages/c/sshnpd/src/handle_ssh_request.c b/packages/c/sshnpd/src/handle_ssh_request.c index 36cb0158b..34be62bda 100644 --- a/packages/c/sshnpd/src/handle_ssh_request.c +++ b/packages/c/sshnpd/src/handle_ssh_request.c @@ -142,15 +142,13 @@ void handle_ssh_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn char *buffer = NULL; res = atclient_get_public_key(atclient, &atkey, &buffer, NULL); + atclient_atkey_free(&atkey); if (res != 0) { atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to get public key\n"); - atclient_atkey_free(&atkey); cJSON_Delete(envelope); return; } - atclient_atkey_free(&atkey); - atchops_rsa_key_public_key requesting_atsign_publickey; atchops_rsa_key_public_key_init(&requesting_atsign_publickey); @@ -276,7 +274,7 @@ void handle_ssh_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn if (!encrypt_rvd_traffic) { atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "encryptRvdTraffic=false is not supported by this daemon\n"); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } cJSON_Delete(envelope); return; @@ -288,7 +286,7 @@ void handle_ssh_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn "encryptRvdTraffic was requested, but no client ephemeral public key / key type was provided\n"); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } cJSON_Delete(envelope); return; @@ -298,7 +296,7 @@ void handle_ssh_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn if ((res = atchops_aes_generate_key(key, ATCHOPS_AES_256)) != 0) { atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to generate session aes key\n"); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } cJSON_Delete(envelope); return; @@ -309,7 +307,7 @@ void handle_ssh_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn if (res != 0) { atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to generate session aes key\n"); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } cJSON_Delete(envelope); return; @@ -319,7 +317,7 @@ void handle_ssh_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn if ((res = atchops_iv_generate(iv)) != 0) { atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to generate session iv\n"); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } cJSON_Delete(envelope); return; @@ -330,7 +328,7 @@ void handle_ssh_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn if (res != 0) { atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to generate session iv\n"); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } cJSON_Delete(envelope); return; @@ -352,7 +350,7 @@ void handle_ssh_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to populate client ephemeral pk\n"); atchops_rsa_key_public_key_free(&ac); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } cJSON_Delete(envelope); return; @@ -364,7 +362,7 @@ void handle_ssh_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn "Failed to allocate memory to encrypt the session aes key\n"); atchops_rsa_key_public_key_free(&ac); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } cJSON_Delete(envelope); return; @@ -375,7 +373,7 @@ void handle_ssh_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to encrypt the session aes key\n"); atchops_rsa_key_public_key_free(&ac); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } free(session_aes_key_encrypted); cJSON_Delete(envelope); @@ -391,7 +389,7 @@ void handle_ssh_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn "Failed to allocate memory to base64 encode the session aes key\n"); atchops_rsa_key_public_key_free(&ac); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } free(session_aes_key_encrypted); cJSON_Delete(envelope); @@ -406,7 +404,7 @@ void handle_ssh_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to base64 encode the session aes key\n"); atchops_rsa_key_public_key_free(&ac); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } free(session_aes_key_base64); free(session_aes_key_encrypted); @@ -422,7 +420,7 @@ void handle_ssh_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to allocate memory to encrypt the session iv\n"); atchops_rsa_key_public_key_free(&ac); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } free(session_aes_key_base64); cJSON_Delete(envelope); @@ -435,7 +433,7 @@ void handle_ssh_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn if (res != 0) { atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to encrypt the session iv\n"); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } free(session_iv_encrypted); free(session_aes_key_base64); @@ -450,7 +448,7 @@ void handle_ssh_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to allocate memory to base64 encode the session iv\n"); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } free(session_iv_encrypted); free(session_aes_key_base64); @@ -465,7 +463,7 @@ void handle_ssh_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn if (res != 0) { atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to base64 encode the session iv\n"); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } free(session_iv_base64); free(session_iv_encrypted); @@ -484,7 +482,7 @@ void handle_ssh_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "%s is not an accepted key type for encrypting the aes key\n", pk_type); if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } cJSON_Delete(envelope); return; @@ -521,12 +519,11 @@ void handle_ssh_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn int res = run_srv_process(rvd_host_str, rvd_port_int, requested_host_str, requested_port_int, authenticate_to_rvd, rvd_auth_string, encrypt_rvd_traffic, multi, session_aes_key, session_iv); - free(rvd_host_str); *is_child_process = true; if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } cJSON_Delete(envelope); exit(res); @@ -658,7 +655,7 @@ void handle_ssh_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn clean_res: { free(keyname); } clean_final_res_value: { atclient_atkey_free(&final_res_atkey); - free(final_res_value); + cJSON_free(final_res_value); } clean_json: { cJSON_Delete(final_res_envelope); @@ -671,7 +668,7 @@ void handle_ssh_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshn } cancel: if (authenticate_to_rvd) { - free(rvd_auth_string); + cJSON_free(rvd_auth_string); } if (free_session_base64) { free(session_iv_base64); From 90808fa354ad52cabf96538ce00e264675d1ae73 Mon Sep 17 00:00:00 2001 From: xavierchanth Date: Mon, 30 Sep 2024 13:27:33 -0400 Subject: [PATCH 3/7] chore: add todo --- packages/c/sshnpd/src/background_jobs.c | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/c/sshnpd/src/background_jobs.c b/packages/c/sshnpd/src/background_jobs.c index 762a4114c..52fd55a6d 100644 --- a/packages/c/sshnpd/src/background_jobs.c +++ b/packages/c/sshnpd/src/background_jobs.c @@ -120,6 +120,7 @@ void *refresh_device_entry(void *void_refresh_device_entry_params) { } // Build each atkey + // TODO: @xavierchanth - review this implementation int interval_seconds = 60 * 60; // once an hour int counter = 0; while (*params->should_run) { From e9f16be70b1bbc019bbf8a0e2763d866e5369095 Mon Sep 17 00:00:00 2001 From: xavierchanth Date: Mon, 30 Sep 2024 13:27:41 -0400 Subject: [PATCH 4/7] fix: tests --- packages/c/sshnpd/tests/CMakeLists.txt | 5 +++- packages/c/sshnpd/tests/test_sshnpd_params.c | 26 ++++++++++++++------ 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/packages/c/sshnpd/tests/CMakeLists.txt b/packages/c/sshnpd/tests/CMakeLists.txt index 6a31dc594..c152c34fb 100644 --- a/packages/c/sshnpd/tests/CMakeLists.txt +++ b/packages/c/sshnpd/tests/CMakeLists.txt @@ -5,6 +5,9 @@ foreach(file ${files}) string(REPLACE ".c" "" filename ${filename}) add_executable(${filename} ${file}) - target_link_libraries(${filename} PRIVATE sshnpd_lib) + target_link_libraries( + ${filename} + PRIVATE sshnpd-lib argparse::argparse-static + ) add_test(NAME ${filename} COMMAND $) endforeach() diff --git a/packages/c/sshnpd/tests/test_sshnpd_params.c b/packages/c/sshnpd/tests/test_sshnpd_params.c index 0e9f78fd7..529406d46 100644 --- a/packages/c/sshnpd/tests/test_sshnpd_params.c +++ b/packages/c/sshnpd/tests/test_sshnpd_params.c @@ -7,25 +7,35 @@ int default_values_test(); int parse_params_test(); int atsign_mandatory_test(); -int manager_mandatory_test(); +int manager_policy_mandatory_test(); int ssh_algorithm_parse_test(); +int permit_open_parse_test(); int main() { int ret = 0; if (default_values_test()) { + printf("Default values test failed\n"); ret++; } if (parse_params_test()) { + printf("Parse params test failed\n"); ret++; } if (atsign_mandatory_test()) { + printf("atSign mandatory test failed\n"); ret++; } - if (manager_mandatory_test()) { + if (manager_policy_mandatory_test()) { + printf("manager/policy mandatory test failed\n"); ret++; } if (ssh_algorithm_parse_test()) { + printf("ssh algorithm parse test failed\n"); + ret++; + } + if (permit_open_parse_test()) { + printf("permit open parse test failed\n"); ret++; } @@ -36,8 +46,8 @@ int main() { // Define the tests int default_values_test() { int ret = 0; - sshnpd_params_t *params = malloc(sizeof(sshnpd_params_t)); - apply_default_values_to_params(params); + sshnpd_params *params = malloc(sizeof(sshnpd_params)); + apply_default_values_to_sshnpd_params(params); if (strcmp(params->device, "default") != 0) { ret = 1; @@ -45,7 +55,7 @@ int default_values_test() { if (params->sshpublickey != 0) { ret = 1; } - if (params->unhide != 0) { + if (params->hide != 0) { ret = 1; } if (params->verbose != 0) { @@ -71,7 +81,7 @@ int default_values_test() { int parse_params_test() { int ret = 0; - sshnpd_params_t *params = malloc(sizeof(sshnpd_params_t)); + sshnpd_params *params = malloc(sizeof(sshnpd_params)); const char *argv[] = {"sshnpd", "-a", "atsign", "-m", "manager"}; int argc = 4; @@ -82,6 +92,8 @@ int parse_params_test() { int atsign_mandatory_test() { return 0; } -int manager_mandatory_test() { return 0; } +int manager_policy_mandatory_test() { return 0; } int ssh_algorithm_parse_test() { return 0; } + +int permit_open_parse_test() { return 0; } From 94216133bc627edb5ebfeaff8bfbca88797df01a Mon Sep 17 00:00:00 2001 From: xavierchanth Date: Mon, 30 Sep 2024 23:05:43 -0400 Subject: [PATCH 5/7] test: permit open parsing and regression tests --- packages/c/.gitignore | 1 + packages/c/sshnpd/src/params.c | 4 +- packages/c/sshnpd/tests/test_permit_open.c | 223 +++++++++++++++++++ packages/c/sshnpd/tests/test_sshnpd_params.c | 195 ++++++++++++++-- 4 files changed, 407 insertions(+), 16 deletions(-) create mode 100644 packages/c/sshnpd/tests/test_permit_open.c diff --git a/packages/c/.gitignore b/packages/c/.gitignore index 12ea806fd..7e41986ae 100644 --- a/packages/c/.gitignore +++ b/packages/c/.gitignore @@ -5,3 +5,4 @@ compile_commands.json core valgrind-out.txt* +Testing/ diff --git a/packages/c/sshnpd/src/params.c b/packages/c/sshnpd/src/params.c index e95235078..ab48d8a63 100644 --- a/packages/c/sshnpd/src/params.c +++ b/packages/c/sshnpd/src/params.c @@ -17,7 +17,6 @@ void apply_default_values_to_sshnpd_params(sshnpd_params *params) { params->hide = 0; params->verbose = 0; params->ssh_algorithm = ED25519; - params->ephemeral_permission = ""; params->root_domain = "root.atsign.org"; params->local_sshd_port = 22; params->storage_path = NULL; @@ -27,6 +26,7 @@ int parse_sshnpd_params(sshnpd_params *params, int argc, const char **argv) { char *ssh_algorithm_input = ""; char *manager = NULL; char *permitopen = NULL; + char *ephemeral_permissions = NULL; ArgparseOption options[] = { OPT_HELP(), @@ -46,7 +46,7 @@ int parse_sshnpd_params(sshnpd_params *params, int argc, const char **argv) { "Comma separated-list of host:port to which the daemon will permit a connection from an authorized " "client. (defaults to \"localhost:22,localhost:3389\")"), OPT_STRING(0, "ssh-algorithm", &ssh_algorithm_input, "SSH algorithm to use"), - OPT_STRING(0, "ephemeral-permission", ¶ms->ephemeral_permission, "Ephemeral permission to use"), + OPT_STRING(0, "ephemeral-permission", &ephemeral_permissions, "(Kept for compatibility)"), OPT_STRING(0, "root-domain", ¶ms->root_domain, "Root domain to use"), OPT_INTEGER(0, "local-sshd-port", ¶ms->local_sshd_port, "Local sshd port to use"), OPT_STRING(0, "storage-path", ¶ms->storage_path, NULL), diff --git a/packages/c/sshnpd/tests/test_permit_open.c b/packages/c/sshnpd/tests/test_permit_open.c new file mode 100644 index 000000000..b0d846a70 --- /dev/null +++ b/packages/c/sshnpd/tests/test_permit_open.c @@ -0,0 +1,223 @@ +#include "sshnpd/permitopen.h" +#include + +int star_star_test(); +int localhost_star_test(); +int star_port_test(); +int localhost_port_test(); +int list_test(); + +int main() { + int ret = 0; + + if (star_star_test()) { + printf("*:* test failed\n"); + ret++; + } + + if (localhost_star_test()) { + printf("localhost:* test failed\n"); + ret++; + } + if (star_port_test()) { + printf("*:22 test failed\n"); + ret++; + } + if (localhost_port_test()) { + printf("localhost:22 test failed\n"); + ret++; + } + if (list_test()) { + printf("localhost:22,foo.bar.com:3389 test failed\n"); + ret++; + } + + printf("Tests failed: %d\n", ret); + return ret; +} + +int star_star_test() { + permitopen_params params; + char *hosts[] = {"*"}; + uint16_t ports[] = {0}; + params.permitopen_hosts = hosts; + params.permitopen_ports = ports; + params.permitopen_len = 1; + + bool allow; + + params.requested_host = "localhost"; + params.requested_port = 22; + if (!should_permitopen(¶ms)) { + return 1; + } + + params.requested_host = "123.123.123.123"; + params.requested_port = 7878; + if (!should_permitopen(¶ms)) { + return 1; + } + + params.requested_host = "foo.bar.com"; + params.requested_port = 53; + if (!should_permitopen(¶ms)) { + return 1; + } + + return 0; +} + +int localhost_star_test() { + permitopen_params params; + char *hosts[] = {"localhost"}; + uint16_t ports[] = {0}; + params.permitopen_hosts = hosts; + params.permitopen_ports = ports; + params.permitopen_len = 1; + bool allow; + params.requested_host = "localhost"; + params.requested_port = 22; + if (!should_permitopen(¶ms)) { + return 1; + } + params.requested_host = "localhost"; + params.requested_port = 7878; + if (!should_permitopen(¶ms)) { + return 1; + } + + params.requested_host = "foo.bar.com"; + params.requested_port = 53; + if (should_permitopen(¶ms)) { + return 1; + } + + return 0; +} +int star_port_test() { + permitopen_params params; + char *hosts[] = {"*"}; + uint16_t ports[] = {22}; + params.permitopen_hosts = hosts; + params.permitopen_ports = ports; + params.permitopen_len = 1; + + bool allow; + + params.requested_host = "localhost"; + params.requested_port = 22; + if (!should_permitopen(¶ms)) { + return 1; + } + + params.requested_host = "123.123.123.123"; + params.requested_port = 22; + if (!should_permitopen(¶ms)) { + return 1; + } + + params.requested_host = "foo.bar.com"; + params.requested_port = 53; + if (should_permitopen(¶ms)) { + return 1; + } + + return 0; +} +int localhost_port_test() { + permitopen_params params; + char *hosts[] = {"localhost"}; + uint16_t ports[] = {22}; + params.permitopen_hosts = hosts; + params.permitopen_ports = ports; + params.permitopen_len = 1; + + bool allow; + + params.requested_host = "localhost"; + params.requested_port = 22; + if (!should_permitopen(¶ms)) { + return 1; + } + + params.requested_host = "123.123.123.123"; + params.requested_port = 22; + if (should_permitopen(¶ms)) { + return 1; + } + + params.requested_host = "localhost"; + params.requested_port = 7878; + if (should_permitopen(¶ms)) { + return 1; + } + + params.requested_host = "foo.bar.com"; + params.requested_port = 53; + if (should_permitopen(¶ms)) { + return 1; + } + + return 0; +} +int list_test() { + permitopen_params params; + char *hosts[] = {"localhost", "foo.bar.com"}; + uint16_t ports[] = {22, 3389}; + params.permitopen_hosts = hosts; + params.permitopen_ports = ports; + params.permitopen_len = 2; + + bool allow; + + params.requested_host = "localhost"; + params.requested_port = 22; + if (!should_permitopen(¶ms)) { + return 1; + } + + params.requested_host = "123.123.123.123"; + params.requested_port = 22; + if (should_permitopen(¶ms)) { + return 1; + } + + params.requested_host = "localhost"; + params.requested_port = 7878; + if (should_permitopen(¶ms)) { + return 1; + } + + params.requested_host = "foo.bar.com"; + params.requested_port = 3389; + if (!should_permitopen(¶ms)) { + return 1; + } + + params.requested_host = "123.123.123.123"; + params.requested_port = 3389; + if (should_permitopen(¶ms)) { + return 1; + } + + params.requested_host = "foo.bar.com"; + params.requested_port = 7878; + if (should_permitopen(¶ms)) { + return 1; + } + + // mixed and matched, these should fail + params.requested_host = "localhost"; + params.requested_port = 3389; + if (should_permitopen(¶ms)) { + return 1; + } + + params.requested_host = "foo.bar.com"; + params.requested_port = 22; + if (should_permitopen(¶ms)) { + return 1; + } + + return 0; +} diff --git a/packages/c/sshnpd/tests/test_sshnpd_params.c b/packages/c/sshnpd/tests/test_sshnpd_params.c index 529406d46..f20286d48 100644 --- a/packages/c/sshnpd/tests/test_sshnpd_params.c +++ b/packages/c/sshnpd/tests/test_sshnpd_params.c @@ -1,4 +1,5 @@ #include "sshnpd/params.h" +#include "sshnpd/permitopen.h" #include #include #include @@ -8,7 +9,6 @@ int default_values_test(); int parse_params_test(); int atsign_mandatory_test(); int manager_policy_mandatory_test(); -int ssh_algorithm_parse_test(); int permit_open_parse_test(); int main() { @@ -30,10 +30,6 @@ int main() { printf("manager/policy mandatory test failed\n"); ret++; } - if (ssh_algorithm_parse_test()) { - printf("ssh algorithm parse test failed\n"); - ret++; - } if (permit_open_parse_test()) { printf("permit open parse test failed\n"); ret++; @@ -64,9 +60,6 @@ int default_values_test() { if (params->ssh_algorithm != ED25519) { ret = 1; } - if (strcmp(params->ephemeral_permission, "") != 0) { - ret = 1; - } if (strcmp(params->root_domain, "root.atsign.org") != 0) { ret = 1; } @@ -83,17 +76,191 @@ int parse_params_test() { sshnpd_params *params = malloc(sizeof(sshnpd_params)); - const char *argv[] = {"sshnpd", "-a", "atsign", "-m", "manager"}; - int argc = 4; + const char *argv[] = { + "sshnpd", + "-a", + "@atsign", + "-m", + "@manager", + "-d", + "my_device", + "-s", + "-h", + "-v", + "--ssh-algorithm", + "ssh-rsa", + "--root-domain", + "vip.ve.atsign.zone", + "--local-sshd-port", + "6222", + }; + + apply_default_values_to_sshnpd_params(params); + ret = parse_sshnpd_params(params, 16, argv); + + if (strcmp(params->atsign, "@atsign") != 0) { + ret = 1; + } + if (strcmp(params->manager_list[0], "@manager") != 0) { + ret = 1; + } + if (strcmp(params->device, "my_device") != 0) { + ret = 1; + } + if (params->sshpublickey != 1) { + ret = 1; + } + if (params->hide != 1) { + ret = 1; + } + if (params->verbose != 1) { + ret = 1; + } + if (params->ssh_algorithm != RSA) { + ret = 1; + } + if (strcmp(params->root_domain, "vip.ve.atsign.zone") != 0) { + ret = 1; + } + if (params->local_sshd_port != 6222) { + ret = 1; + } + + free(params); + return ret; +} + +int atsign_mandatory_test() { + int ret = 0; + + sshnpd_params *params = malloc(sizeof(sshnpd_params)); + + const char *argv[] = { + "sshnpd", + "-m", + "@manager", + "-d", + "my_device", + "-s", + "-h", + "-v", + "--ssh-algorithm", + "ssh-rsa", + "--root-domain", + "vip.ve.atsign.zone", + "--local-sshd-port", + "6222", + }; + + apply_default_values_to_sshnpd_params(params); + ret = parse_sshnpd_params(params, 14, argv); + // expect this to return non-zero since atsign is missing + if (ret == 0) { + ret = 1; + } else { + ret = 0; + } + + free(params); + return ret; +} + +int manager_policy_mandatory_test() { + int ret = 0; + + sshnpd_params *params = malloc(sizeof(sshnpd_params)); + + const char *argv[] = { + "sshnpd", "-a", "@atsign", "-d", "my_device", + }; + + apply_default_values_to_sshnpd_params(params); + ret = parse_sshnpd_params(params, 5, argv); + // expect this to return non-zero since manager & policy are missing + if (ret == 0) { + ret = 1; + } else { + ret = 0; + } + + free(params); + return ret; +} + +int manager_list_test() { + int ret = 0; + + return 0; + sshnpd_params *params = malloc(sizeof(sshnpd_params)); + + const char *argv[] = { + "sshnpd", "-a", "@atsign", "-m", "@foo,@bar,@baz", "-d", "my_device", + }; + + apply_default_values_to_sshnpd_params(params); + ret = parse_sshnpd_params(params, 7, argv); + if (ret == 0) { + ret = 1; + } else { + ret = 0; + } free(params); return ret; } -int atsign_mandatory_test() { return 0; } +int permit_open_parse_test() { + int ret = 0; + + // FIXME: bus error + char **permitopen_hosts = NULL; + uint16_t *permitopen_ports = NULL; + size_t permitopen_len; + ret = parse_permitopen(strdup("*:*"), &permitopen_hosts, &permitopen_ports, &permitopen_len, false); -int manager_policy_mandatory_test() { return 0; } + if (ret != 0 || permitopen_len != 1 || strcmp(permitopen_hosts[0], "*") != 0 || permitopen_ports[0] != 0) { + ret = 1; + } -int ssh_algorithm_parse_test() { return 0; } + char **permitopen_hosts2 = NULL; + uint16_t *permitopen_ports2 = NULL; + size_t permitopen_len2; + ret = parse_permitopen(strdup("localhost:*"), &permitopen_hosts2, &permitopen_ports2, &permitopen_len2, false); -int permit_open_parse_test() { return 0; } + if (ret != 0 || permitopen_len2 != 1 || strcmp(permitopen_hosts2[0], "localhost") != 0 || permitopen_ports2[0] != 0) { + ret = 1; + } + + return 0; + char **permitopen_hosts3 = NULL; + uint16_t *permitopen_ports3 = NULL; + size_t permitopen_len3; + ret = parse_permitopen(strdup("*:22"), &permitopen_hosts3, &permitopen_ports3, &permitopen_len3, false); + + if (ret != 0 || permitopen_len3 != 1 || strcmp(permitopen_hosts3[0], "*") != 0 || permitopen_ports3[0] != 22) { + ret = 1; + } + + char **permitopen_hosts4 = NULL; + uint16_t *permitopen_ports4 = NULL; + size_t permitopen_len4; + ret = parse_permitopen(strdup("localhost:22"), &permitopen_hosts4, &permitopen_ports4, &permitopen_len4, false); + + if (ret != 0 || permitopen_len4 != 1 || strcmp(permitopen_hosts4[0], "localhost") != 0 || + permitopen_ports4[0] != 22) { + ret = 1; + } + + char **permitopen_hosts5 = NULL; + uint16_t *permitopen_ports5 = NULL; + size_t permitopen_len5; + ret = parse_permitopen(strdup("localhost:22,foo.bar.com:3389"), &permitopen_hosts5, &permitopen_ports5, + &permitopen_len5, false); + + if (ret != 0 || permitopen_len5 != 2 || strcmp(permitopen_hosts5[0], "localhost") != 0 || + permitopen_ports5[0] != 22 || strcmp(permitopen_hosts5[1], "foo.bar.com") != 0 || permitopen_ports5[1] != 3389) { + ret = 1; + } + + return 0; +} From e609b41ee8e2591a09a64aa96f22c4380a614d66 Mon Sep 17 00:00:00 2001 From: xavierchanth Date: Mon, 30 Sep 2024 23:09:21 -0400 Subject: [PATCH 6/7] ci: add unit tests --- .github/workflows/c_unit_tests.yaml | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 .github/workflows/c_unit_tests.yaml diff --git a/.github/workflows/c_unit_tests.yaml b/.github/workflows/c_unit_tests.yaml new file mode 100644 index 000000000..d16d3bf89 --- /dev/null +++ b/.github/workflows/c_unit_tests.yaml @@ -0,0 +1,24 @@ +name: C Unit Tests + +on: + workflow_dispatch: + push: + branches: [trunk] + pull_request: + branches: [trunk] + +permissions: # added using https://github.com/step-security/secure-repo + contents: read + +jobs: + unit-tests: + runs-on: "ubuntu-latest" + steps: + - uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0 + + - name: atsdk Unit CTest + working-directory: packages/c + run: | + cmake -S . -B build -DSSHNPD_BUILD_TESTS=ON + cmake --build build + ctest --test-dir build/sshnpd/tests --output-on-failure --timeout 2 From a769e5fc99d51306d22dbbcab19a852bb19e96c2 Mon Sep 17 00:00:00 2001 From: xavierchanth Date: Mon, 30 Sep 2024 23:11:02 -0400 Subject: [PATCH 7/7] fix: add test dependency --- .github/workflows/c_unit_tests.yaml | 2 +- packages/c/sshnpd/tests/CMakeLists.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/c_unit_tests.yaml b/.github/workflows/c_unit_tests.yaml index d16d3bf89..ea2f504b6 100644 --- a/.github/workflows/c_unit_tests.yaml +++ b/.github/workflows/c_unit_tests.yaml @@ -16,7 +16,7 @@ jobs: steps: - uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0 - - name: atsdk Unit CTest + - name: sshnpd Unit CTest working-directory: packages/c run: | cmake -S . -B build -DSSHNPD_BUILD_TESTS=ON diff --git a/packages/c/sshnpd/tests/CMakeLists.txt b/packages/c/sshnpd/tests/CMakeLists.txt index c152c34fb..76c336941 100644 --- a/packages/c/sshnpd/tests/CMakeLists.txt +++ b/packages/c/sshnpd/tests/CMakeLists.txt @@ -7,7 +7,7 @@ foreach(file ${files}) add_executable(${filename} ${file}) target_link_libraries( ${filename} - PRIVATE sshnpd-lib argparse::argparse-static + PRIVATE sshnpd-lib argparse::argparse-static atlogger ) add_test(NAME ${filename} COMMAND $) endforeach()