Use of NamedTemporaryFile creates rings with restricted permissions

Bug #1302700 reported by James Page
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Object Storage (swift)
Undecided
James Page
swift (Ubuntu)
High
Unassigned
Trusty
High
Unassigned

Bug Description

https://github.com/openstack/swift/commit/c6cebb6e621a245c9c2d5bff0df59689b0140373 introduced the use of NamedTemporaryFile when writing rings; this creates files with 0600 permissions by default, so when the file is renamed to the actual ring filename, it has different permissions to those created by earlier versions of swift.

We use the root account to create rings, but swift-proxy runs as the swift user and as a result can't read the rings.

Revision history for this message
James Page (james-page) wrote :

Example:

root@armstrong:/etc/swift# ls -lrt
total 2088
-rw-r--r-- 1 root root 2473 Apr 4 07:44 swift.conf
-rw-r--r-- 1 root root 16147 Apr 4 07:44 proxy-server.conf
-rw-r--r-- 1 root root 417 Apr 4 07:54 container.builder
-rw-r--r-- 1 root root 417 Apr 4 07:54 object.builder
drwxr-xr-x 2 root root 4096 Apr 4 07:54 backups
-rw------- 1 root root 1725 Apr 4 07:54 account.ring.gz
-rw-r--r-- 1 root root 2099746 Apr 4 07:54 account.builder

Revision history for this message
James Page (james-page) wrote :

I'm using the 1.13.1 rc1 for Icehouse:

swift:
  Installed: 1.13.1~rc1-0ubuntu1
  Candidate: 1.13.1~rc1-0ubuntu1
  Version table:
 *** 1.13.1~rc1-0ubuntu1 0
        500 http://archive.ubuntu.com/ubuntu/ trusty-proposed/universe amd64 Packages
        100 /var/lib/dpkg/status
     1.13.0-0ubuntu1 0
        500 http://archive.ubuntu.com/ubuntu/ trusty/universe amd64 Packages

description: updated
tags: added: icehouse-rc-potential
description: updated
Revision history for this message
James Page (james-page) wrote :

Reading the comments in the commit details, I don't think limiting permissions was in the scope of intent of that change.

James Page (james-page)
Changed in swift:
assignee: nobody → James Page (james-page)
status: New → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift (master)

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

James Page (james-page)
Changed in swift (Ubuntu Trusty):
importance: Undecided → High
status: New → Triaged
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package swift - 1.13.1~rc1-0ubuntu2

---------------
swift (1.13.1~rc1-0ubuntu2) trusty; urgency=medium

  * d/p/ring-perms.patch: Ensure that generated rings can be read by
    the swift user, fixing autopkgtest failure (LP: #1302700).
 -- James Page <email address hidden> Sun, 06 Apr 2014 21:10:51 +0100

Changed in swift (Ubuntu Trusty):
status: Triaged → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to swift (master)

Reviewed: https://review.openstack.org/85546
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=b9b5fef89af51c66905de33e2436c063f4b09d36
Submitter: Jenkins
Branch: master

commit b9b5fef89af51c66905de33e2436c063f4b09d36
Author: James Page <email address hidden>
Date: Sat Apr 5 09:38:12 2014 +0100

    Set permissions on generated ring files

    The use of NamedTemporaryFile creates rings with permissions 0600;
    however most installs probably generate the rings as root but the
    swift-proxy runs as user swift.

    Set the permissions on the generated ring to 0644 prior to rename so
    that the swift user can read the rings.

    Change-Id: Ia511931f471c5c9840012c3a75b89c1f35b1b245
    Closes-Bug: #1302700

Changed in swift:
status: In Progress → Fix Committed
Thierry Carrez (ttx)
tags: added: icehouse-backport-potential
removed: icehouse-rc-potential
Revision history for this message
Openstack Gerrit (openstack-gerrit) wrote : Fix proposed to swift (feature/ec)

Fix proposed to branch: feature/ec
Review: https://review.openstack.org/88359

Revision history for this message
Openstack Gerrit (openstack-gerrit) wrote : Fix merged to swift (feature/ec)
Download full text (8.0 KiB)

Reviewed: https://review.openstack.org/88359
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=282548d538153fa2fae1d2a4ec19d75dc2a41028
Submitter: Jenkins
Branch: feature/ec

commit 856c15539a958dabe3b8a1f22d305048ca39de9a
Author: Paul Luse <email address hidden>
Date: Mon Apr 14 15:19:22 2014 -0700

    Fix testcase test_print_db_info_metadata()

    Test compares cluster info to hardcoded expected data and wasn't
    sorting the two sets of things being compared leading to some
    sporadic unit test failures.

    Change-Id: I3ef98260a62c15d06ba8cc196196d4e90abca3f0

commit 67fff5b2973f5ca12316041664eddf3e8ece45be
Author: Madhuri Kumari <email address hidden>
Date: Fri Apr 11 11:53:48 2014 +0530

    Print 'Container Count' in data base info

    Currently, 'Container Count' was missing in data base info.
    So this patch will help printing 'Container Count' also.

    Change-Id: I1ca80ee79e71b086b30fd2d1ab024ea1cfb324f5

commit deb0dfd090cee2feef664bdd4449194c274d830e
Author: Peter Portante <email address hidden>
Date: Thu Apr 10 14:36:43 2014 -0400

    Add includes of referenced SAIO bin scripts

    Change-Id: I6810e69a757336a3aed0a38146c27f270fe2dde1

commit 2c2ede22338d6ca9637233076157f1d26248fbf1
Author: Chuck Thier <email address hidden>
Date: Thu Apr 10 18:59:01 2014 +0000

    Fix logging issue when services stop on py26

    On older versions of python 2.6, exceptions would be spewed to the error
    log whenever a service would stop. This gets magnified by the
    container-updater which seems to do it with every pass. This catches
    and squelches the error.

    Change-Id: I128c09c240e768e8195af1f6fe79b10d4e432471
    Closes-Bug: #1306027

commit 5ff6a4d5d6cdbf39ba9f6d5d416cdd1c6c6a52ac
Author: Peter Portante <email address hidden>
Date: Mon Apr 7 13:01:44 2014 -0400

    Use eventlet instead of threading for timeout

    The only explicit use of Python threading is found in the
    testFileSizeLimit test. Using eventlet seems a bit easier to follow,
    accomplishing the same goal, and does not constrain us to a
    multi-threaded environment.

    The chunks() and timeout() module level functions are only used by one
    test each, so we just move them to those tests to indicate they are not
    used globally.

    Change-Id: I50b9fb798fbfd1d552b3c3f90309f6b86da34853

commit 58fe2f256fccc14f5078e9fde9b9f7b2219a06e5
Author: anc <email address hidden>
Date: Tue Apr 8 18:44:06 2014 +0100

    Add tests and comments re constraints in /info

    Add test to check that only the expected keys are
    reported by proxy in /info, and add comments to
    raise awareness that default constraints will be
    automatically published by proxy in response to /info
    requests.

    Change-Id: Ia5f6339b06cdc2e1dc960d1f75562a2505530202

commit c2744caac43586b745c43c37a9c31483f7a126fc
Author: Samuel Merritt <email address hidden>
Date: Tue Apr 8 11:44:58 2014 -0700

    Fix deprecation warning

    Accessing BaseException.message spews a warning; we can get the same
    information with str(err), which does not spew.

    Change-Id: I67...

Read more...

Thierry Carrez (ttx)
Changed in swift:
status: Fix Committed → Fix Released
milestone: none → 2.0.0
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers