Skip to content

Updates re MD5/SHA1 usage. #2917

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Aug 7, 2019
Merged

Updates re MD5/SHA1 usage. #2917

merged 4 commits into from
Aug 7, 2019

Conversation

@TimShererWithAquent
Copy link
Contributor

@TimShererWithAquent TimShererWithAquent commented Aug 2, 2019 •
edited by Thraka
Loading

Summary

Add recommendation to avoid MD5/SHA1.

Fixes 1572540

Contributes to dotnet/docs#13441

@Thraka @LizCasey

Andy - Because of the number of files involved, I'm going to do more than one PR. This one and the next one will be about 25 files.

@Thraka
Copy link
Contributor

Thraka commented Aug 2, 2019

Looks good, thanks!

@Thraka Thraka added the vendor-project Indicates the issue/pr is related to a vendor project. label Aug 2, 2019
@Thraka
Copy link
Contributor

Thraka commented Aug 2, 2019 •
edited
Loading

Will just leave open for a bit so that @karelz can do a quick review he has time.

@mairaw
Copy link
Contributor

mairaw commented Aug 6, 2019

@bartonjs this is related to the security project that we discussed more than a year ago. Would you be the right person to review this?

Also, the changes where the sentence was added as the only summary for some APIs is wrong. The summary should explain what the API is and then the remarks for that API or the remarks for the enumeration in the case of enum fields should call out that.

@Thraka
Copy link
Contributor

Thraka commented Aug 6, 2019

@bartonjs this is related to the security project that we discussed more than a year ago. Would you be the right person to review this?

Also, the changes where the sentence was added as the only summary for some APIs is wrong. The summary should explain what the API is and then the remarks for that API or the remarks for the enumeration in the case of enum fields should call out that.

Good catch!

Thraka
Thraka reviewed Aug 7, 2019
View reviewed changes
xml/System.Security.Authentication/HashAlgorithmType.xml Outdated
<MemberValue>32771</MemberValue>
<Docs>
<summary>The Message Digest 5 (MD5) hashing algorithm.</summary>
<summary>The Message Digest 5 (MD5) hashing algorithm. Due to collision problems with MD5, Microsoft recommends SHA256.</summary>
Copy link
Contributor

@Thraka Thraka Aug 7, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

These two changes in this file should also move to remarks since they are enum types.

I think after this change it should be good to merge. Thanks again!

@Thraka Thraka merged commit 7bc1ba2 into dotnet:master Aug 7, 2019
@TimShererWithAquent TimShererWithAquent deleted the us1572540a branch October 16, 2019 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@karelz karelz Awaiting requested review from karelz

1 more reviewer

@Thraka Thraka Thraka approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

vendor-project Indicates the issue/pr is related to a vendor project.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@TimShererWithAquent @Thraka @mairaw