feat(tls): agent can pass TLS client auth cert #491
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andrewazores
force-pushed
the
tls-client-auth
branch
from
a64cb86
to
85c2a27
Compare
7 tasks
andrewazores
force-pushed
the
tls-client-auth
branch
2 times, most recently
from
bdc3efd
to
f2e606d
Compare
|
I think using the looks like it does the trick [0]. Worth noting that this only works when doing dynamic attach with late-binding configuration properties however, because of the I didn't have any luck trying to use these configuration properties to use the [0] caveat: this deployed the latest |
andrewazores
force-pushed
the
tls-client-auth
branch
from
6842f59
to
e39641d
Compare
|
Using sample application Agent: Cryostat: Proxy: This Agent/Cryostat version cominbation works normally as expected in smoketest, so it seems like somehow something about the proxy being in the middle is what causes this change in behaviour. Since this is broken after the V4 API changes, these are my suspects:
|
|
"Fixed" by having the Agent specify its own Pod IP for the callback URL, rather than its app Service DNS name. Apply this patch: diff --git a/config/samples/sample-app.yaml b/config/samples/sample-app.yaml
index b9ba843..7d43926 100644
--- a/config/samples/sample-app.yaml
+++ b/config/samples/sample-app.yaml
@@ -16,7 +16,7 @@ spec:
app: quarkus-test
spec:
containers:
- - image: quay.io/andrewazores/quarkus-test:latest
+ - image: quay.io/andrewazores/quarkus-cryostat-agent:tls-client-auth-16
imagePullPolicy: Always
name: quarkus-test
ports:
@@ -25,12 +25,9 @@ spec:
- containerPort: 9097
protocol: TCP
resources:
- requests:
- cpu: 200m
- memory: 96Mi
limits:
cpu: 500m
- memory: 128Mi
+ memory: 256Mi
securityContext:
allowPrivilegeEscalation: false
capabilities:Then do: $ make sample_app
$ SAMPLE_POD="$(oc get pod -l app=quarkus-test -o jsonpath='{$.items[0].metadata.name}')" ; \
oc wait pod $SAMPLE_POD --for condition=ready; \
for i in tls.key tls.crt ca.crt; do oc cp $i $SAMPLE_POD:/tmp/ \
; done
$ POD_IP="$(oc get pod -l app=quarkus-test -o jsonpath='{$.items[0].status.podIP}')"
$ oc exec -it $SAMPLE_POD -- java -jar /deployments/app/cryostat-agent.jar \
-Dcryostat.agent.baseuri=https://cryostat-sample-agent.cryostat.svc:8282 \
-Dcryostat.agent.callback=http://${POD_IP}:9977 \
-Dcryostat.agent.api.writes-enabled=true \
-Dcryostat.agent.webclient.tls.client-auth.cert.path=/tmp/tls.crt \
-Dcryostat.agent.webclient.tls.client-auth.key.path=/tmp/tls.key \
-Dcryostat.agent.webclient.tls.truststore.cert[0].path=/tmp/ca.crt \
-Dcryostat.agent.webclient.tls.truststore.cert[0].type=X.509 \
-Dcryostat.agent.webclient.tls.truststore.cert[0].alias=cryostat
$ oc wait pod -l app=quarkus-test --for condition=ready ; oc logs -f -l app=quarkus-testI'll update cryostatio/cryostat-operator#928 once this PR and cryostatio/cryostat-operator#957 are in to accommodate for these further changes. |
7 tasks
ebaron
previously approved these changes
Oct 3, 2024
There was a problem hiding this comment.
I tested successfully combined with cryostatio/cryostat-operator#957 and the following deployment:
apiVersion: apps/v1
kind: Deployment
metadata:
labels:
app: quarkus-test-agent
name: quarkus-test-agent
namespace: cryostat-operator-system
spec:
selector:
matchLabels:
app: quarkus-test-agent
template:
metadata:
labels:
app: quarkus-test-agent
spec:
containers:
- env:
- name: CRYOSTAT_AGENT_APP_NAME
value: agent-test
- name: NAMESPACE
valueFrom:
fieldRef:
apiVersion: v1
fieldPath: metadata.namespace
- name: CRYOSTAT_AGENT_API_WRITES_ENABLED
value: "true"
- name: CRYOSTAT_AGENT_BASEURI
value: https://cryostat-sample-agent.$(NAMESPACE).svc:8282
- name: POD_IP
valueFrom:
fieldRef:
apiVersion: v1
fieldPath: status.podIP
- name: CRYOSTAT_AGENT_CALLBACK
value: http://$(POD_IP):9977
- name: JAVA_OPTS_APPEND
value: |-
-Dquarkus.http.host=0.0.0.0
-Djava.util.logging.manager=org.jboss.logmanager.LogManager
-javaagent:/deployments/app/cryostat-agent.jar
-Dcryostat.agent.webclient.tls.client-auth.cert.path=/var/run/secrets/io.cryostat/cryostat-agent/tls.crt
-Dcryostat.agent.webclient.tls.client-auth.key.path=/var/run/secrets/io.cryostat/cryostat-agent/tls.key
-Dcryostat.agent.webclient.tls.truststore.cert[0].path=/var/run/secrets/io.cryostat/cryostat-agent/ca.crt
-Dcryostat.agent.webclient.tls.truststore.cert[0].type=X.509
-Dcryostat.agent.webclient.tls.truststore.cert[0].alias=cryostat
image: quay.io/andrewazores/quarkus-cryostat-agent:tls-client-auth-16
imagePullPolicy: Always
name: quarkus-test-agent
ports:
- containerPort: 10010
protocol: TCP
- containerPort: 9097
protocol: TCP
resources:
limits:
cpu: 500m
memory: 192Mi
requests:
cpu: 200m
memory: 96Mi
securityContext:
allowPrivilegeEscalation: false
capabilities:
drop:
- ALL
volumeMounts:
- mountPath: /var/run/secrets/io.cryostat/cryostat-agent
name: agent-tls
securityContext:
runAsNonRoot: true
volumes:
- name: agent-tls
secret:
defaultMode: 420
secretName: cryostat-agent-c44e3ce7d8452282f4cf1ab14d08cfda2875fa727912e41595c6979bffe0f693On a side note, would it be possible to add a key options for the web server in addition to the existing keystore options, similar to what you've done here for client-auth? It would make it easier to configure HTTPS for the server.
I'll open a follow-up PR with these changes in a moment, just to avoid potentially screwing up something in here. |
ebaron
approved these changes
Oct 4, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #490
See also cryostatio/cryostat-operator#957
See above PR for steps on retrieving
tls.crt,tls.key,ca.crtfrom the Operator's generated Secret for the Agent to use.Then, to test this combined with the above Operator PR:
sample-app.yamlto deploy the containerquay.io/andrewazores/quarkus-cryostat-agent:tls-client-auth-15(an application containing the latest build of this PR)make sample_appexport SAMPLE_POD="$(kubectl get pod -l app=quarkus-test -o jsonpath='{$.items[0].metadata.name}')" ; oc wait pod $SAMPLE_POD --for condition=ready; for i in tls.key tls.crt ca.crt; do oc cp $i $SAMPLE_POD:/tmp/ ; doneto get the secret contents into the Agent application's Podoc wait pod -l app=quarkus-test --for condition=ready ; oc logs -f -l app=quarkus-testto watch the log output from the sample application Pod. Initially the Agent should start up but bail out because thebaseuriproperty is empty. This is expected.oc exec -it $SAMPLE_POD -- java -jar /deployments/app/cryostat-agent.jar -Dcryostat.agent.baseuri=https://cryostat-sample-agent.cryostat.svc:8282 -Dcryostat.agent.callback=http://quarkus-test:9977 -Dcryostat.agent.api.writes-enabled=true -Dcryostat.agent.webclient.tls.trust-all=true -Dcryostat.agent.webclient.tls.client-auth.cert.path=/tmp/tls.crt -Dcryostat.agent.webclient.tls.client-auth.key.path=/tmp/tls.key -Dcryostat.agent.webclient.tls.client-auth.keystore.pass=changeit1 -Dcryostat.agent.webclient.tls.client-auth.key.pass=changeit2. This forks a process to invoke the Agent to dynamically attach itself to the sample application JVM, passing the various required configuration properties as late binding arguments. The important properties for this PR are the ones containingtls.client-auth:cryostat.agent.webclient.tls.client-auth.cert.pathis analogous tocurl --cert;cryostat.agent.webclient.tls.client-auth.key.pathis analagous tocurl --key. For the ease of testing,cryostat.agent.webclient.tls.trust-allis also passed here, which tells the client to trust any TLS certificate presented by the server. Patchingsample-app-agent.yamlto deploy the same container image, then doingmake sample_app_agent, should show that the current changes do not change the existing functionality for configuring the HTTP client to correctly trust the server certificate (in this case the OAuth proxy, not the Agent TLS proxy, but it's the same concept and code path, just a different cert and server URL).trust-allproperty. This should result in the Agent's HTTP client refusing to connect since it does not trust theca.crtpresented by the server.