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

salt/meta/salt.yml

@@ -27,11 +27,16 @@ minion:
     {%- 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 @@ minion:
         - 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 %}

salt/minion/ca.sls

@@ -6,20 +6,47 @@ include:
 
 {%- for ca_name,ca in minion.ca.iteritems() %}
 
-/etc/pki/ca/{{ ca_name }}/certs:
+{%- 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") %}
+
+{%- 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:
-  - makedirs: true
+    - names:
+      - {{ ca_dir }}
+      - {{ ca_key_dir }}
+      - {{ ca_certs_dir }}
+    - makedirs: true
 
-/etc/pki/ca/{{ ca_name }}/ca.key:
+{{ ca_key_file }}:
   x509.private_key_managed:
   - bits: 4096
   - backup: True
   - require:
-    - file: /etc/pki/ca/{{ ca_name }}/certs
+    - file: {{ ca_certs_dir }}
+
+# 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 }}
 
-/etc/pki/ca/{{ ca_name }}/ca.crt:
+{{ 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 @@ include:
   - 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
-  - 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
+      glob_path: {{ ca_file }}
   - require:
-    - x509: /etc/pki/ca/{{ ca_name }}/ca.key
+    - x509: {{ ca_file }}
 
 {%- endfor %}
 

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 @@ salt_minion_cert_{{ cert_name }}_dirs:
       - {{ key_dir }}
       - {{ cert_dir }}
       - {{ ca_dir }}
-      - {{ ca_key_dir }}
     - makedirs: true
     - replace: false
 
@@ -46,6 +42,7 @@ salt_minion_cert_{{ cert_name }}_dirs:
       - 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 @@ salt_minion_cert_{{ cert_name }}_dirs:
     - group: {{ cert.get("group", "root") }}
     {%- endif %}
     - replace: false
-    - watch:
+    - require:
       - x509: {{ key_file }}
 
 {{ cert_file }}:
@@ -94,6 +91,7 @@ salt_minion_cert_{{ cert_name }}_dirs:
       - 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 @@ salt_minion_cert_{{ cert_name }}_dirs:
     - 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 @@ salt_minion_cert_{{ cert_name }}_dirs:
     {%- 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 @@ salt_minion_cert_{{ cert_name }}_dirs:
     {%- if salt['group.info'](cert.get("group", "root")) %}
     - group: {{ cert.get("group", "root") }}
     {%- endif %}
-    - watch:
+    - require:
       - x509: {{ ca_file }}
 
 {%- endif %}
@@ -145,38 +144,6 @@ salt_minion_cert_{{ cert_name }}_dirs:
 {%- 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 @@ salt_minion_cert_{{ cert_name }}_all:
     - group: {{ cert.get("group", "root") }}
     {%- endif %}
     - replace: false
-    - watch:
+    - require:
       - cmd: salt_minion_cert_{{ cert_name }}_all
 {%- endif %}
 
@@ -229,10 +196,10 @@ salt_update_certificates:
 {%- 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 }}:

tests/pillar/minion_pki_ca.sls

@@ -44,3 +44,28 @@ salt:
           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

tests/pillar/minion_pki_cert.sls

@@ -59,3 +59,23 @@ salt:
           #   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