saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Get access to the Salt software package repository here:
https://repo.saltproject.io/
Apache License 2.0
14.1k stars 5.47k forks source link

saltstack x509 incompatible to m2crypto 0.26.0 #40490

Closed alxwr closed 5 years ago

alxwr commented 7 years ago

Description of Issue/Question

----------
          ID: /var/db/predefined_crypto/x509/vpn/client/meanmachine2.crt
    Function: x509.certificate_managed
      Result: False
     Comment: An exception occurred in this state: Traceback (most recent call last):
                File "/usr/local/lib/python2.7/site-packages/salt/state.py", line 1746, in call
                  **cdata['kwargs'])
                File "/usr/local/lib/python2.7/site-packages/salt/loader.py", line 1703, in wrap
per
                  return f(*args, **kwargs)
                File "/usr/local/lib/python2.7/site-packages/salt/states/x509.py", line 464, in 
certificate_managed
                  current = __salt__['x509.read_certificate'](certificate=name)
                File "/usr/local/lib/python2.7/site-packages/salt/modules/x509.py", line 549, in
 read_certificate
                  'Subject': _parse_subject(cert.get_subject()),
                File "/usr/local/lib/python2.7/site-packages/salt/modules/x509.py", line 333, in
 _parse_subject
                  val = getattr(subject, nid_name)
                File "/usr/local/lib/python2.7/site-packages/M2Crypto/X509.py", line 320, in __g
etattr__
                  return util.py3str(m2.x509_name_by_nid(self.x509_name, self.nid[attr]))
                File "/usr/local/lib/python2.7/site-packages/M2Crypto/util.py", line 66, in py3s
tr
                  raise TypeError('No string argument provided')
              TypeError: No string argument provided
     Started: 15:14:15.821837
    Duration: 1.387 ms
     Changes:

Fails with M2Crypto 0.26.0. Works with M2Crypto 0.25.1.

I narrowed the failure down to m2.x509_name_by_nid(self.x509_name, self.nid[attr]) (in M2Crypto/X509.py) returning None, but I've obviously got no idea why. :-)

Tested on FreeBSD 10.3.

Setup

# -- certs.sls

/path/to/HOST.key
  x509.private_key_managed:
    - bits: 4096

/path/to/HOST.crt
  x509.certificate_managed:
    - ca_server: FQDN
    - signing_policy: mypol
    - public_key: /path/to/HOST.key
    - CN: {{ grains['fqdn'] }}
    - days_remaining: 15
    - backup: True
pip install M2Crypto==0.26.0

Steps to Reproduce Issue

salt 'HOST' state.highstate

Versions Report

Salt Version:
           Salt: 2016.11.3

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.6.0
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.8
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: 0.26.0
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.7
   mysql-python: 1.2.5
      pycparser: Not Installed
       pycrypto: 2.6.1
         pygit2: Not Installed
         Python: 2.7.13 (default, Jan  3 2017, 01:35:50)
   python-gnupg: Not Installed
         PyYAML: 3.11
          PyZMQ: 16.0.2
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.4.2
            ZMQ: 4.1.5

System Versions:
           dist:
        machine: amd64
        release: 10.3-RELEASE-p11
         system: FreeBSD
        version: Not Installed
Ch3LL commented 7 years ago

@alxwr looks like i'm able to replicate this. Here is a docker container to easily replicate the issue:

  1. docker run -it -v /home/ch3ll/git/salt/:/testing/ ch3ll/issues:40490 salt-call --local state.sls test -ldebug (where /home/ch3ll/git/salt is a local cloned git repo of salt)

Looks like that typerror was added here in m2crypto. We will need to get this fixed. thanks

Note this is also broken in 2016.3

MorphBonehunter commented 7 years ago

Since 2017-07-13 M2Crypto 0.26.0 is the actual version on Arch Linux. So Arch Users are now also effected of this.

ndobbs commented 7 years ago

I'm running into this same issue on ubuntu 14.04 and 16.04 minions with m2crypto 0.26.0 installed.

AFriemann commented 7 years ago

@Ch3LL Is there an issue on the m2crypto gitlab? I couldn't find anything related. I reverted the change locally and it seems to work again, though I would argue the new behaviour seems more "correct" to me than before.

AFriemann commented 7 years ago

Ok nevermind. I created a pull request but I can't say I'm too happy with it.

4s1 commented 7 years ago

I narrowed the failure down to m2.x509_name_by_nid(self.x509_name, self.nid[attr]) (in M2Crypto/X509.py) returning None, but I've obviously got no idea why. :-) I traced it down a bit further.

A X509_NAME Object has a list of valid nids (attribute ID<>String) wich is prepopulated.

Salt's x509 module blindly loops over this list of valid IDs and tries to read every attribute (even if it is not available, i.e. results in a None) https://github.com/saltstack/salt/blob/0931281ebd1ca794b27bf7dc4e918efd3a05c612/salt/modules/x509.py#L330

m2crypo's getattr() assumes the requested attribute is available and can be converted to a string, otherwise the given exception is thrown, because None is not a valid string

To me it looks lik this can be fixed in two ways:

  1. either m2crypto changes their getattr to allow None to be passed, or
  2. salt's x509 module uses another iterator to loop only over existing attributes (instead of known Attribute IDs)

BTW: Am I the only one getting a None when reading SerialNumber (105)?

oliver-dungey commented 6 years ago

Same problem for me with CentOS6.9:

Salt Version:
           Salt: 2017.7.1

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: Not Installed
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.8.1
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: 0.26.4
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.13 (default, Jul 12 2017, 17:32:34)
   python-gnupg: Not Installed
         PyYAML: 3.11
          PyZMQ: 14.5.0
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.0.5

System Versions:
           dist: centos 6.9 Final
         locale: UTF-8
        machine: x86_64
        release: 2.6.32-696.10.3.el6.x86_64
         system: Linux
        version: CentOS 6.9 Final
4s1 commented 6 years ago

Any news on this issue? Patch #42760 yields to a newly created certificate everytime state.apply is called, even if no param changed and the certificate is still within it's designated lifetime.

konfiot commented 6 years ago

Any news ?

Ch3LL commented 6 years ago

So it seems the PR has created another issue @4s1 ? Can you provide a test case and which salt version you were running and how you patched your minion?

MorphBonehunter commented 6 years ago

I would say this is fixed at least for 2017.7.5. and m2crypto 0.29.0 Testcase:

test_key:
    x509.private_key_managed:
        - name: /tmp/test.key
        - bits: 4096
        - passphrase: 'testpass'
        - cipher: aes_256_cbc

test_cert:
    x509.certificate_managed:
        - name: /tmp/test.crt
        - signing_private_key: /tmp/test.key
        - signing_private_key_passphrase: 'testpass'
        - CN: test.example.com
        - basicConstraints: "critical CA:true"
        - keyUsage: "critical cRLSign, keyCertSign"
        - subjectKeyIdentifier: hash
        - authorityKeyIdentifier: keyid,issuer:always
        - days_valid: 3650
        - days_remaining: 0
        - backup: True
        - require:
            - x509: test_key

1st run:

salt-call state.apply testcase 
.....................................................................................................................++
.................++
local:
----------
          ID: test_key
    Function: x509.private_key_managed
        Name: /tmp/test.key
      Result: True
     Comment: File /tmp/test.key updated
     Started: 09:03:03.646517
    Duration: 2306.363 ms
     Changes:   Invalid Changes data: New private key generated
----------
          ID: test_cert
    Function: x509.certificate_managed
        Name: /tmp/test.crt
      Result: True
     Comment: File /tmp/test.crt updated
     Started: 09:03:05.953599
    Duration: 58.871 ms
     Changes:   
              ----------
              Certificate:
                  ----------
                  New:
                      ----------
                      Issuer:
                          ----------
                          CN:
                              test.example.com
                      Issuer Hash:
                          30:8F:9F:79
                      Key Size:
                          4096
                      MD5 Finger Print:
                          6B:72:45:02:B5:69:36:FC:1B:72:A8:2F:2B:65:6C:A9
                      Not After:
                          2028-04-20 07:03:05
                      Not Before:
                          2018-04-23 07:03:05
                      Public Key:
                          -----BEGIN PUBLIC KEY-----
                          MIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIICCgKCAgEAqKKkovnEFNiu6O+qgT4t
                          cQh3oHo/tKvoyE6RRkKXfSgPM2qTfjVXRBRnRpJAbiMIfvm6WlHRr18dvUFkOlqM
                          K5MXLhLK2uvauXQDQNCh+S2sfvrsuQzeRfb1P9HFZeGhk52vF7rnFZN+ctpS8EJ6
                          /Mx9c73YUlH/UN7hpSLvlLim1gR32D4ZXsiCf3p/1V2K5uts4updG2owTMY78Wbo
                          u2n8kd8oBsKmsPdUO1y3MtJRsbGmJCqzF/PPyTP7jGslTyJp5Ty6yMqWfLHNCszW
                          ay9+k461+baiHoEKk7sxe7pePCCuBu6sGH8Yr7QOTjV03MG5nES19iiQ2IFSjIis
                          KBM8wFzL9lnLiYJ1kqsAod8kxnpIzJQW+faYT+llrHxYhBr15Q/2TddUwbl4zG7e
                          pGzYKDXueXwyWDv+6O/Crr3jQOjLEneuLPTgKDOk9hJwp+TENRDHZPfDPZKrTSay
                          XVSwFrLSv9r7fqqX0pCl/tboteZa9xZf/q7rQso8SS9sPZ6EXJWHA4PdJNxOlmXz
                          CeVCoYjbAOwZxoGbQXY97DMEkpBu/rQ9I8I7MEQke/Ubra8uYwG07hTYfitpaJPl
                          3qbo04nI5xfaFs5P2xbiK/HdEDT+yfxmpvJ2m+HO2HFrsCmpBD5fHCNnDkWDfvpS
                          LhmK6GLpfEmq1XrZOAkIJwMCAwEAAQ==
                          -----END PUBLIC KEY-----
                      SHA-256 Finger Print:
                          0E:89:7C:F2:EA:F1:6F:92:F7:AC:0E:62:B1:39:BC:5A:98:B7:8B:16:49:AB:11:9C:EF:11:FB:AB:57:EA:04:43
                      SHA1 Finger Print:
                          17:4F:A6:23:20:84:5E:8A:B2:0D:92:8B:82:5C:CC:8A:8B:E6:25:70
                      Serial Number:
                          4F:8D:82:E1:09:DC:64:55
                      Subject:
                          ----------
                          CN:
                              test.example.com
                      Subject Hash:
                          30:8F:9F:79
                      Version:
                          3
                      X509v3 Extensions:
                          ----------
                          basicConstraints:
                              critical CA:TRUE
                          keyUsage:
                              critical Certificate Sign, CRL Sign
                          subjectKeyIdentifier:
                              1F:41:C3:59:5E:8F:4C:43:DD:25:22:35:92:F6:EE:7F:6F:2D:70:37
                          authorityKeyIdentifier:
                              keyid:1F:41:C3:59:5E:8F:4C:43:DD:25:22:35:92:F6:EE:7F:6F:2D:70:37
                              DirName:/CN=test.example.com
                              serial:4F:8D:82:E1:09:DC:64:55
                  Old:
                      /tmp/test.crt does not exist.

Summary for local
------------
Succeeded: 2 (changed=2)
Failed:    0
------------
Total states run:     2
Total run time:   2.365 s

consecutive runs:

salt-call state.apply testcase 
local:
----------
          ID: test_key
    Function: x509.private_key_managed
        Name: /tmp/test.key
      Result: True
     Comment: File /tmp/test.key is in the correct state
     Started: 09:04:49.601908
    Duration: 33.764 ms
     Changes:   
----------
          ID: test_cert
    Function: x509.certificate_managed
        Name: /tmp/test.crt
      Result: True
     Comment: File /tmp/test.crt is in the correct state
     Started: 09:04:49.636392
    Duration: 19.716 ms
     Changes:   

Summary for local
------------
Succeeded: 2
Failed:    0
------------
Total states run:     2
Total run time:  53.480 ms

So the cert is also created once and not every run.

I can not test this for 2018.3 because of #47236

alxwr commented 5 years ago

Works for me using:

Supposedly still doesn't work with 0.26.0, but I think we can close this issue. :-)