Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remote Code Execution Vulnerability via MITM. #1074

Closed
StormTide opened this issue Sep 6, 2012 · 50 comments
Closed

Remote Code Execution Vulnerability via MITM. #1074

StormTide opened this issue Sep 6, 2012 · 50 comments

Comments

@StormTide
Copy link

Composer runs code from HTTP sources without validating the source of the download or the code downloaded. As such, trivial man-in-the-middle attacks through any number of vectors (dns, networking, local server exploit, etc) will result in execution of code of an attackers choosing at the userlevel of the user running composer. (Typically a developer account)

Example command to reproduce. (User)

php composer.phar self-update

src/Composer/Command/SelfUpdateCommand.php lines 45, 50 and 59 are key.

Exploitation:

Replace getcomposer.org for a given network perspective by replacing it with a malicious http instance (eg by changing the DNS locally, at the lan, at an isp or hosting provider dns resolver, or globally or equally easily by replacing a route to the legitimate server (eg arpspoof)) . The http server instance is configured to serve a malicious /composer.phar and a /version url that produces random data. When users run self-update, the malicious code will be downloaded and run as the user that is executing the self-update command.

The same technique can be used for any package configured over http.

Fix:

Disable HTTP scheme's in composer and implement TLS or redesign composer to support digital signatures. (See AptSecure)

@Seldaek
Copy link
Member

Seldaek commented Sep 8, 2012

Doing secure HTTPS in php is unfortunately not as easy as it should be. Basically we'd have to ship our cert pubkey (or at least our CA's) within the sources, which means that at some point the old composer code would stop working if the cert changes. I'm not sure how to work around that yet, so for now we run in insecure mode (and let's be honest.. there are many many ways you can be in trouble if someone can MITM your machine, which is not to say it shouldn't be fixed)

@Seldaek
Copy link
Member

Seldaek commented Sep 8, 2012

I guess what might be a better solution than just relying on SSL is to do something similar as ssh does with the known_hosts, where we'd host the public key on packagist, and the first time if you don't have it on your machine it asks you whether you trust it (or maybe we do it silently if you don't have the domain yet) then if it changes you get a big fat warning. Then we could use the private key to sign everything else, and have the client verify it using the trusted pubkey. How does that sound?

@EvanDotPro
Copy link

...and the first time if you don't have it on your machine it asks you whether you trust it (or maybe we do it silently if you don't have the domain yet) then if it changes you get a big fat warning.

Silently accepting new keys is a bad idea. You should always prompt the user to add new keys, showing the fingerprint to the user. The fingerprint should also be available for confirmation via a valid, trusted https:// address.

@EvanDotPro
Copy link

(Sorry if anyone subscribed to this thread got an e-mail with an incomplete version of this message before.)

Disclaimer: I am by no means a security expert. I like to think I know the basics, but I welcome (and encourage) those who know better to correct anything I miss or don't get quite right.

This is an issue I've been discussing with the Zend Framework 2 community for several months and is the primary reason I cannot, in good conscience, recommend using Composer+Packagist to my peers yet. I'd really like to see security taken seriously and improved soon if we want Composer+Packagist to be a viable de facto standard in the PHP community. I'll go as far as to say that it worries me that Composer has become so popular without something so obvious being addressed.

In an ideal world, what I'd like to see, instead of securing the transport layer, is proper support for repository-level package signing and/or the ability for users to add authors' public keys to their trusted keys so that they (or Composer) can verify the validity of packages downloaded. We've seen this model for decades in Linux distros and other communities, and for good reason -- it's proven secure and it works. This way you are not tied to only downloading via SSL from trusted domains. It opens the possibility for people to host and use mirrors safely and securely without relying on each mirror to properly purchase and install a valid, properly signed SSL certificate and for users to add the public key of each server or mirror they wish to use. The Linux model is generally one canonical repository, one public key. With the content of the packages signed and verified, you no longer have to worry about the integrity and security of the transport layer. Public FTP? No problem. Insecure HTTP? No problem. Packages are signed so it doesn't matter.

The ideal solution to this is PGP; however PHP doesn't make PGP easy to implement in a cross-platform way. In order to use PGP, we'd have to require they have gpg installed and on their path (a pain on Windows) and build packages into phar/tar/zip/etc, and use detached signature files, as Phar only seems to support OpenSSL built-in signatures (and worse, if OpenSSL is not available, I've heard it silently falls back to a SHA-1 checksum, though I've yet to verify this in php-src). I've posted on php-internals about PGP Phar support, but no one seems to know what happened to that -- there's hints it was meant to get in but never made it. For packages where a Git repository is the source, we would want to, by default, only allow checking out signed tags, which can be verified with a trusted public key. For the sake of usability, you could make exceptions, allowing Composer to check out unsigned tags if they specifically have Github's public key trusted and the source is from https://github.com/, etc.

Of course, this brings with it plenty of other logistic issues like managing the repository's private key -- if you're allowing user-submitted and updated packages, you'll need to automate the signing of they packages. Great care would need to be taken to keep the signing server segregated and secure so the repository's private key does not get compromised. There are folks smarter than myself that could probably offer specific advice regarding that.

Unfortunately, I don't see Composer / Packagist actually implementing package signing; it would be a lot of work and require some redesigning of the Composer client and the Packagist backend, as well as overcoming some logistical issues that may, at first glance, seem insurmountable. So, if we're going to settle for a compromise, then yes, everything needs to default to SSL, and care needs to be taken that the SSL certificate is valid and trusted, as we all know PHP's default is useless in this regard.

and let's be honest.. there are many many ways you can be in trouble if someone can MITM your machine

Correct me if I'm wrong, but isn't DNS a distributed system, and can be (and has been) compromised at levels beyond just your own network; isn't this why DNSSEC exists? I've always operated under the assumption that you can have a "perfectly" secure server and network, yet still get back a compromised DNS response from upstream and that you should always take additional steps to verify who you're actually connecting to if security is at all a concern (and with a package manager, it is very much so).

Anyway, let's get this taken care of, please.

@StormTide
Copy link
Author

and let's be honest.. there are many many ways you can be in trouble if someone can MITM your machine

You can easily MITM anyone on the Internet today. This system is decentralized, and offers thousands of points of attack. Getting in-between any company's developers is often as simple as getting a client on the same WIFI network, running arpspoof and hijacking the IP for getcomposer.org. (Think PHP conference, think firesheep... this isnt complicated stuff)

But more likely, someone will hijack the getcomposer.org dns from some network perspective. Either the entire domain (you're using a high security dns provider right???) or at a big datacenter's dns server. A little client profiling (eg only inject code into clients from a certain network) and its almost undetectable.

Installing unauthenticated packages takes us back a full decade. Theres lots of ways to do authentication, either with TLS setup correctly or with package signing. Silent TOFU wont work and is actually a rather hilarious way to propose solving this problem. (You do verify your host keys when connecting to a ssh server for the first time... riiiight?)

@EvanDotPro
Copy link

I've issued a pull request which attempts to address this issue. It's not my ideal fix (see my above comment), but it's probably the next best solution to fully implementing proper package signing.

EvanDotPro added a commit to EvanDotPro/composer that referenced this issue Sep 10, 2012
- Self-update and packagist are now forced to use SSL.
- This introduces a hard dependency on the openssl extension when using
  self-update or packagist. Users should not be using either without
  utilizing a proper SSL connection anyway.
- Keeps a trusted CA bundle file in the composer home directory.
- Attempts to copy over the operating system CA bundle if available.
- Upon encountering an untrusted certificate, the user is prompted if
  they would like to trust the certificate and the CA in the future. If
  they choose yes, the root CA for that cert is added to the CA bundle
  in the composer home directory.
- Expired certificates generate an extra warning and prompt.
- Invalid common names generate an extra warning and prompt.
@till
Copy link
Contributor

till commented May 25, 2013

Sorry for commenting on an old issue — but I feel like this issue should be addressed. Did anyone make any progress or have any thoughts how to progress?

I personally see how potentially SSL can be a painpoint to the adoption of composer — but then again it could be configurable so people can add CAs if the ones included in code expire. Maybe composer should attempt to use SSL by default and have a configuration option to turn it off?

Maybe packagist should start making everything available over SSL and lead by example?

I wouldn't remove support for http:// entirely though — e.g. on a LAN this may be acceptable. Maybe it should come with a warning though. ;)

On signing packages — that would be great too.

I think the format of the packages.json file could be easily extended to include a signature and it could be checked when available. Kind of how sha1 checksums are currently used. I don't know any cross-platform way to verify this though — it seems pretty straight forward on the Mac, Linux and Unix. But I have no idea about Windows. I'd appreciate pointers though.

@webmozart
Copy link

Why isn't this getting more attention? As I see it, this is a very serious security issue. PHP was known for a ignorance and lack of security once, let's not start this again.

@till
Copy link
Contributor

till commented Jun 17, 2013

@bschussek Want to comment/work on #1941? I need this fixed/improved as well.

@till
Copy link
Contributor

till commented Jun 17, 2013

@odnanref Keep it classy and on topic, please.

@ivan-novakov
Copy link

This issue is really disturbing. The solution might be to make the validation optional, so developers, who are aware of the risk and don't mind doing a little extra work, could use it.

@jameshalsall
Copy link

@ivan-novakov I'd say that is a bad approach. That's how PHP got it's bad name originally, because it's easy to do things in an insecure fashion

@AlekseyKorzun
Copy link

Why are you guys circle jerking around https:// protocol? This is not a fix, if I were to spoof DNS I could obtain a SSL certificate for $8/year and sign it.

This is not a solution.

@dongilbert
Copy link

@AlekseyKorzun It's obviously not a solution, but if you're not going to offer your skills in helping the situation, then it's best to not comment at all, it just adds fuel to the fire. Especially given your choice of words.

@EvanDotPro
Copy link

@AlekseyKorzun that has already been discussed. The underlying problems are known, as well as "correct" solutions such as proper repository / package signing. Please actually read the historical discussions to see how and why the topic of SSL came up if you're truly interested. It's a waste of everyone's time to rehash the same conversations over and over.

Additionally, pointing out what's "not the solution" is in no way a helpful contribution. Please try to be a bit more considerate of people's time, as posting comments in that sort of tone clearly will only serve to stir up old, unproductive arguments and not bring anyone closer to a solution.

@rmccue
Copy link
Contributor

rmccue commented Oct 7, 2013

Hi! @jbroadway mentioned this ticket to me, as I've been working on Requests 1.6 with a whole bunch of work on SSL in it, plus I'm working on package signing and similar issues for WordPress, so hopefully I can contribute here.

In a nutshell, this comes down to two separate (but related) issues:

  • Connection security (handled by SSL)
  • Package verification (handled by something else)

The two issues are often conflated, but they're really separate issues. Connection security is something that has been studied extensively, and TLS/SSL is the solution that everyone uses, for good reason.

Package verification, however, doesn't really have a clear-cut answer. A fair few desktop package managers use PGP, but there's no real easy way to use that in PHP (especially without system calls). If you want something that's guaranteed to be available here, phpseclib is a good solution, but you need to decide how to sign packages. PKCS#1 seems like it could be a good solution here, as it requires both a private and public key. Unlike PGP, however, there's no real "chain of trust", so you'd need to handle that separately. Theoretically, that shouldn't be too hard.

I'd also like to touch on some of the comments made previous.

@Seldaek on SSL certificates:

Doing secure HTTPS in php is unfortunately not as easy as it should be. Basically we'd have to ship our cert pubkey (or at least our CA's) within the sources, which means that at some point the old composer code would stop working if the cert changes.

Every piece of software has to face this reality. Thankfully, the root certificates in a normal CA root file have long expirations (2030 for most of them), and it's reasonable to rely on this, keeping in mind that CAs will start issuing new root certificates much earlier.

@EvanDotPro:

In an ideal world, what I'd like to see, instead of securing the transport layer, is proper support for repository-level package signing

Neither of these is a replacement for each other. Package signing removes some of the need for connection security, but security should be achieved in depth rather than relying on a single feature.

Correct me if I'm wrong, but isn't DNS a distributed system, and can be (and has been) compromised at levels beyond just your own network; isn't this why DNSSEC exists?

DNS issues are related, but not the same as MITM issues. Proxies can hijack requests to specific IPs and leave the client oblivious to the fact, subverting DNSSEC altogether. DNSSEC is about ensuring that DNS itself cannot be hijacked.

@StormTide is completely correct in saying that MITM-ing people is ridiculously easy given the right tools.

So, what I'd recommend is three separate steps:

  • Introduce secure connections with TLS/SSL - and if you're not using cURL, be aware there are problems with TCP over TLS sockets, and be prepared to work around those (or just use Requests and be done with it 😉 )
  • Introduce package signing with a signing algorithm like PKCS#1
  • At the same time, introduce a web-of-trust model, allowing authors to use their own private keys to sign their packages, and signing their keys with a global Composer key

@till
Copy link
Contributor

till commented Oct 8, 2013

For the package signing algorithm: What about a test implementation in satis? We could introduce custom signing there and trial it. Then use a composer plugin (with something like a new POST_FILE_DOWNLOAD event) to verify it?

@padraic
Copy link
Contributor

padraic commented Feb 22, 2014

This still isn't fixed or mitigated... The method of using secure TLS in PHP has been known for quite a while and, despite some early confusion what with PHP's incomplete support for SANs, it actually does work if you configure it correctly. Are we waiting for the end of the Universe? ;)

@webmozart
Copy link

@padraic Do you know how/have time to implement this properly? I would highly appreciate that.

@padraic
Copy link
Contributor

padraic commented Feb 23, 2014

@webmozart I'm doing a bit of testing here and it seems like the typical fix should work. I think what's happened here is that issue has fallen to a few separate problems we should just ignore, e.g. getting a cert bundle. All major linux distros should have a cert bundle locally. Those on Windows can go download one. Problem solved. We can improve usability later after the basic vulnerability is fixed.

@padraic
Copy link
Contributor

padraic commented Feb 23, 2014

Working on a SSL/TLS PR here: #2745

@StormTide
Copy link
Author

🍰 Happy 2nd birthday, composer RCE.

@mikedugan
Copy link

Sorry for the necrobump, but as a not-security-guru, could someone help me understand what the core problem is here and what has prevented it from being fixed after well over 2 years? Conversely, is there a reason this is no longer as much of an issue as it was 2 years ago?

Not trying to ruffle feathers, just want to understand what's going on here.

@padraic
Copy link
Contributor

padraic commented Jan 20, 2015

No excuse for people to protest over having actual working HTTPS ;). If they don't like it, it's their problem to go and solve.

I'm busy on Humbug after a very long break so I never did get around to looking into packag signing in exhausting detail. I had implemented something around public keys but I may swing back to it in time even it's just to throw GPG there and have something, anything.

@padraic
Copy link
Contributor

padraic commented Jan 20, 2015

@mikedugan Basically, how to prove the code you just downloaded was actually the code published by a trusted programmer and not ISIS, North Korea or Darth Vadar. At present, we can't make any such assertion.

HTTPS support secures the transport from point A to point B, however we can't be entirely certain what is sitting on point A. How secure is any given programmer's Github access? The solution is to sign every package, using a private key, which can be verified after download and before the code is allowed to be executed.

The problem in the way is...getting everyone used to the idea of signing packages. Kevin is fairly accurate that PHP, as a community, has little appetite for this. If it did, we'd have a solution knocked out by dozens of programmers collaborating on the code. The second problem is key invalidation - the more effort is needed, the less appetite for it there is. I actually have public key signing implemented for Composer, but I vanished last spring before I finished it and fished it around for opinions. It would have gone some way to alleviating Kevin's concerns, but certainly not all of them (web of trust? revokation? adoption rate? not GPG?).

@till
Copy link
Contributor

till commented Jan 20, 2015

Just adding my 2 cents (again): https is not at all a silver bullet. It doesn't solve the code integrity issue as @padraic pointed out. Besides, I don't think it makes anything that more secure. The challenge is to educate people.

Doing it properly relies on a totally new build process and toolchain. Then it's not as simple as linking to github. And automatically signing an artefact with a key sounds convenient, but moves the issue somewhere else (e.g. composer infrastructure and trust in it as the ultimate source of truth).

Maybe it would be more appropriate to brainstorm that vs. rehashing the obvious, like the fact that someone hasn't done it all for you already. E.g. I would be up pioneering this in satis and then we could move on making packagist safer. I don't think the other way around is that feasible.

@sybarite
Copy link

@StormTide I do agree with you, though composer is being touted as the next best thing after PEAR packages, it really has a lot of issues concerning speed, security etc. Composer still does not have 3rd party repository caching implemented #2929

I mean I know its open source, but that still I think it should hold up to a higher standard as its trying to become an important part of the tool chain for one of the biggest programming communities online. I know @Seldaek is a very talented guy but I don't think this is a one man job. The community will have to rally and get things done too if we need to make composer as highly intuitive as apt-get is right now.

@vimishor
Copy link

@kpande If you can't bring something constructive into the conversation and/or code, you could at least not comment and move along.

@GeeH
Copy link

GeeH commented Jul 7, 2015

You don't have to use Composer.

@TYoung86
Copy link

Hosting a copy of the public key on packagist won't matter, but should be nice to verify from differential routes.

Distribute the trusted public key with composer.

Verify the public key of the remote host is the known, shared, trusted public key (digital signature verification).

The endpoint can either serve files directly or 302 redirect to a trusted CDN.

Validate foreign key certificate chain in standard fashion.

After content is downloaded, verify any downloaded content via a secure hash hosted at the trusted gateway (and only from the trusted gateway, never from a 3rd party or the CDN; if a redirect is received, fail).

So long as the distribution of composer is not compromised, this should mitigate MITM during update.

This should also allow using a CDN to distribute updates to mitigate load. The hashes can be stored on the CDN (for display purposes or bulk-transfer in uncompromised scenarios) and on the gateway, but should only ever be trusted from the gateway. It's also quite a fair bit of work.

Somewhat mitigates bandwidth concerns by allowing a CDN, but will still require generating and hosting hashes of update content.

Then composer just needs to profess they are not compromised, so long as you verify you're talking to them (via public key).

@Ocramius
Copy link
Contributor

Note: PHP 5.6 has decent SSL/TLS. Assuming we drop (fine by me, not by pretty much everyone else) < 5.6 support, we should be ok.

Ref: http://php.net/manual/en/migration56.openssl.php

@greencopper
Copy link

We need to seriously remove composer and NOT have people use it until this problem has been dealt with!

Anything less is a serious neglect and a show of just how ignorant and foolish PHP people are about security.

Sticking our heads deep in the ground while hoping nothing bad will happen is just not okay at all! This issue haven't got any serious attention at all since it was opened. Yet, composer is still available for download.

Please remove composer until the issue has been solved or at least make people aware on the website.

@sadortun
Copy link

sadortun commented Jan 5, 2016

@TYoung86 Maybe you could use https://letsencrypt.org/ to verify the chain of thrust and signing ?

@greencopper
Copy link

Meaning what exactly?
There is nothing about any key signing or anything on https://getcomposer.org/download/

@paragonie-scott
Copy link

As of #4775 Composer's self-update command should include RSA signing. That takes care of the first leg of this problem.

Now to make it possible for developers to publish signatures of releases.

@StormTide
Copy link
Author

Its not really the self-update command thats important here, but rather that theres unsigned code running from http. #4775 solves that for composer itself, but this is a weakest-link-in-the-chain issue, so any http sources that aren't signed/validated still working will keep this bug open. @Seldaek is working on a https-only flag (to be set enabled by default) that should finally close this bug when it goes live. (Though it'll probably break a lot of people who unsafely use http spec'd sources)... the hash checking is being worked on too, but really is only useful for sources that do clean/clear releases. Project will need keys for upgrade-compatible packages going forward, and thats still a ways away.

TL;DR, close this bug when http sources no longer work.

@paragonie-scott
Copy link

TL;DR, close this bug when http sources no longer work.

Makes sense. Then we can focus on allowing developers to publish their own signatures... which might be far more involved than anything else done in this sector (x509 lol).

@StormTide
Copy link
Author

@kpande that would create a sort of DRM scheme in composer and violate the spirit of open source software licensing. IMO, Composer, the project, should really have no influence over the signing of packages. There are well established patterns for using technologies like Keyrings via HKP to create federated, updatable and trusted key stores without central authorities.

@StormTide
Copy link
Author

Debian uses HKP via http://keyring.debian.org/ as a way to bootstrap trust. SecureApt contains the apt-key utility for managing your local keyring, including the ability to add keys of trusted third party signers not approved by the project. It does not have the ability to revoke keys and disable dev's certs in the way you're suggesting (ie as compared to say the Android marketplace which can retract a developer and stop their code running in the ecosystem)...

http://man.he.net/man8/apt-key for reference.

@Seldaek
Copy link
Member

Seldaek commented Feb 11, 2016

Closing as we now do ssl properly, the phars are signed, and the installer's hash is also verifiable as per https://getcomposer.org/download/ and https://composer.github.io/pubkeys.html

@Seldaek Seldaek closed this as completed Feb 11, 2016
@StormTide
Copy link
Author

"Composer runs code from HTTP sources without validating the source of the download or the code downloaded."

While this appears to be resolved for the composer phar itself, I'm not aware the unvalidated HTTP sources was resolved? I know a https-only default mode was being worked on, did it get launched?

As above; "TL;DR, close this bug when http sources no longer work." <-- has this condition been met? If not, this bug should be re-opened.

@Seldaek
Copy link
Member

Seldaek commented Feb 11, 2016

It isn't done yet but it's tracked in #4777 and it is being worked on in #4907 so I think we're good, it's not getting forgotten :)

@StormTide
Copy link
Author

Ok, just so long as no one gets the impression this MITM RCE has been resolved by the closing of this issue. ;)

If you catch this thread somehow, go see #4777 and #4907 and make sure they're completed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests