A collection of small fixes (#198)

* tests: Remove if-pyramids

* tests: Switch run_command arguments

* tests: Make run_command a vararg function

* tests: Xcode doesn't obey CMake's test working directory

* openssl: move manual AES-CTR cipher into crypto init

* cmake: Move our include dir before all other include paths

docs/HACKING.CRYPTO

@@ -284,10 +284,6 @@ LIBSSH2_AES_CTR
 #define as 1 if the crypto library supports AES in CTR mode, else 0.
 If defined as 0, the rest of this section can be omitted.
 
-void _libssh2_init_aes_ctr(void);
-Initialize static AES CTR ciphers.
-This procedure is already prototyped in crypto.h.
-
 _libssh2_cipher_aes128ctr
 AES-128-CTR algorithm identifier initializer.
 #define with constant value of type _libssh2_cipher_type().

src/CMakeLists.txt

@@ -217,7 +217,7 @@ set_target_properties(libssh2 PROPERTIES PREFIX "")
 
 target_compile_definitions(libssh2 PRIVATE ${PRIVATE_COMPILE_DEFINITIONS})
 target_include_directories(libssh2
-  PRIVATE ${PRIVATE_INCLUDE_DIRECTORIES}
+  PRIVATE "${PROJECT_SOURCE_DIR}/include/" ${PRIVATE_INCLUDE_DIRECTORIES}
   PUBLIC
     $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/include>
     $<INSTALL_INTERFACE:$<INSTALL_PREFIX>/${CMAKE_INSTALL_INCLUDEDIR}>)

src/crypto.h

@@ -194,6 +194,4 @@ int _libssh2_pub_priv_keyfilememory(LIBSSH2_SESSION *session,
                                     size_t privatekeydata_len,
                                     const char *passphrase);
 
-void _libssh2_init_aes_ctr(void);
-
 #endif

src/global.c

@@ -46,9 +46,6 @@ libssh2_init(int flags)
 {
     if(_libssh2_initialized == 0 && !(flags & LIBSSH2_INIT_NO_CRYPTO)) {
         libssh2_crypto_init();
-#if LIBSSH2_AES_CTR
-        _libssh2_init_aes_ctr();
-#endif
     }
 
     _libssh2_initialized++;

src/openssl.c

@@ -617,17 +617,26 @@ _libssh2_EVP_aes_256_ctr(void)
 #endif
 }
 
-void _libssh2_init_aes_ctr(void)
+#endif /* LIBSSH2_AES_CTR */
+
+void _libssh2_openssl_crypto_init(void)
 {
+#if OPENSSL_VERSION_NUMBER >= 0x10100000L
+    ENGINE_load_builtin_engines();
+    ENGINE_register_all_complete();
+#else
+    OpenSSL_add_all_algorithms();
+    OpenSSL_add_all_ciphers();
+    ENGINE_load_builtin_engines();
+    ENGINE_register_all_complete();
+#endif
+#ifndef HAVE_EVP_AES_128_CTR
     _libssh2_EVP_aes_128_ctr();
     _libssh2_EVP_aes_192_ctr();
     _libssh2_EVP_aes_256_ctr();
+#endif
 }
 
-#else
-void _libssh2_init_aes_ctr(void) {}
-#endif /* LIBSSH2_AES_CTR */
-
 /* TODO: Optionally call a passphrase callback specified by the
  * calling program
  */

src/openssl.h

@@ -280,18 +280,8 @@ int _libssh2_md5_init(libssh2_md5_ctx *ctx);
 #define libssh2_hmac_cleanup(ctx) HMAC_cleanup(ctx)
 #endif
 
-#if OPENSSL_VERSION_NUMBER >= 0x10100000L
+extern void _libssh2_openssl_crypto_init(void);
-#define libssh2_crypto_init() \
+#define libssh2_crypto_init() _libssh2_openssl_crypto_init()
-  ENGINE_load_builtin_engines(); \
-  ENGINE_register_all_complete()
-#else
-#define libssh2_crypto_init() \
-  OpenSSL_add_all_algorithms(); \
-  OpenSSL_add_all_ciphers(); \
-  ENGINE_load_builtin_engines(); \
-  ENGINE_register_all_complete()
-#endif
-
 #define libssh2_crypto_exit()
 
 #define libssh2_rsa_ctx RSA

src/os400qc3.c

@@ -2402,11 +2402,6 @@ _libssh2_os400qc3_rsa_sha1_signv(LIBSSH2_SESSION *session,
     return 0;
 }
 
-void
-_libssh2_init_aes_ctr(void)
-{
-}
-
 #endif /* LIBSSH2_OS400QC3 */
 
 /* vim: set expandtab ts=4 sw=4: */

src/wincng.c

@@ -2164,15 +2164,4 @@ _libssh2_dh_dtor(_libssh2_dh_ctx *dhctx)
     *dhctx = NULL;
 }
 
-
-/*
- * Windows CNG backend: other functions
- */
-
-void _libssh2_init_aes_ctr(void)
-{
-    /* no implementation */
-    (void)0;
-}
-
 #endif /* LIBSSH2_WINCNG */

src/wincng.h

@@ -391,7 +391,6 @@ _libssh2_bn *_libssh2_wincng_bignum_init(void);
 void _libssh2_wincng_init(void);
 void _libssh2_wincng_free(void);
 int _libssh2_wincng_random(void *buf, int len);
-void _libssh2_init_aes_ctr(void);
 
 int
 _libssh2_wincng_hash_init(_libssh2_wincng_hash_ctx *ctx,

tests/CMakeLists.txt

@@ -43,6 +43,7 @@ include(SocketLibraries)
 ## Platform checks
 check_include_files(inttypes.h HAVE_INTTYPES_H)
 check_include_files(unistd.h HAVE_UNISTD_H)
+check_include_files(sys/param.h HAVE_SYS_PARAM_H)
 check_include_files(sys/socket.h HAVE_SYS_SOCKET_H)
 check_include_files(arpa/inet.h HAVE_ARPA_INET_H)
 check_include_files(windows.h HAVE_WINDOWS_H)
@@ -89,6 +90,7 @@ foreach(test ${TESTS})
   target_link_libraries(test_${test} libssh2 runner ${LIBRARIES})
   target_include_directories(test_${test} PRIVATE "${CMAKE_CURRENT_BINARY_DIR}")
   list(APPEND TEST_TARGETS test_${test})
+  add_definitions(-DFIXTURE_WORKDIR="${CMAKE_CURRENT_SOURCE_DIR}")
 
   add_test(
     NAME test_${test} COMMAND $<TARGET_FILE:test_${test}>

tests/libssh2_config_cmake.h.in

@@ -37,6 +37,7 @@
 /* Headers */
 #cmakedefine HAVE_UNISTD_H
 #cmakedefine HAVE_INTTYPES_H
+#cmakedefine HAVE_SYS_PARAM_H
 #cmakedefine HAVE_SYS_SOCKET_H
 #cmakedefine HAVE_ARPA_INET_H
 #cmakedefine HAVE_NETINET_IN_H

tests/openssh_fixture.c

@@ -57,87 +57,98 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <stdarg.h>
 
-static int run_command(const char *command, char **output)
+static int run_command_varg(char **output, const char *command, va_list args)
 {
     FILE *pipe;
     char command_buf[BUFSIZ];
+    char buf[BUFSIZ];
+    char *p;
     int ret;
     if(output) {
         *output = NULL;
     }
 
-    /* Rewrite the command to redirect stderr to stdout to we can output it */
+    /* Format the command string */
-    ret = snprintf(command_buf, sizeof(command_buf), "%s 2>&1", command);
+    ret = vsnprintf(command_buf, sizeof(command_buf), command, args);
     if(ret < 0 || ret >= BUFSIZ) {
         fprintf(stderr, "Unable to format command (%s)\n", command);
         return -1;
     }
 
+    /* Rewrite the command to redirect stderr to stdout to we can output it */
+    if(strlen(command_buf) + 6 >= sizeof(command_buf)) {
+        fprintf(stderr, "Unable to rewrite command (%s)\n", command);
+        return -1;
+    }
+
+    strncat(command_buf, " 2>&1", 6);
+
     fprintf(stdout, "Command: %s\n", command);
 #ifdef WIN32
     pipe = _popen(command_buf, "r");
 #else
     pipe = popen(command_buf, "r");
 #endif
-    if(pipe) {
+    if(!pipe) {
-        char buf[BUFSIZ];
-        char *p = buf;
-        while(fgets(p, sizeof(buf) - (p - buf), pipe) != NULL)
-            ;
-
-#ifdef WIN32
-        ret = _pclose(pipe);
-#else
-        ret = pclose(pipe);
-#endif
-        if(ret == 0) {
-            if(output) {
-                /* command output may contain a trailing newline, so we trim
-                 * whitespace here */
-                size_t end = strlen(buf) - 1;
-                while(end > 0 && isspace(buf[end])) {
-                    buf[end] = '\0';
-                }
-
-                *output = strdup(buf);
-            }
-        }
-        else {
-            fprintf(stderr, "Error running command '%s' (exit %d): %s\n",
-                    command, ret, buf);
-        }
-        return ret;
-    }
-    else {
         fprintf(stderr, "Unable to execute command '%s'\n", command);
         return -1;
     }
+    p = buf;
+    while(fgets(p, sizeof(buf) - (p - buf), pipe) != NULL)
+        ;
+
+#ifdef WIN32
+    ret = _pclose(pipe);
+#else
+    ret = pclose(pipe);
+#endif
+    if(ret != 0) {
+        fprintf(stderr, "Error running command '%s' (exit %d): %s\n",
+                command, ret, buf);
+    }
+
+    if(output) {
+        /* command output may contain a trailing newline, so we trim
+         * whitespace here */
+        size_t end = strlen(buf) - 1;
+        while(end > 0 && isspace(buf[end])) {
+            buf[end] = '\0';
+        }
+
+        *output = strdup(buf);
+    }
+    return ret;
+}
+
+static int run_command(char **output, const char *command, ...)
+{
+    va_list args;
+    int ret;
+
+    va_start(args, command);
+    ret = run_command_varg(output, command, args);
+    va_end(args);
+
+    return ret;
 }
 
 static int build_openssh_server_docker_image()
 {
-    return run_command("docker build -t libssh2/openssh_server openssh_server",
+    return run_command(NULL, "docker build -t libssh2/openssh_server openssh_server");
-                       NULL);
 }
 
 static int start_openssh_server(char **container_id_out)
 {
-    return run_command("docker run --detach -P libssh2/openssh_server",
+    return run_command(container_id_out,
-                       container_id_out);
+                       "docker run --detach -P libssh2/openssh_server"
+                       );
 }
 
 static int stop_openssh_server(char *container_id)
 {
-    char command_buf[BUFSIZ];
+    return run_command(NULL, "docker stop %s", container_id);
-    int rc = snprintf(command_buf, sizeof(command_buf), "docker stop %s",
-                      container_id);
-    if(rc > -1 && rc < BUFSIZ) {
-        return run_command(command_buf, NULL);
-    }
-    else {
-        return rc;
-    }
 }
 
 static const char *docker_machine_name()
@@ -156,12 +167,7 @@ static int ip_address_from_container(char *container_id, char **ip_address_out)
         int attempt_no = 0;
         int wait_time = 500;
         for(;;) {
-            char command_buf[BUFSIZ];
+            return run_command(ip_address_out, "docker-machine ip %s", active_docker_machine);
-            int rc = snprintf(command_buf, sizeof(command_buf),
-                              "docker-machine ip %s", active_docker_machine);
-            if(rc > -1 && rc < BUFSIZ) {
-                return run_command(command_buf, ip_address_out);
-            }
 
             if(attempt_no > 5) {
                 fprintf(
@@ -185,93 +191,75 @@ static int ip_address_from_container(char *container_id, char **ip_address_out)
         }
     }
     else {
-        char command_buf[BUFSIZ];
+        return run_command(ip_address_out,
-        int rc = snprintf(
+                           "docker inspect --format "
-            command_buf, sizeof(command_buf),
+                           "\"{{ index (index (index .NetworkSettings.Ports "
-            "docker inspect --format \"{{ index (index (index "
+                           "\\\"22/tcp\\\") 0) \\\"HostIp\\\" }}\" %s",
-            ".NetworkSettings.Ports \\\"22/tcp\\\") 0) \\\"HostIp\\\" }}\" %s",
+                           container_id);
-            container_id);
-        if(rc > -1 && rc < BUFSIZ) {
-            return run_command(command_buf, ip_address_out);
-        }
-        else {
-            return rc;
-        }
     }
 }
 
 static int port_from_container(char *container_id, char **port_out)
 {
-    char command_buf[BUFSIZ];
+    return run_command(port_out,
-    int rc = snprintf(
+                       "docker inspect --format "
-        command_buf, sizeof(command_buf),
+                       "\"{{ index (index (index .NetworkSettings.Ports "
-        "docker inspect --format \"{{ index (index (index "
+                       "\\\"22/tcp\\\") 0) \\\"HostPort\\\" }}\" %s",
-        ".NetworkSettings.Ports \\\"22/tcp\\\") 0) \\\"HostPort\\\" }}\" %s",
+                       container_id);
-        container_id);
-    if(rc > -1 && rc < BUFSIZ) {
-        return run_command(command_buf, port_out);
-    }
-    else {
-        return rc;
-    }
 }
 
 static int open_socket_to_container(char *container_id)
 {
     char *ip_address = NULL;
+    char *port_string = NULL;
+    unsigned long hostaddr;
+    int sock;
+    struct sockaddr_in sin;
 
     int ret = ip_address_from_container(container_id, &ip_address);
-    if(ret == 0) {
+    if(ret != 0) {
-        char *port_string = NULL;
+        fprintf(stderr, "Failed to get IP address for container %s\n", container_id);
-        ret = port_from_container(container_id, &port_string);
+        ret = -1;
-        if(ret == 0) {
+        goto cleanup;
-            unsigned long hostaddr = inet_addr(ip_address);
-            if(hostaddr != (unsigned long)(-1)) {
-                int sock = socket(AF_INET, SOCK_STREAM, 0);
-                if(sock > -1) {
-                    struct sockaddr_in sin;
-
-                    sin.sin_family = AF_INET;
-                    sin.sin_port = htons((short)strtol(port_string, NULL, 0));
-                    sin.sin_addr.s_addr = hostaddr;
-
-                    if(connect(sock, (struct sockaddr *)(&sin),
-                               sizeof(struct sockaddr_in)) == 0) {
-                        ret = sock;
-                    }
-                    else {
-                        fprintf(stderr, "Failed to connect to %s:%s\n",
-                                ip_address, port_string);
-                        ret = -1;
-                    }
-                }
-                else {
-                    fprintf(stderr, "Failed to open socket (%d)\n", sock);
-                    ret = -1;
-                }
-            }
-            else {
-                fprintf(stderr, "Failed to convert %s host address\n",
-                        ip_address);
-                ret = -1;
-            }
-
-            free(port_string);
-        }
-        else {
-            fprintf(stderr, "Failed to get port for container %s\n",
-                    container_id);
-            ret = -1;
-        }
-
-        free(ip_address);
     }
-    else {
+
-        fprintf(stderr, "Failed to get IP address for container %s\n",
+    ret = port_from_container(container_id, &port_string);
-                container_id);
+    if(ret != 0) {
+        fprintf(stderr, "Failed to get port for container %s\n", container_id);
         ret = -1;
     }
 
+    hostaddr = inet_addr(ip_address);
+    if(hostaddr == (unsigned long)(-1)) {
+        fprintf(stderr, "Failed to convert %s host address\n", ip_address);
+        ret = -1;
+        goto cleanup;
+    }
+
+    sock = socket(AF_INET, SOCK_STREAM, 0);
+    if(sock <= 0) {
+        fprintf(stderr, "Failed to open socket (%d)\n", sock);
+        ret = -1;
+        goto cleanup;
+    }
+
+    sin.sin_family = AF_INET;
+    sin.sin_port = htons((short)strtol(port_string, NULL, 0));
+    sin.sin_addr.s_addr = hostaddr;
+
+    if(connect(sock, (struct sockaddr *)(&sin),
+               sizeof(struct sockaddr_in)) != 0) {
+        fprintf(stderr, "Failed to connect to %s:%s\n", ip_address, port_string);
+        ret = -1;
+        goto cleanup;
+    }
+
+    ret = sock;
+
+cleanup:
+    free(ip_address);
+    free(port_string);
+
     return ret;
 }
 

tests/session_fixture.c

@@ -40,6 +40,10 @@
 #include "openssh_fixture.h"
 
 #include <stdio.h>
+#include <stdlib.h>
+#ifdef HAVE_UNISTD_H
+#include <unistd.h>
+#endif
 
 #ifdef HAVE_WINDOWS_H
 #include <windows.h>
@@ -50,58 +54,80 @@
 #ifdef HAVE_SYS_SOCKET_H
 #include <sys/socket.h>
 #endif
+#ifdef HAVE_SYS_PARAM_H
+#include <sys/param.h>
+#endif
 
 LIBSSH2_SESSION *connected_session = NULL;
 int connected_socket = -1;
 
 static int connect_to_server()
 {
+    int rc;
     connected_socket = open_socket_to_openssh_server();
-    if(connected_socket > -1) {
+    if(connected_socket <= 0) {
-        int rc = libssh2_session_handshake(connected_session, connected_socket);
-        if(rc == 0) {
-            return 0;
-        }
-        else {
-            print_last_session_error("libssh2_session_handshake");
-            return -1;
-        }
-    }
-    else {
         return -1;
     }
+
+    rc = libssh2_session_handshake(connected_session, connected_socket);
+    if(rc != 0) {
+        print_last_session_error("libssh2_session_handshake");
+        return -1;
+    }
+
+    return 0;
+}
+
+void setup_fixture_workdir()
+{
+    char *wd = getenv("FIXTURE_WORKDIR");
+#ifdef FIXTURE_WORKDIR
+    if(!wd) {
+        wd = FIXTURE_WORKDIR;
+    }
+#endif
+    if(!wd) {
+#ifdef WIN32
+        char wd_buf[_MAX_PATH];
+#else
+        char wd_buf[MAXPATHLEN];
+#endif
+        getcwd(wd_buf, sizeof(wd_buf));
+        wd = wd_buf;
+    }
+
+    chdir(wd);
 }
 
 LIBSSH2_SESSION *start_session_fixture()
 {
-    int rc = start_openssh_fixture();
+    int rc;
-    if(rc == 0) {
+
-        rc = libssh2_init(0);
+    setup_fixture_workdir();
-        if(rc == 0) {
+
-            connected_session = libssh2_session_init_ex(NULL, NULL, NULL, NULL);
+    rc = start_openssh_fixture();
-            libssh2_session_set_blocking(connected_session, 1);
+    if(rc != 0) {
-            if(connected_session != NULL) {
-                rc = connect_to_server();
-                if(rc == 0) {
-                    return connected_session;
-                }
-                else {
-                    return NULL;
-                }
-            }
-            else {
-                fprintf(stderr, "libssh2_session_init_ex failed\n");
-                return NULL;
-            }
-        }
-        else {
-            fprintf(stderr, "libssh2_init failed (%d)\n", rc);
-            return NULL;
-        }
-    }
-    else {
         return NULL;
     }
+    rc = libssh2_init(0);
+    if(rc != 0) {
+        fprintf(stderr, "libssh2_init failed (%d)\n", rc);
+        return NULL;
+    }
+
+    connected_session = libssh2_session_init_ex(NULL, NULL, NULL, NULL);
+    libssh2_session_set_blocking(connected_session, 1);
+    if(connected_session == NULL) {
+        fprintf(stderr, "libssh2_session_init_ex failed\n");
+        return NULL;
+    }
+
+    rc = connect_to_server();
+    if(rc != 0) {
+        return NULL;
+    }
+
+    return connected_session;
 }
 
 void print_last_session_error(const char *function)