Skip to content

Comments

Allow empty message in 448 sign.#6817

Closed
richsalz wants to merge 1 commit intoopenssl:masterfrom
richsalz:gh6800
Closed

Allow empty message in 448 sign.#6817
richsalz wants to merge 1 commit intoopenssl:masterfrom
richsalz:gh6800

Conversation

@richsalz
Copy link
Contributor

Thanks to GitHub user Bugcheckers for reporting this.

Fixes #6800

Thanks to GitHub user Bugcheckers for reporting this.
@richsalz richsalz added branch: master Applies to master branch approval: review pending This pull request needs review by a committer branch: 1.1.1 Applies to OpenSSL_1_1_1-stable branch (EOL) labels Jul 30, 2018
@richsalz richsalz self-assigned this Jul 30, 2018
Copy link
Contributor

@paulidale paulidale left a comment

Choose a reason for hiding this comment

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

I'm fine with this fix, but mightn't it make more sense for the length check to be in EVP_DigestUpdate instead of spread around?

Maybe await the outcome in #6800 ?

@paulidale paulidale added approval: done This pull request has the required number of approvals hold and removed approval: review pending This pull request needs review by a committer labels Jul 30, 2018
@richsalz
Copy link
Contributor Author

I think moving it to the EVP layer makes sense and am closing this. I don't think "hash of zero data" is reasonable or expected by openssl users.

@richsalz richsalz closed this Jul 31, 2018
@richsalz richsalz deleted the gh6800 branch July 31, 2018 15:34
mspncp added a commit to mspncp/openssl that referenced this pull request Aug 4, 2018
The majority of callers does not check for an empty buffer (buffer_length == 0) before calling
EVP_Digest*Update(), because all implementations handle this case correctly as no-op.
(See discussions in openssl#6800, openssl#6817, and openssl#6838.)
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 Applies to master branch branch: 1.1.1 Applies to OpenSSL_1_1_1-stable branch (EOL)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants