diff --git a/pkg/api/tlscert_test.go b/pkg/api/tlscert_test.go index 0a739e9d..01d398d8 100644 --- a/pkg/api/tlscert_test.go +++ b/pkg/api/tlscert_test.go @@ -193,5 +193,131 @@ func TestCertReloaderDirectly(t *testing.T) { So(err, ShouldBeNil) So(cert, ShouldNotBeNil) }) + + Convey("GetCertificateFunc should handle only cert file modification", func() { + reloader, err := api.NewCertReloader(certPath, keyPath) + So(err, ShouldBeNil) + + getCert := reloader.GetCertificateFunc() + initialCert, err := getCert(nil) + So(err, ShouldBeNil) + So(initialCert, ShouldNotBeNil) + + // Wait to ensure modification time will be different + time.Sleep(2 * time.Second) + + // Modify only the cert file (touch it to update mtime) + // Generate new cert with same key + newServerOpts := &tlsutils.CertificateOptions{ + Hostname: "127.0.0.1", + CommonName: "Updated Cert Only", + NotAfter: time.Now().AddDate(1, 0, 0), + } + // Read the existing key + keyData, err := os.ReadFile(keyPath) + So(err, ShouldBeNil) + + // Generate new cert using the existing key + err = tlsutils.GenerateServerCertToFile(caCertPEM, caKeyPEM, certPath, keyPath, newServerOpts) + So(err, ShouldBeNil) + + // Restore original key + err = os.WriteFile(keyPath, keyData, 0o600) + So(err, ShouldBeNil) + + // Get certificate again - should reload + updatedCert, err := getCert(nil) + So(err, ShouldBeNil) + So(updatedCert, ShouldNotBeNil) + }) + + Convey("GetCertificateFunc should handle concurrent access", func() { + reloader, err := api.NewCertReloader(certPath, keyPath) + So(err, ShouldBeNil) + + getCert := reloader.GetCertificateFunc() + + // Launch multiple goroutines to access certificate concurrently + done := make(chan error, 10) + for i := 0; i < 10; i++ { + go func() { + var lastErr error + for j := 0; j < 100; j++ { + cert, err := getCert(nil) + if err != nil || cert == nil { + lastErr = err + break + } + } + done <- lastErr + }() + } + + // Wait for all goroutines to complete and check for errors + for i := 0; i < 10; i++ { + err := <-done + So(err, ShouldBeNil) + } + }) + + Convey("GetCertificateFunc should not reload if files haven't changed", func() { + reloader, err := api.NewCertReloader(certPath, keyPath) + So(err, ShouldBeNil) + + getCert := reloader.GetCertificateFunc() + + // Get certificate multiple times + cert1, err := getCert(nil) + So(err, ShouldBeNil) + So(cert1, ShouldNotBeNil) + + cert2, err := getCert(nil) + So(err, ShouldBeNil) + So(cert2, ShouldNotBeNil) + + cert3, err := getCert(nil) + So(err, ShouldBeNil) + So(cert3, ShouldNotBeNil) + + // All should return the same certificate instance (pointer equality) + So(cert1, ShouldEqual, cert2) + So(cert2, ShouldEqual, cert3) + }) + + Convey("GetCertificateFunc should reload when key file changes", func() { + reloader, err := api.NewCertReloader(certPath, keyPath) + So(err, ShouldBeNil) + + getCert := reloader.GetCertificateFunc() + initialCert, err := getCert(nil) + So(err, ShouldBeNil) + So(initialCert, ShouldNotBeNil) + + // Wait to ensure modification time will be different + time.Sleep(2 * time.Second) + + // Generate completely new cert and key + newServerOpts := &tlsutils.CertificateOptions{ + Hostname: "127.0.0.1", + CommonName: "New Key Cert", + NotAfter: time.Now().AddDate(1, 0, 0), + } + err = tlsutils.GenerateServerCertToFile(caCertPEM, caKeyPEM, certPath, keyPath, newServerOpts) + So(err, ShouldBeNil) + + // Get certificate again - should reload due to key change + updatedCert, err := getCert(nil) + So(err, ShouldBeNil) + So(updatedCert, ShouldNotBeNil) + + // Verify certificates are different + initialLeaf, err := x509.ParseCertificate(initialCert.Certificate[0]) + So(err, ShouldBeNil) + updatedLeaf, err := x509.ParseCertificate(updatedCert.Certificate[0]) + So(err, ShouldBeNil) + + So(initialLeaf.Subject.CommonName, ShouldEqual, "Initial Cert") + So(updatedLeaf.Subject.CommonName, ShouldEqual, "New Key Cert") + }) }) } diff --git a/test/blackbox/tls_cert_reload.bats b/test/blackbox/tls_cert_reload.bats new file mode 100644 index 00000000..36df1e0c --- /dev/null +++ b/test/blackbox/tls_cert_reload.bats @@ -0,0 +1,211 @@ +load helpers_zot +load ../port_helper + +function verify_prerequisites { + if [ ! $(command -v curl) ]; then + echo "you need to install curl as a prerequisite to running the tests" >&3 + return 1 + fi + + if [ ! $(command -v openssl) ]; then + echo "you need to install openssl as a prerequisite to running the tests" >&3 + return 1 + fi + + return 0 +} + +# Generate TLS certificates for testing +function generate_certs() { + local cert_dir=$1 + mkdir -p ${cert_dir} + + # Generate CA certificate + openssl req -newkey rsa:2048 -nodes -days 365 -x509 \ + -keyout ${cert_dir}/ca.key \ + -out ${cert_dir}/ca.crt \ + -subj "/CN=Test CA" 2>/dev/null + + # Generate initial server certificate (version 1) + openssl req -newkey rsa:2048 -nodes \ + -keyout ${cert_dir}/server.key \ + -out ${cert_dir}/server.csr \ + -subj "/OU=TestServer/CN=Server v1" 2>/dev/null + + openssl x509 -req -days 365 -sha256 \ + -in ${cert_dir}/server.csr \ + -CA ${cert_dir}/ca.crt \ + -CAkey ${cert_dir}/ca.key \ + -CAcreateserial \ + -out ${cert_dir}/server.cert \ + -extfile <(echo subjectAltName = IP:127.0.0.1) 2>/dev/null +} + +# Generate new server certificate with different CN +function regenerate_server_cert() { + local cert_dir=$1 + local version=$2 + + # Generate new server certificate (version 2) + openssl req -newkey rsa:2048 -nodes \ + -keyout ${cert_dir}/server.key \ + -out ${cert_dir}/server.csr \ + -subj "/OU=TestServer/CN=Server v${version}" 2>/dev/null + + openssl x509 -req -days 365 -sha256 \ + -in ${cert_dir}/server.csr \ + -CA ${cert_dir}/ca.crt \ + -CAkey ${cert_dir}/ca.key \ + -CAcreateserial \ + -out ${cert_dir}/server.cert \ + -extfile <(echo subjectAltName = IP:127.0.0.1) 2>/dev/null +} + +function setup_file() { + # Verify prerequisites are available + if ! $(verify_prerequisites); then + exit 1 + fi + + # Generate certificates + local cert_dir=${BATS_FILE_TMPDIR}/certs + generate_certs ${cert_dir} + + # Setup zot server with TLS + local zot_root_dir=${BATS_FILE_TMPDIR}/zot + local zot_config_file=${BATS_FILE_TMPDIR}/zot_config.json + zot_port=$(get_free_port_for_service "zot") + echo ${zot_port} > ${BATS_FILE_TMPDIR}/zot.port + + mkdir -p ${zot_root_dir} + + cat > ${zot_config_file}</dev/null | \ + openssl x509 -noout -subject 2>/dev/null | grep "Server v1") + + # Verify we got the initial certificate (v1) + [ ! -z "$cert_subject" ] +} + +@test "reload certificate and verify new cert is used" { + zot_port=`cat ${BATS_FILE_TMPDIR}/zot.port` + cert_dir=${BATS_FILE_TMPDIR}/certs + + # Verify initial connection works + run curl --cacert ${cert_dir}/ca.crt https://127.0.0.1:${zot_port}/v2/ + [ "$status" -eq 0 ] + + # Wait a moment to ensure modification time will be different + sleep 2 + + # Generate new certificate with different CommonName + regenerate_server_cert ${cert_dir} 2 + + # Wait for certificate to be detected and reloaded + sleep 2 + + # Verify connection still works with new certificate + run curl --cacert ${cert_dir}/ca.crt https://127.0.0.1:${zot_port}/v2/ + [ "$status" -eq 0 ] + + # Get certificate subject from server + cert_subject=$(echo | openssl s_client -connect 127.0.0.1:${zot_port} -showcerts 2>/dev/null | \ + openssl x509 -noout -subject 2>/dev/null | grep "Server v2") + + # Verify we got the new certificate (v2) + [ ! -z "$cert_subject" ] +} + +@test "verify multiple certificate reloads work" { + zot_port=`cat ${BATS_FILE_TMPDIR}/zot.port` + cert_dir=${BATS_FILE_TMPDIR}/certs + + for i in 3 4 5; do + # Generate new certificate + regenerate_server_cert ${cert_dir} ${i} + + # Wait for reload + sleep 2 + + # Verify connection works + run curl --cacert ${cert_dir}/ca.crt https://127.0.0.1:${zot_port}/v2/ + [ "$status" -eq 0 ] + + # Verify new certificate is in use + cert_subject=$(echo | openssl s_client -connect 127.0.0.1:${zot_port} -showcerts 2>/dev/null | \ + openssl x509 -noout -subject 2>/dev/null | grep "Server v${i}") + [ ! -z "$cert_subject" ] + done +} + +@test "verify server continues working if certificate reload fails" { + zot_port=`cat ${BATS_FILE_TMPDIR}/zot.port` + cert_dir=${BATS_FILE_TMPDIR}/certs + + # Get current certificate version + cert_subject_before=$(echo | openssl s_client -connect 127.0.0.1:${zot_port} -showcerts 2>/dev/null | \ + openssl x509 -noout -subject 2>/dev/null) + + # Temporarily remove certificate files (will cause reload to fail) + mv ${cert_dir}/server.cert ${cert_dir}/server.cert.backup + + # Wait and try to connect - should still work with old certificate + sleep 2 + run curl --cacert ${cert_dir}/ca.crt https://127.0.0.1:${zot_port}/v2/ + [ "$status" -eq 0 ] + + # Restore certificate + mv ${cert_dir}/server.cert.backup ${cert_dir}/server.cert + + # Verify still using old certificate + cert_subject_after=$(echo | openssl s_client -connect 127.0.0.1:${zot_port} -showcerts 2>/dev/null | \ + openssl x509 -noout -subject 2>/dev/null) + [ "$cert_subject_before" = "$cert_subject_after" ] +}