5 ms·
Putting passwords in the php file? That's not a seasoned developer in my mind.
by VMG 16y ago
Putting passwords in the php file? That's not a seasoned developer in my mind.
- zaidf 16y agoGood point. Minus the keys/passwords the "harm" would be limited.
- Joakal 16y agoIt's needed to connect to databases with authentication. What would you suggest?
- eam 16y agoI believe the way to do it "correctly" would be to put the actual file containing the credentials in a non-public location and just do an include where you need to access it. At least that's how I do it. I could be wrong...
- windsurfer 16y agoIf you're doing an include, wouldn't the passwords still be in a PHP file?
- lsc 16y agoif the php file is printed rather than executed, the include will not be followed. You'd see the "include /path/to/inaccessible/file" but you wouldn't see the passwords within the include. I think. I haven't seriously used php since 2003 or so.
- russss 16y agoYes, but included files still need a <?php tag at the top, or PHP just prints them out.
- nbpoole 16y agoThat wouldn't help in this case, since a typo in the config file is making the file not be parsed as PHP. If you want to avoid that possibility, you can use a non-PHP format (eg: YAML) and parse it.
- drdaeman 16y agoIf one would made a similar typo in that file, its contents will be displayed too. A solution would be to have a .ini-like (or some other simple-to-parse format) config file and PHP code to read its contents. PHP code could be leaked, but config file contents wouldn't.
- deleted 16y ago[deleted]
- sewerhorse 16y agoit's always included to the document root (the bootstrap file). it doesn't matter where you're including from, a broken open tag would cause errors like this one.
- drdaeman 16y agoI believe you are wrong. include()/require() would equally happily display any file's contents outside of `<?php ... ?>` scope (the case with "i?php"), within document root or not. Edit: I've tested this: $ php -v PHP 5.2.6-3ubuntu4.6 with Suhosin-Patch 0.9.6.2 (cli) (built: Sep 16 2010 19:51:25) Copyright (c) 1997-2008 The PHP Group Zend Engine v2.2.0, Copyright (c) 1998-2008 Zend Technologies $ cat test.php <?php require "/tmp/test2.php"; ?> $ cat /tmp/test2.php i?php define("TEST", "test"); ?> $ GET http://localhost/test.php i?php define("TEST", "test"); ?>
- CWIZO 16y agoOff course, but in this case, passwords would only be exposed if the config file had a miss-typed opening PHP tag. If "test.php" had it, you wouldn't be able to see the contents of "test2.php".
- drdaeman 16y agoYes, you are right. And in this exact case they (mis-)edited the file, that contained passwords (i.e. test2.php in my improvised example).
- petervandijck 16y agoTypically, these files are not web accessible. Problem solved.
- drdaeman 16y agoClient SSL certificates? I don't know what DB they're using, but at least Postgres and MySQL support certificate-based auth.
- pilif 16y agoHow is a client certificate different from a long random password? Both have to be stored somewhere. The correct solution is to keep this (and everything else) out of your web root. If that i?php blunder happened to me, my users would see 10 lines of code. One include for the framework (which lives outside the web root), plus three calls to get the framework to handle the current request. Those 10 lines would be totally unproblematic
- drdaeman 16y ago.pem file won't be leaked in this ("i?php") manner. Only file's location would be revealed. Of course, you are completely right, it should still be inaccessible from the web.
- deleted 16y ago[deleted]
- tjogin 16y agoA huge mistake, but Tumblr's staff are probably quite experienced, nonetheless. How about that gotcha though? A single character is mistakenly changed from "<" to "i" and that exposes the source code to the browser. Think about that.
- hasenj 16y agoI wonder if the 'i' has anything to do with vim
- moe 16y agoClearly they should ban vim from their development process! ;-) No, joking aside, the normal way to handle this is to have the perimeter server (nginx or varnish) catch any 5* responses and turn them into a user-friendly error-page. That way you never expose sensitive stack traces to your users. So, this is standard stuff and easy to fix. However who of us hasn't screwed up on a similarly trivial issue before? I wouldn't judge them too hard on this one, happens to the best of us.
- tjogin 16y agoThat wouldn't have caught this though, this code wasn't even executed, just sent as plain text.
- moe 16y agoNginx/varnish can also catch Content-type: text/plain (or text/php?) and treat that like a 5xx response. Yes, that's a bit of a hack, but unless tumblr needs to serve text-files (which I doubt) it's a legit safeguard.
- nikcub 16y agoI know you were kidding, removing all text editors from your production servers might be a good idea
- weinzierl 16y ago
- russss 16y agoI'm going to disagree with you. The problem with PHP is that there's no easy way of sharing state between requests. If you have a separate (ini-style) configuration file, every time you get a new request, the file will have to be read in from disk and parsed. On heavily-loaded web servers this can be a significant performance issue. Configuration stored in a PHP file will be cached by your opcode cache and so doesn't incur any per-request parsing/reading overhead. The problem here is not that Tumblr stored their configuration in PHP. The problem is their lack of testing their changes. (As I mentioned elsewhere in the thread, this particular nasty issue can be solved by simply enforcing that every .php file begins in '<?'.)
- Kudos 16y agoSure there is, APC. But that aside, Linux won't read that file every time, it will end up living in the disk buffers that it maintains.
- moe 16y agoThe disk buffers make the operation cheap but not free, i.e. keeping the information cached is the right thing to do.
- jasonlotito 16y ago> If you have a separate (ini-style) configuration file, every time you get a new request... it will just pull it from the cache, not needing to be parsed or read from disk. Configuration stored in a PHP file is bad. Changing working code simply because you want to add a new slave is a horrible idea. It would be like having to recompile Apache from source every time you want to add a new vhost because all the entries are written in C. Interpreted languages like PHP, Python, Perl, etc all make it easy to put config options directly into the language. When your app is small, it's fine. But as it gets larger, move it out. But yes, that they didn't even catch this is in testing is the real problem.
- russss 16y ago> it will just pull it from the cache, not needing to be parsed or read from disk. I don't believe PHP's ini_read caches anything. Sure, your OS cache is going to have that file in, but PHP still has to do the fopen(), fread(), parse, fclose() dance on every page hit. This does not happen with an opcode-cached source file. I have actually benchmarked this. Amortized across billions of page hits per month it produces a notable saving. Kudos points out that you could use APC - this is absolutely true but IMO it basically equates to doing the same thing in a more complex way. > Configuration stored in a PHP file is bad. Changing working code simply because you want to add a new slave is a horrible idea. It would be like having to recompile Apache from source every time you want to add a new vhost because all the entries are written in C. This is a false comparison between interpreted and compiled languages. Not to mention that plenty of C programs use #defines for configuration. Varnish even translates its configuration language into C and dynamically loads it as a module, which provides significant flexibility. In a dynamic language, and where you trust the person doing the configuration, there's no reason why your configuration shouldn't be in a source file. Both Django and Rails do it this way.
- themonk 16y agoNo, better words would be: "Putting passwords in php file under doc root, why not keep it outside doc root?"