x509: fix type issues; a 2019.2-based variant of PR #52014, fixes #52026

[alxwr]

What does this PR do?

This is just an updated version of #52014.

What issues does this PR fix or reference?

#52014 #52026 #52180

Previous Behavior

modules.x509 fails to parse existing certificates.

New Behavior

modules.x509

Tests written?

No, but tested on FreeBSD 11.2 and on other machines (see #52026)

Commits signed with GPG?

No

---

I just hope this helps getting #52014 into upstream. All credit goes to @arsiesys.

[dwoz]

@alxwr We still need a regression test for this bugfix.

[alxwr]

@dwoz I'll be happy to write one. Would you please point me to an example to get me up to speed?

[alxwr]

@dwoz I found another related bug and fixed it: 31b6a12 33390266a5

When I tried to use x509.certificate_managed with csr: it failed with TypeError: a bytes-like object is required, not 'str':

    Function: x509.certificate_managed                                                                    
      Result: False                                                                                       
     Comment: Attempt 1: Returned a result of "False", with the following comment: "An exception occurred 
in this state: Traceback (most recent call last):                                                         
                File "/usr/local/lib/python3.6/site-packages/salt/state.py", line 1933, in call           
                  **cdata['kwargs'])                                                                      
                File "/usr/local/lib/python3.6/site-packages/salt/loader.py", line 1939, in wrapper       
                  return f(*args, **kwargs)                                                               
                File "/usr/local/lib/python3.6/site-packages/salt/states/x509.py", line 502, in certificat
e_managed                                                                                                 
                  new = __salt__['x509.create_certificate'](testrun=True, **kwargs)                       
                File "/usr/local/lib/python3.6/site-packages/salt/modules/x509.py", line 1378, in create_c
ertificate                                                                                                
                  pem_type='CERTIFICATE REQUEST').replace('\n', '')                                       
              TypeError: a bytes-like object is required, not 'str'                                       
              "

This might be related: #53294

---

Regarding the tests:

I followed https://github.com/saltstack/salt/blob/develop/HACKING.rst, but I can't get the tests to run locally on a Ubuntu 18.04. They fail with:

    self.run()
  File "/home/a/sw_dev/github/salt/salt/utils/process.py", line 765, in _run
    return self._original_run()
  File "/home/a/sw_dev/github/salt/salt/master.py", line 1142, in run
    self.__bind()
  File "/home/a/sw_dev/github/salt/salt/master.py", line 1032, in __bind
    req_channel.post_fork(self._handle_payload, io_loop=self.io_loop)  # TODO: cleaner? Maybe lazily?
  File "/home/a/sw_dev/github/salt/salt/transport/zeromq.py", line 703, in post_fork
    self.stream = zmq.eventloop.zmqstream.ZMQStream(self._socket, io_loop=self.io_loop)
  File "/usr/lib/python2.7/dist-packages/zmq/eventloop/zmqstream.py", line 114, in __init__
    self._init_io_state()
  File "/usr/lib/python2.7/dist-packages/zmq/eventloop/zmqstream.py", line 535, in _init_io_state
    self.io_loop.add_handler(self.socket, self._handle_events, self._state)
  File "/home/a/.local/lib/python2.7/site-packages/tornado/ioloop.py", line 910, in add_handler
    self._impl.register(fd, events | self.ERROR)
TypeError: argument must be an int, or have a fileno() method

[Ch3LL]

@alxwr you might be running into #46905

what version of pyzmq do you have installed?

[alxwr]

@Ch3LL I'm using PyZMQ 16.0.2. Thanks for the hint! I'll try to update.

[Ch3LL]

no problem. Let me know if i can help with anything else :)

[alxwr]

I found two some similar bugs in modules.x509.read_crl and modules.x509.crl_verify, so I extended this PR.
Will add tests as soon as my time allows that.

[dawidmalina]

I have exactly the same issue (saltstack 2019.2.0):

[ERROR   ] An exception occurred in this state: Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/salt/state.py", line 1933, in call
    **cdata['kwargs'])
  File "/usr/lib/python3/dist-packages/salt/loader.py", line 1939, in wrapper
    return f(*args, **kwargs)
  File "/usr/lib/python3/dist-packages/salt/states/x509.py", line 502, in certificate_managed
    new = __salt__['x509.create_certificate'](testrun=True, **kwargs)
  File "/usr/lib/python3/dist-packages/salt/modules/x509.py", line 1383, in create_certificate
    passphrase=kwargs['public_key_passphrase']).replace('\n', '')
TypeError: a bytes-like object is required, not 'str'

After applying this patch all work as expected. It's said that we will not have this fix in 2019.2.2 :(

@Ch3LL, @waynew when we could expect this fix will be released?

[waynew]

@alxwr with our recent shift to using master, if you could edit this PR to point to master & rebase your changes on master (or alternatively, close this PR and open a new one) when you add some tests, that would be awesome.

[ClaudiuLSH]

I confirm this PR fixes the issue, had the same error. Sad to see this is not merged after so long.

[alxwr]

@waynew Sry for the long delay!
@ClaudiuPID thanks for commenting on the issue (so it would show up at the top of my messages list)!

I'd love to write tests, but I'd have to read up on them in order to write proper tests. I just haven't got the time to do that now. As I states in the description: this is a mere update and re-application of patches I didn't write myself.

[mchugh19]

@alxwr I think the most reliable test in this case would likely be a sample cert or two which triggered the bug behavior. The integration tests already have tests/integration/files/file/base/certs/salttest.p12. If that isn't triggering the behavior, can you upload a file example to tests/integration/files/file/base/certs? It should then be pretty easy to write up a state which accesses the cert in a way which demonstrates the decoding errors.

Also #52014 had a test case attached you could just copy into this PR.

[waynew]

@alxwr did that explanation make sense? Or do you need a bit more help in regards to testing?

[waynew]

@alxwr are you still interested in writing tests & getting this merged in?

[waynew]

@alxwr We're going to be running a test clinic this Thursday on Zoom - if you're interested check out our community calendar here - we'll be holding it during the Community Open Hour on April 2.

[waynew]

@alxwr Are you still interested in driving this to completion? We'd love to help you know how to best write tests for this!

[mchugh19]

Just getting example certs which display the errors would be enough to allow others to help writing tests.

[alxwr]

@waynew I was interested, but just didn't have the time. Sry.

Seems this issue has been dealt with in a much better way than I currently can provide:

• Rewrite x509.certificate_managed to be easier to use #52935 (merged 6 days ago, description says it fixes [2018.3.4] x509.certificate_managed comparisons fail on py3 #52180)
• x509.certificate_managed triggers a change when switching form python 2.7 to 3.6 #56556 (comment)

I'm closing this PR, because when I rebase the patches against master this amounts to something like this, which indicates #52935 took care of the problem:

diff --git a/salt/modules/x509.py b/salt/modules/x509.py
index c278723038..e02fd61b66 100644
--- a/salt/modules/x509.py
+++ b/salt/modules/x509.py
@@ -742,7 +742,7 @@ def get_public_key(key, passphrase=None, asObj=False):
         return evppubkey
 
     rsa.save_pub_key_bio(bio)
-    return bio.read_all()
+    return bio.read_all().decode()

Thanks for your help! Sry I didn't manage to finish the PR in time.