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 can't handle managed_private_key #39608

Closed
4s1 opened this issue Feb 23, 2017 · 21 comments
Closed

x509 certificate_managed can't handle managed_private_key #39608

4s1 opened this issue Feb 23, 2017 · 21 comments
Labels
Bug broken, incorrect, or confusing behavior expected-behavior intended functionality Platform Relates to OS, containers, platform-based utilities like FS, system based apps severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Milestone

Comments

@4s1
Copy link

4s1 commented Feb 23, 2017

Description of Issue/Question

The x509.certificate_managed state doesn't work when it's configured to handle it's own private key as explained in the docs or shown in the example and it's not existing yet

Setup

# self signed cert
/etc/nginx/certs/localhost.cert.pem:
  x509.certificate_managed:
    - signing_private_key: /etc/nginx/private/localhost.key.pem
    - CN: localhost
    - days_valid: 365
    - days_remaining: 90
    - managed_private_key:
       name: /etc/nginx/private/localhost.key.pem
       bits: 2048
    - require:
       - pkg: python-m2crypto

Steps to Reproduce Issue

  1. configure state as shown above
  2. make sure key file doesn't exist, but path is writable
  3. and apply state.
         ID: /etc/nginx/certs/localhost.cert.pem
    Function: x509.certificate_managed
      Result: False
     Comment: An exception occurred in this state: Traceback (most recent call last):
                File "/usr/lib/python2.7/site-packages/salt/state.py", line 1745, in call
                  **cdata['kwargs'])
                File "/usr/lib/python2.7/site-packages/salt/loader.py", line 1702, in wrapper
                  return f(*args, **kwargs)
                File "/usr/lib/python2.7/site-packages/salt/states/x509.py", line 473, in certificate_managed
                  new = __salt__['x509.create_certificate'](testrun=True, **kwargs)
                File "/usr/lib/python2.7/site-packages/salt/modules/x509.py", line 1484, in create_certificate
                  passphrase=kwargs['signing_private_key_passphrase'])
                File "/usr/lib/python2.7/site-packages/salt/modules/x509.py", line 679, in get_public_key
                  text = get_pem_entry(text)
                File "/usr/lib/python2.7/site-packages/salt/modules/x509.py", line 472, in get_pem_entry
                  raise salt.exceptions.SaltInvocationError(errmsg)
              SaltInvocationError: PEM text not valid:
              /etc/nginx/private/localhost.key.pem
     Started: 16:29:53.585609
    Duration: 99.44 ms
     Changes:   

Versions Report

Salt Version:                                                                                                                                                                                      
           Salt: 2016.11.2                                                                                                                                                                         
                                                                                                                                                                                                   
Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.9.5
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: 0.24.0
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.8
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
         pygit2: Not Installed
         Python: 2.7.13 (default, Dec 21 2016, 07:16:46)
   python-gnupg: Not Installed
         PyYAML: 3.12
          PyZMQ: 16.0.2
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.4.2
            ZMQ: 4.2.0
 
System Versions:
           dist:   
        machine: x86_64
        release: 4.7.4-1-ARCH
         system: Linux
        version: Not Installed
@gtmanfred gtmanfred added Bug broken, incorrect, or confusing behavior severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around Platform Relates to OS, containers, platform-based utilities like FS, system based apps TEAM Platform P3 Priority 3 labels Feb 23, 2017
@gtmanfred gtmanfred added this to the Approved milestone Feb 23, 2017
@gtmanfred
Copy link
Contributor

Thanks for reporting this, I have replicated this issue.

Thanks,
Daniel

@blbradley
Copy link
Contributor

Confirming on 2016.11.3

@tazaki
Copy link

tazaki commented Mar 8, 2017

Also have this issue on salt 2016.11.3 (Carbon)

@dawidmalina
Copy link

We also have this issue :(

@etiennepouliot
Copy link

I you want to bypass this error, make the key in another config and include it the in cert ca. So the example in the doc would become :

/etc/pki/ca.key:                                                
  x509.private_key_managed:                                     
    - bits: 4096                                                
    - backup: True                                              
    - require:                                                  
      - file: /etc/pki                                          
                                                                
/etc/pki/ca.crt:                                                
  x509.certificate_managed:                                     
    - signing_private_key: /etc/pki/ca.key                      
    - CN: ca.example.com                                        
    - C: US                                                     
    - ST: Utah                                                  
    - L: Salt Lake City                                         
    - basicConstraints: "critical CA:true"                      
    - keyUsage: "critical cRLSign, keyCertSign"                 
    - subjectKeyIdentifier: hash                                
    - authorityKeyIdentifier: keyid,issuer:always               
    - days_valid: 3650                                          
    - days_remaining: 0                                         
    - backup: True                                              
    - require:                                                  
      - file: /etc/pki                                          
      - x509: /etc/pki/ca.key    
                               `

@tazaki
Copy link

tazaki commented Mar 9, 2017

This workaround worked for me. Thanks @etiennepouliot !

@jerrykan
Copy link
Contributor

jerrykan commented Nov 7, 2017

I can confirm that this is still a bug in v2017.7.2

The workaround works fine though. Maybe removing the managed_private_key option might be the easiest path forward seeing as though it has be broken for so long (at least v2016.11.2).

@rnickle
Copy link

rnickle commented Jun 20, 2018

Still seeing the bug, and the workaround works. Thanks for pointing out how to do the require with x509: Etienne, I was trying to work that out when I found your report.

Salt Version:
Salt: 2018.3.1

Dependency Versions:
cffi: Not Installed
cherrypy: 3.5.0
dateutil: 2.4.2
docker-py: Not Installed
gitdb: 0.6.4
gitpython: 1.0.1
ioflo: Not Installed
Jinja2: 2.8
libgit2: Not Installed
libnacl: Not Installed
M2Crypto: 0.21.1
Mako: 1.0.3
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.12 (default, Dec 4 2017, 14:50:18)
python-gnupg: 0.3.8
PyYAML: 3.11
PyZMQ: 15.2.0
RAET: Not Installed
smmap: 0.9.0
timelib: Not Installed
Tornado: 4.2.1
ZMQ: 4.1.4

System Versions:
dist: Ubuntu 16.04 xenial
locale: UTF-8
machine: x86_64
release: 4.4.0-96-generic
system: Linux
version: Ubuntu 16.04 xenial

@ymasson
Copy link
Contributor

ymasson commented Oct 16, 2018

Hi

Still present in 2018.3.2

Salt Version:
           Salt: 2018.3.2
 
Dependency Versions:
           cffi: Not Installed
       cherrypy: 3.5.0
       dateutil: 2.5.3
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.9.4
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: 0.24.0
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.8
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.13 (default, Sep 26 2018, 18:42:22)
   python-gnupg: Not Installed
         PyYAML: 3.12
          PyZMQ: 16.0.2
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.4.3
            ZMQ: 4.2.1
 
System Versions:
           dist: debian 9 
         locale: UTF-8
        machine: x86_64
        release: 4.9.0-8-amd64
         system: Linux
        version: debian 9

@Poil
Copy link

Poil commented Nov 29, 2018

Hi

Still present in 2018.3.3, and with the fix I have another error, due to Python 3

          ID: /etc/pki/ca.crt
    Function: x509.certificate_managed
      Result: False
     Comment: An exception occurred in this state: Traceback (most recent call last):
                File "/usr/lib/python3.4/site-packages/salt/state.py", line 1913, in call
                  **cdata['kwargs'])
                File "/usr/lib/python3.4/site-packages/salt/loader.py", line 1898, in wrapper
                  return f(*args, **kwargs)
                File "/usr/lib/python3.4/site-packages/salt/states/x509.py", line 554, in certificate_managed
                  file_args['contents'] += certificate
              TypeError: Can't convert 'bytes' object to str implicitly
     Started: 10:50:51.096927
    Duration: 21.669 ms
     Changes:   
Salt Version:
           Salt: 2018.3.3
 
Dependency Versions:
           cffi: 1.11.5
       cherrypy: unknown
       dateutil: Not Installed
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.10
        libgit2: 0.26.5
        libnacl: Not Installed
       M2Crypto: 0.28.2
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: 2.18
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: 0.26.4
         Python: 3.4.9 (default, Aug 14 2018, 21:28:57)
   python-gnupg: Not Installed
         PyYAML: 3.11
          PyZMQ: 15.3.0
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.4.2
            ZMQ: 4.1.4
 
System Versions:
           dist: centos 7.5.1804 Core
         locale: UTF-8
        machine: x86_64
        release: 3.10.0-862.14.4.el7.x86_64
         system: Linux
        version: CentOS Linux 7.5.1804 Core

@gtmanfred
Copy link
Contributor

@saltstack/team-triage ping

@Ch3LL
Copy link
Contributor

Ch3LL commented Dec 5, 2018

ping @Poil any chance you can try the head of 2018.3? I am thinking that these fixes might resolve the issue you pointed out: #49782

#49782

and possibly duplicate of #50637

@Poil
Copy link

Poil commented Dec 5, 2018

Yep they fixed it; I forgot to reply here, but that's why I close #50676

@rnickle
Copy link

rnickle commented Dec 5, 2018 via email

@Poil
Copy link

Poil commented Dec 5, 2018

I've also push another fix because today the certs are renewed at each run #50734

@OrangeDog
Copy link
Contributor

Seems to still be broken in 2018.3.4.

Salt Version:
           Salt: 2018.3.4

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.6.1
      docker-py: Not Installed
          gitdb: 2.0.3
      gitpython: 2.1.8
          ioflo: Not Installed
         Jinja2: 2.10
        libgit2: 0.26.0
        libnacl: Not Installed
       M2Crypto: 0.32.0
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: 0.26.2
         Python: 3.6.7 (default, Oct 22 2018, 11:32:17)
   python-gnupg: 0.4.1
         PyYAML: 3.12
          PyZMQ: 16.0.2
           RAET: Not Installed
          smmap: 2.0.3
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.2.5

System Versions:
           dist: Ubuntu 18.04 bionic
         locale: ISO-8859-1
        machine: x86_64
        release: 4.15.0-46-generic
         system: Linux
        version: Ubuntu 18.04 bionic

@rnickle
Copy link

rnickle commented Mar 13, 2019 via email

glynnforrest added a commit to glynnforrest/salt that referenced this issue May 7, 2019
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 (saltstack#41858)
* New certificates are created every run (saltstack#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 saltstack#39608 (comment)
`managed_private_key` has not worked since at least v2016.11.2.
@stale
Copy link

stale bot commented Jan 8, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

@stale stale bot added the stale label Jan 8, 2020
@OrangeDog
Copy link
Contributor

unstale

@stale
Copy link

stale bot commented Jan 8, 2020

Thank you for updating this issue. It is no longer marked as stale.

@stale stale bot removed the stale label Jan 8, 2020
glynnforrest added a commit to glynnforrest/salt that referenced this issue Apr 10, 2020
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 saltstack#52180, saltstack#39608, saltstack#41858 and others:

* Error messages from the x509 module calls are written directly to
the certificate file - fixed, the certificate file is only created
when the x509 module calls succeed.
* Certificates are created when no changes are required - fixed, the
comparison logic has been updated.

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 saltstack#39608 (comment)
`managed_private_key` has not worked since at least v2016.11.2.
glynnforrest added a commit to glynnforrest/salt that referenced this issue Apr 10, 2020
It doesn't work and hasn't for a long time, see
saltstack#39608 (comment).
Throwing a SaltInvocationError with a clear path to resolve the issue
is more useful.
@sagetherage sagetherage removed the P3 Priority 3 label Jun 3, 2020
@OrangeDog
Copy link
Contributor

@sagetherage this can be closed now: As Designed

@sagetherage sagetherage added the expected-behavior intended functionality label Jun 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior expected-behavior intended functionality Platform Relates to OS, containers, platform-based utilities like FS, system based apps severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Projects
None yet
Development

No branches or pull requests