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

Drop SHA-1 fingerprints #3824

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Drop SHA-1 fingerprints #3824

wants to merge 1 commit into from

Conversation

edewata
Copy link
Contributor

@edewata edewata commented Nov 5, 2021

No description provided.

Copy link
Contributor

@ckelleyRH ckelleyRH left a comment

Choose a reason for hiding this comment

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

I think the time has come to un-defer this.

There are a few places where I think renewed examples in the javadoc would help.

Also, can we lose MD2 and MD5 at the same time and maybe add in SHA-384 fingerprints? I don't know what the consequences of doing this in the C code are. Are they conforming to some external expectations for algorithms or can we drop MD2/MD5; add SHA-384 there too?

@@ -1853,7 +1853,7 @@ public Hashtable<String, byte[]> makeFingerPrints(CRSPKIMessage req) {
Hashtable<String, byte[]> fingerprints = new Hashtable<>();

MessageDigest md;
String[] hashes = new String[] { "MD2", "MD5", "SHA1", "SHA256", "SHA512" };
Copy link
Contributor

Choose a reason for hiding this comment

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

Drop MD2/MD5, add SHA-384?

* MD5, MD2 and SHA1 hashes.
* MD5 and MD2 hashes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Drop MD2/MD5, add SHA-384?

* @return a String with fingerprints using the MD5, MD2 and SHA1 hashes.
* @return a String with fingerprints using the MD5 and MD2 hashes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Drop MD2/MD5, add SHA-384?

Comment on lines 780 to -784
* MD2: 78:7E:D1:F9:3E:AF:50:18:68:A7:29:50:C3:21:1F:71
*
* MD5: 0E:89:91:AC:40:50:F7:BE:6E:7B:39:4F:56:73:75:75
*
* SHA1: DC:D9:F7:AF:E2:83:10:B2:F7:0A:77:E8:50:E2:F7:D1:15:9A:9D:00
Copy link
Contributor

Choose a reason for hiding this comment

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

Make a new example with SHA-256, SHA-384 and SHA-512?

Comment on lines -791 to +789
String[] hashes = new String[] {"MD2", "MD5", "SHA1"};
String[] hashes = new String[] {"MD2", "MD5"};
Copy link
Contributor

Choose a reason for hiding this comment

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

Drop MD2/MD5, add SHA-256, SHA-384 and SHA-512?

Comment on lines 816 to -822
* MD2: 78:7E:D1:F9:3E:AF:50:18:68:A7:29:50:C3:21:1F:71
*
* MD5: 0E:89:91:AC:40:50:F7:BE:6E:7B:39:4F:56:73:75:75
*
* SHA1: DC:D9:F7:AF:E2:83:10:B2:F7:0A:77:E8:50:E2:F7:D1:15:9A:9D:00
Copy link
Contributor

Choose a reason for hiding this comment

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

Make a new example with SHA-256, SHA-384 and SHA-512?

Comment on lines -809 to +807
* MD5, MD2 and SHA1 hashes.
* MD5 and MD2 hashes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Drop MD2/MD5, add SHA-256, SHA-384 and SHA-512?

Comment on lines -814 to +812
* @return a String with fingerprints using the MD5, MD2 and SHA1 hashes.
* @return a String with fingerprints using the MD5 and MD2 hashes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Drop MD2/MD5, add SHA-256, SHA-384 and SHA-512?

Comment on lines -828 to +824
String[] hashes = new String[] { "MD2", "MD5", "SHA1", "SHA256", "SHA512" };
String[] hashes = new String[] { "MD2", "MD5", "SHA256", "SHA512" };
Copy link
Contributor

Choose a reason for hiding this comment

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

Drop MD2/MD5, add SHA-256, SHA-384 and SHA-512?

@ckelleyRH
Copy link
Contributor

I started commenting on individual lines before I realised that I could probably just summarise in a few lines, there are still some "unmarked" places where I didn't leave comments.

@edewata
Copy link
Contributor Author

edewata commented Apr 13, 2022

@ckelleyRH Thanks for your comments!
@ladycfu Do you have any objections about this PR and the suggested changes?

@ckelleyRH
Copy link
Contributor

@edewata @ladycfu - bump

@edewata
Copy link
Contributor Author

edewata commented Jul 8, 2022

I'd have to defer to @ladycfu.

@ckelleyRH
Copy link
Contributor

@edewata shall we close this?

@edewata
Copy link
Contributor Author

edewata commented Mar 15, 2023

Let's check with @ladycfu.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants