Bug 1150555 - AUDIT-1: storeBackup: review of cron job file(s): /etc/cron.daily/storebackup
AUDIT-1: storeBackup: review of cron job file(s): /etc/cron.daily/storebackup
Status: RESOLVED FIXED
Classification: openSUSE
Product: openSUSE Tumbleweed
Classification: openSUSE
Component: Security
Current
Other Other
: P5 - None : Normal (vote)
: ---
Assigned To: Security Team bot
E-mail List
:
Depends on:
Blocks: 1150175
  Show dependency treegraph
 
Reported: 2019-09-12 12:04 UTC by Matthias Gerstner
Modified: 2020-01-30 11:14 UTC (History)
4 users (show)

See Also:
Found By: ---
Services Priority:
Business Priority:
Blocker: ---
Marketing QA Status: ---
IT Deployment: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Matthias Gerstner 2019-09-12 12:04:59 UTC
+++ This bug was initially created as a clone of Bug #1150175
As discussed in the proactive security team we want to restrict the
installation of cron job files in the future. To achieve this we first need to
cover the currently existing packages that do this.

storeBackup installs a cron file in /etc/cron.daily/storebackup. It should be
reviewed and whitelisted if all is well.
Comment 1 Matthias Gerstner 2019-11-14 12:00:17 UTC
I've looked into this cron job. By default it's safe, because it only performs
any action if configuration files are placed in /etc/storebackup.d. The rest
fully depends upon the individual configuration files placed there by an
Administrator.

It would probably be a bad idea to backup directories under user control with
storeBackup, because difficult to control race conditions are involved in this
case. storeBackup is written in perl, consists of over 30.000 lines of code
with the main perl script counting 8.000 lines alone. So it's difficult to
perform a full code review. The last release was in 2014 and upstream seems to
consist of a single author, there's no upstream repository and infrastructure.
So it looks rather unmaintained currently.

The cron job can be whitelisted in this form but we should consider whether
keeping such outdated packages around is generally a good idea.
Comment 2 Jan Ritzerfeld 2019-11-14 17:28:40 UTC
(In reply to Matthias Gerstner from comment #1)
> [...] 
> It would probably be a bad idea to backup directories under user control with
> storeBackup, because difficult to control race conditions are involved in
> this case.

I have to object: Almost any of my backups done by storeBackup are of directories that are under my control as a user. Obviously, this is a manual process and you have to log out or stop all writing background manually. Furthermore, I don't want my backup destination volume to be online all the time (accidentally deleting them or getting them encrypted by a malicious attack). Besides, I want it to be encrypted, too, like the backup sources.

Personally, I never used the cron based automatic backup feature but only did basic testing for SR regarding this feature. Thus, I'm biased towards this use case and, as the package maintainer, would be totally happy to drop cron support completely rather than putting any effort into fixing it. 

> storeBackup is written in perl, consists of over 30.000 lines of
> code with the main perl script counting 8.000 lines alone. So it's difficult
> to perform a full code review. 

Indeed.

> The last release was in 2014 and upstream seems
> to consist of a single author, there's no upstream repository

Unfortunately, this seems to be true.

> and infrastructure.

https://savannah.nongnu.org/projects/storebackup/
On the plus side, it has an excellent German and English documentation.

> So it looks rather unmaintained currently.

There was activity of the author last year in
https://savannah.nongnu.org/support/?group=storebackup
and
https://savannah.nongnu.org/bugs/?group=storebackup

> The cron job can be whitelisted in this form but we should consider whether
> keeping such outdated packages around is generally a good idea.

I see. However, it is mature software that just works. There are only a few open bugs and personally I would have closed most of them.
Comment 3 Matthias Gerstner 2019-11-15 09:32:56 UTC
(In reply to suse@bugs.jan.ritzerfeld.org from comment #2)
> (In reply to Matthias Gerstner from comment #1)
> > [...] 
> > It would probably be a bad idea to backup directories under user control with
> > storeBackup, because difficult to control race conditions are involved in
> > this case.
> 
> I have to object: Almost any of my backups done by storeBackup are of
> directories that are under my control as a user.

Maybe we're not quite on the same page. I didn't want to say that it won't
funtionally work to do this. I only wanted to express that it won't be a
secure way of using storeBackup. At least not if you run storeBackup as the
'root' user to backup files that are under user control.

> Personally, I never used the cron based automatic backup feature but only
> did basic testing for SR regarding this feature. Thus, I'm biased towards
> this use case and, as the package maintainer, would be totally happy to drop
> cron support completely rather than putting any effort into fixing it. 

The cron job is not actually a big problem at the moment. In the security team
we're mostly concerned that the default installation/configuration of package
doesn't introcude security problems. Since this cron job does nothing by
default there's nothing to worry about.

> > The cron job can be whitelisted in this form but we should consider whether
> > keeping such outdated packages around is generally a good idea.
> 
> I see. However, it is mature software that just works. There are only a few
> open bugs and personally I would have closed most of them.

I won't pressure to remove it. I just want to make sure we don't ship more or
less broken packages that aren't cared for any more. Since you seem to be an
active maintainer (thank you for that!) and since there are at least some
people that still use it I'm fine with keeping it. As long as no security
nightmares are involved ;-).
Comment 4 Jan Ritzerfeld 2019-11-15 10:35:47 UTC
(In reply to Matthias Gerstner from comment #3)
> [...] 
> Maybe we're not quite on the same page. I didn't want to say that it won't
> funtionally work to do this.

Perfect. Looks like we're back on the same page. :)

> I only wanted to express that it won't be a
> secure way of using storeBackup. At least not if you run storeBackup as the
> 'root' user to backup files that are under user control.

Yes, and you have to hope that the user isn't altering their files when the cron job starts storeBackup.
 
> [...]
> The cron job is not actually a big problem at the moment. In the security
> team we're mostly concerned that the default installation/configuration of
> package doesn't introcude security problems. Since this cron job does nothing
> by default there's nothing to worry about.

Many thanks for that: I wouldn't be happy either if a package I maintain made systems vulnerable by just installing it.

> [...] 
> I won't pressure to remove it. I just want to make sure we don't ship more or
> less broken packages that aren't cared for any more. Since you seem to be an
> active maintainer (thank you for that!) and since there are at least some
> people that still use it I'm fine with keeping it. As long as no security
> nightmares are involved ;-).

That's perfectly sensible. Thank you. And time will tell. Fixing bugs as a maintainer isn't always the best idea (Debian openssl)...
Comment 5 Matthias Gerstner 2020-01-30 11:14:55 UTC
So the CVE in bug 1156767 is fixed in Factory and updates for Leap have been submitted. Thank you for your efforts here!

I'm decoupling this cron job review bug here from the CVE bug (which needs to stay open a bit longer for tracking by the reactive security team).

I will submit a whitelisting for the storeBackup cron job soon and therefore I'm closing this bug as FIXED.