ceph backend, secret key leak

Bug #1849624 reported by raphael.glon
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Cinder
Fix Committed
Low
Brian Rosmaita
OpenStack Security Advisory
Won't Fix
Undecided
Unassigned
OpenStack Security Notes
Fix Released
Undecided
Brian Rosmaita

Bug Description

Cinder + ceph backend, secret key leak

Conditions: cinder + ceph backend + rbd_keyring_conf set in cinder config files

As an authenticated simple user create a cinder volume that ends up on a ceph backend,
Then reuse the os.initialize_connection api call
(used by nova-compute/cinder-backup to attach volumes locally to the host running the services):

curl -g -i -X POST https://<cinder_controller>/v3/c495530af57611e9bc14bbaa251e1e96/volumes/7e59b91e-d426-4294-bfc5-dfdebcb21879/action \
    -H "Accept: application/json" \
    -H "Content-Type: application/json" \
    -H "OpenStack-API-Version: volume 3.15" \
    -H "X-Auth-Token: $TOKEN" \
    -d '{"os-initialize_connection": {"connector":{}}}'

If you do not want to forge the http request, openstack clients and extensions may prove helpful.

As root:

apt-get install python3-oslo.privsep virtualenv python3-dev python3-os-brick gcc ceph-common
virtualenv -p python3 venv_openstack
source venv_openstack/bin/activate
pip install python-openstackclient
pip install python-cinderclient
pip install os-brick
pip install python-brick-cinderclient-ext
cinder create vol 1
cinder --debug local-attach 7e59b91e-d426-4294-bfc5-dfdebcb21879

This leaks the ceph credentials for the whole ceph cluster, leaving anyone able to go through ceph acls to get access
to all the volumes within the cluster.

{
   "connection_info" : {
      "data" : {
         "access_mode" : "rw",
         "secret_uuid" : "SECRET_UUID",
         "cluster_name" : "ceph",
         "encrypted" : false,
         "auth_enabled" : true,
         "discard" : true,
         "qos_specs" : {
            "write_iops_sec" : "3050",
            "read_iops_sec" : "3050"
         },
         "keyring" : "SECRETFILETOHIDE",
         "ports" : [
            "6789",
            "6789",
            "6789"
         ],
         "name" : "volumes/volume-7e59b91e-d426-4294-bfc5-dfdebcb21879",
         "secret_type" : "ceph",
         "hosts" : [
            "ceph_host1",
            "ceph_host2",
            ...
         ],
         "volume_id" : "7e59b91e-d426-4294-bfc5-dfdebcb21879",
         "auth_username" : "cinder"
      },
      "driver_volume_type" : "rbd"
   }
}

Quick workaround:
1. Remove rbd_keyring_conf param from any cinder config file, this will mitigate the information disclosure.
2. For cinder backups to still work, providers should instead deploy their ceph keyring secrets directly on cinder-backup hosts
(/etc/cinder/<backend_name>.keyring.conf, to be confirmed).

Note that nova-compute hosts should not be impacted by the change, because ceph secrets are expected to be stored in
libvirt secrets already, thus making this keyring disclose useless to it.
(to be confirmed, there may be other compute drivers that might be impacted by this)

Quick code fix:
Mandatory: revert this commit https://review.opendev.org/#/c/456672/
Optional: revert this one https://review.opendev.org/#/c/465044/, harmless in itself, but pointless once the first one has been reverted

Long term code fix proposals:
What the os.initialize_connection api call is meant to: allow simple users to use cinder as block storage as a service
in order to attach volumes outside the scope of any virtual machines/nova.
Thus, information returned by this call should give enough information for a volume attach to be possible for the caller but they should not disclose
anything that would allow him to do more than that.
Since it is not possible at all with ceph to do so (no tenant isolation within ceph cluster),
the related cinder backend for ceph should not implement this route at all
There is indeed no reason why cinder should disclose anything here about ceph cluster, including hosts, cluster-ids,
if the attach is doomed to fail for users missing secret informations anyway.
Then, any 'admin' service using this call to locally attach the volumes (nova-compute, cinder-backup...) should be modified to:
- check caller rw permissions on requested volumes
- escalate the request
- go through a new admin api route, not this 'user' one

description: updated
Revision history for this message
Sean McGinnis (sean-mcginnis) wrote :

Eric, is this something you can comment on? Or know someone on the team that can?

Changed in cinder:
assignee: nobody → Eric Harney (eharney)
Revision history for this message
Jeremy Stanley (fungi) wrote :

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.

Changed in ossa:
status: New → Incomplete
description: updated
Revision history for this message
Jeremy Stanley (fungi) wrote :

Note that it's possible for Nova to leak Ceph credentials under certain situations if the fix for bug 1837877 (OSSA-2019-003) is not applied.

Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

First, we need to be clear that if the rbd_keyring_conf option is NOT used (which is the standard deployment scenario), this vulnerability does not arise.

Second, my proposal for fixing this is:
(1) Deprecate the option now (in Ussuri) for removal in V.
(2) Issue the OSSA explaining the problem and explaining that the mitigation is to not use the rbd_keyring_conf option.

Revision history for this message
Jeremy Stanley (fungi) wrote :

Based on this, I think that (2) will more correctly be an OSSN (security note rather than advisory) since it's recommending a change in configuration rather than providing a patch for deployers to apply. Probably closest classification in our report taxonomy is B2 "A vulnerability that can only be fixed in master, security note for stable branches, e.g., default config value is insecure" (though in this case it sounds like it's at least not the default value): https://security.openstack.org/vmt-process.html#incident-report-taxonomy

Revision history for this message
Jeremy Stanley (fungi) wrote :

Er, I meant class B1 there (which is the classification matching the description I quoted).

Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

Thanks, Jeremy. An OSSN for class B1 sounds correct to me.

I'd like to get some feedback on my proposal to fix this by simply removing the option. It's not clear to me from the patch that introduced the rbd_keyring_conf (or the bug associated with it) why this config option is necessary, but I may be missing something.

Revision history for this message
Walt Boring (walter-boring) wrote :

Just so we are clear here. The keyring itself is not being passed in the call, what is being passed is the path and name of the keyring file on disk. So the credentials aren't being leaked here, just the name and path of the file that exists. You would still have to get access on the host where the keyring exists to even read it.

Revision history for this message
Walt Boring (walter-boring) wrote :

Also, as of note, the reason this feature was put in place to begin with is that a host can have access to multiple ceph clusters, and it's not always possible to force the filename to the 'standard' naming scheme. This would in fact make cinder not able to talk to a secondary cluster that requires a different keyring filename.

Revision history for this message
Eric Harney (eharney) wrote :
Revision history for this message
Walt Boring (walter-boring) wrote :

ok my bad, I was looking at the original patch that seemed like it was returning the file path. returning the key itself is not good.

Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

OK, to move this along:

1. it looks like there is not a security fix we can issue at this time without redesigning the feature
2. the vulnerability is restricted to non-standard deployments
3. there is a workaround (the "Quick Workaround" outlined by Raphael in the Bug Description)
4. so, as of now, the mitigation is to use the workaround
5. since a patch isn't coming, we should issue an OSSN describing the vulnerability and the workaround
6. Cinder will deprecate the option referencing the OSSN
7. we can then make this bug public and continue a discussion of Walt's point that this is a legitimate use case and figure out how to address it

It would be great to have 1-6 done this week so that we can discuss this at the Forum/PTG next week so we can get an idea of how many users this affects (in the sense that they can't do the workaround, i.e., point #7).

Revision history for this message
Jeremy Stanley (fungi) wrote :

Brian's plan makes sense aside from #7. Instead I recommend we switch the bug to public while seeking volunteers to draft a note about it at https://wiki.openstack.org/wiki/Security_Notes for increased visibility. Keeping it under a private embargo when there's no patch forthcoming serves no purpose that I can see, since the advised workaround is already available and can be found in the discussion here.

Revision history for this message
Gage Hugo (gagehugo) wrote :

Agreed on switching this to public and having someone draft an OSSN for this.

Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

I have no objection to Jeremy's recommendation to make the bug public first and then draft the OSSN.

Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

OK, to continue moving this along, I will volunteer to draft the OSSN.

Revision history for this message
Jeremy Stanley (fungi) wrote :

Since there were no objections to the revised plan, I've switched the report to public, set the advisory task to won't fix and added a confirmed task for a security note. Thanks!

Changed in ossa:
status: Incomplete → Won't Fix
Changed in ossn:
status: New → Confirmed
assignee: nobody → Brian Rosmaita (brian-rosmaita)
description: updated
information type: Private Security → Public
tags: added: security
Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

Using the Cinder part of the bug to track deprecating the option for removal in V; deprecation notice should reference the OSSN.

Changed in cinder:
assignee: Eric Harney (eharney) → Brian Rosmaita (brian-rosmaita)
importance: Undecided → Low
milestone: none → ussuri-1
status: New → In Progress
Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :
Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

A note about step 2 in the Quick Workaround in the bug description: Gorka Eguileor noticed that the correct file location is actually:

  /etc/ceph/<cluster_name>.client.<user_name>.keyring

See https://opendev.org/openstack/os-brick/src/commit/87171abef8bf2336f15ce3a7949f77d7999e11b7/os_brick/initiator/connectors/rbd.py#L76

Gage Hugo (gagehugo)
Changed in ossn:
status: Confirmed → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (master)

Fix proposed to branch: master
Review: https://review.opendev.org/692428

Changed in ossn:
status: In Progress → Fix Released
Changed in cinder:
status: In Progress → Fix Committed
Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

This actually hasn't merged yet. It's been approved but has been stuck in the gate due to circumstances beyond Cinder's control.

Changed in cinder:
status: Fix Committed → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to cinder (master)

Reviewed: https://review.opendev.org/692428
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=b3c68b777a53e78fb77b7af98952bcc7d97bb06f
Submitter: Zuul
Branch: master

commit b3c68b777a53e78fb77b7af98952bcc7d97bb06f
Author: Brian Rosmaita <email address hidden>
Date: Thu Oct 31 14:17:19 2019 -0400

    Deprecate rbd_keyring_conf option

    This option presents a security risk; see OSSN-0085.

    Change-Id: I345a3b4bf3b328b0e547016f481518d252f734b9
    Partial-bug: #1849624

Changed in cinder:
status: In Progress → Fix Committed
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to cinder (stable/ussuri)

Related fix proposed to branch: stable/ussuri
Review: https://review.opendev.org/727779

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to cinder (stable/train)

Related fix proposed to branch: stable/train
Review: https://review.opendev.org/727853

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to cinder (stable/ussuri)

Reviewed: https://review.opendev.org/727779
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=0f7a3ddd3c4d4173612b4ea86c31c8b301ff2153
Submitter: Zuul
Branch: stable/ussuri

commit 0f7a3ddd3c4d4173612b4ea86c31c8b301ff2153
Author: Sean McGinnis <email address hidden>
Date: Wed May 13 09:27:18 2020 -0500

    [stable only] Add warning about rbd_keyring_conf

    This adds a warning message to the driver documentation page to make
    sure it is visible that this config option should not be used due to
    security concerns. We can't backport the deprecation of the config
    option, but we can backport this doc warning to help prevent this option
    from being used.

    Related-bug: #1849624

    Change-Id: Ief2c868d6a9baf6793cd9070a4451835a90752aa
    Signed-off-by: Sean McGinnis <email address hidden>

tags: added: in-stable-ussuri
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to cinder (stable/train)

Reviewed: https://review.opendev.org/727853
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=ac6e0c472fb6276afbd6d421c410ccc844369563
Submitter: Zuul
Branch: stable/train

commit ac6e0c472fb6276afbd6d421c410ccc844369563
Author: Sean McGinnis <email address hidden>
Date: Wed May 13 09:27:18 2020 -0500

    [stable only] Add warning about rbd_keyring_conf

    This adds a warning message to the driver documentation page to make
    sure it is visible that this config option should not be used due to
    security concerns. We can't backport the deprecation of the config
    option, but we can backport this doc warning to help prevent this option
    from being used.

    Also includes part of a squash for the release note from:
    Deprecate rbd_keyring_conf option
    Change-Id: I345a3b4bf3b328b0e547016f481518d252f734b9

    Related-bug: #1849624

    Change-Id: Ief2c868d6a9baf6793cd9070a4451835a90752aa
    Signed-off-by: Sean McGinnis <email address hidden>
    (cherry picked from commit 0f7a3ddd3c4d4173612b4ea86c31c8b301ff2153)

tags: added: in-stable-train
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to cinder (stable/stein)

Related fix proposed to branch: stable/stein
Review: https://review.opendev.org/727896

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to cinder (stable/stein)

Reviewed: https://review.opendev.org/727896
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=7a33e5fa791b77ead0bddb052b875cffd24933de
Submitter: Zuul
Branch: stable/stein

commit 7a33e5fa791b77ead0bddb052b875cffd24933de
Author: Sean McGinnis <email address hidden>
Date: Wed May 13 09:27:18 2020 -0500

    [stable only] Add warning about rbd_keyring_conf

    This adds a warning message to the driver documentation page to make
    sure it is visible that this config option should not be used due to
    security concerns. We can't backport the deprecation of the config
    option, but we can backport this doc warning to help prevent this option
    from being used.

    Also includes part of a squash for the release note from:
    Deprecate rbd_keyring_conf option
    Change-Id: I345a3b4bf3b328b0e547016f481518d252f734b9

    Related-bug: #1849624

    Change-Id: Ief2c868d6a9baf6793cd9070a4451835a90752aa
    Signed-off-by: Sean McGinnis <email address hidden>
    (cherry picked from commit 0f7a3ddd3c4d4173612b4ea86c31c8b301ff2153)
    (cherry picked from commit ac6e0c472fb6276afbd6d421c410ccc844369563)

tags: added: in-stable-stein
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to cinder (stable/rocky)

Related fix proposed to branch: stable/rocky
Review: https://review.opendev.org/728163

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to cinder (stable/rocky)

Reviewed: https://review.opendev.org/728163
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=4aa3f20af41bce1b4ed1538720b685d0959bc829
Submitter: Zuul
Branch: stable/rocky

commit 4aa3f20af41bce1b4ed1538720b685d0959bc829
Author: Sean McGinnis <email address hidden>
Date: Wed May 13 09:27:18 2020 -0500

    [stable only] Add warning about rbd_keyring_conf

    This adds a warning message to the driver documentation page to make
    sure it is visible that this config option should not be used due to
    security concerns. We can't backport the deprecation of the config
    option, but we can backport this doc warning to help prevent this option
    from being used.

    Also includes part of a squash for the release note from:
    Deprecate rbd_keyring_conf option
    Change-Id: I345a3b4bf3b328b0e547016f481518d252f734b9

    Related-bug: #1849624

    Change-Id: Ief2c868d6a9baf6793cd9070a4451835a90752aa
    Signed-off-by: Sean McGinnis <email address hidden>
    (cherry picked from commit 0f7a3ddd3c4d4173612b4ea86c31c8b301ff2153)
    (cherry picked from commit ac6e0c472fb6276afbd6d421c410ccc844369563)
    (cherry picked from commit 7a33e5fa791b77ead0bddb052b875cffd24933de)

tags: added: in-stable-rocky
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (master)

Fix proposed to branch: master
Review: https://review.opendev.org/738216

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to cinder (master)

Reviewed: https://review.opendev.org/738216
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=be02e0f3467b8cac20eaf73c52164f6bed88f7de
Submitter: Zuul
Branch: master

commit be02e0f3467b8cac20eaf73c52164f6bed88f7de
Author: Gorka Eguileor <email address hidden>
Date: Fri Jun 26 14:42:09 2020 +0200

    RBD: cinderlib support for rbd_keyring_conf option

    In the last cycle we deprecated the RBD configuration option as per
    OSSN-0085, and it was removed for victoria by Change
    I3cc58b2d74d82ab6b2186e9ea7cfafeb4c3de822

    This patch modifies the RBD driver to support cinderlib use cases that
    are not affected by the security vulnerability.

    Even if we have the configuration option in cinder.conf it will not be
    seen by the Cinder RBD driver, it will only see it if we skip the Oslo
    Config mechanism and set it directly on the instance as an attribute,
    like cinderlib does.

    Related-Bug: #1849624
    Implements: blueprint rbd-remove-option-causing-ossn-0085
    Change-Id: Iae63aee973932b2eef26d832a7f413a04bad0df1

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

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.