Skip to content

Refactor CRMF_poposigningkey_init() to work with provider keys#11126

Merged
openssl-machine merged 1 commit intoopenssl:masterfrom
levitte:refactor-CRMF_poposigningkey_init-for-provider
Mar 9, 2020
Merged

Refactor CRMF_poposigningkey_init() to work with provider keys#11126
openssl-machine merged 1 commit intoopenssl:masterfrom
levitte:refactor-CRMF_poposigningkey_init-for-provider

Conversation

@levitte
Copy link
Member

@levitte levitte commented Feb 19, 2020

The code in this function was almost entirely a copy of the
functionality in ASN1_item_sign(), so it gets refactored to actually
call ASN1_item_sign(), and thereby automatically gets support for
EVP_PKEYs with only provider side keys.

@levitte levitte added branch: master Applies to master branch approval: review pending This pull request needs review by a committer labels Feb 19, 2020
@levitte levitte added this to the 3.0.0 milestone Feb 19, 2020
@levitte levitte force-pushed the refactor-CRMF_poposigningkey_init-for-provider branch from 45d2c0a to b7213a3 Compare February 24, 2020 18:03
@mattcaswell
Copy link
Member

Looks ok, but it would be good to have an opinion from @DDvO

@slontis slontis requested a review from DDvO March 2, 2020 22:36
Copy link
Contributor

@DDvO DDvO left a comment

Choose a reason for hiding this comment

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

Oops, so far I did not notice this PR and the review request for me.
Very good to take advantage of the existing function ASN1_item_sign().

@DDvO
Copy link
Contributor

DDvO commented Mar 7, 2020

Would be good if ASN1_item_sign() and ASN1_item_sign_ctx() were documented.

@DDvO DDvO 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 Mar 7, 2020
@levitte
Copy link
Member Author

levitte commented Mar 7, 2020

Yeah, I plan on doing that, just not nownownow.
Although, I really want to replace them with something better. They do too much in one function, and some bits are... a but clumsy.

@slontis slontis 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 Mar 9, 2020
The code in this function was almost entirely a copy of the
functionality in ASN1_item_sign(), so it gets refactored to actually
call ASN1_item_sign(), and thereby automatically gets support for
EVP_PKEYs with only provider side keys.

Reviewed-by: David von Oheimb <[email protected]>
(Merged from openssl#11126)
@levitte levitte force-pushed the refactor-CRMF_poposigningkey_init-for-provider branch from b7213a3 to db4b3d8 Compare March 9, 2020 05:24
@openssl-machine openssl-machine merged commit db4b3d8 into openssl:master Mar 9, 2020
@levitte levitte deleted the refactor-CRMF_poposigningkey_init-for-provider branch March 9, 2020 05:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Applies to master branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants