diff --git a/src/azure-cli/azure/cli/command_modules/role/custom.py b/src/azure-cli/azure/cli/command_modules/role/custom.py index bf768a61680..d411ae20f01 100644 --- a/src/azure-cli/azure/cli/command_modules/role/custom.py +++ b/src/azure-cli/azure/cli/command_modules/role/custom.py @@ -1534,22 +1534,30 @@ def _create_self_signed_cert(start_date, end_date): # pylint: disable=too-many- from OpenSSL import crypto from datetime import timedelta - _, cert_file = tempfile.mkstemp() - _, key_file = tempfile.mkstemp() - - # create a file with both cert & key so users can use to login - # leverage tempfile ot produce a random file name - _, temp_file = tempfile.mkstemp() - creds_file = path.join(path.expanduser("~"), path.basename(temp_file) + '.pem') - - # create a key pair + # Create a PEM file ~/tmpxxxxxxxx.pem with both PRIVATE KEY & CERTIFICATE so users can use to log in. + # The PEM file looks like + # -----BEGIN PRIVATE KEY----- + # MIIEv... + # -----END PRIVATE KEY----- + # -----BEGIN CERTIFICATE----- + # MIICo... + # -----END CERTIFICATE----- + + # Leverage tempfile to produce a random file name. The temp file itself is automatically deleted. + # There doesn't seem to be a good way to create a random file name without creating the file itself: + # https://stackoverflow.com/questions/26541416/generate-temporary-file-names-without-creating-actual-file-in-python + with tempfile.NamedTemporaryFile() as f: + temp_file_name = f.name + creds_file = path.join(path.expanduser("~"), path.basename(temp_file_name) + '.pem') + + # Create a key pair k = crypto.PKey() k.generate_key(crypto.TYPE_RSA, 2048) - # create a self-signed cert + # Create a self-signed cert cert = crypto.X509() subject = cert.get_subject() - # as long it works, we skip fileds C, ST, L, O, OU, which we have no reasonable defaults for + # As long as it works, we skip fields C, ST, L, O, OU, which we have no reasonable defaults for subject.CN = 'CLI-Login' cert.set_serial_number(1000) asn1_format = '%Y%m%d%H%M%SZ' @@ -1561,23 +1569,16 @@ def _create_self_signed_cert(start_date, end_date): # pylint: disable=too-many- cert.set_pubkey(k) cert.sign(k, 'sha1') - with open(cert_file, "wt") as f: - f.write(crypto.dump_certificate(crypto.FILETYPE_PEM, cert).decode()) - with open(key_file, "wt") as f: - f.write(crypto.dump_privatekey(crypto.FILETYPE_PEM, k).decode()) - - cert_string = None - with open(creds_file, 'wt') as cf: - with open(key_file, 'rt') as f: - cf.write(f.read()) - with open(cert_file, "rt") as f: - cert_string = f.read() - cf.write(cert_string) - os.chmod(creds_file, 0o600) # make the file readable/writable only for current user - - # get rid of the header and tails for upload to AAD: ----BEGIN CERT....---- - cert_string = re.sub(r'\-+[A-z\s]+\-+', '', cert_string).strip() - return (cert_string, creds_file, cert_start_date, cert_end_date) + cert_string = crypto.dump_certificate(crypto.FILETYPE_PEM, cert).decode() + key_string = crypto.dump_privatekey(crypto.FILETYPE_PEM, k).decode() + + with os.fdopen(_open(creds_file), 'w+') as cf: + cf.write(key_string) + cf.write(cert_string) + + # Get rid of the header and tail for uploading to AAD: -----BEGIN CERTIFICATE-----, -----END CERTIFICATE----- + cert_string = re.sub(r'-+[A-z\s]+-+', '', cert_string).strip() + return cert_string, creds_file, cert_start_date, cert_end_date def _create_self_signed_cert_with_keyvault(cli_ctx, years, keyvault, keyvault_cert_name): # pylint: disable=too-many-locals @@ -1870,3 +1871,9 @@ def _random_password(length): password = first_character + ''.join(password_list) return password + + +def _open(location): + """Open a file that only the current user can access.""" + # The 600 seems no-op on Windows, and that is fine. + return os.open(location, os.O_RDWR | os.O_CREAT | os.O_TRUNC, 0o600) diff --git a/src/azure-cli/azure/cli/command_modules/role/tests/latest/test_role.py b/src/azure-cli/azure/cli/command_modules/role/tests/latest/test_role.py index 6968668182d..6ca1c78f5ef 100644 --- a/src/azure-cli/azure/cli/command_modules/role/tests/latest/test_role.py +++ b/src/azure-cli/azure/cli/command_modules/role/tests/latest/test_role.py @@ -6,6 +6,7 @@ # AZURE CLI RBAC TEST DEFINITIONS import json import os +import sys import tempfile import time import datetime @@ -118,10 +119,22 @@ def test_create_for_rbac_with_cert_no_assignment(self, resource_group): self.kwargs['app_id'] = result['appId'] self.assertTrue(result['fileWithCertAndPrivateKey'].endswith('.pem')) + + # On Linux or MacOS, check the cert file is a regular file (S_IFREG 0100000) with permission 600 + # https://manpages.ubuntu.com/manpages/focal/man7/inode.7.html + # Windows doesn't have the Linux permission concept. + if sys.platform != 'win32': + assert os.stat(result['fileWithCertAndPrivateKey']).st_mode == 0o100600 + os.remove(result['fileWithCertAndPrivateKey']) + result = self.cmd('ad sp credential reset -n {app_id} --create-cert', checks=self.check('name', '{app_id}')).get_output_in_json() self.assertTrue(result['fileWithCertAndPrivateKey'].endswith('.pem')) + + if sys.platform != 'win32': + assert os.stat(result['fileWithCertAndPrivateKey']).st_mode == 0o100600 + os.remove(result['fileWithCertAndPrivateKey']) finally: self.cmd('ad app delete --id {app_id}',