Skip to content

Implement pem_read_key directly through OSSL_DECODER#15045

Closed
t8m wants to merge 2 commits intoopenssl:masterfrom
t8m:pem-read-regression
Closed

Implement pem_read_key directly through OSSL_DECODER#15045
t8m wants to merge 2 commits intoopenssl:masterfrom
t8m:pem-read-regression

Conversation

@t8m
Copy link
Member

@t8m t8m commented Apr 27, 2021

Using OSSL_STORE is too heavy and breaks things.

There were also needed various fixes mainly for missing proper
handling of the SM2 keys in the OSSL_DECODER.

Fixes #14788

Checklist
  • tests are added or updated

@t8m t8m added the branch: master Applies to master branch label Apr 27, 2021
@t8m
Copy link
Member Author

t8m commented Apr 27, 2021

This is still WIP because there is missing legacy PEM handling.

@t8m
Copy link
Member Author

t8m commented Apr 27, 2021

This is still WIP because there is missing legacy PEM handling.

As reflected in the external GOST engine test failure.

@beldmit
Copy link
Member

beldmit commented Apr 27, 2021

As reflected in the external GOST engine test failure.

Yes :)

@t8m t8m closed this Apr 27, 2021
@t8m t8m reopened this Apr 27, 2021
@t8m t8m changed the title WIP: Implement pem_read_key directly through OSSL_DECODER Implement pem_read_key directly through OSSL_DECODER Apr 27, 2021
@t8m t8m added the approval: review pending This pull request needs review by a committer label Apr 27, 2021
@t8m
Copy link
Member Author

t8m commented Apr 27, 2021

Ready for review now.

@t8m
Copy link
Member Author

t8m commented Apr 27, 2021

I do not think the non-caching build failure is relevant. This seems to be some intermittent leak in the threads test.

@t8m
Copy link
Member Author

t8m commented May 4, 2021

Ping for reviews

@t8m t8m force-pushed the pem-read-regression branch 2 times, most recently from dc12f9c to 5a1d7eb Compare May 7, 2021 14:00
@t8m
Copy link
Member Author

t8m commented May 7, 2021

Rebased again, ping again for reviews!

@t8m t8m added this to the 3.0.0 beta1 milestone May 7, 2021
Copy link
Contributor

@paulidale paulidale left a comment

Choose a reason for hiding this comment

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

One formatting NIT.

Copy link
Contributor

Choose a reason for hiding this comment

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

blank line

@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 May 12, 2021
@openssl-machine openssl-machine removed the approval: done This pull request has the required number of approvals label May 13, 2021
@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 May 13, 2021
@t8m t8m force-pushed the pem-read-regression branch from 5a1d7eb to e9949cb Compare May 13, 2021 09:45
@t8m
Copy link
Member Author

t8m commented May 13, 2021

Rebased to resolve the conflict in checksums

@t8m t8m force-pushed the pem-read-regression branch from e9949cb to cff5ff4 Compare May 13, 2021 10:16
@t8m
Copy link
Member Author

t8m commented May 13, 2021

One more clean rebase to test the FIPS checksums action

Using OSSL_STORE is too heavy and breaks things.

There were also needed various fixes mainly for missing proper
handling of the SM2 keys in the OSSL_DECODER.

Fixes openssl#14788
@t8m t8m force-pushed the pem-read-regression branch from cff5ff4 to d16f8f7 Compare May 13, 2021 11:06
@t8m
Copy link
Member Author

t8m commented May 13, 2021

And one more

@github-actions github-actions bot added the severity: fips change The pull request changes FIPS provider sources label May 13, 2021
@t8m
Copy link
Member Author

t8m commented May 13, 2021

Hooray! The labeling works.

openssl-machine pushed a commit that referenced this pull request May 13, 2021
Using OSSL_STORE is too heavy and breaks things.

There were also needed various fixes mainly for missing proper
handling of the SM2 keys in the OSSL_DECODER.

Fixes #14788

Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #15045)
openssl-machine pushed a commit that referenced this pull request May 13, 2021
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #15045)
@t8m
Copy link
Member Author

t8m commented May 13, 2021

Merged to master. Thank you for the review!

@t8m t8m closed this May 13, 2021
devnexen pushed a commit to devnexen/openssl that referenced this pull request Jul 7, 2021
Using OSSL_STORE is too heavy and breaks things.

There were also needed various fixes mainly for missing proper
handling of the SM2 keys in the OSSL_DECODER.

Fixes openssl#14788

Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from openssl#15045)
devnexen pushed a commit to devnexen/openssl that referenced this pull request Jul 7, 2021
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from openssl#15045)
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 severity: fips change The pull request changes FIPS provider sources

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Calling PEM_read_bio_PrivateKey() and PEM_read_bio_X509() on the same BIO is broken

5 participants