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

PROV: Make the DER to KEY deserializer decode parameters too #12569

Closed
wants to merge 1 commit into from

Conversation

levitte
Copy link
Member

@levitte levitte commented Aug 2, 2020

It should be noted that this may be dodgy if we ever encounter
parameter objects that look like something else. However, experience
with the OSSL_STORE 'file:' loader, which does exactly this kind of
thing, has worked fine so far.

A possibility could be that to decode parameters specifically, we
demand that there's an incoming data type specifying this, which
demands by extension that parameters can only come from a file format
that has the parameter type encoded, such as PEM. This would be a
future effort.

Fixes #12568

It should be noted that this may be dodgy if we ever encounter
parameter objects that look like something else.  However, experience
with the OSSL_STORE 'file:' loader, which does exactly this kind of
thing, has worked fine so far.

A possibility could be that to decode parameters specifically, we
demand that there's an incoming data type specifying this, which
demands by extension that parameters can only come from a file format
that has the parameter type encoded, such as PEM.  This would be a
future effort.

Fixes openssl#12568
@levitte levitte added branch: master Merge to master branch approval: review pending This pull request needs review by a committer labels Aug 2, 2020
@levitte levitte added this to In progress in 3.0 New Core + FIPS via automation Aug 2, 2020
@levitte
Copy link
Member Author

levitte commented Aug 2, 2020

Regarding the dodginess of this fix, another variant would be to implement a more robust structure to store key parameters. A bit like PKCS#8, but specifically for parameters alone. On the other hand, this is so very OpenSSL specific so demanding PEM origins may be the more viable option for now (does anyone store them in DER form?).

3.0 New Core + FIPS automation moved this from In progress to Reviewer approved Aug 3, 2020
@paulidale paulidale 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 Aug 3, 2020
@openssl-machine openssl-machine removed the approval: done This pull request has the required number of approvals label Aug 4, 2020
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@openssl-machine openssl-machine added the approval: ready to merge The 24 hour grace period has passed, ready to merge label Aug 4, 2020
@paulidale
Copy link
Contributor

Merged to master.

@paulidale paulidale closed this Aug 4, 2020
3.0 New Core + FIPS automation moved this from Reviewer approved to Done Aug 4, 2020
openssl-machine pushed a commit that referenced this pull request Aug 4, 2020
It should be noted that this may be dodgy if we ever encounter
parameter objects that look like something else.  However, experience
with the OSSL_STORE 'file:' loader, which does exactly this kind of
thing, has worked fine so far.

A possibility could be that to decode parameters specifically, we
demand that there's an incoming data type specifying this, which
demands by extension that parameters can only come from a file format
that has the parameter type encoded, such as PEM.  This would be a
future effort.

Fixes #12568

Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from #12569)
@levitte levitte deleted the fix-12568 branch August 4, 2020 08:21
swenkeratmicrosoft pushed a commit to swenkeratmicrosoft/openssl that referenced this pull request Sep 1, 2020
It should be noted that this may be dodgy if we ever encounter
parameter objects that look like something else.  However, experience
with the OSSL_STORE 'file:' loader, which does exactly this kind of
thing, has worked fine so far.

A possibility could be that to decode parameters specifically, we
demand that there's an incoming data type specifying this, which
demands by extension that parameters can only come from a file format
that has the parameter type encoded, such as PEM.  This would be a
future effort.

Fixes openssl#12568

Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from openssl#12569)
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 Merge to master branch
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

OSSL_DESERIALIZER_from_bio fails to read EC .pem encoded key files
3 participants