Make TPM-based encryption more explicit (#995)

## Problem

Sometimes, Agama decides to use the encryption method `TPM_FDE` which
results in the system being configured via `fde-tools` to open the
encryption devices automatically during system boot without needing to
enter the password.

That happens if the configuration parameter `encryption.tpm_luks_open`
is set AND the system supports TPM unlocking. If that the case, the
`TPM_FDE` encryption method is used without even asking the user. In any
other case, the encryption method specified at the configuration
parameter `encryption.method` is used.

That's all quite obscure, the users don't know whether TPM-based
unlocking is going to be configured. Not even if it's possible to
configure it or not.

## Solution

This pull request introduces some changes in how the whole thing is
managed.

Now if the system or the distribution being installed don't support
TPM-based decryption, the encryption method `LUKS2` is used and nothing
is shown in the UI.


![tpm_not_available](https://github.com/openSUSE/agama/assets/3638289/f53925cf-8101-4b13-a1ba-e19b6d78907d)

So no big change for the user except the fact that now LUKS2 with PBKDF2
as derivation function is the default for all distributions (it's a
pretty sensible default for distributions based on Grub2 at 2024).

But if the system and the distribution both support to configure
TPM-based opening of the LUKS devices, the user can choose between the
`TPM_FDE` and the `LUKS2` encryption methods via a checkbox shown in the
UI.


![attempt](https://github.com/openSUSE/agama/assets/3638289/ed539bbb-08f8-4761-9834-c3fa05c6b27f)

The default encryption method (and thus, the default value of the
checkbox) is configured per-product at `encryption.method`. If the value
there is `"tpm_fde"` but the system does not support such a method (eg.
there is no TPMv2 chip), Agama will use the default encryption method
(`LUKS2`) as fallback.

Additionally, to make sure the user does not overlook the need to boot
the machine directly to the new system in order to complete the setup,
the following warning has been added to the page at the end of the
installation process.


![finish-tpm-b2](https://github.com/openSUSE/agama/assets/3638289/a410242d-7680-4108-a6f7-013dca905259)

Expanded version:


![finish-tpm-a2](https://github.com/openSUSE/agama/assets/3638289/82b2d1a4-0f2b-4e39-b899-b47e96a4ea57)

Of course, if TPM encryption was not used, the hint is not there.


![finish-no-tpm2](https://github.com/openSUSE/agama/assets/3638289/de385977-08ee-4d3f-b3df-22548a45ebdf)

## Testing

- Tested manually
- New unit tests for the `InstallationFinished` page
- No tests added to the storage page, since it's going to [heavily
change](#982) soon

products.d/ALP-Dolomite.yaml

@@ -63,9 +63,7 @@ security:
 storage:
  space_policy: delete
  encryption:
- method: luks2
- pbkd_function: pbkdf2
- tpm_luks_open: true
+ method: tpm_fde
  volumes:
  - "/"
  volume_templates:

service/lib/agama/dbus/storage/manager.rb

@@ -34,6 +34,7 @@
 require "agama/dbus/storage/with_iscsi_auth"
 require "agama/dbus/with_service_status"
 require "agama/storage/volume_templates_builder"
+require "agama/storage/encryption_settings"
 
 Yast.import "Arch"
 
@@ -117,13 +118,20 @@ def available_devices
  proposal.available_devices.map { |d| system_devices_tree.path_for(d) }
  end
 
- # List of meaningful mount points for the the current product.
+ # List of meaningful mount points for the current product.
  #
  # @return [Array<String>]
  def product_mount_points
  volume_templates_builder.all.map(&:mount_path).reject(&:empty?)
  end
 
+ # List of possible encryption methods for the current system and product
+ #
+ # @return [Array<String>]
+ def encryption_methods
+ Agama::Storage::EncryptionSettings.available_methods.map { |m| m.id.to_s }
+ end
+
  # Path of the D-Bus object containing the calculated proposal
  #
  # @return [::DBus::ObjectPath] Proposal object path or root path if no exported proposal yet
@@ -161,6 +169,8 @@ def calculate_proposal(dbus_settings)
 
  dbus_reader :product_mount_points, "as"
 
+ dbus_reader :encryption_methods, "as"
+
  dbus_reader :result, "o"
 
  dbus_method :DefaultVolume, "in mount_path:s, out volume:a{sv}" do |mount_path|

service/lib/agama/storage/encryption_settings.rb

@@ -28,6 +28,13 @@ module Storage
  class EncryptionSettings
  include Y2Storage::SecretAttributes
 
+ # @see .encryption_methods
+ METHODS = [
+ Y2Storage::EncryptionMethod::LUKS2,
+ Y2Storage::EncryptionMethod::TPM_FDE
+ ].freeze
+ private_constant :METHODS
+
  # @!attribute encryption_password
  # Password to use when creating new encryption devices
  # @return [String, nil] nil if undetermined
@@ -41,8 +48,30 @@ class EncryptionSettings
  # @return [Y2Storage::PbkdFunction, nil] Can be nil if using LUKS1.
  attr_accessor :pbkd_function
 
+ # All known encryption methods
+ #
+ # This includes all the potentially accepted methods, no matter whether they are
+ # possible for the current system and product.
+ #
+ # @return [Array<Y2Storage::EncryptionMethod::Base>]
+ def self.encryption_methods
+ METHODS
+ end
+
+ # All methods that can be used to calculate a proposal for the current system and product
+ #
+ # @return [Array<Y2Storage::EncryptionMethod::Base>]
+ def self.available_methods
+ encryption_methods.reject { |m| m.respond_to?(:possible?) && !m.possible? }
+ end
+
+ # Constructor
  def initialize
- @method = Y2Storage::EncryptionMethod::LUKS1
+ # LUKS2 with PBKDF2 is the most sensible option nowadays:
+ # - LUKS2 offers additional features over LUKS1
+ # - PBKDF2 keeps memory usage similar to LUKS1 and is supported by Grub2
+ @method = Y2Storage::EncryptionMethod::LUKS2
+ @pbkd_function = Y2Storage::PbkdFunction::PBKDF2
  end
 
  # Whether the proposal must create encrypted devices

service/lib/agama/storage/proposal_settings_reader.rb

@@ -69,24 +69,19 @@ def lvm_reader(settings, value)
  # @param settings [Agama::Storage::ProposalSettings]
  # @param encryption [Hash]
  def encryption_reader(settings, encryption)
- method =
- if try_tpm_fde?(encryption)
- Y2Storage::EncryptionMethod::TPM_FDE
- else
- Y2Storage::EncryptionMethod.find(encryption.fetch("method", ""))
- end
+ method = Y2Storage::EncryptionMethod.find(encryption.fetch("method", ""))
  pbkd_function = Y2Storage::PbkdFunction.find(encryption.fetch("pbkd_function", ""))
 
- settings.encryption.method = method if method
+ settings.encryption.method = method if available_method?(method)
  settings.encryption.pbkd_function = pbkd_function if pbkd_function
  end
 
- # @param encryption [Hash]
+ # @param method [Y2Storage::EncryptionMethod::Base, nil]
  # @return [Boolean]
- def try_tpm_fde?(encryption)
- return false unless encryption["tpm_luks_open"] == true
+ def available_method?(method)
+ return false unless method
 
- Y2Storage::EncryptionMethod::TPM_FDE.possible?
+ EncryptionSettings.available_methods.include?(method)
  end
 
  # @param settings [Agama::Storage::ProposalSettings]

service/package/rubygem-agama.changes

@@ -1,3 +1,9 @@
+-------------------------------------------------------------------
+Thu Jan 18 08:35:01 UTC 2024 - Ancor Gonzalez Sosa <ancor@suse.com>
+
+- New default encryption settings: LUKS2 with PBKDF2.
+- Expose encryption methods at D-Bus API (gh#openSUSE/agama#995).
+
 -------------------------------------------------------------------
 Tue Jan 16 10:49:14 UTC 2024 - Michal Filka <mfilka@suse.com>
 

service/test/agama/dbus/storage/proposal_settings_conversion/to_dbus_test.rb

@@ -50,8 +50,8 @@
  "LVM" => false,
  "SystemVGDevices" => [],
  "EncryptionPassword" => "",
- "EncryptionMethod" => "luks1",
- "EncryptionPBKDFunction" => "",
+ "EncryptionMethod" => "luks2",
+ "EncryptionPBKDFunction" => "pbkdf2",
  "SpacePolicy" => "keep",
  "SpaceActions" => {},
  "Volumes" => []

service/test/agama/storage/proposal_settings_reader_test.rb

@@ -99,8 +99,8 @@
  ),
  encryption: an_object_having_attributes(
  password: nil,
- method: Y2Storage::EncryptionMethod::LUKS1,
- pbkd_function: nil
+ method: Y2Storage::EncryptionMethod::LUKS2,
+ pbkd_function: Y2Storage::PbkdFunction::PBKDF2
  ),
  space: an_object_having_attributes(
  policy: :keep,
@@ -127,13 +127,13 @@
 
  expect(settings).to have_attributes(
  encryption: an_object_having_attributes(
- method: Y2Storage::EncryptionMethod::LUKS1
+ method: Y2Storage::EncryptionMethod::LUKS2
  )
  )
  end
  end
 
- context "when the config contains an unknown encryption function" do
+ context "when the config contains an unknown password derivation function" do
  let(:config_data) do
  {
  "storage" => {
@@ -144,12 +144,12 @@
  }
  end
 
- it "does not set a PBKD function" do
+ it "sets the default derivation function" do
  settings = subject.read
 
  expect(settings).to have_attributes(
  encryption: an_object_having_attributes(
- pbkd_function: be_nil
+ pbkd_function: Y2Storage::PbkdFunction::PBKDF2
  )
  )
  end

web/package/cockpit-agama.changes

@@ -1,3 +1,8 @@
+-------------------------------------------------------------------
+Thu Jan 18 08:33:52 UTC 2024 - Ancor Gonzalez Sosa <ancor@suse.com>
+
+- Make TPM-based encryption more explicit (gh#openSUSE/agama#995)
+
 -------------------------------------------------------------------
 Tue Jan 16 15:27:28 UTC 2024 - José Iván López González <jlopez@suse.com>
 

web/src/assets/styles/blocks.scss

@@ -430,3 +430,17 @@ ul[data-of="agama/timezones"] {
  }
  }
 }
+
+.tpm-hint {
+ container-type: inline-size;
+ container-name: tpm-info;
+ text-align: start;
+
+ .pf-v5-c-alert__title {
+ margin-block-end: var(--spacer-small);
+ }
+
+ .pf-v5-c-alert__description {
+ max-inline-size: 100%;
+ }
+}

web/src/assets/styles/layout.scss

@@ -55,6 +55,8 @@
  grid-area: body;
  overflow: auto;
  padding-block: var(--spacer-normal);
+ container-type: inline-size;
+ container-name: agama-page-content;
  }
 
  footer {

web/src/client/storage.js

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) [2022-2023] SUSE LLC
+ * Copyright (c) [2022-2024] SUSE LLC
  *
  * All Rights Reserved.
  *
@@ -46,6 +46,17 @@ const ZFCP_CONTROLLER_IFACE = "org.opensuse.Agama.Storage1.ZFCP.Controller";
 const ZFCP_DISKS_NAMESPACE = "/org/opensuse/Agama/Storage1/zfcp_disks";
 const ZFCP_DISK_IFACE = "org.opensuse.Agama.Storage1.ZFCP.Disk";
 
+/**
+ * Enum for the encryption method values
+ *
+ * @readonly
+ * @enum { string }
+ */
+const EncryptionMethods = Object.freeze({
+ LUKS2: "luks2",
+ TPM: "tpm_fde"
+});
+
 /**
  * Removes properties with undefined value
  *
@@ -227,6 +238,7 @@ class ProposalManager {
  * @typedef {object} ProposalSettings
  * @property {string} bootDevice
  * @property {string} encryptionPassword
+ * @property {string} encryptionMethod
  * @property {boolean} lvm
  * @property {string} spacePolicy
  * @property {string[]} systemVGDevices
@@ -283,6 +295,16 @@ class ProposalManager {
  return proxy.ProductMountPoints;
  }
 
+ /**
+ * Gets the list of encryption methods accepted by the proposal
+ *
+ * @returns {Promise<string[]>}
+ */
+ async getEncryptionMethods() {
+ const proxy = await this.proxies.proposalCalculator;
+ return proxy.EncryptionMethods;
+ }
+
  /**
  * Obtains the default volume for the given mount path
  *
@@ -345,6 +367,7 @@ class ProposalManager {
  spacePolicy: proxy.SpacePolicy,
  systemVGDevices: proxy.SystemVGDevices,
  encryptionPassword: proxy.EncryptionPassword,
+ encryptionMethod: proxy.EncryptionMethod,
  volumes: proxy.Volumes.map(this.buildVolume),
  // NOTE: strictly speaking, installation devices does not belong to the settings. It
  // should be a separate method instead of an attribute in the settings object.
@@ -365,7 +388,7 @@ class ProposalManager {
  * @param {ProposalSettings} settings
  * @returns {Promise<number>} 0 on success, 1 on failure
  */
- async calculate({ bootDevice, encryptionPassword, lvm, spacePolicy, systemVGDevices, volumes }) {
+ async calculate({ bootDevice, encryptionPassword, encryptionMethod, lvm, spacePolicy, systemVGDevices, volumes }) {
  const dbusVolume = (volume) => {
  return removeUndefinedCockpitProperties({
  MountPath: { t: "s", v: volume.mountPath },
@@ -381,6 +404,7 @@ class ProposalManager {
  const settings = removeUndefinedCockpitProperties({
  BootDevice: { t: "s", v: bootDevice },
  EncryptionPassword: { t: "s", v: encryptionPassword },
+ EncryptionMethod: { t: "s", v: encryptionMethod },
  LVM: { t: "b", v: lvm },
  SpacePolicy: { t: "s", v: spacePolicy },
  SystemVGDevices: { t: "as", v: systemVGDevices },
@@ -1420,4 +1444,4 @@ class StorageClient extends WithIssues(
  ), STORAGE_OBJECT
 ) { }
 
-export { StorageClient };
+export { StorageClient, EncryptionMethods };