From a514aa0f985e659fa0e5f225fdfeaabd2912bff8 Mon Sep 17 00:00:00 2001
From: "James E. Blair" <jeblair@redhat.com>
Date: Wed, 17 Jun 2020 10:32:17 -0700
Subject: [PATCH] Zookeeper: listen on plain and TLS ports

To prepare for switching to TLS, set up TLS certs for Zookeeper and
all of Nodepool and Zuul, but do not have them connect over TLS yet.
We have observed problems with Kazoo using TLS in production.  This
will let us run the ZK quorum using TLS internally, and have Zuul
and Nodepool connect over plaintext while also exposing the TLS
client port so that we can perform some more production tests.

Change-Id: If93b27f5b55be42be1cf6ee23258127fab5ce9ea
---
 inventory/service/group_vars/zookeeper.yaml   |   4 +
 playbooks/roles/nodepool-base/tasks/main.yaml |   8 ++
 playbooks/roles/zk-ca/README.rst              |   4 +
 playbooks/roles/zk-ca/defaults/main.yaml      |   5 +
 playbooks/roles/zk-ca/tasks/main.yaml         |  49 +++++++++
 playbooks/roles/zk-ca/zk-ca.sh                | 104 ++++++++++++++++++
 .../zookeeper-compose/docker-compose.yaml     |   1 +
 playbooks/roles/zookeeper/tasks/main.yaml     |   8 ++
 .../roles/zookeeper/templates/zoo.cfg.j2      |   9 +-
 playbooks/roles/zuul/tasks/main.yaml          |   7 ++
 playbooks/roles/zuul/templates/zuul.conf.j2   |   2 +-
 testinfra/test_zookeeper.py                   |   4 +
 12 files changed, 203 insertions(+), 2 deletions(-)
 create mode 100644 playbooks/roles/zk-ca/README.rst
 create mode 100644 playbooks/roles/zk-ca/defaults/main.yaml
 create mode 100644 playbooks/roles/zk-ca/tasks/main.yaml
 create mode 100755 playbooks/roles/zk-ca/zk-ca.sh

diff --git a/inventory/service/group_vars/zookeeper.yaml b/inventory/service/group_vars/zookeeper.yaml
index f74b283b0f..5816d2ff18 100644
--- a/inventory/service/group_vars/zookeeper.yaml
+++ b/inventory/service/group_vars/zookeeper.yaml
@@ -3,8 +3,12 @@ zookeeper_group: zookeeper
 zookeeper_uid: 10001
 zookeeper_gid: 10001
 iptables_extra_allowed_groups:
+  # Insecure
   - {'protocol': 'tcp', 'port': '2181', 'group': 'nodepool'}
   - {'protocol': 'tcp', 'port': '2181', 'group': 'zuul'}
+  # Secure
+  - {'protocol': 'tcp', 'port': '2281', 'group': 'nodepool'}
+  - {'protocol': 'tcp', 'port': '2281', 'group': 'zuul'}
   # Zookeeper election
   - {'protocol': 'tcp', 'port': '2888', 'group': 'zookeeper'}
   # Zookeeper leader
diff --git a/playbooks/roles/nodepool-base/tasks/main.yaml b/playbooks/roles/nodepool-base/tasks/main.yaml
index 956a702ee5..456240fcbe 100644
--- a/playbooks/roles/nodepool-base/tasks/main.yaml
+++ b/playbooks/roles/nodepool-base/tasks/main.yaml
@@ -26,6 +26,14 @@
     group: '{{ nodepool_group }}'
     mode: 0755
 
+- name: Generate ZooKeeper TLS cert
+  include_role:
+    name: zk-ca
+  vars:
+    zk_ca_cert_dir: /etc/nodepool
+    zk_ca_cert_dir_owner: '{{ nodepool_user }}'
+    zk_ca_cert_dir_group: '{{ nodepool_group }}'
+
 - name: Create nodepool log dir
   file:
     name: /var/log/nodepool
diff --git a/playbooks/roles/zk-ca/README.rst b/playbooks/roles/zk-ca/README.rst
new file mode 100644
index 0000000000..e314c2ab20
--- /dev/null
+++ b/playbooks/roles/zk-ca/README.rst
@@ -0,0 +1,4 @@
+Generate TLS certs for ZooKeeper
+
+This will copy the certs to the remote node into the /etc/zuul
+directory by default.
diff --git a/playbooks/roles/zk-ca/defaults/main.yaml b/playbooks/roles/zk-ca/defaults/main.yaml
new file mode 100644
index 0000000000..db0eb65f72
--- /dev/null
+++ b/playbooks/roles/zk-ca/defaults/main.yaml
@@ -0,0 +1,5 @@
+zk_ca_root: /var/zk-ca
+zk_ca_server: "{{ inventory_hostname }}"
+zk_ca_cert_dir: /etc/zuul
+zk_ca_cert_dir_owner: 10001
+zk_ca_cert_dir_group: 10001
diff --git a/playbooks/roles/zk-ca/tasks/main.yaml b/playbooks/roles/zk-ca/tasks/main.yaml
new file mode 100644
index 0000000000..f76f82937b
--- /dev/null
+++ b/playbooks/roles/zk-ca/tasks/main.yaml
@@ -0,0 +1,49 @@
+- name: Ensure zk-ca directory exists
+  delegate_to: localhost
+  file:
+    path: "{{ zk_ca_root }}"
+    state: directory
+
+# Run this in flock so that we can run it in plays for multiple target
+# hosts in parallel while serializing access to the CA files.
+- name: Run zk-ca.sh
+  delegate_to: localhost
+  script: "zk-ca.sh {{ zk_ca_root }} {{ zk_ca_server }}"
+  args:
+    executable: "flock {{ zk_ca_root }}/lock"
+
+- name: Ensure cert dir exists
+  file:
+    path: "{{ zk_ca_cert_dir }}/certs"
+    state: directory
+    owner: "{{ zk_ca_cert_dir_owner }}"
+    group: "{{ zk_ca_cert_dir_group }}"
+    mode: '0755'
+
+- name: Ensure keys dir exists
+  file:
+    path: "{{ zk_ca_cert_dir }}/keys"
+    state: directory
+    owner: "{{ zk_ca_cert_dir_owner }}"
+    group: "{{ zk_ca_cert_dir_group }}"
+    mode: '0700'
+
+- name: Copy TLS cacert into place
+  copy:
+    src: "/var/zk-ca/certs/cacert.pem"
+    dest: "{{ zk_ca_cert_dir }}/certs/cacert.pem"
+
+- name: Copy TLS cert into place
+  copy:
+    src: "/var/zk-ca/certs/{{ inventory_hostname }}.pem"
+    dest: "{{ zk_ca_cert_dir }}/certs/cert.pem"
+
+- name: Copy TLS key into place
+  copy:
+    src: "/var/zk-ca/keys/{{ inventory_hostname }}key.pem"
+    dest: "{{ zk_ca_cert_dir }}/keys/key.pem"
+
+- name: Copy TLS keystore into place
+  copy:
+    src: "/var/zk-ca/keystores/{{ inventory_hostname }}.pem"
+    dest: "{{ zk_ca_cert_dir }}/keys/keystore.pem"
diff --git a/playbooks/roles/zk-ca/zk-ca.sh b/playbooks/roles/zk-ca/zk-ca.sh
new file mode 100755
index 0000000000..69c393bb02
--- /dev/null
+++ b/playbooks/roles/zk-ca/zk-ca.sh
@@ -0,0 +1,104 @@
+#!/bin/sh -e
+
+# Copyright 2020 Red Hat, Inc
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+# Manage a CA for Zookeeper
+
+CAROOT=$1
+SERVER=$2
+
+SUBJECT='/C=US/ST=California/L=Oakland/O=Company Name/OU=Org'
+TOOLSDIR=$(dirname $0)
+# Unlike the zk-ca.sh in zuul, we use the system openssl.cnf
+CONFIG=""
+
+make_ca() {
+    mkdir $CAROOT/demoCA
+    mkdir $CAROOT/demoCA/reqs
+    mkdir $CAROOT/demoCA/newcerts
+    mkdir $CAROOT/demoCA/crl
+    mkdir $CAROOT/demoCA/private
+    chmod 700 $CAROOT/demoCA/private
+    touch $CAROOT/demoCA/index.txt
+    touch $CAROOT/demoCA/index.txt.attr
+    mkdir $CAROOT/certs
+    mkdir $CAROOT/keys
+    mkdir $CAROOT/keystores
+    chmod 700 $CAROOT/keys
+    chmod 700 $CAROOT/keystores
+
+    openssl req $CONFIG -new -nodes -subj "$SUBJECT/CN=caroot" \
+            -keyout $CAROOT/demoCA/private/cakey.pem \
+            -out $CAROOT/demoCA/reqs/careq.pem
+    openssl ca $CONFIG -create_serial -days 3560 -batch -selfsign -extensions v3_ca \
+            -out $CAROOT/demoCA/cacert.pem \
+            -keyfile $CAROOT/demoCA/private/cakey.pem \
+            -infiles $CAROOT/demoCA/reqs/careq.pem
+    cp $CAROOT/demoCA/cacert.pem $CAROOT/certs
+}
+
+make_client() {
+    openssl req $CONFIG -new -nodes -subj "$SUBJECT/CN=client" \
+            -keyout $CAROOT/keys/clientkey.pem \
+            -out $CAROOT/demoCA/reqs/clientreq.pem
+    openssl ca $CONFIG -batch -policy policy_anything -days 3560 \
+            -out $CAROOT/certs/client.pem \
+            -infiles $CAROOT/demoCA/reqs/clientreq.pem
+}
+
+make_server() {
+    openssl req $CONFIG -new -nodes -subj "$SUBJECT/CN=$SERVER" \
+            -keyout $CAROOT/keys/${SERVER}key.pem \
+            -out $CAROOT/demoCA/reqs/${SERVER}req.pem
+    openssl ca $CONFIG -batch -policy policy_anything -days 3560 \
+            -out $CAROOT/certs/$SERVER.pem \
+            -infiles $CAROOT/demoCA/reqs/${SERVER}req.pem
+    cat $CAROOT/certs/$SERVER.pem $CAROOT/keys/${SERVER}key.pem \
+        > $CAROOT/keystores/$SERVER.pem
+}
+
+help() {
+    echo "$0 CAROOT [SERVER]"
+    echo
+    echo "  CAROOT is the path to a directory in which to store the CA"
+    echo "         and certificates."
+    echo "  SERVER is the FQDN of a server for which a certificate should"
+    echo "         be generated"
+}
+
+if [ ! -d "$CAROOT" ]; then
+    echo "CAROOT must be a directory"
+    help
+    exit 1
+fi
+
+cd $CAROOT
+CAROOT=`pwd`
+
+if [ ! -d "$CAROOT/demoCA" ]; then
+    echo 'Generate CA'
+    make_ca
+    echo 'Generate client certificate'
+    make_client
+fi
+
+if [ -f "$CAROOT/certs/$SERVER.pem" ]; then
+    echo "Certificate for $SERVER already exists"
+    exit 0
+fi
+
+if [ "$SERVER" != "" ]; then
+    make_server
+fi
diff --git a/playbooks/roles/zookeeper/files/zookeeper-compose/docker-compose.yaml b/playbooks/roles/zookeeper/files/zookeeper-compose/docker-compose.yaml
index ae6aa3a9c4..96e0ecbe00 100644
--- a/playbooks/roles/zookeeper/files/zookeeper-compose/docker-compose.yaml
+++ b/playbooks/roles/zookeeper/files/zookeeper-compose/docker-compose.yaml
@@ -12,3 +12,4 @@ services:
       - "/var/zookeeper/data:/data"
       - "/var/zookeeper/datalog:/datalog"
       - "/var/zookeeper/logs:/logs"
+      - "/var/zookeeper/tls:/tls"
diff --git a/playbooks/roles/zookeeper/tasks/main.yaml b/playbooks/roles/zookeeper/tasks/main.yaml
index 10ceaa2dba..1acc60d29e 100644
--- a/playbooks/roles/zookeeper/tasks/main.yaml
+++ b/playbooks/roles/zookeeper/tasks/main.yaml
@@ -27,6 +27,14 @@
     - data
     - datalog
     - logs
+    - tls
+- name: Generate ZooKeeper TLS cert
+  include_role:
+    name: zk-ca
+  vars:
+    zk_ca_cert_dir: /var/zookeeper/tls
+    zk_ca_cert_dir_owner: 10001
+    zk_ca_cert_dir_group: 10001
 - name: Write config
   template:
     src: zoo.cfg.j2
diff --git a/playbooks/roles/zookeeper/templates/zoo.cfg.j2 b/playbooks/roles/zookeeper/templates/zoo.cfg.j2
index 63dff1b82a..d943db37b8 100644
--- a/playbooks/roles/zookeeper/templates/zoo.cfg.j2
+++ b/playbooks/roles/zookeeper/templates/zoo.cfg.j2
@@ -23,6 +23,13 @@ maxClientCnxns=60
 standaloneEnabled=true
 admin.enableServer=true
 clientPort=2181
+secureClientPort=2281
+ssl.keyStore.location=/tls/keys/keystore.pem
+ssl.trustStore.location=/tls/certs/cacert.pem
 {% for host in groups['zookeeper'] %}
-server.{{ loop.index }}={{ (hostvars[host].ansible_default_ipv4.address) }}:2888:3888
+server.{{ loop.index }}={{ (hostvars[host].public_v4) }}:2888:3888
 {% endfor %}
+sslQuorum=true
+serverCnxnFactory=org.apache.zookeeper.server.NettyServerCnxnFactory
+ssl.quorum.keyStore.location=/tls/keys/keystore.pem
+ssl.quorum.trustStore.location=/tls/certs/cacert.pem
diff --git a/playbooks/roles/zuul/tasks/main.yaml b/playbooks/roles/zuul/tasks/main.yaml
index 4c1738a18c..1451caff02 100644
--- a/playbooks/roles/zuul/tasks/main.yaml
+++ b/playbooks/roles/zuul/tasks/main.yaml
@@ -21,6 +21,13 @@
     owner: "{{ zuul_user }}"
     group: "{{ zuul_group }}"
 
+- name: Generate ZooKeeper TLS cert
+  include_role:
+    name: zk-ca
+  vars:
+    zk_ca_cert_dir_owner: "{{ zuul_user_id }}"
+    zk_ca_cert_dir_group: "{{ zuul_group_id }}"
+
 - name: Create Zuul SSL dir
   file:
     state: directory
diff --git a/playbooks/roles/zuul/templates/zuul.conf.j2 b/playbooks/roles/zuul/templates/zuul.conf.j2
index eb8d512cb6..93f27de39d 100644
--- a/playbooks/roles/zuul/templates/zuul.conf.j2
+++ b/playbooks/roles/zuul/templates/zuul.conf.j2
@@ -28,7 +28,7 @@ relative_priority=true
 user=zuul
 
 [zookeeper]
-hosts={% for host in groups['zookeeper'] %}{{ (hostvars[host].ansible_default_ipv4.address) }}{% if not loop.last %},{% endif %}{% endfor %}
+hosts={% for host in groups['zookeeper'] %}{{ (hostvars[host].public_v4) }}:2181{% if not loop.last %},{% endif %}{% endfor %}
 
 session_timeout=40
 
diff --git a/testinfra/test_zookeeper.py b/testinfra/test_zookeeper.py
index 0890bfaa46..6327a71c8f 100644
--- a/testinfra/test_zookeeper.py
+++ b/testinfra/test_zookeeper.py
@@ -24,3 +24,7 @@ def test_id_file(host):
 def test_zk_listening(host):
     zk = host.socket("tcp://0.0.0.0:2181")
     assert zk.is_listening
+
+def test_zk_listening_ssl(host):
+    zk = host.socket("tcp://0.0.0.0:2281")
+    assert zk.is_listening