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.

Sign up for GitHub

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

Jump to bottom

x509.pem_managed, two failure modes: "TypeError" and "PEM text not valid" #50637

Closed
rnickle opened this issue Nov 26, 2018 · 7 comments
Closed

x509.pem_managed, two failure modes: "TypeError" and "PEM text not valid" #50637

rnickle opened this issue Nov 26, 2018 · 7 comments
Labels
Duplicate Duplicate of another issue or PR - will be closed fixed-pls-verify fix is linked, bug author to confirm fix
Milestone
Blocked

Comments

@rnickle
Copy link

rnickle commented Nov 26, 2018

Description of Issue/Question

Using the certificate authority based on the Salt mine described in:

https://docs.saltstack.com/en/latest/ref/states/all/salt.states.x509.html

I would expect simple Jinja expansion to be correctly processed as input to pem_managed, but I receive a "TypeError: a bytes-like object is required, not 'str'" error.

To work around this, I tried converting that input into ascii with encode(), but that causes x509.get_pem_entry to fail the certificate because it doesn't pass a regex check for the certificate formatting (I'm hypothesizing that the expanded string includes the u' type prefix?)

Setup

The salt mine is working correctly, and entries can be retrieved, I can even cut + paste that certificate and use openssl to validate it.

sudo salt 'ubbt3dt3.*' mine.get 'salt.holycross.edu' x509.get_pem_entries | head
ubbt3dt3.holycross.edu:
    ----------
    salt.holycross.edu:
        ----------
        /etc/pki/ca_test.crt:
            -----BEGIN CERTIFICATE-----
            MIIF8TCCA9mgAwIBAgIIerDv/onLAJswDQYJKoZIhvcNAQELBQAwUzELMAkGA1UE
            BhMCVVMxGDAWBgNVBAMMD2RvY2tlci10ZW1wbGF0ZTESMBAGA1UEBwwJV29yY2Vz
            dGVyMRYwFAYDVQQIDA1NYXNzYWNodXNldHRzMB4XDTE4MDcwNjAxMjgyM1oXDTI4
            MDcwMzAxMjgyM1owUzELMAkGA1UEBhMCVVMxGDAWBgNVBAMMD2RvY2tlci10ZW1w

I'm using this formula to fetch the certificate from the Salt mine and install it in my /etc/ssl/certs directory:

{% set mine_ca_cert = 'ca_test.crt' %}
{% set mine_ca_path = '/etc/pki/' + mine_ca_cert %}
{% set ca_cert = '/etc/ssl/certs/' + mine_ca_cert %}
{% set ca_host = 'salt.holycross.edu' %}
{% set mine_fetch = salt['mine.get'](ca_host,'x509.get_pem_entries')[ca_host][mine_ca_path]|replace('\n','')%}
{% set mine_fetch_ascii = mine_fetch.encode("ascii","ignore") %}

TLS_CA_Cert:
  x509.pem_managed:
    - name: {{ca_cert}}
    - text: {{mine_fetch}}
    - user: root
    - group: root
    - show_changes: False
    - encoding: utf-8
    - encoding_errors: ignore
    - mode: 640

Steps to Reproduce Issue

[PROFILE ] Time (in seconds) to render '/var/cache/salt/minion/files/base/Test/tls.sls' using 'yaml' renderer: 0.011218786239624023
[DEBUG   ] LazyLoaded config.option
[DEBUG   ] LazyLoaded x509.get_pem_entry
[DEBUG   ] LazyLoaded x509.pem_managed
[INFO    ] Running state [/etc/ssl/certs/ca_test.crt] at time 10:36:07.024470
[INFO    ] Executing state x509.pem_managed for [/etc/ssl/certs/ca_test.crt]
[DEBUG   ] LazyLoaded file.managed
[ERROR   ] An exception occurred in this state: Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/salt/state.py", line 1913, in call
    **cdata['kwargs'])
  File "/usr/lib/python3/dist-packages/salt/loader.py", line 1898, in wrapper
    return f(*args, **kwargs)
  File "/usr/lib/python3/dist-packages/salt/states/x509.py", line 719, in pem_managed
    return __states__['file.managed'](**file_args)
  File "/usr/lib/python3/dist-packages/salt/loader.py", line 1898, in wrapper
    return f(*args, **kwargs)
  File "/usr/lib/python3/dist-packages/salt/states/file.py", line 2421, in managed
    elif isinstance(use_contents, six.string_types) and str('\0') in use_contents:
TypeError: a bytes-like object is required, not 'str'

[INFO    ] Completed state [/etc/ssl/certs/ca_test.crt] at time 10:36:07.040641 (duration_in_ms=16.172)
[DEBUG   ] File /var/cache/salt/minion/accumulator/140347082302744 does not exist, no need to cleanup
[DEBUG   ] LazyLoaded state.check_result
[DEBUG   ] Initializing new AsyncZeroMQReqChannel for ('/etc/salt/pki/minion', 'ubbt3dt3.holycross.edu', 'tcp://10.108.193.179:4506', 'aes')
[DEBUG   ] Initializing new AsyncAuth for ('/etc/salt/pki/minion', 'ubbt3dt3.holycross.edu', 'tcp://10.108.193.179:4506')
[DEBUG   ] Connecting the Minion to the Master URI (for the return server): tcp://10.108.193.179:4506
[DEBUG   ] Trying to connect to: tcp://10.108.193.179:4506
[DEBUG   ] LazyLoaded highstate.output
local:
----------
          ID: TLS_CA_Cert
    Function: x509.pem_managed
        Name: /etc/ssl/certs/ca_test.crt
      Result: False
     Comment: An exception occurred in this state: Traceback (most recent call last):
                File "/usr/lib/python3/dist-packages/salt/state.py", line 1913, in call
                  **cdata['kwargs'])
                File "/usr/lib/python3/dist-packages/salt/loader.py", line 1898, in wrapper
                  return f(*args, **kwargs)
                File "/usr/lib/python3/dist-packages/salt/states/x509.py", line 719, in pem_managed
                  return __states__['file.managed'](**file_args)
                File "/usr/lib/python3/dist-packages/salt/loader.py", line 1898, in wrapper
                  return f(*args, **kwargs)
                File "/usr/lib/python3/dist-packages/salt/states/file.py", line 2421, 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:36:07.024469
    Duration: 16.172 ms
     Changes:   

Summary for local
------------
Succeeded: 0
Failed:    1
------------
Total states run:     1
Total run time:  16.172 ms

Okay, reading the other bug reports concerning typed strings and unicode/ascii, that's probably related to the changes required to accommodate Python 3. I used the encode() function to reformat my text:

{% set mine_fetch_ascii = mine_fetch.encode("ascii","ignore") %}

And changed my formula kwarg for 'text' to pull that instead:

    - text: {{mine_fetch_ascii}}

But instead of success, I found another issue:

[PROFILE ] Time (in seconds) to render '/var/cache/salt/minion/files/base/Docker/tls.sls' using 'yaml' renderer: 0.012524843215942383
[DEBUG   ] LazyLoaded config.option
[DEBUG   ] LazyLoaded x509.get_pem_entry
[DEBUG   ] LazyLoaded x509.pem_managed
[INFO    ] Running state [/etc/ssl/certs/ca_docker-template.crt] at time 10:51:22.742387
[INFO    ] Executing state x509.pem_managed for [/etc/ssl/certs/ca_docker-template.crt]
[ERROR   ] An exception occurred in this state: Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/salt/state.py", line 1913, in call
    **cdata['kwargs'])
  File "/usr/lib/python3/dist-packages/salt/loader.py", line 1898, in wrapper
    return f(*args, **kwargs)
  File "/usr/lib/python3/dist-packages/salt/states/x509.py", line 717, in pem_managed
    file_args['contents'] = __salt__['x509.get_pem_entry'](text=text)
  File "/usr/lib/python3/dist-packages/salt/modules/x509.py", line 480, in get_pem_entry
    raise salt.exceptions.SaltInvocationError(errmsg)
salt.exceptions.SaltInvocationError: PEM text not valid:
b'-----BEGIN CERTIFICATE-----MIIF8TCCA9mgAwIBAgIIerDv/onLAJswDQYJKoZIhvcNAQELBQAwUzELMAkGA1UEBhMCVVMxGDAWBgNVBAMMD2RvY2tlci10ZW1wbGF0ZTESMBAGA1UEBwwJV29yY2VzdGVyMRYwFAYDVQQIDA1NYXNzYWNodXNldHRzMB4XDTE4MDcwNjAxMjgyM1oXDTI4MDcwMzAxMjgyM1owUzELMAkGA1UEBhMCVVMxGDAWBgNVBAMMD2RvY2tlci10ZW1wbGF0ZTESMBAGA1UEBwwJV29yY2VzdGVyMRYwFAY<<<TRIMMED ALL BUT FIRST FOUR LINES>>>-----END CERTIFICATE-----'

[INFO    ] Completed state [/etc/ssl/certs/ca_docker-template.crt] at time 10:51:22.751280 (duration_in_ms=8.894)
[DEBUG   ] File /var/cache/salt/minion/accumulator/140250953336032 does not exist, no need to cleanup
[DEBUG   ] LazyLoaded state.check_result
[DEBUG   ] Initializing new AsyncZeroMQReqChannel for ('/etc/salt/pki/minion', 'ubbt3dt3.holycross.edu', 'tcp://10.108.193.179:4506', 'aes')
[DEBUG   ] Initializing new AsyncAuth for ('/etc/salt/pki/minion', 'ubbt3dt3.holycross.edu', 'tcp://10.108.193.179:4506')
[DEBUG   ] Connecting the Minion to the Master URI (for the return server): tcp://10.108.193.179:4506
[DEBUG   ] Trying to connect to: tcp://10.108.193.179:4506
[DEBUG   ] LazyLoaded highstate.output
local:
----------
          ID: Docker_TLS_CA_Cert
    Function: x509.pem_managed
        Name: /etc/ssl/certs/ca_docker-template.crt
      Result: False
     Comment: An exception occurred in this state: Traceback (most recent call last):
                File "/usr/lib/python3/dist-packages/salt/state.py", line 1913, in call
                  **cdata['kwargs'])
                File "/usr/lib/python3/dist-packages/salt/loader.py", line 1898, in wrapper
                  return f(*args, **kwargs)
                File "/usr/lib/python3/dist-packages/salt/states/x509.py", line 717, in pem_managed
                  file_args['contents'] = __salt__['x509.get_pem_entry'](text=text)
                File "/usr/lib/python3/dist-packages/salt/modules/x509.py", line 480, in get_pem_entry
                  raise salt.exceptions.SaltInvocationError(errmsg)
              salt.exceptions.SaltInvocationError: PEM text not valid:
              b'-----BEGIN CERTIFICATE-----MIIF8TCCA9mgAwIBAgIIerDv/onLAJswDQYJKoZIhvcNAQELBQAwUzELMAkGA1UEBhMCVVMxGDAWBgNVBAMMD2RvY2tlci10ZW1wbGF0ZTESMBAGA1UEBwwJV29yY2VzdGVyMRYwFAYDVQQIDA1NYXNzYWNodXNldHRzMB4XDTE4MDcwNjAxMjgyM1oXDTI4MDcwMzAxMjgyM1owUzELMAkGA1UEBhMCVVMxGDAWBgNVBAMMD2RvY2tlci10ZW1wbGF0ZTESMBAGA1UEBwwJV29yY2VzdGVyMRYwFAY<<<TRIMMED ALL BUT FIRST FOUR LINES>>>-----END CERTIFICATE-----'
     Started: 10:51:22.742386
    Duration: 8.894 ms
     Changes:   

Summary for local
------------
Succeeded: 0
Failed:    1
------------
Total states run:     1
Total run time:   8.894 ms

Versions Report

Salt Version:
           Salt: 2018.3.3
 
Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.6.1
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.10
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: 0.31.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: Not Installed
         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: Not Installed
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.2.5
 
System Versions:
           dist: Ubuntu 18.04 bionic
         locale: UTF-8
        machine: x86_64
        release: 4.15.0-39-generic
         system: Linux
        version: Ubuntu 18.04 bionic
@Ch3LL
Copy link
Contributor

Ch3LL commented Nov 26, 2018

hmm it looks like it should be handling byte type objects in teh first if statement in states/file.py.

ping @terminalmage any ideas here?

@Ch3LL Ch3LL added the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Nov 26, 2018
@Ch3LL Ch3LL added this to the Blocked milestone Nov 26, 2018
@rnickle
Copy link
Author

rnickle commented Nov 28, 2018

I'm following 'latest', not sure how to proceed here.

@terminalmage terminalmage added Duplicate Duplicate of another issue or PR - will be closed and removed Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged labels Nov 29, 2018
@terminalmage
Copy link
Contributor

This is a duplicate of #50219, as the issue affected anything that used contents, contents_pillar, or contents_grains, and used bytestring contents. @dwoz has fixed this already and the fix will be in 2018.3.4.

@terminalmage
Copy link
Contributor

Note that the x509 states don't accept the contents argument but rather call out to the file.managed state using this argument.

@terminalmage terminalmage added the fixed-pls-verify fix is linked, bug author to confirm fix label Nov 29, 2018
@rnickle
Copy link
Author

rnickle commented Nov 29, 2018

Thanks very much!

I'm looking forward to this feature, I don't see a workaround in the other bug.

I'm not sure what the release cadence is, although the gap between 3.3.2 and 3.3.3 appears to be about 16 weeks.

@sathieu
Copy link
Contributor

sathieu commented Dec 4, 2018

Dup of #39608?

@Ch3LL
Copy link
Contributor

Ch3LL commented Dec 5, 2018

thanks for tying the issues @sathieu :) I will close this issue and mention in the other

@Ch3LL Ch3LL closed this as completed Dec 5, 2018
@Ch3LL Ch3LL mentioned this issue Dec 5, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Duplicate Duplicate of another issue or PR - will be closed fixed-pls-verify fix is linked, bug author to confirm fix
Projects
None yet
Milestone
Blocked
Development

No branches or pull requests
4 participants
@terminalmage @sathieu @Ch3LL @rnickle