Skip to content

Commit

Permalink
Fix: compare bytes when comparing certificates (fixes #56556)
Browse files Browse the repository at this point in the history
alxwr authored and dwoz committed Nov 13, 2020

Unverified

This user has not yet uploaded their public signing key.
1 parent 7ba3fd6 commit ea409f0
Showing 4 changed files with 95 additions and 0 deletions.
1 change: 1 addition & 0 deletions changelog/58296.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix comparison of certificate values
3 changes: 3 additions & 0 deletions salt/states/x509.py
Original file line number Diff line number Diff line change
@@ -429,6 +429,9 @@ def _certificate_info_matches(cert_info, required_cert_info, check_serial=False)

diff = []
for k, v in required_cert_info.items():
# cert info comes as byte string
if isinstance(v, str):
v = salt.utils.stringutils.to_bytes(v)
try:
if v != cert_info[k]:
if k == "Subject Hash":
65 changes: 65 additions & 0 deletions tests/integration/files/file/base/x509/proper_cert_comparison.sls
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
{% set tmp_dir = pillar['tmp_dir'] %}

{{ tmp_dir }}/pki:
file.directory

{{ tmp_dir }}/pki/issued_certs:
file.directory

{{ tmp_dir }}/pki/ca.key:
x509.private_key_managed:
# speed this up
- bits: 1024
- require:
- file: {{ tmp_dir }}/pki

{{ tmp_dir }}/pki/ca.crt:
x509.certificate_managed:
- signing_private_key: {{ tmp_dir }}/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: {{ tmp_dir }}/pki
- x509: {{ tmp_dir }}/pki/ca.key

{{ tmp_dir }}/pki/test.key:
x509.private_key_managed:
# speed this up
- bits: 1024
- backup: True

test_crt:
x509.certificate_managed:
- name: {{ tmp_dir }}/pki/test.crt
- ca_server: minion
- signing_policy: ca_policy
- public_key: {{ tmp_dir }}/pki/test.key
- CN: minion
- days_remaining: 30
- backup: True
- require:
- x509: {{ tmp_dir }}/pki/ca.crt
- x509: {{ tmp_dir }}/pki/test.key

second_test_crt:
x509.certificate_managed:
- name: {{ tmp_dir }}/pki/test.crt
- ca_server: minion
- signing_policy: ca_policy
- public_key: {{ tmp_dir }}/pki/test.key
- CN: minion
- days_remaining: 30
- backup: True
- require:
- x509: {{ tmp_dir }}/pki/ca.crt
- x509: {{ tmp_dir }}/pki/test.key
- x509: {{ tmp_dir }}/pki/test.crt
26 changes: 26 additions & 0 deletions tests/integration/states/test_x509.py
Original file line number Diff line number Diff line change
@@ -165,6 +165,32 @@ def test_cert_signing_based_on_csr(self):
assert "Certificate" in ret[key]["changes"]
assert "New" in ret[key]["changes"]["Certificate"]

@slowTest
def test_proper_cert_comparison(self):
# In this SLS we define two certs which have identical content.
# The first one is expected to be created.
# The second one is expected to be recognized as already present.
ret = self.run_function(
"state.apply",
["x509.proper_cert_comparison"],
pillar={"tmp_dir": RUNTIME_VARS.TMP},
)
# check the first generated cert
first_key = "x509_|-test_crt_|-{}/pki/test.crt_|-certificate_managed".format(
RUNTIME_VARS.TMP
)
assert first_key in ret
assert "changes" in ret[first_key]
assert "Certificate" in ret[first_key]["changes"]
assert "New" in ret[first_key]["changes"]["Certificate"]
# check whether the second defined cert is considered to match the first one
second_key = "x509_|-second_test_crt_|-{}/pki/test.crt_|-certificate_managed".format(
RUNTIME_VARS.TMP
)
assert second_key in ret
assert "changes" in ret[second_key]
assert ret[second_key]["changes"] == {}

@slowTest
def test_crl_managed(self):
ret = self.run_function(

0 comments on commit ea409f0

Please sign in to comment.