From 1229d95e152191fbd3f42bcff610c7ee76e331f8 Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Fri, 12 Jan 2024 11:42:18 +0000 Subject: [PATCH 1/9] [Service] Changes related to EncryptionMethod - Use LUKS2 with PBKDF2 as default. - Expose the list of available methods (LUKS2 and TPM_FDE so far) in the D-Bus API. --- products.d/ALP-Dolomite.yaml | 4 +-- service/lib/agama/dbus/storage/manager.rb | 12 ++++++- .../lib/agama/storage/encryption_settings.rb | 31 ++++++++++++++++++- .../agama/storage/proposal_settings_reader.rb | 17 ++++------ .../to_dbus_test.rb | 4 +-- .../storage/proposal_settings_reader_test.rb | 12 +++---- 6 files changed, 56 insertions(+), 24 deletions(-) diff --git a/products.d/ALP-Dolomite.yaml b/products.d/ALP-Dolomite.yaml index 45208d4525..60103a6393 100644 --- a/products.d/ALP-Dolomite.yaml +++ b/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: diff --git a/service/lib/agama/dbus/storage/manager.rb b/service/lib/agama/dbus/storage/manager.rb index 57cf3261a5..188062579c 100644 --- a/service/lib/agama/dbus/storage/manager.rb +++ b/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] 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] + 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| diff --git a/service/lib/agama/storage/encryption_settings.rb b/service/lib/agama/storage/encryption_settings.rb index 5e308a6398..eb755d5b62 100644 --- a/service/lib/agama/storage/encryption_settings.rb +++ b/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] + def self.encryption_methods + METHODS + end + + # All methods that can be used to calculate a proposal for the current system and product + # + # @return [Array] + 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 diff --git a/service/lib/agama/storage/proposal_settings_reader.rb b/service/lib/agama/storage/proposal_settings_reader.rb index 8b04c0beac..fff9245e9e 100644 --- a/service/lib/agama/storage/proposal_settings_reader.rb +++ b/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] diff --git a/service/test/agama/dbus/storage/proposal_settings_conversion/to_dbus_test.rb b/service/test/agama/dbus/storage/proposal_settings_conversion/to_dbus_test.rb index de6ad33d09..168ab4852e 100644 --- a/service/test/agama/dbus/storage/proposal_settings_conversion/to_dbus_test.rb +++ b/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" => [] diff --git a/service/test/agama/storage/proposal_settings_reader_test.rb b/service/test/agama/storage/proposal_settings_reader_test.rb index 88188076f1..b5bed59cf0 100644 --- a/service/test/agama/storage/proposal_settings_reader_test.rb +++ b/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 From 49c98064290e2d31aa247c783f8a522f0e6a2a82 Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Fri, 12 Jan 2024 11:46:26 +0000 Subject: [PATCH 2/9] [Web] Allow to select TPM_FDE encryption if available --- web/src/client/storage.js | 15 ++++- .../overview/StorageSection.test.jsx | 3 + web/src/components/storage/ProposalPage.jsx | 15 ++++- .../components/storage/ProposalPage.test.jsx | 1 + .../storage/ProposalSettingsSection.jsx | 62 ++++++++++++++----- 5 files changed, 79 insertions(+), 17 deletions(-) diff --git a/web/src/client/storage.js b/web/src/client/storage.js index 7290b74bc3..338d7af153 100644 --- a/web/src/client/storage.js +++ b/web/src/client/storage.js @@ -227,6 +227,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 +284,16 @@ class ProposalManager { return proxy.ProductMountPoints; } + /** + * Gets the list of encryption methods accepted by the proposal + * + * @returns {Promise} + */ + async getEncryptionMethods() { + const proxy = await this.proxies.proposalCalculator; + return proxy.EncryptionMethods; + } + /** * Obtains the default volume for the given mount path * @@ -345,6 +356,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 +377,7 @@ class ProposalManager { * @param {ProposalSettings} settings * @returns {Promise} 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 +393,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 }, diff --git a/web/src/components/overview/StorageSection.test.jsx b/web/src/components/overview/StorageSection.test.jsx index 9c29e02f34..ba08280298 100644 --- a/web/src/components/overview/StorageSection.test.jsx +++ b/web/src/components/overview/StorageSection.test.jsx @@ -35,6 +35,8 @@ const availableDevices = [ { name: "/dev/sdb", size: 697932185600 } ]; +const encryptionMethods = ["luks2", "tpm_fde"]; + const proposalResult = { settings: { bootDevice: "/dev/sda", @@ -48,6 +50,7 @@ const storageMock = { probe: jest.fn().mockResolvedValue(0), proposal: { getAvailableDevices: jest.fn().mockResolvedValue(availableDevices), + getEncryptionMethods: jest.fn().mockResolvedValue(encryptionMethods), getResult: jest.fn().mockResolvedValue(proposalResult), calculate: jest.fn().mockResolvedValue(0) }, diff --git a/web/src/components/storage/ProposalPage.jsx b/web/src/components/storage/ProposalPage.jsx index 335c939167..4680d9cf1a 100644 --- a/web/src/components/storage/ProposalPage.jsx +++ b/web/src/components/storage/ProposalPage.jsx @@ -54,6 +54,11 @@ const reducer = (state, action) => { return { ...state, availableDevices }; } + case "UPDATE_ENCRYPTION_METHODS": { + const { encryptionMethods } = action.payload; + return { ...state, encryptionMethods }; + } + case "UPDATE_VOLUME_TEMPLATES": { const { volumeTemplates } = action.payload; return { ...state, volumeTemplates }; @@ -89,6 +94,10 @@ export default function ProposalPage() { return await cancellablePromise(client.proposal.getAvailableDevices()); }, [client, cancellablePromise]); + const loadEncryptionMethods = useCallback(async () => { + return await cancellablePromise(client.proposal.getEncryptionMethods()); + }, [client, cancellablePromise]); + const loadVolumeTemplates = useCallback(async () => { const mountPoints = await cancellablePromise(client.proposal.getProductMountPoints()); const volumeTemplates = []; @@ -127,6 +136,9 @@ export default function ProposalPage() { const availableDevices = await loadAvailableDevices(); dispatch({ type: "UPDATE_AVAILABLE_DEVICES", payload: { availableDevices } }); + const encryptionMethods = await loadEncryptionMethods(); + dispatch({ type: "UPDATE_ENCRYPTION_METHODS", payload: { encryptionMethods } }); + const volumeTemplates = await loadVolumeTemplates(); dispatch({ type: "UPDATE_VOLUME_TEMPLATES", payload: { volumeTemplates } }); @@ -137,7 +149,7 @@ export default function ProposalPage() { dispatch({ type: "UPDATE_ERRORS", payload: { errors } }); if (result !== undefined) dispatch({ type: "STOP_LOADING" }); - }, [calculateProposal, cancellablePromise, client, loadAvailableDevices, loadErrors, loadProposalResult, loadVolumeTemplates]); + }, [calculateProposal, cancellablePromise, client, loadAvailableDevices, loadEncryptionMethods, loadErrors, loadProposalResult, loadVolumeTemplates]); const calculate = useCallback(async (settings) => { dispatch({ type: "START_LOADING" }); @@ -200,6 +212,7 @@ export default function ProposalPage() { Promise.resolve({ mountPath })), diff --git a/web/src/components/storage/ProposalSettingsSection.jsx b/web/src/components/storage/ProposalSettingsSection.jsx index dc0b82ab40..37c66d5228 100644 --- a/web/src/components/storage/ProposalSettingsSection.jsx +++ b/web/src/components/storage/ProposalSettingsSection.jsx @@ -385,13 +385,18 @@ created in a logical volume of the system volume group."); * @callback onValidateFn * @param {boolean} valid */ -const EncryptionPasswordForm = ({ +const EncryptionSettingsForm = ({ id, password: passwordProp, + method: methodProp, + methods, onSubmit = noop, onValidate = noop }) => { const [password, setPassword] = useState(passwordProp || ""); + const [method, setMethod] = useState(methodProp); + const tpmId = "tpm_fde"; + const luks2Id = "luks2"; useEffect(() => { if (password.length === 0) onValidate(false); @@ -399,9 +404,13 @@ const EncryptionPasswordForm = ({ const changePassword = (_, v) => setPassword(v); + const changeMethod = (_, value) => { + value ? setMethod(tpmId) : setMethod(luks2Id); + }; + const submitForm = (e) => { e.preventDefault(); - onSubmit(password); + onSubmit(password, method); }; return ( @@ -412,16 +421,29 @@ const EncryptionPasswordForm = ({ onChange={changePassword} onValidation={onValidate} /> + + } + /> ); }; /** - * Allows to selected encryption + * Allows to define encryption * @component * * @param {object} props * @param {string} [props.password=""] - Password for encryption + * @param {string} [props.method=""] - Encryption method * @param {boolean} [props.isChecked=false] - Whether encryption is selected * @param {boolean} [props.isLoading=false] - Whether to show the selector as loading * @param {onChangeFn} [props.onChange=noop] - On change callback @@ -429,14 +451,17 @@ const EncryptionPasswordForm = ({ * @callback onChangeFn * @param {object} settings */ -const EncryptionPasswordField = ({ +const EncryptionField = ({ password: passwordProp = "", + method: methodProp = "", + methods, isChecked: isCheckedProp = false, isLoading = false, onChange = noop }) => { const [isChecked, setIsChecked] = useState(isCheckedProp); const [password, setPassword] = useState(passwordProp); + const [method, setMethod] = useState(methodProp); const [isFormOpen, setIsFormOpen] = useState(false); const [isFormValid, setIsFormValid] = useState(true); @@ -444,10 +469,11 @@ const EncryptionPasswordField = ({ const closeForm = () => setIsFormOpen(false); - const acceptForm = (newPassword) => { + const acceptForm = (newPassword, newMethod) => { closeForm(); setPassword(newPassword); - onChange({ isChecked, password: newPassword }); + setMethod(newMethod); + onChange({ isChecked, password: newPassword, method: newMethod }); }; const cancelForm = () => { @@ -468,10 +494,10 @@ const EncryptionPasswordField = ({ } }; - const ChangePasswordButton = () => { + const ChangeSettingsButton = () => { return ( - { isChecked && } + { isChecked && } - - {_("Accept")} + {_("Accept")} @@ -618,6 +646,7 @@ const SpacePolicyField = ({ * @param {ProposalSettings} props.settings * @param {StorageDevice[]} [props.availableDevices=[]] * @param {Volume[]} [props.volumeTemplates=[]] + * @param {String[]} [props.encryptionMethods=[]] * @param {boolean} [isLoading=false] * @param {onChangeFn} [props.onChange=noop] * @@ -628,6 +657,7 @@ export default function ProposalSettingsSection({ settings, availableDevices = [], volumeTemplates = [], + encryptionMethods = [], isLoading = false, onChange = noop }) { @@ -643,8 +673,8 @@ export default function ProposalSettingsSection({ onChange(settings); }; - const changeEncryption = ({ password }) => { - onChange({ encryptionPassword: password }); + const changeEncryption = ({ password, method }) => { + onChange({ encryptionPassword: password, encryptionMethod: method }); }; const changeSpacePolicy = (policy) => { @@ -673,8 +703,10 @@ export default function ProposalSettingsSection({ isLoading={settings.lvm === undefined} onChange={changeLVM} /> - Date: Fri, 12 Jan 2024 15:10:19 +0000 Subject: [PATCH 3/9] [Web] Add explanation about TPM and reboot --- web/src/components/storage/ProposalSettingsSection.jsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/web/src/components/storage/ProposalSettingsSection.jsx b/web/src/components/storage/ProposalSettingsSection.jsx index 37c66d5228..fdd3c207d4 100644 --- a/web/src/components/storage/ProposalSettingsSection.jsx +++ b/web/src/components/storage/ProposalSettingsSection.jsx @@ -22,7 +22,7 @@ import React, { useEffect, useState } from "react"; import { Button, - Form, Skeleton, Switch, + Form, Skeleton, Switch, Checkbox, ToggleGroup, ToggleGroupItem, Tooltip } from "@patternfly/react-core"; @@ -424,10 +424,12 @@ const EncryptionSettingsForm = ({ From d5a6fcf3f60d5ea6f366c0f13831bf3db95f5bc1 Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Mon, 15 Jan 2024 15:20:37 +0000 Subject: [PATCH 4/9] [Web] Make TPM implications more clear --- .../components/core/InstallationFinished.jsx | 45 ++++++++++++++++++- .../core/InstallationFinished.test.jsx | 5 ++- .../storage/ProposalSettingsSection.jsx | 2 + 3 files changed, 49 insertions(+), 3 deletions(-) diff --git a/web/src/components/core/InstallationFinished.jsx b/web/src/components/core/InstallationFinished.jsx index b6a2566f26..525ec47960 100644 --- a/web/src/components/core/InstallationFinished.jsx +++ b/web/src/components/core/InstallationFinished.jsx @@ -26,18 +26,46 @@ import { EmptyStateBody, EmptyStateHeader, EmptyStateIcon, + ExpandableSection, + Hint, + HintBody, } from "@patternfly/react-core"; -import { Page } from "~/components/core"; +import { Page, If } from "~/components/core"; import { Center, Icon } from "~/components/layout"; import { useInstallerClient } from "~/context/installer"; import { _ } from "~/i18n"; +const TpmHint = () => { + const [isExpanded, setIsExpanded] = useState(false); + + return ( + + + setIsExpanded(!isExpanded)} + toggleText={ + + {_("Make sure to boot directly to the new system to finish the configuration of TPM-based decryption.")} + + } + > + + {_("The final step to configure the TPM to automatically open devices will take place during the first boot of the new system. For that to work, the machine needs to boot directly to the new boot loader. If a local media was used to run this installer, make sure is removed before next boot.")} + + + + + ); +}; + const SuccessIcon = () => ; function InstallationFinished() { const client = useInstallerClient(); const [iguana, setIguana] = useState(false); + const [tpm, setTpm] = useState(false); const closingAction = () => client.manager.finishInstallation(); const buttonCaption = iguana // TRANSLATORS: button label @@ -51,7 +79,17 @@ function InstallationFinished() { setIguana(ret); } + // FIXME: This logic should likely not be placed here, it's too complex and too coupled to storage internals. + // Something to fix when this whole page is refactored in a (hopefully near) future. + async function getTpm() { + const result = await client.storage.proposal.getResult(); + const method = result.settings.encryptionMethod; + const tpmId = "tpm_fde"; + setTpm(method === tpmId); + } + getIguana(); + getTpm(); }); return ( @@ -73,7 +111,10 @@ function InstallationFinished() { : _("At this point you can reboot the machine to log in to the new system.") } - {_("Have a lot of fun! Your openSUSE Development Team.")} + } + /> diff --git a/web/src/components/core/InstallationFinished.test.jsx b/web/src/components/core/InstallationFinished.test.jsx index f3af126630..0b8dd93ec3 100644 --- a/web/src/components/core/InstallationFinished.test.jsx +++ b/web/src/components/core/InstallationFinished.test.jsx @@ -32,6 +32,8 @@ jest.mock("~/components/core/Sidebar", () => () =>
Agama sidebar
); const finishInstallationFn = jest.fn(); +const storageSettings = { encryptionMethod: "luks2" }; + describe("InstallationFinished", () => { beforeEach(() => { createClient.mockImplementation(() => { @@ -39,7 +41,8 @@ describe("InstallationFinished", () => { manager: { finishInstallation: finishInstallationFn, useIguana: () => Promise.resolve(false) - } + }, + storage: { proposal: { getResult: jest.fn().mockResolvedValue({ settings: storageSettings }) } } }; }); }); diff --git a/web/src/components/storage/ProposalSettingsSection.jsx b/web/src/components/storage/ProposalSettingsSection.jsx index fdd3c207d4..4168e0d25f 100644 --- a/web/src/components/storage/ProposalSettingsSection.jsx +++ b/web/src/components/storage/ProposalSettingsSection.jsx @@ -428,6 +428,8 @@ const EncryptionSettingsForm = ({ id="encryption_method" label={_("Use the TPM to decrypt automatically on each boot")} description={ + // TRANSLATORS: The word 'directly' is key here. For example, booting to the installer media and then choosing + // 'Boot from Hard Disk' from there will not work. Keep it sort (this is a hint in a form) but keep it clear. _("If this option is enabled, make sure to boot directly the new system after installation. Remove this installation media if needed.") } isChecked={method === tpmId} From 58bfe7826ea8fb5bf2f0748d95cc65356fee985e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Mon, 15 Jan 2024 22:11:26 +0000 Subject: [PATCH 5/9] [web] Add tests for InstallationFinished page For checking that conditional information is rendered when appropiate. Even though that page will be reworked in a near future, keeping the testsuite up-to-date worth more than its cost. --- .../core/InstallationFinished.test.jsx | 28 +++++++++++++++++-- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/web/src/components/core/InstallationFinished.test.jsx b/web/src/components/core/InstallationFinished.test.jsx index 0b8dd93ec3..f34998ffb3 100644 --- a/web/src/components/core/InstallationFinished.test.jsx +++ b/web/src/components/core/InstallationFinished.test.jsx @@ -31,18 +31,22 @@ jest.mock("~/client"); jest.mock("~/components/core/Sidebar", () => () =>
Agama sidebar
); const finishInstallationFn = jest.fn(); - -const storageSettings = { encryptionMethod: "luks2" }; +let encryptionMethod; describe("InstallationFinished", () => { beforeEach(() => { + encryptionMethod = "luks2"; createClient.mockImplementation(() => { return { manager: { finishInstallation: finishInstallationFn, useIguana: () => Promise.resolve(false) }, - storage: { proposal: { getResult: jest.fn().mockResolvedValue({ settings: storageSettings }) } } + storage: { + proposal: { + getResult: jest.fn().mockResolvedValue({ settings: { encryptionMethod } }) + }, + } }; }); }); @@ -63,4 +67,22 @@ describe("InstallationFinished", () => { await user.click(rebootButton); expect(finishInstallationFn).toHaveBeenCalled(); }); + + describe("when TPM was set to decrypt automatically on each boot", () => { + beforeEach(() => { + encryptionMethod = "tpm_fde"; + }); + + it("shows the TPM reminder", async () => { + installerRender(); + await screen.findAllByText(/TPM/); + }); + }); + + describe("when TPM was not set to decrypt automatically on each boot", () => { + it("does not show the TPM reminder", () => { + installerRender(); + expect(screen.queryByText(/TPM/)).toBeNull(); + }); + }); }); From 77e833983da9f1d7bf27c92661c6c9e4ead85e70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Mon, 15 Jan 2024 18:33:31 +0000 Subject: [PATCH 6/9] [web] Code improvements for InstallationFinished page In spite of the InstallationPage needs to be reworked a bit more (and possibly to include a loading internal state or similar), this commit introduces minor code improvements that were cheaper to apply right now. First of all, the TPM reminder is only shown if encryption was set. Additionally, the reminder is improved to avoid it changing the width when the user expands it. Done by relaying in CSS minmax function and CSS container queries. Both with wide support of modern browsers. This commit also stops centering vertically the page content because the very same reason: having dinamic components make things moving top / down depending on their state. It also moves the TPM id to an enum created in the client/storage for the encryption methods, following the same approach as network code. It can be moved to an specific file like it is already done in client/phase or client/status, though. Something to be defined/polished in next iterations. [1] https://developer.mozilla.org/es/docs/Web/CSS/minmax [2] https://developer.mozilla.org/es/docs/Web/CSS/CSS_container_queries --- web/src/assets/styles/blocks.scss | 5 ++ web/src/assets/styles/layout.scss | 2 + web/src/client/storage.js | 15 +++- .../components/core/InstallationFinished.jsx | 85 +++++++++---------- .../core/InstallationFinished.test.jsx | 54 +++++++++--- 5 files changed, 100 insertions(+), 61 deletions(-) diff --git a/web/src/assets/styles/blocks.scss b/web/src/assets/styles/blocks.scss index 70bac9f72d..85eed3db67 100644 --- a/web/src/assets/styles/blocks.scss +++ b/web/src/assets/styles/blocks.scss @@ -430,3 +430,8 @@ ul[data-of="agama/timezones"] { } } } + +.tpm-hint { + grid-template-columns: minmax(100%, 70cqw); + text-align: start; +} diff --git a/web/src/assets/styles/layout.scss b/web/src/assets/styles/layout.scss index a642ff4176..6e879528f1 100644 --- a/web/src/assets/styles/layout.scss +++ b/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 { diff --git a/web/src/client/storage.js b/web/src/client/storage.js index 338d7af153..ba74801644 100644 --- a/web/src/client/storage.js +++ b/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 * @@ -1433,4 +1444,4 @@ class StorageClient extends WithIssues( ), STORAGE_OBJECT ) { } -export { StorageClient }; +export { StorageClient, EncryptionMethods }; diff --git a/web/src/components/core/InstallationFinished.jsx b/web/src/components/core/InstallationFinished.jsx index 525ec47960..f432e08e59 100644 --- a/web/src/components/core/InstallationFinished.jsx +++ b/web/src/components/core/InstallationFinished.jsx @@ -1,5 +1,5 @@ /* - * Copyright (c) [2022] SUSE LLC + * Copyright (c) [2022-2024] SUSE LLC * * All Rights Reserved. * @@ -31,16 +31,17 @@ import { HintBody, } from "@patternfly/react-core"; -import { Page, If } from "~/components/core"; -import { Center, Icon } from "~/components/layout"; +import { If, Page } from "~/components/core"; +import { Icon } from "~/components/layout"; import { useInstallerClient } from "~/context/installer"; +import { EncryptionMethods } from "~/client/storage"; import { _ } from "~/i18n"; const TpmHint = () => { const [isExpanded, setIsExpanded] = useState(false); return ( - + { - async function getIguana() { - const ret = await client.manager.useIguana(); - setIguana(ret); + async function preparePage() { + const iguana = await client.manager.useIguana(); + // FIXME: This logic should likely not be placed here, it's too coupled to storage internals. + // Something to fix when this whole page is refactored in a (hopefully near) future. + const { settings: { encryptionPassword, encryptionMethod } } = await client.storage.proposal.getResult(); + setUsingIguana(iguana); + setUsingTpm(encryptionPassword?.length && encryptionMethod === EncryptionMethods.TPM); } - // FIXME: This logic should likely not be placed here, it's too complex and too coupled to storage internals. - // Something to fix when this whole page is refactored in a (hopefully near) future. - async function getTpm() { - const result = await client.storage.proposal.getResult(); - const method = result.settings.encryptionMethod; - const tpmId = "tpm_fde"; - setTpm(method === tpmId); - } - - getIguana(); - getTpm(); + // TODO: display the page in a loading mode while needed data is being fetched. + preparePage(); }); return ( // TRANSLATORS: page title -
- - } - /> - - {_("The installation on your machine is complete.")} - - { - iguana - ? _("At this point you can power off the machine.") - : _("At this point you can reboot the machine to log in to the new system.") - } - + + } + /> + + {_("The installation on your machine is complete.")} + } + condition={usingIguana} + then={_("At this point you can power off the machine.")} + else={_("At this point you can reboot the machine to log in to the new system.")} /> - - -
+ + } + /> + + - {buttonCaption} + + {usingIguana ? _("Finish") : _("Reboot")} +
); diff --git a/web/src/components/core/InstallationFinished.test.jsx b/web/src/components/core/InstallationFinished.test.jsx index f34998ffb3..a70c9d731c 100644 --- a/web/src/components/core/InstallationFinished.test.jsx +++ b/web/src/components/core/InstallationFinished.test.jsx @@ -1,5 +1,5 @@ /* - * Copyright (c) [2022] SUSE LLC + * Copyright (c) [2022-2024] SUSE LLC * * All Rights Reserved. * @@ -24,6 +24,7 @@ import React from "react"; import { screen } from "@testing-library/react"; import { installerRender } from "~/test-utils"; import { createClient } from "~/client"; +import { EncryptionMethods } from "~/client/storage"; import InstallationFinished from "./InstallationFinished"; @@ -31,11 +32,13 @@ jest.mock("~/client"); jest.mock("~/components/core/Sidebar", () => () =>
Agama sidebar
); const finishInstallationFn = jest.fn(); +let encryptionPassword; let encryptionMethod; describe("InstallationFinished", () => { beforeEach(() => { - encryptionMethod = "luks2"; + encryptionPassword = "n0tS3cr3t"; + encryptionMethod = EncryptionMethods.LUKS2; createClient.mockImplementation(() => { return { manager: { @@ -44,7 +47,9 @@ describe("InstallationFinished", () => { }, storage: { proposal: { - getResult: jest.fn().mockResolvedValue({ settings: { encryptionMethod } }) + getResult: jest.fn().mockResolvedValue({ + settings: { encryptionMethod, encryptionPassword } + }) }, } }; @@ -68,21 +73,46 @@ describe("InstallationFinished", () => { expect(finishInstallationFn).toHaveBeenCalled(); }); - describe("when TPM was set to decrypt automatically on each boot", () => { + describe("when TPM is set as encryption method", () => { beforeEach(() => { - encryptionMethod = "tpm_fde"; + encryptionMethod = EncryptionMethods.TPM; }); - it("shows the TPM reminder", async () => { - installerRender(); - await screen.findAllByText(/TPM/); + describe("and encryption was set", () => { + it("shows the TPM reminder", async () => { + installerRender(); + await screen.findAllByText(/TPM/); + }); + }); + + describe("but encryption was not set", () => { + beforeEach(() => { + encryptionPassword = ""; + }); + + it("does not show the TPM reminder", async () => { + const { user } = installerRender(); + // Forcing the test to slow down a bit with a fake user interaction + // because actually the reminder will be not rendered immediately + // making the queryAllByText to produce a false positive if triggered + // too early here. + const congratsText = screen.getByText("Congratulations!"); + await user.click(congratsText); + expect(screen.queryAllByText(/TPM/)).toHaveLength(0); + }); }); }); - describe("when TPM was not set to decrypt automatically on each boot", () => { - it("does not show the TPM reminder", () => { - installerRender(); - expect(screen.queryByText(/TPM/)).toBeNull(); + describe("when TPM is not set as encryption method", () => { + it("does not show the TPM reminder", async () => { + const { user } = installerRender(); + // Forcing the test to slow down a bit with a fake user interaction + // because actually the reminder will be not rendered immediately + // making the queryAllByText to produce a false positive if triggered + // too early here. + const congratsText = screen.getByText("Congratulations!"); + await user.click(congratsText); + expect(screen.queryAllByText(/TPM/)).toHaveLength(0); }); }); }); From 25260c6fa77151f3d5536ec5bb0595cc27f25362 Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Wed, 17 Jan 2024 12:30:41 +0000 Subject: [PATCH 7/9] [web] Fix from core review --- web/src/components/storage/ProposalPage.jsx | 1 + 1 file changed, 1 insertion(+) diff --git a/web/src/components/storage/ProposalPage.jsx b/web/src/components/storage/ProposalPage.jsx index 4680d9cf1a..cbeb1d84ec 100644 --- a/web/src/components/storage/ProposalPage.jsx +++ b/web/src/components/storage/ProposalPage.jsx @@ -34,6 +34,7 @@ const initialState = { loading: true, availableDevices: [], volumeTemplates: [], + encryptionMethods: [], settings: {}, actions: [], errors: [] From c4819a1dd6d182e38fccf021e299875dc62879fa Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Wed, 17 Jan 2024 16:07:20 +0000 Subject: [PATCH 8/9] [web] Rework TPM texts --- web/src/assets/styles/blocks.scss | 11 +++++++- .../components/core/InstallationFinished.jsx | 28 +++++++++---------- .../storage/ProposalSettingsSection.jsx | 17 +++++++---- 3 files changed, 36 insertions(+), 20 deletions(-) diff --git a/web/src/assets/styles/blocks.scss b/web/src/assets/styles/blocks.scss index 85eed3db67..d59ceebc60 100644 --- a/web/src/assets/styles/blocks.scss +++ b/web/src/assets/styles/blocks.scss @@ -432,6 +432,15 @@ ul[data-of="agama/timezones"] { } .tpm-hint { - grid-template-columns: minmax(100%, 70cqw); + 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%; + } } diff --git a/web/src/components/core/InstallationFinished.jsx b/web/src/components/core/InstallationFinished.jsx index f432e08e59..98fb3d8732 100644 --- a/web/src/components/core/InstallationFinished.jsx +++ b/web/src/components/core/InstallationFinished.jsx @@ -21,14 +21,13 @@ import React, { useState, useEffect } from "react"; import { + Alert, Text, EmptyState, EmptyStateBody, EmptyStateHeader, EmptyStateIcon, ExpandableSection, - Hint, - HintBody, } from "@patternfly/react-core"; import { If, Page } from "~/components/core"; @@ -39,25 +38,26 @@ import { _ } from "~/i18n"; const TpmHint = () => { const [isExpanded, setIsExpanded] = useState(false); + const title = _("TPM sealing requires the new system to be booted directly."); return ( - - + {title}}> +
+ {_("If a local media was used to run this installer, remove it before the next boot.")} setIsExpanded(!isExpanded)} - toggleText={ - - {_("Make sure to boot directly to the new system to finish the configuration of TPM-based decryption.")} - - } + toggleText={isExpanded ? _("Hide details") : _("See more details")} > - - {_("The final step to configure the TPM to automatically open devices will take place during the first boot of the new system. For that to work, the machine needs to boot directly to the new boot loader. If a local media was used to run this installer, make sure is removed before next boot.")} - + TPM to automatically open encrypted devices will take place during the first boot of the new system. For that to work, the machine needs to boot directly to the new boot loader.") + }} + /> - - +
+
); }; diff --git a/web/src/components/storage/ProposalSettingsSection.jsx b/web/src/components/storage/ProposalSettingsSection.jsx index 4168e0d25f..d5b7bccc16 100644 --- a/web/src/components/storage/ProposalSettingsSection.jsx +++ b/web/src/components/storage/ProposalSettingsSection.jsx @@ -413,6 +413,17 @@ const EncryptionSettingsForm = ({ onSubmit(password, method); }; + const Description = () => ( + TPM can verify the integrity of the system. TPM sealing requires the new system to be booted directly on its first run.") + }} + /> + ); + return (
} isChecked={method === tpmId} onChange={changeMethod} /> From 432b5b48ab8719540aa1c00e6be81f4dc9f4c192 Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Thu, 18 Jan 2024 09:38:40 +0100 Subject: [PATCH 9/9] Changelog entries --- service/package/rubygem-agama.changes | 6 ++++++ web/package/cockpit-agama.changes | 5 +++++ 2 files changed, 11 insertions(+) diff --git a/service/package/rubygem-agama.changes b/service/package/rubygem-agama.changes index 15837bc65c..065179651c 100644 --- a/service/package/rubygem-agama.changes +++ b/service/package/rubygem-agama.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Thu Jan 18 08:35:01 UTC 2024 - Ancor Gonzalez Sosa + +- 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 diff --git a/web/package/cockpit-agama.changes b/web/package/cockpit-agama.changes index 8882dd0190..130068a264 100644 --- a/web/package/cockpit-agama.changes +++ b/web/package/cockpit-agama.changes @@ -1,3 +1,8 @@ +------------------------------------------------------------------- +Thu Jan 18 08:33:52 UTC 2024 - Ancor Gonzalez Sosa + +- 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