There is quite some irony here - I was always taught to code by using standard libraries because they've been tested to destruction and back in ways I'd never be able to work out, and that rolling my own was destined for failure. As far as good practise goes, OpenSSH seems to tick all the right boxes. This article seems to criticise this, but the deeper meaning is that the standard C library is not designed with security in mind.
There's no inherent problem with the approach taken (not like OpenSSL reinventing several wheels and causing things like Heartbleed). In fact, the article's conclusion that the OpenSSH team should have rolled their own memory management code rather than use tried, tested and approved libraries doesn't sound like a good recommendation to me.
> In fact, the article's conclusion that the OpenSSH team should have
> rolled their own memory management code rather than use tried,
> tested and approved libraries
That's not the conclusion I draw from the article.
I rather read it as warning that even innocent and quite common
operations like realloc() or using buffered I/O can have security
implications, so one need to be extra extra extra careful when
writing code that handles sensitive data. In other words, security can
be even harder than security professionals think.
I agree with you. And while I don't think there's any secure way to use realloc(), I would like to remind people that stdio does give you control over the internal buffer (setvbuf()), which you can potentially set to a buffer that can't get swapped to disk and which you can zero out at the appropriate time. You can also call fflush() to be sure that the data in stdio's buffer can be zeroed even before the file is closed.
One alternative would be to sidestep this memory-reallocation-and-buffering problem altogether and just use the code that already exists to connect to an ssh-agent, even when only private keys from the local filesystem are used for authentication:
Let ssh spawn an ephemeral ssh-agent just for the duration of the key-exchange and just for this single ssh instance. Kill it once authentication is complete.
> In fact, the article's conclusion that the OpenSSH team should have rolled their own memory management code rather than use tried, tested and approved libraries doesn't sound like a good recommendation to me.
I agree with your general statements but disagree with your ending conclusion. Notice that they never mentioned `malloc` in that post, nor `read` and `write`, etc. If you're writing code that needs to be secure, then it's important to be able to recognize what features you can use, which you can't, and thus what pieces of the standard-library are usable.
`malloc` is perfectly fine, because it will never touch the memory you put inside it - If you clear the memory with a `memset` before `free` (And make sure the `memset` isn't removed), then `malloc` and `free` will never be able to copy your memory or leave it laying around. `realloc` breaks this guarantee because it explicitly will make a copy if it has to find a new place to put the buffer, and thus it's not usable if you need to keep the above guarantees about copies of the data.
`fread` and `fwrite` are obviously out for the same reason - There's no guarantee they'll clear their buffers. For such a situation you definitely want unbuffered IO, and the best way to do that is to just directly do `read` and `write`. You could write your own buffer layer on top of it (that is securely cleared), but it's not necessary to use buffers at all - Especially if the file is small enough that you could just read the entire thing into a single buffer anyway. (Note that it may be possible to get those guarantees through some `setvbuf` and similar calls - I'm not completely sure since I've never tried - It's not the defaults regardless).
The big problem with the approach taken is that it doesn't take enough precautions when handling secure data, and uses functions that explicitly do things they don't want.
> Part of how they are so easy to overlook is that we are trained to think in terms of abstractions, not the details underneath them.
This is a problem in direct proportion to how leaky the abstractions are. C's abstraction over memory is comically leaky, so indeed, if you're using it, you need to think about the details underneath it.
But the solution isn't for everyone to forget about abstractions, it's to use better-made abstractions.
For memory in particular, it's been understood what it takes to have a safe abstraction for quite a long time now, and numerous widely-available languages provide one. Use one of those instead of C.
> For memory in particular, it's been understood what it takes to have a safe abstraction for quite a long time now, and numerous widely-available languages provide one. Use one of those instead of C.
That advice doesn't really help the OpenSSH team at all. OpenSSH is a C project and it certainly is not going to be rewritten in a safer language. Their are many reasons for that, first reimplementing OpenSSH would take lot's of effort, and the OpenSSH(OpenBSD) people don't have much man power to spare. Secondly OpenBSD is a C shop for better or worse, like most open source operating systems. Thirdly what language can fulfill all the constraints of OpenSSH and be memory safe? Let me try and explain, OpenSSH is included in many operating systems, like OpenBSD, FreeBSD, most of the linux distros, etc. Most of these groups are not going to want to include languages that require big complicated runtimes into their base operating systems. So that get's rid of any JVM based language, erlang, and any other non compiled language. So that leaves languages like Go or rust, which most certainly are safer languages than C, but can they run on all the platforms listed on OpenSSH portable?[1] And just as importantly will Operating systems add another programming language to their base system, which increases the effort required for maintenance just so they can switch the implementation of a working program. I personally don't think most major operating system would make the switch. Finally OpenSSH has a pretty good track record when it comes to security, especially considering how little funding they receive, it would probably require less effort to continue improving OpenSSH then to ditch it and start over and get everyone to use the new version.
I take your point, but OpenSSH is almost a counterexample. The OpenSSH team's goal is to make the best possible SSH daemon for OpenBSD; it's a nice bonus that they can make a portable version as well. They're doing this as part of the broader OpenBSD effort. OpenBSD is a cohesive, relatively small (for a full-blown operating system!), independent, and tightly focused project; they are in a better position than anyone to introduce a new language. Plus, they already have a track record in throwing away old software and replacing it with something entirely new [1] [2].
You're right that languages with big runtimes would probably not be worth the cost, and new languages like Rust and Go are still too risky. But how about something like Ada or Modula 2? Both are well-established (virtually ancient) languages with mature compilers, small runtimes, and proven use in systems development.
> For memory in particular, it's been understood what it takes to have a safe abstraction for quite a long time now, and numerous widely-available languages provide one. Use one of those instead of C.
Is it not the implementation of those abstractions that determines whether they are safe? Offhand, I don't recall seeing guarantees of safety in this regard being offered by any language I am familiar with, though I have to admit that it has not been something I have been looking for.
It is the implementation which matters, or rather, the observable consequences of the implementation, which in effect form part of the interface of the abstraction - they might not be intended, but they're there. Those are what we call leaks.
The abstraction i was thinking of was byte arrays, which are provided by the language, rather than anything higher-level. Many languages have byte arrays which guarantee that you can't read past the end, and can't read values that you didn't write. Java would be one example.
Could you be more specific on what you have in mind here?
As far as I can see, if something like Heartbleed exists, you can't rely on "safe memory abstractions" at a lower level, because Heartbleed let the attacker look at the internal memory of those abstractions. They quit being abstractions; their raw memory is open to the attacker.
Or is your claim that safe abstractions would have prevented Heartbleed (and all other possible attacks that let the attacker read memory)?
For Heartbleed, you need an abstraction like "container that i can write bytes to and then read the same bytes from" which has the leak "sometimes you can read other bytes too".
In C, the memory abstraction the language gives you has that leak, so it's really hard to build a container on top of it that doesn't have that leak.
In safer languages, the memory abstraction doesn't have that leak, because bounds are checked, and new byte buffers are initialised to zero. You can still build an abstraction which does have that leak - say, by writing your own reusable byte buffer class which doesn't check bounds or zero on reuse - but at least you have the option of getting it right.
In an even safer language, the type system might make it impossible to make that mistake, or the library might supply a safe implementation of that abstraction already.
Remember, Heartbleed is not about the attacker doing some kind of Jedi memory trick to read your secrets: they are relying on your own code to read things it shouldn't. If the abstractions your program is built on don't allow that, then there's no way for it to be used to that end.
Pretty sure he is claiming that bad abstractions make it very likely that you will produce code which doesn't do what you think it will do, such as leaking memory contents.
The bug is in the compiler, not OpenSSH. The OpenSSH developers explicitly erased memory with memset. GCC decided to remove those lines without any warning. There's the bug. (Here come the language lawyers...)
What's the difference between GCC deleting parts of your code, and an attacker hacking into the source code repository and deleting those parts of the code?
The C standards committee addressed the problem in 2009 with memset_s. [0] The GNU developers reject patches and state they hope this feature is never implemented. [1]
Pretty much every claim you just made is wrong, but at least you provided the sources to demonstrate such. In short, dead store removal is not a bug, it is explicitly allowed. C11 Annex K is optional, and the rejection was not based on this feature, it was based on other problematic requirements imposed on a conforming implementation.
CompCert is not free software. This non-commercial release can only be used for evaluation, research, educational and personal purposes. A commercial version of CompCert, without this restriction and with professional support, can be purchased from AbsInt. See the file LICENSE for more information.
I'm not entirely sure if something like that is good for an unpaid-for open source project...
This of course is all true and important. But there's an alternative here - keep private data unreadable. I believe if you use smartcard authentication via gpg-agent, ssh simply has no access to your key. The only thing that could be leaked is the session key (which the other side could get via standard exchange anyway).
I moved to yubikey some time ago and don't regret it.
The downside of Yubikey is that if it was manufactured before April and you used OpenPGP card mode, anyone who manages to steal your Yubikey could use it to log into your servers without needing to know your PIN/password because PIN authentication was completely broken and bypassable on the earlier Yubikeys. YubiCo also heavily downplayed the importance of this issue, claiming in their announcement that it didn't matter because anyone who got hold of your Yubikey could get hold of your PIN anyway. See https://developers.yubico.com/ykneo-openpgp/SecurityAdvisory...
Every system had, has, or will have issues. That's why you have multiple layers of protection from cpu rings to user training. Sure, they had issues, but I'll still take hardened box, with ssh, with yubikey over ssh on its own. I don't trust any of those layers, but each provides more difficulty in exploitation.
(Also, this still doesn't provide the key itself. That means even with the vulnerable yubikey you can only spoof connections while it's plugged in)
It means that anyone who can swipe your Yubikey for a couple of minutes or even just wave an NFC-enabled phone past it can SSH to your server and then at their leisure convert that connection into a persistent backdoor. How often do you check your .authorized_keys? Your list of running processes? Your .bashrc for a line redirecting sudo to a small binary that logs your password for later use? Arguably this even made it less secure than a normal USB key with your SSH key on it encrypted with a strong passphrase.
I think the more obvious way to keep private data unreadable is ensuring memory safety: write only the performance critical parts in memory unsafe languages like C and prove them memory safe.
You realise that definition also applies to hardware keys, right? They're unreadable, private data via memory safety - there's only limited interface to operate on that data in valid ways.
Proving code correct is far from obvious though. Apart from single instances like sel and python's sort, can you remember any verified software with proofs right now?
Memory safety is only a subset of verification. In fact, most programming languages are memory safe (although I think there are no formal proofs, I haven't come across memory unsafety in a language that claims to be memory safe).
seL4 is actually another verified software I know.
I remember people groaning (and I was probably amongst them) about DJB for being unnecessarily paranoid for implementing his own IO functions instead of using stdio (for Qmail for example).
I think the real answer to this sort of problem would be to set up a unit test that involves making a full memory dump and scanning it for private key material. The traps here are just too subtle to try to catch all the failure-to-zero bugs in any other way.
Some people used to claim that you should never use your own crypto code because the libraries are designed by people infinitely smarter than you who thought of everything.
The sentiment behind that is to keep out those that don't know how to code. If everybody followed that advice there wouldn't be crypto code.
If you have the ability to create good code and are confident you can understand the maths of cryptography then by all means contribute to the community. If the past year has taught us anything it's that the crypto community needs more good engineers.
People write their own toy kernels or malloc()s all the time, and there's never the assumption that they would actually try to use them for real, and the security implications are just as real there as for toy crypto.
It is very useful to know how good the state of the art is to get a feel for if you could contribute or not. And I think that the state of the art has been exaggerated and that has discouraged contribution.
We can not just trust that these libraries are higher level esoteric magic that no mortal could understand. It is time to shine light on exactly what guarantees different libraries are providing, and how.
But we also know the state of the art of people who don't really know what they are doing going to write crypto, and that state of the art is "laughable".
You're being pointed straight at the problems discovered by other people. Of course they're obvious to you now. The question is, can you just pick up some OpenSSH code, read it and run it, and find a new problem yourself when nobody is pointing you straight at it? Because I'm sure there's at least one in there for you to find.
(Note I did not ask you if you could find this problem. Too easy to imagine that you can now, too hard to realistically pretend you don't already know about it. I'm asking you about new problems that nobody currently knows about.)
apart from the fact that the current openssh issues are pretty much unrelated to crypto code, I'll still stick with not implementing my own crypto, just because
a) I would not have thought of the issue of using a standard library call to load data.
b) nobody would have checked my code and fixed it
so in the end, I'd still be vulnerable while openssh is now fixed.
I think this is a very key point. OpenSSH gets targeted for vulnerability analysis by very talented people, on a regular basis. I would be surprised if there are more than a dozen organizations on the planet that can deploy those kind of resources against their homegrown code.
It is kind of true that you should not _only_ rely on homegrown and not well-tested solutions because they may contain pretty trivial bugs. When using several layers of security, adding non-standard or home-grown layers may add significantly to the security considering widespread exploits of well-known software.
E.g. use VPN and password protection and encrypted volumes which are pretty standard. Add in a custom hardware switch that physically disconnects the power/network of a server containing sensitive data which is rarely needed and you will be protected against many exploits.
Higher-level code is just an abstraction layer over the lower-level workings, and if that abstraction layer doesn't clear memory properly there may be no way to fix that without bypassing it altogether.
Ok, work hard to clear secretive data from memory after using it... Just be sure to sanitize that code that manages untrusted inputs first (and by all means, make sure those two are different) because:
- Your input parser can compromise many more things than what you'll think about erasing.
- You won't be able to clean all secretive data anyway. You can't win here.
- You can be sure to a huge degree that your input parsing code won't get outside of its bounds.
If the abstraction layer doesn't let you read memory that's left over from some previous use, then it doesn't matter (for this vulnerability) if it clears it or not.
Rowhammer is a great demonstration that the memory abstraction in almost all languages does let you statistically modify the memory of other processes on appropriate hardware!
I don't think rowhammer lets you read memory, though; does it? There are certainly some cache timing attacks that do, though.
There's no inherent problem with the approach taken (not like OpenSSL reinventing several wheels and causing things like Heartbleed). In fact, the article's conclusion that the OpenSSH team should have rolled their own memory management code rather than use tried, tested and approved libraries doesn't sound like a good recommendation to me.