diff --git a/.github/workflows/build-rpm.yml b/.github/workflows/build-rpm.yml index 61af6950..965b0797 100644 --- a/.github/workflows/build-rpm.yml +++ b/.github/workflows/build-rpm.yml @@ -335,12 +335,15 @@ jobs: # Regression coverage for issues #140 / #141 on disk-persist + stable-key # (the combinations the 24-job test-convert matrix does NOT exercise, since # it uses the otp key provider which recreates the delta every boot). + # Both scenarios boot, then RE-boot the same persistent overlay; both + # boots must reach login. # fsck-141: disk-persist + integrity=true -> first-boot dm-snapshot - # creation must succeed (COW header fully initialized through - # the authenticated mapping). - # fsck-140: disk-persist + integrity=false -> boot, then RE-boot the same - # persistent overlay; both boots must reach login (offline - # resize2fs must not refuse a clean fs with s_lastcheck offline resize2fs must not + # refuse a clean fs with s_lastcheck < s_mtime on the second + # boot (e2fsck runs first). # UKI only (both bugs fire in the dracut FDE service, bootloader-agnostic). # +4 jobs: 2 distros x 2 scenarios. test-convert-persist: @@ -363,7 +366,7 @@ jobs: - scenario: fsck-141 delta_key: stable integrity: "true" - second_boot: "" + second_boot: "true" runs-on: ubuntu-latest needs: build env: diff --git a/cryptpilot-fde/src/cmd/boot_service/stage/before_sysroot.rs b/cryptpilot-fde/src/cmd/boot_service/stage/before_sysroot.rs index 801c5aba..c2f20954 100644 --- a/cryptpilot-fde/src/cmd/boot_service/stage/before_sysroot.rs +++ b/cryptpilot-fde/src/cmd/boot_service/stage/before_sysroot.rs @@ -24,6 +24,15 @@ use cryptpilot::{ const CRYPTPILOT_LVM_SYSTEM_DIR: &str = "/usr/lib/cryptpilot/lvm/"; +/// dm-snapshot chunk size in 512-byte sectors, shared by the snapshot table +/// and the COW header initialization: the persistent snapshot driver reads +/// and writes whole chunks, so any region the guest initializes on the COW +/// device must cover a full chunk or the read fails (with dm-integrity, +/// sectors never written through the mapping have no valid tag). +const SNAPSHOT_CHUNK_SIZE_SECTORS: u64 = 16; + +const SNAPSHOT_CHUNK_SIZE_BYTES: usize = SNAPSHOT_CHUNK_SIZE_SECTORS as usize * 512; + pub async fn setup_volumes_required_by_fde() -> Result<()> { let fde_config = crate::config::get_fde_config_source() .await @@ -165,6 +174,7 @@ pub async fn setup_volumes_required_by_fde() -> Result<()> { dm_verity_output_device, Path::new(DELTA_DEVICE), matches!(delta_location, DeltaLocation::DiskPersist), + recreate, ) .await?; @@ -191,8 +201,13 @@ pub async fn setup_volumes_required_by_fde() -> Result<()> { tracing::info!("Creating zram device for COW storage"); let cow_device = create_zram_cow_device().await?; // Build dm-snapshot device chain - setup_dm_snapshot_device_chain(dm_verity_output_device, &cow_device, false) - .await?; + setup_dm_snapshot_device_chain( + dm_verity_output_device, + &cow_device, + false, + false, + ) + .await?; // Resize rootfs filesystem to fill the expanded device after building snapshot chain resize_ext4_filesystem(Path::new(ROOTFS_DEVICE)).await?; } @@ -437,8 +452,11 @@ async fn create_zram_cow_device() -> Result { Ok(PathBuf::from(format!("/dev/zram{}", zram_id))) } -/// Wipe a device by writing zeros using tokio async I/O. -/// Writes 4KB megabytes of zeros to the device. +/// Zero the COW header region through the device mapping. +/// +/// Must cover a full snapshot chunk: the persistent snapshot driver reads +/// and writes whole chunks, and with dm-integrity only sectors written +/// through the mapping carry a valid tag. async fn wipe_cow_device_header_async(device_path: &Path) -> Result<(), anyhow::Error> { use tokio::fs::OpenOptions; @@ -448,18 +466,53 @@ async fn wipe_cow_device_header_async(device_path: &Path) -> Result<(), anyhow:: .await .context("Failed to open device for wiping")?; - file.write_all(&[0u8; 4096]).await?; - - // Final sync to ensure everything is written + file.write_all(&vec![0u8; SNAPSHOT_CHUNK_SIZE_BYTES]) + .await?; file.sync_all().await?; - Ok(()) } +/// Wipe the COW header chunk only when the header region reads back blank. +/// +/// blkid reports "no signatures" both for a genuinely blank device and for +/// one whose probe reads fail (dm-integrity rejects sectors whose tags were +/// never written, e.g. the end-of-device probe), so "no signatures" alone +/// is not proof of blank. The header chunk is: unreadable means we cannot +/// tell (fail closed), non-zero content is left untouched for the snapshot +/// to interpret (valid "SnAp" metadata gets reused; anything else makes +/// dmsetup fail closed), all zeros mean blank and safe to initialize. +async fn wipe_cow_header_after_read_check(device_path: &Path) -> Result<(), anyhow::Error> { + use tokio::fs::OpenOptions; + use tokio::io::AsyncReadExt; + + let mut file = OpenOptions::new() + .read(true) + .open(device_path) + .await + .context("Failed to open COW device for header check")?; + + let mut header = vec![0u8; SNAPSHOT_CHUNK_SIZE_BYTES]; + file.read_exact(&mut header).await.context( + "COW header chunk is unreadable, cannot tell whether the volume is blank; refusing to wipe", + )?; + drop(file); + + if header.iter().any(|&b| b != 0) { + tracing::info!( + "COW header has existing content, keeping it for the snapshot to reuse \ + (blkid misreported the device as clean)" + ); + return Ok(()); + } + + wipe_cow_device_header_async(device_path).await +} + async fn setup_dm_snapshot_device_chain( rootfs_device: &Path, cow_device: &Path, persistent: bool, + fresh_delta: bool, ) -> Result<()> { tracing::info!( ?rootfs_device, @@ -506,6 +559,15 @@ async fn setup_dm_snapshot_device_chain( wipe_cow_device_header_async(cow_device) .await .context("Failed to wipe COW device")?; + } else if fresh_delta { + // The COW device was created on this boot and is blank by + // construction; unwritten sectors fail integrity reads, so probing + // here would report "no signatures" for the wrong reason. Initialize + // the header chunk directly. + tracing::info!("Persistent mode: fresh delta, initializing COW header"); + wipe_cow_device_header_async(cow_device) + .await + .context("Failed to wipe COW device")?; } else { // Persistent mode: probe COW device to determine safe action tracing::info!("Persistent mode: probing COW device state"); @@ -513,9 +575,11 @@ async fn setup_dm_snapshot_device_chain( match probe { cryptpilot::fs::blkid::BlkidProbeResult::NoSignatures => { - // Clean device, safe to wipe - tracing::info!("COW device is clean, wiping"); - wipe_cow_device_header_async(cow_device) + // blkid's "no signatures" also covers probe reads that + // failed, so the header chunk itself decides: wiped only + // when blank, kept when it holds snapshot metadata. + tracing::info!("COW device reports clean, verifying header"); + wipe_cow_header_after_read_check(cow_device) .await .context("Failed to wipe COW device")?; } @@ -546,11 +610,12 @@ async fn setup_dm_snapshot_device_chain( .arg(ROOTFS_NAME) .arg("--table") .arg(format!( - "0 {} snapshot {} {} {} 16", // chunk size is 16 sectors (8KB) + "0 {} snapshot {} {} {} {}", linear_size, ROOTFS_EXTENDED_DEVICE, cow_device.to_string_lossy(), - if persistent { "PO" } else { "N" } + if persistent { "PO" } else { "N" }, + SNAPSHOT_CHUNK_SIZE_SECTORS )) .run() .await @@ -649,6 +714,67 @@ mod tests { Ok(()) } + #[tokio::test] + async fn test_wipe_cow_header_covers_full_snapshot_chunk() -> Result<()> { + let dir = tempfile::tempdir()?; + let cow = dir.path().join("cow.img"); + // Non-zero pattern makes a partial wipe visible. + tokio::fs::write(&cow, vec![0xAAu8; SNAPSHOT_CHUNK_SIZE_BYTES * 2]).await?; + + wipe_cow_device_header_async(&cow).await?; + + let data = tokio::fs::read(&cow).await?; + assert!( + data[..SNAPSHOT_CHUNK_SIZE_BYTES].iter().all(|&b| b == 0), + "wipe must zero the whole first chunk the snapshot driver reads" + ); + assert!(data[SNAPSHOT_CHUNK_SIZE_BYTES..].iter().all(|&b| b == 0xAA)); + Ok(()) + } + + #[tokio::test] + async fn test_guarded_wipe_requires_readable_header_chunk() -> Result<()> { + let dir = tempfile::tempdir()?; + let cow = dir.path().join("short.img"); + // Shorter than one chunk: reading the header region fails, so the + // wipe must refuse rather than treat the failure as a blank device. + tokio::fs::write(&cow, vec![0xAAu8; SNAPSHOT_CHUNK_SIZE_BYTES / 2]).await?; + + let result = wipe_cow_header_after_read_check(&cow).await; + + assert!(result.is_err(), "unreadable header must abort the wipe"); + let data = tokio::fs::read(&cow).await?; + assert!( + data.iter().all(|&b| b == 0xAA), + "refused wipe must leave the device untouched" + ); + Ok(()) + } + + #[tokio::test] + async fn test_guarded_wipe_preserves_nonzero_header() -> Result<()> { + let dir = tempfile::tempdir()?; + let cow = dir.path().join("snapcow.img"); + // dm-snapshot persistent metadata starts with the "SnAp" magic. + // blkid misreports such a device as clean when its probe reads fail + // under dm-integrity, so the wipe must verify the header content + // itself and keep anything non-zero for the snapshot to interpret. + let mut header = vec![0u8; SNAPSHOT_CHUNK_SIZE_BYTES]; + header[0..4].copy_from_slice(b"SnAp"); + header[8..12].copy_from_slice(&1u32.to_le_bytes()); + header[12..16].copy_from_slice(&(SNAPSHOT_CHUNK_SIZE_SECTORS as u32).to_le_bytes()); + tokio::fs::write(&cow, &header).await?; + + wipe_cow_header_after_read_check(&cow).await?; + + let data = tokio::fs::read(&cow).await?; + assert_eq!( + data, header, + "non-zero header holds dm-snapshot metadata; wiping it would destroy the previous boot's delta" + ); + Ok(()) + } + #[tokio::test] async fn test_resize_clean_ext4_checked_before_last_mount() -> Result<()> { let (_dir, image) = ext4_image().await?; diff --git a/tests/test-convert.sh b/tests/test-convert.sh index c1440423..78d6953d 100755 --- a/tests/test-convert.sh +++ b/tests/test-convert.sh @@ -739,8 +739,22 @@ test_qemu_boot_direct() { log::step "Direct QEMU boot for: ${test_name} (cpu=${cpu_cores}, ram=${ram_size}, image=${image_bn})" + # Alinux 4 ships real Docker (not podman-docker) and the test container + # has no init system, so dockerd must be started explicitly. No-op on + # Alinux 3 where docker is the daemonless podman emulation. A self-started + # dockerd has no bridge, so the QEMU container needs the host network + # namespace for guest user-mode networking (NTP time sync). + if ! ensure_docker_runtime; then + return 1 + fi + local docker_net_args=() + if [[ "${DOCKERD_STARTED:-0}" == "1" ]]; then + docker_net_args+=(--network=host) + fi + docker rm -f "${container_name}" 2>/dev/null || true docker run -d --rm --privileged \ + "${docker_net_args[@]}" \ -v "$(dirname "${image}"):/diskdir:rw" \ --name "${container_name}" --entrypoint /bin/bash \ "ghcr.io/qemus/qemu:7.29" -c ' @@ -765,7 +779,12 @@ exec qemu-system-x86_64 \ boot_success=true break fi - if grep -qi "Emergency Mode\|emergency shell\|Kernel panic\|Failed to setup volumes required by FDE\|Failed to create dm-snapshot\|INTEGRITY AEAD ERROR\|Please run 'e2fsck\|Failed to resize ext4\|Failed to start Cryptpilot FDE" "${boot_log}" 2>/dev/null; then + # NOTE: bare "INTEGRITY AEAD ERROR" is not a boot-failure marker: + # blkid probing the end of a freshly created delta reads sectors + # whose integrity tags were never initialized, and those errors are + # noise. The fatal markers are the dmsetup failure, emergency mode, + # or the FDE service failing. + if grep -qi "Emergency Mode\|emergency shell\|Kernel panic\|Failed to setup volumes required by FDE\|Failed to create dm-snapshot device\|reload ioctl .* failed\|Please run 'e2fsck\|Failed to resize ext4\|Failed to start Cryptpilot FDE" "${boot_log}" 2>/dev/null; then log::error "Boot failure detected - boot failed!" break fi @@ -799,13 +818,18 @@ test_qemu_boot_persist_second() { local cpu_cores="${3:-4}" local ram_size="${4:-4G}" - local overlay="${WORKDIR}/${test_name}-persist-overlay.qcow2" - local overlay_bn; overlay_bn=$(basename "${overlay}") + # The overlay must live in the SAME directory as the output image: the + # backing file is referenced by its bare name so qemu resolves it inside + # the boot container (host workdir <-> container /diskdir), and the boot + # container only mounts that one directory. + local image_dir; image_dir="$(dirname "${output_image}")" + local overlay="${image_dir}/${test_name}-persist-overlay.qcow2" rm -f "${overlay}" - # Relative backing filename so the path resolves inside the container - # (host workdir <-> container /diskdir share the same directory). - ( cd "$(dirname "${overlay}")" \ - && qemu-img create -f qcow2 -F qcow2 -b "$(basename "${output_image}")" "${overlay_bn}" >/dev/null ) + if ! ( cd "${image_dir}" \ + && qemu-img create -f qcow2 -F qcow2 -b "$(basename "${output_image}")" "$(basename "${overlay}")" >/dev/null ); then + log::error "Failed to create persistent overlay in ${image_dir} for: ${test_name}" + return 1 + fi log::step "Persist two-boot test for: ${test_name}" log::info "Boot #1 (expect success; fs mounted RW updates s_mtime via the persistent delta)"