[FEATURE] Add SHA3 algorithms for .NET 8+#901
Conversation
Introduce SHA3-256, SHA3-384 and SHA3-512 enum members to HashAlgorithmType under NET8_0_OR_GREATER and register their corresponding SHA3 implementations in Security's algorithm map. This enables use of .NET 8+ built-in SHA3 hashing when targeting .NET 8 or later.
Reviewer's Guide.NET 8+ only support for SHA3 hash algorithms is added by extending the HashAlgorithmType enum and wiring those new enum values into the Security hash algorithm map, all under NET8_0_OR_GREATER preprocessor guards for backward compatibility. Sequence diagram for Security.Hash using SHA3 under .NET 8+sequenceDiagram
actor Developer
participant Security
participant HashAlgorithmMap
participant SHA3_256
Developer->>Security: Hash(value, Sha3_256)
Security->>HashAlgorithmMap: lookup Sha3_256
HashAlgorithmMap-->>Security: SHA3_256 instance
Security->>SHA3_256: ComputeHash(value bytes)
SHA3_256-->>Security: hash bytes
Security-->>Developer: hex string hash
Class diagram for updated hash types and Security mappingclassDiagram
class HashAlgorithmType {
<<enumeration>>
Md5
Sha1
Sha256
Sha384
Sha512
Sha3_256
Sha3_384
Sha3_512
}
class Security {
+static string Hash(string value, HashAlgorithmType type)
-static readonly Dictionary<HashAlgorithmType, HashAlgorithm> _algorithms
}
class SHA3_256 {
+static SHA3_256 Create()
}
class SHA3_384 {
+static SHA3_384 Create()
}
class SHA3_512 {
+static SHA3_512 Create()
}
Security --> HashAlgorithmType : uses
Security --> SHA3_256 : maps Sha3_256
Security --> SHA3_384 : maps Sha3_384
Security --> SHA3_512 : maps Sha3_512
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- If
HashAlgorithmTypevalues are ever serialized or persisted, consider assigning explicit numeric values to the new SHA3 entries, as the#if NET8_0_OR_GREATERblock will change the enum layout between target frameworks. - It may be safer to guard the SHA3 registrations not only with
NET8_0_OR_GREATERbut also with a runtime check (e.g.,CryptoConfig.AllowOnlyFipsAlgorithmsor platform capability) in case some .NET 8+ platforms do not support these algorithms.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- If `HashAlgorithmType` values are ever serialized or persisted, consider assigning explicit numeric values to the new SHA3 entries, as the `#if NET8_0_OR_GREATER` block will change the enum layout between target frameworks.
- It may be safer to guard the SHA3 registrations not only with `NET8_0_OR_GREATER` but also with a runtime check (e.g., `CryptoConfig.AllowOnlyFipsAlgorithms` or platform capability) in case some .NET 8+ platforms do not support these algorithms.Help me be more useful! Please click π or π on each comment and I'll use the feedback to improve your reviews.
WalkthroughThe cryptography module is extended to support SHA-3 hashing algorithms (256, 384, and 512-bit variants) conditionally for .NET 8 or later. New enum members are added to Changes
Estimated code review effortπ― 2 (Simple) | β±οΈ ~8 minutes Poem
π₯ Pre-merge checks | β 5β Passed checks (5 passed)
βοΈ Tip: You can configure your own custom pre-merge checks in the settings. β¨ Finishing Touchesπ§ͺ Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and canβt be posted inline due to platform limitations.
β οΈ Outside diff range comments (1)
Src/CrispyWaffle/Cryptography/Security.cs (1)
153-165:β οΈ Potential issue | π΄ CriticalCritical:
SHA3_*.Create()throws on platforms without SHA-3 support, breaking access to the entireSecurityclass.On .NET 8,
SHA3_256/384/512are only supported on platforms with adequate crypto library support (e.g., Windows 11 build 26016+, OpenSSL 1.1.1+). On unsupported platforms,SHA3_xxx.Create()throwsPlatformNotSupportedException.Since
_hashAlgorithmsis a static readonly field initializer, a single failingCreate()call will throwTypeInitializationExceptionwhenSecurityis first accessedβrendering all hashing (MD5/SHA1/SHA2) and all other static members of the class unusable on those platforms. This is a runtime regression for any consumer targeting .NET 8 on a system without SHA-3 support.Gate each registration with the corresponding
IsSupportedprobe so unsupported variants are simply omitted, matching the existing behavior of throwingArgumentOutOfRangeExceptionon line 129 for unknown types:π‘οΈ Proposed fix
private static readonly Dictionary<HashAlgorithmType, HashAlgorithm> _hashAlgorithms = new() { { HashAlgorithmType.Md5, MD5.Create() }, { HashAlgorithmType.Sha1, SHA1.Create() }, { HashAlgorithmType.Sha256, SHA256.Create() }, { HashAlgorithmType.Sha384, SHA384.Create() }, { HashAlgorithmType.Sha512, SHA512.Create() }, -#if NET8_0_OR_GREATER - { HashAlgorithmType.Sha3_256, SHA3_256.Create() }, - { HashAlgorithmType.Sha3_384, SHA3_384.Create() }, - { HashAlgorithmType.Sha3_512, SHA3_512.Create() }, -#endif }; + +#if NET8_0_OR_GREATER + static Security() + { + if (SHA3_256.IsSupported) + { + _hashAlgorithms[HashAlgorithmType.Sha3_256] = SHA3_256.Create(); + } + if (SHA3_384.IsSupported) + { + _hashAlgorithms[HashAlgorithmType.Sha3_384] = SHA3_384.Create(); + } + if (SHA3_512.IsSupported) + { + _hashAlgorithms[HashAlgorithmType.Sha3_512] = SHA3_512.Create(); + } + } +#endifAlternatively, catch
PlatformNotSupportedExceptionat the call site, or document clearly that SHA-3 operations on unsupported platforms will throw.π€ Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Src/CrispyWaffle/Cryptography/Security.cs` around lines 153 - 165, The static initializer for _hashAlgorithms calls SHA3_256.Create/ SHA3_384.Create/ SHA3_512.Create unconditionally, which can throw PlatformNotSupportedException on some .NET 8 platforms and causes TypeInitializationException for the Security class; change the initializer to only add HashAlgorithmType.Sha3_256 / Sha3_384 / Sha3_512 entries when their corresponding SHA3_*.IsSupported (or wrap each Create() in a try/catch for PlatformNotSupportedException and skip registration on failure), so the _hashAlgorithms dictionary is built safely without throwing and retains MD5/SHA1/SHA2 functionality on platforms that don't support SHA-3.
π€ Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@Src/CrispyWaffle/Cryptography/Security.cs`:
- Around line 153-165: The static initializer for _hashAlgorithms calls
SHA3_256.Create/ SHA3_384.Create/ SHA3_512.Create unconditionally, which can
throw PlatformNotSupportedException on some .NET 8 platforms and causes
TypeInitializationException for the Security class; change the initializer to
only add HashAlgorithmType.Sha3_256 / Sha3_384 / Sha3_512 entries when their
corresponding SHA3_*.IsSupported (or wrap each Create() in a try/catch for
PlatformNotSupportedException and skip registration on failure), so the
_hashAlgorithms dictionary is built safely without throwing and retains
MD5/SHA1/SHA2 functionality on platforms that don't support SHA-3.
βΉοΈ Review info
βοΈ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 33aa5677-00c7-4ec2-9598-3999319a6f3f
π Files selected for processing (2)
Src/CrispyWaffle/Cryptography/HashAlgorithmType.csSrc/CrispyWaffle/Cryptography/Security.cs
|
β Build CrispyWaffle 10.0.1528 completed (commit 1a84a66665 by @christopher-conley) |
|
Failed to generate code suggestions for PR |
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
|
Hi @christopher-conley π, Thank you so much for your pull request! π I appreciate the time and effort you put into this contribution. Thanks again for your valuable contribution! π |
π Description
Introduce
SHA3_256,SHA3_384andSHA3_512enum members toHashAlgorithmType, guarded by theNET8_0_OR_GREATERpreprocessor symbol, and register their corresponding SHA3 implementations inCrispyWaffle.Cryptography.Security's algorithm map, also guarded by theNET8_0_OR_GREATERpreprocessor symbol.This PR selectively enables the use of .NET 8+ built-in SHA3 hash algorithms when targeting .NET 8 or later, while still compiling correctly for older supported frameworks.
β Checks
β’οΈ Does this introduce a breaking change?
βΉ Additional Information
All added code is wrapped within a C# preprocessor
#if NET8_0_OR_GREATERblock, so this PR is backward-compatible with existing code, regardless of the target framework. Existing code targeting .NET 7 or below requires no changes; the new hash algorithms simply won't be available on those frameworks, nor appear in IntelliSense.Summary by Sourcery
Add support for SHA3 hash algorithms when targeting .NET 8 or later.
New Features:
Summary by CodeRabbit