SSL & Unit Variation - Contributing

[jbxonline]

Hey all! Really liking the images!

I've been testing out the Unit variation with PHP7.4 using my own local certificates generated using Minica.
I came across a bug, where the docs say if you are using your own certs, they should be placed in a folder that is mapped to /etc/ssl/private. However the code to check for a cert is looking in the config dir /etc/unit/config.d. I first thought about mapping to there, but then it overrides the templates and things start to get messy, so I have updated the 10-init-unit.sh script to check the /etc/ssl/private directory for $UNIT_CERTIFICATE_NAME.pem and if found, upload and use it. Small change, but works as expected and now follows what would be expected from the docs.

So that's the background. Here is my question...

I'm new to contributing to open source projects. So what do I do with my change? Am I able to simply create a new branch and upload my change and open a PR? Or do I need to do it in a fork?
Also, is there a way to contribute changes, corrections etc to the documentation?

Hoping to become an active contributor to the unit variation as I think I've noticed a couple of other bugs too, so looking forward to helping out.

Thanks in advance,

Jon

[jaydrogers]

Thanks for your willingness to help!

If you could jot down your notes on this thread on what needs to be fixed, I will circle back when I am ready for a PR. I have a big PR that I am about to merge (#311) and it's going to require a lot of changes.

I'd love to improve that 10-init-unit.sh script.

This is what I looked like when I was putting it together 🤣

[jbxonline]

Hey Jay, no problem at all, happy to help out on this.

I'm looking to use this base image for our app initially in local envs, but also in productions environments in the near future, so it's worth my while spending some of my work hours on this project to ensure everything is running well :)

Currently, I've just looked into the SSL side of things. We're using minica to create our certs, as this enables us to trust the root ca locally and use a custom domain, so this means I need to provide our own cert bundle to the docker image.

Here is an extract from my docker-compose:

    environment:
      PHP_POST_MAX_SIZE: "500M"
      PHP_UPLOAD_MAX_FILE_SIZE: "500M"
      SSL_MODE: "full"
      UNIT_CERTIFICATE_NAME: "bundle"
    volumes:
      - ./laravel:/var/www/html:cached
      - ./certs:/etc/ssl/private

And here is the diff with my changes:

diff --git a/src/variations/unit/etc/entrypoint.d/10-init-unit.sh b/src/variations/unit/etc/entrypoint.d/10-init-unit.sh
index 361ba2e..744885e 100644
--- a/src/variations/unit/etc/entrypoint.d/10-init-unit.sh
+++ b/src/variations/unit/etc/entrypoint.d/10-init-unit.sh
@@ -104,13 +104,13 @@ configure_unit() {
     # this curl call will get a reply once unit is fully launched
     /usr/bin/curl -s -X GET --unix-socket "$UNIT_SOCKET_LOCATION" http://localhost/
 
-    echo "$script_name: Looking for certificate bundles in $UNIT_CONFIG_DIRECTORY..."
-    for f in $(/usr/bin/find "$UNIT_CONFIG_DIRECTORY" -type f -name "*.pem"); do
+    echo "$script_name: Looking for certificate bundles in /etc/ssl/private..."
+    for f in $(/usr/bin/find "/etc/ssl/private" -type f -name "$UNIT_CERTIFICATE_NAME.pem"); do
         echo "$script_name: Uploading certificates bundle: $f"
         curl_put "--data-binary" "$f" "certificates/$(basename $f .pem)"
     done
 
-    set_debug_output "/usr/bin/find $UNIT_CONFIG_DIRECTORY -type f -name \"*.pem\""
+    set_debug_output "/usr/bin/find /etc/ssl/private -type f -name \"$UNIT_CERTIFICATE_NAME.pem\""
 
     echo "$script_name: Looking for JavaScript modules in $UNIT_CONFIG_DIRECTORY..."
     for f in $(/usr/bin/find $UNIT_CONFIG_DIRECTORY -type f -name "*.js"); do
@@ -154,7 +154,7 @@ configure_unit() {
 }
 
 validate_ssl(){
-    available_ssl_bundles=$(/usr/bin/find "$UNIT_CONFIG_DIRECTORY" -type f -name "*.pem")
+    available_ssl_bundles=$(/usr/bin/find "/etc/ssl/private" -type f -name "$UNIT_CERTIFICATE_NAME.pem")

The main points here is that we need to look inside /etc/ssl/private for the certs, not the config directory and use the bundle name from the $UNIT_CERTIFICATE_NAME var. I'm thinking that the var should probably also include the file extension, rather than .pem being manually appended. That might be more natural for users adding the name in docker-compose. I would expect to include the extension there.

Thoughts?

Jon

[jaydrogers]

Thanks for your patience @jbxonline! I was able to get that PR merged and shipped 😃

Looking at your suggestions, I have a few comments:

docker-php/src/variations/unit/etc/entrypoint.d/10-init-unit.sh

Lines 156 to 168 in 068e658

	validate_ssl(){
	available_ssl_bundles=$(/usr/bin/find "$UNIT_CONFIG_DIRECTORY" -type f -name "*.pem")
	if [ -n "$available_ssl_bundles" ]; then
	echo "ℹ️ NOTICE ($script_name): SSL Certbundle already exists, so we'll use the existing files."
	return 0
	fi
	echo "$script_name: 🔐 SSL Certbundle not found. Generating self-signed SSL bundle..."
	mkdir -p /etc/ssl/private/
	openssl req -x509 -subj "/C=US/ST=Wisconsin/L=Milwaukee/O=IT/CN=default.test" -nodes -newkey rsa:2048 -keyout "/etc/ssl/private/$UNIT_CERTIFICATE_NAME.key" -out "/etc/ssl/private/$UNIT_CERTIFICATE_NAME.crt" -days 365 >/dev/null 2>&1
	cat "/etc/ssl/private/$UNIT_CERTIFICATE_NAME.key" "/etc/ssl/private/$UNIT_CERTIFICATE_NAME.crt" > "$UNIT_CONFIG_DIRECTORY/$UNIT_CERTIFICATE_NAME.pem"
	}

NGINX Unit Certificate Location

I like where this is going, but the loaded certificates are required to be in the NGINX Unit config to run (I'm 70% confident on that)

You can see in line 167, if you provide your own certs in /etc/ssl/private, I do the combining of the certs into a "bundle" so the public and private key are loaded from a single file (https://unit.nginx.org/configuration/#ssl-tls-configuration)

So your best place is to place your files at /etc/ssl/private, in two separate files so the script can add them in for you.

I'm thinking that the var should probably also include the file extension, rather than .pem being manually appended. That might be more natural for users adding the name in docker-compose. I would expect to include the extension there.

I think leaving this out will be best. Most people use .pem. I've seen others use .key or .public and I don't want to make hard assumptions on their use case.

Contributing

Thanks so much for your willingness to help!

I'm new to contributing to open source projects. So what do I do with my change? Am I able to simply create a new branch and upload my change and open a PR? Or do I need to do it in a fork?

Check this video out, it has a great overview https://www.youtube.com/watch?v=CML6vfKjQss

Also, is there a way to contribute changes, corrections etc to the documentation?

Absolutely! Check our contributing guide, where it links to where the docs are located: https://serversideup.net/open-source/docker-php/docs/getting-started/contributing

Let me know if what I shared makes sense and if you're happy with this approach 👍

[jbxonline]

You can see in line 167, if you provide your own certs in /etc/ssl/private, I do the combining of the certs into a "bundle"

Ok, I think this is where the main issue is. Actually what is happening, is that if I provide my own certs in the manner described, you still generate a self signed and use that, overwriting the certificates provided. Essentially , you're first checking if the certificate has been provided in the config directory (which isn't possible) then generating a self signed.

Thanks for the contributing links. Using these, I've created a PR from my fork for you to take a look at here: #321

Let me know what you think!

BTW: I'm on UK time, so may not always reply later in the day 👍🏻