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

Accept v6 interfaces and interface names in whitelist [21531] #128

Merged
merged 1 commit into from
Sep 6, 2024
Merged

Accept v6 interfaces and interface names in whitelist [21531] #128

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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 ...articipants/include/ddspipe_participants/configuration/SimpleParticipantConfiguration.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ struct SimpleParticipantConfiguration : public ParticipantConfiguration

core::types::DomainId domain {0u};

std::set<participants::types::IpType> whitelist {};
std::set<participants::types::WhitelistType> whitelist {};

core::types::TransportDescriptors transport {core::types::TransportDescriptors::builtin};

Expand Down
2 changes: 1 addition & 1 deletion ddspipe_participants/include/ddspipe_participants/participant/rtps/CommonParticipant.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ class CommonParticipant
template<typename T>
DDSPIPE_PARTICIPANTS_DllAPI
static std::shared_ptr<T> create_descriptor(
std::set<types::IpType> whitelist = {});
std::set<types::WhitelistType> whitelist = {});

protected:

Expand Down
2 changes: 2 additions & 0 deletions ddspipe_participants/include/ddspipe_participants/types/address/Address.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ namespace types {
using LocatorType = uint32_t;
// Data Type for IP
using IpType = std::string;
// Data Type for an interfaces whitelist entry
using WhitelistType = std::string;
// Data Type for Domain name (DNS)
using DomainType = std::string;
// Data Type for Port
Expand Down
10 changes: 0 additions & 10 deletions ddspipe_participants/src/cpp/configuration/SimpleParticipantConfiguration.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,6 @@ bool SimpleParticipantConfiguration::is_valid(
return false;
}

// Check whitelist interfaces
for (types::IpType ip : whitelist)
{
if (!types::Address::is_ipv4_correct(ip))
{
error_msg << "Incorrect IPv4 address " << ip << " in whitelist interfaces. ";
return false;
}
}

return true;
}

Expand Down
76 changes: 20 additions & 56 deletions ddspipe_participants/src/cpp/participant/rtps/CommonParticipant.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,25 +211,16 @@ template<>
DDSPIPE_PARTICIPANTS_DllAPI
std::shared_ptr<eprosima::fastdds::rtps::UDPv4TransportDescriptor>
CommonParticipant::create_descriptor(
std::set<types::IpType> whitelist)
std::set<types::WhitelistType> whitelist)
{
std::shared_ptr<eprosima::fastdds::rtps::UDPv4TransportDescriptor> udp_transport =
std::make_shared<eprosima::fastdds::rtps::UDPv4TransportDescriptor>();

for (const types::IpType& ip : whitelist)
for (const types::WhitelistType& iface : whitelist)
{
if (types::Address::is_ipv4_correct(ip))
{
udp_transport->interfaceWhiteList.emplace_back(ip);
EPROSIMA_LOG_INFO(DDSPIPE_COMMON_PARTICIPANT,
"Adding " << ip << " to UDP whitelist interfaces.");
}
else
{
// Invalid address, continue with next one
EPROSIMA_LOG_WARNING(DDSPIPE_COMMON_PARTICIPANT,
"Not valid IPv4. Discarding UDP whitelist interface " << ip << ".");
}
Comment on lines -221 to -232
Copy link
Contributor

Choose a reason for hiding this comment

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

Future Suggestion:
Why not validate both IPv4 and IPv6 formats to ensure correctness?

  • Current behavior when an incorrect format is used:
    The product continues running but doesn't function as expected, with the following FastDDS error:
    [TRANSPORT_UDPV4 Error] All whitelist interfaces were filtered out.

  • Expected behavior:
    The product should terminate with a DDSPipe error indicating the invalid format.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because now that interface names are accepted, it cannot be (easily) checked from our layer whether an element is an invalid IPv4 address (due to a typo for example) or an interface name. These checks are internally performed by Fast-DDS library, and their choice is that warnings are displayed if they fail (instead of throwing an exception or similar as we would prefer to have).

udp_transport->interfaceWhiteList.emplace_back(iface);
EPROSIMA_LOG_INFO(DDSPIPE_COMMON_PARTICIPANT,
"Adding " << iface << " to UDP whitelist interfaces.");
}

return udp_transport;
Expand All @@ -239,25 +230,16 @@ template<>
DDSPIPE_PARTICIPANTS_DllAPI
std::shared_ptr<eprosima::fastdds::rtps::UDPv6TransportDescriptor>
CommonParticipant::create_descriptor(
std::set<types::IpType> whitelist)
std::set<types::WhitelistType> whitelist)
{
std::shared_ptr<eprosima::fastdds::rtps::UDPv6TransportDescriptor> udp_transport =
std::make_shared<eprosima::fastdds::rtps::UDPv6TransportDescriptor>();

for (const types::IpType& ip : whitelist)
for (const types::WhitelistType& iface : whitelist)
{
if (types::Address::is_ipv6_correct(ip))
{
udp_transport->interfaceWhiteList.emplace_back(ip);
EPROSIMA_LOG_INFO(DDSPIPE_COMMON_PARTICIPANT,
"Adding " << ip << " to UDP whitelist interfaces.");
}
else
{
// Invalid address, continue with next one
EPROSIMA_LOG_WARNING(DDSPIPE_COMMON_PARTICIPANT,
"Not valid IPv6. Discarding UDP whitelist interface " << ip << ".");
}
udp_transport->interfaceWhiteList.emplace_back(iface);
EPROSIMA_LOG_INFO(DDSPIPE_COMMON_PARTICIPANT,
"Adding " << iface << " to UDP whitelist interfaces.");
}

return udp_transport;
Expand All @@ -267,25 +249,16 @@ template<>
DDSPIPE_PARTICIPANTS_DllAPI
std::shared_ptr<eprosima::fastdds::rtps::TCPv4TransportDescriptor>
CommonParticipant::create_descriptor(
std::set<types::IpType> whitelist)
std::set<types::WhitelistType> whitelist)
{
std::shared_ptr<eprosima::fastdds::rtps::TCPv4TransportDescriptor> tcp_transport =
std::make_shared<eprosima::fastdds::rtps::TCPv4TransportDescriptor>();

for (const types::IpType& ip : whitelist)
for (const types::WhitelistType& iface : whitelist)
{
if (types::Address::is_ipv4_correct(ip))
{
tcp_transport->interfaceWhiteList.emplace_back(ip);
EPROSIMA_LOG_INFO(DDSPIPE_COMMON_PARTICIPANT,
"Adding " << ip << " to TCP whitelist interfaces.");
}
else
{
// Invalid address, continue with next one
EPROSIMA_LOG_WARNING(DDSPIPE_COMMON_PARTICIPANT,
"Not valid IPv4. Discarding TCP whitelist interface " << ip << ".");
}
tcp_transport->interfaceWhiteList.emplace_back(iface);
EPROSIMA_LOG_INFO(DDSPIPE_COMMON_PARTICIPANT,
"Adding " << iface << " to TCP whitelist interfaces.");
}

return tcp_transport;
Expand All @@ -295,25 +268,16 @@ template<>
DDSPIPE_PARTICIPANTS_DllAPI
std::shared_ptr<eprosima::fastdds::rtps::TCPv6TransportDescriptor>
CommonParticipant::create_descriptor(
std::set<types::IpType> whitelist)
std::set<types::WhitelistType> whitelist)
{
std::shared_ptr<eprosima::fastdds::rtps::TCPv6TransportDescriptor> tcp_transport =
std::make_shared<eprosima::fastdds::rtps::TCPv6TransportDescriptor>();

for (const types::IpType& ip : whitelist)
for (const types::WhitelistType& iface : whitelist)
{
if (types::Address::is_ipv6_correct(ip))
{
tcp_transport->interfaceWhiteList.emplace_back(ip);
EPROSIMA_LOG_INFO(DDSPIPE_COMMON_PARTICIPANT,
"Adding " << ip << " to TCP whitelist interfaces.");
}
else
{
// Invalid address, continue with next one
EPROSIMA_LOG_WARNING(DDSPIPE_COMMON_PARTICIPANT,
"Not valid IPv6. Discarding TCP whitelist interface " << ip << ".");
}
tcp_transport->interfaceWhiteList.emplace_back(iface);
EPROSIMA_LOG_INFO(DDSPIPE_COMMON_PARTICIPANT,
"Adding " << iface << " to TCP whitelist interfaces.");
}

return tcp_transport;
Expand Down
9 changes: 6 additions & 3 deletions ddspipe_yaml/src/cpp/YamlReader_participants.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,8 @@ void YamlReader::fill(
// Optional whitelist interfaces
if (YamlReader::is_tag_present(yml, WHITELIST_INTERFACES_TAG))
{
object.whitelist = YamlReader::get_set<participants::types::IpType>(yml, WHITELIST_INTERFACES_TAG, version);
object.whitelist = YamlReader::get_set<participants::types::WhitelistType>(yml, WHITELIST_INTERFACES_TAG,
version);
}

// Optional get Transport descriptors
Expand Down Expand Up @@ -194,7 +195,8 @@ void YamlReader::fill(
// Optional whitelist interfaces
if (YamlReader::is_tag_present(yml, WHITELIST_INTERFACES_TAG))
{
object.whitelist = YamlReader::get_set<participants::types::IpType>(yml, WHITELIST_INTERFACES_TAG, version);
object.whitelist = YamlReader::get_set<participants::types::WhitelistType>(yml, WHITELIST_INTERFACES_TAG,
version);
}

// Optional listening addresses
Expand Down Expand Up @@ -262,7 +264,8 @@ void YamlReader::fill(
// Optional whitelist interfaces
if (YamlReader::is_tag_present(yml, WHITELIST_INTERFACES_TAG))
{
object.whitelist = YamlReader::get_set<participants::types::IpType>(yml, WHITELIST_INTERFACES_TAG, version);
object.whitelist = YamlReader::get_set<participants::types::WhitelistType>(yml, WHITELIST_INTERFACES_TAG,
version);
}

// Optional listening addresses
Expand Down
Loading