openssl_csr: fix bad tests, avoid accepting invalid crl_distribution_points records (#560)

* Improve error handling.

* Remove invalid tests.

* Add changelog fragment.

* Fix tests.

* Improve exception catching.

Co-authored-by: Kristian Heljas <11139388+kristianheljas@users.noreply.github.com>

* Prevent empty full_name.

* Fix condition. Make sure errors are caught.

* Add more checks.

Co-authored-by: Kristian Heljas <11139388+kristianheljas@users.noreply.github.com>

changelogs/fragments/560-openssl_csr-crl_distribution_points.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - "openssl_csr, openssl_csr_pipe - prevent invalid values for ``crl_distribution_points`` that do not have one of ``full_name``, ``relative_name``, and ``crl_issuer`` (https://github.com/ansible-collections/community.crypto/pull/560)."

plugins/module_utils/crypto/module_backends/csr.py

@@ -270,8 +270,12 @@ def parse_crl_distribution_points(module, crl_distribution_points):
                 reasons=None,
             )
             if parse_crl_distribution_point['full_name'] is not None:
+                if not parse_crl_distribution_point['full_name']:
+                    raise OpenSSLObjectError('full_name must not be empty')
                 params['full_name'] = [cryptography_get_name(name, 'full name') for name in parse_crl_distribution_point['full_name']]
             if parse_crl_distribution_point['relative_name'] is not None:
+                if not parse_crl_distribution_point['relative_name']:
+                    raise OpenSSLObjectError('relative_name must not be empty')
                 try:
                     params['relative_name'] = cryptography_parse_relative_distinguished_name(parse_crl_distribution_point['relative_name'])
                 except Exception:
@@ -280,6 +284,8 @@ def parse_crl_distribution_points(module, crl_distribution_points):
                         raise OpenSSLObjectError('Cannot specify relative_name for cryptography < 1.6')
                     raise
             if parse_crl_distribution_point['crl_issuer'] is not None:
+                if not parse_crl_distribution_point['crl_issuer']:
+                    raise OpenSSLObjectError('crl_issuer must not be empty')
                 params['crl_issuer'] = [cryptography_get_name(name, 'CRL issuer') for name in parse_crl_distribution_point['crl_issuer']]
             if parse_crl_distribution_point['reasons'] is not None:
                 reasons = []
@@ -287,7 +293,7 @@ def parse_crl_distribution_points(module, crl_distribution_points):
                     reasons.append(REVOCATION_REASON_MAP[reason])
                 params['reasons'] = frozenset(reasons)
             result.append(cryptography.x509.DistributionPoint(**params))
-        except OpenSSLObjectError as e:
+        except (OpenSSLObjectError, ValueError) as e:
             raise OpenSSLObjectError('Error while parsing CRL distribution point #{index}: {error}'.format(index=index, error=e))
     return result
 
@@ -651,7 +657,8 @@ def get_csr_argument_spec():
                         'aa_compromise',
                     ]),
                 ),
-                mutually_exclusive=[('full_name', 'relative_name')]
+                mutually_exclusive=[('full_name', 'relative_name')],
+                required_one_of=[('full_name', 'relative_name', 'crl_issuer')],
             ),
             select_crypto_backend=dict(type='str', default='auto', choices=['auto', 'cryptography']),
         ),

plugins/modules/openssl_csr.py

@@ -339,9 +339,10 @@ def main():
     if not os.path.isdir(base_dir):
         module.fail_json(name=base_dir, msg='The directory %s does not exist or the file is not a directory' % base_dir)
 
+    try:
         backend = module.params['select_crypto_backend']
         backend, module_backend = select_backend(module, backend)
-    try:
+
         csr = CertificateSigningRequestModule(module, module_backend)
         if module.params['state'] == 'present':
             csr.generate(module)

plugins/modules/openssl_csr_pipe.py

@@ -167,9 +167,10 @@ def main():
         supports_check_mode=True,
     )
 
+    try:
         backend = module.params['select_crypto_backend']
         backend, module_backend = select_backend(module, backend)
-    try:
+
         csr = CertificateSigningRequestModule(module, module_backend)
         csr.generate(module)
         result = csr.dump()

tests/integration/targets/openssl_csr/tasks/impl.yml

@@ -950,7 +950,6 @@
               - CN=ca.example.com
             reasons:
               - certificate_hold
-          - {}
         select_crypto_backend: '{{ select_crypto_backend }}'
       register: crl_distribution_endpoints_1
 
@@ -973,7 +972,6 @@
               - CN=ca.example.com
             reasons:
               - certificate_hold
-          - {}
         select_crypto_backend: '{{ select_crypto_backend }}'
       register: crl_distribution_endpoints_2
 
@@ -984,9 +982,7 @@
         subject:
           commonName: www.ansible.com
         crl_distribution_points:
-          - full_name:
-              - "URI:https://ca.example.com/revocations.crl"
-            crl_issuer:
+          - crl_issuer:
               - "URI:https://ca.example.com/"
             reasons:
               - key_compromise