Merge pull request #49561 from dwoz/m2crypto_test

x509 module and state bug fixes

salt/fileserver/roots.py

@@ -161,7 +161,7 @@ def update():
     old_mtime_map = {}
     # if you have an old map, load that
     if os.path.exists(mtime_map_path):
-        with salt.utils.files.fopen(mtime_map_path, 'r') as fp_:
+        with salt.utils.files.fopen(mtime_map_path, 'rb') as fp_:
             for line in fp_:
                 line = salt.utils.stringutils.to_unicode(line)
                 try:
@@ -189,10 +189,10 @@ def update():
     mtime_map_path_dir = os.path.dirname(mtime_map_path)
     if not os.path.exists(mtime_map_path_dir):
         os.makedirs(mtime_map_path_dir)
-    with salt.utils.files.fopen(mtime_map_path, 'w') as fp_:
+    with salt.utils.files.fopen(mtime_map_path, 'wb') as fp_:
         for file_path, mtime in six.iteritems(new_mtime_map):
             fp_.write(
-                salt.utils.stringutils.to_str(
+                salt.utils.stringutils.to_bytes(
                     '{0}:{1}\n'.format(file_path, mtime)
                 )
             )
@@ -240,7 +240,7 @@ def file_hash(load, fnd):
     # if we have a cache, serve that if the mtime hasn't changed
     if os.path.exists(cache_path):
         try:
-            with salt.utils.files.fopen(cache_path, 'r') as fp_:
+            with salt.utils.files.fopen(cache_path, 'rb') as fp_:
                 try:
                     hsum, mtime = salt.utils.stringutils.to_unicode(fp_.read()).split(':')
                 except ValueError:

salt/modules/x509.py

@@ -26,6 +26,7 @@ import sys
 import salt.utils.files
 import salt.utils.path
 import salt.utils.stringutils
+import salt.utils.platform
 import salt.exceptions
 from salt.ext import six
 from salt.utils.odict import OrderedDict
@@ -315,12 +316,24 @@ def _dec2hex(decval):
     return _pretty_hex('{0:X}'.format(decval))
 
 
+def _isfile(path):
+    '''
+    A wrapper around os.path.isfile that ignores ValueError exceptions which
+    can be raised if the input to isfile is too long.
+    '''
+    try:
+        return os.path.isfile(path)
+    except ValueError:
+        pass
+    return False
+
+
 def _text_or_file(input_):
     '''
     Determines if input is a path to a file, or a string with the
     content to be parsed.
     '''
-    if os.path.isfile(input_):
+    if _isfile(input_):
         with salt.utils.files.fopen(input_) as fp_:
             return salt.utils.stringutils.to_str(fp_.read())
     else:
@@ -1425,12 +1438,18 @@ def create_certificate(
         kwargs['serial_number'] = _dec2hex(
             random.getrandbits(kwargs['serial_bits']))
     serial_number = int(kwargs['serial_number'].replace(':', ''), 16)
-    # With Python3 we occasionally end up with an INT
-    # that is too large because Python3 no longer supports long INTs.
-    # If we're larger than the maxsize value
-    # then we adjust the serial number.
-    if serial_number > sys.maxsize:
-        serial_number = serial_number - sys.maxsize
+    # With Python3 we occasionally end up with an INT that is greater than a C
+    # long max_value. This causes an overflow error due to a bug in M2Crypto.
+    # See issue: https://gitlab.com/m2crypto/m2crypto/issues/232
+    # Remove this after M2Crypto fixes the bug.
+    if six.PY3:
+        if salt.utils.platform.is_windows():
+            INT_MAX = 2147483647
+            if serial_number >= INT_MAX:
+                serial_number -= int(serial_number / INT_MAX) * INT_MAX
+        else:
+            if serial_number >= sys.maxsize:
+                serial_number -= int(serial_number / sys.maxsize) * sys.maxsize
     cert.set_serial_number(serial_number)
 
     # Set validity dates

salt/states/file.py

@@ -1140,7 +1140,9 @@ def _validate_str_list(arg):
     '''
     ensure ``arg`` is a list of strings
     '''
-    if isinstance(arg, six.string_types):
+    if isinstance(arg, six.binary_type):
+        ret = [salt.utils.stringutils.to_unicode(arg)]
+    elif isinstance(arg, six.string_types):
         ret = [arg]
     elif isinstance(arg, Iterable) and not isinstance(arg, Mapping):
         ret = []
@@ -2416,9 +2418,9 @@ def managed(name,
                 .format(contents_id)
             )
 
-        if isinstance(use_contents, bytes) and b'\0' in use_contents:
+        if isinstance(use_contents, six.binary_type) and b'\0' in use_contents:
             contents = use_contents
-        elif isinstance(use_contents, six.string_types) and str('\0') in use_contents:
+        elif isinstance(use_contents, six.text_type) and str('\0') in use_contents:
             contents = use_contents
         else:
             validated_contents = _validate_str_list(use_contents)

salt/states/x509.py

@@ -551,7 +551,7 @@ def certificate_managed(name,
             if not private_ret['result']:
                 return private_ret
 
-    file_args['contents'] += certificate
+    file_args['contents'] += salt.utils.stringutils.to_str(certificate)
 
     if not append_certs:
         append_certs = []

tests/integration/files/file/base/issue-49008.sls

@@ -0,0 +1,18 @@
+/test-ca-49008.crt:
+  x509.certificate_managed:
+    - signing_private_key: /test-ca-49008.key
+    - CN: testy-mctest
+    - basicConstraints: "critical CA:true"
+    - keyUsage: "critical cRLSign, keyCertSign"
+    - subjectKeyIdentifier: hash
+    - authorityKeyIdentifier: keyid,issuer:always
+    - days_valid: 1460
+    - days_remaining: 0
+    - backup: True
+    - watch:
+      - x509: /test-ca-49008.key
+
+/test-ca-49008.key:
+  x509.private_key_managed:
+    - bits: 4096
+    - backup: True

tests/integration/files/file/base/issue-49027.sls

@@ -0,0 +1,46 @@
+/test-49027.crt:
+  x509.pem_managed:
+    - text: |
+        -----BEGIN CERTIFICATE-----
+        MIIHjzCCBnegAwIBAgIIBzL2FMQfSVYwDQYJKoZIhvcNAQELBQAwVDELMAkGA1UE
+        BhMCVVMxHjAcBgNVBAoTFUdvb2dsZSBUcnVzdCBTZXJ2aWNlczElMCMGA1UEAxMc
+        R29vZ2xlIEludGVybmV0IEF1dGhvcml0eSBHMzAeFw0xODA3MjQxNjA4MjVaFw0x
+        ODEwMDIxNjAwMDBaMGYxCzAJBgNVBAYTAlVTMRMwEQYDVQQIDApDYWxpZm9ybmlh
+        MRYwFAYDVQQHDA1Nb3VudGFpbiBWaWV3MRMwEQYDVQQKDApHb29nbGUgTExDMRUw
+        EwYDVQQDDAwqLmdvb2dsZS5jb20wWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAASs
+        8tMhHKTNkKBHuyC9u0qbTibi9ZkpyvkFSPhBziOsLn7uDkU/PSKjHnSCswip07o9
+        F0kYWilWXKKxB5w2QQ0qo4IFHDCCBRgwEwYDVR0lBAwwCgYIKwYBBQUHAwEwDgYD
+        VR0PAQH/BAQDAgeAMIID4QYDVR0RBIID2DCCA9SCDCouZ29vZ2xlLmNvbYINKi5h
+        bmRyb2lkLmNvbYIWKi5hcHBlbmdpbmUuZ29vZ2xlLmNvbYISKi5jbG91ZC5nb29n
+        bGUuY29tghQqLmRiODMzOTUzLmdvb2dsZS5jboIGKi5nLmNvgg4qLmdjcC5ndnQy
+        LmNvbYIWKi5nb29nbGUtYW5hbHl0aWNzLmNvbYILKi5nb29nbGUuY2GCCyouZ29v
+        Z2xlLmNsgg4qLmdvb2dsZS5jby5pboIOKi5nb29nbGUuY28uanCCDiouZ29vZ2xl
+        LmNvLnVrgg8qLmdvb2dsZS5jb20uYXKCDyouZ29vZ2xlLmNvbS5hdYIPKi5nb29n
+        bGUuY29tLmJygg8qLmdvb2dsZS5jb20uY2+CDyouZ29vZ2xlLmNvbS5teIIPKi5n
+        b29nbGUuY29tLnRygg8qLmdvb2dsZS5jb20udm6CCyouZ29vZ2xlLmRlggsqLmdv
+        b2dsZS5lc4ILKi5nb29nbGUuZnKCCyouZ29vZ2xlLmh1ggsqLmdvb2dsZS5pdIIL
+        Ki5nb29nbGUubmyCCyouZ29vZ2xlLnBsggsqLmdvb2dsZS5wdIISKi5nb29nbGVh
+        ZGFwaXMuY29tgg8qLmdvb2dsZWFwaXMuY26CFCouZ29vZ2xlY29tbWVyY2UuY29t
+        ghEqLmdvb2dsZXZpZGVvLmNvbYIMKi5nc3RhdGljLmNugg0qLmdzdGF0aWMuY29t
+        ggoqLmd2dDEuY29tggoqLmd2dDIuY29tghQqLm1ldHJpYy5nc3RhdGljLmNvbYIM
+        Ki51cmNoaW4uY29tghAqLnVybC5nb29nbGUuY29tghYqLnlvdXR1YmUtbm9jb29r
+        aWUuY29tgg0qLnlvdXR1YmUuY29tghYqLnlvdXR1YmVlZHVjYXRpb24uY29tggcq
+        Lnl0LmJlggsqLnl0aW1nLmNvbYIaYW5kcm9pZC5jbGllbnRzLmdvb2dsZS5jb22C
+        C2FuZHJvaWQuY29tghtkZXZlbG9wZXIuYW5kcm9pZC5nb29nbGUuY26CHGRldmVs
+        b3BlcnMuYW5kcm9pZC5nb29nbGUuY26CBGcuY2+CBmdvby5nbIIUZ29vZ2xlLWFu
+        YWx5dGljcy5jb22CCmdvb2dsZS5jb22CEmdvb2dsZWNvbW1lcmNlLmNvbYIYc291
+        cmNlLmFuZHJvaWQuZ29vZ2xlLmNuggp1cmNoaW4uY29tggp3d3cuZ29vLmdsggh5
+        b3V0dS5iZYILeW91dHViZS5jb22CFHlvdXR1YmVlZHVjYXRpb24uY29tggV5dC5i
+        ZTBoBggrBgEFBQcBAQRcMFowLQYIKwYBBQUHMAKGIWh0dHA6Ly9wa2kuZ29vZy9n
+        c3IyL0dUU0dJQUczLmNydDApBggrBgEFBQcwAYYdaHR0cDovL29jc3AucGtpLmdv
+        b2cvR1RTR0lBRzMwHQYDVR0OBBYEFK/WqypxoW4KZ4D8CDU5lyVLJXPNMAwGA1Ud
+        EwEB/wQCMAAwHwYDVR0jBBgwFoAUd8K4UJpndnaxLcKG0IOgfqZ+ukswIQYDVR0g
+        BBowGDAMBgorBgEEAdZ5AgUDMAgGBmeBDAECAjAxBgNVHR8EKjAoMCagJKAihiBo
+        dHRwOi8vY3JsLnBraS5nb29nL0dUU0dJQUczLmNybDANBgkqhkiG9w0BAQsFAAOC
+        AQEAbi8VuaNKx/otlEsrZ8+A0VbNvjOaQqqYodBbcu+/0MjGPLn4H9TKGVjsFtbY
+        piod3iX72Pg7X1WoQIoJUcybmZk64jocUBZOdZkZe2bjTAf6JQg9v7jh1pXgsEvv
+        UJ/86PBm6HsWAM2oMcIEOYO1e0/X0wJc1TogJn5/jTMA6u6JF4aQCLe1izgCSTeY
+        1efJiOYjVLfh/24+72yNpbS1z7whRVEHreXe2j2CrSiXnk60Wp7SZ88Ws1G7YPqa
+        Xqs1gJBb41sPz2dnR1vVIurciU6AD5nROQhhVWRF789Qf92gotfvvQDGrIcX2igm
+        j+CcQEW13qYWL+H8gReGc+vsvg==
+        -----END CERTIFICATE-----

tests/integration/states/test_x509.py

@@ -0,0 +1,66 @@
+# -*- coding: utf-8 -*-
+from __future__ import absolute_import, unicode_literals
+import os
+import logging
+from tests.support.paths import BASE_FILES
+from tests.support.case import ModuleCase
+from tests.support.unit import skipIf
+from tests.support.mixins import SaltReturnAssertsMixin
+
+import salt.utils.files
+
+try:
+    import M2Crypto  # pylint: disable=W0611
+    HAS_M2CRYPTO = True
+except ImportError:
+    HAS_M2CRYPTO = False
+
+
+log = logging.getLogger(__name__)
+
+
+class x509Test(ModuleCase, SaltReturnAssertsMixin):
+
+    def tearDown(self):
+        paths = [
+            '/test-49027.crt',
+            '/test-ca-49008.key',
+            '/test-ca-49008.crt',
+        ]
+        for path in paths:
+            try:
+                os.remove(path)
+            except Exception:
+                pass
+
+    @staticmethod
+    def get_cert_lines(path):
+        lines = []
+        started = False
+        with salt.utils.files.fopen(path, 'rb') as fp:
+            for line in fp:
+                if line.find(b'-----BEGIN CERTIFICATE-----') != -1:
+                    started = True
+                    continue
+                if line.find(b'-----END CERTIFICATE-----') != -1:
+                    break
+                if started:
+                    lines.append(line.strip())
+        return lines
+
+    @skipIf(not HAS_M2CRYPTO, 'Skip when no M2Crypto found')
+    def test_issue_49027(self):
+        expected = self.get_cert_lines(os.path.join(BASE_FILES, 'issue-49027.sls'))
+        started = False
+        ret = self.run_function('state.sls', ['issue-49027'])
+        log.warn("ret = %s", repr(ret))
+        self.assertSaltTrueReturn(ret)
+        self.assertEqual(expected, self.get_cert_lines('/test-49027.crt'))
+
+    @skipIf(not HAS_M2CRYPTO, 'Skip when no M2Crypto found')
+    def test_issue_49008(self):
+        ret = self.run_function('state.sls', ['issue-49008'])
+        log.warn("ret = %s", repr(ret))
+        self.assertSaltTrueReturn(ret)
+        self.assertTrue(os.path.exists('/test-ca-49008.key'))
+        self.assertTrue(os.path.exists('/test-ca-49008.crt'))