Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/Add reason param to All protocol NAKs #3462

Merged
merged 19 commits into from
Sep 4, 2020
Merged

Feature/Add reason param to All protocol NAKs #3462

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
a050e33
Add reason param to StartService and EndService NAK payloads
ShobhitAd Jul 20, 2020
ba8ffec
Update the default protocol version
ShobhitAd Jul 20, 2020
2ca4da8
Fix unit tests
ShobhitAd Jul 21, 2020
f5d503b
Implement minor version check for reason param
ShobhitAd Jul 27, 2020
f1fc114
Fix unit tests
ShobhitAd Jul 27, 2020
b87d3a1
Apply suggestions from code review
ShobhitAd Aug 12, 2020
c85e87f
Make semantic versions const and fix handshake NAK reason messages
ShobhitAd Aug 12, 2020
6d9f2a8
Make reason a required param
ShobhitAd Aug 13, 2020
c785dfa
Add error check for protocolversionused call
ShobhitAd Aug 13, 2020
e8fa2af
Use full version for reason param version check
ShobhitAd Aug 13, 2020
4c55f0b
Get protocol NAK reason from OnSessionEndedCallback
ShobhitAd Aug 19, 2020
c289ecf
Fix failing unit tests
ShobhitAd Aug 19, 2020
350a638
Remove unnecessary condition from protected/unprotected reason check
ShobhitAd Aug 19, 2020
e64109d
Add unit tests for NACK reason param
ShobhitAd Aug 21, 2020
6b78940
Add unit test for EndServiceNAK with reason param
ShobhitAd Aug 21, 2020
3b974c8
Fix calltimes in StartSession_NACKReason_SessionObserverReject
ShobhitAd Aug 21, 2020
c6805cd
Merge branch 'develop' into feature/protocol_nak_reason_param
ShobhitAd Sep 1, 2020
1a9c6f0
Get reason string for service start failed
ShobhitAd Sep 2, 2020
945a34e
Add more details to the invalid cert nak reason
ShobhitAd Sep 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions src/components/protocol_handler/include/protocol_handler/protocol_handler_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,12 +373,13 @@ class ProtocolHandlerImpl
* \param session_id ID of session to be sent to mobile application
* \param protocol_version Version of protocol used for communication
* \param service_type Type of session: RPC or BULK Data. RPC by default
* \param reason String stating the reason for the rejecting the start service
*/
void SendStartSessionNAck(ConnectionID connection_id,
uint8_t session_id,
uint8_t protocol_version,
uint8_t service_type,
const std::string reason = std::string());
const std::string reason);

/**
* \brief Sends fail of starting session to mobile application
Expand All @@ -387,13 +388,14 @@ class ProtocolHandlerImpl
* \param protocol_version Version of protocol used for communication
* \param service_type Type of session: RPC or BULK Data. RPC by default
* \param rejected_params List of rejected params to send in payload
* \param reason String stating the reason for the rejecting the start service
*/
void SendStartSessionNAck(ConnectionID connection_id,
uint8_t session_id,
uint8_t protocol_version,
uint8_t service_type,
std::vector<std::string>& rejectedParams,
const std::string reason = std::string());
const std::string reason);

/**
* \brief Sends acknowledgement of end session/service to mobile application
Expand All @@ -418,12 +420,13 @@ class ProtocolHandlerImpl
* \param session_id ID of session ment to be ended
* \param protocol_version Version of protocol used for communication
* \param service_type Type of session: RPC or BULK Data. RPC by default
* \param reason String stating the reason for the rejecting the end service
*/
void SendEndSessionNAck(ConnectionID connection_id,
uint32_t session_id,
uint8_t protocol_version,
uint8_t service_type,
const std::string reason = std::string());
const std::string reason);
/**
* \brief Sends fail of ending session to mobile application (variant for
* Protocol v5)
Expand All @@ -433,13 +436,14 @@ class ProtocolHandlerImpl
* \param protocol_version Version of protocol used for communication
* \param service_type Type of session: RPC or BULK Data. RPC by default
* \param rejected_params List of rejected params to send in payload
* \param reason String stating the reason for the rejecting the end service
*/
void SendEndSessionNAck(ConnectionID connection_id,
uint32_t session_id,
uint8_t protocol_version,
uint8_t service_type,
std::vector<std::string>& rejected_params,
const std::string reason = std::string());
const std::string reason);

SessionObserver& get_session_observer() OVERRIDE;

Expand Down
33 changes: 14 additions & 19 deletions src/components/protocol_handler/src/handshake_handler.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,11 +208,10 @@ void HandshakeHandler::ProcessSuccessfulHandshake(const uint32_t connection_key,
if (can_be_protected && is_already_protected) {
return "Service is already protected";
} else if (!can_be_protected && is_already_protected) {
return "Service can be protected but is not already protected";
} else if (!can_be_protected && !is_already_protected) {
return "Service cannot be protected but is already protected";
} else { // !can_be_protected && !is_already_protected
return "Service cannot be protected";
}
return std::string();
};
protocol_handler_.SendStartSessionNAck(
context_.connection_id_,
Expand Down Expand Up @@ -257,27 +256,23 @@ void HandshakeHandler::ProcessFailedHandshake(BsonObject& params,
service_status_update_handler_.OnServiceUpdate(
this->connection_key(), context_.service_type_, service_status);

auto gen_reason_msg =
[](const ServiceStatus service_status) -> std::string {
switch (service_status) {
case ServiceStatus::PTU_FAILED:
return "Policy Table Update failed";
case ServiceStatus::CERT_INVALID:
return "Invalid certificate";
case ServiceStatus::INVALID_TIME:
return "Failed to get system time";
case ServiceStatus::UNSECURE_START_FAILED:
return "Unsecure start failed";
default:
return "Unknown cause of failure";
}
};
std::string reason_msg =
(service_status == ServiceStatus::PTU_FAILED)
? "Policy Table Update failed"
: (service_status == ServiceStatus::CERT_INVALID)
? "Invalid certificate"
: (service_status == ServiceStatus::INVALID_TIME)
? "Failed to get system time"
: (service_status ==
ServiceStatus::UNSECURE_START_FAILED)
? "Unsecure start failed"
: "Unknown cause of failure";

protocol_handler_.SendStartSessionNAck(context_.connection_id_,
context_.new_session_id_,
protocol_version_,
context_.service_type_,
gen_reason_msg(service_status));
reason_msg);
}
}

Expand Down
65 changes: 33 additions & 32 deletions src/components/protocol_handler/src/protocol_handler_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,10 @@ std::string ConvertPacketDataToString(const uint8_t* data,

const size_t kStackSize = 131072;

utils::SemanticVersion default_protocol_version(5, 3, 0);
utils::SemanticVersion min_multiple_transports_version(5, 1, 0);
utils::SemanticVersion min_cloud_app_version(5, 2, 0);
utils::SemanticVersion min_reason_param_version(5, 3, 0);
const utils::SemanticVersion default_protocol_version(5, 3, 0);
const utils::SemanticVersion min_multiple_transports_version(5, 1, 0);
const utils::SemanticVersion min_cloud_app_version(5, 2, 0);
const utils::SemanticVersion min_reason_param_version(5, 3, 0);

ProtocolHandlerImpl::ProtocolHandlerImpl(
const ProtocolHandlerSettings& settings,
Expand Down Expand Up @@ -313,13 +313,14 @@ void ProtocolHandlerImpl::SendStartSessionAck(
&params, strings::hash_id)));

// Minimum protocol version supported by both
utils::SemanticVersion* min_version =
const utils::SemanticVersion& min_version =
(full_version.major_version_ < PROTOCOL_VERSION_5)
? &default_protocol_version
: utils::SemanticVersion::min(full_version,
default_protocol_version);
? default_protocol_version
: ((full_version < default_protocol_version)
? full_version
: default_protocol_version);
char protocol_version_string[256];
strncpy(protocol_version_string, (*min_version).toString().c_str(), 255);
strncpy(protocol_version_string, (min_version).toString().c_str(), 255);

const bool protocol_ver_written = bson_object_put_string(
&params, strings::protocol_version, protocol_version_string);
Expand All @@ -336,7 +337,7 @@ void ProtocolHandlerImpl::SendStartSessionAck(
std::vector<std::string> secondaryTransports;
std::vector<int32_t> audioServiceTransports;
std::vector<int32_t> videoServiceTransports;
if (*min_version >= min_multiple_transports_version) {
if (min_version >= min_multiple_transports_version) {
if (ParseSecondaryTransportConfiguration(connection_id,
secondaryTransports,
audioServiceTransports,
Expand Down Expand Up @@ -417,7 +418,7 @@ void ProtocolHandlerImpl::SendStartSessionAck(

std::string policy_app_id =
connection_handler_.GetCloudAppID(connection_id);
if (*min_version >= min_cloud_app_version && !policy_app_id.empty()) {
if (min_version >= min_cloud_app_version && !policy_app_id.empty()) {
sync_primitives::AutoLock lock(auth_token_map_lock_);
auto it = auth_token_map_.find(policy_app_id);
if (it != auth_token_map_.end()) {
Expand Down Expand Up @@ -493,14 +494,13 @@ void ProtocolHandlerImpl::SendStartSessionNAck(
uint8_t maxProtocolVersion = SupportedSDLProtocolVersion();

utils::SemanticVersion full_version;
session_observer_.ProtocolVersionUsed(
connection_id, session_id, full_version);
const utils::SemanticVersion& min_version =
(full_version.major_version_ < PROTOCOL_VERSION_5)
? default_protocol_version
: ((full_version < default_protocol_version)
? full_version
: default_protocol_version);
if (!session_observer_.ProtocolVersionUsed(
connection_id, session_id, full_version)) {
LOG4CXX_WARN(logger_,
"Connection: " << connection_id << " and/or session: "
<< session_id << "no longer exist(s).");
return;
}

if (protocol_version >= PROTOCOL_VERSION_5 &&
maxProtocolVersion >= PROTOCOL_VERSION_5) {
Expand All @@ -518,7 +518,7 @@ void ProtocolHandlerImpl::SendStartSessionNAck(
bson_object_put_array(
&payloadObj, strings::rejected_params, &rejectedParamsArr);
}
if (!reason.empty() && min_version >= min_reason_param_version) {
if (!reason.empty() && full_version >= min_reason_param_version) {
bson_object_put_string(
&payloadObj, strings::reason, const_cast<char*>(reason.c_str()));
}
Expand All @@ -536,7 +536,7 @@ void ProtocolHandlerImpl::SendStartSessionNAck(
<< connection_id << " for service_type "
<< static_cast<int32_t>(service_type) << " session_id "
<< static_cast<int32_t>(session_id)
<< ((!reason.empty()) ? " reason \"" + reason + "\"" : ""));
<< (reason.empty() ? "" : " reason \"" + reason + "\""));
}

void ProtocolHandlerImpl::SendEndSessionNAck(ConnectionID connection_id,
Expand Down Expand Up @@ -576,14 +576,13 @@ void ProtocolHandlerImpl::SendEndSessionNAck(
uint8_t maxProtocolVersion = SupportedSDLProtocolVersion();

utils::SemanticVersion full_version;
session_observer_.ProtocolVersionUsed(
connection_id, session_id, full_version);
const utils::SemanticVersion& min_version =
(full_version.major_version_ < PROTOCOL_VERSION_5)
? default_protocol_version
: ((full_version < default_protocol_version)
? full_version
: default_protocol_version);
if (!session_observer_.ProtocolVersionUsed(
connection_id, session_id, full_version)) {
LOG4CXX_WARN(logger_,
"Connection: " << connection_id << " and/or session: "
<< session_id << "no longer exist(s).");
return;
}

if (protocol_version >= PROTOCOL_VERSION_5 &&
maxProtocolVersion >= PROTOCOL_VERSION_5) {
Expand All @@ -602,7 +601,7 @@ void ProtocolHandlerImpl::SendEndSessionNAck(
bson_object_put_array(
&payloadObj, strings::rejected_params, &rejectedParamsArr);
}
if (!reason.empty() && min_version >= min_reason_param_version) {
if (!reason.empty() && full_version >= min_reason_param_version) {
bson_object_put_string(
&payloadObj, strings::reason, const_cast<char*>(reason.c_str()));
}
Expand All @@ -620,7 +619,7 @@ void ProtocolHandlerImpl::SendEndSessionNAck(
<< connection_id << " for service_type "
<< static_cast<int32_t>(service_type) << " session_id "
<< static_cast<int32_t>(session_id)
<< ((!reason.empty()) ? " reason \"" + reason + "\"" : ""));
<< (reason.empty() ? "" : " reason \"" + reason + "\""));
}

SessionObserver& ProtocolHandlerImpl::get_session_observer() {
Expand Down Expand Up @@ -1645,7 +1644,9 @@ RESULT_CODE ProtocolHandlerImpl::HandleControlMessageEndSession(
SendEndSessionNAck(connection_id,
current_session_id,
packet.protocol_version(),
service_type);
service_type,
"Refused to end session for service of type " +
std::to_string(service_type));
}
}
return RESULT_OK;
Expand Down