I'm not going to say don't write crypto. Instead I will give more useful advice: if you do write crypto, make it boring. Really boring. Do not invent anything. Do not be creative. Use an established modern construction with modern ciphers and use them in only one way. That one way should be the way cryptographers recommend with no deviations.
The crypto in Magento is extremely boring. In fact: it's too boring. It predates misuse-resistant interfaces like Nacl. It's not enough to be boring: you have to be the right amount of boring.
I was joking 10 years ago when I wrote "if you're typing A-E-S into your code you're doing it wrong", but more and more I feel like I was on to something.
The situation is far from ideal. Recommending libsodium-php is also not as simple, as this requires an additional thing to be installed on a server, which probably isn't the case for most shared hosting environments.
I think it's great that stuff like this is brought to the surface, but it also troubles me that the author makes no mention of submitting a PR to improve the _open source_ code base... They seem more content to discuss boycotting and abandonment instead.
Is the author obligated to fix the broken code? If they aren't willing to fix it, would you prefer they said nothing and left everyone using insecure crypto?
Definitely not. That's why I said it I think it's great that this stuff is brought to the surface. I can see how what I wrote isn't entirely clear though. What I am trying to say is that it would be nice if the message also aimed at encouraging contribution to open source software as well. It seems healthier to OSS as a whole to encourage improvement rather than abandonment.
It definitely is open source insofar as it being a public repository that accepts contributions. However, the disclaimer at the top of the repo does seem to suggest that free offerings might be scooped up and put to other uses... I can see where the degree of "open sourceness" might be questioned in a case like this. I will re-iterate that I am willing to accept that there might be other politics at play here.
There is simply no way that realistically a third party contributor will rewrite a core part of a platform product for a company that generally does not accept such work, and has an enterprise version of the software that really gets all the goodies instead, which is where their focus is.
If it were true open source I think the project would get a lot more contributions. The enterprise edition, with the "best" features, is very much not free.
This is absolutely true. They actually have a hostile attitude towards contributions. I used to work at a Magento shop (bad times all round) and my (extremely talented) colleague would often submit patches for long-standing bugs through the ticket tracker which would never appear in general release. Tickets would go for months without responses. It's commercial software at the end of the day. With an obscene price tag on it to scam cash out of non-technical business founders.
None as far as I can tell. There's OpenMage, which accepts contributions. But the problem is that Magento Enterprise is shit, and the Community Edition is shit with the functional bits removed.
Magento is very much open-core in all practical terms.
It sure would be nice if the author did that, but crypto is a non-trivial amount of work to get right.
It's easy to look at one small part of a codebase and say "this is wrong", but to get it right you have to learn large chunks of the code. Be aware what level of work you are shoving onto the author in your comment.
I did aim to clarify things in one of my responses below. It wasn't my intent to suggest that the author should necessarily be the one making a PR. My main point is that contribution and improvement should be encouraged where this post leans more towards staying away from a piece of open source code altogether.
But keep in mind that people have plenty of time to have a look at a codebase, but often do not in any way shape or form have anywhere near enough time to actually work on that code, even to file what you might call "a simple PR". Spotting errors is a very quick job. Writing even a small PR on a codebase you do not work on, that properly passes tests (in this case crucial) as well as fuzzing (again, it's crypto, it's crucial) takes hours if not days. Sometimes, and this is one of those times, the only thing you're going to realistically get is feedback that you should act on ASAP, not code that helps you get started. And that's a critical contribution, too.
I'm not certain that one would necessarily need/want to take a another stab a "rewriting" core code here. If they are leaning on an interface, which it appears as such (EncryptorInterface), and have managed to avoid implicit coupling etc, it might be possible to swap out the implementation with something else. Maybe one of the other libs that have been recommended in this thread.
I also wrote https://github.com/paragonie/halite which is a PHP library aims to make libsodium incredibly easy for PHP developers to get right.
For systems that need asymmetric crypto and can't install libsodium, I also wrote https://github.com/paragonie/EasyRSA which side-steps a lot of the mistakes developers make. EasyRSA uses defuse for symmetric-key encryption, then encrypts the AES key with RSA.
Writing secure cryptography isn't trivial. Assuming the three projects I just linked to are secure enough to use (all indications point to: they are), there's literally no reason to reinvent the wheel solely for Magento.
Magento already uses Composer; they could just add them to their composer.json file and rewrite their routines to use defuse + maybe EasyRSA if they have a use case for it.
Magento is a for profit platform owned by eBay. He's already donated them enough of his time. It is open sores, not open source, although it's easy to confuse the two!
Look, I don't care if you use PHP. But if you aren't an expert, and your code hasn't been audited by experts, don't write your own crypto. Use libsodium, or libressl's libcrypto (if you think openssl is a good bet, you really need to see the talk that discusses the reasons for that fork), or libgcrypt. But for crying out loud, use a library that is actively used and vetted by people who KNOW WHAT THEY'RE TALKING ABOUT. And do your best to know what you're talking about, too.
No! Libre/Open/BoringSSL's libcrypto exposes the same low-level primitives as PHP's terrible crypto bindings do. Please, please don't build things directly with libcrypto.
And that - in an ironic nutshell - is the problem with crypto: had you been tasked with implementing the crypto, you would have (unknowingly) done it wrong and called it a day. Someone else would be berating you to "leave it to the experts"
I did laugh - but not at you. As I said - the problem is with crypto (not you). Crypto gives the appearance of being simple, straightforward and immutable - but its not. A lot of things can go wrong with your PRNG, or the key exchange, or the cipher, or a combination of any/all of the above. Oh, by the way a novel attack was discovered last week; it's the full package, complete with a mildly ominous ALLCAPS name, a fancy logo and a bloody domain - the state-of-the-art has changed. Again.
I don't know if the 'crypto problem' can be solved for all F/OSS projects that need it - there might not be enough experts to go around.
Aside from the obvious deficiencies, who the heck writes code like "if ($false === initVector)"? Is this a side effect of people who speak other languages with different grammatical structures? Very odd to read through code written "backwards" like that.
> Magento, one of the largest open source e-commerce platforms, ships a broken cryptography library that clueless developers are probably using to encrypt your credit card information for their client's customers.
Nope. There is a built in credit card method that would utilise the Magento crypt() but you cannot actually hook this up to a bank and get any actual money. The lamest of lamest Magento developers will use an off the shelf payment gateway that will use things like iframes so that even a non https Magento site will not have any credit card information pass through Magento, instead you get payment references.
In theory you could break the crypto and get into Magento admin, to then export out customer email and address details. You could probably refund all customers orders but not get fresh money out of them.
I appreciate the code may be 2008 vintage but there is no new vulnerability here that gives any means to access any Magento credit card data in a meaningful way, e.g. a lucrative way.
Magento is incompetent on much simpler levels than this.
1. Someone thought chmod 777 was a good idea, ever, under any circumstances. Not only is this standard practice in Magento installs (it's a how-to step in many books on Magento), it's all through the actual codebase.
The below is from the Magento Enterprise 1.14.0.1 tarball, downloaded from the company (and I double-checked this after someone questioned this last time I brought this up):
The way we eventually dealt with hosting Magento (which we had strongly advised against) was a concrete sarcophagus and a thirty-kilometre exclusion zone:
* a cron line specifically to remove o-w permissions from all files in the webroot every minute (which is very inelegant, but the alternative is maintaining our own patches to core).
* Files not owned www-data, except where Magento must be able to write to them.
* deploy all webroot files as a user the webserver can't write.
* cron.sh (Magento's internal cron) runs as root out of the box. We ran it as www-data.
* AppArmor to keep Magento from ever, ever being able to pull shit. This caught Magento's more antisocial tendencies on more than one occasion.
* Admin login: use a path other than "/admin" to foil quite a lot of attack bots at the very simplest level.
We have outsourced our remaining Magento, thankfully, and I don't personally have to maintain the above any more. (You know you've been administering Magento a bit long when you can hum along to bits of "Metal Machine Music" accurately.)
The use case for Magento is (apparently deliberately) confused. It's an unholy melange of a CMS and a shopping basket. There is no good out-of-the-box experience; in practice it's a job creation scheme for consultants.
Even crap-tier "well technically I can tell my boss's boss we have paid support" support, with ...
> The use case for Magento is (apparently deliberately) confused. It's an unholy melange of a CMS and a shopping basket. There is no good out-of-the-box experience; in practice it's a job creation scheme for consultants.
The CMS capabilities of Magento are quite limited, as the Community Edition does not even include a "menu" module or a hierarchy for your static pages. My opinion is similar to yours, it is a powerful shopping basket that will require lots of custom development or third-party module juggling to get a decent result.
I have been working with Magento 1.x CE for a few years now, and I wanted to add a few more factoids from a developer perspective:
* The database API has an "you want it, you got it!" mentality - if you tell it to add a unique index and MySQL refuses to because existing data is not unique, the API will parse the MySQL error message to identify the duplicated fields/values and delete them silently. You will get your index, but you might no longer have all your data. [0]
* The model classes store nearly all the in-memory data in an untyped dictionary. You can call $model->setFoobar('12345') and later $model->getFoobar() to get it back - all unknown method calls get forwarded to a "catch-all" method which treats the method name as a key into this dictionary, and gets/sets a value of that key [1]. Most of the time, these calls are unannotated, so IDEs can't make any sense of them (see the "Undefined method" part in [2]). This unknown method forwarding is also a performance issue, so they make optimizations like [3] by inlining pieces of the catch-all handler code for commonly-used data.
* The JavaScript is based on Prototype.js, not jQuery, so half the modules and themes bundle their own copies and drop them in different locations. Some of them provide a configuration setting so you can disable their jQuery and use your own, some don't.
* It includes a "Developer Mode" where php notices and warnings are turned into exceptions like in sane languages. But plenty of modules and themes have never been tested with this setting on and they cause faults on every page load - either you have to disable the Developer Mode, or fix this third-party code.
> I don't have a good answer on the shopping basket, but Magento was bad enough at that too that we went back to our in-house homerolled system.
Our clients were originally in love with Magento because "we can just add all these existing modules and get all sorts of awesome functionality without having to pay you for it!" We warned them that that was a pipe dream, but they didn't listen. Now they have several thousands of hours worth of custom development on top of Magento 1.x (95% of it written solely by me, no bus factor problems with that) and it's not even finished yet. Last I checked, the end-of-life for M1.x is planned in December 2018, and the client has no plan for what to do then.
> The database API has an "you want it, you got it!" mentality - if you tell it to add a unique index and MySQL refuses to because existing data is not unique, the API will parse the MySQL error message to identify the duplicated fields/values and delete them silently. You will get your index, but you might no longer have all your data. [0]
What does the comment about not using authentication mean? What would be authenticated in this case?
I think I'm in a bit over my head looking at the code, but does the author mean that somehow the data should be verified to come from a trusted source first? But doesn't the attacker have to have some of the encrypted data first for that to be meaningful?
Authentication in this context means baking in a way to verify that what you receive is something you encypted and has not been altered. Or, in other words, it lets you know that the data hasn't been tampered with.
As a practical example, imagine using encrypted cookies for a session store (like Rails can do), but imagine that they're not authenticated. It's possible that with clever manipulation of the encrypted cookie, I can alter the cookie such that it still decrypts into the structure ecorcyed, but with different data. For example, maybe I change the initialization vector a bit in a clever way and that makes the userId stored in the cookie change.
For a more detailed explanation and hypotherical, I thought [1] was a great article. It uses PHP for the examples, but it's mostly the theory that's of interest.
Of note, things are actually worse in Magento's case than this hypotherical since you can, as an attacker, get Magneto to use some pretty bad crypto when decrypting your message even if they used something decent when encrypting their payload (remember: since it's unauthenticated, what they give you and what you give back are not necessarily the same).
thanks for this. If I may sum up and you could verify this:
When I encrypt something and give it to you, I should take a signature (hash of the encrypted msg) and append it to the encrypted message. Then if you twiddle bits before asking me to decrypt it, I'll see that your provided signature is incorrect.
I know there's more in it, and I've briefly read through the article you provided, which I may look at it in more detail later, but I think that's the gist of it.
Exactly! In fact, that's embarrassingly concise given my rambling. The only major thing missed is that the hash input has to contain something that the attack doesn't know in addition to the message. Or in other words, the private key is usually also involved in the hashing. If you ever want to get into details, the MAC [1] and HMAC [2] wiki pages go into depth.
52 comments
[ 3.4 ms ] story [ 120 ms ] threadI was joking 10 years ago when I wrote "if you're typing A-E-S into your code you're doing it wrong", but more and more I feel like I was on to something.
https://paragonie.com/book/pecl-libsodium
Docs happen to be written by the OP too...
I often find his posts interesting and sometimes a welcome adjustment to some long-running counterproductive memes.
The situation is far from ideal. Recommending libsodium-php is also not as simple, as this requires an additional thing to be installed on a server, which probably isn't the case for most shared hosting environments.
I didn't like my magento job much :/
Magento is very much open-core in all practical terms.
It's easy to look at one small part of a codebase and say "this is wrong", but to get it right you have to learn large chunks of the code. Be aware what level of work you are shoving onto the author in your comment.
I also wrote https://github.com/paragonie/halite which is a PHP library aims to make libsodium incredibly easy for PHP developers to get right.
For systems that need asymmetric crypto and can't install libsodium, I also wrote https://github.com/paragonie/EasyRSA which side-steps a lot of the mistakes developers make. EasyRSA uses defuse for symmetric-key encryption, then encrypts the AES key with RSA.
Writing secure cryptography isn't trivial. Assuming the three projects I just linked to are secure enough to use (all indications point to: they are), there's literally no reason to reinvent the wheel solely for Magento.
Magento already uses Composer; they could just add them to their composer.json file and rewrite their routines to use defuse + maybe EasyRSA if they have a use case for it.
Zend\Crypt is also an acceptable choice, as long as you don't install the version with an RSA implementation vulnerable to padding oracle attacks. https://framework.zend.com/security/advisory/ZF2015-10
https://magento.com/company/press-room/press-releases/magent...
Go ahead, laugh. It means you'll be hesitant to make that kind of mistake, which is the main thing.
I don't know if the 'crypto problem' can be solved for all F/OSS projects that need it - there might not be enough experts to go around.
Yoda code, Wordpress introduced me to the phrase.
Nope. There is a built in credit card method that would utilise the Magento crypt() but you cannot actually hook this up to a bank and get any actual money. The lamest of lamest Magento developers will use an off the shelf payment gateway that will use things like iframes so that even a non https Magento site will not have any credit card information pass through Magento, instead you get payment references.
In theory you could break the crypto and get into Magento admin, to then export out customer email and address details. You could probably refund all customers orders but not get fresh money out of them.
I appreciate the code may be 2008 vintage but there is no new vulnerability here that gives any means to access any Magento credit card data in a meaningful way, e.g. a lucrative way.
1. Someone thought chmod 777 was a good idea, ever, under any circumstances. Not only is this standard practice in Magento installs (it's a how-to step in many books on Magento), it's all through the actual codebase.
The below is from the Magento Enterprise 1.14.0.1 tarball, downloaded from the company (and I double-checked this after someone questioned this last time I brought this up):
2. The company thinks there's nothing wrong with storing money as floats: https://github.com/magento/magento2/issues/555The way we eventually dealt with hosting Magento (which we had strongly advised against) was a concrete sarcophagus and a thirty-kilometre exclusion zone:
* a cron line specifically to remove o-w permissions from all files in the webroot every minute (which is very inelegant, but the alternative is maintaining our own patches to core).
* Files not owned www-data, except where Magento must be able to write to them.
* deploy all webroot files as a user the webserver can't write.
* cron.sh (Magento's internal cron) runs as root out of the box. We ran it as www-data.
* AppArmor to keep Magento from ever, ever being able to pull shit. This caught Magento's more antisocial tendencies on more than one occasion.
* Admin login: use a path other than "/admin" to foil quite a lot of attack bots at the very simplest level.
We have outsourced our remaining Magento, thankfully, and I don't personally have to maintain the above any more. (You know you've been administering Magento a bit long when you can hum along to bits of "Metal Machine Music" accurately.)
The use case for Magento is (apparently deliberately) confused. It's an unholy melange of a CMS and a shopping basket. There is no good out-of-the-box experience; in practice it's a job creation scheme for consultants.
Even crap-tier "well technically I can tell my boss's boss we have paid support" support, with ...
The CMS capabilities of Magento are quite limited, as the Community Edition does not even include a "menu" module or a hierarchy for your static pages. My opinion is similar to yours, it is a powerful shopping basket that will require lots of custom development or third-party module juggling to get a decent result.
I have been working with Magento 1.x CE for a few years now, and I wanted to add a few more factoids from a developer perspective:
* The database API has an "you want it, you got it!" mentality - if you tell it to add a unique index and MySQL refuses to because existing data is not unique, the API will parse the MySQL error message to identify the duplicated fields/values and delete them silently. You will get your index, but you might no longer have all your data. [0]
* The model classes store nearly all the in-memory data in an untyped dictionary. You can call $model->setFoobar('12345') and later $model->getFoobar() to get it back - all unknown method calls get forwarded to a "catch-all" method which treats the method name as a key into this dictionary, and gets/sets a value of that key [1]. Most of the time, these calls are unannotated, so IDEs can't make any sense of them (see the "Undefined method" part in [2]). This unknown method forwarding is also a performance issue, so they make optimizations like [3] by inlining pieces of the catch-all handler code for commonly-used data.
* The JavaScript is based on Prototype.js, not jQuery, so half the modules and themes bundle their own copies and drop them in different locations. Some of them provide a configuration setting so you can disable their jQuery and use your own, some don't.
* It includes a "Developer Mode" where php notices and warnings are turned into exceptions like in sane languages. But plenty of modules and themes have never been tested with this setting on and they cause faults on every page load - either you have to disable the Developer Mode, or fix this third-party code.
[0]: https://github.com/OpenMage/magento-mirror/blob/magento-1.9/...
[1]: https://github.com/OpenMage/magento-mirror/blob/magento-1.9/...
[2]: https://imgur.com/RMxWEgR
[3]: https://github.com/OpenMage/magento-mirror/blob/magento-1.9/...
> I don't have a good answer on the shopping basket, but Magento was bad enough at that too that we went back to our in-house homerolled system.
Our clients were originally in love with Magento because "we can just add all these existing modules and get all sorts of awesome functionality without having to pay you for it!" We warned them that that was a pipe dream, but they didn't listen. Now they have several thousands of hours worth of custom development on top of Magento 1.x (95% of it written solely by me, no bus factor problems with that) and it's not even finished yet. Last I checked, the end-of-life for M1.x is planned in December 2018, and the client has no plan for what to do then.
> I understand some work ...
Holy shit. Just, holy shit.
I think I'm in a bit over my head looking at the code, but does the author mean that somehow the data should be verified to come from a trusted source first? But doesn't the attacker have to have some of the encrypted data first for that to be meaningful?
As a practical example, imagine using encrypted cookies for a session store (like Rails can do), but imagine that they're not authenticated. It's possible that with clever manipulation of the encrypted cookie, I can alter the cookie such that it still decrypts into the structure ecorcyed, but with different data. For example, maybe I change the initialization vector a bit in a clever way and that makes the userId stored in the cookie change.
For a more detailed explanation and hypotherical, I thought [1] was a great article. It uses PHP for the examples, but it's mostly the theory that's of interest.
Of note, things are actually worse in Magento's case than this hypotherical since you can, as an attacker, get Magneto to use some pretty bad crypto when decrypting your message even if they used something decent when encrypting their payload (remember: since it's unauthenticated, what they give you and what you give back are not necessarily the same).
[1] https://paragonie.com/blog/2015/05/using-encryption-and-auth...
When I encrypt something and give it to you, I should take a signature (hash of the encrypted msg) and append it to the encrypted message. Then if you twiddle bits before asking me to decrypt it, I'll see that your provided signature is incorrect.
I know there's more in it, and I've briefly read through the article you provided, which I may look at it in more detail later, but I think that's the gist of it.
[1] https://en.wikipedia.org/wiki/Message_authentication_code [2] https://en.wikipedia.org/wiki/Hash-based_message_authenticat...