Merge pull request #1726 from atsign-foundation/c-memcheck-tools

feat: C memcheck tools

packages/c/.gitignore

@@ -3,6 +3,8 @@ build*
 
 compile_commands.json
 
+just.env
+
+valgrind-sshnpd-*.log
 core
-valgrind-out.txt*
 Testing/

packages/c/CMakeLists.txt

@@ -1,5 +1,6 @@
 cmake_minimum_required(VERSION 3.19)
 set(CMAKE_C_STANDARD 99)
 cmake_policy(SET CMP0135 NEW)
-project(csshnpd VERSION 0.4.1 LANGUAGES C)
+project(csshnpd VERSION 0.4.3 LANGUAGES C)
 add_subdirectory(sshnpd)
+add_subdirectory(graceful-shutdown-tool)

packages/c/CMakePresets.json

@@ -0,0 +1,55 @@
+{
+  "version": 8,
+  "configurePresets": [
+    {
+      "name": "dev",
+      "binaryDir": "${sourceDir}/build/dev",
+      "cacheVariables": {
+        "BUILD_SHARED_LIBS": "OFF",
+        "CMAKE_BUILD_TYPE": "Debug",
+        "CMAKE_C_FLAGS": "-std=c99 -Wno-error",
+        "CMAKE_EXPORT_COMPILE_COMMANDS": "ON",
+        "ATSDK_BUILD_TESTS": "OFF",
+        "ATSDK_MEMCHECK": "OFF",
+        "ENABLE_TESTING": "OFF",
+        "ENABLE_PROGRAMS": "OFF",
+        "ENABLE_CJSON_TEST": "OFF",
+        "CJSON_OVERRIDE_BUILD_SHARED_LIBS": "ON",
+        "CJSON_BUILD_SHARED_LIBS": "OFF",
+        "BUILD_SHARED_AND_STATIC_LIBS": "OFF",
+        "ENABLE_TARGET_EXPORT": "OFF",
+        "ENABLE_CJSON_VERSION_SO": "OFF"
+      }
+    },
+    {
+      "name": "dev-ninja",
+      "inherits": ["dev"],
+      "generator": "Ninja"
+    },
+    {
+      "name": "strict",
+      "binaryDir": "${sourceDir}/build/strict",
+      "cacheVariables": {
+        "BUILD_SHARED_LIBS": "OFF",
+        "CMAKE_BUILD_TYPE": "Debug",
+        "CMAKE_C_FLAGS": "-std=c99 -Wall -Wextra -Werror-implicit-function-declaration",
+        "CMAKE_EXPORT_COMPILE_COMMANDS": "OFF",
+        "ATSDK_BUILD_TESTS": "OFF",
+        "ATSDK_MEMCHECK": "OFF",
+        "ENABLE_TESTING": "OFF",
+        "ENABLE_PROGRAMS": "OFF",
+        "ENABLE_CJSON_TEST": "OFF",
+        "CJSON_OVERRIDE_BUILD_SHARED_LIBS": "ON",
+        "CJSON_BUILD_SHARED_LIBS": "OFF",
+        "BUILD_SHARED_AND_STATIC_LIBS": "OFF",
+        "ENABLE_TARGET_EXPORT": "OFF",
+        "ENABLE_CJSON_VERSION_SO": "OFF"
+      }
+    },
+    {
+      "name": "strict-ninja",
+      "inherits": ["strict"],
+      "generator": "Ninja"
+    }
+  ]
+}

packages/c/cmake/atsdk.cmake

@@ -3,7 +3,7 @@ if(NOT atsdk_FOUND)
   FetchContent_Declare(
     atsdk
     GIT_REPOSITORY https://github.com/atsign-foundation/at_c.git
-    GIT_TAG e04224997321819c5c9db1caa490f9faa6f23de8
+    GIT_TAG 54e4cc64654eeed19bb92989e4619df103f14675
   )
   FetchContent_MakeAvailable(atsdk)
   install(

packages/c/graceful-shutdown-tool/CMakeLists.txt

@@ -0,0 +1,16 @@
+cmake_minimum_required(VERSION 3.19)
+set(CMAKE_C_STANDARD 99)
+cmake_policy(SET CMP0135 NEW)
+project(graceful-shutdown-tool VERSION 0.4.1 LANGUAGES C)
+
+include(FetchContent)
+include(GNUInstallDirs)
+cmake_policy(SET CMP0135 NEW)
+
+include(${CMAKE_CURRENT_SOURCE_DIR}/../cmake/atsdk.cmake)
+add_executable(${PROJECT_NAME} ${CMAKE_CURRENT_LIST_DIR}/main.c)
+
+target_link_libraries(
+  ${PROJECT_NAME}
+  PRIVATE atclient atchops atlogger cjson argparse::argparse-static
+)

packages/c/graceful-shutdown-tool/README.md

@@ -0,0 +1,20 @@
+# graceful-shutdown-tool
+
+When sshnpd is built with the `SSHNPD_ENABLE_TESTING_SHUTDOWN_NOTIFICATION`
+compile definition, then it enables an extra notification handler for the key
+`graceful_shutdown`. When sshnpd receives this key, it does a graceful shutdown,
+exiting the main handler loop and proceeding with shutdown.
+
+Normally this type of exit only occurs when sshnpd has some unrecoverable error.
+Passing SIGINT to sshnpd does not gracefully exit, so in order for sshnpd to
+exit in such a way that we can properly run memcheck, this special notification,
+which is only available behind the compile definition is used to trigger an exit.
+
+This graceful-shutdown-tool can be used to send this notification.
+
+## Usage
+
+`./graceful-shutdown-tool $FROM $TO`
+
+> FROM is the -a flag in sshnpd
+> TO is the -m flag in sshnpd

packages/c/graceful-shutdown-tool/main.c

@@ -0,0 +1,75 @@
+#include "atclient/notify.h"
+#include "atclient/notify_params.h"
+#include "atlogger/atlogger.h"
+#include <atclient/atclient.h>
+#include <string.h>
+
+// proper error handling and memory management is not done in this program
+// since it is only used as a development tool, it is not perfect code
+
+int main(int argc, char **argv) {
+  if (argc != 4) {
+    printf("Usage: %s <from_atsign> <to_atsign> <device_name>\n", argv[0]);
+    exit(1);
+  }
+  atlogger_set_logging_level(ATLOGGER_LOGGING_LEVEL_INFO);
+
+  char *from = argv[1];
+  char *to = argv[2];
+  char *device_name = argv[3];
+
+  atclient_atkeys_file atkeys_file;
+  atclient_atkeys_file_init(&atkeys_file);
+
+  char *file_path = atkeys_file_get_default_path(from);
+  if (file_path == NULL) {
+    printf("A\n");
+    return 1;
+  }
+
+  atclient_atkeys atkeys;
+  atclient_atkeys_init(&atkeys);
+
+  int ret = atclient_atkeys_populate_from_path(&atkeys, file_path);
+  if (ret != 0) {
+    printf("B\n");
+    return ret;
+  }
+
+  atclient_atkey atkey;
+  atclient_atkey_init(&atkey);
+
+  size_t namespace_len = strlen(device_name) + 1 + 5 + 1;
+  char namespace[namespace_len];
+  snprintf(namespace, namespace_len, "%s.sshnp", device_name);
+  atclient_atkey_set_shared_with(&atkey, to);
+  atclient_atkey_set_key(&atkey, "graceful_shutdown");
+  atclient_atkey_set_namespace_str(&atkey, namespace);
+  atclient_atkey_set_shared_by(&atkey, from);
+
+  atclient_notify_params params;
+  atclient_notify_params_init(&params);
+
+  atclient_notify_params_set_atkey(&params, &atkey);
+  atclient_notify_params_set_operation(&params, ATCLIENT_NOTIFY_OPERATION_UPDATE);
+  atclient_notify_params_set_value(&params, "foo"); // ignored, value doesn't matter
+  atclient_notify_params_set_should_encrypt(&params, true);
+
+  atclient atclient;
+  atclient_init(&atclient);
+
+  ret = atclient_pkam_authenticate(&atclient, from, &atkeys, NULL, NULL);
+  if (ret != 0) {
+    printf("C\n");
+    return ret;
+  }
+
+  ret = atclient_notify(&atclient, &params, NULL);
+  if (ret != 0) {
+    printf("D\n");
+    return ret;
+  }
+
+  printf("sent\n");
+  return 0;
+}

packages/c/just.template.env

@@ -0,0 +1,6 @@
+C_COMPILER=gcc
+GENERATOR=Ninja
+# Used by memcheck & graceful shutdown tool in justfile
+FROM_ATSIGN=
+TO_ATSIGN=
+DEVICE_NAME=

packages/c/justfile

@@ -0,0 +1,104 @@
+alias dev := build-dev
+alias strict := build-strict
+alias vald := valgrind-docker
+alias memd := memcheck-docker
+alias shutdown := graceful-shutdown
+
+set dotenv-filename := "just.env"
+set dotenv-load
+
+postfix := quote(os() + "-" + arch())
+dev_dir := quote(justfile_directory() / "build/dev-") + postfix
+strict_dir := quote(justfile_directory() / "build/strict-") + postfix
+
+# SETUP COMMANDS
+
+setup: configure-dev
+  [ -f "$PWD/compile_commands.json" ] && rm $PWD/compile_commands.json || true
+  ln -s {{dev_dir}}/compile_commands.json $PWD/
+
+clean:
+  rm -rf $PWD/build
+  rm $PWD/compile_commands.json
+
+# INSTALL COMMANDS
+
+install: build-dev
+  cmake --build {{dev_dir}} --target install
+
+# BUILD COMMANDS
+
+build-dev: configure-dev
+  cmake --build {{ dev_dir }}
+
+build-strict: configure-strict
+  cmake --build {{ strict_dir }}
+
+dev-docker +ARGS='':
+  docker run --rm --platform linux/amd64 \
+  --mount type=bind,src=$PWD,dst=/mnt/workdir \
+  --mount type=bind,src=$HOME/.atsign/keys,dst=/root/.atsign/keys \
+  atsigncompany/valgrind:latest \
+  just build-dev
+
+strict-docker +ARGS='':
+  docker run --rm --platform linux/amd64 \
+  --mount type=bind,src=$PWD,dst=/mnt/workdir \
+  --mount type=bind,src=$HOME/.atsign/keys,dst=/root/.atsign/keys \
+  atsigncompany/valgrind:latest \
+  just build-strict
+
+memcheck +ARGS='': build-dev
+  #!/usr/bin/env bash
+  file="valgrind-sshnpd-$(date +%s).log"
+  valgrind --log-file="$file" \
+  --tool=memcheck \
+  --leak-check=full \
+  --show-leak-kinds=all \
+  {{dev_dir}}/sshnpd/sshnpd -a $TO_ATSIGN -m $FROM_ATSIGN -d $DEVICE_NAME \
+  --permit-open "*:*" -v
+  echo "memcheck log saved to: $file"
+
+# CONTAINER COMMANDS
+
+graceful-shutdown:
+  {{dev_dir}}/graceful-shutdown-tool/graceful-shutdown-tool \
+    $FROM_ATSIGN $TO_ATSIGN $DEVICE_NAME
+
+memcheck-docker +ARGS='':
+  docker run --rm --platform linux/amd64 -ti \
+  --network=host \
+  --mount type=bind,src=$PWD,dst=/mnt/workdir \
+  --mount type=bind,src=$HOME/.atsign/keys,dst=/root/.atsign/keys \
+  atsigncompany/valgrind:latest  \
+  /bin/bash -c "mkdir /root/.ssh/; touch /root/.ssh/authorized_keys; export USER=root; export HOME=/root; just memcheck {{ARGS}}"
+
+valgrind-docker:
+  docker run --rm --platform linux/amd64 -ti \
+  --network=host \
+  --mount type=bind,src=$PWD,dst=/mnt/workdir \
+  --mount type=bind,src=$HOME/.atsign/keys,dst=/root/.atsign/keys \
+  atsigncompany/valgrind:latest  \
+  /bin/bash -c "export USER=root; export HOME=/root; /bin/bash --login"
+
+# CONFIGURE COMMANDS
+
+configure-dev:
+  cmake -B {{ dev_dir }} \
+    -S $PWD \
+    -G "$GENERATOR" \
+    --preset dev \
+    -DCMAKE_INSTALL_PREFIX="$HOME/.local/" \
+    -DCMAKE_BUILD_TYPE=Debug \
+    -DCMAKE_C_COMPILER=$C_COMPILER \
+    -DSSHNPD_ENABLE_TESTING_SHUTDOWN_NOTIFICATION=ON
+
+configure-strict:
+  cmake -B {{ dev_dir }} \
+    -S $PWD \
+    -G "$GENERATOR" \
+    --preset dev \
+    -DCMAKE_INSTALL_PREFIX="$HOME/.local/" \
+    -DCMAKE_BUILD_TYPE=Debug \
+    -DCMAKE_C_COMPILER=$C_COMPILER
+

packages/c/srv/CMakeLists.txt

@@ -2,7 +2,7 @@ cmake_minimum_required(VERSION 3.19)
 set(CMAKE_C_STANDARD 99)
 cmake_policy(SET CMP0135 NEW)
 
-project(srv VERSION 0.4.1 LANGUAGES C)
+project(srv VERSION 0.4.3 LANGUAGES C)
 
 # 1. Variables - you are free to edit anything in this step
 

packages/c/sshnpd/CMakeLists.txt

@@ -1,7 +1,7 @@
 cmake_minimum_required(VERSION 3.19)
 set(CMAKE_C_STANDARD 99)
 cmake_policy(SET CMP0135 NEW)
-project(sshnpd VERSION 0.4.1 LANGUAGES C)
+project(sshnpd VERSION 0.4.3 LANGUAGES C)
 
 # 1. Variables - you are free to edit anything in this step
 
@@ -22,6 +22,10 @@ set(
   ${CMAKE_CURRENT_LIST_DIR}/src/run_srv_process.c
 )
 
+if(SSHNPD_ENABLE_TESTING_SHUTDOWN_NOTIFICATION)
+  add_compile_definitions(SSHNPD_ENABLE_TESTING_SHUTDOWN_NOTIFICATION)
+endif()
+
 # 1b. Manually add your include directories here
 set(SSHNPD_INCLUDE_DIR ${CMAKE_CURRENT_LIST_DIR}/include)
 

packages/c/sshnpd/include/sshnpd/handle_npt_request.h

@@ -5,6 +5,6 @@
 #include <pthread.h>
 
 void handle_npt_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshnpd_params *params,
-                        bool *is_child_process, atclient_monitor_response *message,
+                        bool *is_child_process, atclient_monitor_message *message,
                         atchops_rsa_key_private_key signing_key);
 #endif

packages/c/sshnpd/include/sshnpd/handle_ping.h

@@ -3,6 +3,6 @@
 #include "sshnpd/params.h"
 #include <atclient/monitor.h>
 #include <pthread.h>
-void handle_ping(sshnpd_params *params, atclient_monitor_response *message, char *ping_response, atclient *atclient,
+void handle_ping(sshnpd_params *params, atclient_monitor_message *message, char *ping_response, atclient *atclient,
                  pthread_mutex_t *atclient_lock);
 #endif

packages/c/sshnpd/include/sshnpd/handle_ssh_request.h

@@ -5,7 +5,7 @@
 #include <pthread.h>
 
 void handle_ssh_request(atclient *atclient, pthread_mutex_t *atclient_lock, sshnpd_params *params,
-                        bool *is_child_process, atclient_monitor_response *message,
+                        bool *is_child_process, atclient_monitor_message *message,
                         atchops_rsa_key_private_key signing_key);
 
 #endif

packages/c/sshnpd/include/sshnpd/handle_sshpublickey.h

@@ -2,6 +2,6 @@
 #define HANDLE_SSHPUBLICKEY_H
 #include "sshnpd/params.h"
 #include <atclient/monitor.h>
-void handle_sshpublickey(sshnpd_params *params, atclient_monitor_response *message, FILE *authkeys_file,
+void handle_sshpublickey(sshnpd_params *params, atclient_monitor_message *message, FILE *authkeys_file,
                          char *authkeys_filename);
 #endif

packages/c/sshnpd/include/sshnpd/handler_commons.h

@@ -14,7 +14,7 @@ int verify_envelope_signature(atchops_rsa_key_public_key *publickey, const unsig
 
 enum payload_type { payload_type_ssh, payload_type_npt };
 
-cJSON *extract_envelope_from_notification(atclient_monitor_response *message);
+cJSON *extract_envelope_from_notification(atclient_monitor_message *message);
 
 int verify_envelope_contents(cJSON *envelope, enum payload_type type);
 

packages/c/sshnpd/include/sshnpd/params.h

@@ -14,7 +14,7 @@ enum SupportedSshAlgorithm {
   RSA,
 };
 
-struct _sshnpd_params {
+typedef struct {
   char *atsign;
   char *device;
 
@@ -41,8 +41,7 @@ struct _sshnpd_params {
 
   char *key_file;
   char *storage_path;
-};
-typedef struct _sshnpd_params sshnpd_params;
+} sshnpd_params;
 
 void apply_default_values_to_sshnpd_params(sshnpd_params *params);
 int parse_sshnpd_params(sshnpd_params *params, int argc, const char **argv);