re-runs self via sudo

[Seth Arnold]

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

[Kiall Mac Innes (kiall)]

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

[OpenStack Infra (hudson-openstack)]

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

[Kiall Mac Innes (kiall)]

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

[Kiall Mac Innes (kiall)]

Apparantly, this isn't unique to Designate either:

http://git.openstack.org/cgit/openstack/cinder/tree/cinder/cmd/manage.py
http://git.openstack.org/cgit/openstack/nova/tree/nova/cmd/manage.py

[Tristan Cacqueray (tristan-cacqueray)]

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

[Sean Dague (sdague)]

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

[Markus Zoeller (markus_z) (mzoeller)]

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

[Kiall Mac Innes (kiall)]

@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 ;))

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[Ben Swartzlander (bswartz)]

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

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

@Graham okay makes sense to me

[Jeremy Stanley (fungi)]

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.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/newton
Review: https://review.openstack.org/385365

[OpenStack Infra (hudson-openstack)]

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)

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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!

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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