Attaching LVM encrypted volumes (with LUKS) could cause data loss if LUKS headers get corrupted

[Patrizio Tufarolo]

I have doubts about the flow of the volume attaching operation, as defined in /usr/lib/python2.6/site-packages/nova/volume/encryptors/luks.py.

If the device is not recognized to be a valid luks device, the script is luks formatting it! So if for some reason the luks header get corrupted, it erases the whole data.
To manage corrupted headers there are the

    cryptsetup luksHeaderBackup

and

    cryptsetup luksHeaderRestore

commands that respectively do the backup and the restore of the headers.

I think that the process has to be reviewed, and the luksFormat operation has to be performed during the volume creation.

[Sean Dague (sdague)]

Why is this marked as a security bug? It seems like a data loss bug.

[Patrizio Tufarolo (patriziotufarolo)]

I marked it as security bug because it is related to a security function that can compromise data integrity. Should I move it elsewhere?

[Sean Dague (sdague)]

I think this is a normal data loss bug. I'm happy to have it as high, but I don't think it should be categorized as a security bug.

[Patrizio Tufarolo (patriziotufarolo)]

I am so sorry for the mistake. I think it is really important to have it fixed because it could cause whole damages to companies that use LUKS in a production environment.

[Duncan Thomas (duncan-thomas)]

Adding cinder on since it is likely the fix will end up in cinder

[Joel Coffman (joel-coffman)]

The question here is really what should happen if the LUKS header become corrupted for some reason.

Implicit to that question is the assumption that the header could be corrupted without also impacting the integrity of the data stored in the volume. While it's probably possible, my inclination is that corrupting the header would also likely corrupt other portions of the volume in which case the user would probably want to restore the volume from a backup. (See patch to support backups of encrypted volumes: https://review.openstack.org/#/c/74532/)

Regarding the use of luksHeaderBackup and luksHeaderRestore, where do you propose storing the backup header file? Would a backup of the whole volume (see above) be sufficient in your opinion, or is there a specific need to backup only the header?

Finally, the decision to format the device in Nova instead of Cinder was intentional: because Cinder never has access to the encryption key (it merely requests the creation of an encryption key), only the compute host must be trusted. That is, the current flow limits trust among the various services in OpenStack. (I do not argue that flow could be different, but there are security trade-offs that should be considered with such a change.)

[Patrizio Tufarolo (patriziotufarolo)]

The bug I submitted wants to be related more to the flow of the volume encryption procedure than to the LUKS header backup, that is also important.
I have no idea, at the moment, on where to store luks headers' backup, I'm looking around.
However a backup of the whole volume could be sufficient, but in my opinion it could become expensive in some cases.
I focused on luksHeaderBackup and luksHeaderRestore to highlight on how formatting a volume is not a valid choice when the device is not recognized as valid luks.
I agree with the decision to format the device in Nova, but I think that Cinder should ask luksFormat to nova at the moment of volume creation, and not when attaching for the problem above.

[Joel Coffman (joel-coffman)]

> [...] I think that Cinder should ask luksFormat to nova at the moment of volume creation [...]

I agree that approach would be cleaner, but it also forces Cinder to delegate a significant amount of work to Nova -- it's basically comparable to attaching a volume to a VM. I'm also not really sure that (encrypted) volume creation should couple Cinder to Nova. It would certainly be a significant change from the creation of non-encrypted volumes.

While it's not optimal, the existing approach is simple -- much simpler than the alternatives in my opinion.

[Patrizio Tufarolo (patriziotufarolo)]

A simpler approach could consist in a flag stored in the cinder db to indicate if the volume has been formatted before.
Then we could handle the volume attachment procedure checking this flag's value first.
If the device has been used but Nova would not be able to mount it, we could return an exception inviting the cloud administrator to check for the volume integrity.
What do you think about that?

[Joel Coffman (joel-coffman)]

That approach seems good in my opinion -- no real downsides apart from the additional metadata.

[John Griffith (john-griffith)]

Adding a flag seems reasonable, I'm actually undecided however if that should be a new key in the DB or perhaps just some admin-meta. My preference here would be admin-meta and I believe that would work here.

I'm also not sure how the implementation would end up looking, first glance it seems like it would be fairly clean to add the check

[Sean Dague (sdague)]

This seems like this really should be handled in cinder scope, marking nova invalid for now. Please flag back to new if we feel it should be in the fix.

[John Griffith (john-griffith)]

@Patrizio/Joel
Couple questions, like how would we like to see the info passed to Cinder to indicate that the Volume has been formatted to set the proposed flag? This is part of the reason why this was all handled on the Nova side to begin with, that's where the encryption *stuff* happens for the most part, and given we don't currently have any mechanism for Nova sending updates back to Cinder there's a good deal of plumbing needed here I think.

Also, is anybody interested enough to work on this?

[Patrizio Tufarolo (patriziotufarolo)]

I don't know if someone could be interested.
As of now, I think we can leave the encryption layer in nova but I would at least change the workflow of the volume encryption operation.
It is crazy to luks format a volume because I am not able to mount it, and it is crazy to suppose that if I am not able to mount a volume, then it's the first time I am mounting it.

[Joel Coffman (joel-coffman)]

> [...] how would we like to see the info passed to Cinder to indicate that the Volume has been formatted to set the proposed flag?

Perhaps something could be added to the VolumeEncryptionMetadata API extension to support toggling the flag when the volume is formatted. Not sure how much would be gained from this approval since it potentially would create a way to (maliciously) trigger reformatting the volume -- maybe it would be write-once so it can only be set (i.e., formatted = True).

> It is crazy to luks format a volume because I am not able to mount it, and it is crazy to suppose that if I am not able to mount a volume, then it's the first time I am mounting it.

You could use the cryptsetup encryptor instead of LUKS, as raw cryptsetup does not format the volume at all.

> Also, is anybody interested enough to work on this?

I'm willing to look into this issue since I'm responsible for the original feature, but it's pretty much at the bottom of my priority list.

I also stand by my original comment on this bug report. We're talking about a situation where 1) the LUKS header is corrupted, 2) the (encrypted) volume "data" is not corrupted, and 3) the user doesn't have backups or snapshots of the volume. Perhaps someone from the Cinder core team will correct me, but I'd guess that Cinder's backends try to avoid data corruption, but it remains the user's responsibility to have snapshots or backups of the volume in case corruption occurs. If so, we're talking about a very specific situation where changing the existing behavior would be beneficial.

[Robert Clark (robert-clark)]

> -----Original Message-----
> From: Dane Fichter [mailto:<email address hidden>]
> Sent: 23 April 2015 14:28
> To: <email address hidden>
> Subject: [Openstack-security] [Bug 1372375] OpenStack Docs
>
> Hey Joel,
>
> besides just docs.openstack.org, is there any other source of OpenStack
> documentation?
>
> Dane

Hi Dane,

This is a good place to start:
https://wiki.openstack.org/wiki/Security