Fix Packagist Entry To Prevent Redirect #40

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
9 participants
@donatj

donatj commented Jul 22, 2014

Packagists entry for XHProf - see: https://packagist.org/packages/facebook/xhprof still points to https://github.com/facebook/xhprof.git despite having been moved to https://github.com/phacility/xhprof.git

This redirect causes composer to prompt for your github username/password unless you already have a ssh key registered with Github. In our automated environments this has caused havoc and we don't want to have to set up a github account just for our automated testing.

Please correct the packagist entry so it no longer redirects. This will need to be done by logging into Packagist and updating where it points.

I would have registered this in Issues if it was turned on, but its not. As such I made a faux pull request.

billf and others added some commits Apr 22, 2014

@donatj donatj changed the title from Fix Packagist! to Fix Packagist Entry To Prevent Redirect Jul 22, 2014

@JohnMaguire

This comment has been minimized.

Show comment
Hide comment

👍

@epriestley

This comment has been minimized.

Show comment
Hide comment
@epriestley

epriestley Aug 27, 2014

Member

I'm not very familiar with Composer or Packagist. It looks like someone named "lox" owns the Packagist package. I don't know who that is. Is it plausible that he's someone unrelated to this project, who just submitted the package? Can we even fix this?

Member

epriestley commented Aug 27, 2014

I'm not very familiar with Composer or Packagist. It looks like someone named "lox" owns the Packagist package. I don't know who that is. Is it plausible that he's someone unrelated to this project, who just submitted the package? Can we even fix this?

@epriestley

This comment has been minimized.

Show comment
Hide comment
@epriestley

epriestley Aug 27, 2014

Member

(I also didn't have "Watch" set up for this project when it moved, so I missed this originally.)

Member

epriestley commented Aug 27, 2014

(I also didn't have "Watch" set up for this project when it moved, so I missed this originally.)

@JohnMaguire

This comment has been minimized.

Show comment
Hide comment
@JohnMaguire

JohnMaguire Aug 27, 2014

it looks like the user @lox on Github owns the project, and he's active on GH. Not sure if this tag will notify him without watching this project, but perhaps he could facilitate deferring ownership to you?

it looks like the user @lox on Github owns the project, and he's active on GH. Not sure if this tag will notify him without watching this project, but perhaps he could facilitate deferring ownership to you?

@epriestley

This comment has been minimized.

Show comment
Hide comment
@epriestley

epriestley Aug 27, 2014

Member

Just to clarify, the way this works is that you bind to some package on packagist.org that could be owned by literally anyone with no affiliation whatsoever with the project, which they can change at any time? And there's no way to get in touch with them if there's a bug with it, except by guessing who they are on some other service? What problem does this solve?

Specifically, it doesn't seem to solve "keep links up to date when projects move".

Member

epriestley commented Aug 27, 2014

Just to clarify, the way this works is that you bind to some package on packagist.org that could be owned by literally anyone with no affiliation whatsoever with the project, which they can change at any time? And there's no way to get in touch with them if there's a bug with it, except by guessing who they are on some other service? What problem does this solve?

Specifically, it doesn't seem to solve "keep links up to date when projects move".

@JohnMaguire

This comment has been minimized.

Show comment
Hide comment
@JohnMaguire

JohnMaguire Aug 27, 2014

Yeah, I'm not sure why @lox registered the project. Presumably this was
because he needed to use the package in his composer.json and didn't want
to set up the repository URL in it himself. Unfortunately, since he's not a
maintainer of the project it makes it difficult now that things have
changed.
On Aug 27, 2014 12:01 PM, "Evan Priestley" notifications@github.com wrote:

Just to clarify, the way this works is that you bind to some package on
packagist.org that could be owned by literally anyone with no affiliation
whatsoever with the project, which they can change at any time? And there's
no way to get in touch with them if there's a bug with it, except by
guessing who they are on some other service? What problem does this solve?

Specifically, it doesn't seem to solve "keep links up to date when
projects move".


Reply to this email directly or view it on GitHub
#40 (comment).

Yeah, I'm not sure why @lox registered the project. Presumably this was
because he needed to use the package in his composer.json and didn't want
to set up the repository URL in it himself. Unfortunately, since he's not a
maintainer of the project it makes it difficult now that things have
changed.
On Aug 27, 2014 12:01 PM, "Evan Priestley" notifications@github.com wrote:

Just to clarify, the way this works is that you bind to some package on
packagist.org that could be owned by literally anyone with no affiliation
whatsoever with the project, which they can change at any time? And there's
no way to get in touch with them if there's a bug with it, except by
guessing who they are on some other service? What problem does this solve?

Specifically, it doesn't seem to solve "keep links up to date when
projects move".


Reply to this email directly or view it on GitHub
#40 (comment).

@JohnMaguire

This comment has been minimized.

Show comment
Hide comment
@JohnMaguire

JohnMaguire Aug 27, 2014

Packagist is how composer finds repositories though, and when done
correctly is very convenient.
On Aug 27, 2014 12:17 PM, "John Maguire" john@johnmaguire.me wrote:

Yeah, I'm not sure why @lox registered the project. Presumably this was
because he needed to use the package in his composer.json and didn't want
to set up the repository URL in it himself. Unfortunately, since he's not a
maintainer of the project it makes it difficult now that things have
changed.
On Aug 27, 2014 12:01 PM, "Evan Priestley" notifications@github.com
wrote:

Just to clarify, the way this works is that you bind to some package on
packagist.org that could be owned by literally anyone with no
affiliation whatsoever with the project, which they can change at any time?
And there's no way to get in touch with them if there's a bug with it,
except by guessing who they are on some other service? What problem does
this solve?

Specifically, it doesn't seem to solve "keep links up to date when
projects move".


Reply to this email directly or view it on GitHub
#40 (comment).

Packagist is how composer finds repositories though, and when done
correctly is very convenient.
On Aug 27, 2014 12:17 PM, "John Maguire" john@johnmaguire.me wrote:

Yeah, I'm not sure why @lox registered the project. Presumably this was
because he needed to use the package in his composer.json and didn't want
to set up the repository URL in it himself. Unfortunately, since he's not a
maintainer of the project it makes it difficult now that things have
changed.
On Aug 27, 2014 12:01 PM, "Evan Priestley" notifications@github.com
wrote:

Just to clarify, the way this works is that you bind to some package on
packagist.org that could be owned by literally anyone with no
affiliation whatsoever with the project, which they can change at any time?
And there's no way to get in touch with them if there's a bug with it,
except by guessing who they are on some other service? What problem does
this solve?

Specifically, it doesn't seem to solve "keep links up to date when
projects move".


Reply to this email directly or view it on GitHub
#40 (comment).

@JohnMaguire

This comment has been minimized.

Show comment
Hide comment
@JohnMaguire

JohnMaguire Aug 27, 2014

Also, I'm not aware of this project's history, but if it's no longer owned/maintained by Facebook, it may be worth just creating a new package on Packagist, called phacility/xhprof, and updating the composer.json of this project to reflect that change.

Also, I'm not aware of this project's history, but if it's no longer owned/maintained by Facebook, it may be worth just creating a new package on Packagist, called phacility/xhprof, and updating the composer.json of this project to reflect that change.

@epriestley

This comment has been minimized.

Show comment
Hide comment
@epriestley

epriestley Aug 27, 2014

Member

I don't doubt that it's convenient, but it seems like this is sacrificing everything else (e.g., security) for convenience. You're basically trusting some some completely random guy to not run arbitrary code on your system, right? It doesn't look like packagist.org even has a change history.

Is there anything which prevents lox from changing the canonical URL to https://github.com/evilloxlolpwnd/xhprof.git and compromising everyone using the package?

I'm asking about this in #composer-dev, but if this system works like I think it does it seems like it is irresponsibly choosing convenience over security and my inclination is to remove Composer support.

Member

epriestley commented Aug 27, 2014

I don't doubt that it's convenient, but it seems like this is sacrificing everything else (e.g., security) for convenience. You're basically trusting some some completely random guy to not run arbitrary code on your system, right? It doesn't look like packagist.org even has a change history.

Is there anything which prevents lox from changing the canonical URL to https://github.com/evilloxlolpwnd/xhprof.git and compromising everyone using the package?

I'm asking about this in #composer-dev, but if this system works like I think it does it seems like it is irresponsibly choosing convenience over security and my inclination is to remove Composer support.

@epriestley

This comment has been minimized.

Show comment
Hide comment
@epriestley

epriestley Aug 27, 2014

Member

just creating a new package on Packagist, called phacility/xhprof, and updating the composer.json

Won't that break everyone using "facebook/xhprof"?

Member

epriestley commented Aug 27, 2014

just creating a new package on Packagist, called phacility/xhprof, and updating the composer.json

Won't that break everyone using "facebook/xhprof"?

@donatj

This comment has been minimized.

Show comment
Hide comment
@donatj

donatj Aug 27, 2014

I agree, you should see about fixing facebook/xhprof before taking such a drastic step.

donatj commented Aug 27, 2014

I agree, you should see about fixing facebook/xhprof before taking such a drastic step.

@donatj

This comment has been minimized.

Show comment
Hide comment
@donatj

donatj Aug 27, 2014

I went and looked at @lox's git information, he has the email address as lachlan@ljd.cc Might be worth somebody shooting him an email.

donatj commented Aug 27, 2014

I went and looked at @lox's git information, he has the email address as lachlan@ljd.cc Might be worth somebody shooting him an email.

@epriestley

This comment has been minimized.

Show comment
Hide comment
@epriestley

epriestley Aug 27, 2014

Member

https://secure.phabricator.com/D10365 removes composer.json, since the package is broken, until we make a decision about the future of Composer support. We may support it under a new name (or possibly under the same name, if we can gain control of the package), but we need to have a much deeper understanding of Composer before we do.

The fact that Composer has this behavior is totally shocking to me.

Member

epriestley commented Aug 27, 2014

https://secure.phabricator.com/D10365 removes composer.json, since the package is broken, until we make a decision about the future of Composer support. We may support it under a new name (or possibly under the same name, if we can gain control of the package), but we need to have a much deeper understanding of Composer before we do.

The fact that Composer has this behavior is totally shocking to me.

@epriestley

This comment has been minimized.

Show comment
Hide comment
@epriestley

epriestley Aug 27, 2014

Member

Broadly, it seems that you've chosen to bind to a name owned by an anonymous third party that we have no control over, on a site with no checks for authenticity. In the general case, we can't fix this, and even if we get in touch with lox the scheme makes me very wary. You should contact packagist.org, attempt to contact lox, or file a bug against Composer to improve this (for example, require package owners to add a key to composer.json to confirm the information on packagist.org, similar to how Google Apps for Domains requires you to add a TXT DNS record to confirm you own a domain).

Member

epriestley commented Aug 27, 2014

Broadly, it seems that you've chosen to bind to a name owned by an anonymous third party that we have no control over, on a site with no checks for authenticity. In the general case, we can't fix this, and even if we get in touch with lox the scheme makes me very wary. You should contact packagist.org, attempt to contact lox, or file a bug against Composer to improve this (for example, require package owners to add a key to composer.json to confirm the information on packagist.org, similar to how Google Apps for Domains requires you to add a TXT DNS record to confirm you own a domain).

@donatj

This comment has been minimized.

Show comment
Hide comment
@donatj

donatj Aug 27, 2014

You're basically trusting some some completely random guy to not run arbitrary code on your system, right? It doesn't look like packagist.org even has a change history.

Basically what packagist is is a connection of a key, eg: facebook/xhprof to a public git/svn repo. Thats it, dead simple.

You're trusting the repo and you're trusting entry. Just like anything else on the internet you're agreeing to some level of trust every time you use a third party package.

Is there anything which prevents lox from changing the canonical URL to https://github.com/evilloxlolpwnd/xhprof.git and compromising everyone using the package?

Composer will lock in to a specific sha-1 hash though so you can be guaranteed the exact same version you've in theory reviewed when installing on other machines. It won't install if it doesn't match the recorded sha1.

File a bug against Composer to improve this (for example, require package owners to add a key to composer.json to confirm the information on packagist.org, similar to how Google Apps for Domains requires you to add a TXT DNS record to confirm you own a domain).

Well in my experience is generally after you set the key in your composer.json its usually moments until you register it on packagist, so hijacking's never been a big deal in the community at large.

That said, they probably should have some sort of optional mechanism for larger projects worried about this sort of thing..

In the general case, we can't fix this, and even if we get in touch with lox the scheme makes me very wary.

I believe @lox could add you as a maintainer of the package, and then you could remove him, essentially changing the owner of the packagist entry. A little bit of a dance, for sure, but probably worth it in the long run.

Until we make a decision about the future of Composer support

Composer support is very important to the PHP community at large, and this really should be straightened out. In this day and age not having composer support on a package is like not distributing Ruby code as a Gem. Its just not done.

Lastly, I'm curious the back story about why this is no longer under Facebook? Have they abandoned xhprof? How did you become the owner?

donatj commented Aug 27, 2014

You're basically trusting some some completely random guy to not run arbitrary code on your system, right? It doesn't look like packagist.org even has a change history.

Basically what packagist is is a connection of a key, eg: facebook/xhprof to a public git/svn repo. Thats it, dead simple.

You're trusting the repo and you're trusting entry. Just like anything else on the internet you're agreeing to some level of trust every time you use a third party package.

Is there anything which prevents lox from changing the canonical URL to https://github.com/evilloxlolpwnd/xhprof.git and compromising everyone using the package?

Composer will lock in to a specific sha-1 hash though so you can be guaranteed the exact same version you've in theory reviewed when installing on other machines. It won't install if it doesn't match the recorded sha1.

File a bug against Composer to improve this (for example, require package owners to add a key to composer.json to confirm the information on packagist.org, similar to how Google Apps for Domains requires you to add a TXT DNS record to confirm you own a domain).

Well in my experience is generally after you set the key in your composer.json its usually moments until you register it on packagist, so hijacking's never been a big deal in the community at large.

That said, they probably should have some sort of optional mechanism for larger projects worried about this sort of thing..

In the general case, we can't fix this, and even if we get in touch with lox the scheme makes me very wary.

I believe @lox could add you as a maintainer of the package, and then you could remove him, essentially changing the owner of the packagist entry. A little bit of a dance, for sure, but probably worth it in the long run.

Until we make a decision about the future of Composer support

Composer support is very important to the PHP community at large, and this really should be straightened out. In this day and age not having composer support on a package is like not distributing Ruby code as a Gem. Its just not done.

Lastly, I'm curious the back story about why this is no longer under Facebook? Have they abandoned xhprof? How did you become the owner?

@epriestley

This comment has been minimized.

Show comment
Hide comment
@epriestley

epriestley Aug 27, 2014

Member

It won't install if it doesn't match the recorded sha1.

Does it actually stop you, or just give you a prompt like "This has been updated, do you want to continue? [Y/n]"

Well in my experience is generally after you set the key in your composer.json its usually moments until you register it on packagist, so hijacking's never been a big deal in the community at large.

That didn't happen here, and I don't think anyone has ever mentioned packagist.org on any of the pull requests to add composer.json that I've received in other projects -- and particularly not in terms of a warning like YOU ABSOLUTELY MUST GO REGISTER A packagist.org PACKAGE BEFORE YOU PULL THIS OR ANYONE CAN TAKE IT OVER AT ANY TIME.

I suspect I could search GitHub for unregistered packages in composer.json files (presumably, there are thousands from interactions like the original request here and requests I've received in other projects), register them all, then point them at evil code and compromise a bunch of machines? Is there any real reason to believe that attack wouldn't be successful in compromising at least some machines, with very little effort and no accountability?

Composer support is very important to the PHP community at large, and this really should be straightened out.

I clearly don't currently have enough expertise with Composer to administrate it safely, given that I was unaware of this. You or anyone else is free to fork the project and add a composer.json if you're confident that your usage is safe or willing to accept the security risk in exchange for convenience.

You could also look at https://github.com/FriendsOfPHP/uprofiler, which is a fork of this profiler with Composer support.

I'm not currently comfortable with the convenience/security tradeoff here.

Lastly, I'm curious the back story about why this is no longer under Facebook? Have they abandoned xhprof? How did you become the owner?

We (Phacility) used to work at Facebook with the original developers/maintainers of XHProf. We left a couple of years ago and started a new company.

Facebook is now focused on the HHVM platform, and HHVM has a built-in profiler which is similar to XHProf but doesn't share code with it. For example, you can see in #7 (comment) that the most active FB maintainer didn't have Zend PHP installed at the time, as far back as 2012. However, we use XHProf heavily in Phabricator.

Facebook recently pruned the list of things it maintains on GitHub, moving some old projects to https://github.com/facebookarchive, which spun our projects off to /phacility/. We offered to take over XHProf at the time, since we rely on it.

Member

epriestley commented Aug 27, 2014

It won't install if it doesn't match the recorded sha1.

Does it actually stop you, or just give you a prompt like "This has been updated, do you want to continue? [Y/n]"

Well in my experience is generally after you set the key in your composer.json its usually moments until you register it on packagist, so hijacking's never been a big deal in the community at large.

That didn't happen here, and I don't think anyone has ever mentioned packagist.org on any of the pull requests to add composer.json that I've received in other projects -- and particularly not in terms of a warning like YOU ABSOLUTELY MUST GO REGISTER A packagist.org PACKAGE BEFORE YOU PULL THIS OR ANYONE CAN TAKE IT OVER AT ANY TIME.

I suspect I could search GitHub for unregistered packages in composer.json files (presumably, there are thousands from interactions like the original request here and requests I've received in other projects), register them all, then point them at evil code and compromise a bunch of machines? Is there any real reason to believe that attack wouldn't be successful in compromising at least some machines, with very little effort and no accountability?

Composer support is very important to the PHP community at large, and this really should be straightened out.

I clearly don't currently have enough expertise with Composer to administrate it safely, given that I was unaware of this. You or anyone else is free to fork the project and add a composer.json if you're confident that your usage is safe or willing to accept the security risk in exchange for convenience.

You could also look at https://github.com/FriendsOfPHP/uprofiler, which is a fork of this profiler with Composer support.

I'm not currently comfortable with the convenience/security tradeoff here.

Lastly, I'm curious the back story about why this is no longer under Facebook? Have they abandoned xhprof? How did you become the owner?

We (Phacility) used to work at Facebook with the original developers/maintainers of XHProf. We left a couple of years ago and started a new company.

Facebook is now focused on the HHVM platform, and HHVM has a built-in profiler which is similar to XHProf but doesn't share code with it. For example, you can see in #7 (comment) that the most active FB maintainer didn't have Zend PHP installed at the time, as far back as 2012. However, we use XHProf heavily in Phabricator.

Facebook recently pruned the list of things it maintains on GitHub, moving some old projects to https://github.com/facebookarchive, which spun our projects off to /phacility/. We offered to take over XHProf at the time, since we rely on it.

@donatj

This comment has been minimized.

Show comment
Hide comment
@donatj

donatj Aug 27, 2014

Does it actually stop you, or just give you a prompt like "This has been updated, do you want to continue? [Y/n]"

It just fails. Gives you a message about how the required commit could not be found.

Everything else: noted.

donatj commented Aug 27, 2014

Does it actually stop you, or just give you a prompt like "This has been updated, do you want to continue? [Y/n]"

It just fails. Gives you a message about how the required commit could not be found.

Everything else: noted.

epriestley added a commit that referenced this pull request Aug 27, 2014

Remove Composer support from XHProf
Summary:
See <#40>. This is currently broken and extremely insecure.

We may want to restore it eventually, but understanding composer is very complex (no one who touched this realized that the package was owned by someone unrelated to the project who can apparently redirect it at will with no accountability). No one on the ticket seems to have any reason why this isn't totally wide open, and I haven't gotten in touch with anyone in `#composer-dev`.

Test Plan: N/A

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D10365
@JohnMaguire

This comment has been minimized.

Show comment
Hide comment
@JohnMaguire

JohnMaguire Aug 27, 2014

It's worth noting that packagist is not the only way a package can be used with composer. You can define a specific repository's URL within composer.json, and this is what users should be doing if they need to use a package that is not already registered with packagist.

Not all users know this however and they may register the package on packagist simply because they can't figure out how to get it to work. As such, composer itself is not a security risk. I can see why you would say "relying on packagist is a security risk," though I disagree that it's any more of a security risk than trusting the maintainers of 's repositories.

You should be checking the versions and locations from which you are pulling dependencies - regardless of from where you pull them. You can add as many additional security layers or make it as convenient as possible for users, but in the end it's up to the user to use software correctly.

I disagree that removing composer support is the right way to go with this. While you are denying users the ability to use your project through packagist, you are also denying them the ability to explicitly define your repository in their composer.json file and have it take care of any dependencies, etc. your project may require.

Furthermore, like many package managers, composer.json generates a lock file when it updates to the latest version of a specified criteria. At this point, the SHA1 for that package is "locked in" until you explicitly tell composer to update it. If the repository URL were to change, there is no issue - either you get the exact same commit (no tampering), or the commit doesn't exist in the repository (red flag.)

Adding a "code" to the composer.json like Google does with TXT DNS records does not help. With Google, the process looks like this:

  1. A specific domain is already under control.
  2. Google would like to offer services to the domain owner.
  3. Google asks you to add a DNS record, assuming only the domain owner has control of the DNS.
  4. Google checks explicitly the single domain, searching for the TXT record.

If you were the maintainer in packagist, you would get your TXT record to add, and then its public. A repo change would be easy because the TXT repo is publicly viewable.

I suppose it would add an extra step, forcing the packagist maintainer to explain to the package maintainer what they're doing, and possibly causing the package maintainer to take a step back and learn about the software they are using, but any package maintainer should be looking into the pull requests being submitted to their project and what they're doing anyway. If someone submits a pull request for a composer.json, it's the repository maintainer's duty to understand what that means.

It's worth noting that packagist is not the only way a package can be used with composer. You can define a specific repository's URL within composer.json, and this is what users should be doing if they need to use a package that is not already registered with packagist.

Not all users know this however and they may register the package on packagist simply because they can't figure out how to get it to work. As such, composer itself is not a security risk. I can see why you would say "relying on packagist is a security risk," though I disagree that it's any more of a security risk than trusting the maintainers of 's repositories.

You should be checking the versions and locations from which you are pulling dependencies - regardless of from where you pull them. You can add as many additional security layers or make it as convenient as possible for users, but in the end it's up to the user to use software correctly.

I disagree that removing composer support is the right way to go with this. While you are denying users the ability to use your project through packagist, you are also denying them the ability to explicitly define your repository in their composer.json file and have it take care of any dependencies, etc. your project may require.

Furthermore, like many package managers, composer.json generates a lock file when it updates to the latest version of a specified criteria. At this point, the SHA1 for that package is "locked in" until you explicitly tell composer to update it. If the repository URL were to change, there is no issue - either you get the exact same commit (no tampering), or the commit doesn't exist in the repository (red flag.)

Adding a "code" to the composer.json like Google does with TXT DNS records does not help. With Google, the process looks like this:

  1. A specific domain is already under control.
  2. Google would like to offer services to the domain owner.
  3. Google asks you to add a DNS record, assuming only the domain owner has control of the DNS.
  4. Google checks explicitly the single domain, searching for the TXT record.

If you were the maintainer in packagist, you would get your TXT record to add, and then its public. A repo change would be easy because the TXT repo is publicly viewable.

I suppose it would add an extra step, forcing the packagist maintainer to explain to the package maintainer what they're doing, and possibly causing the package maintainer to take a step back and learn about the software they are using, but any package maintainer should be looking into the pull requests being submitted to their project and what they're doing anyway. If someone submits a pull request for a composer.json, it's the repository maintainer's duty to understand what that means.

@epriestley

This comment has been minimized.

Show comment
Hide comment
@epriestley

epriestley Aug 27, 2014

Member

If I run composer require facebook/xhprof, or similar, isn't the default behavior to install software which might have nothing to do with this repository, without performing any checks or prompting the user at all?

If users are well-aware that Packagist packages may be owned by random nobodies who aren't related to upstreams, why is this bug filed against the upstream instead of against the Packagist maintainer?

If the Composer/Packagist relationship is obvious, why did it elude everyone who has touched composer.json in this project, and not get mentioned on any other pull requests I've seen in other projects?

My guess is that in theory all of this stuff is well-separated and everyone understands and accepts the risks, but in practice many users have a mental 1:1 equivalency between packagist packages and GitHub repositories and do not understand the relationship deeply.

I get that Composer is super cool and really easy and everyone loves it and that all of these behaviors are fine in practice and no one cares and everyone just wants an easy command that works. For now, I encourage you to fork this project and add a composer.json, which should solve your problem.

We may support Composer in time, but the current behavior is broken and I'm not comfortable just dropping in some new file that points at a different project name without fully understanding the security implications and behavior of Composer.

XHProf does not have dependencies and is unlikely to ever have any, so the utility of Composer within the project is negligible.

Member

epriestley commented Aug 27, 2014

If I run composer require facebook/xhprof, or similar, isn't the default behavior to install software which might have nothing to do with this repository, without performing any checks or prompting the user at all?

If users are well-aware that Packagist packages may be owned by random nobodies who aren't related to upstreams, why is this bug filed against the upstream instead of against the Packagist maintainer?

If the Composer/Packagist relationship is obvious, why did it elude everyone who has touched composer.json in this project, and not get mentioned on any other pull requests I've seen in other projects?

My guess is that in theory all of this stuff is well-separated and everyone understands and accepts the risks, but in practice many users have a mental 1:1 equivalency between packagist packages and GitHub repositories and do not understand the relationship deeply.

I get that Composer is super cool and really easy and everyone loves it and that all of these behaviors are fine in practice and no one cares and everyone just wants an easy command that works. For now, I encourage you to fork this project and add a composer.json, which should solve your problem.

We may support Composer in time, but the current behavior is broken and I'm not comfortable just dropping in some new file that points at a different project name without fully understanding the security implications and behavior of Composer.

XHProf does not have dependencies and is unlikely to ever have any, so the utility of Composer within the project is negligible.

@JohnMaguire

This comment has been minimized.

Show comment
Hide comment
@JohnMaguire

JohnMaguire Aug 27, 2014

If I run composer require facebook/xhprof, or similar, isn't the default behavior to install software which might have nothing to do with this repository, without performing any checks or prompting the user at all?

That is correct. Just as running sudo apt-get install alsa makes no guarantees that you will get software that has anything to do with a given repository. It's up to the package maintainer.

If users are well-aware that Packagist packages may be owned by random nobodies who aren't related to upstreams, why is this bug filed against the upstream instead of against the Packagist maintainer?

Because I believe the idea of this bug, opened by @donatj was that we could get @lox to move the package in Packagist under your control, thereby ensuring future breakages don't occur.

If the Composer/Packagist relationship is obvious, why did it elude everyone who has touched composer.json in this project, and not get mentioned on any other pull requests I've seen in other projects?

Likely because they assumed (perhaps incorrectly) that the repository maintainer would look into what composer.json was doing, what Composer was, and how it worked. As I said, a package on Packagist is not a requirement for composer to work. It does make composer-compliant packages much easier to include and use however.

My guess is that in theory all of this stuff is well-separated and everyone understands and accepts the risks, but in practice many users have a mental 1:1 equivalency between packagist packages and GitHub repositories and do not understand the relationship deeply.

And this will always be the case in any project. Many users believe they understand how a computer works, unaware that their monitor is not their computer.

I get that Composer is super cool and really easy and everyone loves it and that all of these behaviors are fine in practice and no one cares and everyone just wants an easy command that works. For now, I encourage you to fork this project and add a composer.json, which should solve your problem.

By forking the project and adding a composer.json I will have only made the situation worse. There will now be facebook/xhprof which has been redirected here, phacility/xhprof which is the now-maintained repository, and johnmaguire/xhprof which is just merging in upstream changes as I come to them (Will I disappear before the project does? Will I notice upstream changes?) I'm yet another unofficial source which actually makes the whole thing less secure than your hypothetical situation. Now a) someone could change Packagist, and on top of that b) someone could think my repository is the main repository and I could poison the code.

We may support Composer in time, but the current behavior is broken and I'm not comfortable just dropping in some new file that points at a different project name without fully understanding the security implications and behavior of Composer.

The current behavior is not broken. Misunderstood perhaps. You're correct that dropping in a composer.json file without understanding the behavior of Composer or what it is is definitely a mistake. I'm surprised that it was allowed into this project given your concerns with it in the first place. Did you not know what the file was and accept it anyway?

XHProf does not have dependencies and is unlikely to ever have any, so the utility of Composer within the project is negligible.

This may be accurate, but it still prevents other projects from cleanly including XHProf into their project using the de facto PHP standard of Composer.

I think you're really jumping the gun on a non-issue here.

If I run composer require facebook/xhprof, or similar, isn't the default behavior to install software which might have nothing to do with this repository, without performing any checks or prompting the user at all?

That is correct. Just as running sudo apt-get install alsa makes no guarantees that you will get software that has anything to do with a given repository. It's up to the package maintainer.

If users are well-aware that Packagist packages may be owned by random nobodies who aren't related to upstreams, why is this bug filed against the upstream instead of against the Packagist maintainer?

Because I believe the idea of this bug, opened by @donatj was that we could get @lox to move the package in Packagist under your control, thereby ensuring future breakages don't occur.

If the Composer/Packagist relationship is obvious, why did it elude everyone who has touched composer.json in this project, and not get mentioned on any other pull requests I've seen in other projects?

Likely because they assumed (perhaps incorrectly) that the repository maintainer would look into what composer.json was doing, what Composer was, and how it worked. As I said, a package on Packagist is not a requirement for composer to work. It does make composer-compliant packages much easier to include and use however.

My guess is that in theory all of this stuff is well-separated and everyone understands and accepts the risks, but in practice many users have a mental 1:1 equivalency between packagist packages and GitHub repositories and do not understand the relationship deeply.

And this will always be the case in any project. Many users believe they understand how a computer works, unaware that their monitor is not their computer.

I get that Composer is super cool and really easy and everyone loves it and that all of these behaviors are fine in practice and no one cares and everyone just wants an easy command that works. For now, I encourage you to fork this project and add a composer.json, which should solve your problem.

By forking the project and adding a composer.json I will have only made the situation worse. There will now be facebook/xhprof which has been redirected here, phacility/xhprof which is the now-maintained repository, and johnmaguire/xhprof which is just merging in upstream changes as I come to them (Will I disappear before the project does? Will I notice upstream changes?) I'm yet another unofficial source which actually makes the whole thing less secure than your hypothetical situation. Now a) someone could change Packagist, and on top of that b) someone could think my repository is the main repository and I could poison the code.

We may support Composer in time, but the current behavior is broken and I'm not comfortable just dropping in some new file that points at a different project name without fully understanding the security implications and behavior of Composer.

The current behavior is not broken. Misunderstood perhaps. You're correct that dropping in a composer.json file without understanding the behavior of Composer or what it is is definitely a mistake. I'm surprised that it was allowed into this project given your concerns with it in the first place. Did you not know what the file was and accept it anyway?

XHProf does not have dependencies and is unlikely to ever have any, so the utility of Composer within the project is negligible.

This may be accurate, but it still prevents other projects from cleanly including XHProf into their project using the de facto PHP standard of Composer.

I think you're really jumping the gun on a non-issue here.

@epriestley

This comment has been minimized.

Show comment
Hide comment
@epriestley

epriestley Aug 27, 2014

Member

Did you not know what the file was and accept it anyway?

I did not merge composer.json into this project. I have rejected requests to merge it into other projects I maintain in the past, for similar reasons to the reasons mentioned in this ticket (I do have a sufficient understanding of Composer to be confident that the support is correct), although I wasn't even aware of the whole Packagist issue. For example, see:

phacility/libphutil#18
https://github.com/phacility/arcanist/issues/150

Member

epriestley commented Aug 27, 2014

Did you not know what the file was and accept it anyway?

I did not merge composer.json into this project. I have rejected requests to merge it into other projects I maintain in the past, for similar reasons to the reasons mentioned in this ticket (I do have a sufficient understanding of Composer to be confident that the support is correct), although I wasn't even aware of the whole Packagist issue. For example, see:

phacility/libphutil#18
https://github.com/phacility/arcanist/issues/150

@JohnMaguire

This comment has been minimized.

Show comment
Hide comment
@JohnMaguire

JohnMaguire Aug 27, 2014

I should inform you that regardless of what you feel is the perfect line of convenience to security (and yours seems to be further to security and much further from convenience than the majority of PHP developers in terms of dep. management), by removing composer.json you may have broken the package for all current projects depending on it.

I should inform you that regardless of what you feel is the perfect line of convenience to security (and yours seems to be further to security and much further from convenience than the majority of PHP developers in terms of dep. management), by removing composer.json you may have broken the package for all current projects depending on it.

@epriestley

This comment has been minimized.

Show comment
Hide comment
@epriestley

epriestley Aug 27, 2014

Member

Didn't facebook/xhprof being broken already break the package for everyone depending on it?

Member

epriestley commented Aug 27, 2014

Didn't facebook/xhprof being broken already break the package for everyone depending on it?

@JohnMaguire

This comment has been minimized.

Show comment
Hide comment
@JohnMaguire

JohnMaguire Aug 27, 2014

No. facebook/xhprof redirects to this repository, so this repository was still being included. The issue however was that Github was asking for authentication credentials because of the redirect. So for many users who already have a private key setup with Github, nobody noticed. For one of our interns at my workplace, he ran into an issue when trying to install our dependencies. The workaround was to add his public key into Github. I came to this issue in order to try to help remove the redirect and therefore allow public clones through composer.

No. facebook/xhprof redirects to this repository, so this repository was still being included. The issue however was that Github was asking for authentication credentials because of the redirect. So for many users who already have a private key setup with Github, nobody noticed. For one of our interns at my workplace, he ran into an issue when trying to install our dependencies. The workaround was to add his public key into Github. I came to this issue in order to try to help remove the redirect and therefore allow public clones through composer.

@JohnMaguire

This comment has been minimized.

Show comment
Hide comment
@JohnMaguire

JohnMaguire Aug 27, 2014

As the top pull request says:

This redirect causes composer to prompt for your github username/password unless you already have a ssh key registered with Github. In our automated environments this has caused havoc and we don't want to have to set up a github account just for our automated testing.

Please correct the packagist entry so it no longer redirects. This will need to be done by logging into Packagist and updating where it points.

As the top pull request says:

This redirect causes composer to prompt for your github username/password unless you already have a ssh key registered with Github. In our automated environments this has caused havoc and we don't want to have to set up a github account just for our automated testing.

Please correct the packagist entry so it no longer redirects. This will need to be done by logging into Packagist and updating where it points.

@epriestley

This comment has been minimized.

Show comment
Hide comment
@epriestley

epriestley Aug 27, 2014

Member

Regardless of where we go in the future, I'm not going to restore this under facebook/xhprof without controlling that name on Packagist.

Member

epriestley commented Aug 27, 2014

Regardless of where we go in the future, I'm not going to restore this under facebook/xhprof without controlling that name on Packagist.

@donatj

This comment has been minimized.

Show comment
Hide comment
@donatj

donatj Aug 27, 2014

In hindsight I think a new phacility/xhprof is probably the best option if the person with facebook/xhprof isn't in anyway actually associated with facebook.

donatj commented Aug 27, 2014

In hindsight I think a new phacility/xhprof is probably the best option if the person with facebook/xhprof isn't in anyway actually associated with facebook.

epriestley referenced this pull request Aug 28, 2014

Update .arcconfig and acknowledge that we're the new maintainers in X…
…HProf

Summary: Modernize this stuff a bit.

Test Plan: Ran `arc diff` to produce this diff. Read `CREDITS`.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D10366
@pierrejoye

This comment has been minimized.

Show comment
Hide comment
@pierrejoye

pierrejoye Aug 28, 2014

Also there are two things:

  • the extension, which should rely on pickle
  • the php code, example reporting etc., which should rely on composer and use another repo if you plan to improve it, long term

To achieve that I would move this repo out of the facebook org, create your own and create two repositories there. That will also ease development and independent releases while having full composer comparability.

Also there are two things:

  • the extension, which should rely on pickle
  • the php code, example reporting etc., which should rely on composer and use another repo if you plan to improve it, long term

To achieve that I would move this repo out of the facebook org, create your own and create two repositories there. That will also ease development and independent releases while having full composer comparability.

@lox

This comment has been minimized.

Show comment
Hide comment
@lox

lox Aug 29, 2014

Not sure how I missed the notifications for this thread. Just catching up.

Quite a while ago I created the packagist package after Facebook was unresponsive around adding Composer support. Yes, it's pretty terrible that Packagist allows anyone to create an package with an organization prefix other than their username. It would be prudent for them to require some sort of verification from a user before doing this, e.g membership of a github organization or something similar. I'm sure @Seldaek has some thoughts on this, but is probably busy with the myriad of more pressing issues he's dealing with.

The packagist maintainers will make changes to package ownership if you email them, I'm more than happy to hand this over to whomever wants it. Let me know how you would like me to proceed.

lox commented Aug 29, 2014

Not sure how I missed the notifications for this thread. Just catching up.

Quite a while ago I created the packagist package after Facebook was unresponsive around adding Composer support. Yes, it's pretty terrible that Packagist allows anyone to create an package with an organization prefix other than their username. It would be prudent for them to require some sort of verification from a user before doing this, e.g membership of a github organization or something similar. I'm sure @Seldaek has some thoughts on this, but is probably busy with the myriad of more pressing issues he's dealing with.

The packagist maintainers will make changes to package ownership if you email them, I'm more than happy to hand this over to whomever wants it. Let me know how you would like me to proceed.

@JohnMaguire

This comment has been minimized.

Show comment
Hide comment
@JohnMaguire

JohnMaguire Aug 29, 2014

Thanks for getting in touch @lox. I'd really like to see xhprof back on packagist.

Thanks for getting in touch @lox. I'd really like to see xhprof back on packagist.

@lox

This comment has been minimized.

Show comment
Hide comment
@lox

lox Aug 29, 2014

I just noticed I can now mark the package as abandoned, with a suggested replacement. @epriestley if you wanted to merge a composer.json and add a packagist project I could reference your project. Failing that, I'm sure someone will create one anyway that references a fork ;) Hooray for open-source!

lox commented Aug 29, 2014

I just noticed I can now mark the package as abandoned, with a suggested replacement. @epriestley if you wanted to merge a composer.json and add a packagist project I could reference your project. Failing that, I'm sure someone will create one anyway that references a fork ;) Hooray for open-source!

lox added a commit to lox/xhprof that referenced this pull request Aug 29, 2014

Revert "Remove Composer support from XHProf"
See discussion here phacility#40

This reverts commit e3df210.
@lox

This comment has been minimized.

Show comment
Hide comment
@lox

lox Aug 29, 2014

In the meantime, I have forked, reverted e3df210 and "fixed" the packagist package such that it points to my fork. I'd love to remove these, but I figure with 85k odd downloads it's worth not breaking other peoples code.

lox commented Aug 29, 2014

In the meantime, I have forked, reverted e3df210 and "fixed" the packagist package such that it points to my fork. I'd love to remove these, but I figure with 85k odd downloads it's worth not breaking other peoples code.

@Deltachaos

This comment has been minimized.

Show comment
Hide comment
@Deltachaos

Deltachaos Aug 29, 2014

@epriestley it would be grate it you could just get in touch with @lox to solve this problem in upstream. I was shocked that the composer.json was removed from the project and it had cost us some time to handle with this in our project.

@epriestley it would be grate it you could just get in touch with @lox to solve this problem in upstream. I was shocked that the composer.json was removed from the project and it had cost us some time to handle with this in our project.

@Rarst

This comment has been minimized.

Show comment
Hide comment
@Rarst

Rarst Sep 1, 2014

Contributor

Just few points, as a person who has more experience explaining and getting composer.json into projects than I ever wanted to:

  1. Yes, Packagist allows "unrelated" people to create entries for public GitHub repositories.
  2. It's about as much of an issue as protesting "unrelated" people creating HTML links to something. But what if they change the link and it points to something else!? Possibility, but not a practical thing to fight.
  3. Packagist will absolutely transfer entry to project owners on their request.
  4. The burden of security confidence is always and pretty explicitly on consumers of the packages. Anything can be hijacked. In a way you hijacked facebook/xhprof to phacility/xhprof and consumers (Composer or any other way) have to verify validity of the change. Using Composer is not a security risk because it's not a solution for security, it doesn't absolve developer of handling security implications (it is being reasonably helpful about them such as verifying integrity via version control and archive hashes). We reasonably trust file manager to copy files, just like that we reasonably trust Composer to manage dependencies and we verify the results as necessary.
  5. While you might not be personally interested in Composer at this point, for many people this had already become the first and main point of projects consumption.
  6. Since the project changed name and maintainer it does make sense to establish new package name. Note that from Composer point of view it would be responsible to add replace directive that will make transition smooth for consumers of previous package.
Contributor

Rarst commented Sep 1, 2014

Just few points, as a person who has more experience explaining and getting composer.json into projects than I ever wanted to:

  1. Yes, Packagist allows "unrelated" people to create entries for public GitHub repositories.
  2. It's about as much of an issue as protesting "unrelated" people creating HTML links to something. But what if they change the link and it points to something else!? Possibility, but not a practical thing to fight.
  3. Packagist will absolutely transfer entry to project owners on their request.
  4. The burden of security confidence is always and pretty explicitly on consumers of the packages. Anything can be hijacked. In a way you hijacked facebook/xhprof to phacility/xhprof and consumers (Composer or any other way) have to verify validity of the change. Using Composer is not a security risk because it's not a solution for security, it doesn't absolve developer of handling security implications (it is being reasonably helpful about them such as verifying integrity via version control and archive hashes). We reasonably trust file manager to copy files, just like that we reasonably trust Composer to manage dependencies and we verify the results as necessary.
  5. While you might not be personally interested in Composer at this point, for many people this had already become the first and main point of projects consumption.
  6. Since the project changed name and maintainer it does make sense to establish new package name. Note that from Composer point of view it would be responsible to add replace directive that will make transition smooth for consumers of previous package.
@epriestley

This comment has been minimized.

Show comment
Hide comment
@epriestley

epriestley Sep 1, 2014

Member

It's about as much of an issue as protesting "unrelated" people creating HTML links to something. But what if they change the link and it points to something else!? Possibility, but not a practical thing to fight.

I would agree with you if composer.json was defined on packagist.org. But then everyone here would have just filed an issue against packagist.org ("hey, you have a dead link!"), and @lox would have fixed his package ("updated his link"), and everything would work fine.

That isn't how it work, though. If it was, this ticket wouldn't exist, and the upstream would not need to be involved at all.

Packagist will absolutely transfer entry to project owners on their request.

Based on what? Can someone else email them and have the package taken away from me later, if they come up with a convincing story? Having an implicit, undocumented, human-managed manual trust policy is not reassuring to me.

The burden of security confidence is always and pretty explicitly on consumers of the packages.

I suspect most consumers are not aware of this interaction. As a consumer who is aware, I don't see any way to verify the identify of an account on packagist.org. I can't find any documentation on the transfer policy, or documentation that it exists. When I asked about this situation in #composer-dev (via the Packagist about page), I didn't get an answer for 12+ hours. Can you describe a set of steps I can take as a consumer to verify that the owner of a packagist.org package is someone I trust, and that any changes made to the packages are changes they've authorized?

In general, it seems like @lox chose to create a package, and a lot of people chose to trust him and his package. He did this without support from the upstream ("Facebook was unresponsive..."), although he previously relied on unknowing support from the upstream (presence of a composer.json file). He now has a higher burden to maintain the package (he needs to fork), but that seems reasonable because he chose to create it in the first place, and did so knowing that the upstream was not responsive. Users might receive a less useful package (it may be out of date, or he may drop support in the future), but they chose to trust him and his package, so that also seems reasonable to me. This mostly sucks for @lox, who has a choice between breaking users and maintaining his fork forever, but he created the package in the first place and named it facebook/xhprof, so I think this is not unreasonable.

As the upstream, I'm not interested in providing Composer support at this time. I believe package managers have a substantial burden to support and inform users of security concerns, and that Composer is badly failing to meet this burden by having multiple opaque, sometimes-unverifiable, relatively-nonobvious steps between naming a package to install and executing the package's code.

Users who knowingly made a decision to trust @lox also knowingly accepted the risks associated with trusting a package maintainer who is not affiliated with the upstream (or a package maintainer whose affiliation with the upstream is unknowable and unverifiable).

Users who unknowingly made the decision to trust @lox and believed they were trusting Facebook have either been failed by Composer or are themselves at fault, depending on how much of a burden you believe Composer has to inform users.

I understand that Composer is easy, popular, widely used, works well, and is far better than alternatives that existed (or, often, did not exist) before it, and that many users rely on it. But I don't think it's good enough, and this approach to security isn't acceptable to me as an upstream. I also understand that it is good enough for plenty of users, but they can use @lox's package or the https://github.com/FriendsOfPHP/uprofiler fork, or define their own Composer package. We might support Composer in the future if it changes to shoulder a greater security burden, but I'm not willing to endorse a package manager which defaults to requiring consumers to trust multiple unverifiable agents in order to gain a small measure of convenience.

Member

epriestley commented Sep 1, 2014

It's about as much of an issue as protesting "unrelated" people creating HTML links to something. But what if they change the link and it points to something else!? Possibility, but not a practical thing to fight.

I would agree with you if composer.json was defined on packagist.org. But then everyone here would have just filed an issue against packagist.org ("hey, you have a dead link!"), and @lox would have fixed his package ("updated his link"), and everything would work fine.

That isn't how it work, though. If it was, this ticket wouldn't exist, and the upstream would not need to be involved at all.

Packagist will absolutely transfer entry to project owners on their request.

Based on what? Can someone else email them and have the package taken away from me later, if they come up with a convincing story? Having an implicit, undocumented, human-managed manual trust policy is not reassuring to me.

The burden of security confidence is always and pretty explicitly on consumers of the packages.

I suspect most consumers are not aware of this interaction. As a consumer who is aware, I don't see any way to verify the identify of an account on packagist.org. I can't find any documentation on the transfer policy, or documentation that it exists. When I asked about this situation in #composer-dev (via the Packagist about page), I didn't get an answer for 12+ hours. Can you describe a set of steps I can take as a consumer to verify that the owner of a packagist.org package is someone I trust, and that any changes made to the packages are changes they've authorized?

In general, it seems like @lox chose to create a package, and a lot of people chose to trust him and his package. He did this without support from the upstream ("Facebook was unresponsive..."), although he previously relied on unknowing support from the upstream (presence of a composer.json file). He now has a higher burden to maintain the package (he needs to fork), but that seems reasonable because he chose to create it in the first place, and did so knowing that the upstream was not responsive. Users might receive a less useful package (it may be out of date, or he may drop support in the future), but they chose to trust him and his package, so that also seems reasonable to me. This mostly sucks for @lox, who has a choice between breaking users and maintaining his fork forever, but he created the package in the first place and named it facebook/xhprof, so I think this is not unreasonable.

As the upstream, I'm not interested in providing Composer support at this time. I believe package managers have a substantial burden to support and inform users of security concerns, and that Composer is badly failing to meet this burden by having multiple opaque, sometimes-unverifiable, relatively-nonobvious steps between naming a package to install and executing the package's code.

Users who knowingly made a decision to trust @lox also knowingly accepted the risks associated with trusting a package maintainer who is not affiliated with the upstream (or a package maintainer whose affiliation with the upstream is unknowable and unverifiable).

Users who unknowingly made the decision to trust @lox and believed they were trusting Facebook have either been failed by Composer or are themselves at fault, depending on how much of a burden you believe Composer has to inform users.

I understand that Composer is easy, popular, widely used, works well, and is far better than alternatives that existed (or, often, did not exist) before it, and that many users rely on it. But I don't think it's good enough, and this approach to security isn't acceptable to me as an upstream. I also understand that it is good enough for plenty of users, but they can use @lox's package or the https://github.com/FriendsOfPHP/uprofiler fork, or define their own Composer package. We might support Composer in the future if it changes to shoulder a greater security burden, but I'm not willing to endorse a package manager which defaults to requiring consumers to trust multiple unverifiable agents in order to gain a small measure of convenience.

@epriestley epriestley closed this Sep 1, 2014

@pierrejoye

This comment has been minimized.

Show comment
Hide comment
@pierrejoye

pierrejoye Sep 1, 2014

Long debates apart, extensions have nothing to do in packagist IMO. Once
pickle is integrated with composer we should use pecl.php.net as packagist
for extensions. Much cleaner :)
On Sep 1, 2014 7:22 PM, "Evan Priestley" notifications@github.com wrote:

It's about as much of an issue as protesting "unrelated" people creating
HTML links to something. But what if they change the link and it points to
something else!? Possibility, but not a practical thing to fight.

I would agree with you if composer.json was defined on packagist.org.
But then everyone here would have just filed an issue against
packagist.org ("hey, you have a dead link!"), and @lox
https://github.com/lox would have fixed his package ("updated his
link"), and everything would work fine.

That isn't how it work, though. If it was, this ticket wouldn't exist, and
the upstream would not need to be involved at all.

Packagist will absolutely transfer entry to project owners on their
request.

Based on what? Can someone else email them and have the package taken away
from me later, if they come up with a convincing story? Having an implicit,
undocumented, human-managed manual trust policy is not reassuring to me.

The burden of security confidence is always and pretty explicitly on
consumers of the packages.

I suspect most consumers are not aware of this interaction. As a consumer
who is aware, I don't see any way to verify the identify of an account on
packagist.org. I can't find any documentation on the transfer policy, or
documentation that it exists. When I asked about this situation in
#composer-dev (via the Packagist about page), I didn't get an answer for
12+ hours. Can you describe a set of steps I can take as a consumer to
verify that the owner of a packagist.org package is someone I trust, and
that any changes made to the packages are changes they've authorized?

In general, it seems like @lox https://github.com/lox chose to create a
package, and a lot of people chose to trust him and his package. He did
this without support from the upstream ("Facebook was unresponsive..."),
although he previously relied on unknowing support from the upstream
(presence of a composer.json file). He now has a higher burden to
maintain the package (he needs to fork), but that seems reasonable because
he chose to create it in the first place, and did so knowing that the
upstream was not responsive. Users might receive a less useful package (it
may be out of date, or he may drop support in the future), but they chose
to trust him and his package, so that also seems reasonable to me. This
mostly sucks for @lox https://github.com/lox, who has a choice between
breaking users and maintaining his fork forever, but he created the package
in the first place and named it facebook/ xhprof, so I think this is not
unreasonable.

As the upstream, I'm not interested in providing Composer support at this
time. I believe package managers have a substantial burden to support and
inform users of security concerns, and that Composer is badly failing to
meet this burden by having multiple opaque, sometimes-unverifiable,
relatively-nonobvious steps between naming a package to install and
executing the package's code.

Users who knowingly made a decision to trust @lox https://github.com/lox
also knowingly accepted the risks associated with trusting a package
maintainer who is not affiliated with the upstream (or a package maintainer
whose affiliation with the upstream is unknowable and unverifiable).

Users who unknowingly made the decision to trust @lox
https://github.com/lox and believed they were trusting Facebook have
either been failed by Composer or are themselves at fault, depending on how
much of a burden you believe Composer has to inform users.

I understand that Composer is easy, popular, widely used, works well, and
is far better than alternatives that existed (or, often, did not exist)
before it, and that many users rely on it. But I don't think it's good
enough, and this approach to security isn't acceptable to me as an
upstream. I also understand that it is good enough for plenty of users,
but they can use @lox https://github.com/lox's package or the
https://github.com/FriendsOfPHP/uprofiler fork, or define their own
Composer package. We might support Composer in the future if it changes to
shoulder a greater security burden, but I'm not willing to endorse a
package manager which defaults to requiring consumers to trust multiple
unverifiable agents in order to gain a small measure of convenience.


Reply to this email directly or view it on GitHub
#40 (comment).

Long debates apart, extensions have nothing to do in packagist IMO. Once
pickle is integrated with composer we should use pecl.php.net as packagist
for extensions. Much cleaner :)
On Sep 1, 2014 7:22 PM, "Evan Priestley" notifications@github.com wrote:

It's about as much of an issue as protesting "unrelated" people creating
HTML links to something. But what if they change the link and it points to
something else!? Possibility, but not a practical thing to fight.

I would agree with you if composer.json was defined on packagist.org.
But then everyone here would have just filed an issue against
packagist.org ("hey, you have a dead link!"), and @lox
https://github.com/lox would have fixed his package ("updated his
link"), and everything would work fine.

That isn't how it work, though. If it was, this ticket wouldn't exist, and
the upstream would not need to be involved at all.

Packagist will absolutely transfer entry to project owners on their
request.

Based on what? Can someone else email them and have the package taken away
from me later, if they come up with a convincing story? Having an implicit,
undocumented, human-managed manual trust policy is not reassuring to me.

The burden of security confidence is always and pretty explicitly on
consumers of the packages.

I suspect most consumers are not aware of this interaction. As a consumer
who is aware, I don't see any way to verify the identify of an account on
packagist.org. I can't find any documentation on the transfer policy, or
documentation that it exists. When I asked about this situation in
#composer-dev (via the Packagist about page), I didn't get an answer for
12+ hours. Can you describe a set of steps I can take as a consumer to
verify that the owner of a packagist.org package is someone I trust, and
that any changes made to the packages are changes they've authorized?

In general, it seems like @lox https://github.com/lox chose to create a
package, and a lot of people chose to trust him and his package. He did
this without support from the upstream ("Facebook was unresponsive..."),
although he previously relied on unknowing support from the upstream
(presence of a composer.json file). He now has a higher burden to
maintain the package (he needs to fork), but that seems reasonable because
he chose to create it in the first place, and did so knowing that the
upstream was not responsive. Users might receive a less useful package (it
may be out of date, or he may drop support in the future), but they chose
to trust him and his package, so that also seems reasonable to me. This
mostly sucks for @lox https://github.com/lox, who has a choice between
breaking users and maintaining his fork forever, but he created the package
in the first place and named it facebook/ xhprof, so I think this is not
unreasonable.

As the upstream, I'm not interested in providing Composer support at this
time. I believe package managers have a substantial burden to support and
inform users of security concerns, and that Composer is badly failing to
meet this burden by having multiple opaque, sometimes-unverifiable,
relatively-nonobvious steps between naming a package to install and
executing the package's code.

Users who knowingly made a decision to trust @lox https://github.com/lox
also knowingly accepted the risks associated with trusting a package
maintainer who is not affiliated with the upstream (or a package maintainer
whose affiliation with the upstream is unknowable and unverifiable).

Users who unknowingly made the decision to trust @lox
https://github.com/lox and believed they were trusting Facebook have
either been failed by Composer or are themselves at fault, depending on how
much of a burden you believe Composer has to inform users.

I understand that Composer is easy, popular, widely used, works well, and
is far better than alternatives that existed (or, often, did not exist)
before it, and that many users rely on it. But I don't think it's good
enough, and this approach to security isn't acceptable to me as an
upstream. I also understand that it is good enough for plenty of users,
but they can use @lox https://github.com/lox's package or the
https://github.com/FriendsOfPHP/uprofiler fork, or define their own
Composer package. We might support Composer in the future if it changes to
shoulder a greater security burden, but I'm not willing to endorse a
package manager which defaults to requiring consumers to trust multiple
unverifiable agents in order to gain a small measure of convenience.


Reply to this email directly or view it on GitHub
#40 (comment).

@lox

This comment has been minimized.

Show comment
Hide comment
@lox

lox Sep 1, 2014

I'm sad you've come to that decision @epriestley. Composer is the way that the PHP community distributes packages and manages dependencies, and the fact that anyone can easily and quickly publish code has transformed PHP's community vastly for the better. Calling it a "small measure of convenience" is a massive understatement.

The reality of all package managers is that you end up trusting "multiple unverifiable agents" in one way or another. It's an imperfect solution to a difficult social problem. Could Packagist do a better job? Sure. So could RubyGems, etc. Ideally we'd all sign our packages and trust could be established via a web of trust. Ideally we'd not rely on single points of failure for package distribution either. That said, what we've got is a huge step forward from where we were at with PEAR.

I'll continue to maintain the fork for the 83,721 users who are presently using it. Hopefully phacility adds enough value to the upstream version that people will be inclined to go through the extra hassle to consume it.

lox commented Sep 1, 2014

I'm sad you've come to that decision @epriestley. Composer is the way that the PHP community distributes packages and manages dependencies, and the fact that anyone can easily and quickly publish code has transformed PHP's community vastly for the better. Calling it a "small measure of convenience" is a massive understatement.

The reality of all package managers is that you end up trusting "multiple unverifiable agents" in one way or another. It's an imperfect solution to a difficult social problem. Could Packagist do a better job? Sure. So could RubyGems, etc. Ideally we'd all sign our packages and trust could be established via a web of trust. Ideally we'd not rely on single points of failure for package distribution either. That said, what we've got is a huge step forward from where we were at with PEAR.

I'll continue to maintain the fork for the 83,721 users who are presently using it. Hopefully phacility adds enough value to the upstream version that people will be inclined to go through the extra hassle to consume it.

@kusmierz kusmierz referenced this pull request in puphpet/puphpet Sep 2, 2014

Closed

Composer & Xhprof fail provisioning #1071

@Seldaek

This comment has been minimized.

Show comment
Hide comment
@Seldaek

Seldaek Sep 5, 2014

Just a word from the composer side.. a bit late to the battle due to holidays but anyway @epriestley should you change your mind I would not mind transfering the facebook/xhprof ownership to your account or a trusted phacility account on packagist, if you were to create one. I am sure @lox also would support that move, then the repo URL would point to this URL and you would have control over it, so users with a blindly trust would at least blindly trust the right maintainers, and no more risk is involved. And yes anyone can submit a package without name checks as long as a composer.json is present, much like I could push a package called facebook-whatever to most other package managers out there. It's not something that I see as inherently fixable, it allows fair use people to do good things, and of course it allows scams potentially, but that's the usual trade-off of freedom vs control.

Anyway I am happy if this ends up working with pickle and eventually we can delete it altogether from packagist, or perhaps have a xhprof-gui package installable via composer to add to your project, and then the ext as a separate pickle package.

Seldaek commented Sep 5, 2014

Just a word from the composer side.. a bit late to the battle due to holidays but anyway @epriestley should you change your mind I would not mind transfering the facebook/xhprof ownership to your account or a trusted phacility account on packagist, if you were to create one. I am sure @lox also would support that move, then the repo URL would point to this URL and you would have control over it, so users with a blindly trust would at least blindly trust the right maintainers, and no more risk is involved. And yes anyone can submit a package without name checks as long as a composer.json is present, much like I could push a package called facebook-whatever to most other package managers out there. It's not something that I see as inherently fixable, it allows fair use people to do good things, and of course it allows scams potentially, but that's the usual trade-off of freedom vs control.

Anyway I am happy if this ends up working with pickle and eventually we can delete it altogether from packagist, or perhaps have a xhprof-gui package installable via composer to add to your project, and then the ext as a separate pickle package.

@amirkdv amirkdv referenced this pull request in evolvingweb/chef-deploy-drupal Oct 28, 2014

Closed

Installing XHProf fails from Brian Mercer's PPA #40

lox added a commit to lox/xhprof that referenced this pull request Aug 23, 2015

Revert "Remove Composer support from XHProf"
See discussion here phacility#40

This reverts commit e3df210.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment