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

docker: Restore ability to generate SSL certs with LetsEncrypt. #391

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

klardotsh
Copy link
Member

@klardotsh klardotsh commented Feb 28, 2023

Zulip Server 4.9+ regressed Docker setups by always creating a /etc/letsencrypt directory in the top layer of the Docker container, meaning it couldn't be symlinked over from the volume mount. Since that volume mount has useful properties (providing and/or overriding LetsEncrypt setting), restore it and copy the in-image configs into the volume as defaults if and only if those files don't already exist in the volume.

Fixes #381.

Testing Plan

I applied the following diff:

diff --git a/docker-compose.yml b/docker-compose.yml
index 45f5949..9c2b16a 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -49,7 +49,8 @@ services:
     volumes:
       - "redis:/data:rw"
   zulip:
-    image: "zulip/docker-zulip:6.1-0"
+    #image: "zulip/docker-zulip:6.1-0"
+    image: "zulip:local"
     restart: unless-stopped
     build:
       context: .
@@ -66,7 +67,8 @@ services:
       DB_HOST: "database"
       DB_HOST_PORT: "5432"
       DB_USER: "zulip"
-      SSL_CERTIFICATE_GENERATION: "self-signed"
+      #SSL_CERTIFICATE_GENERATION: "self-signed"
+      SSL_CERTIFICATE_GENERATION: "certbot"
       SETTING_MEMCACHED_LOCATION: "memcached:11211"
       SETTING_RABBITMQ_HOST: "rabbitmq"
       SETTING_REDIS_HOST: "redis"

... and then built a zulip:local image from this repo (with no build args). It appears to work: /var/lib/docker/volumes/docker-zulip_zulip/_data/letsencrypt/ contains the cli.ini that originates from a Zulip Server install, and I see the following log output:

docker-compose log output
docker-zulip-zulip-1      | Scheduling LetsEncrypt cert generation ...
docker-zulip-zulip-1      | Generating self-signed certificates ...
docker-zulip-zulip-1      | + is_redhat=false
docker-zulip-zulip-1      | + '[' -e /etc/redhat-release ']'
docker-zulip-zulip-1      | + SSLDIR=/etc/ssl
docker-zulip-zulip-1      | + KEYFILE=/etc/ssl/private/zulip.key
docker-zulip-zulip-1      | + CERTFILE=/etc/ssl/certs/zulip.combined-chain.crt
docker-zulip-zulip-1      | + '[' -n '' ']'
docker-zulip-zulip-1      | + '[' -z '' ']'
docker-zulip-zulip-1      | + '[' -e /etc/ssl/private/zulip.key ']'
docker-zulip-zulip-1      | + '[' -e /etc/ssl/certs/zulip.combined-chain.crt ']'
docker-zulip-zulip-1      | + rm -f /etc/ssl/private/zulip.key /etc/ssl/certs/zulip.combined-chain.crt
docker-zulip-zulip-1      | + [[ localhost.localdomain =~ ^(([0-9]+\.){3}[0-9]+)(:[0-9]+)?$ ]]
docker-zulip-zulip-1      | + [[ localhost.localdomain =~ ^\[([^][]*)](:[0-9]+)?$ ]]
docker-zulip-zulip-1      | + [[ localhost.localdomain =~ ^([^:]+)(:[0-9]+)?$ ]]
docker-zulip-zulip-1      | + subjectAltName=DNS:localhost.localdomain
docker-zulip-zulip-1      | ++ mktemp
docker-zulip-zulip-1      | + config=/tmp/tmp.5Dxc2cuRb4
docker-zulip-zulip-1      | + trap 'rm -f "$config"' EXIT
docker-zulip-zulip-1      | + cat
docker-zulip-zulip-1      | + '[' false = true ']'
docker-zulip-zulip-1      | + apt-get install -y openssl
docker-zulip-zulip-1      | Reading package lists...
docker-zulip-zulip-1      | Building dependency tree...
docker-zulip-zulip-1      | Reading state information...
docker-zulip-zulip-1      | openssl is already the newest version (1.1.1f-1ubuntu2.17).
docker-zulip-zulip-1      | openssl set to manually installed.
docker-zulip-zulip-1      | 0 upgraded, 0 newly installed, 0 to remove and 0 not upgraded.
docker-zulip-zulip-1      | + openssl req -new -x509 -config /tmp/tmp.5Dxc2cuRb4 -days 3650 -nodes -sha256 -out /etc/ssl/certs/zulip.combined-chain.crt -keyout /etc/ssl/private/zulip.key
docker-zulip-zulip-1      | Generating a RSA private key
docker-zulip-zulip-1      | ..+++++
docker-zulip-zulip-1      | ..................+++++
docker-zulip-zulip-1      | writing new private key to '/etc/ssl/private/zulip.key'
docker-zulip-zulip-1      | -----
docker-zulip-zulip-1      | + chmod 644 /etc/ssl/certs/zulip.combined-chain.crt
docker-zulip-zulip-1      | + chmod 640 /etc/ssl/private/zulip.key
docker-zulip-zulip-1      | + rm -f /tmp/tmp.5Dxc2cuRb4
docker-zulip-zulip-1      | Self-signed certificate generation succeeded.
docker-zulip-zulip-1      | Certificates configuration succeeded.

entrypoint.sh Outdated Show resolved Hide resolved
entrypoint.sh Outdated Show resolved Hide resolved
Zulip Server 4.9+ regressed Docker setups by always creating a
/etc/letsencrypt directory in the top layer of the Docker container,
meaning it couldn't be symlinked over from the volume mount. Since that
volume mount has useful properties (providing and/or overriding
LetsEncrypt setting), restore it and copy the in-image configs into the
volume as defaults if and only if those files don't already exist in the
volume.

Fixes zulip#381.
@andersk
Copy link
Member

andersk commented Feb 28, 2023

This didn’t work for me.

On the first docker compose up, it generated a certificate, but did not restart nginx, so it was still serving the self-signed certificate.

After docker compose down; docker compose up, I got

docker-zulip-zulip-1      | === Begin Run Phase ===
docker-zulip-zulip-1      | Waiting for nginx to come online before generating certbot certificate ...
docker-zulip-zulip-1      | Starting Zulip using supervisor with "/etc/supervisor/supervisord.conf" config ...
…
docker-zulip-zulip-1      | Generating LetsEncrypt/certbot certificate ...
docker-zulip-zulip-1      | + case " $os_id $os_id_like " in
docker-zulip-zulip-1      | + apt-get update
…
docker-zulip-zulip-1      | + apt-get install -y certbot
…
docker-zulip-zulip-1      | + certbot certonly --webroot --webroot-path=/var/lib/zulip/certbot-webroot/ -d andersk.zulipdev.org -m [email protected] --agree-tos --force-interactive --no-eff-email
docker-zulip-zulip-1      | Saving debug log to /var/log/letsencrypt/letsencrypt.log
docker-zulip-zulip-1      | Plugins selected: Authenticator webroot, Installer None
docker-zulip-zulip-1      | Cert not yet due for renewal  
docker-zulip-zulip-1      | 
docker-zulip-zulip-1      | You have an existing certificate that has exactly the same domains or certificate name you requested and isn't close to expiry.
docker-zulip-zulip-1      | (ref: /etc/letsencrypt/renewal/andersk.zulipdev.org.conf)
docker-zulip-zulip-1      | 
docker-zulip-zulip-1      | What would you like to do?
docker-zulip-zulip-1      | - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
docker-zulip-zulip-1      | 1: Keep the existing certificate for now
docker-zulip-zulip-1      | 2: Renew & replace the cert (limit ~5 per 7 days)
docker-zulip-zulip-1      | - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
docker-zulip-zulip-1      | Select the appropriate number [1-2] then [enter] (press 'c' to cancel): An unexpected error occurred:
docker-zulip-zulip-1      | EOFError
docker-zulip-zulip-1      | Please see the logfiles in /var/log/letsencrypt for more details.

and it was still serving a self-signed certificate.

After docker compose stop; docker compose up, I got

docker-zulip-zulip-1      | === Begin Initial Configuration Phase ===
docker-zulip-zulip-1      | ln: failed to create symbolic link '/etc/letsencrypt': File exists
docker-zulip-zulip-1 exited with code 1

@klardotsh
Copy link
Member Author

🤔 I'll take another look. I haven't seen either of those error messages so far, but maybe I need more up-stop-up-stop cycles to trigger it.

@klardotsh
Copy link
Member Author

I reproed at least the lack of nginx restarting issue on a DO droplet. I'm now playing with #142 and a potential revert of the underlying zulip/zulip changes (I think there's a path to --standalone followed by a conversion to webroot, but want to confirm on "bare metal" before going the rest of the way down that rabbit hole integrating the changes into Zulip)

@andersk
Copy link
Member

andersk commented Mar 1, 2023

@napter
Copy link

napter commented Nov 14, 2023

This should really be merged. Currently, if you rebuild the docker image you lose your certs and its a whole mess to get them back

@andersk
Copy link
Member

andersk commented Nov 15, 2023

The certbot brokenness is a high importance bug that needs to be solved, but this PR does not solve it. See above comments and the linked discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

certbot support broken in ≥ 4.9-0
3 participants