Skip to content

The SHA256 is not a mandatory digest for DSA.#9015

Closed
t8m wants to merge 1 commit intoopenssl:masterfrom
t8m:dsa-md-nid
Closed

The SHA256 is not a mandatory digest for DSA.#9015
t8m wants to merge 1 commit intoopenssl:masterfrom
t8m:dsa-md-nid

Conversation

@t8m
Copy link
Member

@t8m t8m commented May 27, 2019

The #7408 implemented mandatory digest checking in TLS.
However this broke compatibility of DSS support with GnuTLS
which supports only SHA1 with DSS.

There is no reason why SHA256 would be a mandatory digest
for DSA as other digests in SHA family can be used as well.

The openssl#7408 implemented mandatory digest checking in TLS.
However this broke compatibility of DSS support with GnuTLS
which supports only SHA1 with DSS.

There is no reason why SHA256 would be a mandatory digest
for DSA as other digests in SHA family can be used as well.
@t8m t8m added branch: master Applies to master branch branch: 1.1.1 Applies to OpenSSL_1_1_1-stable branch (EOL) labels May 27, 2019
@paulidale
Copy link
Contributor

1.1.1 seems to have this fix already.

@slontis
Copy link
Member

slontis commented May 28, 2019

I am not sure how it was backported to multiple branches in #7408, but not put into master?

@t8m
Copy link
Member Author

t8m commented May 28, 2019

1.1.1 seems to have this fix already.

I do not see it. Did you look at ec_ameth.c perhaps?

@slontis
Copy link
Member

slontis commented May 28, 2019

#7408 mentions these 2..
1.0.2: #7610
1.1.1: #7609

@t8m t8m added the approval: done This pull request has the required number of approvals label May 28, 2019
@t8m
Copy link
Member Author

t8m commented May 28, 2019

Yes, the #7408 is backported to both 1.1.1 and 1.0.2. But DSA has SHA-256 as mandatory on all active branches. Given this is a bugfix and not security issue fix, I am going to put it on master and 1.1.1 only.
The Travis failure is unrelated.

@t8m t8m closed this May 28, 2019
@t8m t8m deleted the dsa-md-nid branch May 28, 2019 10:03
@mattcaswell
Copy link
Member

Did you mean to close this...its not on master yet?

@mspncp
Copy link
Contributor

mspncp commented May 28, 2019

@mattcaswell for some reason, the master branch got force-pushed and Tomas' commit was lost.

@mspncp
Copy link
Contributor

mspncp commented May 28, 2019

You can find a GitHub notification about this event at #9029

@mattcaswell
Copy link
Member

@mattcaswell for some reason, the master branch got force-pushed and Thomas' commit was lost.

Ah. My guess is that @t8m pushed to the github mirror master rather than the real master. Then when another commit got pushed to the real master it got overwritten when the mirror was updated.

@mattcaswell
Copy link
Member

Similarly I don't see this commit in the 1.1.1 branch, so my guess is that the same thing happened there.

@mspncp
Copy link
Contributor

mspncp commented May 28, 2019

That sounds like a reasonable explanation.

@mspncp
Copy link
Contributor

mspncp commented May 28, 2019

Yes, there they are, thrown off the track:

  • 6acf605b05ede307f4cd8ad0bef82af5c6bba2b7
  • bbf4e0eb548ab98d1eb4eaf1c05381a89b2cf5da

@t8m
Copy link
Member Author

t8m commented May 28, 2019

Ah of course, not sure how I made this :( I am going to push to the right repo now.

levitte pushed a commit that referenced this pull request May 28, 2019
The #7408 implemented mandatory digest checking in TLS.
However this broke compatibility of DSS support with GnuTLS
which supports only SHA1 with DSS.

There is no reason why SHA256 would be a mandatory digest
for DSA as other digests in SHA family can be used as well.

Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from #9015)
levitte pushed a commit that referenced this pull request May 28, 2019
The #7408 implemented mandatory digest checking in TLS.
However this broke compatibility of DSS support with GnuTLS
which supports only SHA1 with DSS.

There is no reason why SHA256 would be a mandatory digest
for DSA as other digests in SHA family can be used as well.

Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from #9015)

(cherry picked from commit cd4c83b)
@t8m
Copy link
Member Author

t8m commented May 28, 2019

Hopefully the mess is cleared up now. I've mistakenly cloned locally wrong repo for the pushes before.

@mspncp
Copy link
Contributor

mspncp commented May 28, 2019

Yes, looks good now.

themiron pushed a commit to RMerl/asuswrt-merlin.ng that referenced this pull request Jun 12, 2019
The #7408 implemented mandatory digest checking in TLS.
However this broke compatibility of DSS support with GnuTLS
which supports only SHA1 with DSS.

There is no reason why SHA256 would be a mandatory digest
for DSA as other digests in SHA family can be used as well.

Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from openssl/openssl#9015)

(cherry picked from commit cd4c83b52423008391b50abcccf18a7d8fcce03b)
john9527 pushed a commit to john9527/asuswrt-merlin that referenced this pull request Feb 13, 2020
The #7408 implemented mandatory digest checking in TLS.
However this broke compatibility of DSS support with GnuTLS
which supports only SHA1 with DSS.

There is no reason why SHA256 would be a mandatory digest
for DSA as other digests in SHA family can be used as well.

Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from openssl/openssl#9015)

(cherry picked from commit cd4c83b52423008391b50abcccf18a7d8fcce03b)
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.

5 participants