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

Jump to bottom

Fixed the SHM segment smashing while reading with security <2.0.x> [10165] #1644

Merged
merged 7 commits into from
Dec 18, 2020
Merged

Fixed the SHM segment smashing while reading with security <2.0.x> [10165] #1644

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f9577ea
Refs #9475: Fixed encrypted RTPS message and submessage decoding
rsanchez15 Dec 16, 2020
f1be4b6
Refs #9475: Added secure communication test for encrypted messages
rsanchez15 Dec 16, 2020
67c04ee
Refs #9475: Uncrustify
rsanchez15 Dec 17, 2020
de40179
Refs #9475: Added buffer to process decoded submessages
rsanchez15 Dec 18, 2020
cda01c2
Refs #9475: Added more comments
rsanchez15 Dec 18, 2020
f5e5948
Refs #9475: Applied suggested changes
rsanchez15 Dec 18, 2020
26f400e
Refs #9475: Uncrustify
rsanchez15 Dec 18, 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
9 changes: 6 additions & 3 deletions include/fastdds/rtps/messages/MessageReceiver.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class MessageReceiver

std::mutex mtx_;
std::vector<RTPSWriter*> associated_writers_;
std::unordered_map<EntityId_t, std::vector<RTPSReader*> > associated_readers_;
std::unordered_map<EntityId_t, std::vector<RTPSReader*>> associated_readers_;

RTPSParticipantImpl* participant_;
//!Protocol version of the message
Expand All @@ -89,8 +89,11 @@ class MessageReceiver
Time_t timestamp_;

#if HAVE_SECURITY
//!Buffer to process the decoded RTPS message
CDRMessage_t crypto_msg_;
#endif
//!Buffer to process each decoded RTPS sub-message
CDRMessage_t crypto_submsg_;
#endif // if HAVE_SECURITY

//!Reset the MessageReceiver to process a new message.
void reset();
Expand Down Expand Up @@ -183,5 +186,5 @@ class MessageReceiver
} /* namespace fastrtps */
} /* namespace eprosima */

#endif
#endif // ifndef DOXYGEN_SHOULD_SKIP_THIS_PUBLIC
#endif /* _FASTDDS_RTPS_MESSAGERECEIVER_H_ */
60 changes: 32 additions & 28 deletions src/cpp/rtps/messages/MessageReceiver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ MessageReceiver::MessageReceiver(
, timestamp_(c_TimeInvalid)
#if HAVE_SECURITY
, crypto_msg_(participant->is_secure() ? rec_buffer_size : 0)
#endif
, crypto_submsg_(participant->is_secure() ? rec_buffer_size : 0)
#endif // if HAVE_SECURITY
{
(void)rec_buffer_size;
logInfo(RTPS_MSG_IN, "Created with CDRMessage of size: " << rec_buffer_size);
Expand Down Expand Up @@ -197,10 +198,13 @@ void MessageReceiver::processCDRMsg(

if (decode_ret == 0)
{
// Swap
std::swap(msg, auxiliary_buffer);
// The original CDRMessage buffer (msg) now points to the proprietary temporary buffer crypto_msg_.
// The auxiliary buffer now points to the propietary temporary buffer crypto_submsg_.
// This way each decoded sub-message will be processed using the crypto_submsg_ buffer.
msg = auxiliary_buffer;
auxiliary_buffer = &crypto_submsg_;
}
#endif
#endif // if HAVE_SECURITY

// Loop until there are no more submessages
bool valid;
Expand All @@ -223,7 +227,7 @@ void MessageReceiver::processCDRMsg(
{
submessage = auxiliary_buffer;
}
#endif
#endif // if HAVE_SECURITY

//First 4 bytes must contain: ID | flags | octets to next header
if (!readSubmessageHeader(submessage, &submsgh))
Expand Down Expand Up @@ -426,7 +430,7 @@ bool MessageReceiver::readSubmessageHeader(
return false;
}

if ( (length == 0) && (smh->submessageId != INFO_TS) && (smh->submessageId != PAD) )
if ((length == 0) && (smh->submessageId != INFO_TS) && (smh->submessageId != PAD))
{
// THIS IS THE LAST SUBMESSAGE
smh->submessageLength = msg->length - msg->pos;
Expand Down Expand Up @@ -599,7 +603,7 @@ bool MessageReceiver::proc_Submsg_Data(

if (inlineQosFlag)
{
if (!ParameterList::updateCacheChangeFromInlineQos(ch, msg, inlineQosSize) )
if (!ParameterList::updateCacheChangeFromInlineQos(ch, msg, inlineQosSize))
{
logInfo(RTPS_MSG_IN, IDSTRING "SubMessage Data ERROR, Inline Qos ParameterList error");
return false;
Expand Down Expand Up @@ -644,7 +648,7 @@ bool MessageReceiver::proc_Submsg_Data(
else
{
logWarning(RTPS_MSG_IN, IDSTRING "Ignoring Serialized Payload for too large key-only data (" <<
payload_size << ")");
payload_size << ")");
}
msg->pos += payload_size;
}
Expand All @@ -657,14 +661,14 @@ bool MessageReceiver::proc_Submsg_Data(
}

logInfo(RTPS_MSG_IN, IDSTRING "from Writer " << ch.writerGUID << "; possible RTPSReader entities: " <<
associated_readers_.size());
associated_readers_.size());

//Look for the correct reader to add the change
findAllReaders(readerID,
[&ch] (RTPSReader* reader)
{
reader->processDataMsg(&ch);
});
findAllReaders(readerID,
[&ch](RTPSReader* reader)
{
reader->processDataMsg(&ch);
});

//TODO(Ricardo) If a exception is thrown (ex, by fastcdr), this line is not executed -> segmentation fault
ch.serializedPayload.data = nullptr;
Expand Down Expand Up @@ -833,14 +837,14 @@ bool MessageReceiver::proc_Submsg_DataFrag(

//FIXME: DO SOMETHING WITH PARAMETERLIST CREATED.
logInfo(RTPS_MSG_IN, IDSTRING "from Writer " << ch.writerGUID << "; possible RTPSReader entities: " <<
associated_readers_.size());
associated_readers_.size());

//Look for the correct reader to add the change
findAllReaders(readerID,
[&ch, sampleSize, fragmentStartingNum, fragmentsInSubmessage] (RTPSReader* reader)
{
reader->processDataFragMsg(&ch, sampleSize, fragmentStartingNum, fragmentsInSubmessage);
});
[&ch, sampleSize, fragmentStartingNum, fragmentsInSubmessage](RTPSReader* reader)
{
reader->processDataFragMsg(&ch, sampleSize, fragmentStartingNum, fragmentsInSubmessage);
});

ch.serializedPayload.data = nullptr;

Expand Down Expand Up @@ -888,10 +892,10 @@ bool MessageReceiver::proc_Submsg_Heartbeat(
std::lock_guard<std::mutex> guard(mtx_);
//Look for the correct reader and writers:
findAllReaders(readerGUID.entityId,
[&writerGUID, &HBCount, &firstSN, &lastSN, finalFlag, livelinessFlag] (RTPSReader* reader)
{
reader->processHeartbeatMsg(writerGUID, HBCount, firstSN, lastSN, finalFlag, livelinessFlag);
});
[&writerGUID, &HBCount, &firstSN, &lastSN, finalFlag, livelinessFlag](RTPSReader* reader)
{
reader->processHeartbeatMsg(writerGUID, HBCount, firstSN, lastSN, finalFlag, livelinessFlag);
});

return true;
}
Expand Down Expand Up @@ -973,10 +977,10 @@ bool MessageReceiver::proc_Submsg_Gap(

std::lock_guard<std::mutex> guard(mtx_);
findAllReaders(readerGUID.entityId,
[&writerGUID, &gapStart, &gapList] (RTPSReader* reader)
{
reader->processGapMsg(writerGUID, gapStart, gapList);
});
[&writerGUID, &gapStart, &gapList](RTPSReader* reader)
{
reader->processGapMsg(writerGUID, gapStart, gapList);
});

return true;
}
Expand Down Expand Up @@ -1161,6 +1165,6 @@ bool MessageReceiver::proc_Submsg_HeartbeatFrag(
return true;
}

}
} /* namespace rtps */
MiguelCompany marked this conversation as resolved.
Show resolved Hide resolved
} /* namespace fastrtps */
} /* namespace eprosima */
65 changes: 65 additions & 0 deletions test/communication/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,20 @@ if(NOT ((MSVC OR MSVC_IDE) AND EPROSIMA_INSTALLER) AND fastcdr_FOUND)
${CMAKE_CURRENT_BINARY_DIR}/liveliness_assertion.120.xml COPYONLY)
configure_file(${CMAKE_CURRENT_SOURCE_DIR}/liveliness_assertion.360.xml
${CMAKE_CURRENT_BINARY_DIR}/liveliness_assertion.360.xml COPYONLY)
configure_file(${CMAKE_CURRENT_SOURCE_DIR}/multiple_subs_secure_crypto_communication.py
${CMAKE_CURRENT_BINARY_DIR}/multiple_subs_secure_crypto_communication.py COPYONLY)
configure_file(${CMAKE_CURRENT_SOURCE_DIR}/secure_msg_crypto_besteffort_pub.xml
${CMAKE_CURRENT_BINARY_DIR}/secure_msg_crypto_besteffort_pub.xml COPYONLY)
configure_file(${CMAKE_CURRENT_SOURCE_DIR}/secure_msg_crypto_besteffort_sub.xml
${CMAKE_CURRENT_BINARY_DIR}/secure_msg_crypto_besteffort_sub.xml COPYONLY)
configure_file(${CMAKE_CURRENT_SOURCE_DIR}/secure_msg_submsg_crypto_besteffort_pub.xml
${CMAKE_CURRENT_BINARY_DIR}/secure_msg_submsg_crypto_besteffort_pub.xml COPYONLY)
configure_file(${CMAKE_CURRENT_SOURCE_DIR}/secure_msg_submsg_crypto_besteffort_sub.xml
${CMAKE_CURRENT_BINARY_DIR}/secure_msg_submsg_crypto_besteffort_sub.xml COPYONLY)
configure_file(${CMAKE_CURRENT_SOURCE_DIR}/secure_submsg_crypto_besteffort_pub.xml
${CMAKE_CURRENT_BINARY_DIR}/secure_submsg_crypto_besteffort_pub.xml COPYONLY)
configure_file(${CMAKE_CURRENT_SOURCE_DIR}/secure_submsg_crypto_besteffort_sub.xml
${CMAKE_CURRENT_BINARY_DIR}/secure_submsg_crypto_besteffort_sub.xml COPYONLY)
if(SECURITY)
configure_file(${PROJECT_SOURCE_DIR}/test/certs/maincacert.pem
${CMAKE_CURRENT_BINARY_DIR}/maincacert.pem COPYONLY)
Expand Down Expand Up @@ -214,6 +228,57 @@ if(NOT ((MSVC OR MSVC_IDE) AND EPROSIMA_INSTALLER) AND fastcdr_FOUND)
set_property(TEST SimpleCommunicationSecureBestEffort APPEND PROPERTY ENVIRONMENT
"PATH=$<TARGET_FILE_DIR:${PROJECT_NAME}>\\;$<TARGET_FILE_DIR:fastcdr>\\;${WIN_PATH}")
endif()

add_test(NAME SimpleCommunicationSecureMsgCryptoBestEffort
COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_BINARY_DIR}/multiple_subs_secure_crypto_communication.py
--pub $<TARGET_FILE:SimpleCommunicationPublisher>
--xml-pub secure_msg_crypto_besteffort_pub.xml
--sub $<TARGET_FILE:SimpleCommunicationSubscriber>
--xml-sub secure_msg_crypto_besteffort_sub.xml
--samples 10 --wait 2 --n-subs 5)

# Set test with label NoMemoryCheck
set_property(TEST SimpleCommunicationSecureMsgCryptoBestEffort PROPERTY LABELS "NoMemoryCheck")

if(WIN32)
string(REPLACE ";" "\\;" WIN_PATH "$ENV{PATH}")
set_property(TEST SimpleCommunicationSecureMsgCryptoBestEffort APPEND PROPERTY ENVIRONMENT
"PATH=$<TARGET_FILE_DIR:${PROJECT_NAME}>\\;$<TARGET_FILE_DIR:fastcdr>\\;${WIN_PATH}")
endif()

add_test(NAME SimpleCommunicationSecureMsgSubmsgCryptoBestEffort
COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_BINARY_DIR}/multiple_subs_secure_crypto_communication.py
--pub $<TARGET_FILE:SimpleCommunicationPublisher>
--xml-pub secure_msg_submsg_crypto_besteffort_pub.xml
--sub $<TARGET_FILE:SimpleCommunicationSubscriber>
--xml-sub secure_msg_submsg_crypto_besteffort_sub.xml
--samples 10 --wait 2 --n-subs 5)

# Set test with label NoMemoryCheck
set_property(TEST SimpleCommunicationSecureMsgSubmsgCryptoBestEffort PROPERTY LABELS "NoMemoryCheck")

if(WIN32)
string(REPLACE ";" "\\;" WIN_PATH "$ENV{PATH}")
set_property(TEST SimpleCommunicationSecureMsgSubmsgCryptoBestEffort APPEND PROPERTY ENVIRONMENT
"PATH=$<TARGET_FILE_DIR:${PROJECT_NAME}>\\;$<TARGET_FILE_DIR:fastcdr>\\;${WIN_PATH}")
endif()

add_test(NAME SimpleCommunicationSecureSubmsgCryptoBestEffort
COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_BINARY_DIR}/multiple_subs_secure_crypto_communication.py
--pub $<TARGET_FILE:SimpleCommunicationPublisher>
--xml-pub secure_submsg_crypto_besteffort_pub.xml
--sub $<TARGET_FILE:SimpleCommunicationSubscriber>
--xml-sub secure_submsg_crypto_besteffort_sub.xml
--samples 10 --wait 2 --n-subs 5)

# Set test with label NoMemoryCheck
set_property(TEST SimpleCommunicationSecureSubmsgCryptoBestEffort PROPERTY LABELS "NoMemoryCheck")

if(WIN32)
string(REPLACE ";" "\\;" WIN_PATH "$ENV{PATH}")
set_property(TEST SimpleCommunicationSecureSubmsgCryptoBestEffort APPEND PROPERTY ENVIRONMENT
"PATH=$<TARGET_FILE_DIR:${PROJECT_NAME}>\\;$<TARGET_FILE_DIR:fastcdr>\\;${WIN_PATH}")
endif()
endif()

add_test(NAME LivelinessAssertion
Expand Down
Loading