Support for FIPS 140-3 #3324

narrieta · 2025-02-27T03:47:54Z

When fetching certificates from WireServer, the Agent uses DES_EDE3_CBC. The PFX it receives has a MAC computed using PKCS12KDF. Both are deprecated on FIPS 140-3.

This PR switches to AES128_CBC for communication with the WireServer (a subsequent PR will change it to AES256_CBC) and skips MAC verification when it is not needed.

The changes also include some minor cleanup to remove data structures that are not used.

Fixes #2600

narrieta · 2025-02-27T03:48:59Z

azurelinuxagent/common/protocol/goal_state.py

-        xml_text = self._wire_client.fetch_config(certs_uri, self._wire_client.get_header_for_cert())
-        certs = Certificates(xml_text, self.logger)
-        # Log and save the certificates summary (i.e. the thumbprint but not the certificate itself) to the goal state history
-        for c in certs.summary:


Moved this logging to the Certificates class

narrieta · 2025-02-27T03:51:07Z

azurelinuxagent/common/protocol/goal_state.py

-        local_file = os.path.join(conf.get_lib_dir(), CERTS_FILE_NAME)
-        fileutil.write_file(local_file, xml_text)
+        try:
+            pfx_file = self._download_certificates_pfx(wire_client, uri)


I need to distinguish between failures in the encryption protocol with the WireServer and failures parsing the PFX, so I split those into separate methods. I also moved the creation of the *.prv and *.crt files from the the Certificates.pem to its own method.

narrieta · 2025-02-27T03:52:49Z

azurelinuxagent/common/protocol/goal_state.py

+        xml_file = os.path.join(conf.get_lib_dir(), CERTS_FILE_NAME)
+        pfx_file = os.path.join(conf.get_lib_dir(), PFX_FILE_NAME)
+
+        for cypher in ["AES128_CBC", "DES_EDE3_CBC"]:


The WireServer and OpenSSL on the VM should both support AES128, but I am keeping Triple DES as a fallback. Depending on telemetry, I may remove the fallback on a subsequent release.

narrieta · 2025-02-27T03:54:58Z

azurelinuxagent/common/protocol/goal_state.py

+
+            try:
+                xml_text = wire_client.fetch_config(uri, headers)
+            except Exception as e:


I am catching a generic Exception to collect telemetry that will help me decide what specific errors should be handled with a retry. For the moment, in the worst case, we are doing a retry that won't help, but this is done on a per-goal-state basis, so the impact is minimal.

Same applies for catching a generic shellutil.CommandError in the code below and in _convert_certificates_pfx_to_pem()

narrieta · 2025-02-27T03:57:25Z

azurelinuxagent/common/protocol/goal_state.py

-                        thumbprints[pub] = thumbprint
-                        # Rename crt with thumbprint as the file name
-                        crt = "{0}.crt".format(thumbprint)
-                        v1_cert_list.append({


this certificate list was not being used, and most of the properties in each item were not even populated

narrieta · 2025-02-27T03:59:57Z

azurelinuxagent/ga/update.py

@@ -85,6 +85,7 @@
    "*.p7m",
    "*.pem",
    "*.prv",
+    "Certificates.xml",


The permissions for those files should be set at creation time. For now, just adding a missing file, will set the permissions on creation on a separate PR.

narrieta · 2025-02-27T04:01:04Z

tests/common/protocol/test_hostplugin.py

@@ -365,8 +365,7 @@ def test_fallback_channel_failure(self, patch_put, patch_upload, _):
            # ensure host plugin is not set as default
            self.assertFalse(wire.HostPluginProtocol.is_default_channel)

-    @patch("azurelinuxagent.common.event.add_event")


I reduced the scope of this mock to the call to upload_status_blob(), otherwise it ends up capturing extra telemetry

narrieta · 2025-02-27T04:01:21Z

tests/common/protocol/test_wire.py

@@ -497,12 +497,6 @@ def test_report_event_http_req_should_do_max_retries_on_throttling_error(self, m
            client.report_event(self._get_telemetry_events_generator(event_list), flush=True)
            self.assertEqual(mock_http_request.call_count, 3)

-    def test_get_header_for_cert_should_use_triple_des(self, *_):


not applicable anymore

azurelinuxagent/common/protocol/wire.py

azurelinuxagent/common/utils/cryptutil.py

maddieford · 2025-03-03T17:31:52Z

azurelinuxagent/common/event.py

+
+    def info(self, op, fmt, *args):
+        self._logger.info(fmt, *args)
+        add_event(op=op, message=fmt.format(*args), is_success=True)


should log_event=False for info events?

log_event is used only when is_success == False:

def add_event(self, name, op=WALAEventOperation.Unknown, is_success=True, duration=0, version=str(CURRENT_VERSION), message="", log_event=True, flush=False): if (not is_success) and log_event: _log_event(name, op, message, duration, is_success=is_success)

azurelinuxagent/common/protocol/goal_state.py

maddieford · 2025-03-04T16:52:35Z

azurelinuxagent/common/utils/cryptutil.py

+    def decrypt_certificates_p7m(self, p7m_file, trans_prv_file, trans_cert_file, pfx_file):
+        umask = None
+        try:
+            umask = os.umask(0o077)


we do this because pfx file should have read and write access for owner only?

yes, I had to do it this way because I could not find an API to create a file with a specific mode that is backwards compatible all the way to 2.6

maddieford · 2025-03-04T17:19:41Z

azurelinuxagent/common/protocol/goal_state.py

-            return
+            try:
+                pem_file = self._convert_certificates_pfx_to_pem(pfx_file)
+            finally:


Should we add a finally block to line 529 and do the pfx removal there if it exists? It seems possible for umask() to raise Exception when we reset the file mode creation in the finally block of decrypt_certificates_p7m?

Good point, thanks. I think it is the responsibility of _download_certificates_pfx to cleanup when it raises. I'll handle this case there

maddieford · 2025-03-04T17:26:24Z

azurelinuxagent/common/protocol/goal_state.py

-                        buf = []
-                        index += 1
+
+        cryptutil = CryptUtil(conf.get_openssl_cmd())


use self._crypt_util instead?

narrieta@microsoft added 2 commits February 25, 2025 09:51

Support for FIPS 140-3

fec46ca

Add unit tests

b1bf35d

narrieta requested review from ZhidongPeng, nagworld9 and maddieford as code owners February 27, 2025 03:47

narrieta commented Feb 27, 2025

View reviewed changes

nagworld9 reviewed Mar 4, 2025

View reviewed changes

azurelinuxagent/common/protocol/wire.py Show resolved Hide resolved

azurelinuxagent/common/utils/cryptutil.py Show resolved Hide resolved

maddieford reviewed Mar 4, 2025

View reviewed changes

narrieta@microsoft and others added 2 commits March 4, 2025 12:23

review feedback

a3abb6c

Merge branch 'develop' into fips

fa70c5b

maddieford approved these changes Mar 4, 2025

View reviewed changes

nagworld9 approved these changes Mar 4, 2025

View reviewed changes

Merge branch 'develop' into fips

f6ec0a2

narrieta merged commit 4ad9ba8 into Azure:develop Mar 4, 2025
11 checks passed

narrieta deleted the fips branch March 4, 2025 20:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for FIPS 140-3 #3324

Support for FIPS 140-3 #3324

narrieta commented Feb 27, 2025 •

edited

Loading

narrieta Feb 27, 2025

narrieta Feb 27, 2025

narrieta Feb 27, 2025 •

edited

Loading

narrieta Feb 27, 2025 •

edited

Loading

narrieta Feb 27, 2025

narrieta Feb 27, 2025

narrieta Feb 27, 2025

narrieta Feb 27, 2025

maddieford Mar 3, 2025

narrieta Mar 4, 2025

maddieford Mar 4, 2025

narrieta Mar 4, 2025

maddieford Mar 4, 2025

narrieta Mar 4, 2025

maddieford Mar 4, 2025

Support for FIPS 140-3 #3324

Support for FIPS 140-3 #3324

Conversation

narrieta commented Feb 27, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

narrieta Feb 27, 2025 • edited Loading

Choose a reason for hiding this comment

narrieta Feb 27, 2025 • edited Loading

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

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

narrieta commented Feb 27, 2025 •

edited

Loading

narrieta Feb 27, 2025 •

edited

Loading

narrieta Feb 27, 2025 •

edited

Loading