fix auth bypass

[koonpeng]

Bug fix

Fixed bug

Something I discovered while reviewing open-rmf/rmf_deployment_template#49. I'm 100% certain if this is an actual bypass in practice as I haven't done a POC, but in theory from what I understand from the code, certain requests can bypass authentication.

rmf_visualization/rmf_visualization_schedule/src/rmf_visualization_schedule/TrajectoryServer.cpp

Lines 121 to 145 in ef42bb4

	bool ok = parse_request(hdl, msg, response);
	// validate jwt only if public key is given (when running with dashboard)
	std::string public_key;
	std::string token;
	bool is_verified = true;
	if (std::getenv("JWT_PUBLIC_KEY"))
	{
	public_key = std::getenv("JWT_PUBLIC_KEY");
	try
	{
	token = Json::parse(msg->get_payload())["token"];
	auto decoded = jwt::decode(
	token,
	jwt::params::algorithms({"RS256"}),
	jwt::params::secret(public_key));
	}
	catch (std::exception& e)
	{
	is_verified = false;
	std::string err_excp = e.what();
	send_error_message(hdl, msg, err_response, server, err_excp);
	std::cerr << "Error: " << e.what() << std::endl;
	}
	}

Here the request is parsed first before auth is checked, in this case "parsing" the request includes processing it. For most requests this can leak information with a timing attack, but there is one operation in particular which is very problematic.

rmf_visualization/rmf_visualization_schedule/src/rmf_visualization_schedule/TrajectoryServer.cpp

Lines 233 to 241 in ef42bb4

	else if (j["request"] == "negotiation_update_subscribe")
	{
	negotiation_subscribed_connections.insert(hdl);
	Json j_res = Json();
	j_res["response"] = "negotiation_update_subscribe";
	j_res["result"] = true;
	response = j_res.dump();
	return true;
	}

The negotiation_update_subscribe request subscribes to negotiation updates and the server will push any new negotiations.

rmf_visualization/rmf_visualization_schedule/src/rmf_visualization_schedule/TrajectoryServer.cpp

Lines 445 to 473 in ef42bb4

	auto status_update_cb =
	[server_ptr](
	uint64_t conflict_version,
	rmf_traffic::schedule::Negotiation::Table::ViewerPtr table_view)
	{
	RCLCPP_DEBUG(server_ptr->_pimpl->schedule_data_node->get_logger(),
	"======== conflict callback version: %lu! ==========",
	conflict_version);
	nlohmann::json negotiation_json;
	negotiation_json["type"] = "negotiation_status";
	negotiation_json["conflict_version"] = conflict_version;
	negotiation_json["participant_id"] = table_view->participant_id();
	negotiation_json["participant_name"] =
	table_view->get_description(table_view->participant_id())->name();
	negotiation_json["defunct"] = table_view->defunct();
	negotiation_json["rejected"] = table_view->rejected();
	negotiation_json["forfeited"] = table_view->forfeited();
	auto versioned_sequence = table_view->sequence();
	for (auto versionedkey : versioned_sequence)
	negotiation_json["sequence"].push_back(versionedkey.participant);
	std::string conflict_str = negotiation_json.dump();
	for (auto connection :
	server_ptr->_pimpl->negotiation_subscribed_connections)
	server_ptr->_pimpl->server->send(
	connection, conflict_str, websocketpp::frame::opcode::text);
	};

Because the request is processed before auth, the subscription should still go through even with bad credentials and the server will start pushing updates.

Fix applied

Perform auth before processing any request, also close the connection if auth fails.