CLIP-1826: Add user provided certificates to the default Java truststore #663

bianchi2 · 2023-09-08T05:43:26Z

While it is possible to create a custom truststore and mount it as a secret using additionalFiles and add additionalJvmArguments, it takes a lot of steps. This PR makes adding custom certs a lot easier.

<product>:
  additionalCertificates:
     secretName

If secretName is defined (it must be the secret that has been pre-created outside helm), then:

an additional init container is added
this container shares a runtime volume with the main container at /var/ssl
the secret is mounted at /tmp/crt in the init container
keytool command imports all files from /tmp/crt into the default java truststore at $JAVA_HOME/lib/security/cacerts
$JAVA_HOME/lib/security/cacerts is copied to /var/ssl/cacerts which is available to the main container
-Djavax.net.ssl.trustStore=/var/ssl/cacerts sysprop is added to JVM args to instruct the server to use a custom truststore

The secret can contain multiple certificates, for example:

data:
  stg.crt: base64encodedcrt
  dev.crt: base64encodedcrt

Certs will be mounted as /tmp/crt/stg.crt, /tmp/crt/dev.srt. When certs are imported, filenames are used as aliases.

In addition to unit tests, this feature is enabled in KinD tests to make sure that the init containers don't fail and main containers can find the custom trust store. Here's KinD run.

Checklist

I have added unit tests
I have applied the change to all applicable products
The E2E test has passed (use e2e label)

nanux

Great improvement @bianchi2! It is great to streamline the experience of setting up the certificates for all users.

Do you think it would be worth adding a paragraph or two to the user facing documentation?
Something along the lines:

How to use custom certificates

nanux · 2023-09-16T04:20:35Z

src/main/charts/bamboo/templates/_helpers.tpl

+{{- if .Values.bamboo.additionalCertificates.customCmd}}
+{{ .Values.bamboo.additionalCertificates.customCmd}}
+{{- else }}
+set -e; for crt in /tmp/crt/*.*; do echo "Adding $crt to keystore"; keytool -import -cacerts -storepass changeit -noprompt -alias $(echo $(basename $crt)) -file $crt; done; cp $JAVA_HOME/lib/security/cacerts /var/ssl/cacerts


What happens when customer has or wants to define different passkey than the default changeit?

We assume that the official image is used. In case of a custom truststore, it should be already mounted as a secret. So, if a user already mounts a custom truststore, importing new certs happens outside helm then.

Right and if customer wants to do something specific in this command, they can always redefine the customCmd.

nanux · 2023-09-16T04:22:35Z

src/main/charts/bamboo/templates/statefulset.yaml

@@ -55,6 +55,18 @@ spec:
          command: ["sh", "-c", {{ include "bamboo.sharedHome.permissionFix.command" . | quote }}]
        {{- end }}
        {{- include "common.jmx.initContainer" . | nindent 8 }}
+        {{- if .Values.bamboo.additionalCertificates.secretName }}
+        - name: import-certs
+          image: {{ include "bamboo.image" . | quote }}


I guess we are running this image because we want to run the keytool util? I am just wondering whether it is necessary to spin up the large product image to run a simple command.

Great question :) In fact, it does not really matter if we use temurin image for keytool init container. This won't save us any time since product image needs to be pulled anyways. It's a matter of which phase takes longer - init or containers start. If we pull product image in the init phase, container runtime will have it available when the product container starts. No gains/benefits with either of the approaches, imho.

Thanks for confirming this. I was thinking that because we pull the image anyway, it is faster on the pull time but for a very short period we take more resources than we need but as the execution is super quick, it is negligible.

Last thing, if this fails, does it stop container execution, and how will customers get to the error logs (are they sufficient to understand why it failed?)

bianchi2 · 2023-09-16T20:17:46Z

@nanux yes, adding docs with examples is a good idea.

UPDATE. Docs added

nanux · 2023-09-17T22:39:54Z

src/main/charts/bamboo/templates/_helpers.tpl

+{{- if .Values.bamboo.additionalCertificates.customCmd}}
+{{ .Values.bamboo.additionalCertificates.customCmd}}
+{{- else }}
+set -e; for crt in /tmp/crt/*.*; do echo "Adding $crt to keystore"; keytool -import -cacerts -storepass changeit -noprompt -alias $(echo $(basename $crt)) -file $crt; done; cp $JAVA_HOME/lib/security/cacerts /var/ssl/cacerts


Right and if customer wants to do something specific in this command, they can always redefine the customCmd.

nanux · 2023-09-17T22:44:54Z

src/main/charts/bamboo/templates/statefulset.yaml

@@ -55,6 +55,18 @@ spec:
          command: ["sh", "-c", {{ include "bamboo.sharedHome.permissionFix.command" . | quote }}]
        {{- end }}
        {{- include "common.jmx.initContainer" . | nindent 8 }}
+        {{- if .Values.bamboo.additionalCertificates.secretName }}
+        - name: import-certs
+          image: {{ include "bamboo.image" . | quote }}


Thanks for confirming this. I was thinking that because we pull the image anyway, it is faster on the pull time but for a very short period we take more resources than we need but as the execution is super quick, it is negligible.

Last thing, if this fails, does it stop container execution, and how will customers get to the error logs (are they sufficient to understand why it failed?)

bianchi2 · 2023-09-17T23:28:35Z

We do set -e, and if the init container fails other containers do not start. It will be possible to get logs of the failed init container to troubleshoot the error. And yes, customCmd is for use cases when the default cmd fails and it needs to be overridden.

errcode1202 · 2023-09-18T00:35:05Z

docs/docs/userguide/CONFIGURATION.md

+
+## :material-certificate: Self Signed Certificates
+
+Accessing applications or websites that are encrypted with SSL using certificates not signed by a public authority will result in a connection error. The stacktrace will contain the following:
+
+```shell
+caused by: javax.net.ssl.SSLHandshakeException: sun.security.validator.ValidatorException: PKIX path building failed:sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target
+```
+
+To fix the issue, self signed certificates need to be added to Java truststore. First off, you need to create a [Kubernetes secret](https://kubernetes.io/docs/concepts/configuration/secret/){.external} containing base64-encoded certificate(s). Here's an example [kubectl command](https://kubernetes.io/docs/tasks/configmap-secret/managing-secret-using-kubectl/#use-source-files){.external} to create a secret from 2 local files:
+
+```shell
+kubectl create secret generic dev-certificates \
+    --from-file=stg.crt=./stg.crt \
+    --from-file=dev.crt=./dev.crt -n $namespace
+```
+
+The resulting secret will have the following data:
+
+```yaml
+data:
+  stg.crt: base64encodedstgcrt
+  dev.crt: base64encodeddevcrt
+```
+
+!!!info "You can have as many keys (certificates) in the secret as required. All keys will be mounted as files to `/tmp/crt` in the container and imported into Java truststore. In the example above, certificates will be mounted as `/tmp/crt/stg.crt` and `/tmp/crt/dev.crt`. File extension in the secret keys does not matter as long as the file is a valid certificate."
+
+Once done, provide the secret name in Helm values: 
+
+```yaml
+jira:
+  additionalCertificates:
+     secretName: dev-certificates
+```
+
+Helm chart will add additional `volumeMounts` and `volumes` to the pod(s), as well as an extra init container that will:
+
+* copy the default Java cacerts to a runtime volume shared between the init container and the main container at `/var/ssl`
+* run [keytool -import](https://docs.oracle.com/javase/8/docs/technotes/tools/unix/keytool.html){.external} to import all certificates in `/tmp/crt` mounted from `dev-certificates` secret to `/var/ssl/cacerts`
+
+`-Djavax.net.ssl.trustStore=/var/ssl/cacerts` system property will be automatically added to `JVM_SUPPORT_RECOMMENDED_ARGS` environment variable.
+
+If necessary, it is possible to override the default `keytool -import` command:
+
+```yaml
+jira:
+  additionalCertificates:
+     secretName: dev-certificates
+     customCmd: keytool -import ...
+```


@bianchi2 Im just wondering if it makes more sense for this to go under the TROUBLESHOOTING section? Maybe not, but as a compromise a section in the troubleshooter page talking to PKIX exceptions with a link to the fix might be good?

It's a good idea! I'll raise another PR with docs changes as this one has been merged.

errcode1202 · 2023-09-18T00:37:01Z

docs/docs/userguide/CONFIGURATION.md

+     secretName: dev-certificates
+```
+
+Helm chart will add additional `volumeMounts` and `volumes` to the pod(s), as well as an extra init container that will:


Suggested change

Helm chart will add additional `volumeMounts` and `volumes` to the pod(s), as well as an extra init container that will:

The product Helm chart will add additional `volumeMounts` and `volumes` to the pod(s), as well as an extra init container that will:

bianchi2 requested review from nanux, errcode1202, yzha645, jjeongatl, bconroyA and ealliaume as code owners September 8, 2023 05:43

bianchi2 added the e2e label Sep 11, 2023

Add certs to Java trust store

39476b5

bianchi2 force-pushed the add-crt-to-keystore branch from caed505 to 39476b5 Compare September 11, 2023 21:33

Merge branch 'main' into add-crt-to-keystore

c4e955b

nanux reviewed Sep 16, 2023

View reviewed changes

bianchi2 and others added 2 commits September 17, 2023 15:41

Merge branch 'main' into add-crt-to-keystore

7d16f34

Add docs

e961e5c

nanux approved these changes Sep 17, 2023

View reviewed changes

bianchi2 merged commit 44684ee into main Sep 18, 2023
3 checks passed

bianchi2 deleted the add-crt-to-keystore branch September 18, 2023 00:38

errcode1202 reviewed Sep 18, 2023

View reviewed changes

bianchi2 mentioned this pull request Sep 18, 2023

Update docs "Add user provided certificates to the default Java truststore" #671

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLIP-1826: Add user provided certificates to the default Java truststore #663

CLIP-1826: Add user provided certificates to the default Java truststore #663

bianchi2 commented Sep 8, 2023 •

edited

Loading

nanux left a comment

nanux Sep 16, 2023

bianchi2 Sep 16, 2023 •

edited

Loading

nanux Sep 17, 2023

nanux Sep 16, 2023

bianchi2 Sep 16, 2023 •

edited

Loading

nanux Sep 17, 2023

bianchi2 commented Sep 16, 2023 •

edited

Loading

nanux Sep 17, 2023

nanux Sep 17, 2023

bianchi2 commented Sep 17, 2023

errcode1202 Sep 18, 2023

bianchi2 Sep 18, 2023

errcode1202 Sep 18, 2023

	Helm chart will add additional `volumeMounts` and `volumes` to the pod(s), as well as an extra init container that will:
	The product Helm chart will add additional `volumeMounts` and `volumes` to the pod(s), as well as an extra init container that will:

CLIP-1826: Add user provided certificates to the default Java truststore #663

CLIP-1826: Add user provided certificates to the default Java truststore #663

Conversation

bianchi2 commented Sep 8, 2023 • edited Loading

Checklist

nanux left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bianchi2 Sep 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bianchi2 Sep 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bianchi2 commented Sep 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bianchi2 commented Sep 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bianchi2 commented Sep 8, 2023 •

edited

Loading

bianchi2 Sep 16, 2023 •

edited

Loading

bianchi2 Sep 16, 2023 •

edited

Loading

bianchi2 commented Sep 16, 2023 •

edited

Loading