-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Robustness Big Issue 1: cron shouldn't be allowed to run as root #2787
Comments
@pgorod - Thank you for you question. However, We use github primarily for reporting of Core SuiteCRM Issues and we feel your question would be more suited to our Community Forum. We will be able to help you with your issue there. |
@shogunpol you have got to be kidding me! I know it's too long, but did you even read it? It's not a question, it is a core SuiteCRM issue, it's a code problem, it's a High Priority bug, and it is actually good news for the project that we have this diagnosis. It also represents a lot of hours on my behalf studying the problem, debugging and testing, and it surely deserves more than 8 minutes of life on Github. |
Now lets make this possible and solve the permissions problems! |
A problem with this solution is that it assumes that everyone has the same deployment strategy and configuration. So for example on windows the permissions may work but the read only flag might be set on the files, which can also cause issues. On some distributions of Linux the user may well actually be root. Perhaps we could check who is the user when an entrypoint (eg index.php) has been accessed. Then when cron.php is called we could compare it with the last user we recorded. If they are different then create a log entry. An other solution would to use the same checks that are in the installer/upgrade. Checking every file would be too time consuming. So maybe we could just check the www root directory(itself) of the instance or the entrypoint file that is being accessed. To see if the there are any issues. Then add them to the log. |
Thanks for joining the discussion! About Windows, I'm leaving this aside for the moment. If there are problems there, I'd say they are different. About Linux, you are right that there might be special cases. I was assuming that using Other than that, the approach you suggest of recording last user going through index.php is a possibility. It's similar to recording the user at the moment of installation, but more dynamic. What do you think of the idea of having a list of allowed users in config.php? We would start that list during installation, simply filling it with whatever user was used for install.php. Then admins could add to that if they had any special requirements of using multiple users. This way things would be simple for the simple installations (no work to be done), and versatile for people with more advanced needs. |
Maybe saving the uid of the www-data user in the config, and then using setuid ( http://php.net/manual/en/function.posix-setuid.php ) could be a solution. You can throw a warning, but at least cron would still work. |
@gunnicom I think the setuid is the better solution for cron.php instead of logging. However this will only work on Unix Machines. @pgorod I think having allowed users is taking it a step too far. As an admin, you should be able to run a database dump and move an instance to an other machine, with minimal configuration. The real issue is mostly an education issue with users who do not necessarily have the experience of correctly deploying suitecrm. That isn't to say that there isn't a problem with permissions. It would be good If SuiteCRM could prevent these issues from occurring. My concern here is performance. We should try to limit system calls as much as possible. Otherwise it will add additional latency to each request a user makes. We should not log such issues as fatal. Perhaps it should be a debug/info instead. So that users don't end up with a massive log file. We need to ensure we have a windows solution as well. There are plenty of users who run SuiteCRM on IIS/Apache on Windows. I don't think we should simply ignore them. I think IIS is covered. As the install informs the user if there are issues. However, checking that the user is ISR or ISR_User could be useful when setting up the cron.php on windows. I am not sure how other servers would work on windows like apache, nginx, lighthttp etc. it would be good if a community member could test that. This is a perfect opportunity for a community member to come up with a solution. |
The only reason I'm not looking for a Windows solution is because we don't have a Windows problem :-) Not here, on this issue. I would have to look into a case of Windows permissions degradation, try to reproduce it, debug it, understand it. I'm fine with that, I was much more familiar with Windows security than Linux before all of this - but it's not what I have achieved here. So I think it is more precise to say here we need a solution that doesn't negatively impact Windows - that's for sure. But we're working on a solution to a documented Linux problem. I think it's fair to try to induce some degree of best-practices, lots of mature programs and systems do it. If running as root is always bad in Linux, then it wouldn't be a stretch to make SuiteCRM a system that refuses to run on such bad practices. But as I said, that's an "if" and I'm not sure to what extent it is true. If anybody can give specific counter-example, please do. About the logging and "dying", A FATAL error and a "die" at installation or upgrade time is not necessarily a bad thing. You have the admin's attention, he scheduled time, he can quickly Google things up and discover he needs a simple tweak to config.php. Believe me, getting a phone call later to learn that the system crumbled inexplicably at a random time, with nothing consistent in the logs is a much worse experience, and a frequent one we're trying to solve here precisely. Apart from the setuid solution that seems problematic and too Linux specific, I wouldn't worry about performance or system calls, since the only call would be once per minute on cron.php (an offline, asynchronous process), checking which user it is running under. About the scenario of dumping the DB and moving elsewhere, isn't writing a user name in config.php (only if you changed it) a "minimal configuration"? Slightly new proposalSo, after reading your suggestions and thinking about this better, my preferred solution now would be to forget about actions targeting specifically the "root" user and instead just do this:
Impact:
What do you think about this revised proposal? |
I am not so optimistic on this two:
Or will think "Oh that crap does not work, i switch to something different"
Or people turn away from the project, because there are other projects that "just work" and they dont have to deal with this. |
@everyone Maybe the words "fatal" and "die" make the proposal seem menacing, but remember that's just the cron jobs; not the main app. Some people go without cron jobs for months without even noticing it; it depends on your use of indexes, notifications, workflows, and a few other things. @gunnicom I don't think people drop projects because something breaks when they configure it wrong. Remember my proposal includes updating the wiki, and the screens giving instructions, and the release notes. There's also a specific log message providing a solution. So they will just come across an issue, learn that it is justified (works in their favour, in the long run), and be instructed how to fix it easily. And I'm not sure everybody here is aware of the degree of chaos that this issue causes. This is the stumbling block for SuiteCRM. I spend a lot of time in the forums. The hassle of the current situation is 10x, maybe 100x, greater than the hassle of this proposal. So we need to take our pick. I think it's pretty obvious, it just takes some time getting our heads around it. Things have been this way for so long we've gotten used to them. |
I have the code I just have to add it into the fork and do the pull request. |
@chris001 your first solution is interesting, I hadn't thought of that. About the second bullet, the"better solution", I don't understand what you mean when you say The two difficulties I see with that approach are a) what happens in Windows? and b) you might have edge cases where setuid will fail because you're actually trying to elevate privileges instead of lowering them. You need to be root to use setuid, which might not be the case, but still be problematic for file/folder ownership. On the other hand, on a weird configuration the web app could be running as root while cron is running with lower privileges, and setuid would fail... If what you meant was setuid, please explain how that could work since I don't know much about it... thanks. |
If someone wants to test this. On Ubuntu you can edit the crontab like: If you ran the cron as root before dont forget to correct your permissions, otherwise the www-data cron wont run: |
Over the weekend I've written and tested the PHP code to fix this bug or enhance the application, depending on how you look at it. Debating whether to add it into an existing file such as |
I'm curious about your approach, and I'd love to see that code. Even if it's not finished yet you could just paste it here so we can take a peek. : - ) Thanks for your help. |
Lets start with the most blatant perceived problem....people running the cron.php as root. It's easy to deal with: $user=posix_getuid(); if ($user==0) //then root } However, how do we find the correct user that the webserver is running as? How about: file_put_contents("tmpFile", "123"); run during initial installation and configuration and value stored somewhere?....perhaps in the database? That stops people inadvertently running as root, as root can be made to run as the correct web server user. trying to come up with a suitable strategy for a non-root user is actually more difficult. Under these circumstances running as root is better as the script can be written to give up root permissions early on. This is the approach used by many system daemons and works for *nix systems. I'm just wondering whether this might break on a windows system? Requirement for php-system? Unfortunately on a shared box, the user may not have root/admin access available to them, and under these circumstances, chris001's appoach to trigger the task via a curl cron task is actually becoming more appealing. |
My heart is still with the "Slightly New Proposal" I gave above. It's straightforward, makes sense, and works everywhere. And it's about a few hours of development time, really ridiculous when compared with the benefits it would bring. I hate myself for having the life of a fireman, running from one place to another, and not yet having had the time to put aside an afternoon to develop this myself... |
Code snippet above allows point 1. |
@samus-aran thank you for coming in on a Saturday to do some merging. We were needing some of that to get our spirits up : - ) If you care to take a look at this issue here I believe it would be in the best interests of this project. Never mind the fact that it's labelled "Suggestion", it's really a bug and a very old and nasty one. If this gets merged I promise I'll put in a couple more permissions/robustness fixes. I have this all figured out and it's perfectly fixable. SuiteCRM does not need to fail as much as it does on account of permissions. We can fix this. I just need to see that your interest in this matches mine... |
@pgorod Haha, don't expect any more Saturdays runs tho! But seriously, had a quick review of the PR supplied and it looks sound probably only needs tiding up. I'll test it but I will not merge this weekend as I will discuss the solution internally next week. 👍 Thanks again for all your hard work! |
Hi @samus-aran Very busy, yet, look for a PR from me coming any day now, to solve this category of issues thoroughly. |
Was going to close this as moving it to the Suggestion Box, but instead I'll keep it open but add it also so you can see the progression. |
Hi @samus-aran. Chris first promised the code back in December, but after insisting with him a few times I just went ahead and coded it myself. I'm not complaining, I see he's been quite busy with numerous other valuable contributions. I am happy with the current proposal and I think it is ready to be merged today. I don't see a point in waiting for a second version of the same thing. However, if @chris001 explains to us that his proposal is different, and better than this one, that's cool with me. Normally when we are both talking to each other here in Github he's teaching me, and I'm just learning : - ) I would hate to see this delayed, though, so let's all try to keep things moving... |
@samus-aran @pgorod No conflicts, the code doesn't overlap, you can try and progress this ASAP. |
Fixes #2787 by checking allowed cron users
I didn't read every line in this thread, but just want to back the general intention of this thread. I've been administering Sugar CE / Suite for 6 years. The re-occurring permissions problem is the most wacko thing about the project as a whole. Everybody just knows something's broken, but since it can (almost) always be fixed with a global resetting of permissions, it's never been fixed. I sincerely hope you guys are coming up with a good solution. Again, I think a solid fix and/or consistent information about this would add alot to the overall feel and appeal of SuiteCRM. |
@EZS-JD I am pretty confident I have the problem diagnosed, basically the permissions problems come from this issue (fixed since 7.8.3, but you can fix it in any version by configuring cron correctly), and from the issue described here #2810 (the initial issue, plus other issues described later in that thread). This is still waiting to be fixed, I hope to do it soon. But it has a lot less influence than the cron jobs issue. But I would say if you configured your cron jobs correctly, then your permissions wouldn't degrade. You don't need a script to reset permissions, what you really need is for them not to degrade. If you want you can read about the whole thing in this blog post, and then if you have any questions start a thread in the forums and I'll help you do it specifically for your system. |
Thanks for the excellent blog post. Should've had it a few years ago :-) I have our systems set up with www-data running cron. But I guess I had no way of knowing whether that would really fix all the permissions issues I had been seeing over the years (and even experts seemed just used to having occasionally), so I got that daily permission settings script running... Now that it's running I see no reason to turn it off :-) But I love that you guys are digging deep and trying to clean up the mess. Thanks for your efforts! It's all a little over my head I'm afraid. |
The one reason for getting rid of you script would be this: if permissions degrade, bugs happen; sometimes these bugs can really break some part of your data, or even of your code files, since SuiteCRM is an app that writes itself (Studio etc). Of course if you reset your permissions nightly, these things are mitigated. But they are not fixed. You are 95%, or 99% covered for problems, not 100%. Your degraded permissions can damage your system between the time when they degrade, and the night-time script. If your cron jobs run under user |
I understand and agree. our apache is running as www-data and suitecrm-scheduler-cron is too. |
So, if you're feeling courageous take down that nightly script, just fix your permissions one final time running these commands from your SuiteCRM directory:
Also make sure in config.php you have something like this:
Until you gain confidence in this, you can regularly watch for any degraded permissions with this command on the same directory:
It will say something like Of course only you can make the decision whether this is too risky for you or not (there are also risks to not doing it!). If you want a really prudent way, just run that |
Thanks for the detailed info. I don't however feel that courageous right now (if it ain't broke, ....) |
This issue has been mentioned on SuiteCRM. There might be relevant details there: |
This could also be titled "Solve permissions problems once and for all" :-)
After recent tests reliably reproducing permissions degradation problems in a controlled testing environment, and finally finding a proper explanation for them, I venture saying that we are POSSIBLY looking at something that will solve ALL permissions problems, or at least we're SURELY looking at something that will solve a LOT of permissions problems that lots of people experience.
We're talking about 5 or 6 cases of this problem every week on the forums, not counting all the other instances where people don't ask, because they already know how to do it. I know I used to fix permissions before and after every upgrade, and some people run cron scripts to fix them every hour... let's put an end to that, shall we?
You can read about the tests here: #1688
What is permissions degradation in SuiteCRM?
It's when file permissions are set correctly, but at a later moment are changed in a way that causes problems. To cause problems, normally it's because files get harder permissions (either ownership by a more secured user like
root
, or losing some mode of access likewriteable
) and then when SuiteCRM needs to read or write those same files, it gets access denied errors. These will break some functionality, probably at an unexpected point, resulting in poor logging/messages to the user and admins.Why does it happen?
It certainly doesn't happen just "because", for no reason at all. It happens because some process running on the server does the changes; it could be a third party process messing with the files, but that's really strange. Why would any process just randomly mess with another app's files?
It could be (and it is) the SuiteCRM application itself. If it's not the web application, then it would have to be the Scheduler jobs (and it is).
How should Scheduler jobs be configured?
They should run under the same user that is used for the Web Server. For example, on a typical Ubuntu install, this would be
www-data
. On Bitnami installs it would bebitnami
, etc.Does anybody configure Schedulers to run as root? Why would they?
I did : - )
I didn't know I could edit the crontab of a user that does not allow logins, like
www-data
(withcrontab -e -u www-data
). Initially I didn't even consider that there was a separate cron for each user. So I just edited crontab as root and made cron.php always run as root. And the jobs worked, so I just forgot about it... Not good practice, I understand now. But Linux actually induces ignorant admins to do just that.Also, the fact is that there is nothing in the wiki documentation about which user to use. The advice commonly given on the forums doesn't mention it. 'SuiteCRM for Developers' doesn't mention it. Finally, the line suggested by the installer doesn't mention it.
From asking in the forums in these past weeks, I know other people did it (just one example: https://suitecrm.com/forum/installation-upgrade-help/12155-import-data-from-a-sugarcrm-6-5-20-db-to-suitecrm-7-7-8).
I suspect LOTS of other people did it.
So, how exactly do Schedulers break permissions when running as root?
They write files, and these will get ownership by root. Then other processes like the SuiteCRM app running from the web server won't be able to access them. The most relevant case has to do with caching: just by accessing certain common data structures, vardefs and other elements, the system will generate cached files. The odds of this happening increase during installations, upgrades, and Quick Repair and Rebuilds, when caches are flushed and get rebuilt from scratch, so whatever the cron jobs touch first, before the rest of the web app, will be owned by root.
How to fix it?
For individual installs, configure cron correctly.
For the project, to really increase the real-world stability and robustness of the app, and to remove the bad fame of being an app that breaks inexplicably, a combination of these things:
Specify in the wiki installation instructions that the crontab must be configured to run under the same user as the web server, recommending usage of crontab -e -u TheWebServerUserName.
Inside the app, make the same message appear in the installer scripts (install/confirmSettings.php, install/ready.php). Tell the user specifically the user name to use.
Make the same message appear in the Admin-->Schedulers page (modules/Schedulers/Scheduler.php)
Make cron.php check under which user it is running. If running as root, log a FATAL error
cron.php is running as root. This will cause serious errors and must be corrected.
. Thendie();
.Have the SuiteCRM installer save both user and group in config.php after install. These might be valuable clues for cron.php to use to compare to its current user.
Other users?
As a final note, strictly speaking, these problems could arise with users different from root, basically with any user that is not the web server user. But I expect root is the culprit 99% of the time.
I've thought about this in the past weeks and it's not easy to get a more robust solution. When running from within
cron.php
(from CLI) there's no easy way to find the web server user. Maybe solution number 5 above would be a good place to start. Possibly in the future we should be moving to a situation where config.php includes a list of possible users under which SuiteCRM and cron will run. All others would cause the app to exit. This would bring the issue of consistent user configuration under a wiser attention from admins.But for now, let's start with just checking for
root
, to keep things simple.High Priority?
I know everybody thinks 'their' bugs are High Priority. But this on definitely is (and it's not 'my' bug, my system is fixed now and stays fixed!).
This is High Priority on all counts: number of people affected, long-standing bug, critically breaks affected systems, very hard to diagnose, affects SuiteCRM public fame, cripples adoption by new users, consumes forum resources greatly and needlessly, etc.
The text was updated successfully, but these errors were encountered: