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

Address issue with embedded certificates in JWT x5c #6440

Merged

Conversation

achamayou
Copy link
Member

@achamayou achamayou commented Aug 14, 2024

There is an issue with the way the Verifier_OpenSSL::Verifier_OpenSSL handles being constructed from either a DER or a PEM.

The code initially tries to construct and X509 type with Unique_X509(certbio, true /* pem */) before retrying with Unique_X509(certbio, false /* pem */) in case the value is DER. Unique_X509() calls PEM_read_bio_X509() internally, which ignores all input prior to -----BEGIN CERTIFICATE-----, and parses from there.

If a certificate embeds other certificates, and is provided in DER form, this results in mis-parsing as the nested certificate.

Longer term, we should avoid calls that take DER|PEM and have a static distinction. For the time being, this adds a validation check to the content before handing it to PEM_read_bio_X509().

@ad-l ad-l self-requested a review August 14, 2024 15:59
@achamayou achamayou changed the title Workaround issue with embedded certificates in JWT x5c Address issue with embedded certificates in JWT x5c Aug 15, 2024
@achamayou achamayou marked this pull request as ready for review August 15, 2024 09:50
@achamayou achamayou requested a review from a team August 15, 2024 09:50
@achamayou achamayou added auto-backport Automatically backport this PR to LTS branch 5.x-todo PRs which should be backported to 5.x labels Aug 15, 2024
CHANGELOG.md Outdated Show resolved Hide resolved
@achamayou achamayou added this pull request to the merge queue Aug 15, 2024
Merged via the queue into microsoft:main with commit 4bb3c1e Aug 15, 2024
7 checks passed
@achamayou achamayou deleted the fix_x5c_handling_with_nested_certs branch August 15, 2024 14:40
ghost pushed a commit that referenced this pull request Aug 15, 2024
@ghost ghost added the backported This PR was successfully backported to LTS branch label Aug 15, 2024
achamayou pushed a commit that referenced this pull request Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5.x-todo PRs which should be backported to 5.x auto-backport Automatically backport this PR to LTS branch backported This PR was successfully backported to LTS branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants