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

Add null digest implementation to the default provider #17016

Closed
wants to merge 2 commits into from

Conversation

t8m
Copy link
Member

@t8m t8m commented Nov 12, 2021

This is necessary to keep compatibility with 1.1.1.

Fixes #16660

Checklist
  • tests are added or updated

This is necessary to keep compatibility with 1.1.1.

Fixes openssl#16660
@t8m t8m added branch: master Merge to master branch approval: review pending This pull request needs review by a committer triaged: bug The issue/pr is/fixes a bug branch: 3.0 Merge to openssl-3.0 branch labels Nov 12, 2021
@github-actions github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Nov 12, 2021
}

#undef PROV_FUNC_DIGEST_FINAL
#define PROV_FUNC_DIGEST_FINAL(name, dgstsize, fin) \
Copy link
Member

Choose a reason for hiding this comment

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

I'm wonder why this custom version of PROV_FUNC_DIGEST_FINAL is necessary. The only difference seems to be the check outsz >= dgstsize which presumably will always be true for the NULL digest since dgstize is 0. Or perhaps this triggers compilation warnings? Is that the justification? Possibly a comment explaining that if that is the case.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this causes compilation warnings otherwise.

@mattcaswell mattcaswell 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 Nov 12, 2021
@openssl-machine openssl-machine removed the approval: done This pull request has the required number of approvals label Nov 13, 2021
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@openssl-machine openssl-machine added the approval: ready to merge The 24 hour grace period has passed, ready to merge label Nov 13, 2021
rhuijben pushed a commit to rhuijben/openssl that referenced this pull request Nov 15, 2021
This is necessary to keep compatibility with 1.1.1.

Fixes #16660

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from openssl/openssl#17016)

(cherry picked from commit bef9b48)
rhuijben pushed a commit to rhuijben/openssl that referenced this pull request Nov 15, 2021
This is necessary to keep compatibility with 1.1.1.

Fixes #16660

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from openssl/openssl#17016)
@t8m
Copy link
Member Author

t8m commented Nov 15, 2021

Merged to master and 3.0 branches. Thank you for the review.

@t8m t8m closed this Nov 15, 2021
ramikhaldi pushed a commit to ramikhaldi/openssl that referenced this pull request Nov 21, 2021
This is necessary to keep compatibility with 1.1.1.

Fixes openssl#16660

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from openssl#17016)
@t8m t8m deleted the null-md-prov branch May 12, 2022 12:40
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 branch: 3.0 Merge to openssl-3.0 branch severity: fips change The pull request changes FIPS provider sources triaged: bug The issue/pr is/fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using EVP_md_null results in crash in OpenSSL 3
3 participants