Skip to content

Comments

Implementing store support for EVP_SKEY#28278

Closed
beldmit wants to merge 2 commits intoopenssl:masterfrom
beldmit:evp_pkey_store
Closed

Implementing store support for EVP_SKEY#28278
beldmit wants to merge 2 commits intoopenssl:masterfrom
beldmit:evp_pkey_store

Conversation

@beldmit
Copy link
Member

@beldmit beldmit commented Aug 15, 2025 •
edited
Loading

Support of EVP_SKEY objects in store.

The tests and documentation TBD.

Checklist
  • documentation is added or updated
  • tests are added or updated

@github-actions github-actions bot added severity: fips change The pull request changes FIPS provider sources severity: ABI change This pull request contains ABI changes and removed severity: fips change The pull request changes FIPS provider sources labels Aug 15, 2025
@beldmit beldmit force-pushed the evp_pkey_store branch 3 times, most recently from 0432701 to 0347a8b Compare August 15, 2025 15:35
@beldmit beldmit added the branch: master Applies to master branch label Aug 15, 2025
@beldmit
Copy link
Member Author

beldmit commented Aug 15, 2025 •
edited
Loading

TODO

  • fix pkcs11-provider test
  • add tests for new enc and maybe storeutl options
  • deal with find-doc-nits failures
  • document file_store changes

@mattcaswell mattcaswell marked this pull request as draft August 18, 2025 07:43
@beldmit beldmit added the style: waived exempted from style checks label Aug 18, 2025
@beldmit
Copy link
Member Author

beldmit commented Aug 18, 2025

Waving style because the original functions were already longer than 200 lines. All the rest issues are resolved

@beldmit beldmit added the tests: present The PR has suitable tests present label Aug 18, 2025
@beldmit beldmit changed the title [Draft] Implementing store support for EVP_SKEY Implementing store support for EVP_SKEY Aug 18, 2025
@beldmit beldmit marked this pull request as ready for review August 18, 2025 14:27
@beldmit
Copy link
Member Author

beldmit commented Aug 18, 2025

I believe that this PR is conceptually ready for review. The pkcs11 test failure seems to be discussed at the pkcs#11 provider side as of now

@beldmit beldmit added the approval: review pending This pull request needs review by a committer label Aug 18, 2025
@beldmit
Copy link
Member Author

beldmit commented Aug 19, 2025 •
edited
Loading

Commenting the code around OSSL_OBJECT_SKEY in pkcs11-provider makes tests passing so it's an issue in pkcs11-provider

paulidale
paulidale reviewed Aug 20, 2025
View reviewed changes
beldmit added a commit to beldmit/pkcs11-provider that referenced this pull request Aug 20, 2025
@beldmit
Improve support of OSSL_OBJECT_SKEY
aead9a7 
OpenSSL PR: openssl/openssl#28278
@beldmit beldmit mentioned this pull request Aug 20, 2025
Merged
10 tasks
beldmit added a commit to beldmit/pkcs11-provider that referenced this pull request Aug 20, 2025
@beldmit
Improve support of OSSL_OBJECT_SKEY
6540f3e 
OpenSSL PR: openssl/openssl#28278

Signed-off-by: Dmitry Belyavskiy <[email protected]>
Jakuje pushed a commit to latchset/pkcs11-provider that referenced this pull request Aug 20, 2025
@beldmit @Jakuje
Improve support of OSSL_OBJECT_SKEY
143e191 
OpenSSL PR: openssl/openssl#28278

Signed-off-by: Dmitry Belyavskiy <[email protected]>
@beldmit beldmit marked this pull request as draft August 20, 2025 14:17
@beldmit
Copy link
Member Author

beldmit commented Aug 20, 2025

OK, after fixing the PKCS11 provider issue I see that the PR is to be adjusted a bit more.

beldmit added a commit to beldmit/pkcs11-provider that referenced this pull request Aug 21, 2025
@beldmit
PKCS11 provider returns a P11PROV_OBJ *
9e1a6d6 
...and should be able to take it back, fill with attributes and return
back

Related: openssl/openssl#28278
@beldmit beldmit mentioned this pull request Aug 21, 2025
Closed
10 tasks
@github-actions github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Aug 21, 2025
@t8m t8m added the triaged: feature The issue/pr requests/adds a feature label Aug 21, 2025
@beldmit
Copy link
Member Author

beldmit commented Dec 5, 2025

Successfully rebased, ready to review

@beldmit beldmit requested a review from simo5 December 5, 2025 12:17
simo5
simo5 reviewed Dec 5, 2025
View reviewed changes
Copy link
Contributor

@simo5 simo5 left a comment

Choose a reason for hiding this comment

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

Looks mostly ok, but I do not get the "importing" via store API, doesn't look very useful or appropriate for the store interface.

@beldmit beldmit mentioned this pull request Dec 5, 2025
Open
nhorman
nhorman previously approved these changes Dec 5, 2025
View reviewed changes
simo5
simo5 requested changes Dec 5, 2025
View reviewed changes
simo5
simo5 previously approved these changes Dec 8, 2025
View reviewed changes
Copy link
Contributor

@simo5 simo5 left a comment

Choose a reason for hiding this comment

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

LGTM

@beldmit
Copy link
Member Author

beldmit commented Dec 8, 2025

@nhorman could you please reapprove?

@beldmit beldmit added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Dec 9, 2025
@openssl-machine openssl-machine added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Dec 10, 2025
@openssl-machine
Copy link
Collaborator

openssl-machine commented Dec 10, 2025

This pull request is ready to merge

@nhorman
Copy link
Contributor

nhorman commented Dec 10, 2025

merged to master, thank you!

@nhorman nhorman closed this Dec 10, 2025
openssl-machine pushed a commit that referenced this pull request Dec 10, 2025
@beldmit @nhorman
Implementing store support for EVP_SKEY
1b0f21f 
Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Simo Sorce <[email protected]>
(Merged from #28278)
openssl-machine pushed a commit that referenced this pull request Dec 10, 2025
@beldmit @nhorman
Document skey decoder
d328874 
Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Simo Sorce <[email protected]>
(Merged from #28278)
cxx194832 pushed a commit to cxx194832/openssl that referenced this pull request Dec 12, 2025
@beldmit @cxx194832
Implementing store support for EVP_SKEY
a3d4b9b 
Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Simo Sorce <[email protected]>
(Merged from openssl#28278)
cxx194832 pushed a commit to cxx194832/openssl that referenced this pull request Dec 12, 2025
@beldmit @cxx194832
Document skey decoder
7dbfd8d 
Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Simo Sorce <[email protected]>
(Merged from openssl#28278)
@beldmit beldmit deleted the evp_pkey_store branch January 15, 2026 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@levitte levitte levitte left review comments

@mattcaswell mattcaswell mattcaswell requested changes

@nhorman nhorman nhorman approved these changes

@simo5 simo5 simo5 approved these changes

@paulidale paulidale Awaiting requested review from paulidale

@t8m t8m Awaiting requested review from t8m

Assignees

No one assigned

Labels

approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Applies to master branch severity: ABI change This pull request contains ABI changes style: waived exempted from style checks tests: present The PR has suitable tests present triaged: feature The issue/pr requests/adds a feature

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

@beldmit @nhorman @The-Mule @simo5 @paulidale @levitte @t-j-h @openssl-machine @mattcaswell @t8m