[remoting host] Send host error details to the client
Host errors are mapped to the error codes and sent to the client, but the error codes are not granular enough and the errors are often mapped to the catch-all error code of INCOMPATIBLE_PROTOCOL. This makes it difficult for us to trobleshoot host side issues and we often have to ask users to collect host logs. This CL makes the host send the error details as a free-form human-readable(ish) string to the client, so that it can be included in the client logs and shown to (corp) users. Bug: 382334458 Change-Id: I54aa2fe6037acb4c88d1f0304958c4fa3b4e0ae8 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6071968 Reviewed-by: Joe Downing <joedow@chromium.org> Auto-Submit: Yuwei Huang <yuweih@chromium.org> Commit-Queue: Yuwei Huang <yuweih@chromium.org> Cr-Commit-Position: refs/heads/main@{#1394585}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
28a727bd02
commit
07c271412f
remoting
host
protocol
BUILD.gnauthenticator.ccauthenticator.hfake_authenticator.ccfake_authenticator.hfake_session.ccfake_session.hjingle_messages.ccjingle_messages.hjingle_messages_unittest.ccjingle_session.ccjingle_session.hme2me_host_authenticator_factory.ccnegotiating_authenticator_base.ccnegotiating_authenticator_base.hnegotiating_client_authenticator.ccnegotiating_host_authenticator.ccpairing_authenticator_base.ccpairing_authenticator_base.hprotocol_mock_objects.hrejecting_authenticator.ccrejecting_authenticator.hsession.ccsession.hsession_authz_authenticator.ccsession_authz_authenticator.hsession_authz_authenticator_unittest.ccspake2_authenticator.ccspake2_authenticator.hvalidating_authenticator.ccvalidating_authenticator.hwebrtc_connection_to_client.cc
@ -415,7 +415,7 @@ TEST_F(ChromotingHostTest, IncomingSessionAccepted) {
|
||||
host_->OnIncomingSession(session_unowned1_.release(), &response);
|
||||
EXPECT_EQ(protocol::SessionManager::ACCEPT, response);
|
||||
|
||||
EXPECT_CALL(*session, Close(_))
|
||||
EXPECT_CALL(*session, Close(_, _, _))
|
||||
.WillOnce(InvokeWithoutArgs(
|
||||
this, &ChromotingHostTest::NotifyConnectionClosed1));
|
||||
ShutdownHost();
|
||||
@ -437,7 +437,7 @@ TEST_F(ChromotingHostTest, LoginBackOffTriggersIfClientsDoNotAuthenticate) {
|
||||
EXPECT_CALL(*session, SetEventHandler(_))
|
||||
.Times(AnyNumber())
|
||||
.WillRepeatedly(SaveArg<0>(&session_event_handlers[i]));
|
||||
EXPECT_CALL(*session, Close(_))
|
||||
EXPECT_CALL(*session, Close(_, _, _))
|
||||
.WillOnce(InvokeWithoutArgs([&session_event_handlers, i]() {
|
||||
session_event_handlers[i]->OnSessionStateChange(Session::CLOSED);
|
||||
}));
|
||||
@ -476,7 +476,7 @@ TEST_F(ChromotingHostTest, LoginBackOffResetsIfClientsAuthenticate) {
|
||||
EXPECT_CALL(*session, SetEventHandler(_))
|
||||
.Times(AnyNumber())
|
||||
.WillRepeatedly(SaveArg<0>(&session_event_handlers[i]));
|
||||
EXPECT_CALL(*session, Close(_))
|
||||
EXPECT_CALL(*session, Close(_, _, _))
|
||||
.WillOnce(InvokeWithoutArgs([&session_event_handlers, i]() {
|
||||
session_event_handlers[i]->OnSessionStateChange(Session::CLOSED);
|
||||
}));
|
||||
@ -503,7 +503,7 @@ TEST_F(ChromotingHostTest, LoginBackOffResetsIfClientsAuthenticate) {
|
||||
EXPECT_CALL(*session, SetEventHandler(_))
|
||||
.Times(AnyNumber())
|
||||
.WillRepeatedly(SaveArg<0>(&session_event_handler));
|
||||
EXPECT_CALL(*session, Close(_))
|
||||
EXPECT_CALL(*session, Close(_, _, _))
|
||||
.WillOnce(InvokeWithoutArgs([&session_event_handler]() {
|
||||
session_event_handler->OnSessionStateChange(Session::CLOSED);
|
||||
}));
|
||||
|
@ -35,6 +35,7 @@ class PamAuthorizer : public protocol::Authenticator {
|
||||
State state() const override;
|
||||
bool started() const override;
|
||||
RejectionReason rejection_reason() const override;
|
||||
RejectionDetails rejection_details() const override;
|
||||
void ProcessMessage(const jingle_xmpp::XmlElement* message,
|
||||
base::OnceClosure resume_callback) override;
|
||||
std::unique_ptr<jingle_xmpp::XmlElement> GetNextMessage() override;
|
||||
@ -97,6 +98,14 @@ protocol::Authenticator::RejectionReason PamAuthorizer::rejection_reason()
|
||||
}
|
||||
}
|
||||
|
||||
protocol::Authenticator::RejectionDetails PamAuthorizer::rejection_details()
|
||||
const {
|
||||
if (local_login_status_ == DISALLOWED) {
|
||||
return RejectionDetails("Local login check failed.");
|
||||
}
|
||||
return underlying_->rejection_details();
|
||||
}
|
||||
|
||||
void PamAuthorizer::ProcessMessage(const jingle_xmpp::XmlElement* message,
|
||||
base::OnceClosure resume_callback) {
|
||||
// |underlying_| is owned, so Unretained() is safe here.
|
||||
|
@ -179,6 +179,7 @@ static_library("protocol") {
|
||||
"sdp_message.h",
|
||||
"secure_channel_factory.cc",
|
||||
"secure_channel_factory.h",
|
||||
"session.cc",
|
||||
"session.h",
|
||||
"session_authz_authenticator.cc",
|
||||
"session_authz_authenticator.h",
|
||||
@ -375,6 +376,7 @@ static_library("test_support") {
|
||||
"//net/traffic_annotation:test_support",
|
||||
"//net/traffic_annotation:traffic_annotation",
|
||||
"//remoting/base:base",
|
||||
"//remoting/base:errors",
|
||||
"//remoting/base:session_policies",
|
||||
"//remoting/codec:encoder",
|
||||
"//remoting/signaling:signaling",
|
||||
|
@ -16,6 +16,23 @@ const jingle_xmpp::StaticQName kAuthenticationQName = {kChromotingXmlNamespace,
|
||||
"authentication"};
|
||||
} // namespace
|
||||
|
||||
Authenticator::RejectionDetails::RejectionDetails() = default;
|
||||
Authenticator::RejectionDetails::RejectionDetails(RejectionDetails&&) = default;
|
||||
Authenticator::RejectionDetails::RejectionDetails(const RejectionDetails&) =
|
||||
default;
|
||||
|
||||
Authenticator::RejectionDetails::RejectionDetails(
|
||||
std::string_view message,
|
||||
const base::Location& location)
|
||||
: message(std::string(message)), location(location) {}
|
||||
|
||||
Authenticator::RejectionDetails::~RejectionDetails() = default;
|
||||
|
||||
Authenticator::RejectionDetails& Authenticator::RejectionDetails::operator=(
|
||||
RejectionDetails&&) = default;
|
||||
Authenticator::RejectionDetails& Authenticator::RejectionDetails::operator=(
|
||||
const RejectionDetails&) = default;
|
||||
|
||||
Authenticator::Authenticator() = default;
|
||||
Authenticator::~Authenticator() = default;
|
||||
|
||||
|
@ -7,8 +7,10 @@
|
||||
|
||||
#include <memory>
|
||||
#include <string>
|
||||
#include <string_view>
|
||||
|
||||
#include "base/functional/callback.h"
|
||||
#include "base/location.h"
|
||||
#include "remoting/base/session_policies.h"
|
||||
#include "remoting/protocol/credentials_type.h"
|
||||
|
||||
@ -101,6 +103,39 @@ class Authenticator {
|
||||
NO_COMMON_AUTH_METHOD,
|
||||
};
|
||||
|
||||
// Details explaining why authentication was rejected.
|
||||
struct RejectionDetails {
|
||||
RejectionDetails();
|
||||
RejectionDetails(RejectionDetails&&);
|
||||
RejectionDetails(const RejectionDetails&);
|
||||
|
||||
// Creates a RejectionDetails object with the message and the location. If
|
||||
// |location| is omitted, the current location where the object is
|
||||
// constructed will be used.
|
||||
explicit RejectionDetails(
|
||||
std::string_view message,
|
||||
// Current() takes location info with default parameters, which is
|
||||
// filled when this constructor is called.
|
||||
const base::Location& location = base::Location::Current());
|
||||
~RejectionDetails();
|
||||
|
||||
RejectionDetails& operator=(RejectionDetails&&);
|
||||
RejectionDetails& operator=(const RejectionDetails&);
|
||||
|
||||
// Returns whether the RejectionDetails is null, i.e. there is no location
|
||||
// info and no rejection message.
|
||||
inline bool is_null() const {
|
||||
return location.program_counter() == nullptr && message.empty();
|
||||
}
|
||||
|
||||
// A free-form human-readable string that describes the reason for the
|
||||
// rejection.
|
||||
std::string message;
|
||||
|
||||
// Denotes where the error occurs in the code.
|
||||
base::Location location;
|
||||
};
|
||||
|
||||
// Callback used for layered Authenticator implementations, particularly
|
||||
// third-party and pairing authenticators. They use this callback to create
|
||||
// base SPAKE2 authenticators.
|
||||
@ -146,6 +181,10 @@ class Authenticator {
|
||||
// Returns rejection reason. Can be called only when in REJECTED state.
|
||||
virtual RejectionReason rejection_reason() const = 0;
|
||||
|
||||
// Returns the rejection details, or null if no details are available.
|
||||
// Can be called only when in REJECTED state.
|
||||
virtual RejectionDetails rejection_details() const = 0;
|
||||
|
||||
// Called in response to incoming message received from the peer.
|
||||
// Should only be called when in WAITING_MESSAGE state. Caller retains
|
||||
// ownership of |message|. |resume_callback| will be called when processing is
|
||||
|
@ -170,6 +170,11 @@ Authenticator::RejectionReason FakeAuthenticator::rejection_reason() const {
|
||||
return RejectionReason::INVALID_CREDENTIALS;
|
||||
}
|
||||
|
||||
Authenticator::RejectionDetails FakeAuthenticator::rejection_details() const {
|
||||
EXPECT_EQ(REJECTED, state());
|
||||
return {};
|
||||
}
|
||||
|
||||
void FakeAuthenticator::ProcessMessage(const jingle_xmpp::XmlElement* message,
|
||||
base::OnceClosure resume_callback) {
|
||||
EXPECT_EQ(WAITING_MESSAGE, state());
|
||||
|
@ -103,6 +103,7 @@ class FakeAuthenticator : public Authenticator {
|
||||
State state() const override;
|
||||
bool started() const override;
|
||||
RejectionReason rejection_reason() const override;
|
||||
RejectionDetails rejection_details() const override;
|
||||
void ProcessMessage(const jingle_xmpp::XmlElement* message,
|
||||
base::OnceClosure resume_callback) override;
|
||||
std::unique_ptr<jingle_xmpp::XmlElement> GetNextMessage() override;
|
||||
|
@ -79,7 +79,9 @@ void FakeSession::SetTransport(Transport* transport) {
|
||||
transport_ = transport;
|
||||
}
|
||||
|
||||
void FakeSession::Close(ErrorCode error) {
|
||||
void FakeSession::Close(ErrorCode error,
|
||||
std::string_view error_details,
|
||||
const base::Location& error_location) {
|
||||
closed_ = true;
|
||||
error_ = error;
|
||||
event_handler_->OnSessionStateChange(CLOSED);
|
||||
@ -90,10 +92,18 @@ void FakeSession::Close(ErrorCode error) {
|
||||
peer_.reset();
|
||||
|
||||
if (signaling_delay_.is_zero()) {
|
||||
peer->Close(error);
|
||||
peer->Close(error, error_details, error_location);
|
||||
} else {
|
||||
base::SingleThreadTaskRunner::GetCurrentDefault()->PostDelayedTask(
|
||||
FROM_HERE, base::BindOnce(&FakeSession::Close, peer, error),
|
||||
FROM_HERE,
|
||||
// Cannot just bind `error_details` as a string view, since the
|
||||
// underlying data could be invalidated before the callback is run.
|
||||
base::BindOnce(
|
||||
[](base::WeakPtr<FakeSession> peer, ErrorCode error,
|
||||
std::string error_details, base::Location error_location) {
|
||||
peer->Close(error, error_details, error_location);
|
||||
},
|
||||
peer, error, std::string(error_details), error_location),
|
||||
signaling_delay_);
|
||||
}
|
||||
}
|
||||
|
@ -8,8 +8,10 @@
|
||||
#include <map>
|
||||
#include <memory>
|
||||
#include <string>
|
||||
#include <string_view>
|
||||
#include <vector>
|
||||
|
||||
#include "base/location.h"
|
||||
#include "base/memory/raw_ptr.h"
|
||||
#include "base/memory/weak_ptr.h"
|
||||
#include "base/time/time.h"
|
||||
@ -26,6 +28,8 @@ class FakeAuthenticator;
|
||||
|
||||
class FakeSession : public Session {
|
||||
public:
|
||||
using Session::Close;
|
||||
|
||||
FakeSession();
|
||||
|
||||
FakeSession(const FakeSession&) = delete;
|
||||
@ -56,7 +60,9 @@ class FakeSession : public Session {
|
||||
const SessionConfig& config() override;
|
||||
const Authenticator& authenticator() const override;
|
||||
void SetTransport(Transport* transport) override;
|
||||
void Close(ErrorCode error) override;
|
||||
void Close(ErrorCode error,
|
||||
std::string_view error_details,
|
||||
const base::Location& error_location) override;
|
||||
void AddPlugin(SessionPlugin* plugin) override;
|
||||
|
||||
private:
|
||||
|
@ -292,6 +292,18 @@ bool JingleMessage::ParseXml(const jingle_xmpp::XmlElement* stanza,
|
||||
}
|
||||
}
|
||||
|
||||
const XmlElement* error_details_tag =
|
||||
jingle_tag->FirstNamed(QName(kChromotingXmlNamespace, "error-details"));
|
||||
if (error_details_tag) {
|
||||
error_details = error_details_tag->BodyText();
|
||||
}
|
||||
|
||||
const XmlElement* error_location_tag =
|
||||
jingle_tag->FirstNamed(QName(kChromotingXmlNamespace, "error-location"));
|
||||
if (error_location_tag) {
|
||||
error_location = error_location_tag->BodyText();
|
||||
}
|
||||
|
||||
if (action == SESSION_TERMINATE) {
|
||||
return true;
|
||||
}
|
||||
@ -395,6 +407,20 @@ std::unique_ptr<jingle_xmpp::XmlElement> JingleMessage::ToXml() const {
|
||||
jingle_tag->AddElement(error_code_tag);
|
||||
error_code_tag->SetBodyText(ErrorCodeToString(error_code));
|
||||
}
|
||||
|
||||
if (!error_details.empty()) {
|
||||
XmlElement* error_details_tag =
|
||||
new XmlElement(QName(kChromotingXmlNamespace, "error-details"));
|
||||
jingle_tag->AddElement(error_details_tag);
|
||||
error_details_tag->SetBodyText(error_details);
|
||||
}
|
||||
|
||||
if (!error_location.empty()) {
|
||||
XmlElement* error_location_tag =
|
||||
new XmlElement(QName(kChromotingXmlNamespace, "error-location"));
|
||||
jingle_tag->AddElement(error_location_tag);
|
||||
error_location_tag->SetBodyText(error_location);
|
||||
}
|
||||
}
|
||||
|
||||
if (action != SESSION_TERMINATE) {
|
||||
|
@ -87,6 +87,16 @@ struct JingleMessage {
|
||||
// message. Useful mainly for session-terminate messages. If it's UNKNOWN,
|
||||
// or reason is UNKNOWN_REASON, this field will be ignored in the xml output.
|
||||
ErrorCode error_code = ErrorCode::UNKNOWN_ERROR;
|
||||
|
||||
// Value from the <google:remoting:error-details> tag if it is present in the
|
||||
// message. Useful mainly for session-terminate messages. If it's empty, or
|
||||
// reason is UNKNOWN_REASON, this field will be ignored in the xml output.
|
||||
std::string error_details;
|
||||
|
||||
// Value from the <google:remoting:error-location> tag if it is present in the
|
||||
// message. Useful mainly for session-terminate messages. If it's empty, or
|
||||
// reason is UNKNOWN_REASON, this field will be ignored in the xml output.
|
||||
std::string error_location;
|
||||
};
|
||||
|
||||
struct JingleMessageReply {
|
||||
|
@ -574,6 +574,41 @@ TEST(JingleMessageTest, RemotingErrorCode) {
|
||||
}
|
||||
}
|
||||
|
||||
TEST(JingleMessageTest, ErrorDetails) {
|
||||
static constexpr char kTestSessionTerminateMessage[] =
|
||||
"<cli:iq from='user@gmail.com/chromoting016DBB07' "
|
||||
"to='user@gmail.com/chromiumsy5C6A652D' type='set' "
|
||||
"xmlns:cli='jabber:client'><jingle action='session-terminate' "
|
||||
"sid='2227053353' xmlns='urn:xmpp:jingle:1'><reason><decline/></reason>"
|
||||
"<gr:error-details xmlns:gr='google:remoting'>"
|
||||
"These are the error details."
|
||||
"</gr:error-details>"
|
||||
"</jingle></cli:iq>";
|
||||
|
||||
JingleMessage message;
|
||||
ParseFormatAndCompare(kTestSessionTerminateMessage, &message);
|
||||
|
||||
EXPECT_EQ(message.error_details, "These are the error details.");
|
||||
}
|
||||
|
||||
TEST(JingleMessageTest, ErrorLocation) {
|
||||
static constexpr char kTestSessionTerminateMessage[] =
|
||||
"<cli:iq from='user@gmail.com/chromoting016DBB07' "
|
||||
"to='user@gmail.com/chromiumsy5C6A652D' type='set' "
|
||||
"xmlns:cli='jabber:client'><jingle action='session-terminate' "
|
||||
"sid='2227053353' xmlns='urn:xmpp:jingle:1'><reason><decline/></reason>"
|
||||
"<gr:error-location xmlns:gr='google:remoting'>"
|
||||
"OnAuthenticated@remoting/protocol/jingle_session.cc:836"
|
||||
"</gr:error-location>"
|
||||
"</jingle></cli:iq>";
|
||||
|
||||
JingleMessage message;
|
||||
ParseFormatAndCompare(kTestSessionTerminateMessage, &message);
|
||||
|
||||
EXPECT_EQ(message.error_location,
|
||||
"OnAuthenticated@remoting/protocol/jingle_session.cc:836");
|
||||
}
|
||||
|
||||
TEST(JingleMessageTest, AttachmentsMessage) {
|
||||
// Ordering of the "attachments" tag and other tags are irrelevant. But the
|
||||
// JingleMessage implementation always puts it before other tags, so we do the
|
||||
|
@ -8,14 +8,18 @@
|
||||
|
||||
#include <limits>
|
||||
#include <memory>
|
||||
#include <string>
|
||||
#include <string_view>
|
||||
#include <utility>
|
||||
|
||||
#include "base/functional/bind.h"
|
||||
#include "base/functional/callback.h"
|
||||
#include "base/location.h"
|
||||
#include "base/logging.h"
|
||||
#include "base/notreached.h"
|
||||
#include "base/ranges/algorithm.h"
|
||||
#include "base/strings/string_split.h"
|
||||
#include "base/strings/stringprintf.h"
|
||||
#include "base/task/single_thread_task_runner.h"
|
||||
#include "base/time/time.h"
|
||||
#include "remoting/base/constants.h"
|
||||
@ -275,9 +279,12 @@ void JingleSession::InitializeIncomingConnection(
|
||||
SessionConfig::SelectCommon(initiate_message.description->config(),
|
||||
session_manager_->protocol_config_.get());
|
||||
if (!config_) {
|
||||
LOG(WARNING) << "Rejecting connection from " << peer_address_.id()
|
||||
<< " because no compatible configuration has been found.";
|
||||
Close(ErrorCode::INCOMPATIBLE_PROTOCOL);
|
||||
Close(ErrorCode::INCOMPATIBLE_PROTOCOL,
|
||||
base::StringPrintf("Rejecting connection from %s because no "
|
||||
"compatible configuration has "
|
||||
"been found.",
|
||||
peer_address_.id()),
|
||||
FROM_HERE);
|
||||
return;
|
||||
}
|
||||
}
|
||||
@ -292,7 +299,8 @@ void JingleSession::AcceptIncomingConnection(
|
||||
initiate_message.description->authenticator_message();
|
||||
|
||||
if (!first_auth_message) {
|
||||
Close(ErrorCode::INCOMPATIBLE_PROTOCOL);
|
||||
Close(ErrorCode::INCOMPATIBLE_PROTOCOL,
|
||||
"Cannot find the first authentication message.", FROM_HERE);
|
||||
return;
|
||||
}
|
||||
|
||||
@ -307,7 +315,10 @@ void JingleSession::AcceptIncomingConnection(
|
||||
void JingleSession::ContinueAcceptIncomingConnection() {
|
||||
DCHECK_NE(authenticator_->state(), Authenticator::PROCESSING_MESSAGE);
|
||||
if (authenticator_->state() == Authenticator::REJECTED) {
|
||||
Close(AuthRejectionReasonToErrorCode(authenticator_->rejection_reason()));
|
||||
Authenticator::RejectionDetails details =
|
||||
authenticator_->rejection_details();
|
||||
Close(AuthRejectionReasonToErrorCode(authenticator_->rejection_reason()),
|
||||
details.message, details.location);
|
||||
return;
|
||||
}
|
||||
|
||||
@ -383,7 +394,9 @@ void JingleSession::SendTransportInfo(
|
||||
}
|
||||
}
|
||||
|
||||
void JingleSession::Close(protocol::ErrorCode error) {
|
||||
void JingleSession::Close(protocol::ErrorCode error,
|
||||
std::string_view error_details,
|
||||
const base::Location& error_location) {
|
||||
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
|
||||
|
||||
if (is_session_active()) {
|
||||
@ -418,6 +431,24 @@ void JingleSession::Close(protocol::ErrorCode error) {
|
||||
peer_address_, JingleMessage::SESSION_TERMINATE, session_id_));
|
||||
message->reason = reason;
|
||||
message->error_code = error;
|
||||
if (!error_details.empty()) {
|
||||
message->error_details = error_details;
|
||||
}
|
||||
if (error_location.program_counter() != nullptr) {
|
||||
message->error_location = error_location.ToString();
|
||||
}
|
||||
if (error != ErrorCode::OK) {
|
||||
std::string additional_logs;
|
||||
if (!error_details.empty()) {
|
||||
additional_logs += ": ";
|
||||
additional_logs += error_details;
|
||||
}
|
||||
if (!message->error_location.empty()) {
|
||||
additional_logs += " (" + message->error_location + ")";
|
||||
}
|
||||
LOG(WARNING) << "Session closed with error " << static_cast<int>(error)
|
||||
<< additional_logs;
|
||||
}
|
||||
SendMessage(std::move(message));
|
||||
}
|
||||
|
||||
@ -489,20 +520,21 @@ void JingleSession::OnMessageResponse(JingleMessage::ActionType request_type,
|
||||
|
||||
// |response| will be nullptr if the request timed out.
|
||||
if (!response) {
|
||||
LOG(ERROR) << type_str << " request timed out.";
|
||||
Close(ErrorCode::SIGNALING_TIMEOUT);
|
||||
Close(ErrorCode::SIGNALING_TIMEOUT,
|
||||
base::StringPrintf("%s request timed out.", type_str), FROM_HERE);
|
||||
return;
|
||||
} else {
|
||||
const std::string& type =
|
||||
response->Attr(jingle_xmpp::QName(std::string(), "type"));
|
||||
if (type != "result") {
|
||||
LOG(ERROR) << "Received error in response to " << type_str
|
||||
<< " message: \"" << response->Str()
|
||||
<< "\". Terminating the session.";
|
||||
|
||||
// TODO(sergeyu): There may be different reasons for error
|
||||
// here. Parse the response stanza to find failure reason.
|
||||
Close(ErrorCode::PEER_IS_OFFLINE);
|
||||
Close(ErrorCode::PEER_IS_OFFLINE,
|
||||
base::StringPrintf(
|
||||
"Received error in response to %s message: \"%s\". "
|
||||
"Terminating the session.",
|
||||
type_str, response->Str()),
|
||||
FROM_HERE);
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -530,9 +562,12 @@ void JingleSession::OnTransportInfoResponse(
|
||||
const std::string& type =
|
||||
response->Attr(jingle_xmpp::QName(std::string(), "type"));
|
||||
if (type != "result") {
|
||||
LOG(ERROR) << "Received error in response to transport-info message: \""
|
||||
<< response->Str() << "\". Terminating the session.";
|
||||
Close(ErrorCode::PEER_IS_OFFLINE);
|
||||
Close(ErrorCode::PEER_IS_OFFLINE,
|
||||
base::StringPrintf(
|
||||
"Received error in response to transport-info message: \"%s\". "
|
||||
"Terminating the session.",
|
||||
response->Str()),
|
||||
FROM_HERE);
|
||||
}
|
||||
}
|
||||
|
||||
@ -597,13 +632,16 @@ void JingleSession::OnAccept(std::unique_ptr<JingleMessage> message,
|
||||
const jingle_xmpp::XmlElement* auth_message =
|
||||
message->description->authenticator_message();
|
||||
if (!auth_message) {
|
||||
DLOG(WARNING) << "Received session-accept without authentication message ";
|
||||
Close(ErrorCode::INCOMPATIBLE_PROTOCOL);
|
||||
Close(ErrorCode::INCOMPATIBLE_PROTOCOL,
|
||||
"Received session-accept without authentication message", FROM_HERE);
|
||||
return;
|
||||
}
|
||||
|
||||
if (!InitializeConfigFromDescription(message->description.get())) {
|
||||
Close(ErrorCode::INCOMPATIBLE_PROTOCOL);
|
||||
std::string error_details;
|
||||
base::Location error_location;
|
||||
if (!InitializeConfigFromDescription(message->description.get(),
|
||||
error_details, error_location)) {
|
||||
Close(ErrorCode::INCOMPATIBLE_PROTOCOL, error_details, error_location);
|
||||
return;
|
||||
}
|
||||
|
||||
@ -625,10 +663,11 @@ void JingleSession::OnSessionInfo(std::unique_ptr<JingleMessage> message,
|
||||
|
||||
if ((state_ != ACCEPTED && state_ != AUTHENTICATING) ||
|
||||
authenticator_->state() != Authenticator::WAITING_MESSAGE) {
|
||||
LOG(WARNING) << "Received unexpected authenticator message "
|
||||
<< message->info->Str();
|
||||
std::move(reply_callback).Run(JingleMessageReply::UNEXPECTED_REQUEST);
|
||||
Close(ErrorCode::INCOMPATIBLE_PROTOCOL);
|
||||
Close(ErrorCode::INCOMPATIBLE_PROTOCOL,
|
||||
base::StringPrintf("Received unexpected authenticator message %s",
|
||||
message->info->Str()),
|
||||
FROM_HERE);
|
||||
return;
|
||||
}
|
||||
|
||||
@ -721,7 +760,10 @@ void JingleSession::OnTerminate(std::unique_ptr<JingleMessage> message,
|
||||
|
||||
void JingleSession::OnAuthenticatorStateChangeAfterAccepted() {
|
||||
if (authenticator_->state() == Authenticator::REJECTED) {
|
||||
Close(AuthRejectionReasonToErrorCode(authenticator_->rejection_reason()));
|
||||
Authenticator::RejectionDetails details =
|
||||
authenticator_->rejection_details();
|
||||
Close(AuthRejectionReasonToErrorCode(authenticator_->rejection_reason()),
|
||||
details.message, details.location);
|
||||
} else {
|
||||
NOTREACHED() << "Unexpected authenticator state: "
|
||||
<< authenticator_->state();
|
||||
@ -729,15 +771,23 @@ void JingleSession::OnAuthenticatorStateChangeAfterAccepted() {
|
||||
}
|
||||
|
||||
bool JingleSession::InitializeConfigFromDescription(
|
||||
const ContentDescription* description) {
|
||||
const ContentDescription* description,
|
||||
std::string& error_details,
|
||||
base::Location& error_location) {
|
||||
DCHECK(description);
|
||||
config_ = SessionConfig::GetFinalConfig(description->config());
|
||||
if (!config_) {
|
||||
LOG(ERROR) << "session-accept does not specify configuration";
|
||||
error_details =
|
||||
"Received session-accept message does not specify the session "
|
||||
"configuration.";
|
||||
error_location = FROM_HERE;
|
||||
return false;
|
||||
}
|
||||
if (!session_manager_->protocol_config_->IsSupported(*config_)) {
|
||||
LOG(ERROR) << "session-accept specifies an invalid configuration";
|
||||
error_details =
|
||||
"Received session-accept message specifies an invalid session "
|
||||
"configuration.";
|
||||
error_location = FROM_HERE;
|
||||
return false;
|
||||
}
|
||||
|
||||
@ -775,7 +825,10 @@ void JingleSession::ProcessAuthenticationStep() {
|
||||
if (authenticator_->state() == Authenticator::ACCEPTED) {
|
||||
OnAuthenticated();
|
||||
} else if (authenticator_->state() == Authenticator::REJECTED) {
|
||||
Close(AuthRejectionReasonToErrorCode(authenticator_->rejection_reason()));
|
||||
Authenticator::RejectionDetails details =
|
||||
authenticator_->rejection_details();
|
||||
Close(AuthRejectionReasonToErrorCode(authenticator_->rejection_reason()),
|
||||
details.message, details.location);
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -6,8 +6,10 @@
|
||||
#define REMOTING_PROTOCOL_JINGLE_SESSION_H_
|
||||
|
||||
#include <string>
|
||||
#include <string_view>
|
||||
#include <vector>
|
||||
|
||||
#include "base/location.h"
|
||||
#include "base/memory/raw_ptr.h"
|
||||
#include "base/rand_util.h"
|
||||
#include "base/strings/string_number_conversions.h"
|
||||
@ -31,6 +33,9 @@ class Transport;
|
||||
// JingleSessionManager.
|
||||
class JingleSession : public Session {
|
||||
public:
|
||||
// The Close() override hides all the Close() overloads in the base class, so
|
||||
// they need to be unhidden with a using statement.
|
||||
using Session::Close;
|
||||
JingleSession(const JingleSession&) = delete;
|
||||
JingleSession& operator=(const JingleSession&) = delete;
|
||||
|
||||
@ -43,7 +48,9 @@ class JingleSession : public Session {
|
||||
const SessionConfig& config() override;
|
||||
const Authenticator& authenticator() const override;
|
||||
void SetTransport(Transport* transport) override;
|
||||
void Close(protocol::ErrorCode error) override;
|
||||
void Close(protocol::ErrorCode error,
|
||||
std::string_view error_details,
|
||||
const base::Location& error_location) override;
|
||||
void AddPlugin(SessionPlugin* plugin) override;
|
||||
|
||||
private:
|
||||
@ -106,8 +113,11 @@ class JingleSession : public Session {
|
||||
ReplyCallback reply_callback);
|
||||
void OnAuthenticatorStateChangeAfterAccepted();
|
||||
|
||||
// Called from OnAccept() to initialize session config.
|
||||
bool InitializeConfigFromDescription(const ContentDescription* description);
|
||||
// Called from OnAccept() to initialize session config. If initialization
|
||||
// fails, |error_details| will be updated.
|
||||
bool InitializeConfigFromDescription(const ContentDescription* description,
|
||||
std::string& error_details,
|
||||
base::Location& error_location);
|
||||
|
||||
// Called after the initial incoming authenticator message is processed.
|
||||
void ContinueAcceptIncomingConnection();
|
||||
|
@ -9,10 +9,10 @@
|
||||
|
||||
#include "base/base64.h"
|
||||
#include "base/functional/bind.h"
|
||||
#include "base/logging.h"
|
||||
#include "base/memory/ptr_util.h"
|
||||
#include "base/strings/string_split.h"
|
||||
#include "base/strings/string_util.h"
|
||||
#include "base/strings/stringprintf.h"
|
||||
#include "remoting/base/rsa_key_pair.h"
|
||||
#include "remoting/protocol/channel_authenticator.h"
|
||||
#include "remoting/protocol/host_authentication_config.h"
|
||||
@ -47,14 +47,19 @@ Me2MeHostAuthenticatorFactory::CreateAuthenticator(
|
||||
// Verify that the client's jid is an ASCII string.
|
||||
auto parts = base::SplitStringOnce(remote_jid, '/');
|
||||
if (!base::IsStringASCII(remote_jid) || !parts) {
|
||||
LOG(ERROR) << "Rejecting incoming connection from " << remote_jid
|
||||
<< ": Invalid signaling id.";
|
||||
return std::make_unique<RejectingAuthenticator>(INVALID_CREDENTIALS);
|
||||
return std::make_unique<RejectingAuthenticator>(
|
||||
INVALID_CREDENTIALS,
|
||||
base::StringPrintf(
|
||||
"Rejecting incoming connection from %s: Invalid signaling id.",
|
||||
remote_jid));
|
||||
}
|
||||
|
||||
auto [email_address, _] = *parts;
|
||||
if (!check_access_permission_callback_.Run(email_address)) {
|
||||
return std::make_unique<RejectingAuthenticator>(INVALID_CREDENTIALS);
|
||||
return std::make_unique<RejectingAuthenticator>(
|
||||
INVALID_CREDENTIALS,
|
||||
base::StringPrintf("Permission check failed for email %s.",
|
||||
email_address));
|
||||
}
|
||||
|
||||
if (!config_->local_cert.empty() && config_->key_pair.get()) {
|
||||
@ -66,7 +71,8 @@ Me2MeHostAuthenticatorFactory::CreateAuthenticator(
|
||||
std::make_unique<HostAuthenticationConfig>(*config_));
|
||||
}
|
||||
|
||||
return std::make_unique<RejectingAuthenticator>(INVALID_CREDENTIALS);
|
||||
return std::make_unique<RejectingAuthenticator>(
|
||||
INVALID_CREDENTIALS, "Invalid HostAuthenticationConfig.");
|
||||
}
|
||||
|
||||
} // namespace remoting::protocol
|
||||
|
@ -69,6 +69,11 @@ Authenticator::RejectionReason NegotiatingAuthenticatorBase::rejection_reason()
|
||||
return rejection_reason_;
|
||||
}
|
||||
|
||||
Authenticator::RejectionDetails
|
||||
NegotiatingAuthenticatorBase::rejection_details() const {
|
||||
return rejection_details_;
|
||||
}
|
||||
|
||||
void NegotiatingAuthenticatorBase::ProcessMessageInternal(
|
||||
const jingle_xmpp::XmlElement* message,
|
||||
base::OnceClosure resume_callback) {
|
||||
@ -103,6 +108,7 @@ void NegotiatingAuthenticatorBase::UpdateState(
|
||||
|
||||
if (state_ == REJECTED) {
|
||||
rejection_reason_ = current_authenticator_->rejection_reason();
|
||||
rejection_details_ = current_authenticator_->rejection_details();
|
||||
}
|
||||
|
||||
std::move(resume_callback).Run();
|
||||
@ -130,6 +136,7 @@ void NegotiatingAuthenticatorBase::NotifyStateChangeAfterAccepted() {
|
||||
state_ = current_authenticator_->state();
|
||||
if (state_ == REJECTED) {
|
||||
rejection_reason_ = current_authenticator_->rejection_reason();
|
||||
rejection_details_ = current_authenticator_->rejection_details();
|
||||
}
|
||||
Authenticator::NotifyStateChangeAfterAccepted();
|
||||
}
|
||||
|
@ -73,6 +73,7 @@ class NegotiatingAuthenticatorBase : public Authenticator {
|
||||
State state() const override;
|
||||
bool started() const override;
|
||||
RejectionReason rejection_reason() const override;
|
||||
RejectionDetails rejection_details() const override;
|
||||
const std::string& GetAuthKey() const override;
|
||||
const SessionPolicies* GetSessionPolicies() const override;
|
||||
std::unique_ptr<ChannelAuthenticator> CreateChannelAuthenticator()
|
||||
@ -112,6 +113,7 @@ class NegotiatingAuthenticatorBase : public Authenticator {
|
||||
std::unique_ptr<Authenticator> current_authenticator_;
|
||||
State state_;
|
||||
RejectionReason rejection_reason_ = RejectionReason::INVALID_CREDENTIALS;
|
||||
RejectionDetails rejection_details_;
|
||||
};
|
||||
|
||||
} // namespace remoting::protocol
|
||||
|
@ -12,6 +12,7 @@
|
||||
#include "base/containers/contains.h"
|
||||
#include "base/functional/bind.h"
|
||||
#include "base/functional/callback.h"
|
||||
#include "base/location.h"
|
||||
#include "base/memory/ptr_util.h"
|
||||
#include "base/notreached.h"
|
||||
#include "base/strings/string_split.h"
|
||||
@ -49,12 +50,21 @@ void NegotiatingClientAuthenticator::ProcessMessage(
|
||||
|
||||
// The host picked a method different from the one the client had selected.
|
||||
if (method != current_method_) {
|
||||
// The host must pick a method that is valid and supported by the client,
|
||||
// and it must not change methods after it has picked one.
|
||||
if (method_set_by_host_ || method == AuthenticationMethod::INVALID ||
|
||||
if (method_set_by_host_) {
|
||||
state_ = REJECTED;
|
||||
rejection_reason_ = RejectionReason::PROTOCOL_ERROR;
|
||||
rejection_details_ = RejectionDetails(
|
||||
"The host must not change methods after it has picked one.");
|
||||
std::move(resume_callback).Run();
|
||||
return;
|
||||
}
|
||||
if (method == AuthenticationMethod::INVALID ||
|
||||
!base::Contains(methods_, method)) {
|
||||
state_ = REJECTED;
|
||||
rejection_reason_ = RejectionReason::PROTOCOL_ERROR;
|
||||
rejection_details_ = RejectionDetails(
|
||||
"The host must pick a method that is valid and supported by the "
|
||||
"client.");
|
||||
std::move(resume_callback).Run();
|
||||
return;
|
||||
}
|
||||
|
@ -56,11 +56,13 @@ void NegotiatingHostAuthenticator::ProcessMessage(
|
||||
std::string method_attr = message->Attr(kMethodAttributeQName);
|
||||
AuthenticationMethod method = ParseAuthenticationMethodString(method_attr);
|
||||
|
||||
// If the host has already chosen a method, it can't be changed by the client.
|
||||
if (current_method_ != AuthenticationMethod::INVALID &&
|
||||
method != current_method_) {
|
||||
state_ = REJECTED;
|
||||
rejection_reason_ = RejectionReason::PROTOCOL_ERROR;
|
||||
rejection_details_ = RejectionDetails(
|
||||
"The host has already chosen an authentication method. "
|
||||
"The client cannot change it.");
|
||||
std::move(resume_callback).Run();
|
||||
return;
|
||||
}
|
||||
@ -75,9 +77,11 @@ void NegotiatingHostAuthenticator::ProcessMessage(
|
||||
std::string supported_methods_attr =
|
||||
message->Attr(kSupportedMethodsAttributeQName);
|
||||
if (supported_methods_attr.empty()) {
|
||||
// Message contains neither method nor supported-methods attributes.
|
||||
state_ = REJECTED;
|
||||
rejection_reason_ = RejectionReason::PROTOCOL_ERROR;
|
||||
rejection_details_ = RejectionDetails(
|
||||
"Message contains neither the 'method' nor the 'supported-methods' "
|
||||
"attributes.");
|
||||
std::move(resume_callback).Run();
|
||||
return;
|
||||
}
|
||||
@ -97,9 +101,10 @@ void NegotiatingHostAuthenticator::ProcessMessage(
|
||||
}
|
||||
|
||||
if (method == AuthenticationMethod::INVALID) {
|
||||
// Failed to find a common auth method.
|
||||
state_ = REJECTED;
|
||||
rejection_reason_ = RejectionReason::NO_COMMON_AUTH_METHOD;
|
||||
rejection_details_ = RejectionDetails(
|
||||
"No common authentication method found between client and host.");
|
||||
std::move(resume_callback).Run();
|
||||
return;
|
||||
}
|
||||
|
@ -8,8 +8,10 @@
|
||||
|
||||
#include "base/functional/bind.h"
|
||||
#include "base/functional/callback.h"
|
||||
#include "base/location.h"
|
||||
#include "base/logging.h"
|
||||
#include "remoting/base/constants.h"
|
||||
#include "remoting/protocol/authenticator.h"
|
||||
#include "remoting/protocol/channel_authenticator.h"
|
||||
#include "remoting/protocol/credentials_type.h"
|
||||
|
||||
@ -53,6 +55,19 @@ Authenticator::RejectionReason PairingAuthenticatorBase::rejection_reason()
|
||||
return spake2_authenticator_->rejection_reason();
|
||||
}
|
||||
|
||||
Authenticator::RejectionDetails PairingAuthenticatorBase::rejection_details()
|
||||
const {
|
||||
if (spake2_authenticator_ &&
|
||||
spake2_authenticator_->state() == State::REJECTED) {
|
||||
Authenticator::RejectionDetails spake2_rejection_details =
|
||||
spake2_authenticator_->rejection_details();
|
||||
if (!spake2_rejection_details.is_null()) {
|
||||
return spake2_rejection_details;
|
||||
}
|
||||
}
|
||||
return RejectionDetails(error_message_);
|
||||
}
|
||||
|
||||
void PairingAuthenticatorBase::ProcessMessage(
|
||||
const jingle_xmpp::XmlElement* message,
|
||||
base::OnceClosure resume_callback) {
|
||||
|
@ -50,6 +50,7 @@ class PairingAuthenticatorBase : public Authenticator {
|
||||
State state() const override;
|
||||
bool started() const override;
|
||||
RejectionReason rejection_reason() const override;
|
||||
RejectionDetails rejection_details() const override;
|
||||
void ProcessMessage(const jingle_xmpp::XmlElement* message,
|
||||
base::OnceClosure resume_callback) override;
|
||||
std::unique_ptr<jingle_xmpp::XmlElement> GetNextMessage() override;
|
||||
|
@ -9,6 +9,7 @@
|
||||
#include <map>
|
||||
#include <memory>
|
||||
#include <string>
|
||||
#include <string_view>
|
||||
#include <utility>
|
||||
|
||||
#include "base/location.h"
|
||||
@ -16,6 +17,7 @@
|
||||
#include "base/task/single_thread_task_runner.h"
|
||||
#include "base/values.h"
|
||||
#include "net/base/ip_endpoint.h"
|
||||
#include "remoting/base/errors.h"
|
||||
#include "remoting/base/session_policies.h"
|
||||
#include "remoting/proto/internal.pb.h"
|
||||
#include "remoting/proto/video.pb.h"
|
||||
@ -59,6 +61,10 @@ class MockAuthenticator : public Authenticator {
|
||||
MOCK_CONST_METHOD0(state, Authenticator::State());
|
||||
MOCK_CONST_METHOD0(started, bool());
|
||||
MOCK_CONST_METHOD0(rejection_reason, Authenticator::RejectionReason());
|
||||
MOCK_METHOD(Authenticator::RejectionDetails,
|
||||
rejection_details,
|
||||
(),
|
||||
(const, override));
|
||||
MOCK_CONST_METHOD0(GetAuthKey, const std::string&());
|
||||
MOCK_CONST_METHOD0(CreateChannelAuthenticatorPtr, ChannelAuthenticator*());
|
||||
MOCK_METHOD2(ProcessMessage,
|
||||
@ -236,7 +242,12 @@ class MockSession : public Session {
|
||||
MOCK_METHOD0(jid, const std::string&());
|
||||
MOCK_METHOD0(config, const SessionConfig&());
|
||||
MOCK_METHOD(const Authenticator&, authenticator, (), (const, override));
|
||||
MOCK_METHOD1(Close, void(ErrorCode error));
|
||||
MOCK_METHOD(void,
|
||||
Close,
|
||||
(ErrorCode error,
|
||||
std::string_view error_details,
|
||||
const base::Location& location),
|
||||
(override));
|
||||
MOCK_METHOD1(AddPlugin, void(SessionPlugin* plugin));
|
||||
};
|
||||
|
||||
|
@ -12,8 +12,12 @@
|
||||
|
||||
namespace remoting::protocol {
|
||||
|
||||
RejectingAuthenticator::RejectingAuthenticator(RejectionReason rejection_reason)
|
||||
: rejection_reason_(rejection_reason) {}
|
||||
RejectingAuthenticator::RejectingAuthenticator(
|
||||
RejectionReason rejection_reason,
|
||||
std::string_view rejection_message,
|
||||
const base::Location& rejection_location)
|
||||
: rejection_reason_(rejection_reason),
|
||||
rejection_details_{std::string(rejection_message), rejection_location} {}
|
||||
|
||||
RejectingAuthenticator::~RejectingAuthenticator() = default;
|
||||
|
||||
@ -39,6 +43,12 @@ Authenticator::RejectionReason RejectingAuthenticator::rejection_reason()
|
||||
return rejection_reason_;
|
||||
}
|
||||
|
||||
Authenticator::RejectionDetails RejectingAuthenticator::rejection_details()
|
||||
const {
|
||||
DCHECK_EQ(state_, REJECTED);
|
||||
return rejection_details_;
|
||||
}
|
||||
|
||||
void RejectingAuthenticator::ProcessMessage(
|
||||
const jingle_xmpp::XmlElement* message,
|
||||
base::OnceClosure resume_callback) {
|
||||
|
@ -6,7 +6,9 @@
|
||||
#define REMOTING_PROTOCOL_REJECTING_AUTHENTICATOR_H_
|
||||
|
||||
#include <string>
|
||||
#include <string_view>
|
||||
|
||||
#include "base/location.h"
|
||||
#include "remoting/protocol/authenticator.h"
|
||||
|
||||
namespace remoting::protocol {
|
||||
@ -14,7 +16,12 @@ namespace remoting::protocol {
|
||||
// Authenticator that accepts one message and rejects connection after that.
|
||||
class RejectingAuthenticator : public Authenticator {
|
||||
public:
|
||||
RejectingAuthenticator(RejectionReason rejection_reason);
|
||||
RejectingAuthenticator(
|
||||
RejectionReason rejection_reason,
|
||||
std::string_view rejection_message,
|
||||
// Current() takes location info with default parameters, which is
|
||||
// filled when this constructor is called.
|
||||
const base::Location& rejection_location = base::Location::Current());
|
||||
|
||||
RejectingAuthenticator(const RejectingAuthenticator&) = delete;
|
||||
RejectingAuthenticator& operator=(const RejectingAuthenticator&) = delete;
|
||||
@ -27,6 +34,7 @@ class RejectingAuthenticator : public Authenticator {
|
||||
State state() const override;
|
||||
bool started() const override;
|
||||
RejectionReason rejection_reason() const override;
|
||||
RejectionDetails rejection_details() const override;
|
||||
void ProcessMessage(const jingle_xmpp::XmlElement* message,
|
||||
base::OnceClosure resume_callback) override;
|
||||
std::unique_ptr<jingle_xmpp::XmlElement> GetNextMessage() override;
|
||||
@ -37,6 +45,7 @@ class RejectingAuthenticator : public Authenticator {
|
||||
|
||||
private:
|
||||
RejectionReason rejection_reason_;
|
||||
RejectionDetails rejection_details_;
|
||||
State state_ = WAITING_MESSAGE;
|
||||
std::string auth_key_;
|
||||
};
|
||||
|
15
remoting/protocol/session.cc
Normal file
15
remoting/protocol/session.cc
Normal file
@ -0,0 +1,15 @@
|
||||
// Copyright 2024 The Chromium Authors
|
||||
// Use of this source code is governed by a BSD-style license that can be
|
||||
// found in the LICENSE file.
|
||||
|
||||
#include "remoting/protocol/session.h"
|
||||
|
||||
#include "base/location.h"
|
||||
|
||||
namespace remoting::protocol {
|
||||
|
||||
void Session::Close(ErrorCode error) {
|
||||
Close(error, /* error_details= */ {}, /* error_location= */ {});
|
||||
}
|
||||
|
||||
} // namespace remoting::protocol
|
@ -7,7 +7,9 @@
|
||||
|
||||
#include <memory>
|
||||
#include <string>
|
||||
#include <string_view>
|
||||
|
||||
#include "base/location.h"
|
||||
#include "remoting/protocol/errors.h"
|
||||
#include "remoting/protocol/session_config.h"
|
||||
#include "remoting/protocol/transport.h"
|
||||
@ -88,9 +90,21 @@ class Session {
|
||||
virtual void SetTransport(Transport* transport) = 0;
|
||||
|
||||
// Closes connection. EventHandler is guaranteed not to be called after this
|
||||
// method returns. |error| specifies the error code in case when the session
|
||||
// is being closed due to an error.
|
||||
virtual void Close(ErrorCode error) = 0;
|
||||
// method returns.
|
||||
// |error| specifies the error code in case when the session is being closed
|
||||
// due to an error.
|
||||
void Close(ErrorCode error);
|
||||
|
||||
// Closes connection. EventHandler is guaranteed not to be called after this
|
||||
// method returns.
|
||||
// |error| specifies the error code in case when the session is being closed
|
||||
// due to an error.
|
||||
// |error_details| is a free-form human-readable string that describes the
|
||||
// reason for closing the connection.
|
||||
// |error_location| denotes where the error occurs in the code.
|
||||
virtual void Close(ErrorCode error,
|
||||
std::string_view error_details,
|
||||
const base::Location& error_location) = 0;
|
||||
|
||||
// Adds a SessionPlugin to handle attachments. To ensure plugin attachments
|
||||
// are processed correctly for session-initiate message, this function must be
|
||||
|
@ -9,8 +9,8 @@
|
||||
|
||||
#include "base/check.h"
|
||||
#include "base/functional/bind.h"
|
||||
#include "base/logging.h"
|
||||
#include "base/notreached.h"
|
||||
#include "base/strings/stringprintf.h"
|
||||
#include "remoting/base/protobuf_http_status.h"
|
||||
#include "remoting/base/session_authz_service_client.h"
|
||||
#include "remoting/proto/session_authz_service.h"
|
||||
@ -77,6 +77,16 @@ Authenticator::RejectionReason SessionAuthzAuthenticator::rejection_reason()
|
||||
return underlying_->rejection_reason();
|
||||
}
|
||||
|
||||
Authenticator::RejectionDetails SessionAuthzAuthenticator::rejection_details()
|
||||
const {
|
||||
DCHECK_EQ(state(), REJECTED);
|
||||
|
||||
if (session_authz_state_ == SessionAuthzState::FAILED) {
|
||||
return rejection_details_;
|
||||
}
|
||||
return underlying_->rejection_details();
|
||||
}
|
||||
|
||||
void SessionAuthzAuthenticator::ProcessMessage(
|
||||
const jingle_xmpp::XmlElement* message,
|
||||
base::OnceClosure resume_callback) {
|
||||
@ -207,10 +217,12 @@ void SessionAuthzAuthenticator::OnVerifiedSessionToken(
|
||||
return;
|
||||
}
|
||||
if (response->session_id != session_id_) {
|
||||
LOG(ERROR) << "Session token verification failed. Expected session ID: "
|
||||
<< session_id_ << ", actual: " << response->session_id;
|
||||
session_authz_state_ = SessionAuthzState::FAILED;
|
||||
session_authz_rejection_reason_ = RejectionReason::INVALID_ACCOUNT_ID;
|
||||
rejection_details_ = RejectionDetails(base::StringPrintf(
|
||||
"Session token verification failed. Expected session ID: %s, actual: "
|
||||
"%s",
|
||||
session_id_, response->session_id));
|
||||
std::move(resume_callback).Run();
|
||||
return;
|
||||
}
|
||||
@ -229,9 +241,9 @@ void SessionAuthzAuthenticator::HandleSessionAuthzError(
|
||||
const std::string_view& action_name,
|
||||
const ProtobufHttpStatus& status) {
|
||||
DCHECK(!status.ok());
|
||||
LOG(ERROR) << "SessionAuthz " << action_name
|
||||
<< " error, code: " << static_cast<int>(status.error_code())
|
||||
<< ", message: " << status.error_message();
|
||||
rejection_details_ = RejectionDetails(base::StringPrintf(
|
||||
"SessionAuthz %s error, code: %d, message: %s", action_name,
|
||||
static_cast<int>(status.error_code()), status.error_message()));
|
||||
session_authz_state_ = SessionAuthzState::FAILED;
|
||||
switch (status.error_code()) {
|
||||
case ProtobufHttpStatus::Code::PERMISSION_DENIED:
|
||||
|
@ -66,6 +66,7 @@ class SessionAuthzAuthenticator : public Authenticator {
|
||||
State state() const override;
|
||||
bool started() const override;
|
||||
RejectionReason rejection_reason() const override;
|
||||
RejectionDetails rejection_details() const override;
|
||||
void ProcessMessage(const jingle_xmpp::XmlElement* message,
|
||||
base::OnceClosure resume_callback) override;
|
||||
std::unique_ptr<jingle_xmpp::XmlElement> GetNextMessage() override;
|
||||
@ -143,6 +144,8 @@ class SessionAuthzAuthenticator : public Authenticator {
|
||||
// rejection reason is delegated to `underlying_->rejection_reason()`.
|
||||
RejectionReason session_authz_rejection_reason_;
|
||||
|
||||
RejectionDetails rejection_details_;
|
||||
|
||||
std::string session_id_;
|
||||
std::string host_token_;
|
||||
};
|
||||
|
@ -84,6 +84,7 @@ class FakeClientAuthenticator : public Authenticator {
|
||||
State state() const override;
|
||||
bool started() const override;
|
||||
RejectionReason rejection_reason() const override;
|
||||
RejectionDetails rejection_details() const override;
|
||||
void ProcessMessage(const jingle_xmpp::XmlElement* message,
|
||||
base::OnceClosure resume_callback) override;
|
||||
std::unique_ptr<jingle_xmpp::XmlElement> GetNextMessage() override;
|
||||
@ -157,6 +158,14 @@ Authenticator::RejectionReason FakeClientAuthenticator::rejection_reason()
|
||||
: underlying_->rejection_reason();
|
||||
}
|
||||
|
||||
Authenticator::RejectionDetails FakeClientAuthenticator::rejection_details()
|
||||
const {
|
||||
if (underlying_ && underlying_->state() == State::REJECTED) {
|
||||
return underlying_->rejection_details();
|
||||
}
|
||||
return {};
|
||||
}
|
||||
|
||||
void FakeClientAuthenticator::ProcessMessage(
|
||||
const jingle_xmpp::XmlElement* message,
|
||||
base::OnceClosure resume_callback) {
|
||||
|
@ -8,6 +8,7 @@
|
||||
|
||||
#include "base/base64.h"
|
||||
#include "base/containers/span.h"
|
||||
#include "base/location.h"
|
||||
#include "base/logging.h"
|
||||
#include "base/memory/ptr_util.h"
|
||||
#include "base/numerics/byte_conversions.h"
|
||||
@ -15,6 +16,7 @@
|
||||
#include "crypto/secure_util.h"
|
||||
#include "remoting/base/constants.h"
|
||||
#include "remoting/base/rsa_key_pair.h"
|
||||
#include "remoting/protocol/authenticator.h"
|
||||
#include "remoting/protocol/ssl_hmac_channel_authenticator.h"
|
||||
#include "third_party/boringssl/src/include/openssl/curve25519.h"
|
||||
#include "third_party/libjingle_xmpp/xmllite/xmlelement.h"
|
||||
@ -162,6 +164,11 @@ Authenticator::RejectionReason Spake2Authenticator::rejection_reason() const {
|
||||
return rejection_reason_;
|
||||
}
|
||||
|
||||
Authenticator::RejectionDetails Spake2Authenticator::rejection_details() const {
|
||||
DCHECK_EQ(state(), REJECTED);
|
||||
return rejection_details_;
|
||||
}
|
||||
|
||||
void Spake2Authenticator::ProcessMessage(const jingle_xmpp::XmlElement* message,
|
||||
base::OnceClosure resume_callback) {
|
||||
ProcessMessageInternal(message);
|
||||
@ -178,14 +185,16 @@ void Spake2Authenticator::ProcessMessageInternal(
|
||||
&remote_cert_)) {
|
||||
state_ = REJECTED;
|
||||
rejection_reason_ = RejectionReason::PROTOCOL_ERROR;
|
||||
rejection_details_ = RejectionDetails(
|
||||
"Failed to decode the remote certificate in the incoming message.");
|
||||
return;
|
||||
}
|
||||
|
||||
// Client always expects certificate in the first message.
|
||||
if (!is_host_ && remote_cert_.empty()) {
|
||||
LOG(WARNING) << "No valid host certificate.";
|
||||
state_ = REJECTED;
|
||||
rejection_reason_ = RejectionReason::PROTOCOL_ERROR;
|
||||
rejection_details_ = RejectionDetails("No valid host certificate.");
|
||||
return;
|
||||
}
|
||||
|
||||
@ -200,15 +209,18 @@ void Spake2Authenticator::ProcessMessageInternal(
|
||||
&verification_hash)) {
|
||||
state_ = REJECTED;
|
||||
rejection_reason_ = RejectionReason::PROTOCOL_ERROR;
|
||||
rejection_details_ = RejectionDetails(
|
||||
"Failed to decode the spake message or the verification hash in the "
|
||||
"incoming message.");
|
||||
return;
|
||||
}
|
||||
|
||||
// |auth_key_| is generated when <spake-message> is received.
|
||||
if (auth_key_.empty()) {
|
||||
if (!spake_message_present) {
|
||||
LOG(WARNING) << "<spake-message> not found.";
|
||||
state_ = REJECTED;
|
||||
rejection_reason_ = RejectionReason::PROTOCOL_ERROR;
|
||||
rejection_details_ = RejectionDetails("<spake-message> not found.");
|
||||
return;
|
||||
}
|
||||
uint8_t key[SPAKE2_MAX_KEY_SIZE];
|
||||
@ -221,6 +233,8 @@ void Spake2Authenticator::ProcessMessageInternal(
|
||||
if (!result) {
|
||||
state_ = REJECTED;
|
||||
rejection_reason_ = RejectionReason::INVALID_CREDENTIALS;
|
||||
rejection_details_ =
|
||||
RejectionDetails("Failed to process SPAKE2 message.");
|
||||
return;
|
||||
}
|
||||
CHECK(key_size);
|
||||
@ -231,16 +245,18 @@ void Spake2Authenticator::ProcessMessageInternal(
|
||||
expected_verification_hash_ =
|
||||
CalculateVerificationHash(!is_host_, remote_id_, local_id_);
|
||||
} else if (spake_message_present) {
|
||||
LOG(WARNING) << "Received duplicate <spake-message>.";
|
||||
state_ = REJECTED;
|
||||
rejection_reason_ = RejectionReason::PROTOCOL_ERROR;
|
||||
rejection_details_ =
|
||||
RejectionDetails("Received duplicate <spake-message>.");
|
||||
return;
|
||||
}
|
||||
|
||||
if (spake_message_sent_ && !verification_hash_present) {
|
||||
LOG(WARNING) << "Didn't receive <verification-hash> when expected.";
|
||||
state_ = REJECTED;
|
||||
rejection_reason_ = RejectionReason::PROTOCOL_ERROR;
|
||||
rejection_details_ =
|
||||
RejectionDetails("Didn't receive <verification-hash> when expected.");
|
||||
return;
|
||||
}
|
||||
|
||||
@ -250,6 +266,7 @@ void Spake2Authenticator::ProcessMessageInternal(
|
||||
base::as_byte_span(expected_verification_hash_))) {
|
||||
state_ = REJECTED;
|
||||
rejection_reason_ = RejectionReason::INVALID_CREDENTIALS;
|
||||
rejection_details_ = RejectionDetails("Verification hash mismatched.");
|
||||
return;
|
||||
}
|
||||
state_ = ACCEPTED;
|
||||
|
@ -51,6 +51,7 @@ class Spake2Authenticator : public Authenticator {
|
||||
State state() const override;
|
||||
bool started() const override;
|
||||
RejectionReason rejection_reason() const override;
|
||||
RejectionDetails rejection_details() const override;
|
||||
void ProcessMessage(const jingle_xmpp::XmlElement* message,
|
||||
base::OnceClosure resume_callback) override;
|
||||
std::unique_ptr<jingle_xmpp::XmlElement> GetNextMessage() override;
|
||||
@ -91,6 +92,7 @@ class Spake2Authenticator : public Authenticator {
|
||||
State state_;
|
||||
bool started_ = false;
|
||||
RejectionReason rejection_reason_ = RejectionReason::INVALID_CREDENTIALS;
|
||||
RejectionDetails rejection_details_;
|
||||
std::string local_spake_message_;
|
||||
bool spake_message_sent_ = false;
|
||||
std::string outgoing_verification_hash_;
|
||||
|
@ -11,6 +11,7 @@
|
||||
#include "base/check_op.h"
|
||||
#include "base/functional/bind.h"
|
||||
#include "base/functional/callback.h"
|
||||
#include "base/location.h"
|
||||
#include "base/memory/ref_counted.h"
|
||||
#include "base/memory/weak_ptr.h"
|
||||
#include "remoting/protocol/authenticator.h"
|
||||
@ -56,6 +57,11 @@ Authenticator::RejectionReason ValidatingAuthenticator::rejection_reason()
|
||||
return rejection_reason_;
|
||||
}
|
||||
|
||||
Authenticator::RejectionDetails ValidatingAuthenticator::rejection_details()
|
||||
const {
|
||||
return rejection_details_;
|
||||
}
|
||||
|
||||
const std::string& ValidatingAuthenticator::GetAuthKey() const {
|
||||
return current_authenticator_->GetAuthKey();
|
||||
}
|
||||
@ -128,6 +134,7 @@ void ValidatingAuthenticator::OnValidateComplete(base::OnceClosure callback,
|
||||
}
|
||||
|
||||
state_ = Authenticator::REJECTED;
|
||||
rejection_details_ = RejectionDetails("Validation failed.");
|
||||
|
||||
// Clear the pending message so the signal strategy will generate a new
|
||||
// SESSION_REJECT message in response to this state change.
|
||||
@ -143,6 +150,7 @@ void ValidatingAuthenticator::UpdateState(base::OnceClosure resume_callback) {
|
||||
state_ = current_authenticator_->state();
|
||||
if (state_ == REJECTED) {
|
||||
rejection_reason_ = current_authenticator_->rejection_reason();
|
||||
rejection_details_ = current_authenticator_->rejection_details();
|
||||
} else if (state_ == MESSAGE_READY) {
|
||||
DCHECK(!pending_auth_message_);
|
||||
pending_auth_message_ = current_authenticator_->GetNextMessage();
|
||||
@ -164,6 +172,7 @@ void ValidatingAuthenticator::NotifyStateChangeAfterAccepted() {
|
||||
state_ = current_authenticator_->state();
|
||||
if (state_ == REJECTED) {
|
||||
rejection_reason_ = current_authenticator_->rejection_reason();
|
||||
rejection_details_ = current_authenticator_->rejection_details();
|
||||
}
|
||||
Authenticator::NotifyStateChangeAfterAccepted();
|
||||
}
|
||||
|
@ -50,6 +50,7 @@ class ValidatingAuthenticator : public Authenticator {
|
||||
State state() const override;
|
||||
bool started() const override;
|
||||
RejectionReason rejection_reason() const override;
|
||||
RejectionDetails rejection_details() const override;
|
||||
const std::string& GetAuthKey() const override;
|
||||
const SessionPolicies* GetSessionPolicies() const override;
|
||||
std::unique_ptr<ChannelAuthenticator> CreateChannelAuthenticator()
|
||||
@ -81,6 +82,7 @@ class ValidatingAuthenticator : public Authenticator {
|
||||
// Returns the rejection reason. Can be called only when in REJECTED state.
|
||||
RejectionReason rejection_reason_ =
|
||||
Authenticator::RejectionReason::INVALID_CREDENTIALS;
|
||||
RejectionDetails rejection_details_;
|
||||
|
||||
std::unique_ptr<Authenticator> current_authenticator_;
|
||||
|
||||
|
@ -84,6 +84,7 @@ void WebrtcConnectionToClient::Disconnect(ErrorCode error) {
|
||||
|
||||
// This should trigger OnConnectionClosed() event and this object
|
||||
// may be destroyed as the result.
|
||||
// TODO: crbug.com/382334458 - Pass WebRTC error details to Close().
|
||||
session_->Close(error);
|
||||
}
|
||||
|
||||
|
Reference in New Issue
Block a user