Things like this everywhere. Just stupid programmers or method?
You have two kind of elite at the top: a bunch of very smart people, completely disconnected from daily work, setting up automated code checks and style rules. Let's call those architects/Engineering VPs. Then you also have a bunch of short term revenue people in a completely different side of the company who owns all the money and set the targets. Let's call those Product VPs
The daily work will be full of agile-cargo-cult (that is, all the meetings agile impose, without any the actual communication), with people in the teams reporting to different chains. Engineers having to abide by the crazy ineffective coding style (usually because nobody cared to provide the tools to help achieve the code quality required), and product/managers abiding by the short term revenue churn.
And since the Product org is the one who decides if target goals were met for bonuses and promotions you see why the actual coders might do that bullshit (e.g. which one ever got you a promotion: "I contributed to hugely announced feature X" vs "I followed all the best practices set up by the architects i have never met and prevented a security report two years in the future from dragging the company in the mud"?)
Most companies denounce the problems of having silos, but silos actually make the smaller org/teams responsible for the entire stack and tools. It is hated by investors because it is more expensive, but in the end, it is the real cost. A very vertical big corp will be cheaper but the reason it is cheaper is because less work is being done and always end up like this for ignoring the human nature of the people that it is made of.
This is usually a sign of "metrics-driven management". They are then surprised why all their measures of "code quality" and "compliance" on their awesome dashboards and analytics are showing 100% while the actual product continues to spew forth bugs and disappoint their customers.
Any rules perceived as stupid will naturally be worked around. "You get what you measure."
but isnt it sufficiently common public language to focus on large /corporate/capitalism etc? i only hoped to offer a note to support thia this conversation from falling into that.
thanks for following up on that, good catching!
Take the "VOS_memcpy_s" function they condemn for not checking one of the arguments on page 35.
If the source contains a check that the destination length is no less than the source length, for instance in an assert(), and the compiler is able to conclude that will always be the case, it is perfectly justified in not generating code for the check.
The trouble is that the check code would also be missing if the "production code" was compiled with asserts disabled.
To make the conclusion they do, they have to have checked that:
A) All implementations lack the check and all are in not-link-time-optimized library locations. (in which case asserts was probably disabled)
B) All distributed implementations (because VOS_memcpy_s was either static inline or #defined) all lack the check and at least one of them is called with arguments where the check does not hold or cannot be determined at compile time. (in which case asserts was probably also disabled.)
If they cannot prove either of these two points, it may just be that the code is in fact correct, and that the compiler was able to determine this and did not need to emit the checks.
For all their bragging about how state of the art their tools are, I see no indication that they have analyzed that deep and comprehensive.
That said, Huawei's code is still has atrocious quality.
Or as the C-team calls it: "Industry Best Practice"
Also in A and B you seem to be saying it's difficult for them to see whether all the locations lack the check. But it sounds to me like their static analysis makes that fairly easy, and that that's what they found.
You are saying that VOS_memcpy_s's source code could be safe but have its safety checks hidden by the compiler if the compiler has strong optimizations enabled while simultaneously having asserts enabled and also every single call to VOS_memcpy_s has an assert before it. That seems like a pretty rare configuration, because AFAIK, asserts are usually disabled when strong optimizations are enabled. And it seems unlikely to me that Huawei would put an assert before every single call to VOS_memcpy_s, for multiple reasons, (1) because Huawei is sloppy, (2) because there's no reason to put an assert before VOS_memcpy_s if it has safety guarantees. So I conclude there's very little likelihood there are any checks in VOS_memcpy_s's source code.
As for disabling asserts in the code you ship: That is such a quaint 1980'ies thing to do, and people should have stopped doing it long time ago.
First, asserts are usually free, in the sense that the values checked are already in registers.
Second, any assert the compiler can evaluate is free, because no code will be generated.
Third, asserts can often enable the compiler to produce better and faster code, because it knows more about the values it produces code for.
And this is not theory: Nobody has ever accused Varnish Cache of being slow, despite the fact that approx 10% of all source lines are asserts which cannot be compiled out.
My money is on Huaweis function doing the check with an assert which was compiled out for deliverable code.
Why else would the have the function in the first place ?
The fact that code is generated for the function as shown, indicates that it was not a static inline function, because the compiler would have reduced that to just the regular memcpy call, so it was probably in a library.
So yeah: Industry Best Practice Incompetence.
Sqlite makes all its asserts noop in release mode.
Chrome disables DCHECK() in release mode, and it's used many times.
RE2 has debug only checking.
In Rust debug mode, signed integer overflow panics, in release mode it wraps.
Firefox disables MOZ_ASSERT() in release mode, and it's used many times.
I doubt Huawei is more modern and less 1980s than all these other projects.
>My money is on Huaweis function doing the check with an assert which was compiled out for deliverable code.
Are you saying it was compiled out because asserts were disabled, or because the compiler could reason about the arguments at every callsite? It seems unlikely to me that the compiler could reason about the arguments at every callsite.
It is natural to have very expensive checks in development and test-environments, they help and aid development and QA to no end, and of course they should be disabled in production.
But the run-of-the-mill assert which documents integer ranges, pointer non-null-ness, data structure relationships and so on, cost nothing in performance and should be left in production code.
And as I said: We developed Varnish Cache to prove this point, and we overwhelmingly have: One bad CVE since 2006, and "only" about 20% of all web-traffic goes through Varnish at some point or other.
The report claims that there where no checks at all, and I'm saying they have not documented evidence for that claim, because the lack of checking code could be either disabled asserts in production code or the compiler reducing it as dead code.
Counting references to libc functions then suggesting these are "unsafe" because they are not calls to the exactly as unsafe Microsoft _s incompatibility functions is the definition of stupidity.
The length calculation always has to be done before the copy. Introducing an extra check in the way memcpy_s does is futile if you didn't get it right in the first place, and if you did, like you should, then it's nothing but bureaucratic bloat that can even make "safety" worse (because now there are two length checks, the one that actually does the work and makes the decisions, and this implicit one that someone in the future updating the code might miss.)
Almost all of the _s shit is just pure paranoia-FUD propagated by basically clueless "security" experts, mainly at Microsoft. To see the depths of this insanity, consider that they even have a "safe coding" standard where memcpy() is "banned", but memmove() is not, and there is no memmove_s() --- despite the fact that with the exception of overlapping areas, memmove() behaves exactly the same as memcpy(). You could simply search-and-replace all occurences of the latter with the former, and suddenly code that is considered "unsafe" is not.
Kind of like melamine letting something pass protein content testing.
There could be plenty of different reason.
Why would you do that? Most attacks will probably target your production environment rather than your development environment.
The most interesting quote regarding these "safer" interfaces from the report:
"The design of the Bounds checking interfaces, though well-intentioned, suffers from far too many problems to correct. Using the APIs has been seen to lead to worse quality, less secure software than relying on established approaches or modern technologies. ... Therefore, we propose that Annex K be either removed from the next revision of the C standard, or deprecated and then removed."
On the other hand, in code that does check for and attempts to handle errors reported by the APIs, the new error handling paths tend to be poorly tested (if at all) because the runtime-constraint violation can typically be triggered only once, the first time it is found and before it's fixed. After the flaw is removed, the handling code can no longer be tested and, as the code evolves, can become a source of defects in the program.
This irked me, because along with useless junk like memcpy_s, Annex K had memset_s, which in particular (and unlike memset(3)) was guaranteed not to be optimized out by the compiler.
Meanwhile Spectre has made it more important than ever for programs to clear any sensitive data out of memory as soon as it's no longer needed, so we actually need that. And it's troublesome to either have half a dozen ifdefs for all the different platform-specific functions that do the same thing, with as many possibilities to call one of them wrong, or have to pick up a dependency on a crypto library just to get e.g. sodium_memzero().
But sodium_memzero() is not safer than memset_s. If only provides a compiler barrier, but no memory barrier. AFAIK only my safeclib memset_s is "safe", but I don't provide a clflush there. Maybe I should.
The report is right to call out Huawei engineers to bypass these checks. They probably had the same attitude to bounds checks as all open source developers and industry practioners.
Likely just stupid. I can easily see:
Manager: "Hey, review kicked your code back. You need to use the foo_s versions."
Minion: "But the foo_s versions are in libsecure and the system doesn't build and link that. I need someone with the authority to add it to the build to do that."
Manager: "Not my problem. Get your code clean by the end of the day."
Minion: <decides that renaming the function locally looks pretty good right now>
And thats their enterprise infrastructure equipment stuff, can you even imagine how their android firmware must look like, holy shit.
When looking at the boxes they compare:
Huawei: CloudEngine 12800 scales up to 2000 * 25GBit or 500 * 40GBit
Juniper: EX4650 Ethernet Switch scales up 48*25GBit
With all due respect but this is comparing apples with orange seeds. A carrier class switch is more complex than an enterprise switch and has a much larger attack surface.
Most network forwarding happens on ASICs, its the only reasonable way to get performance.
Company culture on this have everything to say. So looking at their most sold products is a good testimony for lack of security focus.
The article seems to suggest Hawaii is exceptionally bad at patching or best practices in security.
Regardless, Huawei needs to clean up their practices.
Ineptitude isn't a defense and this level of sloppiness is unacceptable; We have too much at stake with these essential networks, and I think legislation of security practices and standards in these products should be considered.
Sure. So does every crappy home router manufacturer, Android manufacturer, security cam manufacturer etc: singling out Huawei is retarded.
Personally I suspect that biggest issue is that the US doesn't wish to lose their own NSA phone/txt backdoor access (thumbs in everyone's pies). They have access to most US TCP/IP traffic, so they don't need to target router manufacturers.
I would expect a report which produces claims about such comprehensive backdooring and negligence to at least demonstrate how that behavior would play out in the real world. It seems much more like they did static analysis of the binaries, identified any strings with default passwords, and then reported on them. That's okay to do, but you can't conclude that's a problem until you confirm shoddy behavior after provisioning and deployment in practice.
So they sell enterprise infrastructure equipment with such an obvious backdoor? It seems the US boycott of Huawei was fully justified after all, I didn't really believe them.
Might just be some leftover debugging account or for service.
It is a backdoor. The definition of the term doesn't require intent.
That would have been slightly harder to detect.
Here's a dry technical report on how the new-ish _s variants are not really helpful: http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1967.htm
See eg. https://rurban.github.io/safeclib/doc/safec-3.5/index.html
In a non-critical application that kind of stuff is usually fine. But for critical infrastructure there are normally rules against using potentially unsafe methods like those entirely.
Huawei would need to be more transparent to show that their practices are secure and that these uses are wrapped in some kind of protective framework. But until that happens it's reasonable to be skeptical from a security (and stability) perspective.
They will just use memcpy_s with the dest len and the len set to the same var. Or strncpy with the limit set to strlen(src) etc. These guys will tell you it's suddenly using 'modern security practices'.
Conversely depending on the code strcpy / memcpy can be 100% safe.
I think these guys are selling static analysis, so they find themselves using these oversimplified metrics... it's a shame because it looks like there was no lack of real issues.
And if I ever saw code where the size parameters weren't legit (as in someone used the same variable for both) I would also be concerned unless proper checks where taking place elsewhere. But it is a bad smell.
That's the only point in that particular finding. They did detail other ways in which the whole development standards seem bad.
And, yes, you can always shoot your own foot, but it's still best not to aim directly at it.
We're lucky to get this sort of report!
(In case you were looking for an article about the safety of 5G -> https://medium.com/@tomsparks/is-5g-dangerous-405a19e9ea88)
I have not gone through the full report (yet), but while reading just a few sections my sales-pitch-bullshit-meter went on full tilt, at least a few times.
The timing of this report is also peculiar, to say the least. Who is Finite State? What is their track record, to date? Who owns the company? What business relationships/interests do the company and its owners have?
Yes, I realize/know these are suggestive questions. Questions that should nonetheless have satisfying/assuring answers, in order take this report seriously.
There's a lot at stake here, so at least examining the validity of this report/summary isn't just a luxury.
Are the factual findings behind this report publicly available?
Someone knowledgeable about forensics with access to the same devices probably can validate those claims.
Maybe their product is brilliant, and/or maybe they did a lot of manual verification of their automated scans, in which case their report could be accurate (though still hard to verify).
That said, my experience with both static analysis and runtime security scanner is less than stellar. They are without a doubt useful tools, if not indispensable. But they usually generate more leads for further (manual) analyses, rather than generate (accurate) final verdicts.
Granted that pen-testing is a different animal (but one I have more experience with), I often have had to explain to customers that the reports they got from security auditing companies were full of false conclusion. More often than not because those companies were relying/trusting too much on automatically generated results, in addition to them trying to sell additional services. So, forgive me if I'm skeptical.
That Finite State apparently has a stake of their own here (promotion of their product), which makes me only less inclined towards giving them the benefit of a doubt.
As I mentioned before, this is of course all speculation on my part. Still, until Finite State publishes their factual/detailed scan results, I believe I have every reason to consider that this might be either a political motivated report, or more likely be an attempt to use/abuse the current political climate to promote their own product and services.
• We identified 76 instances of firmware where the device was, by default, configured such that a root user with a hard-coded password could log in over the SSH protocol, providing for default backdoor access.
• 8 different firmware images were found to have pre-computed authorized_keys hard coded into the firmware, enabling backdoor access to the holder of the private key.
• 424 different firmware images contained hardcoded private SSH keys, which can enable a man-in-the-middle to manipulate and/or decrypt traffic going to the device.
What a witch hunt... This is state of the art in the industry. Everybody does it like that. No intelligence agency has to be involved at all, it's basic negligence. If you're behind a NAT, your device is unlikely to be attacked via these vectors.
Default passwords can be dealt with.
If they are grievously incompetent at security, or they are intentionally installing backdoors, might make a difference in how we feel about it, but in neither case should we allow their products to be used.
Sure it's a password written on the device, but it's random, you need physical access to see it, and people who are security conscious can change it.
This bad practice isn't excusable, especially not by a company as big as Huawai, not if they want to be taken seriously.
Enterprise equipment is usually not supposed to be just dropped into place, without oversight. It usually needs proper configuration/management, by qualified people.
Whether this also happens in practice can be a different story altogether. Still, the security of enterprise equipment usually involves more policy and procedure than it does with consumer equipment. With the latter, security has to come more or less by default, because the people handling the devices usually have little expert knowledge.
When you get a new device, in order to save your initial configuration on it, you have to set a password.
Cisco used to ship with zero config on their devices and part of the setup process was setting a password as well.
Later versions did not allow passwordless ssh but still allowed it via telnet. Cisco’s ACI platform enforces password on the initial account, then with some smarts you could disable it in OpenLDAP
Can you even a campaign against anything once they take over your information infrastructure.
Yes NSA may do it but cf NSA with PLA ... what is your leverage. Hear of the word totalitarian...