[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Changing how LedgerSMB is run and it's installation is secured

Hi David,
Through our discussion on IRC about the items 1-3 and which way to solve them, we seem to have ended up at trying to solve items 4-8 now, so, as nobody commented on those yet, I'm sending my comments below.

The long and the short of it is the following proposal.
If you have any thoughts or suggestions could you please comment before we start implementing the changes.

We believe items 1-3 NEED to happen, the remaining items SHOULD happen depending on your responses
  1. Only ever run LedgerSMB using Starman (High-performance preforking PSGI/Plack web server)
  2. Only ever bind Starman to localhost
  3. Always reverse proxy Starman port with either NGINX (preferred) or Apache
Generally, I'd say the default should depend on what's already available on a system. If Apache is available and installed, we'd want to support using that, while if Nginx is installed, we want to bind to that. If *neither* is installed, then, yes, choose one.
Yes, agreed. That is essentially what I've discussed with Jame to have the .deb package do.
This means that Apache and Nginx need to only be suggests rather than recommends or depends, and selection and installation needs to be done in the install scripts.

Ok, but shouldn't it also *require* the (virtual) 'httpd' package? (So as to make sure at least one of the HTTPd providing packages gets selected?)
  1. At installation create a system user "ledgersmb"
Ok. And the database user won't be named that anymore in the future, so there's no confusion regarding the type of user which is being referred to?  It's the intent to run the starman process under this user, right? So, it's a low-auth-level user, much like "nobody", but with a separate "namespace"?
Yes, the database admin user will be lsmb_admin to eliminate confusion
Yes, starman would run as ledgersmb either by somthing like systemd setting the USER or by using starman's built in ability to drop privileges

Ok. So, the "system starman" gets to run on any port the admin wants. That's good, I'd say.
Using the separate namespace (ie: not nobody:nogroup) solves the security issue created if temp files are writeable by other processes on the system

 I'm taking the above to say "... solves the security issue that arises if other processes run as nobody [or nogroup] and thereby get read (or write) access to the temp files". In that case, fully agreed.
  1. At installation create a system group "ledgersmb-admin"
Ok. What's the purpose of this user? To be the group to run the starman process under? If not, then what does the starman user get run as, and what is the role of this group?
This is not a user, only a group.
My mistake. I fully understood it was a group.
Any system user that should be able to edit/modify/add/remove files in the ledgersmb install dir would need to be a member of this group.
The idea is that the user running ledgersmb (and starman) should not have write access to the files. but some user is likely to need write access (system admin) so we have a group just for that purpose

Ok. Personally, I think we should not want to provide write access to the css files and templates: the templates are in the database and the CSS directory can be set in the configuration file, so if users want anything other than the CSS that comes with the system, they can set the CSS directory from there.

By way of precedent, I've just checked 2 other web applications that I know of (viewvc and websvn). Both install their CSS files in /usr/share/{,doc/} and have them marked as "root:root -rw-r--r--" which is exactly what I think people are expecting from a package.
  1. all LedgerSMB files should be installed in a single versioned directory (eg: /usr/share/ledgersmb-1.4.26 or /opt/ledgersmb-1.4.26)
If this is to support multiple simultaneously installed versions, then it works as long as these versions don't have different versions of the same library as their dependencies. I think your *only* way to full independent installations is to use Perl's mirror of Python's 'virtualenv': local::lib -- which installs *all* dependencies in a separate directory with the correct dependency version.
For now the assumption is that all instances would be using the same system perl libs, but yes in the long run the use of local::lib solves other issues that we  have previously discussed

 Agreed, but I don't think it's reasonable to expect this behaviour of a Debian-included package. So, indeed it would be great if we can run two successive versions against the same Perl modules, or depend on different modules entirely.
  1. the install directory and contents should be owned by user ledgersmb and group ledgersmb-admin
Ok. I'm not sure I understand why we need this: all Perl libraries get installed as root:root rwxr-xr-x. It applies to all global libraries, including all of LedgerSMB's many dependencies. I think root:root rwxr-xr-x should work fine for us: it's not like we're trying to stop people from looking into these files are we? And if we are? Doesn't that provide a false sense of security? Anybody can just download these files from the web and install them in their home directory and start their own instance...

Other than that, you describe a pretty complex system of permissions. There's a major downside to the system you propose: it's highly complex, yet the software we're writing we try to keep as simple as possible, because of the fact that we are already experiencing that people (users) find it too hard to install correctly. I think that adding a highly non-standard permissions pattern to the mix is asking for trouble there.
Traditionally anything that is being served to the web has been owned by any user with restricted permissions (eg:www-data) (ie: not root) to reduce/prevent the chance that an attack can impact the actual system.
However neither do you want these files to normally be writable by the running user (for the same reasons)
Some systems I've worked with actually use the same type of scheme I've outlined here.

Ok. I can see that one would implement such a scheme in an embedded environment (where hardly anybody is likely to ever look inside the device) or in a corporate environment where dedicated admins know what they're doing and extensive documentation needs to be written to justify the choices to satisfy auditors. However, I think we want to stick as close as we can to "standard practice": the documentation that we deliver with the package (which gets installed in /usr/share/doc/*) is most likely not being read by the installing admin (who might be no more than "just" a Debian/Ubuntu end-user).
I'll concede that having other r-x shouldn't hurt in most cases. EXCEPT when we consider ownership of temp dir's and files.
We have recently seen this issue on IRC where the temp dir (or a few files) were owned by a user other than the one currently running LedgerSMB

True, but my point is that we simply should not create that directory. When we stop doing that, these issues won't show anymore: the /tmp/ledgersmb directory then can't be owned by another user anymore...
This results in difficult to identify issues that can't be reproduced in a developers environment without knowing what's wrong to start with.
Hence excluding non owner users from running ledgersmb

But the problem isn't that people are running the LedgerSMB/Starman application! The problem will be caused by *any* Perl script/application which includes LedgerSMB/Sysconfig.pm (this is why I'm so eager to have your refactorings ;-) [hint, hint]).

I think it's unreasonable to distribute tools like "create-company-database.pl" yet prohibit users from actually using them. So, instead of coming up with a very much non-standard authorization scheme, I'd rather fix the fact that we're creating this /tmp/ledgersmb directory at all.
  1. the install directory and contents should have the following permissions
    For Executable files  (s--r-xrw---- | 4560)
    • s - - for flags   # edit to match 4560
      r - x for owner

    • r w - for group
    • - - - for other

    For Non Executable files (r--rw---- | 460)
    • r - - for owner
    • r w - for group
    • - - - for other

    For Directories (-s-r-xrw---- | 2560)
    • - s - for flags   # edit to match 2560
      r - x for owner

    • r w - for group
    • - - - for other
My idea would be that the default starman process hosting ledgersmb, runs as ledgersmb/ledgersmb-admin (perms dropped from 'root' at startup) and the files in the install dirs are all simply root:root rwxr-xr-x. That way, the starman process can read what it needs, yet it can't modify the code it's running.

If individual users want to run their own starman instances, the same applies to those instances, except that those can never run on a priviledged port (as always).
Having root as the only user that can write to files in the LedgerSMB tree means you can't allocate a non root user permissions to manage things like template and CSS files or site customizations (perhaps plugins at a later date).

That's correct. I think that on a standard system, these things are reserved exclusively for the system admin. There's no need to manage things like templates, because the templates are built into the database and that's where they're read from (not accidentally). The CSS can be changed using the config and - as you pointed out yourself earlier - tinkering with code just shouldn't be allowed.
Nor can you safely have a UI triggered update tool that doesn't run as root.
(consider the site-local translations feature we have discussed)

 That's correct. I'm not sure we should support that out-of-the-box on any system. That is: on Debian, there's a documentation file for Drupal how to enable web-based system updates. But the system updates are configured as root-only actions by default (using the command line tool "drush" to do it).
  1. The temp dir configured in ledgersmb.conf will need to be owned by user ledgersmb and group ledgersmb-admin
I would argue to the contrary: we simply shouldn't be creating a temp dir at all and for all our *temporary* files, simply use /tmp. That's what all processes do. If however we want to generate *log* files, then we should generate log files to a logging output directory with appropriate permissions.
 My personal opinion is that it's a huge mistake that we ever created a /tmp/ledgersmb directory under the user that happens to run code which includes LedgerSMB::Sysconfig.pm.
For as long as we need temp files, I can't see a way around having a temp dir that is owned by the running user that also has restrictive permissions.
All files in that dir should not be writeable by anyone except the running user, and probably should only be readable by that user too.

Ok. Back at the temp files: we currently create such a directory and due to the structure of our code, that's actually a problem: after running any program that indirectly includes LedgerSMB/Sysconfig.pm, only the invoking user of that program can ever run LedgerSMB due to the ownership of /tmp/ledgersmb. This proposal doesn't change that, but I think it should, because starting Starman isn't the only way to trigger the ownership problems.
Any files that other processes on the system may need access to under a different user should probably be rw- r-- r-- and not be in our private temp dir
Agreed, although I'm not aware of any files that we may have like that.
  1. The temp dir would need these permissions (sstrwxrwx--- | 7777)
    • s s t for flags   # edit to match 7777
    • r w x for owner
    • r w x for group
    • - - - for other
Rather, I'd simply want to use /tmp. We're not special, if it works for others, it should work for us. Don't try to reinvent the wheel, do the tried-and-tested. That both reduces the burden on us as developers, because we don't need to come up with new solutions, but it also greatly simplifies using and maintaining an installed instance as it does what admins expect.
I don't see this as reinventing the wheel at all. It's actually common practice
Many pieces of software that use /tmp sets file and dir permissions as rwx --- --- unless other user's processes may need access to those files.
Infact many files (eg: files opened directly from thunderbird mail attachments) have permissions r-- --- --- which are more restrictive again

We're currently not changing ownership on files we create in /tmp. I'm definitely *for* creating files as r-------- in /tmp. What I'm thinking just creates more problems to have the /tmp/ledgersmb dir which different users want to access.
In my proposed scheme group doesn't really need any access to the tempdir, but providing it allows easy inspection, deletion etc while debugging
  1. Any system users that need to be able to modify files in the LedgerSMB dir's would need to have ledgersmb-admin added to their groups.
Ok. But nothing should be changing the files in the LedgerSMB dirs: only software installs should. That's why the templates moved to the database in 1.4. And that's why I think 'root:root' should be Just Fine(tm). Any Starman (global/startup) configuration options can be stored in /etc/ledgersmb/<version>/... or /etc/default/ledgersmb-<version>.
See my comments above about other reasons why changes to files in the install may be desireable

What I saw above was - I think - also in the orginal mail? I.e. that you'd want to open up the option to edit CSS files and templates? I'm thinking that precedent has it (as I stated above) that this is reserved for 'root' by other web applications on the debian platform. I think it makes sense to have a default configuration which looks as much like other web applications as we can.


Keeping everything from the LedgerSMB tree in a single folder instead of splitting amoung multiple system dir locations is necessary for running multiple instances on a system, especially if there will be more than one version of the code.

Running multiple instances is probably beyond the immediate scope of trying to be easy on our users -- although admittedly, it could help some of the people that try to run the software in a hosting scenario. But can't we create a flag or some option or environment variable which indicates to Starman/LedgerSMB which config file it should read? That way, we can run as many as we want, yet still use the usual Debian/RH/... structure for spreading config files over the filesystem. ("When in Rome, do as the Romans do..."?)
Yes, having a means of telling Starman/LedgerSMB what config file to use is already planned and partly implemented in one of my local branches.
It's not the whole solution for running multiple versions/instances though. what about the case where the instances need different CSS or templates (currently templates and CSS are put in a common /var/foo directory but the .deb package)

Well, the config allows to specify both a different templates directory and a different CSS directory, so if we can specify a different config file ,then I think the scenario is covered.
Or even different source code due to a required customisation.

I think this is getting well beyond the scope of the "standard use" we should be trying to facilitate. I mean, it  shouldn't be impossible to set up such a scenario, but I don't think that anything we deliver would be required to support this scenario out-of-the-box. I'm thinking this is the ideal situation for a "from source" installation.
You earlier mentioned the possible need to use local::lib to handle different dependencies as well

Right, however, I'm wondering how likely it is that the average user needs to run a scenario where we are required to support this out of the box (either in install scripts or in a package).

I am certain there are ways to work around that, but simplicity is often the best solution. 

Things like the docs dir can have a symlink /usr/share/doc/ledgersmb-1.4.25 that point to the real dir
The same goes for manpages if we want to.
NOTE: for an official debian package (in the debian.org archives) we may have to split things up to comply with their policy,

You mention simplicity, but needing to split up packages in Debian doesn't sound like we're "going simple". Why would we need to split up the package? And if we do, is there anything we can do to make it so we don't (yet we do comply)?
I think you misread my statement here. We currently do split files out of the tree and put them in various system directories.
eg: css, and templates end up in /var/foo, the docs dir moves to /usr/share/doc/ledgersmb/

Right. I think that's how users of distributions expect things to work, although the /var/foo is probably somewhat unusual (as I noted above, other webapps store that kind of thing in /usr/share).
I think this becomes a problem when you want to have multiple instances/versions installed at the same time,
The problem is even worse if a new version is installed to replace the current one, and there are changes to what files are present where.

Well, I think that depends. In Debian the files that are expected to be change are tested for changes. However, the templates aren't expected to change, as the templates in the database will be customized, not the ones on disk. Any company which needs adjusted templates on disk can store them in /usr/local/share (or anywhere, basically) and adjust the config file to point to that location with changed templates. On update, the changed templates don't get overwritten and nothing happens (other than that the templates need to be manually adjusted/upgraded).
eg: some foo.pm is now in a different location, but the update process doesn't remove the original foo.pm

If everybody sticks to using the package system of their distro, this doesn't happen. Mixing source and package installs is disastrous. I think that's what we have seen before and that's what breaks (nearly) every time.
      We now have 2 foo.pm files, which one is the code going to use? It could easily end up being the wrong one!
I will continue to assert that we need to keep our tree intact, however to meet packaging policy on some systems we should easily be able to provide symlinks in system dir's for things like /usr/share/docs/ledgersmb/1.4 to point to /usr/share/ledgersmb-1.4/doc

I hate when a package doesn't adhere my distro's standard locations because it costs me (lots of) extra time to understand how it *does* work, if not just the standard way. I think we should *not* reason from our own perspective, but instead reason from the user who has experience with their distro (hopefully), but not yet with LedgerSMB.

but there is precedent not to.
Also as debian doesn't allow version updates to packages for the life of a release Official Packages for LedgerSMB are of minimal benefit.
We are better to point people at our own repository.

Ok. We've been over that on IRC: I personally think that there's no problem to provide packages through the official repository. I've been using a 5 years old Subversion client just last week and it worked perfectly fine for my uses. If I want something newer, I can install the version from backports. I think we should agree to disagree, hoping that others chip in on the subject.
Yes, I think we will have to agree to disagree here ;-)
Mainly as I don't thing the OOB first time user experience helps our image if they get a 5 yearold version when they first install.

Ok. I think the age of the software doesn't matter much, especially when users know their system is getting old (and if they don't know it themselves, their admin should be able to tell them). The bad experiences come when the software doesn't "Just Work"(tm). And if it doesn't, it should at least be fixable within a reasonable amount of time -- which means that the admin should be able to rely on his knowledge of how his distro organizes things.
Even worse as happened a couple of days ago if that old version had a significant (albeit trivial) bug that the user hit within a very short time.

But that may even happen with software from our own repository. And we can point people to use our own repo after that happens. Really: Gajim (my chat client) is included in Wheezy and Jessie -- yet they have their own repo with the latest release and nightlies. Although for a chat client, using nightlies is less problematic than using nightlies for an accounting application.
If backports was seamless to use, and enabled by default on a normal debian system I'd agree with you, but otherwise <shrug>
Actually, it is, nowadays. All you need to do is "aptitude install -t <distro-name>-backports <package name>". No need to change the apt sources anymore, I think.



http://efficito.com -- Hosted accounting and ERP.
Robust and Flexible. No vendor lock-in.
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
Ledger-smb-devel mailing list