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

On 03/04/16 02:58, 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

Only ever run LedgerSMB using Starman (High-performance preforking PSGI/Plack web server)

Only ever bind Starman to localhost

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

I'm not so sure about that, In my experience requiring (or recommending) the httpd virtual package results in getting apache installed by default.
I haven't investigated to see if this is a general limitation of the packaging systems or with the way the packages have been built, but I suspect the former.
The only real certain method I can think of is to leave the specific httpd packages we support as suggests, and in our package install scripts make sure one of them gets installed.

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.

Yep, I'd prefer to use starman's ability to drop privileges as that allows the use of privileged and unprivileged ports, while using systemd to set the user to run as only allows unprivileged ports to be used.

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.

Yep

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.

Hmm, I'd say /usr/share/doc/ledgersmb/* would be an odd location to put these files, as a sysadmin I'd expect to find documentation there (or links to it) that is not essential to the uperation of the software, not content that is required for the software to function.
as for /usr/share/ledgersmb/* if that is the root of the install tree then yep, I'd agree it is a sensible location, but if it's not then we need a generic "make install" process that can by used by all platforms to "copy" our templates across. This really shouldn't be handled in different ways by different packages (treating a tarball or source install as package types)
As for the permissions, these files are directly web presented data, I'm surprised that root:root is what is used.
apache and other httpd's have traditionally (by default) required all served files are owned by the running user (www-data) and not editable by that user.

If we refer to this article on serverfault.com in-particular the section "You can have your cake and eat it too" (you will need to read the previous section as well) there is a nice explanation of why I'm proposing the permissions that I am. ie r-x rwx ---

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.

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.

Ok, so I can't actually think of a way to eliminate the need for a temp dir, some things simply need to be written to file for a while. (eg: tempfiles during a print process, or PDF creation)

As for tools like "create-company-database.pl" (which by the way doesn't exist) the proposed scheme would no more prevent it being run than having it owned by root.
The running user is still going to need to get elevated privileges somehow, our current "delete-company-database.sh" script uses "su" for this, which probably should be fixed to not internally get elevated permissions, but let the executing user do so as there are a variety of ways such as sudo, calife, userv to name a few of at least 10 open source options, and there are commercial options out there as well.
Basically yes, at times we will need privilege escalation, but that should not be done by us running as a normal user, instead the user should use system specific tools to gain elevated privileges to run the script.
If we port all of our scripts to perl instead of bash then we can be smart and drop privs for everything that doesn't require them in a system agnostic way.

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

Agreed for system updates (including ledgersmb updates) but things like translations and potentially plugins (when they are supported and written) are a common thing to do as a "software" admin user.

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.

I'd think that there are issues with our code structure yes. but on the other hand I can't see a way around creating a temp dir.
Making it so that ALL programs / scripts that are in the LedgerSMB tree HAVE to be run as the Installed User solves the problem.
It becomes impossible for the temp dir to be owned by anyone else that way.

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.

Temp Files used for printing probably come under that heading.

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.

Ok, so we agree that we need a process specific user that owns our files in /tmp and that those files should generally be r-- --- ---
You're thinking a /tmp/ledgersmb dir makes things complicated as if there are multiple users wanting to put files in that dir we get permissions issues.
I would propose that we actually want to have our temp dir's named more like....
    /tmp/ledgersmb-$user-$port/
where
    $user is the running user
    $port is the starman port assigned to that instance
Which incidentally is the same naming scheme I'd suggest for ledgersmb.conf (ie: ledgersmb-$user-$port.conf) for multi instance systems
(keeping in mind that we will be able to pass a config file path/name to the process in an env var)

In my proposed scheme group doesn't really need any access to the tempdir, but providing it allows easy inspection, deletion etc while debugging

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.

Yep, I'm happy with needing to allow template, and CSS to exist in another user specified dir, but only as a manual config change done by an admin, and really we shouldn't care or even specify where that will be.
Unless the config option is explicitly set in ledgersmb.conf we should use our built in files (which stay in tree)
So, the use of ledgersmb-admin would not normally be needed, except for system users that have a *specific* need such as local customizations etc
Using this group allows that to be done without requiring root access (which any other method would)

[snip]

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 don't know the answer to that, but I do know that the current method of installing perl modules nor available from the system repo (ie: using cpan) can easily go wrong, if the user chooses the wrong cpan config option they can be installed in system perl dir's instead of local dirs. this is a know cause of system wide perl issues if the version of a module being installed clashes with other uses on the system.

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

I'm not certain that all packaging systems correctly remove old files without a full uninstall, possibly even a purge.
In our existing debian packages a remove only handles running "apache2_invoke disconf ledgersmb.conf"
Which may leave traces of an old system behind.

Also our existing .deb packages do not handle templates and CSS in the manner you describe.
Instead of testing for changes and automatically replacing teh files if there is no change, or prompting the user to make a decision if there are changes,
the current packages keep the old versions of the entire templates, and CSS directories without exception.
I will guarantee that there are existing systems out there that have been installed and updated via our .deb's that are using CSS and templates that do not match with the appropriate source files.
We at least need to handle this as part of the package install by setting all of these files as config files so the deb packaging tools deal with it for us (which only works if we keep the files in tree I believe) or probably better handle it by commenting out any custom locations for these files (defaulting to the in tree shipped version) but provide a warning in both setup.pl and above the Main Menu saying that local customizations are not being used

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.

I'm not so sure about sticking with packaging systems preventing this.
If our search path for a .pm during execution is /usr/share/local/ledgersmb:/user/share/ledgersmb and any old package installed files in /usr/share/local/ledgersmb but the current package installs in /usr/share/ledgersmb we have a problem.
If the original package is properly removed before the new package is installed then it's only a problem if local customizations have been made.
But, if multiple instances have been setup (eg: 1.4.25 and 1.4.26) that have a similar change made we, once again run into this issue.

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.

I agree, but, my suggested fix is to use symlinks to expose things in locations that a user/admin may expect to find them instead of actually moving our files.
This is a method that has been used by other packages.
The exception to this would be templates and CSS which I still think should stay in tree but via the config options a local copy could be provided out of tree if customizations are to be done locally.

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.

To use backports isn't quite that seemless, looking at
http://backports.debian.org/Instructions/
You need to add the sources entry for $release-backports, and possibly even $release-backports-sloppy as well
Doing a dist-upgrade will require manual fixes to those entries also (from memory backports just get commented out on dist-upgrade)
It's unclear exactly what happens when a new package version is added to backports, It used to be that you had to manually install the upgrade by running apt -t... again, although that may have been resolved in wheezy and later