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

X509_digest_sig: Handle RSA-PSS and EDDSA certificates #15618

Closed
wants to merge 3 commits into from

Conversation

t8m
Copy link
Member

@t8m t8m commented Jun 4, 2021

Identify digest from sigalg params for RSA-PSS and fallback
to SHA-256 for EDDSA.

Fixes #15477

Checklist
  • documentation is added or updated

Identify digest from sigalg params for RSA-PSS and fallback
to SHA-256 for EDDSA.

Fixes openssl#15477
@t8m t8m requested a review from DDvO June 4, 2021 11:59
doc/man3/X509_digest.pod Outdated Show resolved Hide resolved
doc/man3/X509_digest.pod Outdated Show resolved Hide resolved
@t8m
Copy link
Member Author

t8m commented Jun 4, 2021

fixup pushed

@t8m t8m added approval: review pending This pull request needs review by a committer branch: master Merge to master branch labels Jun 4, 2021
@t8m t8m added this to the 3.0.0 milestone Jun 4, 2021
Copy link
Contributor

@DDvO DDvO left a comment

Choose a reason for hiding this comment

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

I have some suggestions for improvements and a concern.

crypto/x509/x_all.c Outdated Show resolved Hide resolved
crypto/x509/x_all.c Show resolved Hide resolved
crypto/x509/x_all.c Show resolved Hide resolved
crypto/x509/x_all.c Show resolved Hide resolved
Copy link
Contributor

@DDvO DDvO left a comment

Choose a reason for hiding this comment

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

LGTM

@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 Jun 7, 2021
@DDvO
Copy link
Contributor

DDvO commented Jun 7, 2021

Sorry, I forgot to set the "approval:done". Can this still be merged now?

@t8m
Copy link
Member Author

t8m commented Jun 7, 2021

We can wait a day. There is no hurry.

@openssl-machine
Copy link
Collaborator

24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually.

@t8m t8m 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 Jun 8, 2021
openssl-machine pushed a commit that referenced this pull request Jun 8, 2021
Identify digest from sigalg params for RSA-PSS and fallback
to SHA-256 for EDDSA.

Fixes #15477

Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
(Merged from #15618)
@t8m
Copy link
Member Author

t8m commented Jun 8, 2021

Merged to master. Thank you for the reviews.

@t8m t8m closed this Jun 8, 2021
devnexen pushed a commit to devnexen/openssl that referenced this pull request Jul 7, 2021
Identify digest from sigalg params for RSA-PSS and fallback
to SHA-256 for EDDSA.

Fixes openssl#15477

Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
(Merged from openssl#15618)
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: master Merge to master branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

X509_digest_sig reports unsupported algorithm with RSA-PSS
4 participants