From 5a36b315a3a66b827bfede27b98f414047ac14da Mon Sep 17 00:00:00 2001 From: Grant Limberg Date: Tue, 18 Jul 2023 14:10:31 -0400 Subject: [PATCH] Exit if loading an invalid identity from disk (#2058) * Exit if loading an invalid identity from disk Previously, if an invalid identity was loaded from disk, ZeroTier would generate a new identity & chug along and generate a brand new identity as if nothing happened. When running in containers, this introduces the possibility for key matter loss; especially when running in containers where the identity files are mounted in the container read only. In this case, ZT will continue chugging along with a brand new identity with no possibility of recovering the private key. ZeroTier should exit upon loading of invalid identity.public/identity.secret #2056 * add validation test for #2056 --- .github/workflows/report.sh | 6 +++ .github/workflows/validate-1m-linux.sh | 32 ++++++++++++++- node/Constants.hpp | 1 + node/Node.cpp | 6 ++- service/OneService.cpp | 55 ++++++++++++++++++++++++-- 5 files changed, 95 insertions(+), 5 deletions(-) diff --git a/.github/workflows/report.sh b/.github/workflows/report.sh index cc6716213..c79139544 100755 --- a/.github/workflows/report.sh +++ b/.github/workflows/report.sh @@ -13,3 +13,9 @@ echo -e "\nBytes of memory definitely lost: $DEFINITELY_LOST" if [[ "$DEFINITELY_LOST" -gt 0 ]]; then exit 1 fi + +EXIT_TEST_FAILED=$(cat *test-results/*summary.json | jq .exit_test_failed) + +if [[ "$EXIT_TEST_FAILED" -gt 0 ]]; then + exit 1 +fi diff --git a/.github/workflows/validate-1m-linux.sh b/.github/workflows/validate-1m-linux.sh index 3d852a8d7..25d6bd473 100755 --- a/.github/workflows/validate-1m-linux.sh +++ b/.github/workflows/validate-1m-linux.sh @@ -9,6 +9,8 @@ ZTO_VER=$(git describe --tags $(git rev-list --tags --max-count=1)) ZTO_COMMIT=$(git rev-parse HEAD) ZTO_COMMIT_SHORT=$(git rev-parse --short HEAD) TEST_DIR_PREFIX="$ZTO_VER-$ZTO_COMMIT_SHORT-test-results" +EXIT_TEST_FAILED=0 + echo "Performing test on: $ZTO_VER-$ZTO_COMMIT_SHORT" TEST_FILEPATH_PREFIX="$TEST_DIR_PREFIX/$ZTO_COMMIT_SHORT" mkdir $TEST_DIR_PREFIX @@ -18,6 +20,9 @@ mkdir $TEST_DIR_PREFIX ################################################################################ main() { echo -e "\nRunning test for $RUN_LENGTH seconds" + + check_exit_on_invalid_identity + NS1="ip netns exec ns1" NS2="ip netns exec ns2" @@ -390,7 +395,8 @@ main() { "mean_latency_ping_netns": $POSSIBLY_LOST, "mean_pdv_random": $POSSIBLY_LOST, "mean_pdv_netns": $POSSIBLY_LOST, - "mean_perf_netns": $POSSIBLY_LOST + "mean_perf_netns": $POSSIBLY_LOST, + "exit_test_failed": $EXIT_TEST_FAILED } EOF ) @@ -431,4 +437,28 @@ spam_cli() { done } +check_exit_on_invalid_identity() { + echo "Checking ZeroTier exits on invalid identity..." + mkdir -p $(pwd)/exit_test + ZT1="sudo ./zerotier-one -p9999 $(pwd)/exit_test" + echo "asdfasdfasdfasdf" > $(pwd)/exit_test/identity.secret + echo "asdfasdfasdfasdf" > $(pwd)/exit_test/authtoken.secret + + echo "Launch ZeroTier with an invalid identity" + $ZT1 & + my_pid=$! + + echo "Waiting 5 secons" + sleep 5 + + # check if process is running + kill -0 $my_pid + if [ $? -eq 0 ]; then + EXIT_TEST_FAILED=1 + echo "Exit test FAILED: Process still running after being fed an invalid identity" + else + echo "Exit test PASSED" + fi +} + main "$@" diff --git a/node/Constants.hpp b/node/Constants.hpp index ba3026751..32492293a 100644 --- a/node/Constants.hpp +++ b/node/Constants.hpp @@ -687,6 +687,7 @@ #define ZT_EXCEPTION_OUT_OF_MEMORY 101 #define ZT_EXCEPTION_PRIVATE_KEY_REQUIRED 102 #define ZT_EXCEPTION_INVALID_ARGUMENT 103 +#define ZT_EXCEPTION_INVALID_IDENTITY 104 #define ZT_EXCEPTION_INVALID_SERIALIZED_DATA_INVALID_TYPE 200 #define ZT_EXCEPTION_INVALID_SERIALIZED_DATA_OVERFLOW 201 #define ZT_EXCEPTION_INVALID_SERIALIZED_DATA_INVALID_CRYPTOGRAPHIC_TOKEN 202 diff --git a/node/Node.cpp b/node/Node.cpp index 8da39700c..9b748c6d0 100644 --- a/node/Node.cpp +++ b/node/Node.cpp @@ -80,7 +80,11 @@ Node::Node(void *uptr,void *tptr,const struct ZT_Node_Callbacks *callbacks,int64 RR->identity.toString(false,RR->publicIdentityStr); RR->identity.toString(true,RR->secretIdentityStr); } else { - n = -1; + throw ZT_EXCEPTION_INVALID_IDENTITY; + } + + if (!RR->identity.locallyValidate()) { + throw ZT_EXCEPTION_INVALID_IDENTITY; } } diff --git a/service/OneService.cpp b/service/OneService.cpp index bd82ac568..edaca9c8c 100644 --- a/service/OneService.cpp +++ b/service/OneService.cpp @@ -786,6 +786,7 @@ public: httplib::Server _controlPlane; std::thread _serverThread; + bool _serverThreadRunning; bool _allowTcpFallbackRelay; bool _forceTcpRelay; @@ -887,6 +888,7 @@ public: ,_updateAutoApply(false) ,_controlPlane() ,_serverThread() + ,_serverThreadRunning(false) ,_forceTcpRelay(false) ,_primaryPort(port) ,_udpPortPickerCounter(0) @@ -938,8 +940,9 @@ public: #endif _controlPlane.stop(); - _serverThread.join(); - + if (_serverThreadRunning) { + _serverThread.join(); + } #ifdef ZT_USE_MINIUPNPC delete _portMapper; @@ -1005,7 +1008,6 @@ public: _node = new Node(this,(void *)0,&cb,OSUtils::now()); } - // local.conf readLocalSettings(); applyLocalConfig(); @@ -1260,6 +1262,51 @@ public: Mutex::Lock _l(_termReason_m); _termReason = ONE_UNRECOVERABLE_ERROR; _fatalErrorMessage = std::string("unexpected exception in main thread: ")+e.what(); + } catch (int e) { + Mutex::Lock _l(_termReason_m); + _termReason = ONE_UNRECOVERABLE_ERROR; + switch (e) { + case ZT_EXCEPTION_OUT_OF_BOUNDS: { + _fatalErrorMessage = "out of bounds exception"; + break; + } + case ZT_EXCEPTION_OUT_OF_MEMORY: { + _fatalErrorMessage = "out of memory"; + break; + } + case ZT_EXCEPTION_PRIVATE_KEY_REQUIRED: { + _fatalErrorMessage = "private key required"; + break; + } + case ZT_EXCEPTION_INVALID_ARGUMENT: { + _fatalErrorMessage = "invalid argument"; + break; + } + case ZT_EXCEPTION_INVALID_IDENTITY: { + _fatalErrorMessage = "invalid identity loaded from disk. Please remove identity.public and identity.secret from " + _homePath + " and try again"; + break; + } + case ZT_EXCEPTION_INVALID_SERIALIZED_DATA_INVALID_TYPE: { + _fatalErrorMessage = "invalid serialized data: invalid type"; + break; + } + case ZT_EXCEPTION_INVALID_SERIALIZED_DATA_OVERFLOW: { + _fatalErrorMessage = "invalid serialized data: overflow"; + break; + } + case ZT_EXCEPTION_INVALID_SERIALIZED_DATA_INVALID_CRYPTOGRAPHIC_TOKEN: { + _fatalErrorMessage = "invalid serialized data: invalid cryptographic token"; + break; + } + case ZT_EXCEPTION_INVALID_SERIALIZED_DATA_BAD_ENCODING: { + _fatalErrorMessage = "invalid serialized data: bad encoding"; + break; + } + default: { + _fatalErrorMessage = "unexpected exception code: " + std::to_string(e); + break; + } + } } catch ( ... ) { Mutex::Lock _l(_termReason_m); _termReason = ONE_UNRECOVERABLE_ERROR; @@ -2077,11 +2124,13 @@ public: } _serverThread = std::thread([&] { + _serverThreadRunning = true; fprintf(stderr, "Starting Control Plane...\n"); if(!_controlPlane.listen_after_bind()) { fprintf(stderr, "Error on listen_after_bind()\n"); } fprintf(stderr, "Control Plane Stopped\n"); + _serverThreadRunning = false; }); }