Repository: Azure/WALinuxAgent
Pull request #3190 (2024-08-24 03:24:27)

Fix JIT for FIPS 140-3
Extensions with protected settings and JIT requests are both broken under FIPS 140-3.

The fix for extensions requires fixes on other Azure components, including CRP, Fabric and the Extensions themselves. The corresponding fixes in the Agent will be addressed along with those.

Fixing JIT requests is a lot simpler, though, since it only involves changing the cipher user for RemoteAccess requests. This PR does that. The PR also includes a fix in protocol detection caused by the mentioned issues in Extensions (more details are given in the code).

nagworld9: changes LGTM, I don't know how to validate JIT flow though
nagworld9: Discussed offline, changes looks good

Patch:
diff --git a/azurelinuxagent/common/protocol/goal_state.py b/azurelinuxagent/common/protocol/goal_state.py
index 2eb89c1e..607710e3 100644
--- a/azurelinuxagent/common/protocol/goal_state.py
+++ b/azurelinuxagent/common/protocol/goal_state.py
@@ -212,7 +212,7 @@ class GoalState(object):
         except GoalStateInconsistentError as e:
             message = "Detected an inconsistency in the goal state: {0}".format(ustr(e))
             self.logger.warn(message)
-            add_event(op=WALAEventOperation.GoalState, is_success=False, message=message)
+            add_event(op=WALAEventOperation.GoalState, is_success=False, log_event=False, message=message)
 
             self._update(force_update=True)
 
@@ -503,7 +503,7 @@ class GoalState(object):
             if GoalStateProperties.RemoteAccessInfo & self._goal_state_properties:
                 remote_access_uri = findtext(container, "RemoteAccessInfo")
                 if remote_access_uri is not None:
-                    xml_text = self._wire_client.fetch_config(remote_access_uri, self._wire_client.get_header_for_cert())
+                    xml_text = self._wire_client.fetch_config(remote_access_uri, self._wire_client.get_header_for_remote_access())
                     remote_access = RemoteAccess(xml_text)
                     if self._save_to_history:
                         self._history.save_remote_access(xml_text)
diff --git a/azurelinuxagent/common/protocol/wire.py b/azurelinuxagent/common/protocol/wire.py
index 6ec5b1e2..751dd2af 100644
--- a/azurelinuxagent/common/protocol/wire.py
+++ b/azurelinuxagent/common/protocol/wire.py
@@ -38,7 +38,7 @@ from azurelinuxagent.common.exception import ProtocolNotFoundError, \
     ResourceGoneError, ExtensionDownloadError, InvalidContainerError, ProtocolError, HttpError, ExtensionErrorCodes
 from azurelinuxagent.common.future import httpclient, bytebuffer, ustr
 from azurelinuxagent.common.protocol.goal_state import GoalState, TRANSPORT_CERT_FILE_NAME, TRANSPORT_PRV_FILE_NAME, \
-    GoalStateProperties
+    GoalStateProperties, GoalStateInconsistentError
 from azurelinuxagent.common.protocol.hostplugin import HostPluginProtocol
 from azurelinuxagent.common.protocol.restapi import DataContract, ProvisionStatus, VMInfo, VMStatus
 from azurelinuxagent.common.telemetryevent import GuestAgentExtensionEventsSchema
@@ -86,7 +86,22 @@ class WireProtocol(DataContract):
         # Initialize the goal state, including all the inner properties
         if init_goal_state:
             logger.info('Initializing goal state during protocol detection')
-            self.client.reset_goal_state(save_to_history=save_to_history)
+            #
+            # TODO: Currently protocol detection retrieves the entire goal state. This is not needed; in particular, retrieving the Extensions goal state
+            #       is not needed. However, the goal state is cached in self.client._goal_state and other components, including the Extension Handler,
+            #       depend on this cached value. This has been a long-standing issue that causes multiple problems. Before removing the cached goal state,
+            #       though, a careful review of these dependencies is needed.
+            #
+            #       One of the problems of fetching the full goal state is that issues while retrieving it can block protocol detection and make the
+            #       Agent go into a retry loop that can last 1 full hour. One particular error, GoalStateInconsistentError, can arise if the certificates
+            #       needed by extensions are missing from the goal state; for example, if a FastTrack goal state is out of sync with the corresponding
+            #       Fabric goal state that contains the certificates, or if decryption of the certificates fais (and hence, the certificate list is
+            #       empty). The try/except below handles only this one particular problem.
+            #
+            try:
+                self.client.reset_goal_state(save_to_history=save_to_history)
+            except GoalStateInconsistentError as error:
+                logger.warn("{0}", ustr(error))
 
     def update_host_plugin_from_goal_state(self):
         self.client.update_host_plugin_from_goal_state()
@@ -1126,6 +1141,12 @@ class WireClient(object):
         }
 
     def get_header_for_cert(self):
+        return self._get_header_for_encrypted_request("DES_EDE3_CBC")
+
+    def get_header_for_remote_access(self):
+        return self._get_header_for_encrypted_request("AES128_CBC")
+
+    def _get_header_for_encrypted_request(self, cypher):
         trans_cert_file = os.path.join(conf.get_lib_dir(), TRANSPORT_CERT_FILE_NAME)
         try:
             content = fileutil.read_file(trans_cert_file)
@@ -1136,7 +1157,7 @@ class WireClient(object):
         return {
             "x-ms-agent-name": "WALinuxAgent",
             "x-ms-version": PROTOCOL_VERSION,
-            "x-ms-cipher-name": "DES_EDE3_CBC",
+            "x-ms-cipher-name": cypher,
             "x-ms-guest-agent-public-x509-cert": cert
         }
 


Test patch:
diff --git a/tests/common/protocol/test_wire.py b/tests/common/protocol/test_wire.py
index 8cf0c7d4..2fbd817f 100644
--- a/tests/common/protocol/test_wire.py
+++ b/tests/common/protocol/test_wire.py
@@ -479,6 +479,28 @@ class TestWireProtocol(AgentTestCase, HttpRequestPredicates):
 
         self.assertEqual(patch_send_event.call_count, 0)
 
+    def test_get_header_for_cert_should_use_triple_des(self, *_):
+        with mock_wire_protocol(wire_protocol_data.DATA_FILE) as protocol:
+            headers = protocol.client.get_header_for_cert()
+            self.assertIn("x-ms-cipher-name", headers)
+            self.assertEqual(headers["x-ms-cipher-name"], "DES_EDE3_CBC", "Unexpected x-ms-cipher-name")
+
+    def test_get_header_for_remote_access_should_use_aes128(self, *_):
+        with mock_wire_protocol(wire_protocol_data.DATA_FILE) as protocol:
+            headers = protocol.client.get_header_for_remote_access()
+            self.assertIn("x-ms-cipher-name", headers)
+            self.assertEqual(headers["x-ms-cipher-name"], "AES128_CBC", "Unexpected x-ms-cipher-name")
+
+    def test_detect_should_handle_inconsistent_goal_state_errors(self, *_):
+        data_file = wire_protocol_data.DATA_FILE_VM_SETTINGS  # Certificates are checked only on FastTrack goal states
+        data_file['certs'] = "wire/certs-2.xml"  # Change the certificates to force a GoalStateInconsistentError
+        with mock_wire_protocol(data_file, detect_protocol=False) as protocol:
+            with patch("azurelinuxagent.common.logger.warn") as mock_warn:
+                protocol.detect()
+                self.assertTrue(
+                    any(len(args) == 2 and  args[1].startswith("[GoalStateInconsistentError]") for args, _ in mock_warn.call_args_list),
+                    "Did not find any warnings about an GoalStateInconsistentError: {0}".format(mock_warn.call_args_list))
+
 
 class TestWireClient(HttpRequestPredicates, AgentTestCase):
     def test_get_ext_conf_without_extensions_should_retrieve_vmagent_manifests_info(self, *args):  # pylint: disable=unused-argument
diff --git a/tests/lib/mock_wire_protocol.py b/tests/lib/mock_wire_protocol.py
index 78cbc59e..2cf2b10e 100644
--- a/tests/lib/mock_wire_protocol.py
+++ b/tests/lib/mock_wire_protocol.py
@@ -22,7 +22,7 @@ from tests.lib import wire_protocol_data
 
 
 @contextlib.contextmanager
-def mock_wire_protocol(mock_wire_data_file, http_get_handler=None, http_post_handler=None, http_put_handler=None, do_not_mock=lambda method, url: False, fail_on_unknown_request=True, save_to_history=False):
+def mock_wire_protocol(mock_wire_data_file, http_get_handler=None, http_post_handler=None, http_put_handler=None, do_not_mock=lambda method, url: False, fail_on_unknown_request=True, save_to_history=False, detect_protocol=True):
     """
     Creates a WireProtocol object that handles requests to the WireServer, the Host GA Plugin, and some requests to storage (requests that provide mock data
     in wire_protocol_data.py).
@@ -149,7 +149,8 @@ def mock_wire_protocol(mock_wire_data_file, http_get_handler=None, http_post_han
     # go do it
     try:
         protocol.start()
-        protocol.detect(save_to_history=save_to_history)
+        if detect_protocol:
+            protocol.detect(save_to_history=save_to_history)
         yield protocol
     finally:
         protocol.stop()