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

New EncryptionMethod using fde-tools for TPM-based unlocking #1363

Merged
merged 9 commits into from Nov 2, 2023

Conversation

ancorgs
Copy link
Contributor

@ancorgs ancorgs commented Oct 25, 2023

Problem

Both ALP and openSUSE Tumbleweed include a package called fde-tools that allow to setup encrypted devices (using LUKS2) to be automatically unlocked during boot without user intervention based on information stored and validated in the TPM of the system.

The process has its limitations but it certainly works as proven by the preliminary (and rather hacky) support present at Agama. So it's time to move that support from Agama to the core of YaST.

Some more-or-less related links

Solution

This introduces a new encryption method (TPM_FDE) in yast2-storage-ng. Thus, TPM unlocking based on fde-tools can be configured by both AutoYaST and Agama.

For more information about the process, check the fde-tools documentation.

If the system meets all the technical requirements to use the new method, it will be used by Agama. In that regard, check below the associated pull request.

Even if the mentioned technical requirements are met, the new method will still not be available in YaST. There are several reasons for that:

  • Lack of support in the inst-sys. It cannot check whether the TPM works.
  • No specific form in the Expert Partitioner
  • We need to ensure all affected devices share the recovery password.
  • During installation, the UI and the AutoYaST documentation need to properly explain the need to boot from the target device after installation (no DVD → boot from hard disk) and check any possible interaction with the second stage.
  • When creating new devices with that encryption method in an already installed system: it’s not 100% clear how the feature should work.

Associated pull requests

openSUSE/agama#826

Testing

Unit tests included.

Tested manually in Agama with fde-tools 0.7.1 in several situations:

  • With a single partition
  • With separate / and /var partitions
  • With an LVM extending over several disks

Review

Pull request structured in several meaningful commit for easier review.

@coveralls
Copy link

coveralls commented Oct 25, 2023

Coverage Status

coverage: 97.782% (+0.02%) from 97.767% when pulling dea440c on ancorgs:tpm_master into 70f4fa7 on yast:master.

@ancorgs ancorgs changed the title WIP: New EncryptionMethod using fde-tools for TPM-based unlocking New EncryptionMethod using fde-tools for TPM-based unlocking Oct 31, 2023
@ancorgs ancorgs marked this pull request as ready for review October 31, 2023 14:00
false
end

alias_method :tpm_present?, :tpm_present
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NP: why not to directly call it #tpm_present?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No strong reason. I just wanted to have the method names equivalent to the fdectl commands (obviously turning hyphens into underscores). Since the fdectl subcommands are tpm-present, add-secondary-password and add-secondary-key...

Copy link
Contributor

@joseivanlopez joseivanlopez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left some comments, but I haven't finished with the review yet :)

#
# This is only ever needed if the available packages might have changed
# since the last use of this class.
def self.drop_cache
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NP: maybe this could be moved to the base class.

@joseivanlopez
Copy link
Contributor

Just for the records: failing tests on leap is expected. Changes in this PR require a new version of libstorage-ng which is not submitted to leap (only to Tumbleweed).

In general, running unit tests on leap for the master branch is useless. Note that SLE-15-SPX branches have already diverged.

Copy link
Contributor

@joseivanlopez joseivanlopez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@imobachgs imobachgs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changelog looks good (the rest was already reviewed).

@ancorgs ancorgs merged commit 87d3e01 into yast:master Nov 2, 2023
8 of 9 checks passed
@yast-bot
Copy link

yast-bot commented Nov 2, 2023

❌ Internal Jenkins job #1142 failed

@yast-bot
Copy link

yast-bot commented Nov 2, 2023

✔️ Internal Jenkins job #1143 successfully finished
✔️ Created OBS submit request #1122814

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants