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

OSSL_CMP_certConf_cb(): fix regression on checking newly enrolled cert #20160

Closed
wants to merge 1 commit into from

Conversation

DDvO
Copy link
Contributor

@DDvO DDvO commented Jan 27, 2023

It was reported that a regression had been incurred by #14128:
the default certificate confirmation callback no more verified the newly enrolled cert,
while this still should be done if a trust store has been provided to this function.

This issue should have captured to some extent by an existing test case,
but the expected failure happend for not the right reason:
it was due to a cert file not being present rather than the intended cert being expired.

While fixing this, also I added corresponding tests and to this end had to update the test server credentials
and remove a workaround in the test configuration (which so far simply had disabled cert expiration checks).
Also made the doc of the callback function and of the related -out_trusted CLI option a little more precise.

Checklist
  • documentation is added or updated
  • tests are added or updated

Also add corresponding tests and to this end update credentials
@DDvO DDvO added branch: master Merge to master branch approval: review pending This pull request needs review by a committer approval: otc review pending This pull request needs review by an OTC member triaged: bug The issue/pr is/fixes a bug triaged: documentation The issue/pr deals with documentation (errors) branch: 3.0 Merge to openssl-3.0 branch branch: 3.1 Merge to openssl-3.1 tests: present The PR has suitable tests present labels Jan 27, 2023
@t8m t8m removed the approval: otc review pending This pull request needs review by an OTC member label Jan 28, 2023
@paulidale paulidale removed the approval: review pending This pull request needs review by a committer label Feb 5, 2023
@DDvO DDvO added the approval: done This pull request has the required number of approvals label Feb 9, 2023
@openssl-machine
Copy link
Collaborator

24 hours has passed since 'approval: done' was set, but this PR has failing CI tests. Once the tests pass it will get moved to 'approval: ready to merge' automatically, alternatively please review and set the label manually.

openssl-machine pushed a commit that referenced this pull request Feb 13, 2023
Also add corresponding tests and to this end update credentials

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
(Merged from #20160)
@DDvO
Copy link
Contributor Author

DDvO commented Feb 13, 2023

Merged to master, 3.0, and 3.1 - thanks @t8m and @paulidale for the approvals.

openssl-machine pushed a commit that referenced this pull request Feb 13, 2023
Also add corresponding tests and to this end update credentials

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
(Merged from #20160)

(cherry picked from commit 6b58f49)
openssl-machine pushed a commit that referenced this pull request Feb 13, 2023
Also add corresponding tests and to this end update credentials

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
(Merged from #20160)

(cherry picked from commit 6b58f49)
@DDvO DDvO closed this Feb 13, 2023
hardik05 pushed a commit to hardik05/openssl that referenced this pull request Feb 22, 2023
Also add corresponding tests and to this end update credentials

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
(Merged from openssl#20160)
hardik05 pushed a commit to hardik05/openssl that referenced this pull request Feb 22, 2023
Also add corresponding tests and to this end update credentials

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
(Merged from openssl#20160)
hardik05 pushed a commit to hardik05/openssl that referenced this pull request Feb 22, 2023
Also add corresponding tests and to this end update credentials

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
(Merged from openssl#20160)
DDvO added a commit to mpeylo/cmpossl that referenced this pull request Jun 29, 2023
…t - complete earlier inclusion

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
(Merged from openssl#20160)
DDvO added a commit to siemens/gencmpclient that referenced this pull request Jun 29, 2023
…lled cert - complete earlier inclusion

Also add corresponding tests and to this end update credentials

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
(Merged from openssl/openssl#20160)
DDvO added a commit to siemens/gencmpclient that referenced this pull request Jul 13, 2023
…lled cert - complete earlier inclusion

Also add corresponding tests and to this end update credentials

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
(Merged from openssl/openssl#20160)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: done This pull request has the required number of approvals branch: master Merge to master branch branch: 3.0 Merge to openssl-3.0 branch branch: 3.1 Merge to openssl-3.1 tests: present The PR has suitable tests present triaged: bug The issue/pr is/fixes a bug triaged: documentation The issue/pr deals with documentation (errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants