Skip to content

Comments

Rework the "by store" X509_LOOKUP method to open the given URI early [3.4]#27550

Closed
levitte wants to merge 4 commits intoopenssl:openssl-3.4from
levitte:openssl-3.4
Closed

Rework the "by store" X509_LOOKUP method to open the given URI early [3.4]#27550
levitte wants to merge 4 commits intoopenssl:openssl-3.4from
levitte:openssl-3.4

Conversation

@levitte
Copy link
Member

@levitte levitte commented May 4, 2025

This is a backport of #27529 to OpenSSL 3.4, with test fixup

levitte and others added 4 commits May 4, 2025 08:59
The cached X509_LOOKUP method data is no longer just the URI, but now
includes the OSSL_STORE_CTX pointer, and required parameters to reopen
the URI at any time.  cache_objects() is modified to handle this, and
only (re)open the URI when it wasn't previously opened, or when it was
closed by an earlier call.

This way, we can call OSSL_STORE_open_ex() in by_store_ctrl_ex(), and
get to see possible errors when the URI is loaded.

This assumes that if the URI could be opened once, it can be opened
again.

Fixes openssl#27461

Reviewed-by: David von Oheimb <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#27529)

(cherry picked from commit 0c48ee2)
Originally from openssl#27507, with some
changes.

Co-authored-by: Richard Levitte <[email protected]>

Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Richard Levitte <[email protected]>
(Merged from openssl#27529)

(cherry picked from commit 927deba)
It was used to pass libctx and propq, which would override the
corresponding values passed to by_store_ctrl_ex().  This wasn't
really reasonable to do either way, as it could potentially be a
surprise to the user, who can reasonably expect that the URI is
opened with the libctx and propq that was passed with the URI, and
not with those passed later.

Reviewed-by: David von Oheimb <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#27529)

(cherry picked from commit af5952d)
@levitte levitte added approval: review pending This pull request needs review by a committer triaged: bug The issue/pr is/fixes a bug tests: present The PR has suitable tests present branch: 3.4 Applies to openssl-3.4 labels May 4, 2025
@levitte
Copy link
Member Author

levitte commented May 5, 2025

Ah, this suffers from the same code style check failure as #27529 did. Ok, waiving style

@levitte levitte added the style: waived exempted from style checks label May 5, 2025
@levitte
Copy link
Member Author

levitte commented May 5, 2025

Close/open to kick the workflows

@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 May 5, 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 May 6, 2025
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@levitte
Copy link
Member Author

levitte commented May 6, 2025

Merged

0f57419 Rework the "by store" X509_LOOKUP method to open the given URI early
2a50e24 Add test_verify tests
76fb0e2 Drop "by store"'s by_store_subject_ex()

openssl-machine pushed a commit that referenced this pull request May 6, 2025
The cached X509_LOOKUP method data is no longer just the URI, but now
includes the OSSL_STORE_CTX pointer, and required parameters to reopen
the URI at any time.  cache_objects() is modified to handle this, and
only (re)open the URI when it wasn't previously opened, or when it was
closed by an earlier call.

This way, we can call OSSL_STORE_open_ex() in by_store_ctrl_ex(), and
get to see possible errors when the URI is loaded.

This assumes that if the URI could be opened once, it can be opened
again.

Fixes #27461

Reviewed-by: David von Oheimb <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from #27550)
openssl-machine pushed a commit that referenced this pull request May 6, 2025
Originally from #27507, with some
changes.

Co-authored-by: Richard Levitte <[email protected]>

Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Richard Levitte <[email protected]>
(Merged from #27550)
openssl-machine pushed a commit that referenced this pull request May 6, 2025
It was used to pass libctx and propq, which would override the
corresponding values passed to by_store_ctrl_ex().  This wasn't
really reasonable to do either way, as it could potentially be a
surprise to the user, who can reasonably expect that the URI is
opened with the libctx and propq that was passed with the URI, and
not with those passed later.

Reviewed-by: David von Oheimb <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from #27550)
@levitte levitte closed this May 6, 2025
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: 3.4 Applies to openssl-3.4 style: waived exempted from style checks tests: present The PR has suitable tests present triaged: bug The issue/pr is/fixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants