Merge pull request #1908 from freedomofpress/fixup-whole-block-encryp…

…tion-support Support unlocked unpartitioned VeraCrypt block device for export
freedomofpress · Mar 13, 2024 · 873883d · 873883d
2 parents a4fbac4 + 7138134
commit 873883d
Show file tree

Hide file tree

Showing 5 changed files with 53 additions and 23 deletions.
diff --git a/client/securedrop_client/gui/conversation/export/export_wizard_constants.py b/client/securedrop_client/gui/conversation/export/export_wizard_constants.py
@@ -31,7 +31,8 @@ class Pages(IntEnum):
     ExportStatus.INVALID_DEVICE_DETECTED: _(
         "Either the drive is not encrypted or there is something else wrong with it."
         "<br />"
-        "If this is a VeraCrypt drive, please unlock it from within `sd-devices`, then try again."
+        "If this is a VeraCrypt drive, please unlock it from within "
+        "the sd-devices VM, then try again."
     ),
     ExportStatus.DEVICE_WRITABLE: _("The device is ready for export."),
     ExportStatus.DEVICE_LOCKED: _("The device is locked."),

diff --git a/client/securedrop_client/locale/messages.pot b/client/securedrop_client/locale/messages.pot
@@ -282,7 +282,7 @@ msgstr ""
 msgid "Too many USB devices detected; please insert one supported device."
 msgstr ""
 
-msgid "Either the drive is not encrypted or there is something else wrong with it.<br />If this is a VeraCrypt drive, please unlock it from within `sd-devices`, then try again."
+msgid "Either the drive is not encrypted or there is something else wrong with it.<br />If this is a VeraCrypt drive, please unlock it from within the sd-devices VM, then try again."
 msgstr ""
 
 msgid "The device is ready for export."

diff --git a/export/securedrop_export/disk/cli.py b/export/securedrop_export/disk/cli.py
@@ -113,12 +113,20 @@ def get_volume(self) -> Union[Volume, MountedVolume]:
                     # Inspect partitions or whole volume.
                     # For sanity, we will only support encrypted partitions one level deep.
                     if "children" in device:
-                        for partition in device.get("children"):
-                            # /dev/sdX1, /dev/sdX2 etc
-                            item = self._get_supported_volume(partition)  # type: ignore
-                            if item:
-                                volumes.append(item)  # type: ignore
-                    # /dev/sdX
+                        for child in device.get("children"):
+                            # Whole block device is encrypted (and unlocked)
+                            if child.get("type") == "crypt" and device.get("type") == "disk":
+                                logger.debug("Checking device {device}")
+                                item = self._get_supported_volume(device)  # type: ignore
+                                if item:
+                                    volumes.append(item)
+                            else:
+                                # /dev/sdX1, /dev/sdX2
+                                logger.debug("Checking partition {child}")
+                                item = self._get_supported_volume(child)  # type: ignore
+                                if item:
+                                    volumes.append(item)  # type: ignore
+                    # /dev/sdX and it's locked
                     else:
                         item = self._get_supported_volume(device)  # type: ignore
                         if item:
@@ -178,22 +186,21 @@ def _get_supported_volume(self, device: dict) -> Optional[Union[Volume, MountedV
                 if vol.encryption == EncryptionScheme.UNKNOWN:
                     vol.encryption = self._is_it_veracrypt(vol)
 
-                if children[0].get("mountpoint"):
-                    logger.debug(f"{vol.device_name} is mounted")
+                # To opportunistically mount any unlocked encrypted partition
+                # (i.e. third-party devices such as IronKeys), remove this condition.
+                if vol.encryption in (EncryptionScheme.LUKS, EncryptionScheme.VERACRYPT):
+                    logger.debug(f"{vol.device_name} encryption scheme is supported")
 
-                    return MountedVolume(
-                        device_name=vol.device_name,
-                        unlocked_name=mapped_name,
-                        encryption=vol.encryption,
-                        mountpoint=children[0].get("mountpoint"),
-                    )
-                else:
-                    # To opportunistically mount any unlocked encrypted partition
-                    # (i.e. third-party devices such as IronKeys), remove this condition.
-                    if vol.encryption in (
-                        EncryptionScheme.LUKS,
-                        EncryptionScheme.VERACRYPT,
-                    ):
+                    if children[0].get("mountpoint"):
+                        logger.debug(f"{vol.device_name} is mounted")
+
+                        return MountedVolume(
+                            device_name=vol.device_name,
+                            unlocked_name=mapped_name,
+                            encryption=vol.encryption,
+                            mountpoint=children[0].get("mountpoint"),
+                        )
+                    else:
                         logger.debug(f"{device_name} is unlocked but unmounted; attempting mount")
                         return self._mount_volume(vol, mapped_name)
 
@@ -212,6 +219,7 @@ def _is_it_veracrypt(self, volume: Volume) -> EncryptionScheme:
         enable VeraCrypt drive detection, which we ship with this package.
         """
         try:
+            logger.debug(f"Check if {volume.device_name} is an unlocked VeraCrypt device")
             info = subprocess.check_output(
                 [
                     "udisksctl",

diff --git a/export/tests/disk/test_cli.py b/export/tests/disk/test_cli.py
@@ -28,6 +28,7 @@
     UDISKS_STATUS_MULTI_CONNECTED,
     UDISKS_STATUS_NOTHING_CONNECTED,
     UDISKS_STATUS_ONE_DEVICE_CONNECTED,
+    WHOLE_DEVICE_VC_WRITABLE,
 )
 
 _PRETEND_LUKS_ID = "/dev/mapper/luks-dbfb85f2-77c4-4b1f-99a9-2dd3c6789094"
@@ -41,6 +42,7 @@
     SINGLE_DEVICE_LOCKED,
     SINGLE_PART_LUKS_WRITABLE,
     SINGLE_PART_VC_WRITABLE,
+    WHOLE_DEVICE_VC_WRITABLE,
 ]
 
 # Volume, expected device name, expected mapped device name

diff --git a/export/tests/lsblk_sample.py b/export/tests/lsblk_sample.py
@@ -84,6 +84,25 @@
     ],
 }
 
+WHOLE_DEVICE_VC_WRITABLE = {
+    "name": "sda",
+    "rm": True,
+    "ro": False,
+    "type": "disk",
+    "mountpoint": None,
+    "fstype": None,
+    "children": [
+        {
+            "name": "tcrypt-2049",
+            "rm": False,
+            "ro": False,
+            "type": "crypt",
+            "mountpoint": "/media/usb/tcrypt-1234",
+            "fstype": "vfat",
+        }
+    ],
+}
+
 SINGLE_PART_LUKS_UNLOCKED_UNMOUNTED = {
     "name": "sda1",
     "type": "part",