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.

Sign up for GitHub

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

Support for FIPS 140-3 #3324

Merged
merged 5 commits into from
Mar 4, 2025
Merged

Support for FIPS 140-3 #3324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fec46ca
Support for FIPS 140-3
Feb 25, 2025
b1bf35d
Add unit tests
Feb 26, 2025
a3abb6c
review feedback
Mar 4, 2025
fa70c5b
Merge branch 'develop' into fips
narrieta Mar 4, 2025
f6ec0a2
Merge branch 'develop' into fips
narrieta Mar 4, 2025
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
20 changes: 20 additions & 0 deletions azurelinuxagent/common/event.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ class WALAEventOperation:
FetchGoalState = "FetchGoalState"
Firewall = "Firewall"
GoalState = "GoalState"
GoalStateCertificates = "GoalStateCertificates"
GoalStateUnsupportedFeatures = "GoalStateUnsupportedFeatures"
HealthCheck = "HealthCheck"
HealthObservation = "HealthObservation"
Expand Down Expand Up @@ -733,6 +734,25 @@ def error(op, fmt, *args):
add_event(op=op, message=fmt.format(*args), is_success=False, log_event=False)


class LogEvent(object):
"""
Helper class that allows the use of info()/warn()/error() using a specific instance of a logger.
"""
def __init__(self, logger_):
self._logger = logger_

def info(self, op, fmt, *args):
self._logger.info(fmt, *args)
add_event(op=op, message=fmt.format(*args), is_success=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

should log_event=False for info events?

Copy link
Member Author

Choose a reason for hiding this comment

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

log_event is used only when is_success == False:

    def add_event(self, name, op=WALAEventOperation.Unknown, is_success=True, duration=0, version=str(CURRENT_VERSION),
                  message="", log_event=True, flush=False):
        if (not is_success) and log_event:
            _log_event(name, op, message, duration, is_success=is_success)


def warn(self, op, fmt, *args):
self._logger.warn(fmt, *args)
add_event(op=op, message="[WARNING] " + fmt.format(*args), is_success=False, log_event=False)

def error(self, op, fmt, *args):
self._logger.error(fmt, *args)
add_event(op=op, message=fmt.format(*args), is_success=False, log_event=False)

def add_log_event(level, message, forced=False, reporter=__event_logger__):
"""
:param level: LoggerLevel of the log event
Expand Down
216 changes: 131 additions & 85 deletions azurelinuxagent/common/protocol/goal_state.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,14 @@
from azurelinuxagent.common import conf
from azurelinuxagent.common import logger
from azurelinuxagent.common.AgentGlobals import AgentGlobals
from azurelinuxagent.common.datacontract import set_properties
from azurelinuxagent.common.event import add_event, WALAEventOperation
from azurelinuxagent.common.event import add_event, WALAEventOperation, LogEvent
from azurelinuxagent.common.exception import ProtocolError, ResourceGoneError
from azurelinuxagent.common.future import ustr
from azurelinuxagent.common.protocol.extensions_goal_state_factory import ExtensionsGoalStateFactory
from azurelinuxagent.common.protocol.extensions_goal_state import VmSettingsParseError, GoalStateSource
from azurelinuxagent.common.protocol.hostplugin import VmSettingsNotSupported, VmSettingsSupportStopped
from azurelinuxagent.common.protocol.restapi import Cert, CertList, RemoteAccessUser, RemoteAccessUsersList, ExtHandlerPackage, ExtHandlerPackageList
from azurelinuxagent.common.utils import fileutil
from azurelinuxagent.common.protocol.restapi import RemoteAccessUser, RemoteAccessUsersList, ExtHandlerPackage, ExtHandlerPackageList
from azurelinuxagent.common.utils import fileutil, shellutil
from azurelinuxagent.common.utils.archive import GoalStateHistory, SHARED_CONF_FILE_NAME
from azurelinuxagent.common.utils.cryptutil import CryptUtil
from azurelinuxagent.common.utils.textutil import parse_doc, findall, find, findtext, getattrib, gettext
Expand All @@ -41,6 +40,7 @@
GOAL_STATE_URI = "http://{0}/machine/?comp=goalstate"
CERTS_FILE_NAME = "Certificates.xml"
P7M_FILE_NAME = "Certificates.p7m"
PFX_FILE_NAME = "Certificates.pfx"
PEM_FILE_NAME = "Certificates.pem"
TRANSPORT_CERT_FILE_NAME = "TransportCert.pem"
TRANSPORT_PRV_FILE_NAME = "TransportPrivate.pem"
Expand Down Expand Up @@ -286,16 +286,8 @@ def update(self, force_update=False, silent=False):
self._check_and_download_missing_certs_on_disk()

def _download_certificates(self, certs_uri):
xml_text = self._wire_client.fetch_config(certs_uri, self._wire_client.get_header_for_cert())
certs = Certificates(xml_text, self.logger)
# Log and save the certificates summary (i.e. the thumbprint but not the certificate itself) to the goal state history
for c in certs.summary:
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved this logging to the Certificates class

message = "Downloaded certificate {0}".format(c)
self.logger.info(message)
add_event(op=WALAEventOperation.GoalState, message=message)
if len(certs.warnings) > 0:
self.logger.warn(certs.warnings)
add_event(op=WALAEventOperation.GoalState, message=certs.warnings)
certs = Certificates(self._wire_client, certs_uri, self.logger)
# 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))
return certs
Expand Down Expand Up @@ -515,100 +507,156 @@ def __init__(self, xml_text):
self.xml_text = xml_text


class Certificates(object):
def __init__(self, xml_text, my_logger):
self.cert_list = CertList()
self.summary = [] # debugging info
self.warnings = []
class Certificates(LogEvent):
def __init__(self, wire_client, uri, logger_):
super(Certificates, self).__init__(logger_)
self.summary = []
self._crypt_util = CryptUtil(conf.get_openssl_cmd())

# Save the certificates
local_file = os.path.join(conf.get_lib_dir(), CERTS_FILE_NAME)
fileutil.write_file(local_file, xml_text)
try:
pfx_file = self._download_certificates_pfx(wire_client, uri)
Copy link
Member Author

Choose a reason for hiding this comment

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

I need to distinguish between failures in the encryption protocol with the WireServer and failures parsing the PFX, so I split those into separate methods. I also moved the creation of the *.prv and *.crt files from the the Certificates.pem to its own method.

if pfx_file is None: # The response from the WireServer may not have any certificates
return

# Separate the certificates into individual files.
xml_doc = parse_doc(xml_text)
data = findtext(xml_doc, "Data")
if data is None:
return
try:
pem_file = self._convert_certificates_pfx_to_pem(pfx_file)
finally:
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add a finally block to line 529 and do the pfx removal there if it exists? It seems possible for umask() to raise Exception when we reset the file mode creation in the finally block of decrypt_certificates_p7m?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, thanks. I think it is the responsibility of _download_certificates_pfx to cleanup when it raises. I'll handle this case there

self._remove_file(pfx_file)

# if the certificates format is not Pkcs7BlobWithPfxContents do not parse it
certificate_format = findtext(xml_doc, "Format")
if certificate_format and certificate_format != "Pkcs7BlobWithPfxContents":
message = "The Format is not Pkcs7BlobWithPfxContents. Format is {0}".format(certificate_format)
my_logger.warn(message)
add_event(op=WALAEventOperation.GoalState, message=message)
return
self.summary = self._extract_certificate(pem_file)

for c in self.summary:
self.info(WALAEventOperation.GoalStateCertificates, "Downloaded certificate {0}", c)

except Exception as e:
self.error(WALAEventOperation.GoalStateCertificates, "Error fetching the goal state certificates: {0}", ustr(e))

def _remove_file(self, file):
if os.path.exists(file):
try:
os.remove(file)
except Exception as e:
self.warn(WALAEventOperation.GoalStateCertificates, "Failed to remove {0}: {1}", file, ustr(e))

def _download_certificates_pfx(self, wire_client, uri):
"""
Downloads the certificates from the WireServer and saves them to a pfx file.
Returns the full path of the pfx file, or None, if the WireServer response does not have a "Data" element
"""
trans_prv_file = os.path.join(conf.get_lib_dir(), TRANSPORT_PRV_FILE_NAME)
trans_cert_file = os.path.join(conf.get_lib_dir(), TRANSPORT_CERT_FILE_NAME)
xml_file = os.path.join(conf.get_lib_dir(), CERTS_FILE_NAME)
pfx_file = os.path.join(conf.get_lib_dir(), PFX_FILE_NAME)

for cypher in ["AES128_CBC", "DES_EDE3_CBC"]:
Copy link
Member Author

@narrieta narrieta Feb 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The WireServer and OpenSSL on the VM should both support AES128, but I am keeping Triple DES as a fallback. Depending on telemetry, I may remove the fallback on a subsequent release.

headers = wire_client.get_headers_for_encrypted_request(cypher)

try:
xml_text = wire_client.fetch_config(uri, headers)
except Exception as e:
Copy link
Member Author

@narrieta narrieta Feb 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I am catching a generic Exception to collect telemetry that will help me decide what specific errors should be handled with a retry. For the moment, in the worst case, we are doing a retry that won't help, but this is done on a per-goal-state basis, so the impact is minimal.

Same applies for catching a generic shellutil.CommandError in the code below and in _convert_certificates_pfx_to_pem()

self.warn(WALAEventOperation.GoalStateCertificates, "Error in Certificates request [cypher: {0}]: {1}", cypher, ustr(e))
continue

cryptutil = CryptUtil(conf.get_openssl_cmd())
fileutil.write_file(xml_file, xml_text)

xml_doc = parse_doc(xml_text)
data = findtext(xml_doc, "Data")
if data is None:
self.info(WALAEventOperation.GoalStateCertificates, "The Data element of the Certificates response is empty")
return None
certificate_format = findtext(xml_doc, "Format")
if certificate_format and certificate_format != "Pkcs7BlobWithPfxContents":
self.warn(WALAEventOperation.GoalStateCertificates, "The Certificates format is not Pkcs7BlobWithPfxContents; skipping. Format is {0}", certificate_format)
return None

p7m_file = Certificates._create_p7m_file(data)

try:
self._crypt_util.decrypt_certificates_p7m(p7m_file, trans_prv_file, trans_cert_file, pfx_file)
except shellutil.CommandError as e:
self.warn(WALAEventOperation.GoalState, "Error in transport decryption [cypher: {0}]: {1}", cypher, ustr(e))
self._remove_file(pfx_file)
continue

return pfx_file

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

@staticmethod
def _create_p7m_file(data):
p7m_file = os.path.join(conf.get_lib_dir(), P7M_FILE_NAME)
p7m = ("MIME-Version:1.0\n" # pylint: disable=W1308
"Content-Disposition: attachment; filename=\"{0}\"\n"
"Content-Type: application/x-pkcs7-mime; name=\"{1}\"\n"
"Content-Transfer-Encoding: base64\n"
"\n"
"{2}").format(p7m_file, p7m_file, data)

fileutil.write_file(p7m_file, p7m)
return p7m_file

trans_prv_file = os.path.join(conf.get_lib_dir(), TRANSPORT_PRV_FILE_NAME)
trans_cert_file = os.path.join(conf.get_lib_dir(), TRANSPORT_CERT_FILE_NAME)
def _convert_certificates_pfx_to_pem(self, pfx_file):
"""
Convert the pfx file to pem file.
"""
pem_file = os.path.join(conf.get_lib_dir(), PEM_FILE_NAME)
# decrypt certificates
cryptutil.decrypt_p7m(p7m_file, trans_prv_file, trans_cert_file, pem_file)

for nomacver in [True, False]:
try:
self._crypt_util.convert_pfx_to_pem(pfx_file, nomacver, pem_file)
return pem_file
except shellutil.CommandError as e:
self._remove_file(pem_file) # An error may leave an empty pem file, which can produce a failure on some versions of open SSL (e.g. 3.2.2) on the next invocation
self.warn(WALAEventOperation.GoalState, "Error converting PFX to PEM [-nomacver: {0}]: {1}", nomacver, ustr(e))
continue

raise Exception("Cannot convert PFX to PEM")

def _extract_certificate(self, pem_file):
"""
Parse the certificates and private keys from the pem file and store them in the certificates directory.
"""
# The parsing process use public key to match prv and crt.
buf = []
prvs = {}
thumbprints = {}
private_keys = {} # map of private keys indexed by public key
thumbprints = {} # map of thumbprints indexed by public key
buffer = [] # buffer for reading lines belonging to a certificate or private key
index = 0
v1_cert_list = []

# Ensure pem_file exists before read the certs data since decrypt_p7m may clear the pem_file wen decryption fails
if os.path.exists(pem_file):
with open(pem_file) as pem:
for line in pem.readlines():
buf.append(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
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)
thumbprint = cryptutil.get_thumbprint_from_crt(tmp_file)
thumbprints[pub] = thumbprint
# Rename crt with thumbprint as the file name
crt = "{0}.crt".format(thumbprint)
v1_cert_list.append({
Copy link
Member Author

Choose a reason for hiding this comment

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

this certificate list was not being used, and most of the properties in each item were not even populated

"name": None,
"thumbprint": thumbprint
})
os.rename(tmp_file, os.path.join(conf.get_lib_dir(), crt))
buf = []
index += 1

with open(pem_file) as pem:
for line in pem.readlines():
buffer.append(line)
if re.match(r'[-]+END.*KEY[-]+', line):
tmp_file = Certificates._write_to_tmp_file(index, 'prv', buffer)
pub = self._crypt_util.get_pubkey_from_prv(tmp_file)
private_keys[pub] = tmp_file
buffer = []
index += 1
elif re.match(r'[-]+END.*CERTIFICATE[-]+', line):
tmp_file = Certificates._write_to_tmp_file(index, 'crt', buffer)
pub = self._crypt_util.get_pubkey_from_crt(tmp_file)
thumbprint = self._crypt_util.get_thumbprint_from_crt(tmp_file)
thumbprints[pub] = thumbprint
# Rename crt with thumbprint as the file name
crt = "{0}.crt".format(thumbprint)
os.rename(tmp_file, os.path.join(conf.get_lib_dir(), crt))
buffer = []
index += 1

# Rename prv key with thumbprint as the file name
for pubkey in prvs:
for pubkey in private_keys:
thumbprint = thumbprints[pubkey]
if thumbprint:
tmp_file = prvs[pubkey]
tmp_file = private_keys[pubkey]
prv = "{0}.prv".format(thumbprint)
os.rename(tmp_file, os.path.join(conf.get_lib_dir(), prv))
else:
# Since private key has *no* matching certificate,
# it will not be named correctly
self.warnings.append("Found NO matching cert/thumbprint for private key!")
# Since private key has *no* matching certificate, it will not be named correctly
self.warn(WALAEventOperation.GoalState, "Found a private key with no matching cert/thumbprint!")

certificates = []
for pubkey, thumbprint in thumbprints.items():
has_private_key = pubkey in prvs
self.summary.append({"thumbprint": thumbprint, "hasPrivateKey": has_private_key})

for v1_cert in v1_cert_list:
cert = Cert()
set_properties("certs", cert, v1_cert)
self.cert_list.certificates.append(cert)
has_private_key = pubkey in private_keys
certificates.append({"thumbprint": thumbprint, "hasPrivateKey": has_private_key})
return certificates

@staticmethod
def _write_to_tmp_file(index, suffix, buf):
Expand All @@ -618,9 +666,7 @@ def _write_to_tmp_file(index, suffix, buf):

class EmptyCertificates:
def __init__(self):
self.cert_list = CertList()
self.summary = [] # debugging info
self.warnings = []
self.summary = []

class RemoteAccess(object):
"""
Expand Down
24 changes: 0 additions & 24 deletions azurelinuxagent/common/protocol/restapi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,30 +43,6 @@ def __init__(self,
self.tenantName = tenantName


class CertificateData(DataContract):
def __init__(self, certificateData=None):
self.certificateData = certificateData


class Cert(DataContract):
def __init__(self,
name=None,
thumbprint=None,
certificateDataUri=None,
storeName=None,
storeLocation=None):
self.name = name
self.thumbprint = thumbprint
self.certificateDataUri = certificateDataUri
self.storeLocation = storeLocation
self.storeName = storeName


class CertList(DataContract):
def __init__(self):
self.certificates = DataContractList(Cert)


class VMAgentFamily(object):
def __init__(self, name):
self.name = name
Expand Down
18 changes: 9 additions & 9 deletions azurelinuxagent/common/protocol/wire.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,7 @@ def get_vminfo(self):
return vminfo

def get_certs(self):
certificates = self.client.get_certs()
return certificates.cert_list
return self.client.get_certs()

def get_goal_state(self):
return self.client.get_goal_state()
Expand Down Expand Up @@ -1140,26 +1139,27 @@ def get_header_for_xml_content(self):
"Content-Type": "text/xml;charset=utf-8"
}

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")
return self.get_headers_for_encrypted_request("AES128_CBC")

def _get_header_for_encrypted_request(self, cypher):
@staticmethod
def get_headers_for_encrypted_request(cypher):
trans_cert_file = os.path.join(conf.get_lib_dir(), TRANSPORT_CERT_FILE_NAME)
try:
content = fileutil.read_file(trans_cert_file)
except IOError as e:
raise ProtocolError("Failed to read {0}: {1}".format(trans_cert_file, e))

cert = get_bytes_from_pem(content)
return {
headers = {
"x-ms-agent-name": "WALinuxAgent",
"x-ms-version": PROTOCOL_VERSION,
"x-ms-cipher-name": cypher,
"x-ms-guest-agent-public-x509-cert": cert
}
if cypher is not None: # the cypher header is optional, currently defaults to AES128_CBC
headers["x-ms-cipher-name"] = cypher

return headers

def get_host_plugin(self):
if self._host_plugin is None:
Expand Down
Loading
Loading