Skip to content

Remove premature context updates in TSSslSecretSet().#9528

Closed
ywkaras wants to merge 1 commit intoapache:masterfrom
ywkaras:rm_premature_ctx
Closed

Remove premature context updates in TSSslSecretSet().#9528
ywkaras wants to merge 1 commit intoapache:masterfrom
ywkaras:rm_premature_ctx

Conversation

@ywkaras
Copy link
Contributor

@ywkaras ywkaras commented Mar 16, 2023

This PR partially reverts PR #8368. SSL contexts (SSL_CTX) should only be updated by TSSslSecretUpdate() (which is only called for the main, not the related secret). The update in TSSslSecretSet() is not merely redundant, it causes errors. When an update is done, OpenSSL will check that the cert public key matches the private key. Since TSSslSecretSet() can only set one at time, if it also updates, they will not match.

Contexts for a loading confguration never have to be updated, becasue no SSL_CTX's have been created using it yet.

This PR partially reverts PR apache#8368.  SSL contexts (SSL_CTX) should only be updated by TSSslSecretUpdate()
(which is only called for the main, not the related secret).  The update in TSSslSecretSet() is not merely redundant,
it causes errors.  When an update is done, OpenSSL will check that the cert public key matches the private key.  Since
TSSslSecretSet() can only set one at time, if it also updates, they will not match.

Contexts for a loading confguration never have to be updated, becasue no SSL_CTX's have been created using it yet.
@ywkaras ywkaras self-assigned this Mar 16, 2023
@ywkaras ywkaras marked this pull request as draft March 16, 2023 20:58
@ywkaras
Copy link
Contributor Author

ywkaras commented Mar 16, 2023

Waiting on a confirmation for a Yahoo prod engineer that this fixes the problem they're having with one of our plugins.

@ywkaras
Copy link
Contributor Author

ywkaras commented Apr 8, 2023

Leaving this as a draft, until the TSSslSecretXxx functions are fixed ( see #9562 ).

@bneradt
Copy link
Contributor

bneradt commented May 12, 2023

[approve ci cmake]

@ezelkow1
Copy link
Member

[approve ci centos]

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. Marking it stale to flag it for further consideration by the community.

@github-actions github-actions bot added the Stale label Aug 18, 2023
@github-actions github-actions bot closed this Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants