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

Fix PKCS7 parser failing to parse degenerated certificate messages #27435

Merged
merged 2 commits into from
Jun 11, 2024

Conversation

stevendpclark
Copy link
Contributor

A properly formatted PKCS7 message with unsigned multiple certificates is hitting a conditional check that the offset value is greater than the buffer. While that is true, in this use case we don't attempt to read from the buffer again so the check for buffer boundaries causes the parser to fail to read a message.

Tweak the offset checks to occur just before we are about to read from buf using the offset variable and not after we increment the offset variable. Augment the TestDegenerateCertificate to parse its own generated message along with OpenSSL that exposes the issue before the fixes to ber.go

@stevendpclark stevendpclark added backport/ent/1.15.x+ent Changes are backported to 1.15.x+ent backport/ent/1.16.x+ent Changes are backported to 1.16.x+ent backport/1.17.x labels Jun 11, 2024
@stevendpclark stevendpclark added this to the 1.15.11 milestone Jun 11, 2024
@stevendpclark stevendpclark requested review from fairclothjm and a team June 11, 2024 14:05
@stevendpclark stevendpclark self-assigned this Jun 11, 2024
@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Jun 11, 2024
Copy link

github-actions bot commented Jun 11, 2024

CI Results:
All Go tests succeeded! ✅

Copy link

github-actions bot commented Jun 11, 2024

Build Results:
All builds succeeded! ✅

Copy link
Contributor

@fairclothjm fairclothjm left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Copy link
Contributor

@victorr victorr left a comment

Choose a reason for hiding this comment

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

👍

@stevendpclark stevendpclark merged commit 64316fa into main Jun 11, 2024
82 of 83 checks passed
@stevendpclark stevendpclark deleted the stevendpclark/vault-28030-pkcs7-issues branch June 11, 2024 16:57
@stevendpclark stevendpclark added backport/ent/1.16.x+ent Changes are backported to 1.16.x+ent and removed backport/ent/1.16.x+ent Changes are backported to 1.16.x+ent labels Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/ent/1.15.x+ent Changes are backported to 1.15.x+ent backport/ent/1.16.x+ent Changes are backported to 1.16.x+ent backport/1.17.x hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants