Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix V5 protocol message bson params #2235

Merged
merged 7 commits into from
Jun 18, 2018

Conversation

AKalinich-Luxoft
Copy link
Contributor

@AKalinich-Luxoft AKalinich-Luxoft commented Jun 5, 2018

Fixes #2142

This PR is ready for review.

Risk

This PR makes no API changes.

Testing Plan

Covered by unit tests
ATF scripts

Summary

There was a problem with V5 control frames encryption/decryption related to the wrong conditions in encrypt/decrypt functions in protocol manager. Also there was a problem with allocated bson params object passed as a pointer to another place.

NOTE
This fix should be merged after #2105 and #2068 features as it contains commit history from these branches. Please review only the last commit from this PR.

CLA

@AKalinich-Luxoft AKalinich-Luxoft requested a review from mrapitis June 5, 2018 19:23
@mrapitis
Copy link
Contributor

mrapitis commented Jun 7, 2018

@theresalech this PR is ready for review.

@AKalinich-Luxoft AKalinich-Luxoft force-pushed the fix/fix_v5_protocol_messages_bson_params branch from 161278f to 038b311 Compare June 15, 2018 09:23
@Jack-Byrne
Copy link
Collaborator

@AKalinich-Luxoft There are a lot of merge conflicts now that getSystemTime has been merged. Can you rebase this PR on the current develop?

@Jack-Byrne
Copy link
Collaborator

Also PR has 38 unit test failures in security_manager_test

Currently, SDL Core ignores both the CertificatePath and KeyPath
keywords that would allow the system integrator to specify certificates
for their environment, instead SDL Core only processes the certificate
provided via the policy table.

This fix makes these keywords functional. Following changes were done:
- Added getters for CertificatePath and KeyPath parameters in
SecurityManagerSettings class to provide another components an access
to these properties
- Added methods for loading certificate and private key data from the
files specified by CertificatePath and KeyPath keywords
- CryptoManager component implementation was updated. Now this component
also read certificate data from files (if they are present and accessible)
on its own initialization
SDL Core should update the module certificate in the local file system when
a policy table update occurs. Currently SDL core is retrieving its
certificate directly out of the policy table.

This fix provides functionality for saving module certificate to the file
system. Following changes were done:
- Added getters for CertificatePath and KeyPath parameters in
SecurityManagerSettings class to provide another components an access
to these properties
- Added methods for saving certificate and private key data to the
files specified by CertificatePath and KeyPath keywords
- CryptoManager component implementation was updated. Now this component
also saves certificate data to files (if write permission allowed) after
PTU
Added new expectations for a security tests due to some
changes in the security flow
Conflicts:
	src/components/security_manager/src/crypto_manager_impl.cc
Also was removed redundant logic
…Types

Updated encrypt/decrypt frame conditions
Fixed bson object double allocation
@AKalinich-Luxoft AKalinich-Luxoft force-pushed the fix/fix_v5_protocol_messages_bson_params branch from 038b311 to 919f8ed Compare June 18, 2018 19:27
@AKalinich-Luxoft
Copy link
Contributor Author

@JackLivio conflicts has been resolved. You just need to merge #2217 and #2218 first.

@Jack-Byrne Jack-Byrne merged commit 2ec732c into develop Jun 18, 2018
@Jack-Byrne Jack-Byrne deleted the fix/fix_v5_protocol_messages_bson_params branch June 18, 2018 21:00
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants