feat: warn/confirm with user if enabling fips downgrades the kernel #2890

catmsred · 2023-12-12T21:30:50Z

This change adds a new message prompting the user to confirm if enabling fips downgrades the kernel.

Why is this needed?

There have been reports of people enabling FIPS (not realizing it will downgrade their kernel), and having their hardware fail as it needed the new kernel. This change adds an additional warning/prompt if a FIPS enablement is determined to downgrade the kernel.

Jira card: SC-1534

Test Steps

On a pro (but not FIPS) VM, enable FIPS and you should see the new messages.

Checklist

I have updated or added any unit tests accordingly
I have updated or added any integration tests accordingly
Changes here need to be documented, and this was done in: [N/A]

Does this PR require extra reviews?

Yes
No

github-actions · 2023-12-12T21:31:05Z

🌎 This PR changes translatable messages. 🌏

Please select which scenarios apply. For further explanation, please read our policy on message changes.

New messages are being added.
- We will ask translators to take a look and add translations if they have time, but it will not block this PR.
Existing messages are being changed.
- ⚠️ Please add a comment with justification of why messages are being altered.
- If the changes are trivial (e.g. a typo fix), then translations must be preserved.
- If the changes are substantial, then we will ask translators to take a look and update translations if they have time, but it will not block this PR.
Existing messages are being deleted.
- No special action needed.

github-actions · 2023-12-12T21:31:05Z

Jira: This PR is not related to a Jira item. (The PR title does not include a SC-#### reference)

GitHub Issues: No GitHub issues are fixed by this PR. (No commits have Fixes: #### references)

Launchpad Bugs: No Launchpad bugs are fixed by this PR. (No commits have LP: #### references)

Documentation: The changes in this PR do not require documentation changes.

👍 this comment to confirm that this is correct.

uaclient/entitlements/fips.py

catmsred · 2024-01-25T00:20:52Z

Integration test failures are just proxy tests which I don't think have anything to do with this code

lucasmoura · 2024-01-26T20:05:42Z

uaclient/entitlements/fips.py

@@ -195,8 +247,14 @@ def install_packages(

        for pkg in desired_packages:
            try:
-                super().install_packages(
-                    package_list=[pkg], cleanup_on_failure=False, verbose=False
+                apt.run_apt_install_command(


Just double checking, why are we changing this ?

Every time super().install_packages is called, the kernel downgrade warning is sent. In my testing, we generally saw it three times. Grant suggested that this call should properly be run_apt_install_command instead.

ohh fair enough, that makes total sense. Thanks for the context here

uaclient/entitlements/fips.py

lucasmoura · 2024-01-26T20:34:34Z

features/enable_fips_pro.feature

        """
        Updating <fips-name> package lists
        Installing <fips-name> packages
-        Updating standard Ubuntu package lists
+        This will downgrade the kernel from .+ to .+\.


Is this test supposed to pass on all releases specified in this Scenario Outline ? I have tested the bionic aws.pro and the test didn't work

Where is this failing for you? It worked for all the iterations I ran of it and is currently passing in CI -- the aws.pro bionic failure is only for an unrelated proxy test.

That's weird. I am getting the following error on bionic:

Assertion Failed: Expected to match regexp: Updating FIPS package lists Installing FIPS packages This will downgrade the kernel from .+ to .+\. Warning: Downgrading the kernel may cause hardware failures. Please ensure the hardware is compatible with the new kernel version before proceeding. Updating standard Ubuntu package lists(\n.*)? FIPS enabled A reboot is required to complete install But got: One moment, checking your subscription first Disabling incompatible service: Livepatch Updating FIPS package lists Installing FIPS packages Updating standard Ubuntu package lists FIPS enabled A reboot is required to complete install.

But I haven't checked the other releases yet

Hi, thanks for the check! I have since fixed the downgrade conditional -- let me know if you're still seeing unexpected failures?

renanrodrigo

LGTM
All CI failures caused by flaky network connections, and not affected by this PR at all.

lucasmoura

@catmsred I can confirm the tests are now passing for me. Just one more addition to this PR and I will approve it

lucasmoura · 2024-01-31T14:27:26Z

uaclient/entitlements/fips.py

@@ -154,6 +155,56 @@ def conditional_packages(self):

        return FIPS_CONDITIONAL_PACKAGES.get(series, [])

+    def prompt_if_kernel_downgrade(


I think we just need some unittests for this method now

lucasmoura

Thanks for all the work on this issue @catmsred. I have just one tiny nit here, but let me know if I am missing something

lucasmoura · 2024-02-01T13:30:44Z

uaclient/entitlements/tests/test_fips.py

+        entitlement,
+    ):
+        """Test kernel downgrades block install if user denies prompt"""
+        m_prompt_for_confirmation = util.prompt_for_confirmation


I think I am missing something. Why do we need this line here ?

We do not! Good catch, that was left over from my experiments for the best way to handle this mock. I will clean it up and then we should be good to merge.

This change adds a new message prompting the user to confirm if enabling fips downgrades the kernel.

orndorffgrant · 2024-02-01T18:30:23Z

CI fails are flakes + two approvals = merging
Thank you @catmsred!

catmsred force-pushed the feature/fips_downgrade_warn branch 5 times, most recently from 6fa3681 to 4ff8838 Compare December 13, 2023 17:06

lucasmoura reviewed Dec 15, 2023

View reviewed changes

uaclient/entitlements/fips.py Outdated Show resolved Hide resolved

uaclient/entitlements/fips.py Outdated Show resolved Hide resolved

uaclient/entitlements/fips.py Outdated Show resolved Hide resolved

catmsred force-pushed the feature/fips_downgrade_warn branch 2 times, most recently from cdfc0fb to 33e8ae7 Compare January 10, 2024 14:09

orndorffgrant changed the base branch from main to next-v32 January 11, 2024 18:56

catmsred force-pushed the feature/fips_downgrade_warn branch 7 times, most recently from 28d6489 to e70b077 Compare January 24, 2024 20:53

catmsred requested review from lucasmoura and renanrodrigo January 25, 2024 00:19

catmsred force-pushed the feature/fips_downgrade_warn branch from e70b077 to 06827a5 Compare January 25, 2024 00:20

lucasmoura reviewed Jan 26, 2024

View reviewed changes

catmsred force-pushed the feature/fips_downgrade_warn branch 3 times, most recently from 255894a to 3708517 Compare January 30, 2024 16:06

renanrodrigo approved these changes Jan 30, 2024

View reviewed changes

catmsred requested a review from lucasmoura January 30, 2024 19:57

lucasmoura reviewed Jan 31, 2024

View reviewed changes

catmsred force-pushed the feature/fips_downgrade_warn branch from 3708517 to 46ec689 Compare January 31, 2024 21:28

catmsred requested a review from lucasmoura January 31, 2024 21:28

lucasmoura approved these changes Feb 1, 2024

View reviewed changes

feat: warn/confirm with user if enabling fips downgrades the kernel

71fd8e6

This change adds a new message prompting the user to confirm if enabling fips downgrades the kernel.

catmsred force-pushed the feature/fips_downgrade_warn branch from 46ec689 to 71fd8e6 Compare February 1, 2024 13:49

orndorffgrant merged commit 39991a1 into next-v32 Feb 1, 2024
23 of 25 checks passed

orndorffgrant deleted the feature/fips_downgrade_warn branch February 1, 2024 18:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: warn/confirm with user if enabling fips downgrades the kernel #2890

feat: warn/confirm with user if enabling fips downgrades the kernel #2890

catmsred commented Dec 12, 2023 •

edited

Loading

github-actions bot commented Dec 12, 2023 •

edited by catmsred

Loading

github-actions bot commented Dec 12, 2023

catmsred commented Jan 25, 2024

lucasmoura Jan 26, 2024

catmsred Jan 26, 2024

lucasmoura Jan 29, 2024

lucasmoura Jan 26, 2024

catmsred Jan 26, 2024

lucasmoura Jan 29, 2024

catmsred Jan 30, 2024

renanrodrigo left a comment

lucasmoura left a comment

lucasmoura Jan 31, 2024 •

edited

Loading

lucasmoura left a comment

lucasmoura Feb 1, 2024

catmsred Feb 1, 2024

orndorffgrant commented Feb 1, 2024

		@@ -154,6 +155,56 @@ def conditional_packages(self):

		return FIPS_CONDITIONAL_PACKAGES.get(series, [])

		def prompt_if_kernel_downgrade(

feat: warn/confirm with user if enabling fips downgrades the kernel #2890

feat: warn/confirm with user if enabling fips downgrades the kernel #2890

Conversation

catmsred commented Dec 12, 2023 • edited Loading

Why is this needed?

Test Steps

Checklist

Does this PR require extra reviews?

github-actions bot commented Dec 12, 2023 • edited by catmsred Loading

github-actions bot commented Dec 12, 2023

catmsred commented Jan 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

renanrodrigo left a comment

Choose a reason for hiding this comment

lucasmoura left a comment

Choose a reason for hiding this comment

lucasmoura Jan 31, 2024 • edited Loading

Choose a reason for hiding this comment

lucasmoura left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

orndorffgrant commented Feb 1, 2024

catmsred commented Dec 12, 2023 •

edited

Loading

github-actions bot commented Dec 12, 2023 •

edited by catmsred

Loading

lucasmoura Jan 31, 2024 •

edited

Loading