Code clean up and look for credentials every 10 seconds.

Author: matt001k

CMakeLists.txt

@@ -75,6 +75,7 @@ add_library(bm_sbc_core STATIC
   src/core/runtime.cpp
   src/core/app_runner.cpp
   src/core/pcap_file_sink.cpp
+  src/core/safe_cmd.cpp
   src/platform/linux/platform_linux.cpp
   src/net/virtual_port_device.cpp
   src/net/gateway_device.cpp

apps/gateway/app_main.cpp

@@ -14,19 +14,17 @@ extern "C" {
 #include "gateway_device.h"
 #include "gateway_ipc.h"
 #include "runtime.h"
+#include "safe_cmd.h"
 #include <arpa/inet.h>
 #include <atomic>
 #include <cstdio>
 #include <cstdlib>
 #include <cstring>
 #include <errno.h>
 #include <filesystem>
-#include <string>
 #include <sys/socket.h>
-#include <sys/wait.h>
 #include <time.h>
 #include <unistd.h>
-#include <vector>
 
 using namespace std::filesystem;
 
@@ -596,27 +594,6 @@ static void get_wifi_enable(void) {
 
 static const std::string connection_name = "user-wifi";
 
-static int run(const std::vector<const char *> &args) {
-  // Prevent command injection from system() by utilizing execvp
-  std::vector<char *> argv;
-  for (auto *a : args) {
-    argv.push_back(const_cast<char *>(a));
-  }
-  argv.push_back(nullptr);
-
-  pid_t pid = fork();
-  if (pid < 0)
-    return -1;
-  if (pid == 0) {
-    execvp(args[0], argv.data());
-    _exit(127);
-  }
-  int status = 0;
-  if (waitpid(pid, &status, 0) < 0)
-    return -1;
-  return WIFEXITED(status) ? WEXITSTATUS(status) : -1;
-}
-
 static bool copy_connections(void) {
   static const path connection_location =
       "/etc/NetworkManager/system-connections/";
@@ -714,17 +691,18 @@ static BmErr set_wifi_credential(std::string &cred, uint8_t *payload) {
   }
 
   // If both SSID and password are available create a new network manager
-  // connection.
+  // connection
   if (CONTEXT.wifi_password.size() && CONTEXT.wifi_ssid.size()) {
 
     bm_log_info("Saving wifi credentials...");
 
     // Remove any existing wifi credentials and then add the new one
-    run({"nmcli", "connection", "delete", connection_name.c_str()});
-    int ret = run({"nmcli", "connection", "add", "type", "wifi", "ifname",
-                   "wlan0", "con-name", connection_name.c_str(), "ssid",
-                   CONTEXT.wifi_ssid.c_str(), "wifi-sec.key-mgmt", "wpa-psk",
-                   "wifi-sec.psk", CONTEXT.wifi_password.c_str()});
+    std::string cmd = "nmcli connection delete " + connection_name;
+    safe_cmd(cmd.c_str());
+    cmd = "nmcli connection add type wifi ifname wlan0 con-name " +
+          connection_name + " ssid " + CONTEXT.wifi_ssid +
+          " wifi-sec.key-mgmt wpa-psk wifi-sec.psk " + CONTEXT.wifi_password;
+    int ret = safe_cmd(cmd.c_str());
     if (ret != 0) {
       bm_log_error("Could not save wifi credentials, err: %d", ret);
       return BmEBADMSG;
@@ -756,7 +734,9 @@ static BmErr wifi_password_cb(uint8_t *payload) {
   return set_wifi_credential(CONTEXT.wifi_password, payload);
 }
 
-static void get_wifi_credentials(void) {
+static void get_wifi_credentials(BmTimer timer = nullptr) {
+  (void)timer;
+
   bm_log_debug("Ticks before bcmp config get in %s: %u", __func__,
                bm_get_tick_count());
   BmErr err = BmOK;
@@ -766,6 +746,27 @@ static void get_wifi_credentials(void) {
                   WIFI_PASS_KEY_LEN, WIFI_PASS_KEY, &err, wifi_password_cb);
 }
 
+static void start_wifi_credentials_timer(void) {
+  static BmTimer timer = nullptr;
+
+  if (timer) {
+    return;
+  }
+
+  // See if credentials are there to begin with
+  get_wifi_credentials();
+
+  // Check for credentials every timer_ms
+  static constexpr uint32_t timer_ms = 10000;
+  static constexpr uint32_t timer_wait_ms = 10;
+  timer = bm_timer_create("creds", timer_ms, true, NULL, get_wifi_credentials);
+  if (!timer) {
+    return;
+  }
+
+  bm_timer_start(timer, timer_wait_ms);
+}
+
 #define MAX_NMEA_RMC_LEN 82
 #define MAX_NMEA_FIELDS 14
 
@@ -1022,7 +1023,7 @@ void setup(void) {
   get_mote_system_configs();
   get_sbc_command();
   get_wifi_enable();
-  get_wifi_credentials();
+  start_wifi_credentials_timer();
   gateway_ipc_init(CONTEXT.mote_node_id);
 }
 

src/core/safe_cmd.cpp

@@ -0,0 +1,52 @@
+#include "safe_cmd.h"
+#include <stdio.h>
+#include <string.h>
+#include <sys/wait.h>
+#include <unistd.h>
+#include <vector>
+
+/*!
+ @brief Prevent command injection with a safe system call
+
+ @details Uses execvp instead of system to prevent command injection from
+          calls that have exposure to the outside world.
+
+ @param cmd command string to run
+
+ @return 0 on success
+         other on failure
+ */
+int safe_cmd(const char *cmd) {
+  std::vector<char *> argv;
+
+  // Copy string to local variable
+  size_t cmd_len = strlen(cmd) + 1;
+  char *local = static_cast<char *>(malloc(cmd_len));
+  if (!local) {
+    return -1;
+  }
+  strncpy(local, cmd, cmd_len);
+
+  char *token = strtok(local, " ");
+  while (token) {
+    argv.push_back(token);
+    token = strtok(NULL, " ");
+  }
+  argv.push_back(nullptr);
+
+  free(local);
+
+  pid_t pid = fork();
+  if (pid < 0) {
+    return -1;
+  }
+  if (pid == 0) {
+    execvp(argv[0], argv.data());
+    _exit(127);
+  }
+  int status = 0;
+  if (waitpid(pid, &status, 0) < 0) {
+    return -1;
+  }
+  return WIFEXITED(status) ? WEXITSTATUS(status) : -1;
+}

src/core/safe_cmd.h

@@ -0,0 +1,6 @@
+#ifndef __SAFE_CMD_H__
+#define __SAFE_CMD_H__
+
+int safe_cmd(const char *cmd);
+
+#endif