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

Migrate keystore to master #27066

Merged
merged 27 commits into from
Dec 18, 2024
Merged

Conversation

pereyra-m
Copy link
Member

@pereyra-m pereyra-m commented Nov 26, 2024

Related issue
Closes #26971

Description

This PR migrates the wazuh-keystore to the master branch and adapts it to the project structure. The QA/UT tests and required utils were also migrated.

The OpenSSL v3.1.0 was added to vcpkg because it was missing and it was the closest version to the one used at branch v4.10.0.

The IndexerConnector class was updated to read the keystore and the plain text credentials were removed.

The SPECS and installer scripts were modified to include the new binary, but the RPM package can't be built due to previous errors.
The folders and binaries permissions were preserved but due to facebook/rocksdb#11503 the internal RocksDB files will have 644 instead of 640.

Until this PR is merged #26908, the root:wazuh permissions will be used instead of wazuh:wazuh.

Tests

I've installed a package built from this branch and configured the keystore credentials.
A test event is properly indexed

2024-11-27_20-44

@pereyra-m pereyra-m self-assigned this Nov 26, 2024
@pereyra-m pereyra-m linked an issue Nov 26, 2024 that may be closed by this pull request
2 tasks
@pereyra-m pereyra-m force-pushed the enhancement/26971-migrate-keystore-tool branch 2 times, most recently from 965bb18 to 19679b8 Compare November 27, 2024 07:39
@pereyra-m pereyra-m marked this pull request as ready for review November 27, 2024 07:41
@pereyra-m pereyra-m force-pushed the enhancement/26971-migrate-keystore-tool branch 3 times, most recently from 4b0b74d to 6a755db Compare November 28, 2024 03:58
Copy link

Coverage reports

The coverage report can be downloaded from here

Module Line coverage Function coverage
base 81.4% ✅ 85.3% ✅
conf 94.1% ✅ 95.3% ✅
indexerconnector 94.7% ✅ 91.9% ✅
keystore 90.7% ✅ 91.7% ✅

🟢 All modules have passed the coverage check

@pereyra-m pereyra-m force-pushed the enhancement/26971-migrate-keystore-tool branch from 6a755db to 2157bda Compare November 28, 2024 12:53
Copy link

Coverage reports

The coverage report can be downloaded from here

Module Line coverage Function coverage
base 81.4% ✅ 85.3% ✅
conf 94.1% ✅ 95.3% ✅
indexerconnector 94.7% ✅ 91.9% ✅
keystore 90.7% ✅ 91.7% ✅

🟢 All modules have passed the coverage check

@pereyra-m pereyra-m force-pushed the enhancement/26971-migrate-keystore-tool branch 3 times, most recently from f6e3f90 to 7a70871 Compare November 28, 2024 13:38
Copy link

Coverage reports

The coverage report can be downloaded from here

Module Line coverage Function coverage
base 81.4% ✅ 85.3% ✅
conf 94.1% ✅ 95.3% ✅
indexerconnector 94.7% ✅ 91.9% ✅
keystore 90.7% ✅ 91.7% ✅

🟢 All modules have passed the coverage check

Copy link

Coverage reports

The coverage report can be downloaded from here

Module Line coverage Function coverage
base 81.4% ✅ 85.3% ✅
conf 94.1% ✅ 95.3% ✅
indexerconnector 94.7% ✅ 91.9% ✅
keystore 90.7% ✅ 91.7% ✅

🟢 All modules have passed the coverage check

Copy link

github-actions bot commented Dec 4, 2024

Coverage reports

The coverage report can be downloaded from here

Module Line coverage Function coverage
base 82.0% ✅ 86.1% ✅
conf 94.1% ✅ 95.3% ✅
indexerconnector 94.7% ✅ 91.9% ✅
keystore 96.0% ✅ 92.3% ✅

🟢 All modules have passed the coverage check

@pereyra-m pereyra-m force-pushed the enhancement/26971-migrate-keystore-tool branch from d2e0e4b to 3d2e058 Compare December 4, 2024 05:06
@pereyra-m pereyra-m requested a review from Dwordcito December 4, 2024 05:06
Copy link

github-actions bot commented Dec 4, 2024

Coverage reports

The coverage report can be downloaded from here

Module Line coverage Function coverage
base 82.1% ✅ 86.1% ✅
conf 94.1% ✅ 95.3% ✅
indexerconnector 94.7% ✅ 91.9% ✅
keystore 96.0% ✅ 92.3% ✅

🟢 All modules have passed the coverage check

@pereyra-m pereyra-m force-pushed the enhancement/26971-migrate-keystore-tool branch from 3d2e058 to 31487ba Compare December 4, 2024 05:22
Copy link

github-actions bot commented Dec 4, 2024

Coverage reports

The coverage report can be downloaded from here

Module Line coverage Function coverage
base 82.1% ✅ 86.1% ✅
conf 94.1% ✅ 95.3% ✅
indexerconnector 94.7% ✅ 91.9% ✅
keystore 96.0% ✅ 92.3% ✅

🟢 All modules have passed the coverage check

Copy link

github-actions bot commented Dec 4, 2024

Coverage reports

The coverage report can be downloaded from here

Module Line coverage Function coverage
base 82.1% ✅ 86.1% ✅
conf 94.1% ✅ 95.3% ✅
indexerconnector 94.7% ✅ 91.9% ✅
keystore 100.0% ✅ 90.0% ✅

🟢 All modules have passed the coverage check

std::ofstream file(m_filePath);
if (!file.is_open())
{
throw std::runtime_error("Error creating key-value file due to: " + std::string(strerror(errno)));
Copy link
Member

Choose a reason for hiding this comment

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

Is it expect to fail if the folder is not created?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question.
In the installer, the folder /etc/wazuh-server should exist.
The problem is that the error message in this case isn't helpful

@Dwordcito Dwordcito force-pushed the enhancement/26971-migrate-keystore-tool branch from 2d954e6 to 414a665 Compare December 18, 2024 18:55
Copy link

Coverage reports

The coverage report can be downloaded from here

Module Line coverage Function coverage
base 81.9% ✅ 86.9% ✅
fs 95.6% ✅ 100.0% ✅
keystore 87.8% ✅ 100.0% ✅

🟢 All modules have passed the coverage check

@Dwordcito
Copy link
Member

@pereyra-m is OOO, I made some changes in the Keystore::filecreate class, because we call the std::ofstream constructor with the filename and not with the filepath.

Copy link

Coverage reports

The coverage report can be downloaded from here

Module Line coverage Function coverage
base 81.9% ✅ 86.9% ✅
fs 95.6% ✅ 100.0% ✅
keystore 85.7% ✅ 100.0% ✅

🟢 All modules have passed the coverage check

Copy link
Contributor

@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.

Testing

Insert values

./wazuh-keystore -k stdin
stdin_value
Key store file updated successfully.

echo "echo_value" | ./wazuh-keystore -k echo      
Key store file updated successfully.

./wazuh-keystore -k file -vp value.txt 
Key store file updated successfully.

./wazuh-keystore -k arg -v arg_value  
Key store file updated successfully.

Read values

python3 read.py
arg:arg_value
echo:echo_value
file:file_value
stdin:stdin_value

Overwrite values

./wazuh-keystore -k arg -v arg_value_new
Key store file updated successfully.

python3 read.py
arg:arg_value_new
echo:echo_value
file:file_value
stdin:stdin_value

@Dwordcito Dwordcito merged commit 5d52679 into master Dec 18, 2024
8 checks passed
@Dwordcito Dwordcito deleted the enhancement/26971-migrate-keystore-tool branch December 18, 2024 23:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate keystore tool
4 participants