Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lxd/instance/driver/qemu: replace sha1 by sha256 in blockNodeName() #12454

Merged
merged 1 commit into from
Oct 24, 2023

Conversation

simondeziel
Copy link
Member

While this sha1 usage is perfectly fine as it's not used for its security properties, there is no downside to using sha256 instead. On the other hand, not using sha1 anywhere makes auditing this part trivial.

While this sha1 usage is perfectly fine as it's not used for its
security properties, there is no downside to using sha256 instead.
On the other hand, not using sha1 anywhere makes auditing this part
trivial.

Signed-off-by: Simon Deziel <simon.deziel@canonical.com>
Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this potentially change the device name of the disk inside the guest? In that case we cannot accept the patch as it will potentially change the devices that users are expecting to see inside the guest.

@simondeziel
Copy link
Member Author

@tomponline while I didn't test, I would think it doesn't because commit 55228ba changed the hash a few months after the initial hash was introduced in commit 13ede0e.

@tomponline
Copy link
Member

@tomponline while I didn't test, I would think it doesn't because commit 55228ba changed the hash a few months after the initial hash was introduced in commit 13ede0e.

IIRC we had to do that otherwise qemu wouldnt start.

@tomponline
Copy link
Member

But please can you try it and see. Thanks

@simondeziel
Copy link
Member Author

I followed the reproduction instructions from #11129 and sdb always appears with the proper name and disk ID whether sha1 or sha256 is used.

Using QMP (thanks for showing me how to us it btw!) here is what we have for sha256:

$ lxc config set kubedee-rookery-worker-xlhfra raw.qemu="-qmp tcp:127.0.0.1:4444,server,nowait"
...
$ nc 127.0.0.1 4444
{"QMP": {"version": {"qemu": {"micro": 1, "minor": 1, "major": 8}, "package": "v8.1.1-dirty"}, "capabilities": ["oob"]}}
{"execute": "qmp_capabilities"}
{ "execute": "query-block" }
{"return": [{"device": "pflash0", "locked": false, "removable": false, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 3653632, "filename": "/snap/lxd/current/share/qemu/OVMF_CODE.4MB.fd", "format": "raw", "actual-size": 3653632, "dirty-flag": false}, "iops_wr": 0, "ro": true, "node-name": "#block193", "backing_file_depth": 0, "drv": "raw", "iops": 0, "bps_wr": 0, "write_threshold": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "cache": {"no-flush": false, "direct": false, "writeback": true}, "file": "/snap/lxd/current/share/qemu/OVMF_CODE.4MB.fd"}, "qdev": "/machine/system.flash0", "type": "unknown"}, {"device": "pflash1", "locked": false, "removable": false, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 540672, "filename": "/dev/fd/4", "format": "raw", "actual-size": 4096, "dirty-flag": false}, "iops_wr": 0, "ro": false, "node-name": "#block360", "backing_file_depth": 0, "drv": "raw", "iops": 0, "bps_wr": 0, "write_threshold": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "cache": {"no-flush": false, "direct": false, "writeback": true}, "file": "/dev/fd/4"}, "qdev": "/machine/system.flash1", "type": "unknown"}, {"io-status": "ok", "device": "", "locked": false, "removable": false, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 10737418240, "filename": "/dev/fdset/0", "format": "host_device", "actual-size": 0, "format-specific": {"type": "file", "data": {}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "node-name": "lxd_root", "backing_file_depth": 0, "drv": "host_device", "iops": 0, "bps_wr": 0, "write_threshold": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "cache": {"no-flush": false, "direct": true, "writeback": true}, "file": "/dev/fdset/0"}, "qdev": "dev-lxd_root", "type": "unknown"}, {"io-status": "ok", "device": "", "locked": false, "removable": false, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 16106127360, "filename": "/dev/fdset/1", "format": "host_device", "actual-size": 0, "format-specific": {"type": "file", "data": {}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "node-name": "lxd_GY8X_x4olTAei12HLInzTniNMF6", "backing_file_depth": 0, "drv": "host_device", "iops": 0, "bps_wr": 0, "write_threshold": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "cache": {"no-flush": false, "direct": true, "writeback": true}, "file": "/dev/fdset/1"}, "qdev": "dev-lxd_kubedee--rookery--worker--xlhfra--sdb", "type": "unknown"}]}

Which parses as:

    {
      "io-status": "ok",
      "device": "",
      "locked": false,
      "removable": false,
      "inserted": {
        "iops_rd": 0,
        "detect_zeroes": "off",
        "image": {
          "virtual-size": 16106127360,
          "filename": "/dev/fdset/1",
          "format": "host_device",
          "actual-size": 0,
          "format-specific": {
            "type": "file",
            "data": {}
          },
          "dirty-flag": false
        },
        "iops_wr": 0,
        "ro": false,
        "node-name": "lxd_GY8X_x4olTAei12HLInzTniNMF6",
        "backing_file_depth": 0,
        "drv": "host_device",
        "iops": 0,
        "bps_wr": 0,
        "write_threshold": 0,
        "encrypted": false,
        "bps": 0,
        "bps_rd": 0,
        "cache": {
          "no-flush": false,
          "direct": true,
          "writeback": true
        },
        "file": "/dev/fdset/1"
      },
      "qdev": "dev-lxd_kubedee--rookery--worker--xlhfra--sdb",
      "type": "unknown"
    }

And for sha1:

...
{"return": [{"device": "pflash0", "locked": false, "removable": false, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 3653632, "filename": "/snap/lxd/current/share/qemu/OVMF_CODE.4MB.fd", "format": "raw", "actual-size": 3653632, "dirty-flag": false}, "iops_wr": 0, "ro": true, "node-name": "#block105", "backing_file_depth": 0, "drv": "raw", "iops": 0, "bps_wr": 0, "write_threshold": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "cache": {"no-flush": false, "direct": false, "writeback": true}, "file": "/snap/lxd/current/share/qemu/OVMF_CODE.4MB.fd"}, "qdev": "/machine/system.flash0", "type": "unknown"}, {"device": "pflash1", "locked": false, "removable": false, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 540672, "filename": "/dev/fd/4", "format": "raw", "actual-size": 4096, "dirty-flag": false}, "iops_wr": 0, "ro": false, "node-name": "#block372", "backing_file_depth": 0, "drv": "raw", "iops": 0, "bps_wr": 0, "write_threshold": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "cache": {"no-flush": false, "direct": false, "writeback": true}, "file": "/dev/fd/4"}, "qdev": "/machine/system.flash1", "type": "unknown"}, {"io-status": "ok", "device": "", "locked": false, "removable": false, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 10737418240, "filename": "/dev/fdset/0", "format": "host_device", "actual-size": 0, "format-specific": {"type": "file", "data": {}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "node-name": "lxd_root", "backing_file_depth": 0, "drv": "host_device", "iops": 0, "bps_wr": 0, "write_threshold": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "cache": {"no-flush": false, "direct": true, "writeback": true}, "file": "/dev/fdset/0"}, "qdev": "dev-lxd_root", "type": "unknown"}, {"io-status": "ok", "device": "", "locked": false, "removable": false, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 16106127360, "filename": "/dev/fdset/1", "format": "host_device", "actual-size": 0, "format-specific": {"type": "file", "data": {}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "node-name": "lxd_LFMsfxCWUIm-eJTuxPtLkTkbRb8", "backing_file_depth": 0, "drv": "host_device", "iops": 0, "bps_wr": 0, "write_threshold": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "cache": {"no-flush": false, "direct": true, "writeback": true}, "file": "/dev/fdset/1"}, "qdev": "dev-lxd_kubedee--rookery--worker--xlhfra--sdb", "type": "unknown"}]}

Which parses as:

    {
      "io-status": "ok",
      "device": "",
      "locked": false,
      "removable": false,
      "inserted": {
        "iops_rd": 0,
        "detect_zeroes": "off",
        "image": {
          "virtual-size": 16106127360,
          "filename": "/dev/fdset/1",
          "format": "host_device",
          "actual-size": 0,
          "format-specific": {
            "type": "file",
            "data": {}
          },
          "dirty-flag": false
        },
        "iops_wr": 0,
        "ro": false,
        "node-name": "lxd_LFMsfxCWUIm-eJTuxPtLkTkbRb8",
        "backing_file_depth": 0,
        "drv": "host_device",
        "iops": 0,
        "bps_wr": 0,
        "write_threshold": 0,
        "encrypted": false,
        "bps": 0,
        "bps_rd": 0,
        "cache": {
          "no-flush": false,
          "direct": true,
          "writeback": true
        },
        "file": "/dev/fdset/1"
      },
      "qdev": "dev-lxd_kubedee--rookery--worker--xlhfra--sdb",
      "type": "unknown"
    }

So only the node-name differs (sha1: lxd_LFMsfxCWUIm-eJTuxPtLkTkbRb8 and sha256: lxd_GY8X_x4olTAei12HLInzTniNMF6) and in both cases, qdev is the same and that what seems to be exposed inside the guest:

# with sha256:
$ lxc exec kubedee-rookery-worker-xlhfra -- sh -c 'ls -l /dev/disk/by-*/*' | grep sdb
lrwxrwxrwx 1 root root  9 Oct 24 18:21 /dev/disk/by-diskseq/2 -> ../../sdb
lrwxrwxrwx 1 root root  9 Oct 24 18:21 /dev/disk/by-id/scsi-0QEMU_QEMU_HARDDISK_lxd_kubedee--rookery -> ../../sdb
lrwxrwxrwx 1 root root  9 Oct 24 18:21 /dev/disk/by-path/pci-0000:02:00.0-scsi-0:0:1:1 -> ../../sdb

# with sha1:
$ lxc exec kubedee-rookery-worker-xlhfra -- sh -c 'ls -l /dev/disk/by-*/*' | grep sdb
lrwxrwxrwx 1 root root  9 Oct 24 18:34 /dev/disk/by-diskseq/2 -> ../../sdb
lrwxrwxrwx 1 root root  9 Oct 24 18:34 /dev/disk/by-id/scsi-0QEMU_QEMU_HARDDISK_lxd_kubedee--rookery -> ../../sdb
lrwxrwxrwx 1 root root  9 Oct 24 18:34 /dev/disk/by-path/pci-0000:02:00.0-scsi-0:0:1:1 -> ../../sdb

@tomponline tomponline merged commit bab0a18 into canonical:main Oct 24, 2023
26 checks passed
@simondeziel simondeziel deleted the no-more-sha1 branch October 24, 2023 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants