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.

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

x509.certificate_managed state fails under py3 m2crypto with "int too large" #49008

Open
lachlanmunro opened this issue Aug 8, 2018 · 9 comments

Comments

Projects
None yet
4 participants
@lachlanmunro
Copy link

commented Aug 8, 2018

Description of Issue/Question

x509 certificate_managed fails on generated key length.

We have something like this working under py2, this change took place after an upgrade to py3 and compiling/adding m2crypto to it on windows.

Setup

c:/test/test-ca.crt:
  x509.certificate_managed:
    - signing_private_key: c:/test/test-ca.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: c:/test/test-ca.key

c:/test/test-ca.key:
  x509.private_key_managed:
    - bits: 4096
    - backup: True

Steps to Reproduce Issue

Running the above state results in:

local:
----------
          ID: c:/restic/test-ca.key
    Function: x509.private_key_managed
      Result: True
     Comment: File c:/restic/test-ca.key updated
     Started: 16:32:14.495656
    Duration: 1886.762 ms
     Changes:
              ----------
              new:
                  New private key generated
----------
          ID: c:/restic/test-ca.crt
    Function: x509.certificate_managed
      Result: False
     Comment: An exception occurred in this state: Traceback (most recent call last):
                File "C:\salt\bin\lib\site-packages\salt\state.py", line 1905, in call
                  **cdata['kwargs'])
                File "C:\salt\bin\lib\site-packages\salt\loader.py", line 1830, in wrapper
                  return f(*args, **kwargs)
                File "C:\salt\bin\lib\site-packages\salt\states\x509.py", line 496, in certificate_managed
                  new = __salt__['x509.create_certificate'](testrun=True, **kwargs)
                File "C:\salt\bin\lib\site-packages\salt\modules\x509.py", line 1440, in create_certificate
                  cert.set_serial_number(serial_number)
                File "C:\salt\bin\lib\site-packages\M2Crypto\X509.py", line 593, in set_serial_number
                  return m2.asn1_integer_set(asn1_integer, serial)
              OverflowError: Python int too large to convert to C long
     Started: 16:32:16.383417
    Duration: 5.005 ms
     Changes:

Versions Report

Salt Version:
           Salt: 2018.3.2

Dependency Versions:
           cffi: 1.10.0
       cherrypy: 10.2.1
       dateutil: 2.6.1
      docker-py: Not Installed
          gitdb: 2.0.3
      gitpython: 2.1.3
          ioflo: Not Installed
         Jinja2: 2.9.6
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: 0.30.1
           Mako: 1.0.6
   msgpack-pure: Not Installed
 msgpack-python: 0.4.8
   mysql-python: Not Installed
      pycparser: 2.17
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 3.5.3 (v3.5.3:1880cb95a742, Jan 16 2017, 16:02:32) [MSC v.1900 64 bit (AMD64)]
   python-gnupg: 0.4.1
         PyYAML: 3.12
          PyZMQ: 16.0.3
           RAET: Not Installed
          smmap: 2.0.3
        timelib: 0.2.4
        Tornado: 4.5.1
            ZMQ: 4.1.6

System Versions:
           dist:
         locale: cp1252
        machine: AMD64
        release: 10
         system: Windows
        version: 10 10.0.17134 SP0 Multiprocessor Free
@Ch3LL

This comment has been minimized.

Copy link
Contributor

commented Aug 8, 2018

can you clarify how you are installing m2crypto python3 on windows to help replicate the issue?

@Ch3LL Ch3LL added the Info Needed label Aug 8, 2018

@Ch3LL Ch3LL added this to the Blocked milestone Aug 8, 2018

@mts-avco

This comment has been minimized.

Copy link

commented Aug 8, 2018

Specifically, as per #48994

@Ch3LL

This comment has been minimized.

Copy link
Contributor

commented Aug 8, 2018

ping @dwoz i know you have tried working with upstream to try to get these packages built. any chance you can see if the instructions @mts-avco gives on how they compiled the package is the correct way to manually compile this package?

And if you have any initial insight as to if this error is indicative of a package build issue or salt code issue.

@Ch3LL Ch3LL added Pending Discussion and removed Info Needed labels Aug 8, 2018

@dwoz

This comment has been minimized.

Copy link
Contributor

commented Aug 8, 2018

@Ch3LL @mts-avco Yes, I will try to reproduce this.

@lachlanmunro

This comment has been minimized.

Copy link
Author

commented Aug 9, 2018

This was walked into while trying to create a test case for an issue around a mine supplied certificate to x509.pem_managed.

----------
          ID: C:\kubectl/client-ca.crt
    Function: x509.pem_managed
      Result: False
     Comment: An exception occurred in this state: Traceback (most recent call last):
                File "C:\salt\bin\lib\site-packages\salt\state.py", line 1905, in call
                  **cdata['kwargs'])
                File "C:\salt\bin\lib\site-packages\salt\loader.py", line 1830, in wrapper
                  return f(*args, **kwargs)
                File "C:\salt\bin\lib\site-packages\salt\states\x509.py", line 712, in pem_managed
                  return __states__['file.managed'](**file_args)
                File "C:\salt\bin\lib\site-packages\salt\loader.py", line 1830, in wrapper
                  return f(*args, **kwargs)
                File "C:\salt\bin\lib\site-packages\salt\states\file.py", line 2274, in managed
                  elif isinstance(use_contents, six.string_types) and str('\0') in use_contents:
              TypeError: a bytes-like object is required, not 'str'
     Started: 10:03:58.630975
    Duration: 2.999 ms
     Changes:

A simpler (crt is directly supplied instead of via mine) test case is at #49027 .

We are worried that that the issue above (and potentially some of the others seen) are related to us having an incorrect build of m2crypto and/or issues around 32bit libraries on a 64bit system and thus are false positives. Will do some further investigation our end/see any observations you have before adding any additional issues.

@mts-avco

This comment has been minimized.

Copy link

commented Aug 9, 2018

My local py2.7-win64 build works perfectly with the above test state against a py2 salt minion, so I think that rules out win64 as the cause - I believe that leaves either my py3 build or the py3 support in m2crypto.

@dwoz

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2018

I've opened an issue for M2Crypto and will submit a patch there as well as a work around for the salt x509 module to use for now.

https://gitlab.com/m2crypto/m2crypto/issues/232

@dwoz

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2018

dwoz added a commit to dwoz/salt that referenced this issue Sep 7, 2018

Fix multiple issues in x509 module and state
Adding a regression test for saltstack#49008 and patching the x509 module and
state to make the test pass.

dwoz added a commit to dwoz/salt that referenced this issue Sep 7, 2018

Fix multiple issues in x509 module and state
Adding a regression test for saltstack#49008 and patching the x509 module and
state to make the test pass.

dwoz added a commit to dwoz/salt that referenced this issue Sep 11, 2018

Fix multiple issues in x509 module and state
Adding a regression test for saltstack#49008 and patching the x509 module and
state to make the test pass.
@lachlanmunro

This comment has been minimized.

Copy link
Author

commented Oct 2, 2018

Thanks, will try to schedule some time to test the fixes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.