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
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
83 changes: 83 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,29 @@ 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
{
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 +1012,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 +1290,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 +1404,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
23 changes: 23 additions & 0 deletions test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,29 @@ foreach(TEST IN LISTS TEST_LIST)
set_property(TEST ${TEST_NAME} PROPERTY LABELS xfail)
endif()

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()

endforeach()

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

This is an S/MIME signed message

------C5F52FFDC6C8C099D91B23188E515C2E
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_rule>
<topic_expression>topic*</topic_expression>
<enable_discovery_protection>true</enable_discovery_protection>
<enable_liveliness_protection>true</enable_liveliness_protection>
<enable_read_access_control>false</enable_read_access_control>
<enable_write_access_control>false</enable_write_access_control>
<metadata_protection_kind>ENCRYPT</metadata_protection_kind>
<data_protection_kind>ENCRYPT</data_protection_kind>
</topic_rule>
Copy link
Contributor

Choose a reason for hiding this comment

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

Wha's the rationale behind this distinction? Why disable access control for reading and writing on topic* ?

Copy link
Member Author

Choose a reason for hiding this comment

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

The second topic rule makes no sense. The idea is to have all the protections enabled for all topics

</topic_access_rules>
</domain_rule>
</domain_access_rules>
</dds>


------C5F52FFDC6C8C099D91B23188E515C2E
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
BgkqhkiG9w0BCQUxDxcNMjMwMjE2MTM1MzM4WjAvBgkqhkiG9w0BCQQxIgQgnYat
+qpSujB+F1VkLqKebFVhML6kkDOYUJprAbUN++QweQYJKoZIhvcNAQkPMWwwajAL
BglghkgBZQMEASowCwYJYIZIAWUDBAEWMAsGCWCGSAFlAwQBAjAKBggqhkiG9w0D
BzAOBggqhkiG9w0DAgICAIAwDQYIKoZIhvcNAwICAUAwBwYFKw4DAgcwDQYIKoZI
hvcNAwICASgwCgYIKoZIzj0EAwIERzBFAiA1XSjy+b788/v9jfT9XyLB5nAaCQjB
66R0cSiLCUrX8AIhAOv4hdmaTllVbs/zB4BZNt0Ikx1PAfudHMwfSN/EgeG9

------C5F52FFDC6C8C099D91B23188E515C2E--

40 changes: 40 additions & 0 deletions test/certs/governance_all_enable.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?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_rule>
<topic_expression>topic*</topic_expression>
<enable_discovery_protection>true</enable_discovery_protection>
<enable_liveliness_protection>true</enable_liveliness_protection>
<enable_read_access_control>false</enable_read_access_control>
<enable_write_access_control>false</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