-
Notifications
You must be signed in to change notification settings - Fork 395
Implement fallback to WireServer when deploying SSH keys #3603
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a101540
54d581f
d1dd817
42f6b6f
c1e150d
3556e21
031fea4
bbc1df9
b2c7416
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
|
||
|
|
@@ -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): | ||
| # | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
|
||
| # | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
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: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,12 +16,6 @@ | |
| <Value>ssh-rsa AAAANOTAREALKEY== foo@bar.local</Value> | ||
| </PublicKey> | ||
| </PublicKeys> | ||
| <KeyPairs> | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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> | ||
|
|
||
| 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> |
| 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> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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> | ||
| 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> |
There was a problem hiding this comment.
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