re-runs self via sudo

Bug #1611171 reported by Seth Arnold on 2016-08-09
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Cinder
Undecided
Pallavi
Designate
High
Dr. Jens Harbott
Manila
Undecided
Tom Barron
OpenStack Compute (nova)
Medium
Markus Zoeller (markus_z)
Newton
Medium
Lee Yarwood
OpenStack Security Advisory
Undecided
Unassigned
Rally
Undecided
iswarya vakati
ec2-api
Undecided
iswarya vakati
gce-api
Undecided
iswarya vakati
masakari
Undecided
Takashi Kajinami

Bug Description

Hello, I'm looking through Designate source code to determine if is appropriate to include in Ubuntu Main. This isn't a full security audit.

This looks like trouble:

./designate/cmd/manage.py

def main():
    CONF.register_cli_opt(category_opt)

    try:
        utils.read_config('designate', sys.argv)
        logging.setup(CONF, 'designate')
    except cfg.ConfigFilesNotFoundError:
        cfgfile = CONF.config_file[-1] if CONF.config_file else None
        if cfgfile and not os.access(cfgfile, os.R_OK):
            st = os.stat(cfgfile)
            print(_("Could not read %s. Re-running with sudo") % cfgfile)
            try:
                os.execvp('sudo', ['sudo', '-u', '#%s' % st.st_uid] + sys.argv)
            except Exception:
                print(_('sudo failed, continuing as if nothing happened'))

        print(_('Please re-run designate-manage as root.'))
        sys.exit(2)

This is an interesting decision -- if the configuration file is _not_ readable by the user in question, give the executing user complete privileges of the user that owns the unreadable file.

I'm not a fan of hiding privilege escalation / modifications in programs -- if a user had recently used sudo and thus had the authentication token already stored for their terminal, this 'hidden' use of sudo may be unexpected and unwelcome, especially since it appears that argv from the first call leaks through to the sudo call.

Is this intentional OpenStack style? Or unexpected for you guys too?

(Feel free to make this public at your convenience.)

Thanks

information type: Private Security → Public Security
Kiall Mac Innes (kiall) wrote :

Wow. This should be removed, and past me given a stern talking to for +2'ing the change that snuck this in.

Changed in designate:
status: New → Confirmed

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

Changed in designate:
assignee: nobody → Kiall Mac Innes (kiall)
status: Confirmed → In Progress
Kiall Mac Innes (kiall) wrote :

For reference, this snuck in via https://review.openstack.org/#/c/98122/

Changed in designate:
importance: Undecided → High

Since this report concerns a possible security risk, an incomplete security advisory task has been added while the core security reviewers for the affected project or projects confirm the bug and discuss the scope of any vulnerability along with potential solutions.

It seems like a class D type of bug (e.g., hardening opportunity) according to VMT taxonomy ( https://security.openstack.org/vmt-process.html#incident-report-taxonomy ).

Changed in ossa:
status: New → Incomplete
Sean Dague (sdague) wrote :

This goes *way* back - https://review.openstack.org/#/c/3777/ - it was definitely an intentional feature at the time.

@sdague: I have a change for Nova for review at https://review.openstack.org/#/c/352866/

Changed in nova:
status: New → In Progress
assignee: nobody → Markus Zoeller (markus_z) (mzoeller)
Kiall Mac Innes (kiall) wrote :

@sdague - Interesting that the first comment on the review was security related. Looking at the original bug - bug #805695 - I suspect the reasoning for this was logfiles ending up as owned by the root user, so sudo'ing to nova meant they were created as the correct use.

I'm left wondering how this affects services today, do we still have this issue? I've marked my Designate patch WIP as I'd like to walk through that possibility and see what other use cases shake out from others on this bug.

(That said - I'd very much like to remove this, there rarely a good reason to sudo on behalf of a user like this, and it's a pet peeve of mine with the typical `curl | bash` quick installs - that and running apt-get install with --yes without asking me ;))

Changed in masakari:
assignee: nobody → SamP (sampath-priyankara)

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

Changed in masakari:
assignee: SamP (sampath-priyankara) → Takashi Kajinami (kajinamit)
status: New → In Progress

Reviewed: https://review.openstack.org/368319
Committed: https://git.openstack.org/cgit/openstack/masakari/commit/?id=53d9c2613d734a48b0f0b30944bfd47ef5c1b06f
Submitter: Jenkins
Branch: master

commit 53d9c2613d734a48b0f0b30944bfd47ef5c1b06f
Author: Takashi Kajinami <email address hidden>
Date: Tue Sep 6 11:07:23 2016 +0900

    Don't attempt to escalate masakari-manage privileges

    Remove code which allowed masakari-manage to attempt to escalate
    privileges so that configuration files can be read by users who
    normally wouldn't have access, but do have sudo access.

    NOTE:
    This change is create based on the change with change id
    I03063d2af14015e6506f1b6e958f5ff219aa4a87 from Kiall Mac Innes
    in designate project.

    Change-Id: Icba07a4bac4f41b921984204b32ad73fdbae4097
    Co-Authored-By: Kiall Mac Innes <email address hidden>
    Closes-Bug: 1611171

Changed in masakari:
status: In Progress → Fix Released
Changed in ec2-api:
assignee: nobody → iswarya vakati (v-iswarya)
Changed in manila:
assignee: nobody → iswarya vakati (v-iswarya)
Pallavi (pallavi-s) on 2016-09-17
Changed in cinder:
assignee: nobody → Pallavi (pallavi-s)
Changed in gce-api:
assignee: nobody → Pallavi (pallavi-s)
Changed in rally:
assignee: nobody → iswarya vakati (v-iswarya)

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

Changed in cinder:
status: New → In Progress

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

Changed in gce-api:
status: New → In Progress

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

Changed in manila:
status: New → In Progress

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

Changed in rally:
status: New → In Progress

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

Changed in ec2-api:
status: New → In Progress
Ben Swartzlander (bswartz) wrote :

I don't see the security hole here. I agree the code looks suspicious but after reading the reasoning behind the original change, it looks sane to me. Can someone explain how this is even conceivably exploitable?

Graham Hayes (grahamhayes) wrote :

Its not exploitable, its just really bad practice.

A command should not elevate its privileges without being explicitly told to do so.

The discussion from the original change basically boils down to "I don't know what user my config files are owned by, and / or I do not know how to run sudo"

Ben Swartzlander (bswartz) wrote :

@Graham okay makes sense to me

Jeremy Stanley (fungi) wrote :

Consensus seems to confirm Tristan's observation this meets the VMT's class D report (security hardening) definition, so I'm marking our advisory task Won't Fix and annotating the bug status and tags accordingly. If the situation is discovered to be explicitly vulnerable after all, we can revisit it at that time.

Changed in ossa:
status: Incomplete → Won't Fix
information type: Public Security → Public
tags: added: security
tags: added: newton-backport-potential

Reviewed: https://review.openstack.org/371915
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=a2839788e4ff74aa083c71c35755bc80579f17bb
Submitter: Jenkins
Branch: master

commit a2839788e4ff74aa083c71c35755bc80579f17bb
Author: pallavi <email address hidden>
Date: Sat Sep 17 16:28:24 2016 +0530

    Don't attempt to escalate cinder-manage privileges

    Remove code which allowed cinder-manage to attempt to escalate
    privileges so that configuration files can be read by users who
    normally wouldn't have access, but do have sudo access.

    Change-Id: Ibdfe5dfbe27856689408987f62d145dfd380fb90
    Closes-Bug: 1611171

Changed in cinder:
status: In Progress → Fix Released

Reviewed: https://review.openstack.org/352866
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=87530b6e674750ab0d55b70cce4d96bf26d1f49a
Submitter: Jenkins
Branch: master

commit 87530b6e674750ab0d55b70cce4d96bf26d1f49a
Author: Markus Zoeller <email address hidden>
Date: Tue Aug 9 13:55:54 2016 +0200

    Don't attempt to escalate nova-manage privileges

    Remove code which allowed nova-manage to attempt to escalate
    privileges so that configuration files can be read by users who
    normally wouldn't have access, but do have sudo access.

    The privilege escalation came into nova-manage with commit e9fd01e
    to solve bug 805695. That bug report didn't describe a faulty behavior
    but a change request.

    NOTE: This is related to change I03063d2 from Kiall Mac Innes who did
    this for the "designate" project. I'm reusing the change-id from his
    change to make it clear that they are related to each other.

    NOTE: I removed the try-except block completely, as it doesn't make
    sense to continue when we cannot read the config file (due to a wrong
    path or permission errors). That's the same approach we used in the
    recent "nova/cmd/policy_check" module.
    https://github.com/openstack/nova/blob/master/nova/cmd/policy_check.py#L158

    Co-Authored-By: Kiall Mac Innes <email address hidden>
    Closes-Bug: 1611171
    Change-Id: I03063d2af14015e6506f1b6e958f5ff219aa4a87

Changed in nova:
status: In Progress → Fix Released
Matt Riedemann (mriedem) on 2016-10-14
Changed in nova:
importance: Undecided → Medium

Reviewed: https://review.openstack.org/385365
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=28e618921c5bf63b0d7d17fcbe3084fdda153997
Submitter: Jenkins
Branch: stable/newton

commit 28e618921c5bf63b0d7d17fcbe3084fdda153997
Author: Markus Zoeller <email address hidden>
Date: Tue Aug 9 13:55:54 2016 +0200

    Don't attempt to escalate nova-manage privileges

    Remove code which allowed nova-manage to attempt to escalate
    privileges so that configuration files can be read by users who
    normally wouldn't have access, but do have sudo access.

    The privilege escalation came into nova-manage with commit e9fd01e
    to solve bug 805695. That bug report didn't describe a faulty behavior
    but a change request.

    NOTE: This is related to change I03063d2 from Kiall Mac Innes who did
    this for the "designate" project. I'm reusing the change-id from his
    change to make it clear that they are related to each other.

    NOTE: I removed the try-except block completely, as it doesn't make
    sense to continue when we cannot read the config file (due to a wrong
    path or permission errors). That's the same approach we used in the
    recent "nova/cmd/policy_check" module.
    https://github.com/openstack/nova/blob/master/nova/cmd/policy_check.py#L158

    Co-Authored-By: Kiall Mac Innes <email address hidden>
    Closes-Bug: 1611171
    Change-Id: I03063d2af14015e6506f1b6e958f5ff219aa4a87
    (cherry picked from commit 87530b6e674750ab0d55b70cce4d96bf26d1f49a)

Changed in gce-api:
assignee: Pallavi (pallavi-s) → iswarya vakati (v-iswarya)

Reviewed: https://review.openstack.org/371917
Committed: https://git.openstack.org/cgit/openstack/gce-api/commit/?id=1e7910cc0886c7d992db75729d0735fe791cb34d
Submitter: Jenkins
Branch: master

commit 1e7910cc0886c7d992db75729d0735fe791cb34d
Author: pallavi <email address hidden>
Date: Sat Sep 17 16:42:01 2016 +0530

    Don't attempt to escalate gce-api-manage privileges

    Remove code which allowed gce-api-manage to attempt to escalate
    privileges so that configuration files can be read by users who
    normally wouldn't have access, but do have sudo access.

    Change-Id: I0a684f0239bef1669c15b6c54a658c6e6f2f430c
    Closes-Bug: 1611171

Changed in gce-api:
status: In Progress → Fix Released

Reviewed: https://review.openstack.org/371927
Committed: https://git.openstack.org/cgit/openstack/rally/commit/?id=2259b17384c0eb7da9325123e6f4041c5b317c79
Submitter: Jenkins
Branch: master

commit 2259b17384c0eb7da9325123e6f4041c5b317c79
Author: Iswarya_Vakati <email address hidden>
Date: Sat Sep 17 18:19:22 2016 +0530

    Don't attempt to escalate rally-manage privileges

    Remove code which allowed rally-manage to attempt to escalate
    privileges so that configuration files can be read by users who
    normally wouldn't have access, but do have sudo access.

    Change-Id: Iff352e463189738d3f371c819edfeb0664fd7684
    Closes-Bug:#1611171

Changed in rally:
status: In Progress → Fix Released

Reviewed: https://review.openstack.org/371930
Committed: https://git.openstack.org/cgit/openstack/ec2-api/commit/?id=f8dbd1cc45a1ceeedebf80607ef72eaaaba174a9
Submitter: Jenkins
Branch: master

commit f8dbd1cc45a1ceeedebf80607ef72eaaaba174a9
Author: Iswarya_Vakati <email address hidden>
Date: Sat Sep 17 18:28:28 2016 +0530

    Don't attempt to escalate ec2-api-manage privileges

    Remove code which allowed ec2-api-manage to attempt to escalate
    privileges so that configuration files can be read by users who
    normally wouldn't have access, but do have sudo access.

    Change-Id: I1ab7052fc117f064054e3127517da77598b6d27b
    Closes-Bug:#1611171

Changed in ec2-api:
status: In Progress → Fix Released

This issue was fixed in the openstack/nova 14.0.2 release.

This issue was fixed in the openstack/cinder 10.0.0.0b1 development milestone.

This issue was fixed in the openstack/nova 15.0.0.0b1 development milestone.

This issue was fixed in the openstack/nova 14.0.2 release.

Change abandoned by Kiall Mac Innes (<email address hidden>) on branch: master
Review: https://review.openstack.org/352843
Reason: Feel free to resurrect if needs be!

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

Changed in designate:
assignee: Kiall Mac Innes (kiall) → Dr. Jens Harbott (j-harbott)
Changed in manila:
assignee: iswarya vakati (v-iswarya) → Tom Barron (tpb)

Reviewed: https://review.openstack.org/513665
Committed: https://git.openstack.org/cgit/openstack/designate/commit/?id=440a67cab18e3ab725383d01b4ed26fa3b1d3da0
Submitter: Zuul
Branch: master

commit 440a67cab18e3ab725383d01b4ed26fa3b1d3da0
Author: Jens Harbott <email address hidden>
Date: Fri Oct 20 08:34:18 2017 +0000

    Don't attempt to escalate designate-manage privileges

    Remove code which allowed designate-manage to attempt to escalate
    privileges so that configuration files can be read by users who
    normally wouldn't have access, but do have sudo access.

    Simpler version of [1].

    [1] I03063d2af14015e6506f1b6e958f5ff219aa4a87
    Closes-Bug: 1611171

    Change-Id: I013754da27e9dd13493bee1abfada3fbc2a004c0

Changed in designate:
status: In Progress → Fix Released

This issue was fixed in the openstack/designate 6.0.0.0b2 development milestone.

Reviewed: https://review.openstack.org/371920
Committed: https://git.openstack.org/cgit/openstack/manila/commit/?id=0d4438368fd769a0e6b83bfdaf1cb980f888c504
Submitter: Zuul
Branch: master

commit 0d4438368fd769a0e6b83bfdaf1cb980f888c504
Author: Iswarya_Vakati <email address hidden>
Date: Sat Sep 17 17:07:16 2016 +0530

    Don't attempt to escalate manila-manage privileges

    Remove code which allowed manila-manage to attempt to escalate
    privileges so that configuration files can be read by users who
    normally wouldn't have access, but do have sudo access.

    Change-Id: Ie3bf9a81ee8d723cd8618643fa9d7382462aae42
    Closes-Bug:#1611171

Changed in manila:
status: In Progress → Fix Released

This issue was fixed in the openstack/manila 7.0.0.0b1 development milestone.

This issue was fixed in the openstack/ec2-api 7.0.0 release.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers