Don't send CA keys to mine
Exposing CA keys in a mine creates a security flaw, thus such
should be avoided.
This change removes code responsible for putting and retrieving
CA key from a mine and changes the ca.sls state to allow configuring
where CA cert and its key would be generated as well as their owners.
Fixes PROD-13439
Change-Id: I6d78b13dcb3754c51606edd7e2d8158e128244a4
diff --git a/salt/meta/salt.yml b/salt/meta/salt.yml
index 9d6a728..6bc9632 100644
--- a/salt/meta/salt.yml
+++ b/salt/meta/salt.yml
@@ -27,11 +27,16 @@
{%- from "salt/map.jinja" import minion with context %}
x509_signing_policies:
{%- for ca_name,ca in minion.ca.items() %}
+
+ {%- set ca_file = ca.get('ca_file', '/etc/pki/ca/' ~ ca_name ~ '/ca.crt') %}
+ {%- set ca_key_file = ca.get('ca_key_file', '/etc/pki/ca/' ~ ca_name ~ '/ca.key') %}
+ {%- set ca_certs_dir = salt['file.dirname'](ca_file) ~ '/certs/' %}
+
{%- for signing_policy_name, signing_policy in ca.signing_policy.iteritems() %}
{{ ca_name }}_{{ signing_policy_name }}:
- minions: '{{ signing_policy.minions }}'
- - signing_private_key: /etc/pki/ca/{{ ca_name }}/ca.key
- - signing_cert: /etc/pki/ca/{{ ca_name }}/ca.crt
+ - signing_private_key: {{ ca_key_file }}
+ - signing_cert: {{ ca_file }}
{%- if ca.country is defined %}
- C: {{ ca.country }}
{%- endif %}
@@ -67,7 +72,7 @@
- subjectKeyIdentifier: hash
- authorityKeyIdentifier: keyid,issuer:always
- days_valid: {{ ca.days_valid.certificate }}
- - copypath: /etc/pki/ca/{{ ca_name }}/certs/
+ - copypath: {{ ca_certs_dir }}
{%- endfor %}
{%- endfor %}
{%- endif %}
diff --git a/salt/minion/ca.sls b/salt/minion/ca.sls
index bdf96c4..fdad603 100644
--- a/salt/minion/ca.sls
+++ b/salt/minion/ca.sls
@@ -6,20 +6,47 @@
{%- for ca_name,ca in minion.ca.iteritems() %}
-/etc/pki/ca/{{ ca_name }}/certs:
- file.directory:
- - makedirs: true
+{%- set ca_file = ca.get('ca_file', '/etc/pki/ca/' ~ ca_name ~ '/ca.crt') %}
+{%- set ca_key_file = ca.get('ca_key_file', '/etc/pki/ca/' ~ ca_name ~ '/ca.key') %}
+{%- set ca_key_usage = ca.get('key_usage',"critical,cRLSign,keyCertSign") %}
-/etc/pki/ca/{{ ca_name }}/ca.key:
+{%- set ca_dir = salt['file.dirname'](ca_file) %}
+{%- set ca_key_dir = salt['file.dirname'](ca_key_file) %}
+{%- set ca_certs_dir = ca_dir ~ '/certs' %}
+
+salt_minion_cert_{{ ca_name }}_dirs:
+ file.directory:
+ - names:
+ - {{ ca_dir }}
+ - {{ ca_key_dir }}
+ - {{ ca_certs_dir }}
+ - makedirs: true
+
+{{ ca_key_file }}:
x509.private_key_managed:
- bits: 4096
- backup: True
- require:
- - file: /etc/pki/ca/{{ ca_name }}/certs
+ - file: {{ ca_certs_dir }}
-/etc/pki/ca/{{ ca_name }}/ca.crt:
+# TODO: Squash this with the previous state after switch to Salt version >= 2016.11.2
+{{ ca_name }}_key_permissions:
+ file.managed:
+ - name: {{ ca_key_file }}
+ - mode: {{ ca.get("mode", 0600) }}
+ {%- if salt['user.info'](ca.get("user", "root")) %}
+ - user: {{ ca.get("user", "root") }}
+ {%- endif %}
+ {%- if salt['group.info'](ca.get("group", "root")) %}
+ - group: {{ ca.get("group", "root") }}
+ {%- endif %}
+ - replace: false
+ - require:
+ - x509: {{ ca_key_file }}
+
+{{ ca_file }}:
x509.certificate_managed:
- - signing_private_key: /etc/pki/ca/{{ ca_name }}/ca.key
+ - signing_private_key: {{ ca_key_file }}
- CN: "{{ ca.common_name }}"
{%- if ca.country is defined %}
- C: {{ ca.country }}
@@ -37,33 +64,37 @@
- OU: {{ ca.organization_unit }}
{%- endif %}
- basicConstraints: "critical,CA:TRUE"
- - keyUsage: "critical,cRLSign,keyCertSign"
+ - keyUsage: {{ ca_key_usage }}
- subjectKeyIdentifier: hash
- authorityKeyIdentifier: keyid,issuer:always
- days_valid: {{ ca.days_valid.authority }}
- days_remaining: 0
- backup: True
- require:
- - x509: /etc/pki/ca/{{ ca_name }}/ca.key
+ - x509: {{ ca_key_file }}
+
+# TODO: Squash this with the previous state after switch to Salt version >= 2016.11.2
+{{ ca_name }}_cert_permissions:
+ file.managed:
+ - name: {{ ca_file }}
+ - mode: 0644
+ {%- if salt['user.info'](ca.get("user", "root")) %}
+ - user: {{ ca.get("user", "root") }}
+ {%- endif %}
+ {%- if salt['group.info'](ca.get("group", "root")) %}
+ - group: {{ ca.get("group", "root") }}
+ {%- endif %}
+ - require:
+ - x509: {{ ca_file }}
salt_system_ca_mine_send_ca_{{ ca_name }}:
module.run:
- name: mine.send
- func: x509.get_pem_entries
- kwargs:
- glob_path: /etc/pki/ca/{{ ca_name }}/ca.crt
+ glob_path: {{ ca_file }}
- require:
- - x509: /etc/pki/ca/{{ ca_name }}/ca.crt
-
-salt_system_ca_mine_send_ca_key_{{ ca_name }}:
- module.run:
- - name: mine.send
- - func: x509_get_private_key
- - kwargs:
- mine_function: x509.get_pem_entries
- glob_path: /etc/pki/ca/{{ ca_name }}/ca.key
- - require:
- - x509: /etc/pki/ca/{{ ca_name }}/ca.key
+ - x509: {{ ca_file }}
{%- endfor %}
diff --git a/salt/minion/cert.sls b/salt/minion/cert.sls
index 017c115..c04215a 100644
--- a/salt/minion/cert.sls
+++ b/salt/minion/cert.sls
@@ -11,7 +11,6 @@
{%- if minion.cert is defined %}
{%- set created_ca_files = [] %}
-{%- set created_ca_key_files = [] %}
{%- for cert_name,cert in minion.get('cert', {}).iteritems() %}
{%- set rowloop = loop %}
@@ -19,11 +18,9 @@
{%- set key_file = cert.get('key_file', '/etc/ssl/private/' + cert.common_name + '.key') %}
{%- set cert_file = cert.get('cert_file', '/etc/ssl/certs/' + cert.common_name + '.crt') %}
{%- set ca_file = cert.get('ca_file', '/etc/ssl/certs/ca-' + cert.authority + '.crt') %}
-{%- set ca_key_file = cert.get('ca_key_file', '/etc/ssl/certs/ca-' + cert.authority + '.key') %}
{%- set key_dir = salt['file.dirname'](key_file) %}
{%- set cert_dir = salt['file.dirname'](cert_file) %}
{%- set ca_dir = salt['file.dirname'](ca_file) %}
-{%- set ca_key_dir = salt['file.dirname'](ca_key_file) %}
{# Only ensure directories exists, don't touch permissions, etc. #}
salt_minion_cert_{{ cert_name }}_dirs:
@@ -32,7 +29,6 @@
- {{ key_dir }}
- {{ cert_dir }}
- {{ ca_dir }}
- - {{ ca_key_dir }}
- makedirs: true
- replace: false
@@ -46,6 +42,7 @@
- cmd: salt_minion_cert_{{ cert_name }}_all
{%- endif %}
+# TODO: Squash this with the previous state after switch to Salt version >= 2016.11.2
{{ key_file }}_key_permissions:
file.managed:
- name: {{ key_file }}
@@ -57,7 +54,7 @@
- group: {{ cert.get("group", "root") }}
{%- endif %}
- replace: false
- - watch:
+ - require:
- x509: {{ key_file }}
{{ cert_file }}:
@@ -94,6 +91,7 @@
- cmd: salt_minion_cert_{{ cert_name }}_all
{%- endif %}
+# TODO: Squash this with the previous state after switch to Salt version >= 2016.11.2
{{ cert_file }}_cert_permissions:
file.managed:
- name: {{ cert_file }}
@@ -105,7 +103,7 @@
- group: {{ cert.get("group", "root") }}
{%- endif %}
- replace: false
- - watch:
+ - require:
- x509: {{ cert_file }}
{%- if cert.host is defined and ca_file not in created_ca_files %}
@@ -125,6 +123,7 @@
{%- endif %}
+# TODO: Squash this with the previous state after switch to Salt version >= 2016.11.2
{{ ca_file }}_cert_permissions:
file.managed:
- name: {{ ca_file }}
@@ -135,7 +134,7 @@
{%- if salt['group.info'](cert.get("group", "root")) %}
- group: {{ cert.get("group", "root") }}
{%- endif %}
- - watch:
+ - require:
- x509: {{ ca_file }}
{%- endif %}
@@ -145,38 +144,6 @@
{%- do created_ca_files.append(ca_file) %}
{%- endif %}
-{%- if cert.host is defined and ca_key_file not in created_ca_key_files %}
-{%- for ca_key_path,ca_key in salt['mine.get'](cert.host, 'x509_get_private_key').get(cert.host, {}).iteritems() %}
-
-{%- if '/etc/pki/ca/'+cert.authority in ca_key_path %}
-
-{{ ca_key_file }}:
- x509.pem_managed:
- - name: {{ ca_key_file }}
- - text: {{ ca_key|replace('\n', '') }}
- - watch:
- - x509: {{ cert_file }}
-
-{{ ca_key_file }}_cert_permissions:
- file.managed:
- - name: {{ ca_key_file }}
- - mode: 0644
- {%- if salt['user.info'](cert.get("user", "root")) %}
- - user: {{ cert.get("user", "root") }}
- {%- endif %}
- {%- if salt['group.info'](cert.get("group", "root")) %}
- - group: {{ cert.get("group", "root") }}
- {%- endif %}
- - watch:
- - x509: {{ ca_key_file }}
-
-{%- endif %}
-
-{%- endfor %}
-
-{%- do created_ca_key_files.append(ca_key_file) %}
-{%- endif %}
-
{%- if cert.all_file is defined %}
salt_minion_cert_{{ cert_name }}_all:
@@ -194,7 +161,7 @@
- group: {{ cert.get("group", "root") }}
{%- endif %}
- replace: false
- - watch:
+ - require:
- cmd: salt_minion_cert_{{ cert_name }}_all
{%- endif %}
@@ -229,10 +196,10 @@
{%- for trusted_ca_minion in minion.get('trusted_ca_minions', []) %}
{%- for ca_host, certs in salt['mine.get'](trusted_ca_minion+'*', 'x509.get_pem_entries').iteritems() %}
{%- for ca_path, ca_cert in certs.iteritems() %}
-{%- if ca_path.startswith('/etc/pki/ca/') and ca_path.endswith('ca.crt') %}
+{%- if ca_path.endswith('ca.crt') %}
{# authority name can be obtained only from a cacert path in case of mine.get #}
-{%- set ca_authority = ca_path.split("/")[4] %}
+{%- set ca_authority = ca_path.split("/")[-2] %}
{%- set cacert_file="%s/ca-%s.crt" % (cacerts_dir,ca_authority) %}
salt_trust_ca_{{ cacert_file }}:
diff --git a/tests/pillar/minion_pki_ca.sls b/tests/pillar/minion_pki_ca.sls
index 935014b..d11adbf 100644
--- a/tests/pillar/minion_pki_ca.sls
+++ b/tests/pillar/minion_pki_ca.sls
@@ -44,3 +44,28 @@
ca_intermediate:
type: v3_intermediate_ca
minions: '*'
+ salt-ca-alt:
+ common_name: Alt CA Testing
+ country: Czech
+ state: Prague
+ locality: Cesky Krumlov
+ days_valid:
+ authority: 3650
+ certificate: 90
+ signing_policy:
+ cert_server:
+ type: v3_edge_cert_server
+ minions: '*'
+ cert_client:
+ type: v3_edge_cert_client
+ minions: '*'
+ ca_edge:
+ type: v3_edge_ca
+ minions: '*'
+ ca_intermediate:
+ type: v3_intermediate_ca
+ minions: '*'
+ ca_file: '/etc/test/ca.crt'
+ ca_key_file: '/etc/test/ca.key'
+ user: test
+ group: test
diff --git a/tests/pillar/minion_pki_cert.sls b/tests/pillar/minion_pki_cert.sls
index 745f3c4..9e6fef5 100644
--- a/tests/pillar/minion_pki_cert.sls
+++ b/tests/pillar/minion_pki_cert.sls
@@ -59,3 +59,23 @@
# salt.ci.local
#signing_policy:
# cert_server
+ test_cert:
+ alternative_names:
+ IP:127.0.0.1,DNS:salt.ci.local,DNS:test.ci.local
+ cert_file:
+ /srv/salt/pki/ci/test.ci.local.crt
+ common_name:
+ test.ci.local
+ key_file:
+ /srv/salt/pki/ci/test.ci.local.key
+ country: CZ
+ state: Prague
+ locality: Cesky Krumlov
+ signing_cert:
+ /etc/test/ca.crt
+ signing_private_key:
+ /etc/test/ca.key
+ # Kitchen-Salt CI trigger `salt-call --local`, below attributes
+ # can't be used as there is no required SaltMaster connectivity
+ authority:
+ salt-ca-alt