7 ms·
I’ve led multiple incident responses at a FAANG, here’s my take. The fundamental problem here is not Rust or the coding error. The problem is: 1. Their bot man
by abalone 10mo ago
I’ve led multiple incident responses at a FAANG, here’s my take. The fundamental problem here is not Rust or the coding error. The problem is:
1. Their bot management system is designed to push a configuration out to their entire network rapidly. This is necessary so they can rapidly respond to attacks, but it creates risk as compared to systems that roll out changes gradually.
2. Despite the elevated risk of system wide rapid config propagation, it took them 2 hours to identify the config as the proximate cause, and another hour to roll it back.
SOP for stuff breaking is you roll back to a known good state. If you roll out gradually and your canaries break, you have a clear signal to roll back. Here was a special case where they needed their system to rapidly propagate changes everywhere, which is a huge risk, but didn’t quite have the visibility and rapid rollback capability in place to match that risk.
While it’s certainly useful to examine the root cause in the code, you’re never going to have defect free code. Reliability isn’t just about avoiding bugs. It’s about understanding how to give yourself clear visibility into the relationship between changes and behavior and the rollback capability to quickly revert to a known good state.
Cloudflare has done an amazing job with availability for many years and their Rust code now powers 20% of internet traffic. Truly a great team.
- ignoramous 10mo ago> Their bot management system is designed to push a configuration out to their entire network rapidly. Once every 5m is not "rapidly". It isn't uncommon for configuration systems to do it every few seconds [0]. > While it’s certainly useful to examine the root cause in the code. Believe the issue is as much an output from a periodic run (clickhouse query) caused by (on the surface, an unrelated change) causing this failure. That is, the system that validated the configuration (FL2) was different to the one that generated it (ML Bot Management DB). Ideally, it is the system that vends a complex configuration that also vends & tests the library to consume it, or the system that consumes it, does so as if it was "tasting" the configuration first before devouring it unconditionally [1]. Of course, as with all distributed system failures, this is all easier said and done in hindsight. [0] Avoiding overload in distributed systems by putting the smaller service in control (pg 4), https://d1.awsstatic.com/builderslibrary/pdfs/Avoiding%20overload%20in%20distributed%20systems%20by%20putting%20the%20smaller%20service%20in%20control-Joe%20Magerramov.pdf https://d1.awsstatic.com/builderslibrary/pdfs/Avoiding%20ove... [1] Lessons from CloudFront (2016), https://youtube.com/watch?v=n8qQGLJeUYA&t=1050 https://youtube.com/watch?v=n8qQGLJeUYA&t=1050
- Hamuko 10mo ago>Once every 5m is not "rapidly". Isn't rapidly more of how long it takes to get from A to Z rather than how often it is performed? You can push out a configuration update every fortnight but if it goes through all of your global servers in three seconds, I'd call it quite rapid.
- abalone 10mo agoBy rapid I mean a rapid rollout of changes to 100% of the fleet, not how often changes are made.
- evntdrvn 10mo agoThanks for sharing that AWS doc
- jbs789 10mo agoThanks for this assessment. In a productive way, this view also shifts the focus to improving the system (visibility etc), empowering the team, rather than focusing on the code which broke (probably strikes fear in the individuals, to do anything!)
- JB_Dev 10mo agoDoes their ring based rollout really truly have to be 0->100% in a few seconds? I don’t really buy this requirement. At least make it configurable with a more reasonable default for “routine” changes. E.g. ramping to 100% over 1 hour. As long as that ramp rate is configurable, you can retain the ability to respond fast to attacks by setting the ramp time to a few seconds if you truly think it’s needed in that moment.
- NicoJuicy 10mo agoI think defence against a DDOS against your network is the best reason for a quick rollout
- jabl 10mo agoMaybe, but in that case maybe have some special casing logic to detect that yes indeed we're under a massive DDOS at this very moment, do a rapid rollout of this thing that will mitigate said DDOS. Otherwise use the default slower one? Of course, this is all so easy to say after the fact..
- xp84 10mo agoIsn’t CF under a ‘massive DDOS’ 24/7 pretty much by definition? When does malicious traffic rest, and how many targets of same aren’t using CF?
- NicoJuicy 10mo agoIt's literally in the blog post as well > In the internal incident chat room, we were concerned that this might be the continuation of the recent spate of high volume Aisuru DDoS attacks:
- matteocontrini 10mo agoThis was not about DDoS defense but the Bot Management feature, which is a paid Enterprise-only feature not enabled by default to block automated requests regardless of whether an attack is going on. https://developers.cloudflare.com/bots/get-started/bot-management/ https://developers.cloudflare.com/bots/get-started/bot-manag...
- pas 10mo agoRolling out new code should be done differently than rolling out new data to fight bots. If every time there's a new bot someone needs to write code that can blow up their whole service, maybe they need to iterate a bit on this design?
- watchful_moose 10mo agoThis isn't what they do, though. This is a data/config push - original article says _a “feature file” used by our Bot Management system_
- dev_l1x_be 10mo agoExactly the right take. Even when you want to have rapid changes on your infra, do it at least by region. You can start with the region where the least amount of users are impacted and if everything is fine, there is no elevated number of crashes for example, you can move forward. It was a standard practice at $RANDOM_FAANG when we had such deployments.
- abalone 10mo agoThank you. I am sympathetic to CF’s need to deploy these configs globally fast and don’t think slowing down their DDoS mitigation is necessarily a good trade off. What I am saying is this presents a bigger reliability risk and needs correspondingly fine crafted observability around such config changes and a rollback runbook. Greater risk -> greater attention.
- twoodfin 10mo agoBut the rapid deployment mechanism for bot features wasn’t where the bug was introduced. In fact, the root bug (faulty assumption?) was in one or more SQL catalog queries that were presumably written some time ago. (Interestingly the analysis doesn’t go into how these erroneous queries made it into production OR whether the assumption was “to spec” and it’s the security principal change work that was faulty. Seems more likely to be the former.)
- abalone 10mo agoIt was a change to the database that is used to generate a bot management config file. That file was the proximate cause for the panics. The kind of observability that would have helped here is “panics are elevated and here are the binary and config changes that preceded it,” along with a rollback runbook for it all. Generally I would say we as an industry are more nonchalant about config changes vs binary changes. Where an org might have great processes and systems in place for binary rollouts, the whole fleet could be reading config from a database in a much more lax fashion. Those systems are quite risky actually.
- nrhrjrjrjtntbt 10mo agoThis guy SREs
- ithkuil 10mo agoBack when it meant Site Reliability Engineer and not Sysadmin Really Expensive
- z0ltan 10mo ago[dead]
- tormeh 10mo agoPartial disagree. There should be lints against 'unwrap's. An 'expect' at least forces you to write down why you are so certain it can't fail. An unwrap is not just hubris, it's also laziness, and has no place in sensitive code. And yes, there is a lint you can use against slicing ('indexing_slicing') and it's absolutely wild that it's not on by default in clippy.
- selfmodruntime 10mo agoI would love to go further and explicitely forbid unwrap and similar calls using a `no_panic` attribute.
- stevefan1999 10mo agoI actually have to do this for programs that runs in bare metal. You can't afford to have nondeterministic panic like this. If things really gone wrong you'd have a watchdog and health checker to verify the state of program.
- selfmodruntime 10mo agoHow do you manage to do this?
- stevefan1999 10mo agoThere's a crate that prevents linking panic symbol in the final stage of the executable generation, forcing it to be undefined symbol, so while it is hard to find out where the panic is, it effectively requires me to inspect throughout the code to find out. Sometimes I have to disassemble the object file to see this
- selfmodruntime 10mo agoit's not the `no_panic` crate by david tolnay, is it?
- 10mo ago
- HelloNurse 10mo agoThe "coding error" is a somewhat deliberate choice to fail eagerly that is usually safe but doesn't align with the need to do something (propagation of the configuration file) without failing. I'm sure that there are misapplied guidelines to do that instead of being nice to incoming bot management configuration files, and someone might have been scolded (or worse) for proposing or attempting to handle them more safely.
- zelphirkalt 10mo agoIt is just 2 different layers. Of course the code is also a problem, if it is in fact as the GP describes it. You are taking the higher level view, which is the second layer of dealing with not only this specific mistake, but also other mistakes, that can be related to arbitrary code paths. Both are important, and I am pretty sure, that someone is gonna fix that line of code pretty soon.
- polack 10mo agoThey failed on so many levels here. How can you write the proxy without handling the config containing more than the maximum features limit you set yourself? How can the database export query not have a limit set if there is a hard limit on number of features? Why do they do non-critical changes in production before testing in a stage environment? Why did they think this was a cyberattack and only after two hours realize it was the config file? Why are they that afraid of a botnet? Does not leave me confident that they will handle the next Aisuru attack. I'm migrating my customers off Cloudflare. I don't think they can swallow the next botnet attacks and everyone on Cloudflare go down with the ship, so it will be safer to not be behind Cloudflare when it hits.
- raxxorraxor 10mo agoI don't think these are realistic requirements for any engineered system to be honest. Realistic is to have contingencies for such cases, which are simply errors. But the case for Cloudflare here is complicated. Every engineer is very free to make a better system though.
- polack 10mo agoWhat is not realistic? To do simple input validation on data that has the potential to break 20% of the internet? To not have a system in place to rollback to the latest known state when things crash? Cloudflare builds a global scale system, not an iphone app. Please act like it.
- dspillett 10mo ago> To do simple input validation on data that has the potential to break 20% of the internet? There will always be bugs in code, even simple code, and sometimes those things don't get caught before they cause significant trouble. The failing here was not having a quick rollback option, or having it and not hitting the button soon enough (even if they thought the problem was probably something else, I think my paranoia about my own code quality is such that I would have been rolling back much sooner just in case I was wrong about the “something else”).
- matternot 10mo agoI don't understand why they didn't validate and sanitize the new config file revision. If bad(whatever that reason is) throw an error and revert back to previous version. You don't need to take down the whole internet for that.
- WJW 10mo agoSame as for almost every bug I think: the dev in question hadn't considered that the input could be bad in the way that it turned out to be. Maybe they were new, or maybe they hadn't slept much because of a newborn baby, or maybe they thought it was a reasonable assumption that there would never be more than 200 ML features in the array in question. I don't think this developer will ever make the same mistake again at least. Let those who have never written a bug before cast the first stone.
- adriand 10mo ago> Maybe they were new, or maybe they hadn't slept much because of a newborn baby Reminds me of House of Dynamite, the movie about nuclear apocalypse that really revolves around these very human factors. This outage is a perfect example of why relying on anything humans have built is risky, which includes the entire nuclear apparatus. “I don’t understand why X wasn’t built in such a way that wouldn’t mean we live in an underground bunker now” is the sentence that comes to mind.
- Yokohiii 10mo agoI don't think this is an error originating from a single human. At CF scale I'd expect that multiple humans saw that code and gave it a pass. Rust or not, but an experienced dev could have seen this can lead to issues. Panicking without restoring a healthy state is just not an option in this case. They *know* that. I guess you are right, likely a social issue, but certainly not a single exhausted parent.
- throw0101c 10mo ago> I don't understand why they didn't validate and sanitize the new config file revision. The new config file was not (AIUI) invalid (syntax-wise) but rather too big: > […] That feature file, in turn, doubled in size. The larger-than-expected feature file was then propagated to all the machines that make up our network. > The software running on these machines to route traffic across our network reads this feature file to keep our Bot Management system up to date with ever changing threats. The software had a limit on the size of the feature file that was below its doubled size. That caused the software to fail.
- rkachowski 10mo agoClassic @devops_borat "To make error is human. To propagate error to all server in automatic way is #devops"
- mongol 10mo agoI miss him. It must be more than 10 years now
- throw0101c 10mo ago> "To make error is human. To propagate error to all server in automatic way is #devops" This saying dates back to 1969: To err is human but to really foul things up requires a computer. * https://quoteinvestigator.com/2010/12/07/foul-computer/ https://quoteinvestigator.com/2010/12/07/foul-computer/ Also: I know there’s a proverb which says ‘To err is human,’ but a human error is nothing to what a computer can do if it tries. * https://quoteinvestigator.com/2017/05/26/computer-error/ https://quoteinvestigator.com/2017/05/26/computer-error/
- BrtByte 10mo agoYou can write the safest code in the world, but if you're shipping config changes globally every few minutes without a robust rollback plan or telemetry that pinpoints when things go sideways, you're flying blind
- mlrtime 10mo agoI've also led a team of Incident Commanders at a FAANG. If this was a routine config change, I could see how it could take 2 hours to start the mediation plan. However they should have dashboards that correlate config setting changes with 500 errors (or equivalent). It gets difficult when you have many of of these going out at the same time and they are slowly rolled out. The root cause document is mostly for high level and the public. The details on this specific outage will be in a internal document with many action items, some of them maybe quarter long projects including fixing this specific bug and maybe some linter/monitor to prevent it from happening again.
- antihero 10mo agoI would say that whilst this is a good top down view, that `.unwrap()` should have been caught at code-review and not allowed. Clippy rule could have saved a lot of money. That and why the hell wasn't their alerting showing up colossal amount of panics in their bot manager thing?
- xmcqdpt2 10mo agoYes the lack of observability is really the disturbing bit here. You have panics in a bunch of your core infrastructure, you would expect there to be a big red banner on the dashboard that people look at when they first start troubleshooting an incident. This is also a pretty good example why having stack traces by default is great. That error could have been immediately understood just from a stack trace and a basic exception message.
- seethishat 10mo agoThe bot is efficient. This is by design. It will push out mistakes just as efficiently as it pushes out good changes. Good or bad... the plane of control is unchanged. This is the danger of automated control systems. If they get hacked or somehow push out bad things (CloudStrike), they will have complete control and be very efficient.