-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
Poor security practices being recommended in documentation #1074
Comments
I agree however this is temporary installation. If I understand correctly, for "regular" users the installation might be still unzip and run install via web. It might/will use composer internally but will not require command line access. Thoughts @tanya-soroka ? |
I am also interested to see community feedback. Feedback on install/deployment so far has been "you cannot satisfy everyone, so don't bother trying to". As long as we have one approach documented that works, smart people can work it out from there how to adapt to their environment. If there is a better way... |
@davidalger , I believe, the intent in this article was to make files downloaded/created by Composer readable by the web-server. I'd support @piotrekkaminski that for shared hosting users (or other users who don't want to or can't use CLI), the workflow should be different - just unzipping an archive, no Composer. For big merchants, I don't expect that they would do anything on production. Still is it an issue for staging environment, where real update would happen? For developers ... I'm not sure that this is a security issue. But I may be wrong - I don't know all developer workflows. If it is, is it fair to expect that developers should be aware of such security issue and just fix it by them-selves and use more secure workflow? Even skipping all my thoughts, @davidalger , any ideas/suggestions how can we describe the workflow better? What should be the recommendation for running Composer, so that web-server could access that files then? I have in mind only something like "create/use a user with www-data group to run Composer", so the user may have enabled shell, while www-data user would not and still the files would be accessible for www-data. |
@buskamuza Yes, the intent is to make the files created by Composer readable by the web-server. However, in doing so in the manner prescribed, the server is made more vulnerable to being compromised as well. Big no no. The documentation should differentiate between the two type of environments then if it's not expected that this be done on live. If it's only expected that big merchants do this, why put it in the generic docs as default information?
Big emphatic no. Developers are not server admins. Giving them a prescribed way to do things in an insecure manner is condoning the practice and suggesting they do it that way. In a perfect world, I would love it if you could count on all developers to think security all the time, but that's not real life — the practices specified by official documentation should be the "best practices" not ones we expect them to find better workflows to. Obviously everyone isn't going to follow the prescribed workflows, but they have got to be something which is not promoting insecure behavior. We have enough issues in the eCommerce industry already with insecure practices being followed without having them promoted in official platform documentation.
I'll have to think about it some and post back more later. :) My first though on reading the page was: maybe it should suggest everyone use php-fpm with worker pools assigned to a shell user. Another thought which immediately followed was: why is the documentation on update procedure telling me to reconfigure my server to work around something only encountered when using mod_php + apache, a really old-school approach to a secure hosting environment. |
On behalf of developer documentation, thank you for bringing this to our attention. We take security issues very seriously because we understand security is critical for our community. We’re working on a solution and we’ll let you know as soon as it’s available. |
@davidalger We changed the instructions to reflect your concerns. Please let us know what you think: http://devdocs.magento.com/guides/v1.0/install-gde/install/prepare-install.html#install-update-depend-apache Thank you again for bringing this to our attention. |
@davidalger, I am closing this ticket. Thank you for your contribution to Magento! |
http://devdocs.magento.com/guides/v1.0/install-gde/install/prepare-install.html
The above documentation page is directing users to enable a shell on the web-service user. This is terrible practice and there is a reason why daemons run under users which have their shells disabled. I realize permissions can be a real bear on a traditional mod_php setup, but it would seem to me that security is of much greater importance than ease and the directing should be towards a solution which works without compromising security.
On a non-security front, this document assumes root level access to a box, something which many users will not have. Shared hosting anyone? Surely the instructions for updating an instillation aren't going to leave out anyone who attempts to use a cPanel / Plesk or other shared hosting (regardless of whether or not it runs mod_php).
Very curious to hear what others perspectives on this are.
The text was updated successfully, but these errors were encountered: