Encryptor connect_volume not changing the symlink

Bug #1967790 reported by Gorka Eguileor
18
This bug affects 4 people
Affects Status Importance Assigned to Milestone
os-brick
Fix Released
High
Gorka Eguileor

Bug Description

There are systems where it has been confirmed that creating an encrypted volume (iSCSI or FC) from an image has resulted in the data being written unencrypted on the disk.

It is expected that this same issue could happen on the Nova side when connecting volumes to instances.

This not only results in an unusable volume (since it fails to attach on an instance), but also in a security risk, because the volume now has the image data unencrypted.

Cases were this frequently happened was when Linux was running in FIPS mode.

Upon investigation, this happens because after returning from the connect_volume call to the os-brick encryptor the symlink that should be pointing to the decrypted device mapper is pointing to the raw device instead.

Upon closer inspection turns out that it's not that the os-brick encryptor code is not replacing the symlink, but that there is a race condition between the os-brick code and the system's udev rules that ends up replacing the symlink.

It all happens because when cryptsetup opens the device (already formatted or not) it generates an kernel uevent that udevd detects and ends up triggering the system udev rules.

Setting the debug mode on udev allows us to see this event:

 systemd-udevd[2610688]: inotify event: 8 for /dev/sda
 systemd-udevd[2610688]: device /dev/sda closed, synthesising 'change'

 . . .

 systemd-udevd[2139145]: found 'b8:0' claiming '/run/udev/links/\x2fdisk\x2fby-id\x2fwwn-0x6001405c74cec22c8334adc84c610bcd'
 systemd-udevd[2139145]: creating link '/dev/disk/by-id/wwn-0x6001405c74cec22c8334adc84c610bcd' to '/dev/sda'
 systemd-udevd[2139145]: preserve already existing symlink '/dev/disk/by-id/wwn-0x6001405c74cec22c8334adc84c610bcd' to '../../sda

The claiming message is because the device already exists and it was pointing to the cryptsetup Device Mapper device, so it's overwritting it.

So this is the race condition that happens:

- cinder calls connect_volume => returns /dev/disk/by-id/wwwn-... linking to /dev/sda
- cinder call connect_volume on the encryptor
  - os-brick calls cryptsetup
    - cryptsetup triggers the udev rules
    - os-brick makes /dev/disk/by-id/wwwn-... link to /dev/mapper/crypt-scsi-...
  - the udev rule claims the symlink and makes it point to /dev/sda again
- cinder writes data to the symlink

Since it's a race condition it doesn't happen every time, as it depends on the speed the udev rules run. If the udev rule checking the /dev/disk/by-id/www- symlink happens "before" os-brick replacing it, then everything works as expected.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to os-brick (master)

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/os-brick/+/836391

Changed in os-brick:
status: New → In Progress
Changed in os-brick:
importance: Undecided → Medium
Changed in os-brick:
importance: Medium → Critical
Changed in os-brick:
importance: Critical → High
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to os-brick (master)

Reviewed: https://review.opendev.org/c/openstack/os-brick/+/836391
Committed: https://opendev.org/openstack/os-brick/commit/1583a5038d34fd560b554e0eee5c1c5a7612722f
Submitter: "Zuul (22348)"
Branch: master

commit 1583a5038d34fd560b554e0eee5c1c5a7612722f
Author: Gorka Eguileor <email address hidden>
Date: Mon Apr 4 20:01:39 2022 +0200

    Fix encryption symlink issues

    This patch fixes 2 issues related to the symlinks, or lack of, that
    connectors' connect_volume methods return.

    Some connectors always return the block device instead of a symlink for
    encrypted volumes, and other connectors return a symlink that is owned
    by the system's udev rules. Both cases are problematic

    Returning the real device can prevent the encryptor connect_volume to
    complete successfully, and in other cases (such as nvmeof) it completes,
    but on the connector's disconnect volume it will leave the device behind
    (i.e., /dev/nvme0n1) preventing new connections that would use that same
    device name.

    Returning a symlink owned by the system's udev rules means that they can
    be reclaimed by those rules at any time. This can happen with
    cryptsetup, because when it creates a new mapping it triggers udev rules
    for the device that can reclaim the symlink after os-brick has replaced
    it.

    This patch creates a couple of decorators to facilitate this for all
    connectors. These decorators transform the paths so that the callers
    gets the expected symlink, but the connector doesn't need to worry about
    it and will always see the value it returns regardless of what symlink
    the caller gets.

    From this moment onwards we use our own custom symlink that starts with
    "/dev/disk/by-id/os-brick".

    The patch fixes bugs in other connectors (such as the RBD local
    connection), but since there are no open bugs they have not been
    reported.

    Closes-Bug: #1964379
    Closes-Bug: #1967790
    Change-Id: Ie373ab050dcc0a35c749d9a53b6cf5ca060bcb58

Changed in os-brick:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to os-brick (stable/yoga)

Fix proposed to branch: stable/yoga
Review: https://review.opendev.org/c/openstack/os-brick/+/845845

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to os-brick (stable/yoga)

Reviewed: https://review.opendev.org/c/openstack/os-brick/+/845845
Committed: https://opendev.org/openstack/os-brick/commit/0bd5dc99152261d1d59ecb788faf4aea3e299edd
Submitter: "Zuul (22348)"
Branch: stable/yoga

commit 0bd5dc99152261d1d59ecb788faf4aea3e299edd
Author: Gorka Eguileor <email address hidden>
Date: Mon Apr 4 20:01:39 2022 +0200

    Fix encryption symlink issues

    This patch fixes 2 issues related to the symlinks, or lack of, that
    connectors' connect_volume methods return.

    Some connectors always return the block device instead of a symlink for
    encrypted volumes, and other connectors return a symlink that is owned
    by the system's udev rules. Both cases are problematic

    Returning the real device can prevent the encryptor connect_volume to
    complete successfully, and in other cases (such as nvmeof) it completes,
    but on the connector's disconnect volume it will leave the device behind
    (i.e., /dev/nvme0n1) preventing new connections that would use that same
    device name.

    Returning a symlink owned by the system's udev rules means that they can
    be reclaimed by those rules at any time. This can happen with
    cryptsetup, because when it creates a new mapping it triggers udev rules
    for the device that can reclaim the symlink after os-brick has replaced
    it.

    This patch creates a couple of decorators to facilitate this for all
    connectors. These decorators transform the paths so that the callers
    gets the expected symlink, but the connector doesn't need to worry about
    it and will always see the value it returns regardless of what symlink
    the caller gets.

    From this moment onwards we use our own custom symlink that starts with
    "/dev/disk/by-id/os-brick".

    The patch fixes bugs in other connectors (such as the RBD local
    connection), but since there are no open bugs they have not been
    reported.

    Closes-Bug: #1964379
    Closes-Bug: #1967790
    Change-Id: Ie373ab050dcc0a35c749d9a53b6cf5ca060bcb58
    (cherry picked from commit 1583a5038d34fd560b554e0eee5c1c5a7612722f)

tags: added: in-stable-yoga
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to os-brick (stable/xena)

Fix proposed to branch: stable/xena
Review: https://review.opendev.org/c/openstack/os-brick/+/846343

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to os-brick (stable/xena)

Reviewed: https://review.opendev.org/c/openstack/os-brick/+/846343
Committed: https://opendev.org/openstack/os-brick/commit/b31b109f0f95c0102300bbda15b011f3623a2873
Submitter: "Zuul (22348)"
Branch: stable/xena

commit b31b109f0f95c0102300bbda15b011f3623a2873
Author: Gorka Eguileor <email address hidden>
Date: Mon Apr 4 20:01:39 2022 +0200

    Fix encryption symlink issues

    This patch fixes 2 issues related to the symlinks, or lack of, that
    connectors' connect_volume methods return.

    Some connectors always return the block device instead of a symlink for
    encrypted volumes, and other connectors return a symlink that is owned
    by the system's udev rules. Both cases are problematic

    Returning the real device can prevent the encryptor connect_volume to
    complete successfully, and in other cases (such as nvmeof) it completes,
    but on the connector's disconnect volume it will leave the device behind
    (i.e., /dev/nvme0n1) preventing new connections that would use that same
    device name.

    Returning a symlink owned by the system's udev rules means that they can
    be reclaimed by those rules at any time. This can happen with
    cryptsetup, because when it creates a new mapping it triggers udev rules
    for the device that can reclaim the symlink after os-brick has replaced
    it.

    This patch creates a couple of decorators to facilitate this for all
    connectors. These decorators transform the paths so that the callers
    gets the expected symlink, but the connector doesn't need to worry about
    it and will always see the value it returns regardless of what symlink
    the caller gets.

    From this moment onwards we use our own custom symlink that starts with
    "/dev/disk/by-id/os-brick".

    The patch fixes bugs in other connectors (such as the RBD local
    connection), but since there are no open bugs they have not been
    reported.

    Closes-Bug: #1964379
    Closes-Bug: #1967790
    Change-Id: Ie373ab050dcc0a35c749d9a53b6cf5ca060bcb58
    (cherry picked from commit 1583a5038d34fd560b554e0eee5c1c5a7612722f)
    (cherry picked from commit 0bd5dc99152261d1d59ecb788faf4aea3e299edd)
    Conflicts:
            os_brick/initiator/connectors/lightos.py
            os_brick/utils.py

tags: added: in-stable-xena
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/os-brick 5.2.1

This issue was fixed in the openstack/os-brick 5.2.1 release.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/os-brick 6.0.0

This issue was fixed in the openstack/os-brick 6.0.0 release.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/os-brick 5.0.3

This issue was fixed in the openstack/os-brick 5.0.3 release.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to os-brick (stable/wallaby)

Fix proposed to branch: stable/wallaby
Review: https://review.opendev.org/c/openstack/os-brick/+/856576

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to os-brick (stable/wallaby)

Reviewed: https://review.opendev.org/c/openstack/os-brick/+/856576
Committed: https://opendev.org/openstack/os-brick/commit/cf69f9247061bd0a945a6fb6bf688acff617eb2c
Submitter: "Zuul (22348)"
Branch: stable/wallaby

commit cf69f9247061bd0a945a6fb6bf688acff617eb2c
Author: Gorka Eguileor <email address hidden>
Date: Mon Apr 4 20:01:39 2022 +0200

    Fix encryption symlink issues

    This patch fixes 2 issues related to the symlinks, or lack of, that
    connectors' connect_volume methods return.

    Some connectors always return the block device instead of a symlink for
    encrypted volumes, and other connectors return a symlink that is owned
    by the system's udev rules. Both cases are problematic

    Returning the real device can prevent the encryptor connect_volume to
    complete successfully, and in other cases (such as nvmeof) it completes,
    but on the connector's disconnect volume it will leave the device behind
    (i.e., /dev/nvme0n1) preventing new connections that would use that same
    device name.

    Returning a symlink owned by the system's udev rules means that they can
    be reclaimed by those rules at any time. This can happen with
    cryptsetup, because when it creates a new mapping it triggers udev rules
    for the device that can reclaim the symlink after os-brick has replaced
    it.

    This patch creates a couple of decorators to facilitate this for all
    connectors. These decorators transform the paths so that the callers
    gets the expected symlink, but the connector doesn't need to worry about
    it and will always see the value it returns regardless of what symlink
    the caller gets.

    From this moment onwards we use our own custom symlink that starts with
    "/dev/disk/by-id/os-brick".

    The patch fixes bugs in other connectors (such as the RBD local
    connection), but since there are no open bugs they have not been
    reported.

    Closes-Bug: #1964379
    Closes-Bug: #1967790
    Change-Id: Ie373ab050dcc0a35c749d9a53b6cf5ca060bcb58
    (cherry picked from commit 1583a5038d34fd560b554e0eee5c1c5a7612722f)
    (cherry picked from commit 0bd5dc99152261d1d59ecb788faf4aea3e299edd)
    (cherry picked from commit b31b109f0f95c0102300bbda15b011f3623a2873)
    Conflicts:
            os_brick/initiator/connectors/iscsi.py

tags: added: in-stable-wallaby
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/os-brick 4.3.4

This issue was fixed in the openstack/os-brick 4.3.4 release.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to os-brick (stable/victoria)

Fix proposed to branch: stable/victoria
Review: https://review.opendev.org/c/openstack/os-brick/+/866461

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on os-brick (stable/victoria)

Change abandoned by "Elod Illes <email address hidden>" on branch: stable/victoria
Review: https://review.opendev.org/c/openstack/os-brick/+/866461
Reason: stable/victoria branch of openstack/os-brick is about to be deleted. To be able to do that, all open patches need to be abandoned. Please cherry pick the patch to unmaintained/victoria if you want to further work on this patch.

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.