Fix fb9d4af: use different nonces for key exchange and stream encryption

src/network/core/tcp_game.h

@@ -243,6 +243,8 @@ class NetworkGameSocketHandler : public NetworkTCPSocketHandler {
 
 	/**
 	 * Indication to the client that authentication is complete and encryption has to be used from here on forward.
+	 * The encryption uses the shared keys generated by the last AUTH_REQUEST key exchange.
+	 * 24 * uint8_t Nonce for encrypted connection.
 	 * @param p The packet that was just received.
 	 */
 	virtual NetworkRecvStatus Receive_SERVER_ENABLE_ENCRYPTION(Packet &p);

src/network/network_client.cpp

@@ -775,12 +775,14 @@ NetworkRecvStatus ClientNetworkGameSocketHandler::Receive_SERVER_AUTH_REQUEST(Pa
 	}
 }
 
-NetworkRecvStatus ClientNetworkGameSocketHandler::Receive_SERVER_ENABLE_ENCRYPTION(Packet &)
+NetworkRecvStatus ClientNetworkGameSocketHandler::Receive_SERVER_ENABLE_ENCRYPTION(Packet &p)
 {
 	if (this->status != STATUS_AUTH_GAME || this->authentication_handler == nullptr) return NETWORK_RECV_STATUS_MALFORMED_PACKET;
 
 	Debug(net, 9, "Client::Receive_SERVER_ENABLE_ENCRYPTION()");
 
+	if (!this->authentication_handler->ReceiveEnableEncryption(p)) return NETWORK_RECV_STATUS_MALFORMED_PACKET;
+
 	this->receive_encryption_handler = this->authentication_handler->CreateServerToClientEncryptionHandler();
 	this->send_encryption_handler = this->authentication_handler->CreateClientToServerEncryptionHandler();
 	this->authentication_handler = nullptr;

src/network/network_crypto.cpp

@@ -182,14 +182,15 @@ X25519Nonce::~X25519Nonce()
  * @param secret_key The secret key to use for this handler. Defaults to secure random data.
  */
 X25519AuthenticationHandler::X25519AuthenticationHandler(const X25519SecretKey &secret_key) :
-	our_secret_key(secret_key), our_public_key(secret_key.CreatePublicKey()), nonce(X25519Nonce::CreateRandom())
+	our_secret_key(secret_key), our_public_key(secret_key.CreatePublicKey()),
+	key_exchange_nonce(X25519Nonce::CreateRandom()), encryption_nonce(X25519Nonce::CreateRandom())
 {
 }
 
 /* virtual */ void X25519AuthenticationHandler::SendRequest(Packet &p)
 {
 	p.Send_bytes(this->our_public_key);
-	p.Send_bytes(this->nonce);
+	p.Send_bytes(this->key_exchange_nonce);
 }
 
 /**
@@ -205,7 +206,7 @@ bool X25519AuthenticationHandler::ReceiveRequest(Packet &p)
 	}
 
 	p.Recv_bytes(this->peer_public_key);
-	p.Recv_bytes(this->nonce);
+	p.Recv_bytes(this->key_exchange_nonce);
 	return true;
 }
 
@@ -227,7 +228,7 @@ bool X25519AuthenticationHandler::SendResponse(Packet &p, std::string_view deriv
 	RandomBytesWithFallback(message);
 	X25519Mac mac;
 
-	crypto_aead_lock(message.data(), mac.data(), this->derived_keys.ClientToServer().data(), nonce.data(),
+	crypto_aead_lock(message.data(), mac.data(), this->derived_keys.ClientToServer().data(), this->key_exchange_nonce.data(),
 			this->our_public_key.data(), this->our_public_key.size(), message.data(), message.size());
 
 	p.Send_bytes(this->our_public_key);
@@ -245,14 +246,33 @@ std::string X25519AuthenticationHandler::GetPeerPublicKey() const
 	return FormatArrayAsHex(this->peer_public_key);
 }
 
+/**
+ * Send the initial nonce for the encrypted connection.
+ * @param p The packet to send the data in.
+ */
+void X25519AuthenticationHandler::SendEnableEncryption(struct Packet &p) const
+{
+	p.Send_bytes(this->encryption_nonce);
+}
+
+/**
+ * Receive the initial nonce for the encrypted connection.
+ * @param p The packet to read the data from.
+ * @return \c true when enough bytes could be read for the nonce, otherwise \c false.
+ */
+bool X25519AuthenticationHandler::ReceiveEnableEncryption(struct Packet &p)
+{
+	return p.Recv_bytes(this->encryption_nonce) == this->encryption_nonce.size();
+}
+
 std::unique_ptr<NetworkEncryptionHandler> X25519AuthenticationHandler::CreateClientToServerEncryptionHandler() const
 {
-	return std::make_unique<X25519EncryptionHandler>(this->derived_keys.ClientToServer(), this->nonce);
+	return std::make_unique<X25519EncryptionHandler>(this->derived_keys.ClientToServer(), this->encryption_nonce);
 }
 
 std::unique_ptr<NetworkEncryptionHandler> X25519AuthenticationHandler::CreateServerToClientEncryptionHandler() const
 {
-	return std::make_unique<X25519EncryptionHandler>(this->derived_keys.ServerToClient(), this->nonce);
+	return std::make_unique<X25519EncryptionHandler>(this->derived_keys.ServerToClient(), this->encryption_nonce);
 }
 
 /**
@@ -282,7 +302,7 @@ NetworkAuthenticationServerHandler::ResponseResult X25519AuthenticationHandler::
 		return NetworkAuthenticationServerHandler::NOT_AUTHENTICATED;
 	}
 
-	if (crypto_aead_unlock(message.data(), mac.data(), this->derived_keys.ClientToServer().data(), nonce.data(),
+	if (crypto_aead_unlock(message.data(), mac.data(), this->derived_keys.ClientToServer().data(), this->key_exchange_nonce.data(),
 			this->peer_public_key.data(), this->peer_public_key.size(), message.data(), message.size()) != 0) {
 		/*
 		 * The ciphertext and the message authentication code do not match with the encryption key.

src/network/network_crypto.h

@@ -242,6 +242,12 @@ class NetworkAuthenticationClientHandler : public NetworkAuthenticationHandler {
 	 */
 	virtual bool SendResponse(struct Packet &p) = 0;
 
+	/**
+	 * Read the request to enable encryption from the server.
+	 * @param p The request from the server.
+	 */
+	virtual bool ReceiveEnableEncryption(struct Packet &p) = 0;
+
 	static std::unique_ptr<NetworkAuthenticationClientHandler> Create(std::shared_ptr<NetworkAuthenticationPasswordRequestHandler> password_handler, std::string &secret_key, std::string &public_key);
 };
 
@@ -270,6 +276,12 @@ class NetworkAuthenticationServerHandler : public NetworkAuthenticationHandler {
 	 */
 	virtual ResponseResult ReceiveResponse(struct Packet &p) = 0;
 
+	/**
+	 * Create the request to enable encryption to the client.
+	 * @param p The packet to write the enable encryption request to.
+	 */
+	virtual void SendEnableEncryption(struct Packet &p) = 0;
+
 	/**
 	 * Checks whether this handler can be used with the current configuration.
 	 * For example when there is no password, the handler cannot be used.

src/network/network_crypto_internal.h

@@ -105,10 +105,12 @@ class X25519AuthenticationHandler {
 private:
 	X25519SecretKey our_secret_key; ///< The secret key used by us.
 	X25519PublicKey our_public_key; ///< The public key used by us.
-	X25519Nonce nonce; ///< The nonce to prevent replay attacks.
+	X25519Nonce key_exchange_nonce; ///< The nonce to prevent replay attacks of the key exchange.
 	X25519DerivedKeys derived_keys; ///< Keys derived from the authentication process.
 	X25519PublicKey peer_public_key; ///< The public key used by our peer.
 
+	X25519Nonce encryption_nonce; ///< The nonce to prevent replay attacks the encrypted connection.
+
 protected:
 	X25519AuthenticationHandler(const X25519SecretKey &secret_key);
 
@@ -119,6 +121,8 @@ class X25519AuthenticationHandler {
 
 	std::string GetPeerPublicKey() const;
 
+	void SendEnableEncryption(struct Packet &p) const;
+	bool ReceiveEnableEncryption(struct Packet &p);
 	std::unique_ptr<NetworkEncryptionHandler> CreateClientToServerEncryptionHandler() const;
 	std::unique_ptr<NetworkEncryptionHandler> CreateServerToClientEncryptionHandler() const;
 };
@@ -142,6 +146,7 @@ class X25519KeyExchangeOnlyClientHandler : protected X25519AuthenticationHandler
 	virtual std::string_view GetName() const override { return "X25519-KeyExchangeOnly-client"; }
 	virtual NetworkAuthenticationMethod GetAuthenticationMethod() const override { return NETWORK_AUTH_METHOD_X25519_KEY_EXCHANGE_ONLY; }
 
+	virtual bool ReceiveEnableEncryption(struct Packet &p) override { return this->X25519AuthenticationHandler::ReceiveEnableEncryption(p); }
 	virtual std::unique_ptr<NetworkEncryptionHandler> CreateClientToServerEncryptionHandler() const override { return this->X25519AuthenticationHandler::CreateClientToServerEncryptionHandler(); }
 	virtual std::unique_ptr<NetworkEncryptionHandler> CreateServerToClientEncryptionHandler() const override { return this->X25519AuthenticationHandler::CreateServerToClientEncryptionHandler(); }
 };
@@ -167,6 +172,7 @@ class X25519KeyExchangeOnlyServerHandler : protected X25519AuthenticationHandler
 	virtual bool CanBeUsed() const override { return true; }
 
 	virtual std::string GetPeerPublicKey() const override { return this->X25519AuthenticationHandler::GetPeerPublicKey(); }
+	virtual void SendEnableEncryption(struct Packet &p) override { this->X25519AuthenticationHandler::SendEnableEncryption(p); }
 	virtual std::unique_ptr<NetworkEncryptionHandler> CreateClientToServerEncryptionHandler() const override { return this->X25519AuthenticationHandler::CreateClientToServerEncryptionHandler(); }
 	virtual std::unique_ptr<NetworkEncryptionHandler> CreateServerToClientEncryptionHandler() const override { return this->X25519AuthenticationHandler::CreateServerToClientEncryptionHandler(); }
 };
@@ -194,6 +200,7 @@ class X25519PAKEClientHandler : protected X25519AuthenticationHandler, public Ne
 	virtual std::string_view GetName() const override { return "X25519-PAKE-client"; }
 	virtual NetworkAuthenticationMethod GetAuthenticationMethod() const override { return NETWORK_AUTH_METHOD_X25519_PAKE; }
 
+	virtual bool ReceiveEnableEncryption(struct Packet &p) override { return this->X25519AuthenticationHandler::ReceiveEnableEncryption(p); }
 	virtual std::unique_ptr<NetworkEncryptionHandler> CreateClientToServerEncryptionHandler() const override { return this->X25519AuthenticationHandler::CreateClientToServerEncryptionHandler(); }
 	virtual std::unique_ptr<NetworkEncryptionHandler> CreateServerToClientEncryptionHandler() const override { return this->X25519AuthenticationHandler::CreateServerToClientEncryptionHandler(); }
 };
@@ -222,6 +229,7 @@ class X25519PAKEServerHandler : protected X25519AuthenticationHandler, public Ne
 	virtual bool CanBeUsed() const override { return !this->password_provider->GetPassword().empty(); }
 
 	virtual std::string GetPeerPublicKey() const override { return this->X25519AuthenticationHandler::GetPeerPublicKey(); }
+	virtual void SendEnableEncryption(struct Packet &p) override { this->X25519AuthenticationHandler::SendEnableEncryption(p); }
 	virtual std::unique_ptr<NetworkEncryptionHandler> CreateClientToServerEncryptionHandler() const override { return this->X25519AuthenticationHandler::CreateClientToServerEncryptionHandler(); }
 	virtual std::unique_ptr<NetworkEncryptionHandler> CreateServerToClientEncryptionHandler() const override { return this->X25519AuthenticationHandler::CreateServerToClientEncryptionHandler(); }
 };
@@ -247,6 +255,7 @@ class X25519AuthorizedKeyClientHandler : protected X25519AuthenticationHandler,
 	virtual std::string_view GetName() const override { return "X25519-AuthorizedKey-client"; }
 	virtual NetworkAuthenticationMethod GetAuthenticationMethod() const override { return NETWORK_AUTH_METHOD_X25519_AUTHORIZED_KEY; }
 
+	virtual bool ReceiveEnableEncryption(struct Packet &p) override { return this->X25519AuthenticationHandler::ReceiveEnableEncryption(p); }
 	virtual std::unique_ptr<NetworkEncryptionHandler> CreateClientToServerEncryptionHandler() const override { return this->X25519AuthenticationHandler::CreateClientToServerEncryptionHandler(); }
 	virtual std::unique_ptr<NetworkEncryptionHandler> CreateServerToClientEncryptionHandler() const override { return this->X25519AuthenticationHandler::CreateServerToClientEncryptionHandler(); }
 
@@ -278,6 +287,7 @@ class X25519AuthorizedKeyServerHandler : protected X25519AuthenticationHandler,
 	virtual bool CanBeUsed() const override { return this->authorized_key_handler->CanBeUsed(); }
 
 	virtual std::string GetPeerPublicKey() const override { return this->X25519AuthenticationHandler::GetPeerPublicKey(); }
+	virtual void SendEnableEncryption(struct Packet &p) override { this->X25519AuthenticationHandler::SendEnableEncryption(p); }
 	virtual std::unique_ptr<NetworkEncryptionHandler> CreateClientToServerEncryptionHandler() const override { return this->X25519AuthenticationHandler::CreateClientToServerEncryptionHandler(); }
 	virtual std::unique_ptr<NetworkEncryptionHandler> CreateServerToClientEncryptionHandler() const override { return this->X25519AuthenticationHandler::CreateServerToClientEncryptionHandler(); }
 };
@@ -308,6 +318,7 @@ class CombinedAuthenticationClientHandler : public NetworkAuthenticationClientHa
 	virtual std::string_view GetName() const override;
 	virtual NetworkAuthenticationMethod GetAuthenticationMethod() const override;
 
+	virtual bool ReceiveEnableEncryption(struct Packet &p) override { return this->current_handler->ReceiveEnableEncryption(p); }
 	virtual std::unique_ptr<NetworkEncryptionHandler> CreateClientToServerEncryptionHandler() const override { return this->current_handler->CreateClientToServerEncryptionHandler(); }
 	virtual std::unique_ptr<NetworkEncryptionHandler> CreateServerToClientEncryptionHandler() const override { return this->current_handler->CreateServerToClientEncryptionHandler(); }
 };
@@ -334,6 +345,7 @@ class CombinedAuthenticationServerHandler : public NetworkAuthenticationServerHa
 	virtual bool CanBeUsed() const override;
 
 	virtual std::string GetPeerPublicKey() const override { return this->handlers.back()->GetPeerPublicKey(); }
+	virtual void SendEnableEncryption(struct Packet &p) override { this->handlers.back()->SendEnableEncryption(p); }
 	virtual std::unique_ptr<NetworkEncryptionHandler> CreateClientToServerEncryptionHandler() const override { return this->handlers.back()->CreateClientToServerEncryptionHandler(); }
 	virtual std::unique_ptr<NetworkEncryptionHandler> CreateServerToClientEncryptionHandler() const override { return this->handlers.back()->CreateServerToClientEncryptionHandler(); }
 };

src/network/network_server.cpp

@@ -467,6 +467,7 @@ NetworkRecvStatus ServerNetworkGameSocketHandler::SendEnableEncryption()
 	if (this->status != STATUS_AUTH_GAME) return this->CloseConnection(NETWORK_RECV_STATUS_MALFORMED_PACKET);
 
 	auto p = std::make_unique<Packet>(this, PACKET_SERVER_ENABLE_ENCRYPTION);
+	this->authentication_handler->SendEnableEncryption(*p);
 	this->SendPacket(std::move(p));
 	return NETWORK_RECV_STATUS_OKAY;
 }

src/tests/test_network_crypto.cpp

@@ -249,6 +249,14 @@ TEST_CASE("Encryption handling")
 
 	TestAuthentication(server, client, NetworkAuthenticationServerHandler::AUTHENTICATED, NetworkAuthenticationClientHandler::READY_FOR_RESPONSE);
 
+	Packet packet(&mock_socket_handler, PacketType{});
+	server.SendEnableEncryption(packet);
+
+	bool valid;
+	std::tie(packet, valid) = CreatePacketForReading(packet, &mock_socket_handler);
+	CHECK(valid);
+	CHECK(client.ReceiveEnableEncryption(packet));
+
 	MockNetworkSocketHandler server_socket_handler(server.CreateClientToServerEncryptionHandler(), server.CreateServerToClientEncryptionHandler());
 	MockNetworkSocketHandler client_socket_handler(client.CreateServerToClientEncryptionHandler(), client.CreateClientToServerEncryptionHandler());