From c9da460b6cb34f9ad259371bfc3d324fbb13a636 Mon Sep 17 00:00:00 2001 From: Ondrej Kozina Date: Wed, 13 Apr 2022 12:33:43 +0200 Subject: [PATCH] Do not allow dangerous sector size change during reencryption. By changing encryption sector size during reencryption we may increase effective logical block size for dm-crypt active device. For example if hosted filesystem on encrypted data device has block size set to 512 bytes and we increase dm-crypt logical size durign reencryption to 4096 bytes it breaks the filesystem. Do not allow encryption sector size to be increased over value provided by fs superblock in BLOCK_SIZE property. The check is applied while initialising LUKS2 device encryption (reencrypt --encrypt/--new) or when initialising LUKS2 reencryption on active dm-crypt device. Note that this check cannot be applied on offline device (data device is encrypted). --- src/utils_reencrypt.c | 66 +++++++++++++++++++++++++++++--- tests/Makefile.am | 1 + tests/luks2-reencryption-test | 40 ++++++++++++++++++- tests/xfs_512_block_size.img.xz | Bin 0 -> 6284 bytes 4 files changed, 101 insertions(+), 6 deletions(-) create mode 100644 tests/xfs_512_block_size.img.xz diff --git a/src/utils_reencrypt.c b/src/utils_reencrypt.c index d2028257..9320e86c 100644 --- a/src/utils_reencrypt.c +++ b/src/utils_reencrypt.c @@ -302,6 +302,40 @@ static enum device_status_info check_luks_device(const char *device) return dev_st; } +static int reencrypt_check_data_sb_block_size(const char *data_device, uint32_t new_sector_size) +{ + int r; + char sb_name[32]; + unsigned block_size; + + assert(data_device); + + r = tools_superblock_block_size(data_device, sb_name, sizeof(sb_name), &block_size); + if (r <= 0) + return r; + + if (new_sector_size > block_size) { + log_err(_("Requested --sector-size %" PRIu32 " is incompatible with %s superblock\n" + "(block size: %" PRIu32 " bytes) detected on device %s."), + new_sector_size, sb_name, block_size, data_device); + return -EINVAL; + } + + return 0; +} + +static int reencrypt_check_active_device_sb_block_size(const char *active_device, uint32_t new_sector_size) +{ + int r; + char dm_device[PATH_MAX]; + + r = snprintf(dm_device, sizeof(dm_device), "%s/%s", crypt_get_dir(), active_device); + if (r < 0 || (size_t)r >= sizeof(dm_device)) + return -EINVAL; + + return reencrypt_check_data_sb_block_size(dm_device, new_sector_size); +} + static int encrypt_luks2_init(struct crypt_device **cd, const char *data_device, const char *device_name) { int keyslot, r, fd; @@ -350,6 +384,12 @@ static int encrypt_luks2_init(struct crypt_device **cd, const char *data_device, return -EINVAL; } + if (ARG_SET(OPT_SECTOR_SIZE_ID)) { + r = reencrypt_check_data_sb_block_size(data_device, ARG_UINT32(OPT_SECTOR_SIZE_ID)); + if (r < 0) + return r; + } + if (!ARG_SET(OPT_UUID_ID)) { uuid_generate(uuid); uuid_unparse(uuid, uuid_str); @@ -850,12 +890,28 @@ static int reencrypt_luks2_init(struct crypt_device *cd, const char *data_device if (r < 0) goto out; - if (!ARG_SET(OPT_FORCE_OFFLINE_REENCRYPT_ID) && !ARG_SET(OPT_INIT_ONLY_ID)) + /* + * with --init-only lookup active device only if + * blkid probes are allowed and sector size change + * is requested. + */ + if (!ARG_SET(OPT_FORCE_OFFLINE_REENCRYPT_ID) && + (!ARG_SET(OPT_INIT_ONLY_ID) || (tools_blkid_supported() && sector_size_change))) { r = reencrypt_get_active_name(cd, data_device, &active_name); - if (r >= 0) - r = crypt_reencrypt_init_by_passphrase(cd, active_name, kp[keyslot_old].password, - kp[keyslot_old].passwordLen, keyslot_old, kp[keyslot_old].new, - cipher, mode, ¶ms); + if (r < 0) + goto out; + } + + if (sector_size_change && active_name) { + r = reencrypt_check_active_device_sb_block_size(active_name, luks2_params.sector_size); + if (r < 0) + goto out; + } + + r = crypt_reencrypt_init_by_passphrase(cd, + ARG_SET(OPT_INIT_ONLY_ID) ? NULL : active_name, + kp[keyslot_old].password, kp[keyslot_old].passwordLen, + keyslot_old, kp[keyslot_old].new, cipher, mode, ¶ms); out: crypt_safe_free(vk); if (kp) { diff --git a/tests/Makefile.am b/tests/Makefile.am index c24fc451..11a36413 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -50,6 +50,7 @@ EXTRA_DIST = compatimage.img.xz compatv10image.img.xz \ conversion_imgs.tar.xz \ luks2_keyslot_unassigned.img.xz \ img_fs_ext4.img.xz img_fs_vfat.img.xz img_fs_xfs.img.xz \ + xfs_512_block_size.img.xz \ valid_header_file.xz \ luks2_valid_hdr.img.xz \ luks2_header_requirements.xz \ diff --git a/tests/luks2-reencryption-test b/tests/luks2-reencryption-test index c3390aec..4cd42b82 100755 --- a/tests/luks2-reencryption-test +++ b/tests/luks2-reencryption-test @@ -19,6 +19,7 @@ DEV_NAME2=reenc97682 IMG=reenc-data IMG_HDR=$IMG.hdr HEADER_LUKS2_PV=blkid-luks2-pv.img +IMG_FS=xfs_512_block_size.img KEY1=key1 VKEY1=vkey1 PWD1="93R4P4pIqAH8" @@ -99,7 +100,7 @@ function remove_mapping() [ -b /dev/mapper/$OVRDEV-err ] && dmsetup remove --retry $OVRDEV-err 2>/dev/null [ -n "$LOOPDEV" ] && losetup -d $LOOPDEV unset LOOPDEV - rm -f $IMG $IMG_HDR $KEY1 $VKEY1 $DEVBIG $DEV_LINK $HEADER_LUKS2_PV >/dev/null 2>&1 + rm -f $IMG $IMG_HDR $KEY1 $VKEY1 $DEVBIG $DEV_LINK $HEADER_LUKS2_PV $IMG_FS >/dev/null 2>&1 rmmod scsi_debug >/dev/null 2>&1 scsi_debug_teardown $DEV } @@ -717,6 +718,11 @@ function valgrind_run() INFOSTRING="$(basename ${BASH_SOURCE[1]})-line-${BASH_LINENO[0]}" ./valg.sh ${CRYPTSETUP_VALGRIND} "$@" } +function bin_check() +{ + command -v $1 >/dev/null || skip "WARNING: test require $1 binary, test skipped." +} + [ $(id -u) != 0 ] && skip "WARNING: You must be root to run this test, test skipped." [ ! -x "$CRYPTSETUP" ] && skip "Cannot find $CRYPTSETUP, test skipped." fips_mode && skip "This test cannot be run in FIPS mode." @@ -1724,5 +1730,37 @@ $CRYPTSETUP isLuks $HEADER_LUKS2_PV && fail echo $PWD1 | $CRYPTSETUP reencrypt -q --header $IMG_HDR $HEADER_LUKS2_PV $FAST_PBKDF_ARGON --encrypt --force-offline-reencrypt --type luks2 2>/dev/null && fail test -f $IMG_HDR && fail +echo "[31] Prevent dangerous sector size increase" +bin_check blkid +preparebig 64 +xz -dk $IMG_FS.xz +blkid $IMG_FS | grep -q BLOCK_SIZE && BLKID_BLOCK_SIZE_SUPPORT=1 +if [ -n "$DM_SECTOR_SIZE" -a -n "$BLKID_BLOCK_SIZE_SUPPORT" ]; then + # encryption checks must work in offline mode + echo $PWD1 | $CRYPTSETUP reencrypt --encrypt --force-offline-reencrypt --sector-size 1024 -q --header $IMG_HDR $IMG_FS $FAST_PBKDF_ARGON --init-only --type luks2 2>/dev/null && fail + test -f $IMG_HDR && fail + + echo $PWD1 | $CRYPTSETUP reencrypt --encrypt --force-offline-reencrypt --sector-size 1024 -q --header $IMG_HDR $IMG_FS $FAST_PBKDF_ARGON --type luks2 2>/dev/null && fail + test -f $IMG_HDR && fail + + echo $PWD1 | $CRYPTSETUP reencrypt --encrypt --force-offline-reencrypt --sector-size 1024 -q --reduce-device-size 8m $IMG_FS $FAST_PBKDF_ARGON --init-only --type luks2 2>/dev/null && fail + $CRYPTSETUP isLuks $IMG_FS && fail + echo $PWD1 | $CRYPTSETUP reencrypt --encrypt --force-offline-reencrypt --sector-size 1024 -q --reduce-device-size 8m $IMG_FS $FAST_PBKDF_ARGON --type luks2 2>/dev/null && fail + $CRYPTSETUP isLuks $IMG_FS && fail + + echo $PWD1 | $CRYPTSETUP luksFormat -q --sector-size 512 --type luks2 $FAST_PBKDF_ARGON $DEV || fail + echo $PWD1 | $CRYPTSETUP open -q $DEV $DEV_NAME || fail + dd if=$IMG_FS of=/dev/mapper/$DEV_NAME bs=1M >/dev/null 2>&1 + + echo $PWD1 | $CRYPTSETUP reencrypt --init-only -q --sector-size 1024 $FAST_PBKDF_ARGON $DEV 2>/dev/null && fail + $CRYPTSETUP status $DEV_NAME | grep -q "reencryption: in-progress" && fail + echo $PWD1 | $CRYPTSETUP reencrypt --init-only -q --sector-size 1024 --active-name $DEV_NAME $FAST_PBKDF_ARGON 2>/dev/null && fail + $CRYPTSETUP status $DEV_NAME | grep -q "reencryption: in-progress" && fail + echo $PWD1 | $CRYPTSETUP reencrypt -q --sector-size 1024 $FAST_PBKDF_ARGON $DEV 2>/dev/null && fail + $CRYPTSETUP luksDump $DEV | grep -q "sector: 512" || fail + echo $PWD1 | $CRYPTSETUP reencrypt -q --sector-size 1024 --active-name $DEV_NAME $FAST_PBKDF_ARGON 2>/dev/null && fail + $CRYPTSETUP luksDump $DEV | grep -q "sector: 512" || fail +fi + remove_mapping exit 0 diff --git a/tests/xfs_512_block_size.img.xz b/tests/xfs_512_block_size.img.xz new file mode 100644 index 0000000000000000000000000000000000000000..047c7886b72f410863d3f987b9abe6c9a3533961 GIT binary patch literal 6284 zcmeI1S5(v47RLX90HT0IKtVtT5t4uq88j-rD4>xbb*O_#q)StZ5^14HODNHRQlu(G zibyd6CL)R=Oc01PGm4=E0Rce*gvpGv)_uG8VeWO^d6=B%v(`TATW9~ieZI4G^s18! z0Dz|$56!{UJhiyEI5B0=Zs6NG9;hS0xck-Hax`~|AoE*-J!3u(N0pQc%!od$yt6Kh zCSNXn6kGa$=TA@3^Mkn3bgs)y@}_Sevgs2>d5i3Sq01iiX8L19gqlD7c9ND+u$s2N z_LO3U#6bE~W3RLF!T28G<~zRKo$>joU{9Y_Mr~4?^c5Ip?ipK3O~23$TT2M752$LS zRcoEB#kQMq-Dt#RKM*2p8(eh0uBJ7|Ct7D%P+y2c@B9EM%pG3xU1?M;I%d*BH^2UD`cndqp2Rj#38Ytjl#+%g%UYCqI^%vH_oGKlSPx zov>0#73?&RM@FEPWI7CyxAAXe_gZcyk7$i=m}HgNYbc33X=Ej>$j!hLoU@mG^r9b) zJh1#l{XHhiU$HHEsS7TS;Q5H2E~#d8QKvRJM<7?lBYz|%kakU^G7Z<5M1nzX=FIh~ zuX}o@1?9<*8@o&_7c5P+SKJsMpT8={>AO6%gecSQCRaE%vX5-U`{}@ucZTnQ$K1Kg zQK`!5W}(Hi@RVtZa#t6){F#yB2uTTr0&ztu{+xqpQQ)3f&Tr5acJYC7oLWLrxXKH% zW0EaO_0=bXiQ~&=0dfpKnv}I<6J}2AH8*fDvoFhdejQ%85SAFWug?Vs6^gZgU3;QR zxIa1Vyu%05eau2?%u3~*BFy!npXibN3it0t9wkiBuT&V<$!LNI!n z_?grB(rfe(1sZrwQ%58|-Od5kAU)8yJ6c*kyJMABm^E_stnF?yQoFnV*znm1b?}_z zvZ(Tv`-k(&Kz+1B%e`;Vc7@)?&Oci~r`MZD!vtUi%Tbs`!6hZJ%X;H0W=V{4F|QPT z-{8|}`qmxP2$Dm0kmu`GatOvJFu~X5cv$8WWXt-A=5zCvAx!kok+6d8B>SMZ>910a zJhV5vt!JbRT0X{6RPJ(w(??cg8RGIr*CK0h(#k>oLs!2TO52@}AUm@rU4^!-zZkvG z-L%!X+`G^92h;U;b_N_cMpQ4ZM?LV@CcVXH?OGnyI}K6ZrUWSoo@6d3G|HK?qnAg7 zneV*E#WFVw4rw`3@kw8B@ze{?RP8vc*+^}niu#3MT8Nc}SbsL8`u;gZN?!w1*3CJY z?QQGS!T#S%1MS%X#XL!n1~7NYu4=2J7OTCtxFu6ND5R)L#bXZElhjWnrNyXcFGIuE zN_?5ps56VY=!iqPX(IsN*uk0F z0odE_wL(*QFa^!?((~R#u^0$C#{r|86L!?oBM`c@aVD&cr8>F;`9^WElq+TUv~7D` zAw^8dLeJ@)rDKa~{2Iu@7%qJ2sD^c_8Rc$cb9E&HX>;JTcUQVa+kv72?vV#7gc8x| zR70T^)DkoA;+ETN$>t)`p{l;0s^x&U1;#Tek2Sgr5`UD5DGyuD(DTCNJdsaOJlN91 zfYzp~%M! zImq>^it6}OLA^j%Z?l_ZRY+thF*8I{ys)9nJy#;=x|Hh&mW+D|v%O1?h!;!u3f`rF zl6;TRDz2utIIyyrj``g7_cM(ZaTB@Jy@%}@cZr1JV2(9!tVYui2>gK${7DUFEApoduQ3M@U)3YPIF$Ym_L