Comments (3)
Hi @Bobsson,
First of all, thanks for reaching out. Receiving feedback from our users on our rules, and especially security rules, is always something we look forward to.
I'll try to be as precise and objective as possible here but let me know if there is anything you think I am wrong with or you would like to challenge.
TL;DR
SHA1 is still practically safe to use and will remain so for a long time. However, NIST announced transitioning away from it, and this is probably something you want to work on now. If you want to keep using HMAC with a hash function output size of 160 bits, you can consider using a truncated version of a SHA2 function following NIST recommendations.
I think the rule description is OK when it comes to SHA1 but maybe I missed your point.
Why raise on HMAC-SHA1?
To begin with, the source you link, and especially the paper about collision requirements for HMAC, is perfectly true and valid. The fact is, we don't know of any practical attack on HMAC-SHA1 that would require immediate action on the users' side, and no one expects this will happen anytime soon. So, if you rely on HMAC-SHA1 and have no way to switch away from it, be reassured, you are not under any immediate threat.
However, the fact that attacks exist on SHA1, even if only on the collision side, is already proof that the algorithm design is faulty to some extent. Discovering collision attacks on a hash algorithm usually is a good reason to kickstart a deprecation process.
Actually, NIST published a document in December 2022, a few months after the StackOverflow answer you linked was posted, that posed the milestones for SHA1 deprecation. There is also this news article if you prefer a higher-level view of this topic.
While the final deadline for SHA1 removal is only December 2023, NIST SP.800-131Ar2, published in March 2019 was already conservative about SHA1.
and
HMAC Generation:
Any approved hash function may be used
One could argue that this leaves a grey zone for HMAC with SHA1. I would agree with that. However, overall, it seems clear that moving away from SHA1 is generally a sane decision. It is at least what drove the decision to raise on HMAC-SHA1 in S4790.
S4790 description content
When it comes to the rule description content, it seems to me that it correctly reflects the rule implementation, at least when it comes to SHA1.
Cryptographic hash algorithms such as MD2, MD4, MD5, MD6, HAVAL-128, HMAC-MD5, DSA (which uses SHA-1), RIPEMD, RIPEMD-128, RIPEMD-160, HMACRIPEMD160 and SHA-1 are no longer considered secure
It is possible that I am missing your point here. Let me know if this is the case.
I would agree that, overall, the rule description is not very good and could use some improvement both on the side of provided details and general text quality. We are currently undergoing a rework of our crypto-related rules, of which you will soon be able to see the results. Unfortunately, we did not prioritize hotspots and C# language has not been covered yet. We still have plenty to do there so be sure to stay tuned.
Any replacement?
Generally speaking, using a SHA2 family function to replace SHA1 in the HMAC construction is what should be done. It should not involve a lot of performance overhead or any other big issue apart from protocol compatibility matters.
The only reason I can think of for which you might want to keep SHA1 is the output size. SHA1 has an output size of 160 bits which is much shorter than what SHA224 or SHA256 (let alone SHA512) can provide. If this is an issue for you, you could consider using a truncated version of SHA256. This is perfectly safe if done following NIST recommendations that you can find in NIST SP.800-107r1 chapter 5.1.
This would probably be named something like HMAC-SHA256-160 which, if nothing else, at least has a nice name.
I hope this answers your question and helps.
Let me know if there is anything else we can do.
Cheers!
from sonar-dotnet.
Hi @gaetan-ferry-sonarsource. Thanks for the thorough reply on this. I'll take it back and consider it for a bit.
As far as the documentation issue, this is what I'm seeing:
Algorithm | Code? | Documented? |
---|---|---|
DSA | Y | Y |
HMACMD5 | Y | Y |
HMACRIPEMD160 | Y | Y |
HMACSHA1 | Y | N |
MD2 | N | Y |
MD4 | N | Y |
MD5 | Y | Y |
MD6 | N | Y |
RIPEMD | N | Y |
RIPEMD-128 | N | Y |
RIPEMD160 | Y | Y |
SHA1 | Y | Y |
HAVAL-128 | N | Y |
As you can see, HMACSHA1 is the only one that exists in the code and not in the documentation. Everything else is in the documentation list, whether or not it's being scanned for.
from sonar-dotnet.
@Bobsson OK, I get it now. There is indeed an inconsistency in how algorithms are documented there. I would not personally expect that HMAC-* be mentioned specifically. HMAC is not a hashing algorithm but a message authentication algorithm. It is a construction upon an underlying, actual hash algorithm and, as such, should not be mentioned here, or at least not in that way.
I just checked our backlog, and it happens we already have a ticket tracking the poor description quality. Considering we are already working on our crypto rule, I think it will be tackled when we work on crypto-related hotspots later this year. Do not expect an update in the upcoming weeks, but be sure we are tracking this in this year's objectives.
Thanks again for your feedback.
Cheers!
from sonar-dotnet.
Related Issues (20)
- Fix AD0001: Named Attribute Arguments in S6930 HOT 1
- New Rule T0021: Use extension methods for Linq
- Fix S3604 FP: Remove the member initializer, all constructors set an initial value for the member. HOT 1
- Add [CodeCopiedFromAttribute] support
- Fix S2589 FN: Support recursive pattern redundant null check HOT 2
- Fix FP S2589: Assignment for captures are ignored
- Styling rules: Do not show RSPEC URL
- Load Razor source generators from SDK
- Fix S1144 FP: Getters/Setters of property with attribute are being flagged HOT 1
- Fix S1066 FP: Combination of `dynamic` and `out` should not raise HOT 2
- New rule T0036: Move extension method to dedicated class
- Improve T0007: Raise on nondeclaring is { } check
- Change `.Verify` to fail when no issues are reported
- Fix S2094 FP: Allow empty queries HOT 3
- Improve S6964: Report on Controller Action parameters when Model type is in a different project HOT 1
- Fix S2629 FP: Constant fields in interpolated string
- Improve AspNetMvcHelper.ReferencesControllers method: add parameter for .NET Core HOT 1
- Fix S6934 FP: Abstract Controller base class
- Rule S2365: Fix code examples in RSPEC HOT 1
- Update RSPEC before 9.26 release
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from sonar-dotnet.