Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions azurelinuxagent/common/protocol/goal_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class GoalStateProperties(object):


class GoalState(object):
def __init__(self, wire_client, goal_state_properties=GoalStateProperties.All, silent=False, save_to_history=False):
def __init__(self, wire_client, goal_state_properties=GoalStateProperties.All, silent=False, save_to_history=False, ignore_certificate_download_errors=True):
"""
Fetches the goal state using the given wire client.

Expand All @@ -78,6 +78,7 @@ def __init__(self, wire_client, goal_state_properties=GoalStateProperties.All, s
self._wire_client = wire_client
self._history = None
self._save_to_history = save_to_history
self._ignore_certificate_download_errors = ignore_certificate_download_errors
self._extensions_goal_state = None # populated from vmSettings or extensionsConfig
self._goal_state_properties = goal_state_properties
self.logger = logger.Logger(logger.DEFAULT_LOGGER)
Expand Down Expand Up @@ -303,7 +304,7 @@ def update(self, force_update=False, silent=False):
self._check_and_download_missing_certs_on_disk()

def _download_certificates(self, certs_uri):
certs = Certificates(self._wire_client, certs_uri, self.logger)
certs = Certificates(self._wire_client, certs_uri, self.logger, ignore_download_errors=self._ignore_certificate_download_errors)
# Save the certificates summary (i.e. the thumbprints but not the certificates themselves) to the goal state history
if self._save_to_history:
self._history.save_certificates(json.dumps(certs.summary))
Expand Down Expand Up @@ -526,8 +527,9 @@ def __init__(self, xml_text):


class Certificates(LogEvent):
def __init__(self, wire_client, uri, logger_):
def __init__(self, wire_client, uri, logger_, ignore_download_errors=True):
super(Certificates, self).__init__(logger_)
self._ignore_download_errors = ignore_download_errors
self.summary = []
self._crypt_util = CryptUtil(conf.get_openssl_cmd())

Expand All @@ -543,9 +545,12 @@ def __init__(self, wire_client, uri, logger_):
self._convert_certificates_pfx_to_pem(pfx_file, pem_file)
except Exception as e:
# A failure to download the certificates won't necessarily produce an error. Certificates do not change often and they may have
# already been saved to disk on a previous goal state. We simply report the error and continue processing the goal_state; later on,
# already been saved to disk on a previous goal state. Re-raise the exception only if explicitly requested via the ignore_download_errors
# parameter, otherwise simply report the error and continue processing the goal_state; later on,
# before extensions are processed, the Agent checks whether the required certificates are already on disk and refreshes the goal
# state if they are not.
# state if they are not
Comment on lines 547 to +551

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

won't fix; use of a boolean should be easy to understand by future maintaners

if not self._ignore_download_errors:
raise
self.error(WALAEventOperation.GoalStateCertificates, "Error fetching the goal state certificates: {0}", ustr(e))
create_empty_pem_file = True
return
Expand Down Expand Up @@ -613,7 +618,7 @@ def _try_download_certificates_pfx(self, wire_client, uri, pfx_file):

return True

raise Exception("Cannot download certificates using any of the supported cyphers")
raise ProtocolError("Cannot download certificates using any of the supported ciphers")

@staticmethod
def _create_p7m_file(data):
Expand Down
28 changes: 27 additions & 1 deletion azurelinuxagent/pa/provision/default.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,9 @@
from azurelinuxagent.common.exception import ProvisionError, ProtocolError, \
OSUtilError
from azurelinuxagent.common.osutil import get_osutil
from azurelinuxagent.common.protocol.goal_state import GoalState, GoalStateProperties
from azurelinuxagent.common.protocol.restapi import ProvisionStatus
from azurelinuxagent.common.protocol.util import get_protocol_util
from azurelinuxagent.common.protocol.util import get_protocol_util, MAX_RETRY, PROBE_INTERVAL
from azurelinuxagent.common.version import AGENT_NAME
from azurelinuxagent.pa.provision.cloudinitdetect import cloud_init_is_enabled

Expand Down Expand Up @@ -240,9 +241,34 @@ def config_user_account(self, ovfenv):
logger.info("Configure sshd")
self.osutil.conf_sshd(ovfenv.disable_ssh_password_auth)

self._download_ssh_keys_if_needed(ovfenv)
self.deploy_ssh_pubkeys(ovfenv)
self.deploy_ssh_keypairs(ovfenv)

def _download_ssh_keys_if_needed(self, ovfenv):
#

@narrieta narrieta Apr 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the reason of this check, see the implementation of self.deploy_ssh_pubkeys() and self.deploy_ssh_keypairs(). In summary, the former will look for /var/lib/waagent/*.crt when ovfenv.xml doesn't have a value for the key, and the latter will look for /var/lib/waagent/*.prv when ovfenv.xml contains any key pairs.

For the format of ovfenv.xml, see the unit tests.

# We need to download the Certificates package from the Wireserver if any public key in ovfenv.xml has only a thumbprint (i.e. no value for the key) or if any key pairs need to be installed
#
download_certificates = any(value is None and thumbprint is not None for _, thumbprint, value in ovfenv.ssh_pubkeys) or len(ovfenv.ssh_keypairs) > 0
if not download_certificates:
return

#

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Originally, the PA would retrieve the goal state as part of protocol detection, which retries 360 times, with a 10 second period. If that fails, the process exits and is restarted by the system, which goes again into the 360 retries, etc.

I feel just a few tries are enough, let me know if you think otherwise; thanks.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked that cloud-init also retries for a long period of time, so I decided to use the same strategy as protocol detection

# Try to download the certificates. The retry logic mimics ProtocolUtil._detect_protocol(), which can also download the certificates when detecting the WireServer endpoint.
# In this case, though we continue execution if all attempts fail and the code that deploys the SSH keys will report that as a provisioning error.
#
for retry in range(0, MAX_RETRY):
try:
protocol = self.protocol_util.get_protocol(init_goal_state=False)
_ = GoalState(protocol.client, goal_state_properties=GoalStateProperties.Certificates, ignore_certificate_download_errors=False)
return
Comment thread
narrieta marked this conversation as resolved.
except ProtocolError as e:
if retry < MAX_RETRY - 1:
logger.info("Unable to download certificates; will retry after a short delay: {0}", ustr(e))
time.sleep(PROBE_INTERVAL)
else:
logger.error("Unable to download certificates: {0}", ustr(e))

def save_customdata(self, ovfenv):
customdata = ovfenv.customdata
if customdata is None:
Expand Down
6 changes: 0 additions & 6 deletions tests/data/ovf-env-2.xml
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,6 @@
<Value>ssh-rsa AAAANOTAREALKEY== foo@bar.local</Value>
</PublicKey>
</PublicKeys>
<KeyPairs>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The key pairs in the unit tests are not used, and with the new code they would trigger the Certificates download. To fix this, quite a few tests need to be rewritten to mock that download. Since the keys are not used by the tests, I decided to remove them instead.

<KeyPair>
<Fingerprint>EB0C0AB4B2D5FC35F2F0658D19F44C8283E2DD62</Fingerprint>
<Path>$HOME/UserName/.ssh/id_rsa</Path>
</KeyPair>
</KeyPairs>
</SSH>
<CustomData>CustomData</CustomData>
</LinuxProvisioningConfigurationSet>
Expand Down
6 changes: 0 additions & 6 deletions tests/data/ovf-env-4.xml
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,6 @@
<Value>ssh-rsa AAAANOTAREALKEY== foo@bar.local</Value>
</PublicKey>
</PublicKeys>
<KeyPairs>
<KeyPair>
<Fingerprint>EB0C0AB4B2D5FC35F2F0658D19F44C8283E2DD62</Fingerprint>
<Path>$HOME/UserName/.ssh/id_rsa</Path>
</KeyPair>
</KeyPairs>
</SSH>
<CustomData>CustomData</CustomData>
</LinuxProvisioningConfigurationSet>
Expand Down
6 changes: 0 additions & 6 deletions tests/data/ovf-env.xml
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,6 @@
<Value>ssh-rsa AAAANOTAREALKEY== foo@bar.local</Value>
</PublicKey>
</PublicKeys>
<KeyPairs>
<KeyPair>
<Fingerprint>EB0C0AB4B2D5FC35F2F0658D19F44C8283E2DD62</Fingerprint>
<Path>$HOME/UserName/.ssh/id_rsa</Path>
</KeyPair>
</KeyPairs>
</SSH>
<CustomData>CustomData</CustomData>
</LinuxProvisioningConfigurationSet>
Expand Down
33 changes: 33 additions & 0 deletions tests/data/ovf-env_key_pair.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?xml version="1.0" encoding="utf-8"?>
<Environment xmlns="http://schemas.dmtf.org/ovf/environment/1" xmlns:oe="http://schemas.dmtf.org/ovf/environment/1" xmlns:wa="http://schemas.microsoft.com/windowsazure" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
<wa:ProvisioningSection>
<wa:Version>1.0</wa:Version>
<LinuxProvisioningConfigurationSet xmlns="http://schemas.microsoft.com/windowsazure" xmlns:i="http://www.w3.org/2001/XMLSchema-instance">
<ConfigurationSetType>LinuxProvisioningConfiguration</ConfigurationSetType>
<HostName>HostName</HostName>
<UserName>UserName</UserName>
<UserPassword>UserPassword</UserPassword>
<DisableSshPasswordAuthentication>false</DisableSshPasswordAuthentication>
<SSH>
<KeyPairs>
<KeyPair>
<Fingerprint>8979F1AC8C4215827BF3B5A403E6137B504D02A4</Fingerprint>
<Path>$HOME/UserName/.ssh/id_rsa</Path>
</KeyPair>
</KeyPairs>
</SSH>
<CustomData>CustomData</CustomData>
</LinuxProvisioningConfigurationSet>
</wa:ProvisioningSection>
<wa:PlatformSettingsSection>
<wa:Version>1.0</wa:Version>
<wa:PlatformSettings>
<wa:KmsServerHostname>kms.core.windows.net</wa:KmsServerHostname>
<wa:ProvisionGuestAgent>false</wa:ProvisionGuestAgent>
<wa:GuestAgentPackageName xsi:nil="true"/>
<wa:RetainWindowsPEPassInUnattend>true</wa:RetainWindowsPEPassInUnattend>
<wa:RetainOfflineServicingPassInUnattend>true</wa:RetainOfflineServicingPassInUnattend>
<wa:PreprovisionedVm>false</wa:PreprovisionedVm>
</wa:PlatformSettings>
</wa:PlatformSettingsSection>
</Environment>
34 changes: 34 additions & 0 deletions tests/data/ovf-env_public_key.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?xml version="1.0" encoding="utf-8"?>
<Environment xmlns="http://schemas.dmtf.org/ovf/environment/1" xmlns:oe="http://schemas.dmtf.org/ovf/environment/1" xmlns:wa="http://schemas.microsoft.com/windowsazure" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
<wa:ProvisioningSection>
<wa:Version>1.0</wa:Version>
<LinuxProvisioningConfigurationSet xmlns="http://schemas.microsoft.com/windowsazure" xmlns:i="http://www.w3.org/2001/XMLSchema-instance">
<ConfigurationSetType>LinuxProvisioningConfiguration</ConfigurationSetType>
<HostName>HostName</HostName>
<UserName>UserName</UserName>
<UserPassword>UserPassword</UserPassword>
<DisableSshPasswordAuthentication>false</DisableSshPasswordAuthentication>
<SSH>
<PublicKeys>
<PublicKey>
<Fingerprint>EB0C0AB4B2D5FC35F2F0658D19F44C8283E2DD62</Fingerprint>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it expected that the customer should only provide one of fingerprint and value, or is it acceptable to provide both?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original sample file from which I copied this one includes both. In practice, I believe only 1 would be populated.

Now, the logic in OSUtil.deploy_ssh_pubkey() prioritizes the value over the thumbprint, so in terms of the unit test that uses this new file, having both is a good test case.

<Path>$HOME/UserName/.ssh/authorized_keys</Path>
<Value>ssh-rsa AAAANOTAREALKEY== foo@bar.local</Value>
</PublicKey>
</PublicKeys>
</SSH>
<CustomData>CustomData</CustomData>
</LinuxProvisioningConfigurationSet>
</wa:ProvisioningSection>
<wa:PlatformSettingsSection>
<wa:Version>1.0</wa:Version>
<wa:PlatformSettings>
<wa:KmsServerHostname>kms.core.windows.net</wa:KmsServerHostname>
<wa:ProvisionGuestAgent>false</wa:ProvisionGuestAgent>
<wa:GuestAgentPackageName xsi:nil="true"/>
<wa:RetainWindowsPEPassInUnattend>true</wa:RetainWindowsPEPassInUnattend>
<wa:RetainOfflineServicingPassInUnattend>true</wa:RetainOfflineServicingPassInUnattend>
<wa:PreprovisionedVm>false</wa:PreprovisionedVm>
</wa:PlatformSettings>
</wa:PlatformSettingsSection>
</Environment>
33 changes: 33 additions & 0 deletions tests/data/ovf-env_public_key_no_value.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?xml version="1.0" encoding="utf-8"?>
<Environment xmlns="http://schemas.dmtf.org/ovf/environment/1" xmlns:oe="http://schemas.dmtf.org/ovf/environment/1" xmlns:wa="http://schemas.microsoft.com/windowsazure" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
<wa:ProvisioningSection>
<wa:Version>1.0</wa:Version>
<LinuxProvisioningConfigurationSet xmlns="http://schemas.microsoft.com/windowsazure" xmlns:i="http://www.w3.org/2001/XMLSchema-instance">
<ConfigurationSetType>LinuxProvisioningConfiguration</ConfigurationSetType>
<HostName>HostName</HostName>
<UserName>UserName</UserName>
<UserPassword>UserPassword</UserPassword>
<DisableSshPasswordAuthentication>false</DisableSshPasswordAuthentication>
<SSH>
<PublicKeys>
<PublicKey>
<Fingerprint>8979F1AC8C4215827BF3B5A403E6137B504D02A4</Fingerprint>
<Path>$HOME/UserName/.ssh/authorized_keys</Path>
</PublicKey>
</PublicKeys>
</SSH>
<CustomData>CustomData</CustomData>
</LinuxProvisioningConfigurationSet>
</wa:ProvisioningSection>
<wa:PlatformSettingsSection>
<wa:Version>1.0</wa:Version>
<wa:PlatformSettings>
<wa:KmsServerHostname>kms.core.windows.net</wa:KmsServerHostname>
<wa:ProvisionGuestAgent>false</wa:ProvisionGuestAgent>
<wa:GuestAgentPackageName xsi:nil="true"/>
<wa:RetainWindowsPEPassInUnattend>true</wa:RetainWindowsPEPassInUnattend>
<wa:RetainOfflineServicingPassInUnattend>true</wa:RetainOfflineServicingPassInUnattend>
<wa:PreprovisionedVm>false</wa:PreprovisionedVm>
</wa:PlatformSettings>
</wa:PlatformSettingsSection>
</Environment>
92 changes: 89 additions & 3 deletions tests/pa/test_provision.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,26 @@
#
# Requires Python 2.6+ and Openssl 1.0+
#

import contextlib
import getpass
import os
import re
import unittest

import azurelinuxagent.common.conf as conf
from azurelinuxagent.common.exception import ProvisionError
from azurelinuxagent.common.exception import ProvisionError, OSUtilError
from azurelinuxagent.common.future import ustr
from azurelinuxagent.common.osutil.default import DefaultOSUtil
from azurelinuxagent.common.protocol.util import OVF_FILE_NAME
from azurelinuxagent.common.protocol.ovfenv import OvfEnv
from azurelinuxagent.common.protocol.util import OVF_FILE_NAME, MAX_RETRY
from azurelinuxagent.pa.provision import get_provision_handler
from azurelinuxagent.pa.provision.cloudinit import CloudInitProvisionHandler
from azurelinuxagent.pa.provision.default import ProvisionHandler
from azurelinuxagent.common.utils import fileutil
from tests.lib import wire_protocol_data
from tests.lib.http_request_predicates import HttpRequestPredicates
from tests.lib.tools import AgentTestCase, distros, load_data, MagicMock, Mock, patch
from tests.lib.mock_wire_protocol import mock_wire_protocol


class TestProvision(AgentTestCase):
Expand Down Expand Up @@ -377,6 +383,86 @@ def test_get_provision_handler_config_cloudinit(
self.assertIsInstance(provisioning_handler, CloudInitProvisionHandler, 'Provisioning handler should be cloud-init if agent is set to cloud-init')


@staticmethod
@contextlib.contextmanager
def _create_provision_handler_with_mock_protocol():
handler = ProvisionHandler()
handler.protocol_util = Mock()

with mock_wire_protocol(wire_protocol_data.DATA_FILE, detect_protocol=False) as mock_protocol:
handler.protocol_util.get_protocol = Mock(return_value=mock_protocol)
yield handler, mock_protocol

def test_it_should_not_download_certificates_when_the_public_key_has_a_value(self):
ovfenv = OvfEnv(load_data("ovf-env_public_key.xml"))
with TestProvision._create_provision_handler_with_mock_protocol() as (handler, protocol):
handler._download_ssh_keys_if_needed(ovfenv)
self.assertEqual(0, protocol.mock_wire_data.call_counts['certificates'], "The Certificates package should not have been retrieved")

def test_it_should_download_certificates_when_the_public_key_does_not_have_a_value(self):
ovfenv = OvfEnv(load_data("ovf-env_public_key_no_value.xml"))
with TestProvision._create_provision_handler_with_mock_protocol() as (handler, protocol):
handler._download_ssh_keys_if_needed(ovfenv)
self.assertEqual(1, protocol.mock_wire_data.call_counts['certificates'], "The Certificates package should have been retrieved")
ssh_key_path = os.path.join(conf.get_lib_dir(), '8979F1AC8C4215827BF3B5A403E6137B504D02A4.crt')
self.assertTrue(os.path.exists(ssh_key_path), 'The SSH key was not downloaded. Expected: {0}'.format(ssh_key_path))

def test_it_should_download_certificates_when_key_pairs_need_to_be_deployed(self):
ovfenv = OvfEnv(load_data("ovf-env_key_pair.xml"))
Comment thread
narrieta marked this conversation as resolved.
with TestProvision._create_provision_handler_with_mock_protocol() as (handler, protocol):
handler._download_ssh_keys_if_needed(ovfenv)
self.assertEqual(1, protocol.mock_wire_data.call_counts['certificates'], "The Certificates package should have been retrieved")
ssh_key_path = os.path.join(conf.get_lib_dir(), '8979F1AC8C4215827BF3B5A403E6137B504D02A4.crt')
self.assertTrue(os.path.exists(ssh_key_path), 'The SSH key was not downloaded. Expected: {0}'.format(ssh_key_path))

def test_it_should_retry_downloading_the_certificates(self):
ovfenv = OvfEnv(load_data("ovf-env_public_key_no_value.xml"))

def mock_http_get(url, *_, **__):
if HttpRequestPredicates.is_certificates_request(url):
mock_http_get.call_count += 1
if mock_http_get.call_count <= 10:
return Exception("Mock failure")
return None
mock_http_get.call_count = 0

with TestProvision._create_provision_handler_with_mock_protocol() as (handler, protocol):
protocol.set_http_handlers(http_get_handler=mock_http_get)
with patch('azurelinuxagent.pa.provision.default.PROBE_INTERVAL', 0): # set the delay between retries to 0
handler._download_ssh_keys_if_needed(ovfenv)
self.assertEqual(11, mock_http_get.call_count, "Expected 11 requests for Certificates (10 failed and retried requests, and 1 successful request)")
ssh_key_path = os.path.join(conf.get_lib_dir(), '8979F1AC8C4215827BF3B5A403E6137B504D02A4.crt')
self.assertTrue(os.path.exists(ssh_key_path), 'The SSH key was not downloaded. Expected: {0}'.format(ssh_key_path))

def test_it_should_retry_downloading_the_certificates_the_maximum_number_of_retries(self):
ovfenv = OvfEnv(load_data("ovf-env_public_key_no_value.xml"))

def mock_http_get(url, *_, **__):
if HttpRequestPredicates.is_certificates_request(url):
mock_http_get.call_count += 1
return Exception("Mock failure")
return None
mock_http_get.call_count = 0

with TestProvision._create_provision_handler_with_mock_protocol() as (handler, protocol):
protocol.set_http_handlers(http_get_handler=mock_http_get)
with patch('azurelinuxagent.pa.provision.default.PROBE_INTERVAL', 0): # set the delay between retries to 0
handler._download_ssh_keys_if_needed(ovfenv)
self.assertEqual(2 * MAX_RETRY, mock_http_get.call_count, "Expected maximum number of retries ({0}) to have been attempted".format(MAX_RETRY)) # times 2 since two ciphers are attempted for FIPS support
ssh_key_path = os.path.join(conf.get_lib_dir(), '8979F1AC8C4215827BF3B5A403E6137B504D02A4.crt')
self.assertFalse(os.path.exists(ssh_key_path), 'The SSH key should not have been downloaded, since all requests failed. Got: {0}'.format(ssh_key_path))

def test_deploy_ssh_pubkeys_should_raise_if_no_keys_have_been_downloaded(self):
ovfenv_data = load_data("ovf-env_public_key_no_value.xml")
ovfenv_data = ovfenv_data.replace('<UserName>UserName</UserName>', '<UserName>{0}</UserName>'.format(getpass.getuser()))
ovfenv_data = ovfenv_data.replace('<Path>$HOME/UserName/.ssh/authorized_keys</Path>', '<Path>{0}</Path>'.format(os.path.join(self.tmp_dir, "authorized_keys")))
Comment thread
narrieta marked this conversation as resolved.
ovfenv = OvfEnv(ovfenv_data)
with TestProvision._create_provision_handler_with_mock_protocol() as (handler, _):
with self.assertRaises(OSUtilError) as context:
handler.deploy_ssh_pubkeys(ovfenv)
self.assertIn("Can't find 8979F1AC8C4215827BF3B5A403E6137B504D02A4.crt", ustr(context.exception))


Comment thread
nagworld9 marked this conversation as resolved.
if __name__ == '__main__':
unittest.main()

Loading