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] Fix comparison of certificate values #58296

Merged
merged 5 commits into from
Nov 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Expand Up @@ -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":
Expand Down
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
Expand Up @@ -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(
Expand Down