trove updates /etc/fstab in an insecure way

[Dirk Mueller]

trove/guestagent/volume.py:

    def write_to_fstab(self):
        fstab_line = ("%s\t%s\t%s\t%s\t0\t0" %
                      (self.device_path, self.mount_point, self.volume_fstype,
                       self.mount_options))
        LOG.debug("Writing new line to fstab:%s" % fstab_line)
        utils.execute("sudo", "cp", "/etc/fstab", "/etc/fstab.orig")
        utils.execute("sudo", "cp", "/etc/fstab", "/tmp/newfstab")
        utils.execute("sudo", "chmod", "666", "/tmp/newfstab")
        with open("/tmp/newfstab", 'a') as new_fstab:
            new_fstab.write("\n" + fstab_line)
        utils.execute("sudo", "chmod", "640", "/tmp/newfstab")
        utils.execute("sudo", "mv", "/tmp/newfstab", "/etc/fstab")

There are multiple problems with this approach:

- /tmp/newfstab is a predictable file name in a world writeable directory
- the chmod 666 is racy, other competing processes can update the file inbetween
- no locking against concurrency

[Thierry Carrez (ttx)]

Trove has not been released as part of openstack yet so we won't issue an OSSA for this... but this needs to be fixed.

I think making this public is fine, should make the fix easier and faster. Thoughts ?

[Michael Basnight (hubcap)]

+1. im ok w/ making it public.

[Thierry Carrez (ttx)]

Dirk, you fine with opening this up ?

[Dirk Mueller (dmllr)]

Hi Thierry,

Sure, opened it up.

Greetings,
Dirk
 Am 24.01.2014 20:10 schrieb "Thierry Carrez" <email address hidden>:

> Dirk, you fine with opening this up ?
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1267991
>
> Title:
> trove updates /etc/fstab in an insecure way
>
> Status in OpenStack Security Advisories:
> Incomplete
> Status in Trove - Database as a Service:
> New
>
> Bug description:
> trove/guestagent/volume.py:
>
> def write_to_fstab(self):
> fstab_line = ("%s\t%s\t%s\t%s\t0\t0" %
> (self.device_path, self.mount_point,
> self.volume_fstype,
> self.mount_options))
> LOG.debug("Writing new line to fstab:%s" % fstab_line)
> utils.execute("sudo", "cp", "/etc/fstab", "/etc/fstab.orig")
> utils.execute("sudo", "cp", "/etc/fstab", "/tmp/newfstab")
> utils.execute("sudo", "chmod", "666", "/tmp/newfstab")
> with open("/tmp/newfstab", 'a') as new_fstab:
> new_fstab.write("\n" + fstab_line)
> utils.execute("sudo", "chmod", "640", "/tmp/newfstab")
> utils.execute("sudo", "mv", "/tmp/newfstab", "/etc/fstab")
>
> There are multiple problems with this approach:
>
> - /tmp/newfstab is a predictable file name in a world writeable directory
> - the chmod 666 is racy, other competing processes can update the file
> inbetween
> - no locking against concurrency
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/ossa/+bug/1267991/+subscriptions
>

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/69125

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/69125
Committed: https://git.openstack.org/cgit/openstack/trove/commit/?id=e966dd4881b858d05fb76efc2af9a0ce118657ed
Submitter: Jenkins
Branch: master

commit e966dd4881b858d05fb76efc2af9a0ce118657ed
Author: Sushil Kumar <email address hidden>
Date: Sat Jan 25 12:17:41 2014 +0000

    Fixes insecure update of /etc/fstab file

    Reasons:
    - While updating /etc/fstab in write_to_fstab in
      trove/guestagent/volume.py, the temporary file-name was easy to
      guess and had a permanent name making it prone to unwanted access.
    - The temporary file-permission was set to 666, making it insecure.

    Changes:
    - Uses NamedTemporaryFile for temporary file creation.
    - Used file reading instead of copying the file.
    - These changes eliminate the chances of guessing the file-name,
      as well as updating the same.

    Change-Id: I07c3980e40fe218d35605a851a17535cd3972c11
    Closes-Bug: #1267991