-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Skip certificate generation if SSL is disabled #428
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
Conversation
8.0/docker-entrypoint.sh
Outdated
@@ -111,7 +111,8 @@ if [ "$1" = 'mysqld' -a -z "$wantHelp" ]; then | |||
"$@" --initialize-insecure | |||
echo 'Database initialized' | |||
|
|||
if command -v mysql_ssl_rsa_setup > /dev/null && [ ! -e "$DATADIR/server-key.pem" ]; then | |||
sslEnabled="$(_get_config 'ssl' "$@")" | |||
if [ "$sslEnabled" = 'TRUE' ] && command -v mysql_ssl_rsa_setup > /dev/null && [ ! -e "$DATADIR/server-key.pem" ]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ltangvald I was testing this and found that in 8.0, it appears that --initalize-insecure
is creating a certificate itself if SSL is enabled -- should we remove this bit of code entirely from 8.0? (or are there cases where SSL is enabled and --initalize-insecure
doesn't create a server-key.pem
file?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Friendly ping @ltangvald ❤️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Friendly ping @ltangvald 😉
I was testing this and found that in 8.0, it appears that
--initalize-insecure
is creating a certificate itself if SSL is enabled -- should we remove this bit of code entirely from 8.0? (or are there cases where SSL is enabled and--initalize-insecure
doesn't create aserver-key.pem
file?)
I propose this is a less than ideal solution. The use case is: The user has their own certificates/credentials they wan't to supply to mysql AND they don't want to spend time generating new credentials which will never be used. So the user wants to turn off automatic generation of credentials BUT they do not want to turn off SSL. |
|
Rebased/updated to the new |
I think the separate mysql_ssl_rsa_setup utility was mostly just relevant before we started linking openssl (for licensing issues), so this may no longer be needed for any versions? Also, this behavior can be controlled with the auto_generate_certs and sha256_password_auto_generate_rsa_keys settings. So by virtue of me being over 2 years late cough, I think this probably isn't needed any more :) |
So if I'm understanding correctly, this change should be updated to the following, even on MySQL 5.6? diff --git a/.template.Debian/docker-entrypoint.sh b/.template.Debian/docker-entrypoint.sh
index 6845ee4..6024a90 100755
--- a/.template.Debian/docker-entrypoint.sh
+++ b/.template.Debian/docker-entrypoint.sh
@@ -163,13 +163,6 @@ docker_init_database_dir() {
"$@" --initialize-insecure
fi
mysql_note "Database files initialized"
-
- if command -v mysql_ssl_rsa_setup > /dev/null && [ ! -e "$DATADIR/server-key.pem" ]; then
- # https://github.com/mysql/mysql-server/blob/23032807537d8dd8ee4ec1c4d40f0633cd4e12f9/packaging/deb-in/extra/mysql-systemd-start#L81-L84
- mysql_note "Initializing certificates"
- mysql_ssl_rsa_setup --datadir="$DATADIR"
- mysql_note "Certificates initialized"
- fi
}
# Loads various settings that are used elsewhere in the script |
Right. Just need to verify that the resulting datadir is the same. For 5.6 I don't think that block does anything, since it doesn't have this functionality or mysql_ssl_rsa_setup. |
Updated with the removal of the 8.0: $ docker exec -it test sh -c 'ls -l /var/lib/mysql/*.pem'
-rw------- 1 mysql mysql 1676 Nov 20 22:21 /var/lib/mysql/ca-key.pem
-rw-r--r-- 1 mysql mysql 1112 Nov 20 22:21 /var/lib/mysql/ca.pem
-rw-r--r-- 1 mysql mysql 1112 Nov 20 22:21 /var/lib/mysql/client-cert.pem
-rw------- 1 mysql mysql 1680 Nov 20 22:21 /var/lib/mysql/client-key.pem
-rw------- 1 mysql mysql 1680 Nov 20 22:21 /var/lib/mysql/private_key.pem
-rw-r--r-- 1 mysql mysql 452 Nov 20 22:21 /var/lib/mysql/public_key.pem
-rw-r--r-- 1 mysql mysql 1112 Nov 20 22:21 /var/lib/mysql/server-cert.pem
-rw------- 1 mysql mysql 1676 Nov 20 22:21 /var/lib/mysql/server-key.pem 5.7: $ docker exec -it test sh -c 'ls -l /var/lib/mysql/*.pem'
-rw------- 1 mysql mysql 1676 Nov 20 22:22 /var/lib/mysql/ca-key.pem
-rw-r--r-- 1 mysql mysql 1112 Nov 20 22:22 /var/lib/mysql/ca.pem
-rw-r--r-- 1 mysql mysql 1112 Nov 20 22:22 /var/lib/mysql/client-cert.pem
-rw------- 1 mysql mysql 1680 Nov 20 22:22 /var/lib/mysql/client-key.pem
-rw------- 1 mysql mysql 1680 Nov 20 22:22 /var/lib/mysql/private_key.pem
-rw-r--r-- 1 mysql mysql 452 Nov 20 22:22 /var/lib/mysql/public_key.pem
-rw-r--r-- 1 mysql mysql 1112 Nov 20 22:22 /var/lib/mysql/server-cert.pem
-rw------- 1 mysql mysql 1676 Nov 20 22:22 /var/lib/mysql/server-key.pem |
I also verified with 8.0: $ docker exec -it test sh -c 'ls -l /var/lib/mysql/*.pem'
-rw------- 1 mysql mysql 1680 Nov 20 22:23 /var/lib/mysql/private_key.pem
-rw-r--r-- 1 mysql mysql 452 Nov 20 22:23 /var/lib/mysql/public_key.pem 5.7: $ docker exec -it test sh -c 'ls -l /var/lib/mysql/*.pem'
-rw------- 1 mysql mysql 1680 Nov 20 22:23 /var/lib/mysql/private_key.pem
-rw-r--r-- 1 mysql mysql 452 Nov 20 22:23 /var/lib/mysql/public_key.pem |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems fine. Is it safe to merge?
> I _think_ the separate mysql_ssl_rsa_setup utility was mostly just relevant before we started linking openssl (for licensing issues), so this may no longer be needed for any versions? Also, this behavior can be controlled with the auto_generate_certs and sha256_password_auto_generate_rsa_keys settings. So by virtue of me being over 2 years late *cough*, I think this probably isn't needed any more :)
Changes: - docker-library/mysql@6a2fb1d: Merge pull request docker-library/mysql#428 from infosiftr/ssl=0 - docker-library/mysql@2d86ed2: Drop explicit "mysql_ssl_rsa_setup" invocation - docker-library/mysql@b06a707: Merge pull request docker-library/mysql#821 from infosiftr/signed-by - docker-library/mysql@4afbe06: Switch from apt-key/trusted.gpg(.d) to signed-by - docker-library/mysql@0bd6d4c: Fix .gitattributes
Changes: - docker-library/mysql@4d243fc: Merge pull request docker-library/mysql#680 from infosiftr/oracle - docker-library/mysql@6a2fb1d: Merge pull request docker-library/mysql#428 from infosiftr/ssl=0 - docker-library/mysql@c77f720: Add Oracle Linux image variants - docker-library/mysql@2d86ed2: Drop explicit "mysql_ssl_rsa_setup" invocation - docker-library/mysql@b06a707: Merge pull request docker-library/mysql#821 from infosiftr/signed-by - docker-library/mysql@4afbe06: Switch from apt-key/trusted.gpg(.d) to signed-by - docker-library/mysql@0bd6d4c: Fix .gitattributes
Changes: - docker-library/mysql@131ffdd: Merge pull request docker-library/mysql#825 from infosiftr/oracle-gosu - docker-library/mysql@4e7d396: Update Oracle "gosu" to 1.14 - docker-library/mysql@4d243fc: Merge pull request docker-library/mysql#680 from infosiftr/oracle - docker-library/mysql@6a2fb1d: Merge pull request docker-library/mysql#428 from infosiftr/ssl=0 - docker-library/mysql@c77f720: Add Oracle Linux image variants - docker-library/mysql@2d86ed2: Drop explicit "mysql_ssl_rsa_setup" invocation - docker-library/mysql@b06a707: Merge pull request docker-library/mysql#821 from infosiftr/signed-by - docker-library/mysql@4afbe06: Switch from apt-key/trusted.gpg(.d) to signed-by - docker-library/mysql@0bd6d4c: Fix .gitattributes
Closes #379