Skip to content

Replace some of the ERR_clear_error() calls in libcrypto with mark calls#15253

Closed
t8m wants to merge 1 commit intoopenssl:masterfrom
t8m:no-clear-error
Closed

Replace some of the ERR_clear_error() calls in libcrypto with mark calls#15253
t8m wants to merge 1 commit intoopenssl:masterfrom
t8m:no-clear-error

Conversation

@t8m
Copy link
Member

@t8m t8m commented May 12, 2021

Fixes #15219

This does not replace all of them as removal of some of the ERR_clear_error() would require more substantial changes or might be potentially a security problem.

@t8m t8m added approval: review pending This pull request needs review by a committer branch: master Applies to master branch triaged: refactor The issue/pr requests/implements refactoring labels May 12, 2021
@levitte levitte 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
Copy link
Member

@slontis slontis left a comment

Choose a reason for hiding this comment

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

I realize we have run out of time - but test coverage for these would probably be a good idea at some point.

@t8m t8m closed this May 13, 2021
@t8m t8m reopened this May 13, 2021
@t8m t8m 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 May 13, 2021
openssl-machine pushed a commit that referenced this pull request May 13, 2021
Fixes #15219

Reviewed-by: Richard Levitte <[email protected]>
Reviewed-by: Paul Dale <[email protected]>
Reviewed-by: Shane Lontis <[email protected]>
(Merged from #15253)
@t8m
Copy link
Member Author

t8m commented May 13, 2021

Merged to master. Thank you for the reviews.

@t8m t8m closed this May 13, 2021
@DDvO
Copy link
Contributor

DDvO commented May 13, 2021

I'm astonished to see how simple it was to solve #15219 👍

@DDvO
Copy link
Contributor

DDvO commented May 14, 2021

This change revealed some spurious error queue entries in OSSL_STORE when loading private keys, e.g.,

ossl_store_handle_load_result:crypto/store/store_result.c:151:error: unsupported:
asn1_check_tlen:crypto/asn1/tasn_dec.c:1156:error: wrong tag:
asn1_item_embed_d2i:crypto/asn1/tasn_dec.c:322:error: nested asn1 error:Type=EC_PRIVATEKEY

I'll try to come up with a fix.

devnexen pushed a commit to devnexen/openssl that referenced this pull request Jul 7, 2021
Fixes openssl#15219

Reviewed-by: Richard Levitte <[email protected]>
Reviewed-by: Paul Dale <[email protected]>
Reviewed-by: Shane Lontis <[email protected]>
(Merged from openssl#15253)
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 triaged: refactor The issue/pr requests/implements refactoring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

asn1_d2i_read_bio() has brittle design depending on the error queue

5 participants