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

Allow service_discovery to continue without random_device #479

Merged
merged 3 commits into from
Nov 6, 2023
Merged

Allow service_discovery to continue without random_device #479

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
aced54e
Allow service_discovery to continue without random_device
kheaactua Jun 22, 2023
1377228
Remove the check_routing_credentials_ member when security is disable…
kheaactua Jun 22, 2023
9952fea
Update service_discovery_impl.cpp
DiogoPedrozza Nov 6, 2023
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
3 changes: 2 additions & 1 deletion implementation/security/include/policy_manager_impl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,11 @@ class VSOMEIP_IMPORT_EXPORT policy_manager_impl
mutable boost::shared_mutex policy_extension_paths_mutex_;
//map[hostname, pair[path, map[complete path with UID/GID, control loading]]
std::map<std::string, std::pair<std::string, std::map<std::string, bool>>> policy_extension_paths_;

bool check_routing_credentials_;
#endif // !VSOMEIP_DISABLE_SECURITY

bool is_configured_;
bool check_routing_credentials_;

mutable std::mutex routing_credentials_mutex_;
std::pair<uint32_t, uint32_t> routing_credentials_;
Expand Down
4 changes: 2 additions & 2 deletions implementation/security/src/policy_manager_impl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ policy_manager_impl::policy_manager_impl()
allow_remote_clients_(true),
check_whitelist_(false),
policy_base_path_(""),
check_routing_credentials_(false),
#endif // !VSOMEIP_DISABLE_SECURITY
is_configured_(false),
check_routing_credentials_(false)
is_configured_(false)
{
}

Expand Down
30 changes: 25 additions & 5 deletions implementation/service_discovery/src/service_discovery_impl.cpp
View file
Open in desktop
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @kheaactua ,
We agree with the try-catch method, but we would suggest in the fallback to use std::chrono::time_point::time_since_epoch() instead, to seed the Mersenne Twister.
That way will still ensure a decent level of randomness.
Otherwise, the PR seems ok to be merged.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, I'll give it a try and re-push.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @kheaactua, did you had the opportunity to change the implementation to what @fcmonteiro suggested?
Thanks in advance!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Started, but got distracted :( , will get back to it ASAP

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My apologies for it taking so long. I just pushed a version with the Mersenne Twister. I tested this on QNX with and without random

Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,31 @@ service_discovery_impl::init() {
initial_delay_max = tmp;
}

std::random_device r;
std::mt19937 e(r());
std::uniform_int_distribution<std::uint32_t> distribution(
initial_delay_min, initial_delay_max);
initial_delay_ = std::chrono::milliseconds(distribution(e));
try {
std::random_device r;
std::mt19937 e(r());
std::uniform_int_distribution<std::uint32_t> distribution(
initial_delay_min, initial_delay_max);
initial_delay_ = std::chrono::milliseconds(distribution(e));
} catch (const std::exception& e) {
VSOMEIP_ERROR << "Failed to generate random initial delay: " << e.what();

// Fallback to the Mersenne Twister engine
const auto seed = static_cast<std::mt19937::result_type>(
std::chrono::duration_cast<std::chrono::milliseconds>(
std::chrono::high_resolution_clock::now().time_since_epoch())
.count());

std::mt19937 mtwister{seed};

// Interpolate between initial_delay bounds
initial_delay_ = std::chrono::milliseconds(
initial_delay_min +
(static_cast<std::int64_t>(mtwister()) *
static_cast<std::int64_t>(initial_delay_max - initial_delay_min) /
static_cast<std::int64_t>(std::mt19937::max() -
std::mt19937::min())));
}


repetitions_base_delay_ = std::chrono::milliseconds(
Expand Down
Loading