From 9a9febd9c075cf43a4d03bee73bf6f9884552c60 Mon Sep 17 00:00:00 2001 From: Glynn Forrest Date: Tue, 7 May 2019 23:55:39 +0100 Subject: [PATCH] Rewrite x509.certificate_managed to be easier to use The function now displays clearer error messages when a problem occurs and informative messages when comparing an existing certificate. test=True is now supported. It fixes the following errors: * Certificate errors are written to the target file (#41858) * New certificates are created every run (#52167) The `managed_private_key` option has been removed due to the added complexity. The functionality can easily be replicated with an additional call to `x509.private_key_managed`. According to the comment at https://github.com/saltstack/salt/issues/39608#issuecomment-342346894 `managed_private_key` has not worked since at least v2016.11.2. --- salt/states/x509.py | 316 +++++++++++++++++++++++--------------------- 1 file changed, 162 insertions(+), 154 deletions(-) diff --git a/salt/states/x509.py b/salt/states/x509.py index 39571aef272c..be0365a45811 100644 --- a/salt/states/x509.py +++ b/salt/states/x509.py @@ -385,9 +385,109 @@ def csr_managed(name, return ret +def _certificate_info_matches(cert_info, required_cert_info, check_serial=False): + ''' + Return true if the provided certificate information matches the + required certificate information, i.e. it has the required common + name, subject alt name, organization, etc. + + cert_info should be a dict as returned by x509.read_certificate. + required_cert_info should be a dict as returned by x509.create_certificate with testrun=True. + ''' + # don't modify the incoming dicts + cert_info = copy.deepcopy(cert_info) + required_cert_info = copy.deepcopy(required_cert_info) + + ignored_keys = [ + 'Not Before', + 'Not After', + 'MD5 Finger Print', + 'SHA1 Finger Print', + 'SHA-256 Finger Print', + # The integrity of the issuer is checked elsewhere + 'Issuer Public Key' + ] + for key in ignored_keys: + cert_info.pop(key, None) + required_cert_info.pop(key, None) + + if not check_serial: + cert_info.pop('Serial Number', None) + required_cert_info.pop('Serial Number', None) + try: + cert_info['X509v3 Extensions']['authorityKeyIdentifier'] = ( + re.sub(r'serial:([0-9A-F]{2}:)*[0-9A-F]{2}', 'serial:--', + cert_info['X509v3 Extensions']['authorityKeyIdentifier'])) + required_cert_info['X509v3 Extensions']['authorityKeyIdentifier'] = ( + re.sub(r'serial:([0-9A-F]{2}:)*[0-9A-F]{2}', 'serial:--', + required_cert_info['X509v3 Extensions']['authorityKeyIdentifier'])) + except KeyError: + pass + + diff = [] + for k, v in six.iteritems(required_cert_info): + try: + if v != cert_info[k]: + diff.append(k) + except KeyError: + diff.append(k) + + return len(diff) == 0, diff + +def _certificate_days_remaining(cert_info): + try: + expiry = cert_info['Not After'] + return ( + datetime.datetime.strptime(expiry, '%Y-%m-%d %H:%M:%S') - + datetime.datetime.now()).days + except KeyError: + return 0 + + +def _certificate_is_valid(name, days_remaining, append_certs, **cert_spec): + ''' + Return True if the given certificate file exists, is a certificate, matches the given specification, and has the required days remaining. + + If False, also provide a message explaining why. + ''' + if not os.path.isfile(name): + return False, '{0} does not exist'.format(name), {} + + try: + cert_info = __salt__['x509.read_certificate'](certificate=name) + required_cert_info = __salt__['x509.create_certificate'](testrun=True, **cert_spec) + if not isinstance(required_cert_info, dict): + raise salt.exceptions.SaltInvocationError( + 'Unable to create new certificate: x509 module error: {0}'.format(required_cert_info)) + + try: + issuer_public_key = required_cert_info['Issuer Public Key'] + # Verify the certificate has been signed by the ca_server or private_signing_key + if not __salt__['x509.verify_signature'](name, issuer_public_key): + errmsg = 'Certificate is not signed by private_signing_key' if 'signing_private_key' in cert_spec else 'Certificate is not signed by the requested issuer' + return False, errmsg, cert_info + except KeyError: + return False, 'New certificate does not include signing information', cert_info + + matches, diff = _certificate_info_matches( + cert_info, + required_cert_info, + check_serial='serial_number' in cert_spec + ) + if not matches: + return False, 'Certificate properties are different: {0}'.format(', '.join(diff)), cert_info + + actual_days_remaining = _certificate_days_remaining(cert_info) + if days_remaining != 0 and actual_days_remaining < days_remaining: + return False, 'Certificate needs renewal: {0} days remaining but it needs to be at least {1}'.format(actual_days_remaining, days_remaining), cert_info + + return True, '', cert_info + except salt.exceptions.SaltInvocationError: + return False, '{0} is not a valid certificate'.format(name), {} + + def certificate_managed(name, days_remaining=90, - managed_private_key=None, append_certs=None, **kwargs): ''' @@ -400,13 +500,6 @@ def certificate_managed(name, The minimum number of days remaining when the certificate should be recreated. A value of 0 disables automatic renewal. - managed_private_key - Manages the private key corresponding to the certificate. All of the - arguments supported by :py:func:`x509.private_key_managed - ` are supported. If `name` is not - speicified or is the same as the name of the certificate, the private - key and certificate will be written together in the same file. - append_certs: A list of certificates to be appended to the managed file. @@ -415,13 +508,6 @@ def certificate_managed(name, ` or :py:func:`file.managed ` are supported. - ext_mapping: - Provide additional X509v3 extension mappings. This argument should be - in the form of a dictionary and should include both the OID and the - friendly name for the extension. - - .. versionadded:: Neon - Examples: .. code-block:: yaml @@ -458,154 +544,76 @@ def certificate_managed(name, if 'path' in kwargs: name = kwargs.pop('path') - file_args, kwargs = _get_file_args(name, **kwargs) - - rotate_private_key = False - new_private_key = False - if managed_private_key: - private_key_args = { - 'name': name, - 'new': False, - 'overwrite': False, - 'bits': 2048, - 'passphrase': None, - 'cipher': 'aes_128_cbc', - 'verbose': True - } - private_key_args.update(managed_private_key) - kwargs['public_key_passphrase'] = private_key_args['passphrase'] - - if private_key_args['new']: - rotate_private_key = True - private_key_args['new'] = False - - if _check_private_key(private_key_args['name'], - bits=private_key_args['bits'], - passphrase=private_key_args['passphrase'], - new=private_key_args['new'], - overwrite=private_key_args['overwrite']): - private_key = __salt__['x509.get_pem_entry']( - private_key_args['name'], pem_type='RSA PRIVATE KEY') - else: - new_private_key = True - private_key = __salt__['x509.create_private_key'](text=True, bits=private_key_args['bits'], - passphrase=private_key_args['passphrase'], - cipher=private_key_args['cipher'], - verbose=private_key_args['verbose']) - - kwargs['public_key'] = private_key - - current_days_remaining = 0 - current_comp = {} - - if os.path.isfile(name): - try: - current = __salt__['x509.read_certificate'](certificate=name) - current_comp = copy.deepcopy(current) - if 'serial_number' not in kwargs: - current_comp.pop('Serial Number') - if 'signing_cert' not in kwargs: - try: - current_comp['X509v3 Extensions']['authorityKeyIdentifier'] = ( - re.sub(r'serial:([0-9A-F]{2}:)*[0-9A-F]{2}', 'serial:--', - current_comp['X509v3 Extensions']['authorityKeyIdentifier'])) - except KeyError: - pass - current_comp.pop('Not Before') - current_comp.pop('MD5 Finger Print') - current_comp.pop('SHA1 Finger Print') - current_comp.pop('SHA-256 Finger Print') - current_notafter = current_comp.pop('Not After') - current_days_remaining = ( - datetime.datetime.strptime(current_notafter, '%Y-%m-%d %H:%M:%S') - - datetime.datetime.now()).days - if days_remaining == 0: - days_remaining = current_days_remaining - 1 - except salt.exceptions.SaltInvocationError: - current = '{0} is not a valid Certificate.'.format(name) - else: - current = '{0} does not exist.'.format(name) - if 'ca_server' in kwargs and 'signing_policy' not in kwargs: raise salt.exceptions.SaltInvocationError( 'signing_policy must be specified if ca_server is.') - new = __salt__['x509.create_certificate'](testrun=False, text=True, **kwargs) - new = __salt__['x509.read_certificate'](certificate=new) - newcert = __salt__['x509.create_certificate'](testrun=True, **kwargs) - - if isinstance(new, dict): - new_comp = copy.deepcopy(new) - if 'serial_number' not in kwargs: - new_comp.pop('Serial Number') - if 'signing_cert' not in kwargs: - try: - new_comp['X509v3 Extensions']['authorityKeyIdentifier'] = ( - re.sub(r'serial:([0-9A-F]{2}:)*[0-9A-F]{2}', 'serial:--', - new_comp['X509v3 Extensions']['authorityKeyIdentifier'])) - except KeyError: - pass - new_comp.pop('Not Before') - new_comp.pop('Not After') - new_comp.pop('MD5 Finger Print') - new_comp.pop('SHA1 Finger Print') - new_comp.pop('SHA-256 Finger Print') - new_issuer_public_key = new_issuer_public_key = newcert.pop('Issuer Public Key') - else: - new_comp = new + if 'public_key' not in kwargs and 'signing_private_key' not in kwargs: + raise salt.exceptions.SaltInvocationError( + 'Either public_key or signing_private_key must be specified.') - new_certificate = False - if (current_comp == new_comp and - current_days_remaining > days_remaining and - __salt__['x509.verify_signature'](name, new_issuer_public_key)): - certificate = __salt__['x509.get_pem_entry']( - name, pem_type='CERTIFICATE') - else: - if rotate_private_key and not new_private_key: - new_private_key = True - private_key = __salt__['x509.create_private_key']( - text=True, bits=private_key_args['bits'], verbose=private_key_args['verbose']) - kwargs['public_key'] = private_key - new_certificate = True - certificate = __salt__['x509.create_certificate'](text=True, **kwargs) - - file_args['contents'] = '' - private_ret = {} - if managed_private_key: - if private_key_args['name'] == name: - file_args['contents'] = private_key - else: - private_file_args = copy.deepcopy(file_args) - unique_private_file_args, _ = _get_file_args(**private_key_args) - private_file_args.update(unique_private_file_args) - private_file_args['contents'] = private_key - private_ret = __states__['file.managed'](**private_file_args) - if not private_ret['result']: - return private_ret + if 'public_key' in kwargs and 'signing_private_key' in kwargs: + raise salt.exceptions.SaltInvocationError( + 'Either public_key or signing_private_key must be specified, not both.') + + ret = {'name': name, + 'result': False, + 'changes': {}, + 'comment': ''} + + is_valid, invalid_reason, current_cert_info = _certificate_is_valid(name, days_remaining, append_certs, **kwargs) + + if is_valid: + ret['result'] = True + ret['comment'] = 'Certificate {0} is valid and up to date'.format(name) + return ret + + if __opts__['test']: + ret['result'] = None + ret['comment'] = 'Certificate {0} will be created'.format(name) + ret['changes']['Status'] = { + 'Old': invalid_reason, + 'New': 'Certificate will be valid and up to date' + } + return ret - file_args['contents'] += salt.utils.stringutils.to_str(certificate) + contents = __salt__['x509.create_certificate'](text=True, **kwargs) + # Check the module actually returned a cert and not an error message as a string + try: + __salt__['x509.read_certificate'](contents) + except salt.exceptions.SaltInvocationError as e: + ret['result'] = False + ret['comment'] = 'An error occurred creating the certificate {0}. The result returned from x509.create_certificate is not a valid PEM file:\n{1}'.format(name, str(e)) + return ret if not append_certs: append_certs = [] - for append_cert in append_certs: - file_args[ - 'contents'] += __salt__['x509.get_pem_entry'](append_cert, pem_type='CERTIFICATE') - - file_args['show_changes'] = False - ret = __states__['file.managed'](**file_args) - - if ret['changes']: - ret['changes'] = {'Certificate': ret['changes']} - else: - ret['changes'] = {} - if private_ret and private_ret['changes']: - ret['changes']['Private Key'] = private_ret['changes'] - if new_private_key: - ret['changes']['Private Key'] = 'New private key generated' - if new_certificate: - ret['changes']['Certificate'] = { - 'Old': current, - 'New': __salt__['x509.read_certificate'](certificate=certificate)} + for append_file in append_certs: + try: + append_file_contents = __salt__['x509.get_pem_entry'](append_file, pem_type='CERTIFICATE') + contents += append_file_contents + except salt.exceptions.SaltInvocationError as e: + ret['result'] = False + ret['comment'] = '{0} is not a valid certificate file, cannot append it to the certificate:\n{1}'.format(name, str(e)) + return ret + + file_args, extra_args = _get_file_args(name, **kwargs) + file_args['contents'] = contents + file_ret = __states__['file.managed'](**file_args) + + if file_ret['changes']: + ret['changes'] = {'File': file_ret['changes']} + + ret['changes']['Certificate'] = { + 'Old': current_cert_info, + 'New': __salt__['x509.read_certificate'](certificate=name) + } + ret['changes']['Status'] = { + 'Old': invalid_reason, + 'New': 'Certificate is valid and up to date' + } + ret['comment'] = 'Certificate {0} is valid and up to date'.format(name) + ret['result'] = True return ret