ScaleIO (thin) volumes contain previous data (follow-up to 1699573)

[Eric Harney]

Bug 1699573 described an issue in the ScaleIO Cinder driver where new volumes can contain data from previously deleted volumes. [1]

We specifically document [2] that this is a security hazard for Cinder, because it means that end-user data can leak between tenants.

The previous bug discussion and fix indicated that this only affects thick-provisioned volumes from ScaleIO. Further investigation indicates that it also affects thin-provisioned volumes, so the fix was not complete.

It appears that we can fix this issue completely by extending the previous fix to not consider thin-provisioned volumes safe, and apply the same logic to thin volumes that we use for thick volumes. This would force ScaleIO zero padding to be enabled in all cases.

I also think this bug merits a Class A rating per the VMT process. [3] I don't see a reason we can't backport the fix to stable releases.

The text of OSSN-0084 [4] makes this more confusing -- the description described this issue as affecting thin volumes, when the fix only affected thick volumes. The Recommended Actions are also incorrect -- enabling zero padding probably* fixes this issue, but swapping to thin volumes is not relevant.

* (I don't have access to a ScaleIO backend to investigate this directly. I'm relying on some brief discussion with ScaleIO maintainers and customer reports.)

[1] https://bugs.launchpad.net/cinder/+bug/1699573
[2] https://git.openstack.org/cgit/openstack/cinder/tree/doc/source/contributor/drivers.rst?h=13.0.0.0b2#n58
[3] https://security.openstack.org/vmt-process.html#incident-report-taxonomy
[4] http://lists.openstack.org/pipermail/openstack-dev/2018-July/132096.html

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

Could we subscribe ScaleIO maintainers to this bug?

[Jay Bryant (jsbryant)]

Unfortunately a public patch was pushed by the ScaleIO team to address this bug. We had been trying to work this through e-mail with the developer but this has now been slipped out of the Private VMT process.

The overall scope of this bug is limited to the ScaleIO driver and therefore will only impact Cinder users who are using that backend. That limits the impact of this vulnerability. With that said, users who do use the ScaleIO driver could get into a situation where they could see data from other tenants.

I think we should try to get this fixed ASAP now that the patch has been made public. Will defer to the VMT as to what all needs to be done as far as a security report, etc.

[Jeremy Stanley (fungi)]

Thanks Jay! It happens. Can you provide a link to the change in review so I can see how thoroughly it discloses the nature of this issue?

[Jay Bryant (jsbryant)]

Hey Jeremy! Thanks for responding. Here is the patch they posted: https://review.openstack.org/#/c/592001/

Kind of feel like the change proposed in this bug by Eric is easier to backport and a better solution. Thinking that we should have Eric push up his solution into 592001 instead.

[Jeremy Stanley (fungi)]

Seeing as how the linked fix directly references this bug report, I'm switching it from Private Security to Public Security now so that we can get everyone on the same page.

[Jay Bryant (jsbryant)]

Jeremy,

Thank you for the input. We are working this through the review now and hope to have something out there soon.

Jay

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/592001
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=949cc46e162e00092aa85a7be921649c8dbf2bf8
Submitter: Zuul
Branch: master

commit 949cc46e162e00092aa85a7be921649c8dbf2bf8
Author: Matan Sabag <email address hidden>
Date: Wed Aug 15 13:01:36 2018 +0300

    ScaleIO: Disable volume creation without padding

    Applying previous fix for thick volumes to thin volumes to disallow
    volume creation without zero padding unless enabled via config option.

    Change-Id: Ibaf6e9b67d252a5aae1b0f64ec632ec26789c389
    Closes-Bug: #1784871
    Signed-off-by: Matan Sabag <email address hidden>

[Jeremy Stanley (fungi)]

Are there stable backports in the works yet for https://review.openstack.org/592001 or will fixing there require a different approach?

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/rocky
Review: https://review.openstack.org/593188

[Sean McGinnis (sean-mcginnis)]

We do need to get this backported yet. I believe we should be able to just backport and not need to modify the approach.

[Sean McGinnis (sean-mcginnis)]

Looks like bug was not automatically updated. Stable/rocky backport proposed with https://review.openstack.org/593188

[Jay Bryant (jsbryant)]

Jeremy,

I will backport today and propose rc2.

Jay

On Fri, Aug 17, 2018, 4:01 PM Jeremy Stanley <email address hidden> wrote:

> Are there stable backports in the works yet for
> https://review.openstack.org/592001 or will fixing there require a
> different approach?
>
> --
> You received this bug notification because you are a member of Cinder
> Core security contacts, which is subscribed to the bug report.
> https://bugs.launchpad.net/bugs/1784871
>
> Title:
> ScaleIO (thin) volumes contain previous data (follow-up to 1699573)
>
> Status in Cinder:
> Fix Released
> Status in OpenStack Security Advisory:
> Confirmed
>
> Bug description:
> Bug 1699573 described an issue in the ScaleIO Cinder driver where new
> volumes can contain data from previously deleted volumes. [1]
>
> We specifically document [2] that this is a security hazard for
> Cinder, because it means that end-user data can leak between tenants.
>
> The previous bug discussion and fix indicated that this only affects
> thick-provisioned volumes from ScaleIO. Further investigation
> indicates that it also affects thin-provisioned volumes, so the fix
> was not complete.
>
> It appears that we can fix this issue completely by extending the
> previous fix to not consider thin-provisioned volumes safe, and apply
> the same logic to thin volumes that we use for thick volumes. This
> would force ScaleIO zero padding to be enabled in all cases.
>
> I also think this bug merits a Class A rating per the VMT process. [3]
> I don't see a reason we can't backport the fix to stable releases.
>
> The text of OSSN-0084 [4] makes this more confusing -- the description
> described this issue as affecting thin volumes, when the fix only
> affected thick volumes. The Recommended Actions are also incorrect --
> enabling zero padding probably* fixes this issue, but swapping to thin
> volumes is not relevant.
>
> * (I don't have access to a ScaleIO backend to investigate this
> directly. I'm relying on some brief discussion with ScaleIO
> maintainers and customer reports.)
>
> [1] https://bugs.launchpad.net/cinder/+bug/1699573
> [2]
> https://git.openstack.org/cgit/openstack/cinder/tree/doc/source/contributor/drivers.rst?h=13.0.0.0b2#n58
> [3]
> https://security.openstack.org/vmt-process.html#incident-report-taxonomy
> [4]
> http://lists.openstack.org/pipermail/openstack-dev/2018-July/132096.html
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/cinder/+bug/1784871/+subscriptions
>

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/593188
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=3a39d09166bf6d1c7d2bae63caf3e2a954328862
Submitter: Zuul
Branch: stable/rocky

commit 3a39d09166bf6d1c7d2bae63caf3e2a954328862
Author: Matan Sabag <email address hidden>
Date: Wed Aug 15 13:01:36 2018 +0300

    ScaleIO: Disable volume creation without padding

    Applying previous fix for thick volumes to thin volumes to disallow
    volume creation without zero padding unless enabled via config option.

    Change-Id: Ibaf6e9b67d252a5aae1b0f64ec632ec26789c389
    Closes-Bug: #1784871
    Signed-off-by: Matan Sabag <email address hidden>
    (cherry picked from commit 949cc46e162e00092aa85a7be921649c8dbf2bf8)

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/593694

[OpenStack Infra (hudson-openstack)]

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

commit b54b2de4e9b06467df22bf5a96c3611dd5653ef4
Author: Sean McGinnis <email address hidden>
Date: Mon Aug 20 11:32:40 2018 -0500

    ScaleIO: Deprecate sio_allow_non_padded_thick_volumes

    The config option sio_allow_non_padded_thick_volumes was added to
    allow sites to continue to have non-padded thick volumes after it was
    disabled by default due to security concerns. At the time it was only
    thought to be an issue with thick volumes, but has since been determined
    to also be an issue with thin.

    To address the concerns with thin, the general option
    sio_allow_non_padded__volumes was added, making the thick-only option
    redundant. This new option was added without the deprecation of the
    thick-only option to ease backporting to address the securty concern,
    but we now want to work towards removing the thick option so we can just
    have the one option for both provisioning types.

    Related-bug: #1784871

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

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: stable/rocky
Review: https://review.openstack.org/594556

[Summer Long (slong-g)]

Red Hat has created CVE-2017-15139 for this.
Summer

[Jeremy Stanley (fungi)]

Summer: why was a 2017 series CVE number assigned for this issue? It wasn't reported until this month.

Did you mean to assign the CVE for the earlier bug 1699573 instead? I see that https://access.redhat.com/security/cve/cve-2017-15139 mentions OSSN-0084 which was about the earlier fix for thick volumes (only fixed in the master branch of Cinder so far).

Or do we want to consider these duplicate bugs with backports to earlier branches covering thick and thin volumes alike?

[Summer Long (slong-g)]

Hi Jeremy, yes, the latter since the update for the first bug was incomplete, and this bug is the fix for that. CVE-2017-15139 is for the problem of volumes containing data from previously deleted volumes, both thick and thin. Not sure how you tag that administratively, but I'll add the same note to the first one as well.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/594556
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=b067a847cf2a14d450682d15f9555f0d5e9ca9c2
Submitter: Zuul
Branch: stable/rocky

commit b067a847cf2a14d450682d15f9555f0d5e9ca9c2
Author: Sean McGinnis <email address hidden>
Date: Mon Aug 20 11:32:40 2018 -0500

    ScaleIO: Deprecate sio_allow_non_padded_thick_volumes

    The config option sio_allow_non_padded_thick_volumes was added to
    allow sites to continue to have non-padded thick volumes after it was
    disabled by default due to security concerns. At the time it was only
    thought to be an issue with thick volumes, but has since been determined
    to also be an issue with thin.

    To address the concerns with thin, the general option
    sio_allow_non_padded__volumes was added, making the thick-only option
    redundant. This new option was added without the deprecation of the
    thick-only option to ease backporting to address the securty concern,
    but we now want to work towards removing the thick option so we can just
    have the one option for both provisioning types.

    Related-bug: #1784871

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

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/cinder 13.0.0.0rc2 release candidate.

[Sean McGinnis (sean-mcginnis)]

I realized today that the original fix for this was just added in Rocky. So the multi-patch thing we did to avoid backporting deprecations was not necessary and just added extra work. Sorry for not noticing the full history on this.

Working on backporting the final fix today so we can address the security concerns in stable branches.

[Jeremy Stanley (fungi)]

Thanks, Sean. If it was only going to be fixed in Rocky then I planned to classify this as a B1 report (vulnerability that can only be fixed in master, security note for stable branches). With stable backports, we can do an advisory instead. I'll skip requesting a CVE from MITRE in this case and just use the one Red Hat already assigned for it.

[Sean McGinnis (sean-mcginnis)]

It requires backporting a config option, which we normally don't want to do, but I think it should be fine to backport to stable branches to address the security concern.

[Jeremy Stanley (fungi)]

I guess since this is already public, now is a great time to get stable branch reviewers to weigh in on that choice and make sure everyone's happy with it.

[Jeremy Stanley (fungi)]

Just to make sure I understand the proposed backport, will existing deployments need to make any changes to their configuration to mitigate the described vulnerability, or is the configuration option merely going to be for enabling the old insecure mode of operation?

We generally employ a security note instead of an advisory if existing depoyments will need to make configuration changes to enable security fixes rather than being secure by default after applying the patch.

[Sean McGinnis (sean-mcginnis)]

Yeah, there's been some churn here, so it probably would be good to clarify things.

With the patches that are up right now, we should end up with a single config option called 'sio_allow_non_padded_volumes' that defaults to False. That will make things secure by default but give the deployment an option to enable to previously insecure behavior if they so choose.

We would then backport that new config option back through the stable branches. This will be a change in behavior on upgrade to new stable releases, but it is called out in a release note and they can re-enable the old behavior using the new config option.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/queens
Review: https://review.openstack.org/596879

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/596879
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=f0cef07bef5ea8ed29179ee3774df5f4a634ba86
Submitter: Zuul
Branch: stable/queens

commit f0cef07bef5ea8ed29179ee3774df5f4a634ba86
Author: Eric Young <email address hidden>
Date: Thu Mar 22 20:24:01 2018 -0400

    ScaleIO: Prevent usage of unsafe volumes

    It is possible for volumes, created from storage pools
    which have zero-padding disabled, to contain previous data. This
    change prevents these volumes from being created by default. A
    user can override this behavior by acknowleding the possibility
    with a configuration option.

    This is a squash of the four commits that led to the final state in
    rocky to not allow the creation of any type of non-zero-padded volumes
    to be created. This adds a config option that defaults to the safe
    behavior. It is backporting a new config option, and a change in default
    behavior, but it should be acceptable in this case so that the security
    vulnerability can be addressed.

    Closes-Bug: #1784871

    Change-Id: I62f8f48b1624fc9abb7427bd4ca51f7873d35b96
    Closes-bug: #1699573
    (cherry picked from commit 7feb62197d371ab7253dc86a34af6ff8b484b4df)
    (cherry picked from commit 949cc46e162e00092aa85a7be921649c8dbf2bf8)
    (cherry picked from commit 8d0dea694a366cb3797748d389ca76b7864af16f)
    (cherry picked from commit 13a6689ccb7751c9f9b5c37ce0a3f75eb7665a95)

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/pike
Review: https://review.openstack.org/601681

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/ocata
Review: https://review.openstack.org/604105

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/601681
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=6309c097e653c5f8b40e0602950d0ef54a9efb37
Submitter: Zuul
Branch: stable/pike

commit 6309c097e653c5f8b40e0602950d0ef54a9efb37
Author: Eric Young <email address hidden>
Date: Thu Mar 22 20:24:01 2018 -0400

    ScaleIO: Prevent usage of unsafe volumes

    It is possible for volumes, created from storage pools
    which have zero-padding disabled, to contain previous data. This
    change prevents these volumes from being created by default. A
    user can override this behavior by acknowleding the possibility
    with a configuration option.

    This is a squash of the four commits that led to the final state in
    rocky to not allow the creation of any type of non-zero-padded volumes
    to be created. This adds a config option that defaults to the safe
    behavior. It is backporting a new config option, and a change in default
    behavior, but it should be acceptable in this case so that the security
    vulnerability can be addressed.

    Closes-Bug: #1784871

    Change-Id: I62f8f48b1624fc9abb7427bd4ca51f7873d35b96
    Closes-bug: #1699573
    (cherry picked from commit f0cef07bef5ea8ed29179ee3774df5f4a634ba86)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/604105
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=2dc52153215bb6a37532a959c5c98239be21bb56
Submitter: Zuul
Branch: stable/ocata

commit 2dc52153215bb6a37532a959c5c98239be21bb56
Author: Eric Young <email address hidden>
Date: Thu Mar 22 20:24:01 2018 -0400

    ScaleIO: Prevent usage of unsafe volumes

    It is possible for volumes, created from storage pools
    which have zero-padding disabled, to contain previous data. This
    change prevents these volumes from being created by default. A
    user can override this behavior by acknowleding the possibility
    with a configuration option.

    This is a squash of the four commits that led to the final state in
    rocky to not allow the creation of any type of non-zero-padded volumes
    to be created. This adds a config option that defaults to the safe
    behavior. It is backporting a new config option, and a change in default
    behavior, but it should be acceptable in this case so that the security
    vulnerability can be addressed.

    Closes-Bug: #1784871

    Change-Id: I62f8f48b1624fc9abb7427bd4ca51f7873d35b96
    Closes-bug: #1699573
    (cherry picked from commit f0cef07bef5ea8ed29179ee3774df5f4a634ba86)
    (cherry picked from commit 6309c097e653c5f8b40e0602950d0ef54a9efb37)

[Sean McGinnis (sean-mcginnis)]

We've now backported the fix for this to all available stable branches. I'm considering this issue complete unless I hear otherwise.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: driverfixes/newton
Review: https://review.openstack.org/606130

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/cinder 10.0.8 release.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/606130
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=234aab19f1677a337abd4cc37ede5dc5e455f258
Submitter: Zuul
Branch: driverfixes/newton

commit 234aab19f1677a337abd4cc37ede5dc5e455f258
Author: Eric Young <email address hidden>
Date: Thu Mar 22 20:24:01 2018 -0400

    ScaleIO: Prevent usage of unsafe volumes

    It is possible for volumes, created from storage pools
    which have zero-padding disabled, to contain previous data. This
    change prevents these volumes from being created by default. A
    user can override this behavior by acknowleding the possibility
    with a configuration option.

    This is a squash of the four commits that led to the final state in
    rocky to not allow the creation of any type of non-zero-padded volumes
    to be created. This adds a config option that defaults to the safe
    behavior. It is backporting a new config option, and a change in default
    behavior, but it should be acceptable in this case so that the security
    vulnerability can be addressed.

    Closes-Bug: #1784871

    Change-Id: I62f8f48b1624fc9abb7427bd4ca51f7873d35b96
    Closes-bug: #1699573
    (cherry picked from commit f0cef07bef5ea8ed29179ee3774df5f4a634ba86)
    (cherry picked from commit 6309c097e653c5f8b40e0602950d0ef54a9efb37)
    (cherry picked from commit 2dc52153215bb6a37532a959c5c98239be21bb56)

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/cinder 12.0.4 release.

[Gage Hugo (gagehugo)]

Please review this OSSA description for accuracy, as I do not have much ScaleIO expertise. I combined both the Thin/Thick volume bugs:

Title: Data is retained after deletion of a ScaleIO volume
Reporter: Martin Chlumsky/Eric Harney
Products: Cinder
Affects: <10.0.8, <11.1.1, <12.0.4

Description:
Martin Chlumsky/Eric Harney reported a vulnerability in Cinder, specifically with ScaleIO volumes.
When a ScaleIO (Thin or Thick) volume is deleted, the data is not cleared, and it is possible for a new volume to contain old tenant data that a new tenant would not have the authority to access. Only volumes utilizing the ScaleIO driver are affected.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/cinder 11.2.0 release.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/cinder 14.0.0.0rc1 release candidate.

[Jeremy Stanley (fungi)]

As this was fixed in master before the release of Stein, the last affected release was Rocky which has been under "extended maintenance" (no longer receiving stable point releases) for nearly a year already. I doubt there's much point in trying to arrange publishing an advisory at this point, so I'm marking our OSSA task won't fix.