[BUG] x509_v2 certificate_managed is passing newline stripped data to append_certs

[mcd1992]

Description
x509_v2 certificate_managed append_certs parameter is stripping newlines from PEM certificates.

When using the below state the minion will throw a invalid index error in salt/utils/x509.py:814 because the pems = split_pems(cert) is receiving PEM pillar data with its newlines replaced with spaces. On the master split_pems receives proper pillar data with the newlines included. But the minion seems to have newlines replaced with spaces.

For now as a workaround I've just changed the append_certs to run the pillar data into base64_encode which works.

Setup

  x509.certificate_managed:
    - name: /etc/pki/fqdn/public.crt.pem
    - ca_server: master.local
    - signing_policy: encipher
    - private_key: /etc/pki/fqdn/private.key.pem
    - CN: fqdn
    - days_valid: 90
    - days_remaining: 60
    - makedirs: True
    - dir_mode: 0700
    - mode: 0600
    - subjectAltName: 'DNS: fqdn'
    - encoding: pem
    - append_certs:
      - {{pillar['cacerts']['int_ca']}}
      - {{pillar['cacerts']['root_ca']}}

Versions Report

Salt Version:
          Salt: 3007.0
 
Python Version:
        Python: 3.12.3 (main, Apr 23 2024, 09:16:07) [GCC 13.2.1 20240417]
 
Dependency Versions:
          cffi: 1.16.0
      cherrypy: Not Installed
      dateutil: Not Installed
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 3.1.3
       libgit2: Not Installed
  looseversion: 1.3.0
      M2Crypto: 0.40.1
          Mako: Not Installed
       msgpack: 1.0.5
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     packaging: 23.2
     pycparser: 2.22
      pycrypto: Not Installed
  pycryptodome: 3.12.0
        pygit2: Not Installed
  python-gnupg: Not Installed
        PyYAML: 6.0.1
         PyZMQ: 25.1.2
        relenv: Not Installed
         smmap: Not Installed
       timelib: Not Installed
       Tornado: 6.4
           ZMQ: 4.3.5
 
Salt Package Information:
  Package Type: Not Installed
 
System Versions:
          dist: arch  
        locale: utf-8
       machine: x86_64
       release: 6.5.11-8-pve
        system: Linux
       version: Arch Linux  

Both master and minion are Arch Linux on 3007.0

[lkubb]

Without knowledge about the specifics of your setup, I suspect the title does not describe the problem correctly. It should probably be utils.x509.load_cert crashes when confronted with improperly formatted PEM data. In general, load_cert should definitely not cause a traceback, while split_pems should probably work with improperly formatted data.

An initial reaction:

It's generally recommended to dump data (especially strings that can contain newlines) by passing it through the json filter:

# ...
    - append_certs:
      - {{ pillar['cacerts']['int_ca'] | json }}
      - {{ pillar['cacerts']['root_ca'] | json }}

Otherwise, the rendered YAML can become garbled/invalid (+ you might open yourself up to a template injection attack, depending on the source of the data).

Since your template seems to render fine (when I skip |json, the YAML is invalid), I assume the pillar values do not contain the properly formatted certificates verbatim (including newlines). The split_pems function does not fix those though, it expects

-----BEGIN CERTIFICATE-----
MII...
...
-----END CERTIFICATE-----
-----BEGIN CERTIFICATE-----
MII...
...
-----END CERTIFICATE-----
...

and strips anything that's not in between \n?-----BEGIN (CERTIFICATE|...)-----\n[...]\n-----END (CERTIFICATE|...)-----\n?

If the certificates in the pillar are read from a YAML file, they should be defined like:

cacerts:
  int_ca: |
    -----BEGIN CERTIFICATE-----
    MII...
    ...
    -----END CERTIFICATE-----
  root_ca: |
    -----BEGIN CERTIFICATE-----
    MII...
    ...
    -----END CERTIFICATE-----

[mcd1992]

My pillar data is defined just like that, with the multline |. And prior to migrating to x509_v2 this worked without | json or | base64_encode.

What's weird is when I patch utils/x509.py:load_cert on the minion and master, to have a debug print for the cert variable. The master gets a proper PEM string with literal \ns (not 0x0a) in it but the minion gets spaces or some sort of whitespace instead of \n.

When I do salt-call pillar.data on minion and master they both show the proper PEM with newlines. I'll try to replicate with 2 docker containers since I can't replicate with a simple 'masterless' setup.

[lkubb]

That's weird. Does | json fix it (I doubt it)? If not, you can try salt["x509.get_pem_entry"](pillar['cacerts']['int_ca']) | json.

I can attest that the previous x509 modules ran the certificates that were specified in append_certs through x509.get_pem_entry, which fixes improperly formatted PEM entries. Fixing it in the state module is not an option for x509_v2 since it also supports other input types there (base64/hex-encoded data), we would have to adapt the split_pems function.

[mcd1992]

| json does workaround just like | base64_encode

I'll try and get a minimum reproducible setup going for debugging. It may be related somehow to this x509.certificate_managed state being inside a macro as well.

[lkubb]

Awesome. Yes, that's a likely culprit.

| json is not a workaround per se, it is how multiline strings should always be escaped, documented (not in the best location) here:

salt/salt/states/x509_v2.py

Lines 134 to 150 in cf6c1e1

	This example state will instruct all minions to trust certificates signed by
	our new CA. Mind that this example works for Debian-based OS only.
	Also note the Jinja call to encode the string to JSON, which will avoid
	YAML issues with newline characters.
	.. code-block:: jinja
	# /srv/salt/cert.sls
	Ensure the CA trust bundle exists:
	file.directory:
	- name: /usr/local/share/ca-certificates
	Ensure our self-signed CA certificate is included:
	x509.pem_managed:
	- name: /usr/local/share/ca-certificates/myca.crt
	- text: {{ salt["mine.get"]("ca", "x509.get_pem_entries")["ca"]["/etc/pki/ca.crt"] | json }}

If omitted, the resulting YAML should be unparsable:

[salt.state       :4326][CRITICAL][746574] Rendering SLS 'base:x509test' failed: could not find expected ':'; line 17

---
[...]
    - subjectAltName: 'DNS: fqdn'
    - encoding: pem
    - append_certs:
      - -----BEGIN CERTIFICATE-----
MIIDODCCAiCgAwIBAgIIbfpgqP0VGPgwDQYJKoZIhvcNAQELBQAwKzELMAkGA1UE
BhMCVVMxDTALBgNVBAMMBFRlc3QxDTALBgNVBAoMBFNhbHQwHhcNMjIxMTE1MTQw    <======================