From 54bef4c5dad868a9d45fdbfca9729b191c0abab5 Mon Sep 17 00:00:00 2001 From: Etienne Samson Date: Wed, 21 Mar 2018 00:58:14 +0100 Subject: [PATCH] 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 | 4 - src/CMakeLists.txt | 2 +- src/crypto.h | 2 - src/global.c | 3 - src/openssl.c | 19 ++- src/openssl.h | 14 +- src/os400qc3.c | 5 - src/wincng.c | 11 -- src/wincng.h | 1 - tests/CMakeLists.txt | 2 + tests/libssh2_config_cmake.h.in | 1 + tests/openssh_fixture.c | 234 +++++++++++++++----------------- tests/session_fixture.c | 100 +++++++++----- 13 files changed, 194 insertions(+), 204 deletions(-) diff --git a/docs/HACKING.CRYPTO b/docs/HACKING.CRYPTO index 9a209a6..e1c4f38 100644 --- a/docs/HACKING.CRYPTO +++ b/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(). diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index e127094..76e18a3 100644 --- a/src/CMakeLists.txt +++ b/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 $ $/${CMAKE_INSTALL_INCLUDEDIR}>) diff --git a/src/crypto.h b/src/crypto.h index 76f5d57..c967630 100644 --- a/src/crypto.h +++ b/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 diff --git a/src/global.c b/src/global.c index 5f57dac..f88eb33 100644 --- a/src/global.c +++ b/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++; diff --git a/src/openssl.c b/src/openssl.c index 0dae358..f59b4f7 100644 --- a/src/openssl.c +++ b/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 */ diff --git a/src/openssl.h b/src/openssl.h index 3399c2e..35a1246 100644 --- a/src/openssl.h +++ b/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 -#define libssh2_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 - +extern void _libssh2_openssl_crypto_init(void); +#define libssh2_crypto_init() _libssh2_openssl_crypto_init() #define libssh2_crypto_exit() #define libssh2_rsa_ctx RSA diff --git a/src/os400qc3.c b/src/os400qc3.c index 955ed3b..8ba2e1c 100644 --- a/src/os400qc3.c +++ b/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: */ diff --git a/src/wincng.c b/src/wincng.c index 43008a4..b93e45b 100755 --- a/src/wincng.c +++ b/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 */ diff --git a/src/wincng.h b/src/wincng.h index 2d9c061..5966de4 100755 --- a/src/wincng.h +++ b/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, diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 79b8b62..1873038 100644 --- a/tests/CMakeLists.txt +++ b/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 $ diff --git a/tests/libssh2_config_cmake.h.in b/tests/libssh2_config_cmake.h.in index 51461f2..4df27ec 100644 --- a/tests/libssh2_config_cmake.h.in +++ b/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 diff --git a/tests/openssh_fixture.c b/tests/openssh_fixture.c index b0a5c56..093f31e 100644 --- a/tests/openssh_fixture.c +++ b/tests/openssh_fixture.c @@ -57,87 +57,98 @@ #include #include #include +#include -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 */ - ret = snprintf(command_buf, sizeof(command_buf), "%s 2>&1", command); + /* Format the command string */ + 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) { - 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 { + if(!pipe) { 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", - NULL); + return run_command(NULL, "docker build -t libssh2/openssh_server openssh_server"); } static int start_openssh_server(char **container_id_out) { - return run_command("docker run --detach -P libssh2/openssh_server", - container_id_out); + return run_command(container_id_out, + "docker run --detach -P libssh2/openssh_server" + ); } static int stop_openssh_server(char *container_id) { - char command_buf[BUFSIZ]; - 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; - } + return run_command(NULL, "docker stop %s", container_id); } 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]; - 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); - } + return run_command(ip_address_out, "docker-machine ip %s", active_docker_machine); 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]; - int rc = snprintf( - command_buf, sizeof(command_buf), - "docker inspect --format \"{{ index (index (index " - ".NetworkSettings.Ports \\\"22/tcp\\\") 0) \\\"HostIp\\\" }}\" %s", - container_id); - if(rc > -1 && rc < BUFSIZ) { - return run_command(command_buf, ip_address_out); - } - else { - return rc; - } + return run_command(ip_address_out, + "docker inspect --format " + "\"{{ index (index (index .NetworkSettings.Ports " + "\\\"22/tcp\\\") 0) \\\"HostIp\\\" }}\" %s", + container_id); } } static int port_from_container(char *container_id, char **port_out) { - char command_buf[BUFSIZ]; - int rc = snprintf( - command_buf, sizeof(command_buf), - "docker inspect --format \"{{ index (index (index " - ".NetworkSettings.Ports \\\"22/tcp\\\") 0) \\\"HostPort\\\" }}\" %s", - container_id); - if(rc > -1 && rc < BUFSIZ) { - return run_command(command_buf, port_out); - } - else { - return rc; - } + return run_command(port_out, + "docker inspect --format " + "\"{{ index (index (index .NetworkSettings.Ports " + "\\\"22/tcp\\\") 0) \\\"HostPort\\\" }}\" %s", + container_id); } 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) { - char *port_string = NULL; - ret = port_from_container(container_id, &port_string); - if(ret == 0) { - 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); + if(ret != 0) { + fprintf(stderr, "Failed to get IP address for container %s\n", container_id); + ret = -1; + goto cleanup; } - else { - fprintf(stderr, "Failed to get IP address for container %s\n", - container_id); + + ret = port_from_container(container_id, &port_string); + 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; } diff --git a/tests/session_fixture.c b/tests/session_fixture.c index 597c49b..5d8fd21 100644 --- a/tests/session_fixture.c +++ b/tests/session_fixture.c @@ -40,6 +40,10 @@ #include "openssh_fixture.h" #include +#include +#ifdef HAVE_UNISTD_H +#include +#endif #ifdef HAVE_WINDOWS_H #include @@ -50,58 +54,80 @@ #ifdef HAVE_SYS_SOCKET_H #include #endif +#ifdef HAVE_SYS_PARAM_H +#include +#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) { - 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 { + if(connected_socket <= 0) { 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(); - if(rc == 0) { - rc = libssh2_init(0); - if(rc == 0) { - connected_session = libssh2_session_init_ex(NULL, NULL, NULL, NULL); - libssh2_session_set_blocking(connected_session, 1); - 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 { + int rc; + + setup_fixture_workdir(); + + rc = start_openssh_fixture(); + if(rc != 0) { 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)