-
Notifications
You must be signed in to change notification settings - Fork 252
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
Enhanced disk write-protection #2703
Conversation
The specific WRITE_PROTECTED_PARTITION_TABLE_UUIDS is replaced by WRITE_PROTECTED_UUIDS with generic functionality cf. #2626 (comment) and WRITE_PROTECTED_FILE_SYSTEM_LABEL_PATTERNS is shortened to WRITE_PROTECTED_FS_LABEL_PATTERNS
Use is_write_protected instead of is_write_protected_by_pt_uuid and is_write_protected_by_fs_label
New function write_protection_uuids() and replaced is_write_protected_by_pt_uuid() by adapted is_write_protected_by_uuid()
LogPrintError if write protection for RAWDISK cannot be done to have the user informed
Add all UUIDs that exist on USB_DEVICE to WRITE_PROTECTED_UUIDS and all all available file system labels that exist on USB_DEVICE to WRITE_PROTECTED_FS_LABEL_PATTERNS cf. #2626 (comment)
Use WRITE_PROTECTED_UUIDS instead of WRITE_PROTECTED_PARTITION_TABLE_UUIDS and use WRITE_PROTECTED_FS_LABEL_PATTERNS instead of WRITE_PROTECTED_FILE_SYSTEM_LABEL_PATTERNS
I tested this one similar as I did in I tested it on the two VMs with those GPT disks
as in #2626 (comment) I use almost same etc/rear/local.conf
After "rear mkbackup" things in
Those UUIDs and the LABEL match only /dev/sdb3
because prep/USB/default/480_initialize_write_protect_settings.sh
This is good enough for now but I would prefer when "rear recover"
The recreated system boots well from sdb and seems to work OK. |
Excerpts how automated disk mapping changed from
to here where the latest Github master code is used
so there is no longer an automated mapping to |
The "rear recover" log shows why /dev/sdb excluded from device mapping choices
which is wrong because UUID 8bd9f9e8-9043-4a80-84e1-64d417244529
In particular the "rear recover" log shows
so same UUID for both sda and sdb. Investigating... |
Copy and paste fix "$USB_DEVICE" must be "$device" in write_protection_uuids() cf. #2703 (comment)
Beware of misplaced space characters! Now var="$( COMMAND )" instead of var="$( COMMAND) " to get "$var" without an unintended trailing space character.
With latest changes things look better: "rear recover" excerpts:
All user dialogs were accepted with the default (i.e. just proceed) and |
@rear/contributors |
@jsmeix I will have a look next week (this week we will have a holiday). |
@pcahyna |
In write_protection_uuids() use the parent device to get all UUIDs of the whole disk because the goal is to write-protect the whole disk by using all its UUIDs cf. #2703 (comment)
With latest changes now all UUIDs are used. After "rear mkbackup" I have in /etc/rear/rescue.conf
which are all UUIDs of the ReaR's own disk sda in the recovery system
|
Tested "rear recover" with only
and the "rear -D recover" log contains
|
I would like to optimize things by avoiding needless duplicate tests like
cf. #2703 (comment) |
Remember when target devices get known by the "same name and same size" tests that they cannot be used for recreating the current original device to avoid that already excluded target devices get needlessly considered again during the subsequent "same size" tests see #2703 (comment)
With latest commit needless duplicate tests do no longer happen. "rear -D recover" excerpts in same test environment as above in
|
Better wording in user message - now it is "Using /dev/sdX (the only available of the disks) for recreating /dev/sdY" instead of what it was before "Using /dev/sdX (the only appropriate) for recreating /dev/sdY" because "appropriate" sounds as if /dev/sdX is the right one to recreate /dev/sdY but acutally /dev/sdX could be inappropriate to recreate /dev/sdY (e.g. much too small) but /dev/sdX is the only diks that is left available of all existing disks so /dev/sdX is the only possible choice that is left to recreate /dev/sdY under the conditions of the currently already existing mappings and the exclusions from possible mapping targets.
The latest commit
|
Use IDs instead of UUIDs
Use WRITE_PROTECTED_IDS instead of WRITE_PROTECTED_UUIDS
I did not yet test my recent commits. |
@pcahyna
where SERIAL Another possibly too less unique UUID is
where With that I get after "rear mkbackup" in /var/tmp/rear.XXX/rootfs/etc/rear/rescue.conf
|
In the ReaR recovery system on the replacement VM I have now
so the SERIAL number of that exact same ReaR disk Nevertheless by luck "rear recover" just worked |
Do not use SERIAL disk serial number, cf. #2703 (comment)
Do not use SERIAL disk serial number, cf. #2703 (comment)
Do not use SERIAL disk serial number, cf. #2703 (comment)
With latest commits "rear mkbackup" and "rear recover" work well for me. I could not test WWN because the disks on my test VMs don't have a WWN set. |
As always sleeping on an issue helps: From my above experience with different behaviour what values
So I will add one more config variable in default.conf like
and then use it in write_protection_ids() like
This provides final power to the user For example I have no idea how UUID PTUUID PARTUUID WWN |
New WRITE_PROTECTED_ID_TYPES="UUID PTUUID PARTUUID WWN"
Use WRITE_PROTECTED_ID_TYPES in function write_protection_ids()
Corrected comments in prep/USB/default/480_initialize_write_protect_settings.sh how things actually work plus DebugPrint when USB disks get write protected so "rear -D mkrescue/mkbackup" output looks now like (excerpt): -------- USB disk IDs of '/dev/disk/by-label/MY-DATA' added to WRITE_PROTECTED_IDS File system label of '/dev/disk/by-label/MY-DATA' added to WRITE_PROTECTED_FS_LABEL_PATTERNS --------
Implemented #2703 (comment) The same test as above works well for me with that commits. |
In default.conf describe how to use additional 'lsblk' output columns in WRITE_PROTECTED_IDS together with matching WRITE_PROTECTED_ID_TYPES.
Tested again with external real USB disk as ReaR disk I have now on the original VM
I use this etc/rear/local.conf
i.e. same as above with two added WRITE_PROTECTED_ entries "rear mkbackup" (excerpts)
After "rear -D mkbackup" I got now (excerpt)
The duplicate The WRITE_PROTECTED_FS_LABEL_PATTERNS=( 'MY-DATA' ) On the replacement VM I have
The "rear recover" log shows the write-protection test results
So a different ID type also works well for me. |
If there are no objections I would like to merge it tomorrow afternoon. |
I did one more test on the above two VMs Now I have in local.conf only
but no WRITE_PROTECTED_IDS. This results basically the same behaviour as before this pull request After "rear mkbackup" I have
During "rear recover" I get on the terminal
and the recovery log shows
|
I think we need further enhanced disk write-protection |
In the function is_write_protected_by_id() assume a disk without any of UUID PTUUID PARTUUID WWN is empty and meant to be used to recreate the system so it should not be write-protected cf. #2703 (comment)
Improved wiping disks: In layout/recreate/default/120_confirm_wipedisk_disks.sh skip disks that do not exist on the bare hardware in the recovery system cf. #2715 and exclude disks that are write-protected cf. #2703 (comment) and show in any case a user confirmation dialog for the disks that will be wiped. In layout/recreate/default/150_wipe_disks.sh do no longer open (and close) LUKS volumes because encrypted volumes contain meaningless data unless opened and unencrypted so there is no need to wipe anything inside an encrypted LUKS container, cf. "Regarding LUKS" in #2514 (comment)
Type: Enhancement
Impact: Normal
Reference to related issue (URL):
Stop ReaR from overwriting its own disk and backup drives #2626
How was this pull request tested?
Not yet tested by me - will test soon...
Brief description of the changes in this pull request:
The specific WRITE_PROTECTED_PARTITION_TABLE_UUIDS
is replaced by WRITE_PROTECTED_IDS with generic functionality
cf. #2626 (comment)
together with the new WRITE_PROTECTED_ID_TYPES which
defaults to UUID PTUUID PARTUUID WWN so that the user can
specify different lsblk columns as needed in his particular environment
WRITE_PROTECTED_FILE_SYSTEM_LABEL_PATTERNS
is shortened to WRITE_PROTECTED_FS_LABEL_PATTERNS