8 ms·
Things I learned from OpenSSH about reading very sensitive files
- im2w1l 11y agoSome 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. That is starting to ring hollow to me.
- anon1385 11y agoJust because they don't get things perfect doesn't mean they don't do a much better job than you or I could do.
- im2w1l 11y agoIt 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.
- jerf 11y agoBut 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.)
- Xylakant 11y agoapart 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.
- passive 11y agoI 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.
- someguydave 11y agohow many of those dozen organizations would actually release the results of their analysis?
- Xylakant 11y agoGiven the number of CVEs filed a nonzero number. In any case more organization than the number of orgs that would bother to check my code.
- nextw33k 11y ago"you should never use your own crypto code" 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.
- creshal 11y agoThe sentiment should be "write your own code, but don't use it without independent audit". There's another recent discussion about it here: https://www.reddit.com/r/crypto/comments/418nyl/write_crypto_code_dont_publish_it/ https://www.reddit.com/r/crypto/comments/418nyl/write_crypto...
- rhinoceraptor 11y agoPeople 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.
- static_noise 11y agoIt 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.
- Klathmon 11y agoActually using home-grown layers may compromise the entire system. An easily exploitable buffer overflow or an extremely easily overlooked crypto blunder in a second layer can easily allow a way in. I agree with the example given, but it's a rule that's wrong more often than it's right.
- viraptor 11y agoThis 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.
- makomk 11y agoThe 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%202015-04-14.html https://developers.yubico.com/ykneo-openpgp/SecurityAdvisory...
- alexandrerond 11y agoThey are replacing all affected keys for free. That seems the right thing to do, even if they downplayed at first.
- emilecantin 11y agoCan confirm, got a replacement very fast after asking nicely.
- viraptor 11y agoEvery 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)
- marcosdumay 11y agoFunny thing. I just thought it was yet another memory leak, that could be avoided by higher level code handling dangerous network data. But if all the lessons the author took are that secure code should be lower level than this, well... I can not agree with it.
- makomk 11y agoHigher-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.
- twic 11y agoIf 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.
- kuschku 11y agoExcept, it does. Remember RowHammer? https://github.com/IAIK/rowhammerjs https://github.com/IAIK/rowhammerjs
- twic 11y agoRowhammer 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.
- kuschku 11y agoI’m not sure, but it’s a great demonstration that, just because the abstraction limits direct access, it can not limit indirect access.
- 11y ago
- gargravarr 11y agoThere 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.
- HerrMonnezza 11y ago> 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.
- maxlybbert 11y agoI 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.
- cnvogel 11y agoOne 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.
- Hello71 11y agoZeroing buffers is insufficient: https://news.ycombinator.com/item?id=8277928 https://news.ycombinator.com/item?id=8277928
- twic 11y ago> 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.
- AnimalMuppet 11y agoCould 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)?
- voxic11 11y agoPretty 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.
- twic 11y agoFor 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.
- jimrandomh 11y agoI 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.
- vidarh 11y agoI 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).
- eliteraspberrie 11y agoThe 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] The bug fix is to stop using GCC for sensitive code. Use CompCert instead. https://github.com/AbsInt/CompCert https://github.com/AbsInt/CompCert [0] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1381.pdf http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1381.pdf [1] https://sourceware.org/ml/libc-alpha/2014-12/threads.html#00506 https://sourceware.org/ml/libc-alpha/2014-12/threads.html#00...
- zzazzdsa 11y agoI quote from their Github readme: License 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...
- lawnchair_larry 11y agoPretty 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.