Skip to content
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

CLOUD-2689: jboss.container.java.certs module #312

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions jboss/container/java/certs/bash/artifacts/trust-ose-cert.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#!/bin/sh
set -e

if [ -z "$JAVA_DISABLE_TRUST_OPENSHIFT_CA" ]; then

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note that there are 2 different CA... the ca.crt , which I would call "the openshfit CA", and the service-ca.crt is a very specific one (https://docs.openshift.com/online/dev_guide/secrets.html#service-serving-certificate-secrets )

I'd prefer if we (optionally) added both.. However, even if we don't, we should make it clear in the names of the env and comments which CA is being (or not being) added.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed

ca=/var/run/secrets/kubernetes.io/serviceaccount/service-ca.crt
if test -r "$ca" ; then
cp "$ca" /etc/pki/ca-trust/source/anchors/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to copy this or create a link? If we create a link, can we link it to the appropriate location in the image, with a broken link...and will that link resolve appropriately once the file is mounted by OpenShift? If so, we could get away with simply invoking update-ca-trust. wdyt?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I think that would work. I'll take a look

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to work fine. Thanks for the suggestion!

/usr/bin/update-ca-trust
fi
fi
18 changes: 18 additions & 0 deletions jboss/container/java/certs/bash/configure.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#!/bin/sh
# Configure module
set -e

SCRIPT_DIR=$(dirname $0)
ARTIFACTS_DIR=${SCRIPT_DIR}/artifacts

# necessary for random UID user to run update-ca-certs
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to chmod these back afterwards? it seems odd to leave them 777.

Copy link
Collaborator Author

@jmtd jmtd Nov 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately not because the chmod is happening in the build stage but the write access (via update-ca-trust) is happening at runtime.

It's unpleasant and uncomfortable and comparable to where we chmod and chown /etc/passwd, or where we chown and chmod the entire EAP distribution to the jboss user.

I think of it a little like we've put up this big, thick wall around our processes (the container) and we're taking down the walls within that boundary that were more critical before, but might still be an important part of defence against some attack vectors, even now.

I would like us to have some kind of cross-team discussion about security issues like this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it enough to simply ensure the files are owned by root group and have write privs on group?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checking

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes g+w is sufficient. The threat model doesn't really change (same user in both cases) but it feels nicer to not have it as wide as 777. I'll commit this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whoops, 10010000:0

Copy link
Collaborator Author

@jmtd jmtd Dec 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rcernich the test harness I wrote to check this stuff out, I published here. In particular see test.sh line 1: each invocation of the test harness uses a random uid, just like openshift.

This harness gave me the confidence to refine my implementation and was the system I used to ensure that e.g. the symlink thing you suggested would work. It's also very easy to substitute the test CA in that branch with another, such as an internal CA cert, and adjust the invocation of the java program to check an internal HTTPS system, demonstrating that the JKS keystore was updated.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but OpenShift also runs with group 0.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I missed the group spec! I'll adjust my harness and re-test.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed and re-tested locally; seems fine

chmod 777 \
/etc/pki/ca-trust/extracted/openssl \
/etc/pki/ca-trust/extracted/java \
/etc/pki/ca-trust/source/anchors \
/etc/pki/ca-trust/extracted/pem

d=/opt/jboss/container/java/certs
mkdir -p "$d"
cp "${ARTIFACTS_DIR}/trust-ose-cert.sh" "$d"
chmod +x "$d/trust-ose-cert.sh"
13 changes: 13 additions & 0 deletions jboss/container/java/certs/bash/module.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
schema_version: 1
name: jboss.container.java.certs.bash
version: '1.0'
description: Trust runtime-supplied OpenShift/Kubernetes CA

envs:
- name: JAVA_DISABLE_TRUST_OPENSHIFT_CA
example: true
description: When defined, the container will not automatically trust a CA certificate provided by OpenShift at run-time.

execute:
- script: configure.sh
user: root
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,6 @@ source "${JBOSS_CONTAINER_UTIL_LOGGING_MODULE}/logging.sh"
function s2i_core_env_init_hook() {
S2I_TARGET_DATA_DIR="${S2I_TARGET_DATA_DIR:-${JAVA_DATA_DIR:-/deployments/data}}"
}

# Trust OpenShift CA, if provided
/opt/jboss/container/java/certs/trust-ose-cert.sh
1 change: 1 addition & 0 deletions jboss/container/java/s2i/bash/module.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,4 @@ modules:
- name: jboss.container.hawkular.bash
- name: jboss.container.jolokia.bash
- name: jboss.container.util.logging.bash
- name: jboss.container.java.certs.bash