[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
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

Enhance CA handling and error messaging for improved configuration usability #24620

Merged
merged 7 commits into from
Jul 22, 2024
Merged

Enhance CA handling and error messaging for improved configuration usability #24620

merged 7 commits into from
Jul 22, 2024

Conversation

GabrielEValenzuela
Copy link
Member
@GabrielEValenzuela GabrielEValenzuela commented Jul 15, 2024 •
edited
Loading

Objective

This PR enhances the system's ability to handle multiple Certificate Authorities (CAs) by modifying the logic to read multiple CA certificates from the configuration and concatenate them into a single file. Additionally, we have improved error messaging to enhance usability, aligning with the Ghezzi quality software characteristics.

Description

We implemented a new logic that allows the system to read multiple CA entries from the configuration and concatenate them into a single CA bundle file. This streamlined approach simplifies the setup process for scenarios requiring multiple CAs, this approach has been validated as

We also recognized the need to improve the clarity and helpfulness of error messages related to CA configuration. Following the Ghezzi quality software characteristic of usability, we refined our error messaging to be more descriptive, guiding users more effectively through resolving configuration issues.

Quality Assurance

Static Analysis

N/A

Testing

Unit Testing

N/A

Integration Testing

  • Integration testing was conducted to ensure that the system correctly handles the new CA bundle during runtime, especially in environments requiring secure communications authenticated by multiple CAs.
  • Testing Steps:
    1. Configure the system with multiple valid and invalid CA entries. Follow the first step of Validate Multi-CA File Support in OpenSearch via Indexer Connector #24551
    2. Verify that the system concatenates valid CAs correctly and utilizes the resulting CA bundle for secure connections. Use the cat command for example and stat to check file permissions.
    3. Run manager, and indexer connection should be successful.
Screenshots

image

Pasted image

image

image

Additional Notes

  • The path proposal can change according with reviewers.

@GabrielEValenzuela
CL:
3649371 
- Add logic to read the vector of CA.
@GabrielEValenzuela GabrielEValenzuela self-assigned this Jul 15, 2024
@GabrielEValenzuela GabrielEValenzuela linked an issue Jul 15, 2024 that may be closed by this pull request
Indexer Connector - Multiple CA files aren't supported #24295
Closed
2 tasks
@GabrielEValenzuela
CL:
213c447 
- Change error message and path based on review comments.
@GabrielEValenzuela
CL:
eb40c04 
- Fix style.
@GabrielEValenzuela
CL:
17407ca 
- Fix path, permissions and create new folder.
@GabrielEValenzuela
CL:
50cfe3d 
- Make path relative and add logic of folder creation.
sebasfalcone
sebasfalcone previously approved these changes Jul 18, 2024
View reviewed changes
Copy link
Member
@sebasfalcone sebasfalcone left a comment

Choose a reason for hiding this comment

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

LGTM! Great testing @GabrielEValenzuela

Dwordcito
Dwordcito requested changes Jul 19, 2024
View reviewed changes
src/shared_modules/indexer_connector/src/indexerConnector.cpp Outdated Show resolved Hide resolved
src/shared_modules/indexer_connector/src/indexerConnector.cpp Outdated Show resolved Hide resolved
src/shared_modules/indexer_connector/src/indexerConnector.cpp Outdated Show resolved Hide resolved
src/shared_modules/indexer_connector/src/indexerConnector.cpp Outdated Show resolved Hide resolved
packages/rpms/SPECS/wazuh-manager.spec Outdated Show resolved Hide resolved
src/init/inst-functions.sh Outdated Show resolved Hide resolved
@GabrielEValenzuela
CL:
e9b4da7 
- Refactor based on review comments.

- Move logic into function.

- Use tmp folder.

- Fix typo and improve logic.
Dwordcito
Dwordcito reviewed Jul 19, 2024
View reviewed changes
src/shared_modules/indexer_connector/src/indexerConnector.cpp Outdated
Comment on lines 76 to 77
std::filesystem::path dirPath = std::filesystem::path(caRootCertificate).parent_path();
if (!std::filesystem::exists(dirPath) && !std::filesystem::create_directories(dirPath))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
std::filesystem::path dirPath = std::filesystem::path(caRootCertificate).parent_path();
if (!std::filesystem::exists(dirPath) && !std::filesystem::create_directories(dirPath))
if (std::filesystem::path dirPath = std::filesystem::path(caRootCertificate).parent_path();
!std::filesystem::exists(dirPath) && !std::filesystem::create_directories(dirPath))

@GabrielEValenzuela
CL:
1b59671 
- Fix condition.
@Dwordcito Dwordcito merged commit e7fe756 into 4.10.0 Jul 22, 2024
61 of 65 checks passed
@Dwordcito Dwordcito deleted the bug/24295_allow-multiple-ca branch July 22, 2024 13:49
This was referenced Oct 9, 2024
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Dwordcito Dwordcito Dwordcito approved these changes

@sebasfalcone sebasfalcone Awaiting requested review from sebasfalcone

Assignees

@GabrielEValenzuela GabrielEValenzuela

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Indexer Connector - Multiple CA files aren't supported
3 participants
@GabrielEValenzuela @Dwordcito @sebasfalcone