[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 Erik,

On 02/04/16 06:42, Erik Huelsmann wrote:
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
  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
Using the separate namespace (ie: not nobody:nogroup) solves the security issue created if temp files are writeable by other processes on the system
  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. 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
  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
  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.

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
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
  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).
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)
 
  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.
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
  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
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

Running LedgerSMB using Starman and a reverse caching proxy like nginx has significant performance benefits due to both the way Starman preforks and handles Perl assets, and the static asset caching offered by the proxy.

Yup.

There is an additional benefit to running via Starman, it is less likely that an error in a latex template (for example) will cause a hard failure.
This has recently been shown to be an issue with the current debian package for 1.4.25 available from our repository http://apt.ledgersmb.org

Well... That discards the experience John had that for a long time he *could* execute his business processes on CGI, but not on Starman. So, the other way around is true as well. At this time, I personally develop using Starman, so bugs are likely called out the earliest on the Starman install. However, I think we probably want to create a Travis CI test setup which specifically tests the CGI setup, if we want to support that as a secondary option, still.
That way, errors get caught early enough not to affect releases.

Running as a specific system user improves security, and assists with running multiple instances on the same system.
The restricted permissions take this a step further to make it harder for things to be broken or attacked by other processes on the system.

True, but if 'root:root rwxr-xr-x' doesnt' do enough for Perl code?

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)
Or even different source code due to a required customisation.
You earlier mentioned the possible need to use local::lib to handle different dependencies as well

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/
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.
eg: some foo.pm is now in a different location, but the update process doesn't remove the original foo.pm
      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

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.
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.
If backports was seemless to use, and enabled by default on a normal debian system I'd agree with you, but otherwise <shrug>


Erik.

Regards
David G
------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785471&iu=/4140
_______________________________________________
Ledger-smb-devel mailing list
..hidden..
https://lists.sourceforge.net/lists/listinfo/ledger-smb-devel