Skip to content
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

Add support for EC certificates #2936

Merged
merged 6 commits into from
Oct 5, 2023
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
1 change: 1 addition & 0 deletions azurelinuxagent/common/event.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ class WALAEventOperation:
LogCollection = "LogCollection"
NoExec = "NoExec"
OSInfo = "OSInfo"
OpenSsl = "OpenSsl"
Partition = "Partition"
PersistFirewallRules = "PersistFirewallRules"
PluginSettingsVersionMismatch = "PluginSettingsVersionMismatch"
Expand Down
10 changes: 1 addition & 9 deletions azurelinuxagent/common/protocol/goal_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -579,26 +579,19 @@ def __init__(self, xml_text, my_logger):

# The parsing process use public key to match prv and crt.
buf = []
begin_crt = False # pylint: disable=W0612
Copy link
Member Author

Choose a reason for hiding this comment

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

pylint W0612: Unused Variable

begin_prv = False # pylint: disable=W0612
prvs = {}
thumbprints = {}
index = 0
v1_cert_list = []
with open(pem_file) as pem:
for line in pem.readlines():
buf.append(line)
if re.match(r'[-]+BEGIN.*KEY[-]+', line):
begin_prv = True
elif re.match(r'[-]+BEGIN.*CERTIFICATE[-]+', line):
begin_crt = True
elif re.match(r'[-]+END.*KEY[-]+', line):
if re.match(r'[-]+END.*KEY[-]+', line):
tmp_file = Certificates._write_to_tmp_file(index, 'prv', buf)
pub = cryptutil.get_pubkey_from_prv(tmp_file)
prvs[pub] = tmp_file
buf = []
index += 1
begin_prv = False
elif re.match(r'[-]+END.*CERTIFICATE[-]+', line):
tmp_file = Certificates._write_to_tmp_file(index, 'crt', buf)
pub = cryptutil.get_pubkey_from_crt(tmp_file)
Expand All @@ -613,7 +606,6 @@ def __init__(self, xml_text, my_logger):
os.rename(tmp_file, os.path.join(conf.get_lib_dir(), crt))
buf = []
index += 1
begin_crt = False

# Rename prv key with thumbprint as the file name
for pubkey in prvs:
Expand Down
19 changes: 15 additions & 4 deletions azurelinuxagent/common/utils/cryptutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,21 @@ def gen_transport_cert(self, prv_file, crt_file):
def get_pubkey_from_prv(self, file_name):
if not os.path.exists(file_name):
raise IOError(errno.ENOENT, "File not found", file_name)
else:
cmd = [self.openssl_cmd, "rsa", "-in", file_name, "-pubout"]
pub = shellutil.run_command(cmd, log_error=True)
return pub

# OpenSSL's pkey command may not be available on older versions so try 'rsa' first.
try:
command = [self.openssl_cmd, "rsa", "-in", file_name, "-pubout"]
return shellutil.run_command(command, log_error=False)
except shellutil.CommandError as error:
if not ("Not an RSA key" in error.stderr or "expecting an rsa key" in error.stderr):
logger.error(
"Command: [{0}], return code: [{1}], stdout: [{2}] stderr: [{3}]",
" ".join(command),
error.returncode,
error.stdout,
error.stderr)
raise
return shellutil.run_command([self.openssl_cmd, "pkey", "-in", file_name, "-pubout"], log_error=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

If pkey is may not be available on older versions of openssl, should we handle any errors here?

Copy link
Member Author

Choose a reason for hiding this comment

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

how should we handle them? we have no way to extract the certs so at this point i'm just letting the goal state fail by letting the exception bubble up

Copy link
Contributor

Choose a reason for hiding this comment

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

If we catch and log the error, could some extensions in the GS which don't require the cert still provision successfully? And then only the extensions which require the cert fail

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't know what extensions require those certs. The tenant cert and ssh keys for provisioning always use RSA (at least currently). EC would be used on KeyVault certs, which are typically used with CSE or something like that, and the scripts may not be resilient to a failure not finding the cert (and may have already done partial work). At this point I would fail the goal state rather than swallowing the error. Once we start getting telemetry about pkey we can revisit this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense!


def get_pubkey_from_crt(self, file_name):
if not os.path.exists(file_name):
Expand Down
27 changes: 26 additions & 1 deletion azurelinuxagent/ga/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,8 @@ def run(self, debug=False):
logger.info("Python: {0}.{1}.{2}", PY_VERSION_MAJOR, PY_VERSION_MINOR, PY_VERSION_MICRO)

os_info_msg = u"Distro: {dist_name}-{dist_ver}; "\
u"OSUtil: {util_name}; AgentService: {service_name}; "\
u"OSUtil: {util_name}; "\
u"AgentService: {service_name}; "\
u"Python: {py_major}.{py_minor}.{py_micro}; "\
u"systemd: {systemd}; "\
u"LISDrivers: {lis_ver}; "\
Expand All @@ -343,6 +344,7 @@ def run(self, debug=False):

# Send telemetry for the OS-specific info.
add_event(AGENT_NAME, op=WALAEventOperation.OSInfo, message=os_info_msg)
self._log_openssl_info()

#
# Perform initialization tasks
Expand Down Expand Up @@ -409,6 +411,29 @@ def run(self, debug=False):
self._shutdown()
sys.exit(0)

@staticmethod
def _log_openssl_info():
try:
version = shellutil.run_command(["openssl", "version"])
message = "OpenSSL version: {0}".format(version)
logger.info(message)
add_event(op=WALAEventOperation.OpenSsl, message=message, is_success=True)
except Exception as e:
message = "Failed to get OpenSSL version: {0}".format(e)
logger.info(message)
add_event(op=WALAEventOperation.OpenSsl, message=message, is_success=False, log_event=False)
#
# Collect telemetry about the 'pkey' command. CryptUtil get_pubkey_from_prv() uses the 'pkey' command only as a fallback after trying 'rsa'.
# 'pkey' also works for RSA keys, but it may not be available on older versions of OpenSSL. Check telemetry after a few releases and if there
# are no versions of OpenSSL that do not support 'pkey' consider removing the use of 'rsa' altogether.
#
try:
shellutil.run_command(["openssl", "help", "pkey"])
except Exception as e:
message = "OpenSSL does not support the pkey command: {0}".format(e)
logger.info(message)
add_event(op=WALAEventOperation.OpenSsl, message=message, is_success=False, log_event=False)

def _initialize_goal_state(self, protocol):
#
# Block until we can fetch the first goal state (self._try_update_goal_state() does its own logging and error handling).
Expand Down
13 changes: 13 additions & 0 deletions tests/common/utils/test_crypt_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,19 @@ def test_get_pubkey_from_crt(self):
with open(expected_pub_key) as fh:
self.assertEqual(fh.read(), crypto.get_pubkey_from_prv(prv_key))

def test_get_pubkey_from_prv(self):
crypto = CryptUtil(conf.get_openssl_cmd())

def do_test(prv_key, expected_pub_key):
prv_key = os.path.join(data_dir, "wire", prv_key)
expected_pub_key = os.path.join(data_dir, "wire", expected_pub_key)

with open(expected_pub_key) as fh:
self.assertEqual(fh.read(), crypto.get_pubkey_from_prv(prv_key))

do_test("rsa-key.pem", "rsa-key.pub.pem")
do_test("ec-key.pem", "ec-key.pub.pem")

def test_get_pubkey_from_crt_invalid_file(self):
crypto = CryptUtil(conf.get_openssl_cmd())
prv_key = os.path.join(data_dir, "wire", "trans_prv_does_not_exist")
Expand Down
5 changes: 5 additions & 0 deletions tests/data/wire/ec-key.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
-----BEGIN EC PRIVATE KEY-----
MHcCAQEEIEydYXZkSbZjdKaNEurW6x2W3dEOC5+yDxM/Wkq1m6lUoAoGCCqGSM49
AwEHoUQDQgAE8H1M+73QdzCyIDToTyU7OTMfi9cnIt8B4sz7e127ydNBVWjDwgGV
bKXPNtuQSWNgkfGW8A3tf9S8VcKNFxXaZg==
-----END EC PRIVATE KEY-----
4 changes: 4 additions & 0 deletions tests/data/wire/ec-key.pub.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
-----BEGIN PUBLIC KEY-----
MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE8H1M+73QdzCyIDToTyU7OTMfi9cn
It8B4sz7e127ydNBVWjDwgGVbKXPNtuQSWNgkfGW8A3tf9S8VcKNFxXaZg==
-----END PUBLIC KEY-----
28 changes: 28 additions & 0 deletions tests/data/wire/rsa-key.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
-----BEGIN PRIVATE KEY-----
MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQDe7cwx76yO+OjR
hWHJrKt0L1ih9F/Bctyq7Ddi/v3CitVBvkQUve4k+xeT538mHyeoOuGI3QFs5mLh
i535zbOFaHwfMMQI/CI4ZDtRrQh59XrJSsPytu0fXihsJ81IwNURuNDKwxYR0tKI
KUuUN4YxsDSBeqvP5vjSKT05f90gniscuGvPJ6Zgyynmg56KQtSXKaetbyNzPW/4
QFmadyqsgdR7oZHEYj+1Tl6T9/tAPg/dgO55hT7WVdC8JxXeSiaDyRS1NRMFL0bC
fcnLNsO4tni2WJsfuju9a4GTrWe3NQ3+vsQV5s59MtuOhoObuYNVcETYiEjBVVsf
+shxRxL/AgMBAAECggEAfslt/eSbFoFYIHmkoQe0R5L57LpIj4QdHpTT91igyDkf
ipGEtOtEewHXagYaWXsUmehLBwTy35W0HSTDxyQHetNu7GpWw+lqKPpQhmZL0Nkd
aUg9Y1hISjPJ96E3bq5FQBwFm5wSfDaUCF68HmLpzm6xngY/mzF4yEYuDPq8r+RV
SDhVtrovSImpwLbKmPdn634PqC6bPDgO5htkT/lL/TVkR3Sla3U/YYMu90m7DiAA
46DEblx0yt+zBB+mKR3TU4zIPSFiTWYs/Srsm6nUnNqjf5rvupvXFZt0/eDZat7/
L+/V5HPV0BxGIkCGt0Uv+qZYMGpC3eU+aEbByOr/wQKBgQDy+l4Rvgl0i+XzUPyw
N6UrDDpxBVsZ/w48DrBEBMQqTbZxVDK77E2CeMK/JlYMFYFdIT/c9W0U7eWPqe35
kk9jVsPXc3xeoSiZvqK4CZeHEugE9OtJ4jJL1CfDXMcgPM+iSSj/QOJc5v7891QH
3gMOvmVk3Kk/I2MyBAEE6p6WHwKBgQDq4FvO77tsIZRkgmp3gPg4iImcTgwrgDxz
aHqlSVc98o4jzWsUShbZTwRgfcZm+kD3eas+gkux8CevYhwjafWiukrnwu3xvUaO
AKmgXU7ud/kS9bK/AT6ZpJsfoZzM/CQsConFbz0eXVb/tmipCBpyzi2yskLdk6SP
pEZYISknIQKBgHwE9PzjXdoiChYekUu0q1aEoFPN4wkq2W4oJSoisKnTDrtbuaWX
4Jwm3WhJvgPe+i+55+n1T18uakzg9Hm9h03yHHYdGS8H3TxURKPhKXmlWc4l4O7O
SNPRjxY1heHbiDOSWh2nVaMLuL0P1NFLLY5Z+lD4HF8AxgHib06+HoILAoGBALvg
oa+jNhGlvrSzWYSkJmnaVfEwwS1e03whe9GRG/cSeb6Lx3agWSyUt1ST50tiLOuI
aIGE6hW4m5X/7bAqRvFXASnoVDtFgxV91DHR0ZyRXSxcWxHMZg2yjN89gFa77hdI
irHibEpIsZm0iH2FXNqusAE79J6XRlAcQKSoSenhAoGARAP9q1WaftXdK4X7L1Ut
wnWJSVYMx6AsEo58SsJgNGqpbCl/vZMCwnSo6pdgO4xInu2tld3TKdPWZLoRCGCo
PDYVM1GXj5SS8QPmq+h/6fxS65Gl0h0oHUcKXoPD+AxHn2MWWqWzxMdRuthUQATE
MT+l5wgZPiEuiceY3Bp1hYk=
-----END PRIVATE KEY-----
9 changes: 9 additions & 0 deletions tests/data/wire/rsa-key.pub.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
-----BEGIN PUBLIC KEY-----
MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA3u3MMe+sjvjo0YVhyayr
dC9YofRfwXLcquw3Yv79worVQb5EFL3uJPsXk+d/Jh8nqDrhiN0BbOZi4Yud+c2z
hWh8HzDECPwiOGQ7Ua0IefV6yUrD8rbtH14obCfNSMDVEbjQysMWEdLSiClLlDeG
MbA0gXqrz+b40ik9OX/dIJ4rHLhrzyemYMsp5oOeikLUlymnrW8jcz1v+EBZmncq
rIHUe6GRxGI/tU5ek/f7QD4P3YDueYU+1lXQvCcV3komg8kUtTUTBS9Gwn3JyzbD
uLZ4tlibH7o7vWuBk61ntzUN/r7EFebOfTLbjoaDm7mDVXBE2IhIwVVbH/rIcUcS
/wIDAQAB
-----END PUBLIC KEY-----
9 changes: 9 additions & 0 deletions tests_e2e/test_suites/keyvault_certificates.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#
# This test verifies that the Agent can download and extract KeyVault certificates that use different encryption algorithms
#
name: "KeyvaultCertificates"
tests:
- "keyvault_certificates/keyvault_certificates.py"
images:
- "endorsed"
- "endorsed-arm64"
98 changes: 98 additions & 0 deletions tests_e2e/tests/keyvault_certificates/keyvault_certificates.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
#!/usr/bin/env python3

# Microsoft Azure Linux Agent
#
# Copyright 2018 Microsoft Corporation
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#

#
# This test verifies that the Agent can download and extract KeyVault certificates that use different encryption algorithms (currently EC and RSA).
#
from assertpy import fail

from tests_e2e.tests.lib.agent_test import AgentTest
from tests_e2e.tests.lib.logging import log
from tests_e2e.tests.lib.shell import CommandError
from tests_e2e.tests.lib.ssh_client import SshClient
from tests_e2e.tests.lib.virtual_machine_client import VirtualMachineClient


class KeyvaultCertificates(AgentTest):
def run(self):
test_certificates = {
'C49A06B3044BD1778081366929B53EBF154133B3': {
'AzureCloud': 'https://waagenttests.vault.azure.net/secrets/ec-cert/39862f0c6dff4b35bc8a83a5770c2102',
'AzureChinaCloud': 'https://waagenttests.vault.azure.cn/secrets/ec-cert/bb610217ef70412bb3b3c8d7a7fabfdc',
'AzureUSGovernment': 'https://waagenttests.vault.usgovcloudapi.net/secrets/ec-cert/9c20ef55c7074a468f04a168b3488933'
},
'2F846E657258E50C7011E1F68EA9AD129BA4AB31': {
'AzureCloud': 'https://waagenttests.vault.azure.net/secrets/rsa-cert/0b5eac1e66fb457bb3c3419fce17e705',
'AzureChinaCloud': 'https://waagenttests.vault.azure.cn/secrets/rsa-cert/98679243f8d6493e95281a852d8cee00',
'AzureUSGovernment': 'https://waagenttests.vault.usgovcloudapi.net/secrets/rsa-cert/463a8a6be3b3436d85d3d4e406621c9e'
}
}
thumbprints = test_certificates.keys()
certificate_urls = [u[self._context.vm.cloud] for u in test_certificates.values()]

# The test certificates should be downloaded to these locations
expected_certificates = " ".join([f"/var/lib/waagent/{t}.{{crt,prv}}" for t in thumbprints])

# The test may be running on a VM that has already been tested (e.g. while debugging the test), so we need to delete any existing test certificates first
# (note that rm -f does not fail if the given files do not exist)
ssh_client: SshClient = self._context.create_ssh_client()
log.info("Deleting any existing test certificates on the test VM.")
existing_certificates = ssh_client.run_command(f"rm -f -v {expected_certificates}", use_sudo=True)
if existing_certificates == "":
log.info("No existing test certificates were found on the test VM.")
else:
log.info("Some test certificates had already been downloaded to the test VM (they have been deleted now):\n%s", existing_certificates)

vm: VirtualMachineClient = VirtualMachineClient(self._context.vm)

osprofile = {
"location": self._context.vm.location,
"properties": {
"osProfile": {
"secrets": [
{
"sourceVault": {
"id": f"/subscriptions/{self._context.vm.subscription}/resourceGroups/waagent-tests/providers/Microsoft.KeyVault/vaults/waagenttests"
},
"vaultCertificates": [{"certificateUrl": url} for url in certificate_urls]
}
],
}
}
}
log.info("updating the vm's osProfile with the certificates to download:\n%s", osprofile)
vm.update(osprofile)

# If the test has already run on the VM, force a new goal state to ensure the certificates are downloaded since the VM model most likely already had the certificates
# and the update operation would not have triggered a goal state
if existing_certificates != "":
log.info("Reapplying the goal state to ensure the test certificates are downloaded.")
vm.reapply()

try:
output = ssh_client.run_command(f"ls {expected_certificates}", use_sudo=True)
log.info("Found all the expected certificates:\n%s", output)
except CommandError as error:
if error.stdout != "":
log.info("Found some of the expected certificates:\n%s", error.stdout)
fail(f"Failed to find certificates\n{error.stderr}")


if __name__ == "__main__":
KeyvaultCertificates.run_from_command_line()
9 changes: 9 additions & 0 deletions tests_e2e/tests/lib/virtual_machine_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,15 @@ def update(self, properties: Dict[str, Any], timeout: int = AzureClient._DEFAULT
operation_name=f"Update {self._identifier}",
timeout=timeout)

def reapply(self, timeout: int = AzureClient._DEFAULT_TIMEOUT) -> None:
"""
Reapplies the goal state on the virtual machine
"""
self._execute_async_operation(
lambda: self._compute_client.virtual_machines.begin_reapply(self._identifier.resource_group, self._identifier.name),
operation_name=f"Reapply {self._identifier}",
timeout=timeout)

def restart(
self,
wait_for_boot,
Expand Down
Loading