[CA5350] Scope-suppress weak-crypto warning for SHA1 file-integrity checks - #12347
Open
mauroa wants to merge 1 commit into
Open
[CA5350] Scope-suppress weak-crypto warning for SHA1 file-integrity checks#12347mauroa wants to merge 1 commit into
mauroa wants to merge 1 commit into
Conversation
…hecks in DownloadUtils
Addresses CA5350 ("Do Not Use Weak Cryptographic Algorithms") flagged by
DevDiv work item 3012464 on CreateHashAlgorithm in DownloadUtils.cs.
SHA1 here is not a security or authentication boundary: it verifies
externally-published Android SDK file-integrity checksums (SdkManager.Manifest.cs)
and computes Android SDK license hashes matching Google's on-disk licenses/ format
(SdkManager.Licenses.cs ComputeLicenseHash). Removing SHA1 would break
checksum/license verification, so behavior is preserved and the warning is
scope-suppressed to just the SHA1.Create() switch arm.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses CA5350 warnings around SHA1 usage in DownloadUtils.cs by adding a narrowly-scoped warning suppression and an in-code justification. This aligns with the Android SDK tooling needs where SHA1 is used for compatibility with Google-published checksum/license formats rather than as an authentication boundary.
Changes:
- Adds a scoped
#pragma warning disable/restore CA5350around theSHA1.Create()switch arm inCreateHashAlgorithm. - Adds inline comments explaining why SHA1 is still required for checksum/license compatibility.
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
99
to
+106
| static HashAlgorithm CreateHashAlgorithm (ChecksumType checksumType) => checksumType switch { | ||
| ChecksumType.Sha256 => (HashAlgorithm) SHA256.Create (), | ||
| // SHA1 is used only to verify externally-published file-integrity checksums (Android SDK | ||
| // repository manifests publish SHA1 checksums) and to match Google's on-disk license hashes, | ||
| // not as a security or authentication boundary. See ChecksumType / SdkManager.Manifest.cs. | ||
| #pragma warning disable CA5350 // Do Not Use Weak Cryptographic Algorithms | ||
| ChecksumType.Sha1 => SHA1.Create (), | ||
| #pragma warning restore CA5350 |
Member
There was a problem hiding this comment.
can you check if the NoWarn is really there and we wouldn't need this?
Comment on lines
+101
to
+103
| // SHA1 is used only to verify externally-published file-integrity checksums (Android SDK | ||
| // repository manifests publish SHA1 checksums) and to match Google's on-disk license hashes, | ||
| // not as a security or authentication boundary. See ChecksumType / SdkManager.Manifest.cs. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses CA5350 (
Do Not Use Weak Cryptographic Algorithms) flagged by DevDiv work item 3012464 ([roslynanalyzers:Warning]) onCreateHashAlgorithminDownloadUtils.cs.Rationale
SHA1 here is not a security or authentication boundary. It is used only to:
SdkManager.Manifest.cs), andlicenses/format (SdkManager.Licenses.csComputeLicenseHash).Removing SHA1 would break checksum/license verification against Google-published data, so the algorithm is preserved and the warning is narrowly scope-suppressed to just the
SHA1.Create()switch arm with a clear justification.Originally scoped against the archived
dotnet/android-tools; retargeted here since that source now lives indotnet/android.