Browse Source

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
Grant Limberg 2 years ago
parent
commit
5a36b315a3

+ 6 - 0
.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

+ 31 - 1
.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 "$@"

+ 1 - 0
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

+ 5 - 1
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;
 		}
 	}
 

+ 52 - 3
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;
         });
 
     }