Package: cryptsetup Version: 2:1.7.3-2 Severity: normal Tags: patch Dear Maintainer,
Failed passwords entered on the local console should not cause a reboot or long pause. `cryptsetup` has introduced a locking mechanism on multiple decryption key unlock failure after CVE-2016-4484. The mechanism itself is wrong rather than the implementation broken: - What good does sleeping do? If the threat model is for a local user with physical access to the machine, the threat model is really one of casual attempts to unlock a machine (in which case the users' decryption password should be its own throttling mechanism) - otherwise a user will do automated offline decryption after physical removal of the key from the drive. Rebooting the machine forcibly on failure is the worst thing to do in any case, and could be argued to be a simple DoS vector, particularly when the machine in question has an expensive reboot cost associated with it (my laptop for example has a slow BIOS setup process, and my workstation takes around 2 minutes after checking PXE and IPMI, plus a long pause in the BIOS launch, which means when I enter my complex password incorrectly a few times, I'm sometimes not able to login for 10 minutes or so if I'm having a bad typing day. In any case, the rebooting is the wrong fix for a nonexistent problem. Sleeping 15 seconds before reboot has *no* value at all as far as I understand. The second issue is the infinite loop. This only serves to turn a machine into a brick if `reboot` fails, and will force the user to pull the plug, causing possible damage to the machine. The user may have remote serial access, but no physical access, forcing a trip to the datacentre. The behavoiur I personally would expect from a decryption key setup program is to simply loop forever; If the root partition is encrypted with a strong password, what is the threat model from asking for the password again? I've attached a patch as to what I think should go some way to solving the issue (but I'll admit total unfamiliarity with the system, and haven't done any real research on what the canonical expected behaviour would be in a program of this kind). My feeling is that CVE-2016-4484 is something of a red-herring, and the current implementation is more damaging than useful. I'm happy to try things out if it's any help. Thanks Luke -- Package-specific info: -- System Information: Debian Release: stretch/sid APT prefers stable-updates APT policy: (500, 'stable-updates'), (500, 'unstable'), (500, 'stable') Architecture: amd64 (x86_64) Foreign Architectures: i386 Kernel: Linux 4.8.0-1-amd64 (SMP w/4 CPU cores) Locale: LANG=en_GB.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8) Shell: /bin/sh linked to /bin/dash Init: systemd (via /run/systemd/system) Versions of packages cryptsetup depends on: ii cryptsetup-bin 2:1.7.3-2 ii debconf [debconf-2.0] 1.5.59 ii dmsetup 2:1.02.136-1 ii libc6 2.24-7 Versions of packages cryptsetup recommends: ii busybox 1:1.22.0-19 ii console-setup 1.154 ii initramfs-tools [linux-initramfs-tool] 0.125 ii kbd 2.0.3-2 Versions of packages cryptsetup suggests: ii dosfstools 4.0-2 pn keyutils <none> ii liblocale-gettext-perl 1.07-3+b1 -- debconf information excluded
diff --git a/debian/initramfs/cryptroot-script b/debian/initramfs/cryptroot-script index 01e568f..b8563e7 100644 --- a/debian/initramfs/cryptroot-script +++ b/debian/initramfs/cryptroot-script @@ -70,7 +70,6 @@ parse_options() cryptkeyscript="" cryptkey="" # This is only used as an argument to an eventual keyscript cryptkeyslot="" - crypttries=3 crypttcrypt="" cryptveracrypt="" cryptrootdev="" @@ -124,14 +123,6 @@ parse_options() keyslot=*) cryptkeyslot=${x#keyslot=} ;; - tries=*) - crypttries="${x#tries=}" - case "$crypttries" in - *[![:digit:].]*) - crypttries=3 - ;; - esac - ;; tcrypt) crypttcrypt="yes" ;; @@ -179,7 +170,7 @@ activate_vg() setup_mapping() { - local opts count cryptopen cryptremove NEWROOT + local opts cryptopen cryptremove NEWROOT opts="$1" if [ -z "$opts" ]; then @@ -265,7 +256,6 @@ setup_mapping() # We've given up, but we'll let the user fix matters if they can if [ ! -e "${cryptsource}" ]; then - echo " ALERT! ${cryptsource} does not exist." echo " Check cryptopts=source= bootarg: cat /proc/cmdline" echo " or missing modules, devices: cat /proc/modules; ls /dev" @@ -298,9 +288,9 @@ setup_mapping() cryptremove="/sbin/cryptsetup remove $crypttarget" NEWROOT="/dev/mapper/$crypttarget" - # Try to get a satisfactory password $crypttries times + # Try to get a satisfactory password count=0 - while [ $crypttries -le 0 ] || [ $count -lt $crypttries ]; do + while true; do export CRYPTTAB_TRIED="$count" count=$(( $count + 1 )) @@ -363,18 +353,6 @@ setup_mapping() break done - failsleep=15 - if [ $crypttries -gt 0 ] && [ $count -ge $crypttries ]; then - message "cryptsetup: maximum number of tries exceeded for $crypttarget" - message "cryptsetup: going to reboot in $failsleep seconds..." - sleep $failsleep - reboot -f - message "cryptsetup: reboot failed, going to sleep forever..." - while true; do - sleep 60 - done - fi - udev_settle return 0 }