9 ms·
No, complexity strikes again. I haven't used sudo in years, preferring to use doas now. Its essential code is less than 500 lines and it does everything I've ev
by ddevault 6y ago
No, complexity strikes again. I haven't used sudo in years, preferring to use doas now. Its essential code is less than 500 lines and it does everything I've ever used sudo for, and that includes much more than `sudo <runupdates>`.
$ man doas | wc -l
58
$ man doas.conf | wc -l
101
$ man sudo | wc -l
741
$ man sudoers | wc -l
3254
And a bonus:
$ man sudoers | grep -C1 despair
The sudoers file grammar will be described below in Extended Backus-Naur
Form (EBNF). Don't despair if you are unfamiliar with EBNF; it is fairly
simple, and the definitions below are annotated.
That only accounts for a small subset of sudo's complexity. It's easily 100x more complex than it needs to be to solve this problem. Now compare the two CVE lists:
http://ftp.netbsd.org/pub/pkgsrc/current/pkgsrc/security/doas/README.html http://ftp.netbsd.org/pub/pkgsrc/current/pkgsrc/security/doa...
http://ftp.netbsd.org/pub/pkgsrc/current/pkgsrc/security/sudo/README.html http://ftp.netbsd.org/pub/pkgsrc/current/pkgsrc/security/sud...
My reaction to this vulnerability was mild amusement, then later wondering if I should go discredit the inevitable Rust brigade. We don't have to rewrite everything in Rust to get better security. We just have to use simpler tools.
- comex 6y agoBut if sudo were written in Rust, it could have the same level of complexity and not be vulnerable. Yes, it would still be vulnerable to logic errors, like the last famous sudo bug where you pass -1 as the UID. But it wouldn't be vulnerable to this. (And this isn't the first memory safety bug to be found in sudo.) Yes, sudo's complexity is useless for 99.99% of its users. But wouldn't it be nice if the result were merely a gross feeling rather than a security hole?
- ddevault 6y ago>But if sudo were written in Rust, it could have the same level of complexity and not be vulnerable. This is not true. Complexity breeds bugs, including security bugs, and memory safety doesn't change that. Your example is a good one - here's another: doas once failed to limit the environment variables which are passed to the child process, which could be used to nefariously influence the program running (e.g. with LD_PRELOAD). How would Rust prevent that oversight? It wouldn't. A simpler program will generally be more secure than a complicated one, no matter what language either is written in. Furthermore, rewriting an established program from one language to another will always introduce more bugs than it fixes, and more severely the more complex the program is. The single best way to improve security is to reduce the attack surface, and the single best way to do that is to reduce the complexity of your system.
- zamadatix 6y agoIf you go a little further with the quote: "Yes, it would still be vulnerable to logic errors... But it wouldn't be vulnerable to this. " I think you'll find in disagreeing with the comment on logic errors you just said the same thing the comment did about logic errors. Also I think the generalization that rewriting an established bit of code in a new language in a secure language is a bit too general. clearly Firefox not only set out to make Rust for this purpose but it's not had an explosion in vulnerabilities with the modules it has replaced. Quite the opposite actually. Nor has every tool or app rewritten become a security failure compared to the original. I do think it's something that can easily be screwed up though, especially if someone rushes through by focusing on functionality duplication instead of building a more secure version of something. Regardless, both "using a memory safe language results in a more safe program" and "having a minimum attack sufrace results in a more safe program" can be true. There is no need to make it a choice of A or B.
- ddevault 6y ago>I think you'll find in disagreeing with the comment on logic errors you just said the same thing the comment did about logic errors. I think you'll find that my comment explicitly acknowledges this and expands on it with another example. Are we done telling each other to read the things we're writing? >Firefox not only set out to make Rust for this purpose but it's not had an explosion in vulnerabilities with the modules it has replaced. You're setting the bar pretty high with an "explosion" of vulnerabilities here. Rust programs have vulnerabilities, including rewrites. They also have other kinds of bugs, often ones which were not present in the code that they're replacing. You need only browse your nearest convenient RiiR bug tracker to find evidence of this. Let me restate my thesis in mathematical terms. If we presume that 1 in 100 lines of production code has a bug in it, regardless of language (generous, I know), and that 1 in 10 bugs in C programs are memory corruption related, then saving 10% of those bugs by rewriting it in Rust would take a 10,000 line codebase from 100 bugs to 90 bugs. A 1,000 line codebase, still written in C and without the advantage of memory safety, would have only 10. In today's example, sudo is a caricature of runaway complexity. Rust is often touted as a panacea, but C has very little to do with why sudo is insecure. Sudo is comically overengineered and that level of overengineering has no place in a security context. This is the larger issue that needs to be addressed, not Rust.
- musicale 6y ago> But if sudo were written in Rust, it could have the same level of complexity and not be vulnerable I'm puzzled that we don't have a memory-safe ABI (e.g. amd64-safe) and runtime for C so we could just compile things with clang -safe sudo.c to avoid memory errors. I'm fine with sudo (or whatever) taking a 60% performance hit to be more reliable - processors are thousands of times faster now than they were in 1980 when sudo was written. If we had a memory-safe ABI for C/C++ in common use its performance overhead could probably be reduced significantly over time due to implementation improvements, and we might see hardware acceleration for it as well. There are a number of proof-of-concept memory-safe compilers for C using fat pointers, etc., but memory safety hasn't made it into gcc or clang. 64-bit CPUs can help because you can repurpose address bits. Even Pascal (which is largely isometric to C) supported a degree of memory safety via array bounds checking. I believe Ada compilers also support memory safety. PL/I was actually memory safe and is why Multics never had any buffer overflows. Obviously Rust is memory safe, but for a lot of legacy C code it is impractical to rewrite everything in Rust but eminently practical to recompile it with memory safety turned on.
- scatters 6y agoIt's called AddressSanitizer. You enable it with the compiler flag -fsanitize=address. It's supported by clang, gcc and lately MSVC.
- rkeene2 6y agoYou shouldn't use ASan in release builds since it may have exploitable vulnerabilities.
- saagarjha 6y agoAddress Sanitizer is not perfect, nor is it suitable to ship in production.
- kevingadd 6y agoAsan binaries should never be shipped in security sensitive environments. It's not designed for that. It's unsafe.
- gameswithgo 6y agoRust would have prevented the -1 as a UUID too, because you would have used a sum type (Rust enums) instead of a sigil there. Its easier, its idiomatic, its more clear, and the compiler knows how to optimize the overhead away a lot of the time.
- comex 6y agoWell, in this particular case, the special behavior of -1 is baked into the setresuid system call, while sudo thought it was just an ordinary UID. So if you look at one of the Rust operating system projects designed from scratch from-scratch OS designs, it might not have this kind of pitfall. But if you literally just reimplement sudo for existing OSes in Rust – which I think would be a neat project for someone to take on – you’d be at risk of running into it.
- j16sdiz 6y agoIt is a uid (as in user id), not uuid. Don’t think you can use sum type for that
- steveklabnik 6y agoIt is a user id, but that bug happened because a -1 was being returned as an error code in one place, and then being accidentally passed in another place. The sum type would be used as the “this possibly errors” return type in the first function, making the bug effectively impossible to happen by accident.
- 0xbadcafebee 6y agoIf sudo were written in Perl, it would also not be vulnerable.
- MayeulC 6y agoI thought about that. But when using a higher-level language (that comes with a runtime), you need to give privileges to the whole runtime, which arguably has a much bigger attack surface, unless I am mistaken? The kernel won't allow you to setuid scripts, there is a reason for this: it's very easy to leave glaring security holes while doing so.
- 0xbadcafebee 6y agoPerl had a fix for this called suidperl, which was a wrapper which enabled Taint mode and other strict checks (https://perldoc.perl.org/perlsec https://perldoc.perl.org/perlsec). I don't know of any other language or interpreter that go this far for security by default, so Perl might be the most secure language in this regard. However, suidperl was dropped in 5.12. My main point was that you could rewrite sudo in all sorts of languages, but saying "just rewrite it in Perl" (assuming it worked) isn't a enough justification to make it happen. Nobody is going to re-create their own project in Perl, Rust, etc just to eliminate buffer overflows. If somebody wants sudo in Rust, they'll have to do it themselves, and it still might never replace the original.
- MayeulC 6y agoThank you for pointing that out :) If someone would get serious about security, auditing every setuid binary would certainly be something on their list (if they use any). If they really want the functionality, rewriting it to cover just enough of the required functionality wouldn't be unheard of.
- 0xbadcafebee 6y agoI agree. But really the security best practice is to remove setuid from all binaries, and rely on RBAC rules (such as with SELinux). That would solve more security issues than anything else, but this is way more effort than most people are willing to invest in security.
- mulmen 6y agoFair point re: complexity but how are CVEs in one codebase evidence of absence of bugs in another? Or said another way, is the lack of CVEs for doas an indication it is more secure or just less (ab?)used?
- ddevault 6y agoMore CVEs can also sometimes be a function of exposure; Joe Random's program probably has no CVEs but that doesn't mean it's more secure than Jane Popular's tool. In this case, however, both sudo and doas have sufficient exposure to estimate their relative security using CVEs. We can also use the CVEs to characterize the kinds of vulnerabilities each has internally, without comparing them to each other. In general, the CVEs that are discovered for sudo are more severe and damning than those discovered for doas, without respect to their relative occurance.
- arp242 6y ago> We don't have to rewrite everything in Rust to get better security. We just have to use simpler tools. People don't add these features for the fun of it; they're present because they solve a specific use case. I use doas as well; it's a neat little program that covers many of the common use cases, but it doesn't cover all of them. Usually you should use the simplest tool that solves your problem, but sometimes your problem is complex and thus your tool will be complex. That is not to say that sudo can't perhaps be simpler; the first version was released in 1985 and there are probably things that can be improved, but sudo really isn't written by idiots who just add features because they have nothing better to do.
- ddevault 6y agoAdding features because they solve a specific use-case is grossly irresponsible. Solving a specific use-case is only one of many criteria that needs to be met for a feature to be justified. Others include "is it in scope?", "is it a maintenance burden?", "does it make existing features more unreliable?", "will its bugs affect people who don't need it?", "can it be done in a separate tool?" Anyone can come up with a use-case. It's often the maintainer's job to say "no". If doas doesn't cover your edge case, then the responsible thing is to make a new tool which covers just that, and not to shove your complexity into a critical security component that the other 99% of the userbase doesn't need. Remember Heartbleed? The entire internet shat its pants because of a vulnerability in a feature that no one uses. Failing to uphold that principle over and over again leads to broken, unreliable, insecure programs. This is why everything is on fire. Not C.
- arp242 6y agosudoedit is used by many people, and setting a different shell with -s seems like something that would cover a number of edge cases, yes, but writing a new tool just to add "-s" is obviously silly. Nothing in this particular CVE touches on anything that seems particularly obscure to me. The last major sudo bug was in the PAM code (which lead to the creation of doas), which is something many people don't need, but also something that many others do need. And writing separate tools would be the equal (or more!) lines of code and an equal amount of bugs in total (or probably more, since people will be reinventing stuff and there will be fewer reviewers per line of code). This isn't reducing complexity, it's just spreading it out.
- hsaliak 6y agoDon’t know why this comment is getting downvoted. The code in question is complex and outdated. Improving the code and testing or switching to simpler tools can lead to a pragmatic solution without the political baggage.
- gautamcgoel 6y agoHi Drew, I agree with everything you said re complexity and rust. What we really need is a modernized C language, tools that help us catch bugs like this, and a better culture of testing and accountability. I'm curious whether you run OpenBSD, since you mentioned you use doas. Do you have any thoughts on OpenBSD?
- ddevault 6y agoI don't run OpenBSD, for reasons that have little to do with security. In my opinon, OpenBSD is not agressive enough at complexity reduction (doas being an outlier, I think doas is quite a good size).
- gautamcgoel 6y agoI see. What do you run, if you don't mind me asking?
- ddevault 6y agoAlpine Linux.
- kaba0 6y agoModernizes C language = Zig. But I don’t see a point in using systems languages for the usual UNIX tools, they could be rewritten in a more secure language. In the rare case performance is important, there is FFI, but they are usually IO-bound so there is not much point.
- SSLy 6y agoI agree about secure languages. Last week I was daydreaming about reimplementing all non-RT parts in Lua.
- tinco 6y agoRust is modernized C. You are looking for something that already exists. If C programmers would be looking for tools to help catch bugs like this and a better culture of testing and accountability they would be using Rust.
- MayeulC 6y agoI agree about your point on complexity, that was my second thought. Especially for such critical pieces of infrastructure. Thank you for mentioning doa, I will have a look. But the two are pieces of the same puzzle (defence in depth). Ideally, SUID binaries should be formally specified/verified (ada+spark?), though you could still have bugs in the specification. And I'd argue that if you need more specific features than just `sudo`'s core functionality, you should probably just make your own setuid binary. That still exposes you to making the same mistakes, so better keep the complexity low, and still rely on a memory-safe language. Using proved, lightweight libraries helps getting an implementation correct. As much as I like C (which is the language I'm most proficient with), it just gives you too many ways to shoot yourself in the foot, and IMO isn't really the best suited for something where: - performance doesn't really matter - memory safety, typechecking are critical. You could always get away with a transpiler for a DSL, or a compiler that injects more checks, but better suited tools are available anyway.