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.

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

[16881] Secure DS test suite #62

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,7 @@ set_target_properties(${PROJECT_NAME} PROPERTIES RELWITHDEBINFO_POSTFIX -${PROJE
###############################################################################
# Testing
###############################################################################
option(SECURITY "Whether to build security test-suite" OFF)

enable_testing()
include(CTest)
Expand Down
11 changes: 11 additions & 0 deletions include/DiscoveryServerManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,9 @@ class DiscoveryServerManager
// Snapshops container
snapshots_list snapshots;

// Participant PropertiesPolicy
PropertySeq properties_;

volatile bool no_callbacks; // ongoing participant destruction
bool auto_shutdown; // close when event processing is finished?
bool enable_prefix_validation; // allow multiple servers share the same prefix? (only for testing purposes)
Expand Down Expand Up @@ -170,6 +173,13 @@ class DiscoveryServerManager
void saveSnapshots(
const std::string& file) const;

/**
* @brief: This method loads a set of properties into attributes
* @param [in] props_n: element containig the XML properties
*/
bool loadProperties(
tinyxml2::XMLElement* props_n);

// File where to save snapshots
std::string snapshots_output_file;
// validation required
Expand All @@ -186,6 +196,7 @@ class DiscoveryServerManager

DiscoveryServerManager(
const std::string& xml_file_path,
const std::string& props_file_path,
const bool shared_memory_off);
#if FASTRTPS_VERSION_MAJOR >= 2 && FASTRTPS_VERSION_MINOR >= 2
FASTDDS_DEPRECATED_UNTIL(3, "eprosima::discovery_server::DiscoveryServerManager(const std::set<std::string>& xml_snapshot_files,"
Expand Down
4 changes: 3 additions & 1 deletion resources/xsd/discovery-server.xsd
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@
<xs:element name="offsetd3" type="uint16Type" minOccurs="0"/>
</xs:all>
</xs:complexType>

<!-- IpAddressPort accepts the following expressions (RFC 3986 ):
- plain port: 7098
- ipv4+port: 192.168.2.1:7098
Expand Down Expand Up @@ -1076,6 +1076,8 @@
</xs:complexType>
</xs:element>

<xs:element name="properties" type="propertyVectorType" minOccurs="0"/>

<!-- DDS XML TYPES (FULL) -->
<!-- Primitive types -->
<xs:simpleType name="boolean">
Expand Down
84 changes: 84 additions & 0 deletions src/DiscoveryServerManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ const char* TYPES = "types";
const char* PUBLISHER = "publisher";
const char* SUBSCRIBER = "subscriber";
const char* TOPIC = "topic";
const char* PROPERTIES = "properties";
const char* PROPERTY = "property";
const char* VALUE = "value";
} // namespace DSxmlparser
} // namespace fastrtps
} // namespace eprosima
Expand All @@ -60,6 +63,7 @@ const std::chrono::seconds DiscoveryServerManager::last_snapshot_delay_ = std::c

DiscoveryServerManager::DiscoveryServerManager(
const std::string& xml_file_path,
const std::string& props_file_path,
const bool shared_memory_off)
: no_callbacks(false)
, auto_shutdown(true)
Expand Down Expand Up @@ -101,6 +105,35 @@ DiscoveryServerManager::DiscoveryServerManager(
// try load the enable_prefix_validation attribute
enable_prefix_validation = root->BoolAttribute(s_sPrefixValidation.c_str(), enable_prefix_validation);

//try load properties
if (!props_file_path.empty())
{
tinyxml2::XMLDocument props_doc;

if (tinyxml2::XMLError::XML_SUCCESS == props_doc.LoadFile(props_file_path.c_str()))
{
tinyxml2::XMLElement* root = props_doc.FirstChildElement(eprosima::fastrtps::DSxmlparser::PROPERTIES);
if (root != nullptr)
{
if (!loadProperties(root))
{
LOG_ERROR("Error loading PropertiesPolicy from properties file");
return;
}
}
else
{
LOG_ERROR("Error retrieving properties element from properties file");
return;
}
}
else
{
LOG_ERROR("Could not load properties file");
return;
}
}

for (auto child = doc.FirstChildElement(s_sDS.c_str());
child != nullptr; child = child->NextSiblingElement(s_sDS.c_str()))
{
Expand Down Expand Up @@ -713,6 +746,30 @@ void DiscoveryServerManager::loadProfiles(
}
}

bool DiscoveryServerManager::loadProperties(tinyxml2::XMLElement* props_n)
{
bool ret = true;
tinyxml2::XMLElement* prop = props_n->FirstChildElement(eprosima::fastrtps::DSxmlparser::PROPERTY);

for (;prop != nullptr; prop = prop->NextSiblingElement(eprosima::fastrtps::DSxmlparser::PROPERTY))
{
tinyxml2::XMLElement* name = prop->FirstChildElement(eprosima::fastrtps::DSxmlparser::NAME);
tinyxml2::XMLElement* value = prop->FirstChildElement(eprosima::fastrtps::DSxmlparser::VALUE);

if (nullptr != name && nullptr != value)
{
properties_.push_back({name->GetText(), value->GetText()});
}
else
{
LOG_ERROR("Missing name/value for property");
ret = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could be a little more specific on what the failure is with a LOG_ERROR (missing name/value)

}
}

return ret;
}

void DiscoveryServerManager::onTerminate()
{
{
Expand Down Expand Up @@ -956,6 +1013,15 @@ void DiscoveryServerManager::loadServer(
(void)b;
assert(b.discoveryProtocol == SERVER || b.discoveryProtocol == BACKUP);

// Extend Participant properties if applies
if (!properties_.empty())
{
for (auto& prop : properties_)
{
dpQOS.properties().properties().emplace_back(prop);
}
}

// Create the participant or the associated events
DelayedParticipantCreation event(creation_time, std::move(dpQOS), &DiscoveryServerManager::addServer);
if (creation_time == getTime())
Expand Down Expand Up @@ -1225,6 +1291,15 @@ void DiscoveryServerManager::loadClient(
dpQOS.transport().user_transports.push_back(udp_transport);
}

// Extend Participant properties if applies
if (!properties_.empty())
{
for (auto& prop : properties_)
{
dpQOS.properties().properties().emplace_back(prop);
}
}

GUID_t guid(dpQOS.wire_protocol().prefix, c_EntityId_RTPSParticipant);
DelayedParticipantDestruction* destruction_event = nullptr;
DelayedParticipantCreation* creation_event = nullptr;
Expand Down Expand Up @@ -1330,6 +1405,15 @@ void DiscoveryServerManager::loadSimple(
dpQOS.name() = name;
}

// Extend Participant properties if applies
if (!properties_.empty())
{
for (auto& prop : properties_)
{
dpQOS.properties().properties().emplace_back(prop);
}
}

GUID_t guid(dpQOS.wire_protocol().prefix, c_EntityId_RTPSParticipant);
DelayedParticipantDestruction* destruction_event = nullptr;
DelayedParticipantCreation* creation_event = nullptr;
Expand Down
4 changes: 4 additions & 0 deletions src/arguments.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ enum optionIndex
UNKNOWN,
HELP,
CONFIG_FILE,
PROPERTIES_FILE,
OUTPUT_FILE,
SHM
};
Expand All @@ -46,6 +47,9 @@ const option::Descriptor usage[] = {
{ CONFIG_FILE, 0, "c", "config-file", Arg::check_inp,
" -c \t--config-file Mandatory configuration file path\n"},

{ PROPERTIES_FILE, 0, "p", "props-file", Arg::check_inp,
" -p \t--props-file Optional participant properties configuration file path\n"},

{ OUTPUT_FILE, 0, "o", "output-file", Arg::check_inp,
" -o \t--output-file File to write result snapshots. If not specified"
" snapshots will be written in the file specified in the snapshot\n"},
Expand Down
18 changes: 17 additions & 1 deletion src/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,24 @@ int main(
// Load Default XML files
eprosima::fastrtps::xmlparser::XMLProfileManager::loadDefaultXMLFile();

// Load properties file path from arg
pOp = options[PROPERTIES_FILE];

std::string path_to_properties;

if ( nullptr != pOp )
{
if ( pOp->count() != 1)
{
cout << "Only one properties file can be specified." << endl;
return 1;
}

path_to_properties = pOp->arg;
}

// Create DiscoveryServerManager
DiscoveryServerManager manager(path_to_config, options[SHM]);
DiscoveryServerManager manager(path_to_config, path_to_properties, options[SHM]);
if (!manager.correctly_created())
{
return_code = 1;
Expand Down
25 changes: 25 additions & 0 deletions test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,31 @@ foreach(TEST IN LISTS TEST_LIST)
set_property(TEST ${TEST_NAME} PROPERTY LABELS xfail)
endif()

if(SECURITY)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this need a matching OPTION statement detailing the default value and what it does?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As it is the first time SECURITY is included in the repo, maybe it would be a good idea

unset(TEST_NAME)
set(TEST_NAME "discovery_server_test.${TEST}.SECURITY")
list(APPEND TEST_CASE_LIST ${TEST_NAME})
# Test without shared memory
add_test(NAME ${TEST_NAME}
COMMAND ${PYTHON_EXECUTABLE} ${RUN_TEST}
-e $<TARGET_FILE:${PROJECT_NAME}>
-p ${TESTS_PARAMS}
-f $<$<TARGET_EXISTS:fastdds::fast-discovery-server>:$<TARGET_FILE:fastdds::fast-discovery-server>>
-t ${TEST}
-s false
-i false
-S true
-C ${PROJECT_SOURCE_DIR}/test/certs)

set_tests_properties(${TEST_NAME} PROPERTIES
REQUIRED_FILES ${RUN_TEST}
REQUIRED_FILES ${TESTS_PARAMS})

if("${TEST}" IN_LIST FAIL_TEST_CASES)
set_property(TEST ${TEST_NAME} PROPERTY LABELS xfail)
endif()
endif()

endforeach()

# Windows requires an special treatment of environmental variables
Expand Down
72 changes: 72 additions & 0 deletions test/certs/governance_all_enable.smime
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
MIME-Version: 1.0
Content-Type: multipart/signed; protocol="application/x-pkcs7-signature"; micalg="sha-256"; boundary="----205663DC800FC27263B797AC629F745C"

This is an S/MIME signed message

------205663DC800FC27263B797AC629F745C
Content-Type: text/plain

<?xml version="1.0" encoding="utf-8"?>
<dds xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="omg_shared_ca_domain_governance.xsd">
<domain_access_rules>
<domain_rule>
<domains>
<id_range>
<min>0</min>
<max>230</max>
</id_range>
</domains>
<allow_unauthenticated_participants>false</allow_unauthenticated_participants>
<enable_join_access_control>true</enable_join_access_control>
<discovery_protection_kind>ENCRYPT</discovery_protection_kind>
<liveliness_protection_kind>ENCRYPT</liveliness_protection_kind>
<rtps_protection_kind>ENCRYPT</rtps_protection_kind>
<topic_access_rules>
<topic_rule>
<topic_expression>*</topic_expression>
<enable_discovery_protection>true</enable_discovery_protection>
<enable_liveliness_protection>true</enable_liveliness_protection>
<enable_read_access_control>true</enable_read_access_control>
<enable_write_access_control>true</enable_write_access_control>
<metadata_protection_kind>ENCRYPT</metadata_protection_kind>
<data_protection_kind>ENCRYPT</data_protection_kind>
</topic_rule>
</topic_access_rules>
</domain_rule>
</domain_access_rules>
</dds>


------205663DC800FC27263B797AC629F745C
Content-Type: application/x-pkcs7-signature; name="smime.p7s"
Content-Transfer-Encoding: base64
Content-Disposition: attachment; filename="smime.p7s"

MIIEeQYJKoZIhvcNAQcCoIIEajCCBGYCAQExDzANBglghkgBZQMEAgEFADALBgkq
hkiG9w0BBwGgggJAMIICPDCCAeOgAwIBAgIJALZwpgo2sxthMAoGCCqGSM49BAMC
MIGaMQswCQYDVQQGEwJFUzELMAkGA1UECAwCTUExFDASBgNVBAcMC1RyZXMgQ2Fu
dG9zMREwDwYDVQQKDAhlUHJvc2ltYTERMA8GA1UECwwIZVByb3NpbWExHjAcBgNV
BAMMFWVQcm9zaW1hIE1haW4gVGVzdCBDQTEiMCAGCSqGSIb3DQEJARYTbWFpbmNh
QGVwcm9zaW1hLmNvbTAeFw0xNzA5MDYwOTAzMDNaFw0yNzA5MDQwOTAzMDNaMIGa
MQswCQYDVQQGEwJFUzELMAkGA1UECAwCTUExFDASBgNVBAcMC1RyZXMgQ2FudG9z
MREwDwYDVQQKDAhlUHJvc2ltYTERMA8GA1UECwwIZVByb3NpbWExHjAcBgNVBAMM
FWVQcm9zaW1hIE1haW4gVGVzdCBDQTEiMCAGCSqGSIb3DQEJARYTbWFpbmNhQGVw
cm9zaW1hLmNvbTBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABGLlhB3WQ8l1fpUE
3DfOoulA/de38Zfj7hmpKtOnxiH2q6RJbwhxvJeA7R7mkmAKaJKmzx695BjyiXVS
7bE7vgejEDAOMAwGA1UdEwQFMAMBAf8wCgYIKoZIzj0EAwIDRwAwRAIgVTY1BEvT
4pw3GyBMzaUqmp69wi0kBkyOgq04OhyJ13UCICR125vvt0fUhXsXaxOAx28E4Ac9
SVxpI+3UYs2kV5n0MYIB/TCCAfkCAQEwgagwgZoxCzAJBgNVBAYTAkVTMQswCQYD
VQQIDAJNQTEUMBIGA1UEBwwLVHJlcyBDYW50b3MxETAPBgNVBAoMCGVQcm9zaW1h
MREwDwYDVQQLDAhlUHJvc2ltYTEeMBwGA1UEAwwVZVByb3NpbWEgTWFpbiBUZXN0
IENBMSIwIAYJKoZIhvcNAQkBFhNtYWluY2FAZXByb3NpbWEuY29tAgkAtnCmCjaz
G2EwDQYJYIZIAWUDBAIBBQCggeQwGAYJKoZIhvcNAQkDMQsGCSqGSIb3DQEHATAc
BgkqhkiG9w0BCQUxDxcNMjMwMjIxMDk1MjM0WjAvBgkqhkiG9w0BCQQxIgQguOmE
ipH9WhFwZt05wsDRKD9aInelvQO2SQANMKbmGV8weQYJKoZIhvcNAQkPMWwwajAL
BglghkgBZQMEASowCwYJYIZIAWUDBAEWMAsGCWCGSAFlAwQBAjAKBggqhkiG9w0D
BzAOBggqhkiG9w0DAgICAIAwDQYIKoZIhvcNAwICAUAwBwYFKw4DAgcwDQYIKoZI
hvcNAwICASgwCgYIKoZIzj0EAwIERzBFAiBZyyAcaQ7KB5qI/oF276mxSspzFkCI
HH1qbSPHKfjueQIhAIJQDdUCbimYKFGJYYYBZ/JBrdzMaB6Ordmomvkgjcx0

------205663DC800FC27263B797AC629F745C--

31 changes: 31 additions & 0 deletions test/certs/governance_all_enable.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?xml version="1.0" encoding="utf-8"?>
<dds xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="omg_shared_ca_domain_governance.xsd">
<domain_access_rules>
<domain_rule>
<domains>
<id_range>
<min>0</min>
<max>230</max>
</id_range>
</domains>
<allow_unauthenticated_participants>false</allow_unauthenticated_participants>
<enable_join_access_control>true</enable_join_access_control>
<discovery_protection_kind>ENCRYPT</discovery_protection_kind>
<liveliness_protection_kind>ENCRYPT</liveliness_protection_kind>
<rtps_protection_kind>ENCRYPT</rtps_protection_kind>
<topic_access_rules>
<topic_rule>
<topic_expression>*</topic_expression>
<enable_discovery_protection>true</enable_discovery_protection>
<enable_liveliness_protection>true</enable_liveliness_protection>
<enable_read_access_control>true</enable_read_access_control>
<enable_write_access_control>true</enable_write_access_control>
<metadata_protection_kind>ENCRYPT</metadata_protection_kind>
<data_protection_kind>ENCRYPT</data_protection_kind>
</topic_rule>
</topic_access_rules>
</domain_rule>
</domain_access_rules>
</dds>

14 changes: 14 additions & 0 deletions test/certs/maincacert.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
-----BEGIN CERTIFICATE-----
MIICPDCCAeOgAwIBAgIJALZwpgo2sxthMAoGCCqGSM49BAMCMIGaMQswCQYDVQQG
EwJFUzELMAkGA1UECAwCTUExFDASBgNVBAcMC1RyZXMgQ2FudG9zMREwDwYDVQQK
DAhlUHJvc2ltYTERMA8GA1UECwwIZVByb3NpbWExHjAcBgNVBAMMFWVQcm9zaW1h
IE1haW4gVGVzdCBDQTEiMCAGCSqGSIb3DQEJARYTbWFpbmNhQGVwcm9zaW1hLmNv
bTAeFw0xNzA5MDYwOTAzMDNaFw0yNzA5MDQwOTAzMDNaMIGaMQswCQYDVQQGEwJF
UzELMAkGA1UECAwCTUExFDASBgNVBAcMC1RyZXMgQ2FudG9zMREwDwYDVQQKDAhl
UHJvc2ltYTERMA8GA1UECwwIZVByb3NpbWExHjAcBgNVBAMMFWVQcm9zaW1hIE1h
aW4gVGVzdCBDQTEiMCAGCSqGSIb3DQEJARYTbWFpbmNhQGVwcm9zaW1hLmNvbTBZ
MBMGByqGSM49AgEGCCqGSM49AwEHA0IABGLlhB3WQ8l1fpUE3DfOoulA/de38Zfj
7hmpKtOnxiH2q6RJbwhxvJeA7R7mkmAKaJKmzx695BjyiXVS7bE7vgejEDAOMAwG
A1UdEwQFMAMBAf8wCgYIKoZIzj0EAwIDRwAwRAIgVTY1BEvT4pw3GyBMzaUqmp69
wi0kBkyOgq04OhyJ13UCICR125vvt0fUhXsXaxOAx28E4Ac9SVxpI+3UYs2kV5n0
-----END CERTIFICATE-----
5 changes: 5 additions & 0 deletions test/certs/maincakey.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
-----BEGIN PRIVATE KEY-----
MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgRaipe1KYZNzj+35E
N2jvtzjRsQ7n9Me/vm35UKGuVI6hRANCAARi5YQd1kPJdX6VBNw3zqLpQP3Xt/GX
4+4ZqSrTp8Yh9qukSW8IcbyXgO0e5pJgCmiSps8eveQY8ol1Uu2xO74H
-----END PRIVATE KEY-----
Loading