[OSSA 2013-021] Cinder LVM volume driver does not support secure deletion (CVE-2013-4183)

[Rongze Zhu]

the delete action process:
    delete_snapshot(self, snapshot) -> self._delete_volume(snapshot, snapshot['volume_size']) -> self.clear_volume(volume)

    def clear_volume(self, volume):
        """unprovision old volumes to prevent data leaking between users."""

        vol_path = self.local_path(volume)
        size_in_g = volume.get('size')
        size_in_m = self.configuration.volume_clear_size

        if not size_in_g:
            LOG.warning(_("Size for volume: %s not found, "
                          "skipping secure delete.") % volume['name'])
            return
         ....
         ....

Because snapshot without 'size' field, so it will skip secure delete.

[Thierry Carrez (ttx)]

I think it definitely warrants an advisory. Feel free to correct me.

[John Griffith (john-griffith)]

It would warrant one, only because of the expectation.

[Jeremy Stanley (fungi)]

Proposed impact description (will be used in CVE request and public advisory):

----------------------------------
Title: Cinder LVMVolumeDriver does not zero deleted snapshots
Reporter: Rongze Zhu
Products: Cinder
Affects: 2012.2 (Grizzly) and later

Description:
Rongze Zhu reported a vulnerability in the Cinder LVM volume driver.
When deleting an LVM volume snapshot the previous contents may not
be zeroed, resulting in potential exposure of latent data to
subsequent servers for other tenants. Only setups using
LVMVolumeDriver are affected.
----------------------------------

Everyone: please check that the description is accurate.

Rongze Zhu: do you want us to additionally credit the company you work for (SINA, UnitedStack, anyone)?

[Jeremy Stanley (fungi)]

Mmm, I just noticed I got the Affects header wrong. It should be...

Affects: 2013.1 (Grizzly) and later

[Eric Harney (eharney)]

re: Description

Is it true that only LVMVolumeDriver is affected? What about ThinLVMVolumeDriver? (Looks possible.)

[Thierry Carrez (ttx)]

Description looks good, just make sure to cover Eric's concerns.

[Jeremy Stanley (fungi)]

Yes, since I don't personally know much about the interrelationships between various Cinder drivers, I'm hoping someone will answer his question at which point I can update the description accordingly.

[John Griffith (john-griffith)]

Ummm.... do a block by block delete on a thinly provisioned device? That seems like a quick way to make a mess of things. Don't think that's a good idea at all.

So IMO the description could be changed slightly:
The issue here IMO is not so much that the data is not being zero'd BUT that the folks implementing this are expecting
that it is.

We currently have a flag that allows you to turn on/off secure delete, and keep in mind this was turned off on snapshots
intentionally due to the kernel hang issue on Ubuntu/Precise.

There have been a number of proposals for trying to do this in a way that might not cause the kernel crash but they've
encountered objection in reviews (but that's another topic).

[Jeremy Stanley (fungi)]

Okay, so stepping back a bit, it's not really a question of literally zeroing the block device in this case, but rather whether or not secure delete is enabled for LVM2 snapshots. Does this mean the report here is effectively a duplicate of bug 1134768? If so, it sounds like we're proposing an announcement that secure delete is off by default because it's broken on Debian derived distributions, but that deployers who want to enable it for their environments are free to do so.

If that's the situation, this probably is better suited to drop into the OSSN queue (likely with a CVE assignment), rather than issue an OSSA. Am I misinterpreting this?

[John Griffith (john-griffith)]

On Thu, Jul 11, 2013 at 9:48 AM, Jeremy Stanley <email address hidden> wrote:

> Okay, so stepping back a bit, it's not really a question of literally
> zeroing the block device in this case, but rather whether or not secure
> delete is enabled for LVM2 snapshots. Does this mean the report here is
> effectively a duplicate of bug 1134768? If so, it sounds like we're
>

Yes, it's a duplicate.. and before anyone asks, no I don't remember why I
marked it as won't fix, but that should not have been the case.

proposing an announcement that secure delete is off by default because
> it's broken on Debian derived distributions, but that deployers who want
> to enable it for their environments are free to do so.
>

That's my thought exactly, I'll actually just set a flag and turn the old
"dd/zero" method back on as an option while folks argue about the proposals
currently in flight for how to do this better.

>
> If that's the situation, this probably is better suited to drop into the
> OSSN queue (likely with a CVE assignment), rather than issue an OSSA. Am
> I misinterpreting this?
>

I was unfamiliar with OSSN, but that does seem more appropriate to me.

>
> --
> You received this bug notification because you are a member of Cinder
> Bug Team, which is subscribed to Cinder.
> https://bugs.launchpad.net/bugs/1198185
>
> Title:
> delete_snapshot in LVMVolumeDriver not really zero the snapshot
>
> Status in Cinder:
> In Progress
> Status in OpenStack Security Advisories:
> Triaged
>
> Bug description:
> the delete action process:
> delete_snapshot(self, snapshot) -> self._delete_volume(snapshot,
> snapshot['volume_size']) -> self.clear_volume(volume)
>
> def clear_volume(self, volume):
> """unprovision old volumes to prevent data leaking between
> users."""
>
> vol_path = self.local_path(volume)
> size_in_g = volume.get('size')
> size_in_m = self.configuration.volume_clear_size
>
> if not size_in_g:
> LOG.warning(_("Size for volume: %s not found, "
> "skipping secure delete.") % volume['name'])
> return
> ....
> ....
>
>
> Because snapshot without 'size' field, so it will skip secure delete.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/cinder/+bug/1198185/+subscriptions
>

[John Griffith (john-griffith)]

On Thu, Jul 11, 2013 at 9:48 AM, Jeremy Stanley <email address hidden> wrote:
Okay, so stepping back a bit, it's not really a question of literally
zeroing the block device in this case, but rather whether or not secure
delete is enabled for LVM2 snapshots. Does this mean the report here is
effectively a duplicate of bug 1134768? If so, it sounds like we're

<jdg>:Yes, it's a duplicate.. and before anyone asks, no I don't remember why I marked it as won't fix, but that should not have been the case.</jdg>

proposing an announcement that secure delete is off by default because
it's broken on Debian derived distributions, but that deployers who want
to enable it for their environments are free to do so.

<jdg>:That's my thought exactly, I'll actually just set a flag and turn the old "dd/zero" method back on as an option while folks argue about the proposals currently in flight for how to do this better.</jdg>

If that's the situation, this probably is better suited to drop into the
OSSN queue (likely with a CVE assignment), rather than issue an OSSA. Am
I misinterpreting this?

<jdg>I was unfamiliar with OSSN, but that does seem more appropriate to me.</jdg>

[Jeremy Stanley (fungi)]

Thierry and Mikal may correct me on this, but my interpretation is that if it's a security vulnerability in OpenStack for which we've got a fix (maybe even if that just means beefing up available options for deployers or making more verbose disclaimers in logs and interfaces), then the VMT issues an OSSA. On the other hand if there's nothing to be fixed and this is merely a matter of setting long-term expectations on the security failings of a particular feature for the benefit of improved understanding within the OpenStack community, then the OSSG issues an OSSN (with or without a CVE as appropriate).

[Thierry Carrez (ttx)]

@fungi: your interpretation is correct... although I wouldn't consider "adding an option that defaults to disabled" to be an acceptable fix, and if that is our answer to an issue, then it should fall on the OSSN bucket (think "known issues and limitations").

I propose we mark this one as a dupe of the other, and that we reopen the other... but first I would like to get the consequences right. We don't zero on delete, but are we cleaning up the volume before it's given to new users ? Or do all default deployers of Cinder/LVM expect to expose stale data from previous tenants on block devices they hand out ? If the latter, I'd be tempted to change the default for secure_delete to ON and let the distros fix their kernel bug (or take the responsibility to change that option default themselves).

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/36506
Committed: http://github.com/openstack/cinder/commit/0ee31073c5cb432a9cdd2648e99aa802b0ed0a17
Submitter: Jenkins
Branch: master

commit 0ee31073c5cb432a9cdd2648e99aa802b0ed0a17
Author: Rongze Zhu <email address hidden>
Date: Wed Jul 10 09:25:32 2013 -0700

    Enable zero the snapshot when delete snapshot in LVMVolumeDriver

    Because snapshot without 'size' field, So clear_volume method in
    LVMVolumeDriver will skip secure deleting. Get the size of snapshot from
    'volume_size' filed, So it can zero the snapshot.

    Remove the 'size_in_g' parameter in _delete_volume method, because it never
    used. Add a unittest for clear_volume method.

    Fixes Bug #1198185

    Change-Id: Ie919b50ce4fb276f29ab2e0279f868a691ea7bef

[Thierry Carrez (ttx)]

So after discussing this with jgriffith I think it definitely warrants an OSSA because people with secure delete enabled may currently be handing out non-zeroed space in case of deleted snapshots.

As an additional thing (to be filed as another bug) for havana I would consider setting secure_delete to ON and let the distros workaround kernel bugs they may still have by keeping it disabled in their own packaging.

[John Griffith (john-griffith)]

Ok, so after further conversations, this should be a SA, the driver should do secure delete by default and it's up to the distros to fix their bugs.

@eharney: As far as the question regarding thin LVM driver, my opinion is that since the blocks here aren't allocated until they're actually used that it should not be necessary. That being said we should have an intelligent method implemented that actually handles allocated blocks (and only allocated blocks) just to give everybody a warm fuzzy. I would say that it should not be part of this SA but however we should have this work in place as a feature addition to the thin driver for the H release.

[Jeremy Stanley (fungi)]

That being the case, I think a slightly different impact description is in order:

----------------------------------
Title: Cinder LVM volume drivers do not support secure deletion
Reporter: Rongze Zhu
Products: Cinder
Affects: 2013.1 (Grizzly) and later

Description:
Rongze Zhu reported a vulnerability in Cinder LVM volume drivers.
The contents of LVM snapshots may not be cleared upon deletion even
when secure deletes are configured, resulting in potential exposure
of latent data to subsequent servers for other tenants. Only setups
using LVMVolumeDriver or ThinLVMVolumeDriver are affected.
----------------------------------

Everyone: please check that the description is accurate.

Rongze Zhu: do you want us to additionally credit the company you work for (SINA, UnitedStack, anyone)?

[Thierry Carrez (ttx)]

@Jeremy: I would s/or ThinLVMVolumeDriver// since thinly-provisioned volumes are not vulnerable (blocks are not accessed until they are overwritten, per comment #16).

[Rongze Zhu (zrzhit)]

@Jeremy , I am currently working in unitedstack, thank you.

[Jeremy Stanley (fungi)]

Third (and hopefully final) version...

----------------------------------
Title: Cinder LVM volume driver does not support secure deletion
Reporter: Rongze Zhu (UnitedStack)
Products: Cinder
Affects: 2013.1 (Grizzly) and later

Description:
Rongze Zhu from UnitedStack reported a vulnerability in the Cinder
LVM volume driver. The contents of LVM snapshots may not be cleared
upon deletion even when secure deletes are configured, resulting in
potential exposure of latent data to subsequent servers for other
tenants. Only setups using LVMVolumeDriver are affected.
----------------------------------

[Thierry Carrez (ttx)]

+1 on description

[Thierry Carrez (ttx)]

We need the grizzly backport and a CVE allocated before we move on to advisory publication

[John Griffith (john-griffith)]

Do it as soon as I get to the office

On Mon, Jul 29, 2013 at 4:01 AM, Thierry Carrez <<email address hidden>
> wrote:

> We need the grizzly backport and a CVE allocated before we move on to
> advisory publication
>
> ** Also affects: cinder/grizzly
> Importance: Undecided
> Status: New
>
> --
> You received this bug notification because you are a member of Cinder
> Bug Team, which is subscribed to Cinder.
> https://bugs.launchpad.net/bugs/1198185
>
> Title:
> delete_snapshot in LVMVolumeDriver not really zero the snapshot
>
> Status in Cinder:
> Fix Released
> Status in Cinder grizzly series:
> New
> Status in OpenStack Security Advisories:
> Triaged
>
> Bug description:
> the delete action process:
> delete_snapshot(self, snapshot) -> self._delete_volume(snapshot,
> snapshot['volume_size']) -> self.clear_volume(volume)
>
> def clear_volume(self, volume):
> """unprovision old volumes to prevent data leaking between
> users."""
>
> vol_path = self.local_path(volume)
> size_in_g = volume.get('size')
> size_in_m = self.configuration.volume_clear_size
>
> if not size_in_g:
> LOG.warning(_("Size for volume: %s not found, "
> "skipping secure delete.") % volume['name'])
> return
> ....
> ....
>
>
> Because snapshot without 'size' field, so it will skip secure delete.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/cinder/+bug/1198185/+subscriptions
>

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/grizzly
Review: https://review.openstack.org/39565

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/39565
Committed: http://github.com/openstack/cinder/commit/68c597e26b5659a036a7a937622e539bac102308
Submitter: Jenkins
Branch: stable/grizzly

commit 68c597e26b5659a036a7a937622e539bac102308
Author: Rongze Zhu <email address hidden>
Date: Wed Jul 10 09:25:32 2013 -0700

    Enable zero the snapshot when delete snapshot in LVMVolumeDriver

    Because snapshot without 'size' field, So clear_volume method in
    LVMVolumeDriver will skip secure deleting. Get the size of snapshot from
    'volume_size' filed, So it can zero the snapshot.

    Remove the 'size_in_g' parameter in _delete_volume method, because it never
    used. Add a unittest for clear_volume method.

    Fixes Bug #1198185

    Change-Id: Ie919b50ce4fb276f29ab2e0279f868a691ea7bef
    (cherry picked from commit 0ee31073c5cb432a9cdd2648e99aa802b0ed0a17)