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

Conversation

jmtd
Copy link
Collaborator

@jmtd jmtd commented Nov 5, 2018

This module enables the trusting of CA certificates provided to
running containers by OpenShift.

At build-time: various system PKI paths are made world writeable-by-all,
in order that the random-UID runtime user can insert CA certificates
into the trusted store

At run-time: copy the CA file into place and invoke /usr/bin/update-ca-trust
To generate derivative trust stores (e.g. OpenSSL & Java JKS)

Signed-off-by: Jonathan Dowland jdowland@redhat.com

https://issues.jboss.org/browse/CLOUD-2689

Thanks for submitting your Pull Request!

Please make sure your PR meets following requirements:

  • Pull Request title is properly formatted: [CLOUD-XYA] Subject
  • Pull Request contains link to the JIRA issue
  • Pull Request contains description of the issue
  • Pull request does not include fixes for other issues than the main ticket
  • Attached commits represent unit of work and are properly formatted
  • You have read and agreed to the Developer Certificate of Origin (DCO) (see CONTRIBUTING.md)
  • Every commit contains Signed-off-by: Your Name <yourname@redhat.com> - use git commit -s

This module enables the trusting of CA certificates provided to
running containers by OpenShift.

At build-time: various system PKI paths are made world writeable-by-all,
in order that the random-UID runtime user can insert CA certificates
into the trusted store

At run-time: copy the CA file into place and invoke /usr/bin/update-ca-trust
To generate derivative trust stores (e.g. OpenSSL & Java JKS)

Signed-off-by: Jonathan Dowland <jdowland@redhat.com>
@jmtd
Copy link
Collaborator Author

jmtd commented Nov 5, 2018

Further work required

  • trust-ose-cert.sh needs hooking into existing container start-up scripts in (an)other module(s)
  • docs?
  • tests?

This ensures the script will be invoked for all S2I entrypoints

Signed-off-by: Jonathan Dowland <jdowland@redhat.com>
@jmtd
Copy link
Collaborator Author

jmtd commented Nov 7, 2018

Here's the test harness I used to verify this work for myself. It leaves much to be desired but might aid assurance for reviewing this PR

https://github.com/jmtd/redhat-openjdk-18-openshift-image/tree/cloud-2689-certificates-TEST

Ideally it would be refactored into a module test, or a S2I-enabled repo, but I didn't want to delay this PR further at this time

@jmtd jmtd changed the title WIP CLOUD-2689: jboss.container.java.certs module CLOUD-2689: jboss.container.java.certs module Nov 7, 2018
@jmtd
Copy link
Collaborator Author

jmtd commented Nov 7, 2018

  • with an optional env option to disable this feature.

When JAVA_DISABLE_TRUST_OPENSHIFT_CA is defined the trust-openshift-supplied-CA
code will not run.

Signed-off-by: Jonathan Dowland <jdowland@redhat.com>
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

#!/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

if [ -z "$JAVA_DISABLE_TRUST_OPENSHIFT_CA" ]; then
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!

g+w is sufficent (group is root, and OpenShift running user is in
group root) for this approach to work.

Signed-off-by: Jonathan Dowland <jdowland@redhat.com>
Install a static symlink from the OS TLS/PKI trust store to the
location that OpenShift uses for its CA certificate, instead of
copying it at run-time.

Signed-off-by: Jonathan Dowland <jdowland@redhat.com>
@jmtd
Copy link
Collaborator Author

jmtd commented Sep 15, 2020

I don't think we need this anymore: I think OpenShift (4.1? 4.2?) is handling this via an operator.

@jmtd jmtd closed this Sep 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants