Handling encryption logic when only one non-root partition is encrypted (such as /home) #2575
Conversation
…ade crypttab logic able to handle multiple calls via override logic.
…king if root is encrypted or not for keyfile logic
| def should_generate_encryption_keyfile(self, dev: PartitionModification | LvmVolume) -> bool: | ||
| # If all partitions being encrypted, are non-root file systems | ||
| # we will not generate key-files as their safety can not be guaranteed. | ||
| if all([part.mountpoint != Path('/') for part in self.partitions]): |
There was a problem hiding this comment.
just FYI there's a part.is_root() :)
There was a problem hiding this comment.
This check doesn't feel right here...on the caller side this is run for each partition now, but we can probably short circuit it
def generate_keyfile_for_dev(self, dev: PartitionModification | LvmVolume) -> bool:
# If all partitions being encrypted, are non-root file systems
# we will not generate key-files as their safety can not be guaranteed.
if isinstance(dev, PartitionModification):
return dev in self.partitions and dev.mountpoint != Path('/')
elif isinstance(dev, LvmVolume):
return dev in self.lvm_volumes and dev.mountpoint != Path('/')
return False
def generate_keyfiles(self) -> bool:
# If all partitions being encrypted, are non-root file systems
# we will not generate key-files as their safety can not be guaranteed.
if all([part.mountpoint != Path('/') for part in self.partitions]):
return True
return False
And on the caller
def _setup_luks_unlock_partitions(self) -> None:
if not self._disk_encryption.generate_keyfiles():
return
...
What do you think?
There was a problem hiding this comment.
just FYI there's a
part.is_root():)
Silly me just copied the code from two rows down, but now that you mention it I do remember seeing that function, I'll change!
archinstall/lib/luks.py
Outdated
| for index, existing_row in enumerate(existing_data): | ||
| if uuid in existing_row: | ||
| if override is False: | ||
| print(f"Found {uuid} in {existing_row}") |
There was a problem hiding this comment.
Can we change this to debug(...) rather as it's not really relevant for the user
There was a problem hiding this comment.
Of course, it's still a draft and used print when executing blocks of code in isolated test.py outside of archinstall :) but good reminder!
| if uuid in existing_row: | ||
| if override is False: | ||
| print(f"Found {uuid} in {existing_row}") | ||
| return True |
There was a problem hiding this comment.
Is this return here right? If override is passed into the function should this terminate ?
There was a problem hiding this comment.
It's checking if we don't want to override - but it found that we already have added the data - so it's happy and returns "done" :)
archinstall/lib/luks.py
Outdated
|
|
||
| existing_data.append(new_row) | ||
|
|
||
| print(f"Writing {existing_data}") |
There was a problem hiding this comment.
This will show up during the installation for the user probably not very useful, debug(...) probably better here
PR Description:
This should make single-non-root-encrypted-partitions be unlocked via
/etc/crypttabinstead of via keyfile, as the keyfile would be unprotected.Tests and Checks