chore: update CA cert import in README and test #2251
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| #!/bin/bash | ||
| # Script to import custom root CA certificates into the Java keystore. | ||
| # It handles .crt and .pem files mapped into the certs directory. | ||
|
|
||
| # Ensure JAVA_HOME is set, default to a common path if not | ||
| if [ -z "${JAVA_HOME}" ]; then | ||
| if [ -d "/opt/java/openjdk" ]; then | ||
| export JAVA_HOME="/opt/java/openjdk" | ||
| elif [ -d "/usr/lib/jvm/java-11-openjdk" ]; then | ||
| export JAVA_HOME="/usr/lib/jvm/java-11-openjdk" | ||
| fi | ||
| fi | ||
|
|
||
| if [ -z "${JAVA_HOME}" ]; then | ||
| echo "ERROR: JAVA_HOME is not set and could not be determined." >&2 | ||
| exit 0 # Don't crash the container | ||
| fi | ||
|
|
||
| : "${REF:="/usr/share/jenkins/ref"}" | ||
| : "${JENKINS_CUSTOM_CERTS_DIR:="${REF}/certs"}" | ||
|
|
||
| CACERTS_KEYSTORE="${JAVA_HOME}/lib/security/cacerts" | ||
| CACERTS_PASSWORD="${CACERTS_PASSWORD:-changeit}" | ||
|
|
||
| if [ ! -d "${JENKINS_CUSTOM_CERTS_DIR}" ]; then | ||
| exit 0 | ||
| fi | ||
|
|
||
| echo "Scanning for custom certificates in ${JENKINS_CUSTOM_CERTS_DIR}..." | ||
|
|
||
| # Find certs and process them one by one | ||
| # Using a temp file for the list to avoid pipe subshell issues with while loop | ||
| cert_list=$(mktemp) | ||
| find "${JENKINS_CUSTOM_CERTS_DIR}" -maxdepth 1 -type f \( -name "*.crt" -o -name "*.pem" \) 2>/dev/null > "${cert_list}" | ||
|
|
||
| while read -r cert_file; do | ||
| if [ -z "${cert_file}" ]; then continue; fi | ||
|
|
||
| cert_name=$(basename "${cert_file}") | ||
| alias="custom-${cert_name%.*}" | ||
|
|
||
| echo "Checking: ${cert_name} (alias: ${alias})" | ||
|
|
||
| # Check if already exists | ||
| if keytool -list -keystore "${CACERTS_KEYSTORE}" -storepass "${CACERTS_PASSWORD}" -alias "${alias}" >/dev/null 2>&1; then | ||
|
Contributor
There was a problem hiding this comment.
Contributor
Author
There was a problem hiding this comment. Agreed, i will update the script to explicitly use ${JAVA_HOME}/bin/keytool to ensure we are using the binary associated with the current JVM. |
||
| echo " Certificate alias '${alias}' already exists, skipping." | ||
| continue | ||
| fi | ||
|
|
||
| echo " Importing: ${cert_name} ..." | ||
| if keytool -importcert -noprompt -keystore "${CACERTS_KEYSTORE}" -storepass "${CACERTS_PASSWORD}" -alias "${alias}" -file "${cert_file}" >/dev/null 2>&1; then | ||
| echo " Successfully imported ${cert_name}" | ||
| else | ||
| echo " WARNING: Failed to import ${cert_name}. Check file format and permissions." >&2 | ||
| fi | ||
| done < "${cert_list}" | ||
|
|
||
| rm -f "${cert_list}" | ||
|
|
||
| echo "Custom CA certificate import process complete." | ||
| exit 0 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -136,10 +136,15 @@ ENV PATH="${JAVA_HOME}/bin:${PATH}" | |
| COPY --from=jre-and-war /javaruntime $JAVA_HOME | ||
| COPY --from=jre-and-war /war/jenkins.war /usr/share/jenkins/jenkins.war | ||
|
|
||
| # Allow the jenkins user to import custom CA certificates at runtime | ||
| RUN cp "${JAVA_HOME}/lib/security/cacerts" "${JAVA_HOME}/lib/security/cacerts.original" \ | ||
|
Contributor
There was a problem hiding this comment. What is the intent of this instruction exactly? I don't see why duplicating certificates and changing permissions has any reason to be?
Contributor
Author
There was a problem hiding this comment. I realize the chown introduces a security concern, if Jenkins is compromised, the attacker gains write access to the truststore. A safer alternative would be to copy cacerts to JENKINS_HOME at startup and point the JVM at that copy via -Djavax.net.ssl.trustStore, keeping the system keystore root-owned. Would you prefer that approach?
Contributor
There was a problem hiding this comment. No, I'm asking about the purpose of this line change:
Contributor
Author
There was a problem hiding this comment. We’re using chown so the non-root jenkins user actually has permission to update the system truststore (cacerts) at runtime. Without this, the keytool command in our entrypoint just hits a permissions wall and fails. I’ve also added a cacerts.original backup as a safety net, just in case we need to roll back to the default upstream state.
Contributor
There was a problem hiding this comment. Doing such a backup is more a VM/bare metal behavior. In a container image, usually aimed at being immutable, it's better to use volumes. Your proposal above to copy the cert store at runtime makes better sense. I'm more worried by the permissions change to I wonder if, instead, documenting how to generate the new cacert would'nt be better instead? (either with
Contributor
Author
There was a problem hiding this comment. Both approaches sound good to me. Once everyone align on the final scope, let me know and I'll get started on the changes.
Contributor
There was a problem hiding this comment.
I like the idea of documentation, especially with the idea of Docker compose. There are other places on jenkins.io that would benefit from more use of Docker compose. This would be a good step towards wider use of Docker compose in the Jenkins documentation.
Contributor
Author
There was a problem hiding this comment. Thanks for the alignment everyone! I'll:
Would that work?
Contributor
Author
There was a problem hiding this comment.
@MarkEWaite @timja @krisstern @dduportal Hi everyone, just a gentle ping on this! Please let me know if this plan looks good to you all. I'd love to get started on these changes once we have alignment.
Member
There was a problem hiding this comment. That looks good to me, thanks! |
||
| && chown ${user}:${group} "${JAVA_HOME}/lib/security/cacerts" | ||
|
|
||
| USER ${user} | ||
|
|
||
| COPY jenkins-support /usr/local/bin/jenkins-support | ||
| COPY jenkins.sh /usr/local/bin/jenkins.sh | ||
| COPY import-custom-certs.sh /usr/local/bin/import-custom-certs.sh | ||
| COPY jenkins-plugin-cli.sh /bin/jenkins-plugin-cli | ||
|
|
||
| ARG JENKINS_VERSION=2.550 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -139,3 +139,32 @@ runInScriptConsole() { | |
| @test "[${SUT_DESCRIPTION}] ensure that 'ps' command is available" { | ||
| command -v ps # Check for binary presence in the current PATH | ||
| } | ||
|
|
||
| @test "[${SUT_DESCRIPTION}] custom CA certificate is imported into keystore" { | ||
| local container_name test_cert_dir | ||
| container_name="$(get_sut_container_name)" | ||
| cleanup "${container_name}" | ||
| test_cert_dir="$(mktemp -d)" | ||
|
|
||
| # Generate a self-signed test CA certificate using keytool from the SUT image | ||
| # (avoids dependency on openssl being installed on the CI host) | ||
| docker run --rm --user root -v "${test_cert_dir}:/certs" "${SUT_IMAGE}" \ | ||
| bash -c 'keytool -genkeypair -alias testca -keyalg RSA -keysize 2048 \ | ||
| -dname "CN=Test CA" -validity 1 -keypass changeit \ | ||
| -keystore /tmp/test.jks -storepass changeit 2>/dev/null && \ | ||
| keytool -exportcert -alias testca -rfc \ | ||
| -keystore /tmp/test.jks -storepass changeit \ | ||
| -file /certs/test-ca.crt 2>/dev/null && \ | ||
| chmod -R 777 /certs' | ||
|
|
||
| # Start Jenkins with the test cert volume-mounted | ||
| docker run -d --name "${container_name}" \ | ||
| -v "${test_cert_dir}:/usr/share/jenkins/ref/certs:ro" \ | ||
| "${SUT_IMAGE}" | ||
|
|
||
| # Wait for import and verify the certificate was added to the keystore | ||
| retry 30 5 docker exec "${container_name}" \ | ||
| keytool -list -cacerts -storepass changeit -alias custom-test-ca | ||
|
|
||
| rm -rf "${test_cert_dir}" | ||
| } | ||
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.
Unneeded: we always set up
JAVA_HOMEin the Docker images. Feels like defensive programming or generated code. I suggest to simplify intojenkins.shThere 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.
You're right, this is unnecessary. Since JAVA_HOME is always set by the dockerfile, I will remove the entire detection block. I added it out of caution for general script portability,, but it doesnt belong in a controlled Docker image context.