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

[11909] pkcs11 support #2222

Merged
merged 30 commits into from
Dec 22, 2021
Merged

[11909] pkcs11 support #2222

merged 30 commits into from
Dec 22, 2021

Conversation

IkerLuengo
Copy link
Contributor

@IkerLuengo IkerLuengo commented Sep 22, 2021 •
edited by JLBuenoLopez
Loading

Add support for PKCS#11 format URIs for private keys.

Documentation PR: eProsima/Fast-DDS-docs#296

@IkerLuengo IkerLuengo force-pushed the feature/pkcs11-support branch from 1f7fbd1 to 6c02fd3 Compare October 7, 2021 12:52
JLBuenoLopez
JLBuenoLopez suggested changes Oct 22, 2021
View reviewed changes
src/cpp/security/artifact_providers/FileProvider.hpp Outdated Show resolved Hide resolved
src/cpp/security/artifact_providers/FileProvider.cpp Outdated Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
src/cpp/security/artifact_providers/Pkcs11Provider.hpp Outdated Show resolved Hide resolved
src/cpp/security/artifact_providers/Pkcs11Provider.hpp Outdated Show resolved Hide resolved
src/cpp/security/artifact_providers/Pkcs11Provider.cpp Outdated Show resolved Hide resolved
@IkerLuengo IkerLuengo force-pushed the feature/pkcs11-support branch from 6c02fd3 to cc4f52d Compare October 27, 2021 16:01
@IkerLuengo IkerLuengo marked this pull request as ready for review October 27, 2021 16:01
@IkerLuengo IkerLuengo force-pushed the feature/pkcs11-support branch 2 times, most recently from 8c94ac3 to f3b98cc Compare October 28, 2021 07:00
JLBuenoLopez
JLBuenoLopez reviewed Oct 29, 2021
View reviewed changes
test/blackbox/common/BlackboxTestsSecurity.cpp Outdated Show resolved Hide resolved
test/blackbox/common/BlackboxTestsSecurity.cpp Show resolved Hide resolved
test/blackbox/common/BlackboxTestsSecurity.cpp Outdated Show resolved Hide resolved
test/blackbox/common/BlackboxTestsSecurity.cpp Outdated Show resolved Hide resolved
test/blackbox/common/BlackboxTestsSecurity.cpp Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
JLBuenoLopez
JLBuenoLopez suggested changes Oct 29, 2021
View reviewed changes
Copy link
Contributor

@JLBuenoLopez JLBuenoLopez left a comment

Choose a reason for hiding this comment

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

I have only tested it without the feature installed in my system. Once the documentation is done, I would also like to test it locally.

@IkerLuengo IkerLuengo force-pushed the feature/pkcs11-support branch from 297582f to 1fa69f3 Compare October 29, 2021 08:23
JLBuenoLopez
JLBuenoLopez previously approved these changes Nov 2, 2021
View reviewed changes
Copy link
Contributor

@JLBuenoLopez JLBuenoLopez left a comment

Choose a reason for hiding this comment

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

LGTM.

I have not tested this feature locally because the dependencies installation instructions are not yet done.

@JLBuenoLopez JLBuenoLopez added the doc-pending Issue or PR which is pending to be documented label Nov 2, 2021
@JLBuenoLopez
Copy link
Contributor

@richiprosima please test Windows

@JLBuenoLopez
Copy link
Contributor

Also, please fix linter errors

JLBuenoLopez
JLBuenoLopez previously approved these changes Nov 4, 2021
View reviewed changes
Copy link
Contributor

@JLBuenoLopez JLBuenoLopez left a comment

Choose a reason for hiding this comment

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

LGTM

@JLBuenoLopez JLBuenoLopez added this to the v2.5.0 milestone Nov 16, 2021
@JLBuenoLopez JLBuenoLopez added ready-to-merge Ready to be merged. CI and changes have been reviewed and approved. and removed doc-pending Issue or PR which is pending to be documented labels Nov 16, 2021
@MiguelBarro MiguelBarro force-pushed the feature/pkcs11-support branch 5 times, most recently from 44fadad to 803b532 Compare December 9, 2021 10:47
@EduPonz EduPonz removed the ready-to-merge Ready to be merged. CI and changes have been reviewed and approved. label Dec 10, 2021
@MiguelBarro
Copy link
Contributor

@richiprosima Please test windows

1 similar comment
@MiguelBarro
Copy link
Contributor

@richiprosima Please test windows

@MiguelBarro MiguelBarro force-pushed the feature/pkcs11-support branch from b500983 to 4a790af Compare December 16, 2021 08:45
IkerLuengo and others added 17 commits December 21, 2021 12:42
@IkerLuengo
Refs 11914. Avoid singleton and make the provider destroy with plugin
6648966 
Signed-off-by: Iker Luengo <ikerluengo@eprosima.com>
@IkerLuengo
Refs 11914. Conditional compile with libP11
ca0534b 
Signed-off-by: Iker Luengo <ikerluengo@eprosima.com>
@IkerLuengo
Refs 11914. Changes requested on review
46653be 
Signed-off-by: Iker Luengo <ikerluengo@eprosima.com>
@IkerLuengo
Refs 11914. uncrustify
a993743 
Signed-off-by: Iker Luengo <ikerluengo@eprosima.com>
@IkerLuengo
Refs 11914. Suggestions on tests
97ff8af 
Signed-off-by: Iker Luengo <ikerluengo@eprosima.com>
@IkerLuengo
Refs 11914. Update dependencies on README
3096ce5 
Signed-off-by: Iker Luengo <ikerluengo@eprosima.com>
@IkerLuengo
Refs 11914. update fastrtps API pubsubreader
ef9cd3b 
Signed-off-by: Iker Luengo <ikerluengo@eprosima.com>
@IkerLuengo
Refs 11914. Remove unused parameter
ac891f1 
Signed-off-by: Iker Luengo <ikerluengo@eprosima.com>
Refs 11914. Update CMake framework
bf96644 
Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>
Refs 11914. Make p11 windows installer friendly
00c586b 
Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>
Refs 11914. Make test grep independent on windows
20c01e6 
Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>
Refs 11914. Ignore pkcs11 tests if not available.
94d294a 
Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>
Refs 11914. Update system calls on windows
519e8e7 
Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>
Refs 11914. Linux ci fixes.
8618307 
Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>
Refs 11914. Make CMake hint openssl config to blackbox tests on windows
804df63 
Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>
Refs 11914. Rebase fixes.
a23cc9b 
Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>
Refs 11914. Linter.
ecdf86b 
Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>
@MiguelBarro MiguelBarro force-pushed the feature/pkcs11-support branch from a27d4ab to ecdf86b Compare December 21, 2021 11:44
JLBuenoLopez
JLBuenoLopez reviewed Dec 21, 2021
View reviewed changes
Copy link
Contributor

@JLBuenoLopez JLBuenoLopez left a comment

Choose a reason for hiding this comment

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

Only a small improvement 😅

test/unittest/statistics/dds/CMakeLists.txt Outdated
@@ -236,6 +236,7 @@ if (SQLITE3_SUPPORT AND FASTDDS_STATISTICS)
${PROJECT_SOURCE_DIR}/src/cpp/rtps/xmlparser/XMLParser.cpp
${PROJECT_SOURCE_DIR}/src/cpp/rtps/xmlparser/XMLParserCommon.cpp
${PROJECT_SOURCE_DIR}/src/cpp/rtps/xmlparser/XMLProfileManager.cpp
${PROJECT_SOURCE_DIR}/src/cpp/security/artifact_providers/FileProvider.cpp
Copy link
Contributor

Choose a reason for hiding this comment

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

This source file should be included in case SECURITY is enabled (from line 277 on)

Copy link
Contributor

Choose a reason for hiding this comment

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

That's absolutely right!

Refs 11914. Address reviewers comments.
0aead0b 
Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>
JLBuenoLopez
JLBuenoLopez previously approved these changes Dec 21, 2021
View reviewed changes
Copy link
Contributor

@JLBuenoLopez JLBuenoLopez left a comment

Choose a reason for hiding this comment

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

LGTM with greenish CI 🥳

Refs 11914. Address reviewers comments.
6ee7fa8 
Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>
@MiguelBarro
Copy link
Contributor

@richiprosima Please test windows

Refs 11914. Disable pkcs11 windows testing till ci is reviewed
9ffce97 
Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>
@MiguelBarro MiguelBarro force-pushed the feature/pkcs11-support branch from 0a0252c to 9ffce97 Compare December 21, 2021 21:12
JLBuenoLopez
JLBuenoLopez approved these changes Dec 22, 2021
View reviewed changes
Copy link
Contributor

@JLBuenoLopez JLBuenoLopez left a comment

Choose a reason for hiding this comment

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

🆗

@MiguelCompany
Copy link
Member

MiguelCompany commented Dec 22, 2021 •
edited
Loading

  • Uncrustify: Build Status
  • Windows: Build Status
  • Linux: Build Status
  • Mac: Build Status

Windows failures are known to fail sometimes.

@MiguelCompany MiguelCompany merged commit 342c821 into master Dec 22, 2021
@MiguelCompany MiguelCompany deleted the feature/pkcs11-support branch December 22, 2021 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MiguelBarro MiguelBarro MiguelBarro left review comments

@JLBuenoLopez JLBuenoLopez JLBuenoLopez approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v2.5.0
Development

Successfully merging this pull request may close these issues.
5 participants
@IkerLuengo @JLBuenoLopez @MiguelBarro @MiguelCompany @EduPonz