Not all volume drivers set the 'encrypted' key in connection_info

Bug #1440227 reported by Anthony Lee
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Cinder
Fix Released
Medium
Matt Riedemann

Bug Description

The following two tempest tests report a pass when a volume is not actually encrypted properly:

test_encrypted_cinder_volumes_cryptsetup
test_encrypted_cinder_volumes_luks

There was a bug in the HP 3PAR drivers where the 'encrypted' property was not being returned from initialize_connection and thus never triggering an of the encryption code paths.

Here is the code path that gets skipped:

https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L1047

The above two tempest tests give a false pass even though the volume created and attached it was not done using the encryption code path.

Sample log for false positive passing test:

http://15.126.198.151/82/160682/48/check/3par-iscsi-driver-master-client-pip-eos10-dsvm/64b698e/logs/testr_results.html.gz

Once fixed the code paths were actually tested and result in a valid pass.

Here is the Cinder patch that fixes the 3PAR drivers so that they return the 'encrypted' property correctly:

https://review.openstack.org/#/c/170346/

Now the encrypted code path is actually tested and the pass is valid.

Log for actual passing test:

http://15.126.198.151/46/170346/2/check/3par-iscsi-driver-master-client-pip-eos10-dsvm/eeae704/logs/testr_results.html.gz

Revision history for this message
Duncan Thomas (duncan-thomas) wrote :

How can a test driving from the outside (i.e. tempest) tell that something went wrong? We have no interfaces for that...

Revision history for this message
Duncan Thomas (duncan-thomas) wrote :

Ok, so I misunderstood what was going on here. IMO the bug is that the driver every even finds out that host based encryption is being used, that info should be added to the connection properties by the manager IMO

Revision history for this message
Duncan Thomas (duncan-thomas) wrote :

And actually, for devstack based installs, tempest could call initialise_connection and check the output. It wouldn't work on a well configured secure cloud, but might be useful for devstack/CI?

Revision history for this message
David Kranz (david-kranz) wrote :

Tempest is a black box test suite using public OpenStack apis. This should be tested at a functional level in cinder.

Changed in tempest:
status: New → Invalid
Revision history for this message
Anthony Lee (anthony-mic-lee) wrote :

Changing to invalid since this has been open a while. Driver's will need to add unit tests to check that the 'encrypted' flag is properly set if they are interested in making sure the Tempest test isn't passing as a false positive.

Changed in cinder:
status: New → Invalid
Revision history for this message
Matt Riedemann (mriedem) wrote :

See related bug 1463525.

summary: - encrypted volume tests don't check if a volume is actually encrypted
+ Not all volume drivers set the 'encrypted' key in connection_info
Changed in cinder:
status: Invalid → Confirmed
Revision history for this message
Matt Riedemann (mriedem) wrote :

https://review.openstack.org/#/c/193673/ fixed the rbd volume driver to pass the 'encrypted' flag in connection_info during os-initialize_connection, but there are many other volume drivers in cinder that aren't setting that key, so bug should address that.

tags: added: low-hanging-fruit
Revision history for this message
Matt Riedemann (mriedem) wrote :

Seems the volume manager that calls volume driver.initialize_connection could just handle this globally, i.e. check the returned connection_info['data'] and if 'encrypted' is not in there, add it using the generic logic of:

connection_info['data']['encrypted'] = volume.get('encryption_key_id') is not None

Matt Riedemann (mriedem)
Changed in cinder:
status: Confirmed → In Progress
assignee: nobody → Matt Riedemann (mriedem)
importance: Undecided → Medium
Revision history for this message
Matt Riedemann (mriedem) wrote :
Revision history for this message
Matt Riedemann (mriedem) wrote :
no longer affects: tempest
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to cinder (master)

Reviewed: https://review.openstack.org/193673
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=ba6f4037ab982b3bcf2de48cb51f919b3f37cbaf
Submitter: Jenkins
Branch: master

commit ba6f4037ab982b3bcf2de48cb51f919b3f37cbaf
Author: Matt Riedemann <email address hidden>
Date: Fri Jun 19 11:25:43 2015 -0700

    Set encrypted key in connection_info during initialize

    There are encrypted volume tests in Tempest which are passing
    in the ceph job but the encryption providers in nova (luks and
    cryptsetup) are not actually being called in nova because the
    'encrypted' key isn't being set in connection_info for the Rbd
    volume driver, and nova is keying off that value to determine
    if it needs to run it's encryption providers.

    So the tests are passing in the ceph job but it's a total false
    positive and the API should actually fail - either the cinder
    API to create the encrypted volumes if the Rbd volume driver in
    Cinder doesn't support encryption, or the volume attach call in
    nova if it can't encrypt the connected volume, either way the
    test should fail until that support is baked into nova.

    I'm not aware of any kind of 'supports_encryption' or 'encryptable'
    capability flag for cinder volume drivers, but that might be
    needed for the volume create API to fail if trying to create a
    volume from an encrypted volume type where the volume driver
    itself doesn't support encryption - but maybe it's only a nova
    problem since nova has the encryption providers.

    This is fixed generically in the VolumeManager's
    initialize_connection method where we check for the encrypted
    flag being set in the volume driver method and if not, the
    manager sets it based on the volume.encryption_key_id value.
    We do this globally since there are other volume drivers besides
    rbd that aren't setting the encrypted key in connection_info,
    but we test rbd in the gate via the ceph job so most of the
    bug tracking is around validation of the ceph job.

    Related Nova change: I8efc2628b09d4e9e59831353daa080b20e17ccde

    Depends-On: I8548d41095513b9e669f773e3f35353e9228ead9

    DocImpact: Attaching 'encrypted' RBD volumes to a Nova server
               instance created from the libvirt virt driver will fail
               since RBD volume encryption is not currently supported in
               Nova's libvirt driver.

    Closes-Bug: #1440227
    Related-Bug: #1463525

    Change-Id: I03f8cae05cc117e14f7482115de685fc9f3fa54a

Changed in cinder:
status: In Progress → Fix Committed
Changed in cinder:
milestone: none → liberty-2
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in cinder:
milestone: liberty-2 → 7.0.0
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.