diff --git a/.github/workflows/clang-format-check.yml b/.github/workflows/clang-format-check.yml index 5ca40da93..0f41d3856 100644 --- a/.github/workflows/clang-format-check.yml +++ b/.github/workflows/clang-format-check.yml @@ -1,5 +1,5 @@ name: clang-format Check -on: [push, pull_request] +on: [pull_request] jobs: formatting-check: name: Formatting Check @@ -15,6 +15,6 @@ jobs: - name: Run clang-format style check for C/C++/Protobuf source code. uses: jidicula/clang-format-action@v4.11.0 with: - clang-format-version: '13' + clang-format-version: '18' check-path: ${{ matrix.path }} fallback-style: 'Google' diff --git a/.github/workflows/codecov.yml b/.github/workflows/codecov.yml new file mode 100644 index 000000000..caf1f0f54 --- /dev/null +++ b/.github/workflows/codecov.yml @@ -0,0 +1,57 @@ +name: Linux CI + +on: + push: + branches: + - master + pull_request: + +jobs: + codecov: + runs-on: ubuntu-22.04 + + steps: + - uses: actions/checkout@v4 + - name: Setup + shell: bash + run: | + mkdir -p ~/.ssh/ + echo -e "Host github.com\n\tStrictHostKeyChecking no\n" >> ~/.ssh/config + sudo apt-get update + sudo DEBIAN_FRONTEND=noninteractive ACCEPT_EULA=Y apt-get install -y curl zip unzip tar libssl-dev libcurl4-openssl-dev libunwind-dev git cmake ninja-build gdb protobuf-compiler libsodium-dev libgflags-dev libprotobuf-dev libutempter-dev g++ lcov + if [[ -z "${ACT}" ]]; then auth_header="$(git config --local --get http.https://github.com/.extraheader)"; fi + git submodule sync --recursive + git submodule update --init --force --recursive + + # Restore both vcpkg and its artifacts from the GitHub cache service. + - name: Restore vcpkg and its artifacts. + uses: actions/cache@v4 + with: + # The first path is where vcpkg generates artifacts while consuming the vcpkg.json manifest file. + # The second path is the location of vcpkg (it contains the vcpkg executable and data files). + # The other paths starting with '!' are exclusions: they contain temporary files generated during the build of the installed packages. + path: | + ${{ env.CMAKE_BUILD_DIR }}/vcpkg_installed/ + ${{ env.VCPKG_ROOT }} + !${{ env.VCPKG_ROOT }}/buildtrees + !${{ env.VCPKG_ROOT }}/packages + !${{ env.VCPKG_ROOT }}/downloads + # The key is composed in a way that it gets properly invalidated: this must happen whenever vcpkg's Git commit id changes, or the list of packages changes. In this case a cache miss must happen and a new entry with a new key with be pushed to GitHub the cache service. + # The key includes: hash of the vcpkg.json file, the hash of the vcpkg Git commit id, and the used vcpkg's triplet. The vcpkg's commit id would suffice, but computing an hash out it does not harm. + # Note: given a key, the cache content is immutable. If a cache entry has been created improperly, in order the recreate the right content the key must be changed as well, and it must be brand new (i.e. not existing already). + key: | + et-vcpkg-${{ hashFiles( 'vcpkg.json' ) }}-${{ hashFiles( '.git/modules/external/vcpkg/HEAD' )}}-linux-codecov + + - name: Build with code coverage + run: | + mkdir build + pushd build + cmake -DCODE_COVERAGE=ON ../ + make -j`nproc` + ./et-test + lcov --capture --directory . --output-file coverage.info + lcov --remove coverage.info '/usr/*' --output-file coverage.info # filter system-files + lcov --list coverage.info # debug info + # Uploading report to CodeCov + bash <(curl -s https://codecov.io/bash) -f coverage.info || echo "Codecov did not collect coverage reports" + popd diff --git a/.github/workflows/linux_ci.yml b/.github/workflows/linux_ci.yml index b96526311..74687483d 100644 --- a/.github/workflows/linux_ci.yml +++ b/.github/workflows/linux_ci.yml @@ -22,7 +22,17 @@ jobs: echo -e "Host github.com\n\tStrictHostKeyChecking no\n" >> ~/.ssh/config sudo apt-get update sudo DEBIAN_FRONTEND=noninteractive ACCEPT_EULA=Y apt-get install -y curl zip unzip tar libssl-dev libcurl4-openssl-dev libunwind-dev git cmake ninja-build gdb protobuf-compiler libsodium-dev libgflags-dev libprotobuf-dev libutempter-dev g++ - auth_header="$(git config --local --get http.https://github.com/.extraheader)" + + echo "Host localhost\n Port 2222\n\n" >> ~/.ssh/config + + sudo /usr/sbin/sshd -p 2222 + + ssh-keygen -t rsa -f ~/.ssh/id_rsa -P "" -N "" + cat ~/.ssh/id_rsa.pub >> ~/.ssh/authorized_keys + cat ~/.ssh/id_rsa.pub >> ~/.ssh/known_hosts + ssh -vvvvvvv -o "StrictHostKeyChecking no" -o 'PreferredAuthentications=publickey' localhost "echo foobar" # Fails if we can't ssh into localhost without a password + if [[ -z "${ACT}" ]]; then auth_header="$(git config --local --get http.https://github.com/.extraheader)"; fi + git submodule sync --recursive git submodule update --init --force --recursive @@ -45,95 +55,46 @@ jobs: key: | et-vcpkg-${{ hashFiles( 'vcpkg.json' ) }}-${{ hashFiles( '.git/modules/external/vcpkg/HEAD' )}}-linux-${{ matrix.sanitize }} - - name: Test with ubsan + - name: Build with ubsan run: | mkdir build pushd build cmake -DSANITIZE_UNDEFINED=ON ../ make -j`nproc` - TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ./et-test popd - rm -Rf build + ./test/system_tests/connect_with_jumphost.sh + TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ./build/et-test if: matrix.sanitize == 'ubsan' - - name: Test with asan + - name: Build with asan run: | mkdir build pushd build cmake -DSANITIZE_ADDRESS=ON ../ make -j`nproc` - TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ./et-test popd - rm -Rf build + ./test/system_tests/connect_with_jumphost.sh + TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ./build/et-test if: matrix.sanitize == 'asan' - - name: Test with msan + - name: Build with msan run: | mkdir build pushd build cmake -DSANITIZE_MEMORY=ON ../ make -j`nproc` - TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ./et-test popd - rm -Rf build + ./test/system_tests/connect_with_jumphost.sh + TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ./build/et-test if: matrix.sanitize == 'msan' - - name: Test with tsan + - name: Build with tsan run: | mkdir build pushd build cmake -DSANITIZE_THREAD=ON -DSANITIZE_LINK_STATIC=ON ../ make -j`nproc` - TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ./et-test popd - rm -Rf build + ./test/system_tests/connect_with_jumphost.sh + TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ./build/et-test if: matrix.sanitize == 'tsan' - - codecov: - runs-on: ubuntu-22.04 - - steps: - - uses: actions/checkout@v4 - - name: Setup - shell: bash - run: | - mkdir -p ~/.ssh/ - echo -e "Host github.com\n\tStrictHostKeyChecking no\n" >> ~/.ssh/config - sudo apt-get update - sudo DEBIAN_FRONTEND=noninteractive ACCEPT_EULA=Y apt-get install -y curl zip unzip tar libssl-dev libcurl4-openssl-dev libunwind-dev git cmake ninja-build gdb protobuf-compiler libsodium-dev libgflags-dev libprotobuf-dev libutempter-dev g++ lcov - auth_header="$(git config --local --get http.https://github.com/.extraheader)" - git submodule sync --recursive - git submodule update --init --force --recursive - - # Restore both vcpkg and its artifacts from the GitHub cache service. - - name: Restore vcpkg and its artifacts. - uses: actions/cache@v4 - with: - # The first path is where vcpkg generates artifacts while consuming the vcpkg.json manifest file. - # The second path is the location of vcpkg (it contains the vcpkg executable and data files). - # The other paths starting with '!' are exclusions: they contain temporary files generated during the build of the installed packages. - path: | - ${{ env.CMAKE_BUILD_DIR }}/vcpkg_installed/ - ${{ env.VCPKG_ROOT }} - !${{ env.VCPKG_ROOT }}/buildtrees - !${{ env.VCPKG_ROOT }}/packages - !${{ env.VCPKG_ROOT }}/downloads - # The key is composed in a way that it gets properly invalidated: this must happen whenever vcpkg's Git commit id changes, or the list of packages changes. In this case a cache miss must happen and a new entry with a new key with be pushed to GitHub the cache service. - # The key includes: hash of the vcpkg.json file, the hash of the vcpkg Git commit id, and the used vcpkg's triplet. The vcpkg's commit id would suffice, but computing an hash out it does not harm. - # Note: given a key, the cache content is immutable. If a cache entry has been created improperly, in order the recreate the right content the key must be changed as well, and it must be brand new (i.e. not existing already). - key: | - et-vcpkg-${{ hashFiles( 'vcpkg.json' ) }}-${{ hashFiles( '.git/modules/external/vcpkg/HEAD' )}}-linux - - - name: Test with code coverage - run: | - mkdir build - pushd build - cmake -DCODE_COVERAGE=ON ../ - make -j`nproc` - ./et-test - lcov --capture --directory . --output-file coverage.info - lcov --remove coverage.info '/usr/*' --output-file coverage.info # filter system-files - lcov --list coverage.info # debug info - # Uploading report to CodeCov - bash <(curl -s https://codecov.io/bash) -f coverage.info || echo "Codecov did not collect coverage reports" - popd diff --git a/src/terminal/ParseConfigFile.hpp b/src/terminal/ParseConfigFile.hpp index b038bc459..5cb0b5f06 100644 --- a/src/terminal/ParseConfigFile.hpp +++ b/src/terminal/ParseConfigFile.hpp @@ -1431,6 +1431,7 @@ int parse_ssh_config_file(const char *targethost, struct Options *options, } ifstream infile(expandedFilename); + free(expandedFilename); if (!infile.good()) { LOG(INFO) << filename << " not found"; return 0; diff --git a/src/terminal/SshSetupHandler.cpp b/src/terminal/SshSetupHandler.cpp index 03027ed32..1b82928b2 100644 --- a/src/terminal/SshSetupHandler.cpp +++ b/src/terminal/SshSetupHandler.cpp @@ -27,7 +27,8 @@ string genCommand(const string& passkey, const string& id, string SshSetupHandler::SetupSsh(const string& user, const string& host, const string& host_alias, int port, - const string& jumphost, int jport, bool kill, + const string& jumphost, + const string& jServerFifo, bool kill, int vlevel, const string& cmd_prefix, const string& serverFifo, const std::vector& ssh_options) { @@ -72,6 +73,9 @@ string SshSetupHandler::SetupSsh(const string& user, const string& host, ssh_args.push_back(SSH_SCRIPT_DST); + std::string ssh_concat; + for (const auto& piece : ssh_args) ssh_concat += piece + " "; + VLOG(1) << "Trying ssh with args: " << ssh_concat << endl; auto sshBuffer = SubprocessToStringInteractive("ssh", ssh_args); try { @@ -107,9 +111,12 @@ string SshSetupHandler::SetupSsh(const string& user, const string& host, if (!jumphost.empty()) { /* If jumphost is set, we need to pass dst host and port to jumphost * and connect to jumphost here */ - string cmdoptions{"--verbose=" + std::to_string(vlevel)}; - string jump_cmdoptions = cmdoptions + " --jump --dsthost=" + host + - " --dstport=" + to_string(port); + string jump_cmdoptions{"--verbose=" + std::to_string(vlevel)}; + if (!jServerFifo.empty()) { + jump_cmdoptions += " --serverfifo=" + jServerFifo; + } + jump_cmdoptions = jump_cmdoptions + " --jump --dsthost=" + host + + " --dstport=" + to_string(port); string SSH_SCRIPT_JUMP = genCommand(passkey, id, clientTerm, user, kill, cmd_prefix, jump_cmdoptions); diff --git a/src/terminal/SshSetupHandler.hpp b/src/terminal/SshSetupHandler.hpp index 5bc0ddb8d..279bee9b6 100644 --- a/src/terminal/SshSetupHandler.hpp +++ b/src/terminal/SshSetupHandler.hpp @@ -8,8 +8,8 @@ class SshSetupHandler { public: static string SetupSsh(const string &user, const string &host, const string &host_alias, int port, - const string &jumphost, int jport, bool kill, - int vlevel, const string &etterminal_path, + const string &jumphost, const string &jServerFifo, + bool kill, int vlevel, const string &etterminal_path, const string &serverFifo, const std::vector &ssh_options); static const string ETTERMINAL_BIN; diff --git a/src/terminal/TelemetryService.cpp b/src/terminal/TelemetryService.cpp index f9f701f8c..75a6df790 100644 --- a/src/terminal/TelemetryService.cpp +++ b/src/terminal/TelemetryService.cpp @@ -209,11 +209,12 @@ TelemetryService::TelemetryService(const bool _allow, logSendingThread.reset(new thread([this]() { auto nextDumpTime = std::chrono::system_clock::now(); while (true) { - bool lastRun = shuttingDown; + bool lastRun; string payload; int logBufferSize; { lock_guard guard(logMutex); + lastRun = shuttingDown; logBufferSize = (int)logBuffer.size(); } if (logBufferSize) { @@ -253,6 +254,7 @@ TelemetryService::TelemetryService(const bool _allow, } TelemetryService::~TelemetryService() { + lock_guard guard(logMutex); if (!shuttingDown) { cerr << "Destroyed telemetryService without a shutdown"; } @@ -283,10 +285,13 @@ void TelemetryService::logToDatadog(const string& logText, el::Level logLevel, } void TelemetryService::shutdown() { - if (shuttingDown) { - return; + { + lock_guard guard(logMutex); + if (shuttingDown) { + return; + } + shuttingDown = true; } - shuttingDown = true; #ifdef USE_SENTRY sentry_shutdown(); #endif diff --git a/src/terminal/TerminalClientMain.cpp b/src/terminal/TerminalClientMain.cpp index c00ef0bea..0114fef7d 100644 --- a/src/terminal/TerminalClientMain.cpp +++ b/src/terminal/TerminalClientMain.cpp @@ -42,6 +42,25 @@ int main(int argc, char** argv) { // Override easylogging handler for sigint ::signal(SIGINT, et::InterruptSignalHandler); + Options sshConfigOptions = { + NULL, // username + NULL, // host + NULL, // sshdir + NULL, // knownhosts + NULL, // ProxyCommand + NULL, // ProxyJump + 0, // timeout + 0, // port + 0, // StrictHostKeyChecking + 0, // ssh2 + 0, // ssh1 + NULL, // gss_server_identity + NULL, // gss_client_identity + 0, // gss_delegate_creds + 0, // forward_agent + NULL // identity_agent + }; + // Parse command line arguments cxxopts::Options options("et", "Remote shell for the busy and impatient"); try { @@ -82,6 +101,9 @@ int main(int argc, char** argv) { cxxopts::value()) // ("jport", "Jumphost machine port", cxxopts::value()->default_value("2022")) // + ("jserverfifo", + "If set, communicate to jumphost on the matching fifo name", + cxxopts::value()->default_value("")) // ("x,kill-other-sessions", "kill all old sessions belonging to the user") // ("macserver", @@ -213,44 +235,32 @@ int main(int argc, char** argv) { exit(0); } - Options sshConfigOptions = { - NULL, // username - NULL, // host - NULL, // sshdir - NULL, // knownhosts - NULL, // ProxyCommand - NULL, // ProxyJump - 0, // timeout - 0, // port - 0, // StrictHostKeyChecking - 0, // ssh2 - 0, // ssh1 - NULL, // gss_server_identity - NULL, // gss_client_identity - 0, // gss_delegate_creds - 0, // forward_agent - NULL // identity_agent - }; - - char* home_dir = ssh_get_user_home_dir(); - const char* host_from_command = destinationHost.c_str(); - ssh_options_set(&sshConfigOptions, SSH_OPTIONS_HOST, - destinationHost.c_str()); - // First parse user-specific ssh config, then system-wide config. - parse_ssh_config_file(host_from_command, &sshConfigOptions, - string(home_dir) + USER_SSH_CONFIG_PATH); - parse_ssh_config_file(host_from_command, &sshConfigOptions, - SYSTEM_SSH_CONFIG_PATH); - LOG(INFO) << "Parsed ssh config file, connecting to " - << sshConfigOptions.host; - destinationHost = string(sshConfigOptions.host); + { + char* home_dir = ssh_get_user_home_dir(); + const char* host_from_command = destinationHost.c_str(); + ssh_options_set(&sshConfigOptions, SSH_OPTIONS_HOST, + destinationHost.c_str()); + // First parse user-specific ssh config, then system-wide config. + parse_ssh_config_file(host_from_command, &sshConfigOptions, + string(home_dir) + USER_SSH_CONFIG_PATH); + parse_ssh_config_file(host_from_command, &sshConfigOptions, + SYSTEM_SSH_CONFIG_PATH); + if (sshConfigOptions.host) { + LOG(INFO) << "Parsed ssh config file, connecting to " + << sshConfigOptions.host; + destinationHost = string(sshConfigOptions.host); + } + free(home_dir); + } // Parse username: cmdline > sshconfig > localuser if (username.empty()) { if (sshConfigOptions.username) { username = string(sshConfigOptions.username); } else { - username = string(ssh_get_local_username()); + char* usernamePtr = ssh_get_local_username(); + username = string(usernamePtr); + SAFE_FREE(usernamePtr); } } @@ -293,7 +303,11 @@ int main(int argc, char** argv) { exit(1); } - int jport = result["jport"].as(); + string jServerFifo = ""; + if (result["jserverfifo"].as() != "") { + jServerFifo = result["jserverfifo"].as(); + } + string serverFifo = ""; if (result["serverfifo"].as() != "") { serverFifo = result["serverfifo"].as(); @@ -310,9 +324,9 @@ int main(int argc, char** argv) { etterminal_path = result["terminal-path"].as(); } string idpasskeypair = SshSetupHandler::SetupSsh( - username, destinationHost, host_alias, destinationPort, jumphost, jport, - result.count("x") > 0, result["verbose"].as(), etterminal_path, - serverFifo, ssh_options); + username, destinationHost, host_alias, destinationPort, jumphost, + jServerFifo, result.count("x") > 0, result["verbose"].as(), + etterminal_path, serverFifo, ssh_options); string id = "", passkey = ""; // Trim whitespace @@ -364,6 +378,17 @@ int main(int argc, char** argv) { handleParseException(oe, options); } + // Clean up ssh config options + SAFE_FREE(sshConfigOptions.username); + SAFE_FREE(sshConfigOptions.host); + SAFE_FREE(sshConfigOptions.sshdir); + SAFE_FREE(sshConfigOptions.knownhosts); + SAFE_FREE(sshConfigOptions.ProxyCommand); + SAFE_FREE(sshConfigOptions.ProxyJump); + SAFE_FREE(sshConfigOptions.gss_server_identity); + SAFE_FREE(sshConfigOptions.gss_client_identity); + SAFE_FREE(sshConfigOptions.identity_agent); + #ifdef WIN32 WSACleanup(); #endif diff --git a/test/ConnectionTest.cpp b/test/ConnectionTest.cpp index 960920598..8823d4a6a 100644 --- a/test/ConnectionTest.cpp +++ b/test/ConnectionTest.cpp @@ -140,8 +140,8 @@ class TestServerConnection : public ServerConnection { public: TestServerConnection(shared_ptr _socketHandler, SocketEndpoint socketEndpoint) - : ServerConnection(_socketHandler, socketEndpoint){}; - virtual ~TestServerConnection(){}; + : ServerConnection(_socketHandler, socketEndpoint) {} + virtual ~TestServerConnection() {} virtual bool newClient( shared_ptr _serverClientState) { string clientId = _serverClientState->getId(); diff --git a/test/FakeConsole.hpp b/test/FakeConsole.hpp index 6a2323974..f85679c48 100644 --- a/test/FakeConsole.hpp +++ b/test/FakeConsole.hpp @@ -158,7 +158,7 @@ class FakeUserTerminal : public UserTerminal { return getFd(); }; - virtual void runTerminal(){ + virtual void runTerminal() { }; diff --git a/test/system_tests/connect_with_jumphost.sh b/test/system_tests/connect_with_jumphost.sh new file mode 100755 index 000000000..f448df96e --- /dev/null +++ b/test/system_tests/connect_with_jumphost.sh @@ -0,0 +1,27 @@ +#!/bin/bash +set -x +set -e + +ssh -o 'PreferredAuthentications=publickey' localhost "echo" || exit 1 # Fails if we can't ssh into localhost without a password + +# Bypass host check +ssh -o "StrictHostKeyChecking no" localhost echo "Bypassing host check 1" +ssh -o "StrictHostKeyChecking no" 127.0.0.1 echo "Bypassing host check 2" + +mkdir -p /tmp/et_test_logs/connect_with_jumphost/1 +build/etserver --port 9900 --serverfifo=/tmp/etserver.idpasskey.fifo1 -l /tmp/et_test_logs/connect_with_jumphost/1 & +first_server_pid=$! + +mkdir -p /tmp/et_test_logs/connect_with_jumphost/2 +build/etserver --port 9901 --serverfifo=/tmp/etserver.idpasskey.fifo2 -l /tmp/et_test_logs/connect_with_jumphost/2 & +second_server_pid=$! +sleep 3 + +# Make sure servers are working +build/et -c "echo 'Hello World 1!'" --serverfifo=/tmp/etserver.idpasskey.fifo1 --logtostdout --terminal-path $PWD/build/etterminal localhost:9900 +build/et -c "echo 'Hello World 2!'" --serverfifo=/tmp/etserver.idpasskey.fifo2 --logtostdout --terminal-path $PWD/build/etterminal localhost:9901 + +build/et -c "echo 'Hello World 3!'" --serverfifo=/tmp/etserver.idpasskey.fifo2 --logtostdout --terminal-path $PWD/build/etterminal --jumphost localhost --jport 9900 --jserverfifo=/tmp/etserver.idpasskey.fifo1 127.0.0.1:9901 # We can't use 'localhost' for both the jumphost and the destination because ssh doesn't support keeping them the same. + +kill -9 $first_server_pid +kill -9 $second_server_pid