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

Recovery fails on LUKS-encrypted filesystem using simple password #2151

Closed
petroniusniger opened this issue May 28, 2019 · 18 comments
Closed
Assignees
Labels
bug The code does not do what it is meant to do enhancement Adaptions and new features fixed / solved / done
Milestone

Comments

@petroniusniger
Copy link
Contributor

petroniusniger commented May 28, 2019

  • ReaR version ("/usr/sbin/rear -V"): Relax-and-Recover 2.5 / Git

  • OS version ("cat /etc/rear/os.conf" or "lsb_release -a" or "cat /etc/os-release"):

    OS_VENDOR=SUSE_LINUX
    OS_VERSION=15.0
    # The following information was added automatically by the mkbackup workflow:
    ARCH='Linux-i386'
    OS='GNU/Linux'
    OS_VERSION='15.0'
    OS_VENDOR='SUSE_LINUX'
    OS_VENDOR_VERSION='SUSE_LINUX/15.0'
    OS_VENDOR_ARCH='SUSE_LINUX/i386'
    # End of what was added automatically by the mkbackup workflow.

  • ReaR configuration files ("cat /etc/rear/site.conf" and/or "cat /etc/rear/local.conf"):

    OUTPUT=ISO
    BACKUP=NETFS
    BACKUP_URL="nfs://mcbackup.cbptc.org/Stations_bkup/rear/"
    KEEP_OLD_OUTPUT_COPY=1

  • Hardware (PC or PowerNV BareMetal or ARM) or virtual machine (KVM guest or PoverVM LPAR): VMware-based VM

  • System architecture (x86 compatible or PPC64/PPC64LE or what exact ARM device): x86_64

  • Firmware (BIOS or UEFI or Open Firmware) and bootloader (GRUB or ELILO or Petitboot): grub2-uefi

  • Storage (local disk or SSD) and/or SAN (FC or iSCSI or FCoE) and/or multipath (DM or NVMe): local (virtual) disk, 32GB, thin prov.

  • Description of the issue (ideally so that others can reproduce it): the VM I try to recover contains one LUKS-encrypted filesystem (inside a LVM logical volume). The key to decrypt and mount that filesystem is provided as a password, to be typed during boot time. During recovery, ReaR correctly detects that this LV should be encrypted and prompts me to enter a (new) password for it. It then successfully creates the LV, the filesystem and mounts it, then proceed with restoring the data. The failure arises later, when ReaR tries to re-assign a key file that doesn't exist:

    ERROR: 
    ====================
    BUG in /usr/share/rear/finalize/GNU/Linux/240_reassign_luks_keyfiles.sh line 29:
    'temporary keyfile /tmp/LUKS-keyfile-cr_vg00-lvol4 not found'
    --------------------
    Please report this issue at https://github.com/rear/rear/issues
    and include the relevant parts from /var/log/rear/rear-pc-pan.log
    preferably with full debug information via 'rear -D recover'
    ====================
    Some latest log messages since the last called script 240_reassign_luks_keyfiles.sh:
      2019-05-28 10:32:57.311886473 Including finalize/GNU/Linux/240_reassign_luks_keyfiles.sh
      2019-05-28 10:32:57.318490644 Re-assigning keyfile  to LUKS device cr_vg00-lvol4 (/dev/mapper/vg00-lvol4)
  • Workaround, if any: none at this stage

  • Attachments, as applicable ("rear -D mkrescue/mkbackup/recover" debug log files):
    (see below)

@petroniusniger
Copy link
Contributor Author

BAK-rear-pc-pan.log

@petroniusniger
Copy link
Contributor Author

REC-rear-pc-pan.log

@petroniusniger
Copy link
Contributor Author

After some investigation, it seems to be the result of an inconsistency between:

layout/prepare/GNU/Linux/160_include_luks_code.sh

and:

finalize/GNU/Linux/240_reassign_luks_keyfiles.sh

In the first script, the temp. keyfile is only created when a keyfile was specified on the source system (see l. 46-48). In the absence of a keyfile, a password is either read from the options or (as is the case for me) the user is prompted for it.

In the second script, we fail as BUG if the temp. keyfile doesn't exist (see l. 28-29) -- which is the case for us. The script should probably not have reached the inside of the while loop to begin with.

Some error in the awk scriptlet? Or 'keyfile' parameter incorrectly set in the layout file?

@petroniusniger
Copy link
Contributor Author

Second hypothesis correct. Here is relevant line from 'disklayout.conf':

crypt /dev/mapper/cr_vg00-lvol4 /dev/mapper/vg00-lvol4 cipher=aes-xts-plain64 key_size=256 hash=sha256 uuid=3300d4a7-2d75-4968-b55d-d330f79efdc9 keyfile=

This does indeed cause the 'awk' scriptlet to match, but to return an empty string for $original_keyfile.

Will add an extra test in the while loop and re-test the backup/recovery cycle.

@jsmeix
Copy link
Member

jsmeix commented May 28, 2019

@OliverO2
could you have a look here because I know nothing at all about LUKS
and you had done something in this are via
#1493
and
47f015d

@jsmeix
Copy link
Member

jsmeix commented May 28, 2019

@petroniusniger
regarding weird 'awk' issues you may have a look at
#2095
and
#2115

@jsmeix
Copy link
Member

jsmeix commented May 28, 2019

The awk script in finalize/GNU/Linux/240_reassign_luks_keyfiles.sh
is overcomplicated because at least for me it is impossible to
understand what it intends to do by plain looking at the code.

The awk script originated from the very beginning when that script was made, cf.

git log -p --follow usr/share/rear/finalize/GNU/Linux/240_reassign_luks_keyfiles.sh

Of course one could reverse-engineer what it does
but that is not how code in ReaR scripts should be, cf.
https://github.com/rear/rear/wiki/Coding-Style

@OliverO2
Copy link
Contributor

@petroniusniger

Second hypothesis correct. Here is relevant line from 'disklayout.conf':

crypt /dev/mapper/cr_vg00-lvol4 /dev/mapper/vg00-lvol4 cipher=aes-xts-plain64 key_size=256 hash=sha256 uuid=3300d4a7-2d75-4968-b55d-d330f79efdc9 keyfile=

Thanks for the report and your correct analysis. The keyfile option in the layout file should indeed never look that way. So the problem originates in usr/share/rear/layout/save/GNU/Linux/260_crypt_layout.sh, which apparently sees an unexpected line in /etc/crypttab.

To me it looks like /etc/crypttab is not formatted according to crypttab(5) as field 3 seems to be missing but is expected to be a path, or none (or -, but that is already beyond specification). Could you post the contents of the /etc/crypttab file on the original system?

@jsmeix

it is impossible to understand what it intends to do by plain looking at the code.

What exactly is impossible to understand? Looks pretty clear to me.

@petroniusniger
Copy link
Contributor Author

Here is the contents of the original '/etc/crypttab' (from backup archive):

cr_vg00-lvol4 UUID=3300d4a7-2d75-4968-b55d-d330f79efdc9

I checked the (partially) recovered VM, and '/mnt/local/etc/crypttab' is identical (recovery died before call to 260_rename_diskbyid.sh).

@petroniusniger
Copy link
Contributor Author

Just to clarify: I've created the encrypted volume using the YaST2 partitioner module, and I've not edited /etc/crypttab manually afterwards.

@petroniusniger
Copy link
Contributor Author

@OliverO2
Proposed fix in usr/share/rear/layout/save/GNU/Linux/260_crypt_layout.sh:

43c43
<     keyfile_option=$([ -f /etc/crypttab ] && awk '$1 == "'"$target_name"'" && $3 != "none" && $3 != "-" { print "keyfile=" $3; }' /etc/crypttab)
---
>     keyfile_option=$([ -f /etc/crypttab ] && awk '$1 == "'"$target_name"'" && $3 != "none" && $3 != "-" && $3 != "" { print "keyfile=" $3; }' /etc/crypttab)

Shall I test and submit a pull request if successful?

@OliverO2
Copy link
Contributor

@petroniusniger
Thanks for quickly providing the information. I think that your fix is the way to go, I'd appreciate a PR and I'm pretty sure the ReaR maintainers would welcome one, too.

You might want to check crypttab(5) on your system and file a bug with YaST as well.

@petroniusniger
Copy link
Contributor Author

Extracts from crypttab(5) on openSUSE Leap 15.0:

       Empty lines and lines starting with the "#" character are ignored. Each
       of the remaining lines describes one encrypted block device, fields on
       the line are delimited by white space. The first two fields are
       mandatory, the remaining two are optional.

and further down:

       The third field specifies the encryption password. If the field is not
       present or the password is set to "none" or "-", the password has to be
       manually entered during system boot. Otherwise, the field is [...]

So, not a bug from the point of view of SUSE, I guess.
I'll proceed with testing the proposed fix.

@OliverO2
Copy link
Contributor

Thanks for the insights! Just did a quick research on Ubuntu manpages where even the 7 year old Ubuntu 12.04 crypttab(5) manpage says in the description's final section:

Note that all four fields are mandatory and that a missing field will lead to unspecified behaviour.

Anyway, if there are systems around that do not agree, it's always a good idea to call it a bug in ReaR and fix it accordingly.

@petroniusniger
Copy link
Contributor Author

I can confirm that I'm able to successfully recover my test system using the proposed fix.
I can also confirm that the resulting disklayout.conf no longer contains the problematic keyfile= empty option at the end of the encrypted filesystem line:

crypt /dev/mapper/cr_vg00-lvol4 /dev/mapper/vg00-lvol4 cipher=aes-xts-plain64 key_size=256 hash=sha256 uuid=3300d4a7-2d75-4968-b55d-d330f79efdc9

I'll now proceed with the PR creation.

@petroniusniger
Copy link
Contributor Author

Successfully retested after having re-aligned my working copy with trunk/LATEST.

Pull request created: #2154

@jsmeix jsmeix added bug The code does not do what it is meant to do enhancement Adaptions and new features labels Jun 5, 2019
@jsmeix jsmeix added this to the ReaR v2.6 milestone Jun 5, 2019
@gdha gdha self-assigned this Jun 5, 2019
@gdha
Copy link
Member

gdha commented Jun 5, 2019

PR has been merged - if all is well we can close this issue - just let me now how it works out?

@OliverO2
Copy link
Contributor

OliverO2 commented Jun 5, 2019

I think it can be closed immediately. We already have a double confirmation from @petroniusniger that the PR's change resolves the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The code does not do what it is meant to do enhancement Adaptions and new features fixed / solved / done
Projects
None yet
Development

No branches or pull requests

4 participants