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

Fix Handshake Flow issues #3441

Merged
merged 3 commits into from
Jun 18, 2020
Merged

Fix Handshake Flow issues #3441

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4378f15
Fix issue with External PTU timeout
jacobkeeler Jun 16, 2020
5218b90
Add error code check in ProcessInternalError
jacobkeeler Jun 16, 2020
56465b0
Fix style
Jack-Byrne Jun 17, 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
2 changes: 1 addition & 1 deletion src/components/policy/policy_external/src/policy_manager_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1368,7 +1368,7 @@ bool PolicyManagerImpl::IsAllowedRetryCountExceeded() const {
LOG4CXX_AUTO_TRACE(logger_);
sync_primitives::AutoLock auto_lock(retry_sequence_lock_);

return retry_sequence_index_ > retry_sequence_seconds_.size();
return retry_sequence_index_ >= retry_sequence_seconds_.size();
}

void PolicyManagerImpl::IncrementRetryIndex() {
Expand Down
4 changes: 2 additions & 2 deletions src/components/security_manager/include/security_manager/security_manager_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,12 +241,12 @@ class SecurityManagerImpl : public SecurityManager,
* \brief Parse SecurityMessage as HandshakeData request
* \param inMessage SecurityMessage with binary data of handshake
*/
bool ProccessHandshakeData(const SecurityMessage& inMessage);
bool ProcessHandshakeData(const SecurityMessage& inMessage);
/**
* \brief Parse InternalError from mobile side
* \param inMessage SecurityMessage with binary data of handshake
*/
bool ProccessInternalError(const SecurityMessage& inMessage);
bool ProcessInternalError(const SecurityMessage& inMessage);

/**
* \brief Sends security query
Expand Down
40 changes: 22 additions & 18 deletions src/components/security_manager/src/security_manager_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,12 @@ void SecurityManagerImpl::Handle(const SecurityMessage message) {
}
switch (message->get_header().query_id) {
case SecurityQuery::SEND_HANDSHAKE_DATA:
if (!ProccessHandshakeData(message)) {
LOG4CXX_ERROR(logger_, "Proccess HandshakeData failed");
if (!ProcessHandshakeData(message)) {
LOG4CXX_ERROR(logger_, "Process HandshakeData failed");
}
break;
case SecurityQuery::SEND_INTERNAL_ERROR:
if (!ProccessInternalError(message)) {
if (!ProcessInternalError(message)) {
LOG4CXX_ERROR(logger_, "Processing income InternalError failed");
}
break;
Expand Down Expand Up @@ -515,7 +515,7 @@ bool SecurityManagerImpl::IsPolicyCertificateDataEmpty() {
return false;
}

bool SecurityManagerImpl::ProccessHandshakeData(
bool SecurityManagerImpl::ProcessHandshakeData(
const SecurityMessage& inMessage) {
LOG4CXX_INFO(logger_, "SendHandshakeData processing");
DCHECK(inMessage);
Expand Down Expand Up @@ -556,11 +556,11 @@ bool SecurityManagerImpl::ProccessHandshakeData(
&out_data_size);
if (handshake_result == SSLContext::Handshake_Result_AbnormalFail) {
// Do not return handshake data on AbnormalFail or null returned values
const std::string erorr_text(sslContext->LastError());
const std::string error_text(sslContext->LastError());
LOG4CXX_ERROR(logger_,
"SendHandshakeData: Handshake failed: " << erorr_text);
"SendHandshakeData: Handshake failed: " << error_text);
SendInternalError(
connection_key, ERROR_SSL_INVALID_DATA, erorr_text, seqNumber);
connection_key, ERROR_SSL_INVALID_DATA, error_text, seqNumber);
NotifyListenersOnHandshakeDone(connection_key,
SSLContext::Handshake_Result_Fail);
// no handshake data to send
Expand All @@ -584,25 +584,29 @@ bool SecurityManagerImpl::ProccessHandshakeData(
return true;
}

bool SecurityManagerImpl::ProccessInternalError(
bool SecurityManagerImpl::ProcessInternalError(
const SecurityMessage& inMessage) {
LOG4CXX_INFO(logger_,
"Received InternalError with Json message"
<< inMessage->get_json_message());
Json::Value root;
std::string str = inMessage->get_json_message();
const uint32_t connection_key = inMessage->get_connection_key();
LOG4CXX_INFO(logger_, "Received InternalError with Json message" << str);
Json::Value root;
utils::JsonReader reader;

if (!reader.parse(str, &root)) {
LOG4CXX_DEBUG(logger_, "Json parsing fails.");
return false;
}
uint8_t id = root[kErrId].asInt();
LOG4CXX_DEBUG(logger_,
"Received InternalError id "
<< root[kErrId].asString()
<< ", text: " << root[kErrText].asString());
"Received InternalError id " << std::to_string(id) << ", text: "
<< root[kErrText].asString());
if (ERROR_SSL_INVALID_DATA == id || ERROR_NOT_SUPPORTED == id) {
NotifyListenersOnHandshakeDone(connection_key,
SSLContext::Handshake_Result_Fail);
}
return true;
}

void SecurityManagerImpl::SendHandshakeBinData(const uint32_t connection_key,
const uint8_t* const data,
const size_t data_size,
Expand All @@ -619,11 +623,11 @@ void SecurityManagerImpl::SendHandshakeBinData(const uint32_t connection_key,

void SecurityManagerImpl::SendInternalError(const uint32_t connection_key,
const uint8_t& error_id,
const std::string& erorr_text,
const std::string& error_text,
const uint32_t seq_number) {
Json::Value value;
value[kErrId] = error_id;
value[kErrText] = erorr_text;
value[kErrText] = error_text;
const std::string error_str = value.toStyledString();
SecurityQuery::QueryHeader header(
SecurityQuery::NOTIFICATION,
Expand All @@ -642,7 +646,7 @@ void SecurityManagerImpl::SendInternalError(const uint32_t connection_key,
SendQuery(query, connection_key);
LOG4CXX_DEBUG(logger_,
"Sent Internal error id " << static_cast<int>(error_id)
<< " : \"" << erorr_text << "\".");
<< " : \"" << error_text << "\".");
}

void SecurityManagerImpl::SendQuery(const SecurityQuery& query,
Expand Down
11 changes: 5 additions & 6 deletions src/components/security_manager/test/security_manager_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ TEST_F(SecurityManagerTest, StartHandshake_SSLInternalError) {
* Shall send InternallError on
* getting SEND_HANDSHAKE_DATA with NULL data
*/
TEST_F(SecurityManagerTest, ProccessHandshakeData_WrongDataSize) {
TEST_F(SecurityManagerTest, ProcessHandshakeData_WrongDataSize) {
SetMockCryptoManager();
uint32_t connection_id = 0;
uint8_t session_id = 0;
Expand Down Expand Up @@ -600,8 +600,7 @@ TEST_F(SecurityManagerTest, ProccessHandshakeData_WrongDataSize) {
* getting SEND_HANDSHAKE_DATA from mobile side
* for service which is not protected
*/
TEST_F(SecurityManagerTest,
DISABLED_ProccessHandshakeData_ServiceNotProtected) {
TEST_F(SecurityManagerTest, DISABLED_ProcessHandshakeData_ServiceNotProtected) {
SetMockCryptoManager();
// Expect InternalError with ERROR_ID
uint32_t connection_id = 0;
Expand Down Expand Up @@ -649,7 +648,7 @@ TEST_F(SecurityManagerTest,
* SEND_HANDSHAKE_DATA from mobile side with invalid handshake
* data (DoHandshakeStep return NULL pointer)
*/
TEST_F(SecurityManagerTest, ProccessHandshakeData_InvalidData) {
TEST_F(SecurityManagerTest, ProcessHandshakeData_InvalidData) {
SetMockCryptoManager();

// Count handshake calls
Expand Down Expand Up @@ -732,7 +731,7 @@ TEST_F(SecurityManagerTest, ProccessHandshakeData_InvalidData) {
* Shall send HandshakeData on getting SEND_HANDSHAKE_DATA from mobile side
* with correct handshake data Check Fail and sussecc states
*/
TEST_F(SecurityManagerTest, ProccessHandshakeData_Answer) {
TEST_F(SecurityManagerTest, ProcessHandshakeData_Answer) {
SetMockCryptoManager();
// Count handshake calls
const int handshake_emulates = 2;
Expand Down Expand Up @@ -808,7 +807,7 @@ TEST_F(SecurityManagerTest, ProccessHandshakeData_Answer) {
* and return handshake data
* Check Fail and sussecc states
*/
TEST_F(SecurityManagerTest, ProccessHandshakeData_HandshakeFinished) {
TEST_F(SecurityManagerTest, ProcessHandshakeData_HandshakeFinished) {
SetMockCryptoManager();
// Count handshake calls
const int handshake_emulates = 6;
Expand Down