Welcome to Cryostat! 👋

Before contributing, make sure you have:

[ ] Read the contributing guidelines
[ ] Linked a relevant issue which this PR resolves
[ ] Linked any other relevant issues, PR's, or documentation, if any
[ ] Resolved all conflicts, if any
[ ] Rebased your branch PR on top of the latest upstream main branch
[ ] Attached at least one of the following labels to the PR: [chore, ci, docs, feat, fix, test]
[ ] Signed all commits: git commit -S -m "YOUR_COMMIT_MESSAGE"

Fixes: #952

Description of the change:

Adds an nginx container to the Cryostat deployment, which acts as a reverse proxy. Not exposed over an Ingress or Route.
When TLS using cert-manager is enabled, the proxy requires a client certificate signed by our CA to authenticate
Only selected Cryostat HTTP API endpoints are permitted and forwarded by the proxy. These are the endpoints required by the agent.

Motivation for the change:

Allows agents to communicate with Cryostat without using APIServer tokens. The tokens were merely a convenient way of authenticating, but don't work well for #784 without increasing the risk of these tokens being exposed to attackers.

How to manually test:

make create_cryostat_cr
make sample-app
SAMPLE_POD="$(kubectl get pod -l app=quarkus-test -o jsonpath='{$.items[0].metadata.name}'")

Add key/certs to pod

kubectl get secret cryostat-agent-c44e3ce7d8452282f4cf1ab14d08cfda2875fa727912e41595c6979bffe0f693 -o json | jq -r '.data["tls.key"] | @base64d' > /tmp/tls.key
kubectl get secret cryostat-agent-c44e3ce7d8452282f4cf1ab14d08cfda2875fa727912e41595c6979bffe0f693 -o json | jq -r '.data["tls.crt"] | @base64d' > /tmp/tls.crt
kubectl get secret cryostat-agent-c44e3ce7d8452282f4cf1ab14d08cfda2875fa727912e41595c6979bffe0f693 -o json | jq -r '.data["ca.crt"] | @base64d' > /tmp/ca.crt
kubectl cp /tmp/tls.key "${SAMPLE_POD}":/tmp/tls.key
kubectl cp /tmp/tls.crt "${SAMPLE_POD}":/tmp/tls.crt
kubectl cp /tmp/ca.crt "${SAMPLE_POD}":/tmp/ca.crt

Check the proxy

No client cert: kubectl exec "${SAMPLE_POD}" -- curl -sS --cacert /tmp/ca.crt -L https://cryostat-sample-agent.cryostat-operator-system.svc:8282/health

<html>
<head><title>400 No required SSL certificate was sent</title></head>
<body>
<center><h1>400 Bad Request</h1></center>
<center>No required SSL certificate was sent</center>
<hr><center>nginx/1.24.0</center>
</body>
</html>

With client cert: kubectl exec "${SAMPLE_POD}" -- curl -sS --key /tmp/tls.key --cert /tmp/tls.crt --cacert /tmp/ca.crt -L https://cryostat-sample-agent.cryostat-operator-system.svc:8282/health

{"cryostatVersion":"v4.0.0-snapshot","build":{"git":{"hash":"98a9d53519ceeef8eced0fd924b66d3a824b8340"}},"reportsConfigured":false,"reportsAvailable":true,"dashboardAvailable":true,"datasourceAvailable":true,"datasourceConfigured":true,"dashboardConfigured":true}

With bad path: kubectl exec "${SAMPLE_POD}" -- curl -sS --key /tmp/tls.key --cert /tmp/tls.crt --cacert /tmp/ca.crt -L https://cryostat-sample-agent.cryostat-operator-system.svc:8282/api/v1/recordings/
```
<html>
<head><title>404 Not Found</title></head>
<body>
<center><h1>404 Not Found</h1></center>
<hr><center>nginx/1.24.0</center>
</body>
</html>
```

Disable cert-manager

kubectl patch --type=merge -p '{"spec": {"enableCertManager": false}}' cryostat/cryostat-sample

No cert needed: kubectl exec "${SAMPLE_POD}" -- curl -sS -L http://cryostat-sample-agent.cryostat-operator-system.svc:8282/health

{"reportsConfigured":false,"build":{"git":{"hash":"85b43f9781d927debe0bcc909225cbd17f482c02"}},"cryostatVersion":"v4.0.0-snapshot","dashboardConfigured":true,"datasourceConfigured":true,"datasourceAvailable":true,"dashboardAvailable":true,"reportsAvailable":true}

Still restricts paths: kubectl exec "${SAMPLE_POD}" -- curl -sS -L http://cryostat-sample-agent.cryostat-operator-system.svc:8282/api/v1/recordings/

<html>
<head><title>404 Not Found</title></head>
<body>
<center><h1>404 Not Found</h1></center>
<hr><center>nginx/1.24.0</center>
</body>
</html>

Re-enable cert-manager

kubectl patch --type=merge -p '{"spec": {"enableCertManager": true}}' cryostat/cryostat-sample

Correct usage still works: kubectl exec "${SAMPLE_POD}" -- curl -sS --key /tmp/tls.key --cert /tmp/tls.crt --cacert /tmp/ca.crt -L https://cryostat-sample-agent.cryostat-operator-system.svc:8282/health

{"dashboardConfigured":true,"datasourceConfigured":true,"datasourceAvailable":true,"dashboardAvailable":true,"reportsAvailable":true,"reportsConfigured":false,"build":{"git":{"hash":"85b43f9781d927debe0bcc909225cbd17f482c02"}},"cryostatVersion":"v4.0.0-snapshot"}

HTTP doesn't work: kubectl exec "${SAMPLE_POD}" -- curl -sS -L http://cryostat-sample-agent.cryostat-operator-system.svc:8282/health

<html>
<head><title>400 The plain HTTP request was sent to HTTPS port</title></head>
<body>
<center><h1>400 Bad Request</h1></center>
<center>The plain HTTP request was sent to HTTPS port</center>
<hr><center>nginx/1.24.0</center>
</body>
</html>

Re-create the Cryostat CR (and its CA)

make create_cryostat_cr
kubectl exec "${SAMPLE_POD}" -- curl -sS --key /tmp/tls.key --cert /tmp/tls.crt -k -L https://cryostat-sample-agent.cryostat-operator-system.svc:8282/health

<html>
<head><title>400 The SSL certificate error</title></head>
<body>
<center><h1>400 Bad Request</h1></center>
<center>The SSL certificate error</center>
<hr><center>nginx/1.24.0</center>
</body>
</html>

Sep 20 '24 21:09 ebaron

/build_test

Sep 20 '24 21:09 ebaron

@andrewazores I think even after this is merged, we'll need some work on the agent to provide client certificates. I think the key/cert pair is used for the HTTP server, but not client.

Sep 20 '24 21:09 ebaron

/build_test completed successfully ✅. View Actions Run.

Sep 20 '24 21:09 github-actions[bot]

@andrewazores I think even after this is merged, we'll need some work on the agent to provide client certificates. I think the key/cert pair is used for the HTTP server, but not client.

Yes, I think you're right. All the work done in and around https://github.com/cryostatio/cryostat-agent/issues/138 was just focused on getting the Cryostat server and agent to know how to get each of their HTTPS clients to trust the certificates presented by each of their own webservers, so there will need to be a bit more done so that the agent's client also knows how to pass a client certificate for authentication.

Sep 23 '24 14:09 andrewazores

Does the ca.crt for the TLS proxy trust here work the same way as the equivalent server cert used for the auth proxies (or, more recently)? We already have a setup and some testing to ensure that the OAuth proxies can be configured to present TLS, and that the Agent can be configured to trust that same cert, so if this works the same way then that work should already be done.

So far in https://github.com/cryostatio/cryostat-agent/pull/491 I have something working and tested at least for getting the Agent to send its client cert along to the new proxy as authentication, but I haven't checked that the ca.crt you describe in this PR can be trusted by the Agent client. I've just been telling the Agent HTTP client to trust all server certificates, assuming that this works the same way as it does for the other proxies.

Sep 25 '24 17:09 andrewazores

I may be misunderstanding the question, but the ca cert is the same self-signed CA used by all Cryostat components. So from the agent's perspective, the server CA and client CA are the exact same certificates.

Sep 26 '24 18:09 ebaron

I think that clears up most of what I'm unsure of (because I still don't fully understand how the Operator interacts with cert-manager).

So the two CAs are the same actual object, there are just two copies of it in two different Secrets (ex. cryostat-sample-tls and cryostat-sample-agent-tls). I think one is intended for the OAuth proxy and one for the TLS-auth proxy, but they are actually just the same thing then, right?

So then, the existing code and configuration options that we have for setting up the Agent truststore to be able to validate the OAuth proxy would not need any changes to validate the TLS-auth proxy. The (identical) ca.crt from the new Secret has to be mounted/copied in instead, and the existing configuration options would just be set to tell the Agent to load this one into its client truststore instead. Does that sound right?

Sep 26 '24 18:09 andrewazores

I guess to make that a little more concrete:

IIRC the plan is for this Operator feature to work by doing something like watching for changes to Deployments (or Pods?), and checking if some "Cryostat Agent automatic" annotation is set. If it is, then the Operator will go ahead and edit the Deployment so that this cryostat-sample-agent-tls Secret gets mounted to it (providing the tls.crt, tls.key, and "new" ca.crt) into the container filesystem. Then the Agent is somehow configured and launched - maybe by setting environment variables to hook it in to the container as a -javaagent statically loaded Agent, or else by forking a process in the target container to exec and dynamically attach the Agent.

So that "somehow configured and launched" step is where the ca.crt question is, but I think I understand it now. It will hook in to the Agent's custom truststore the same way as how it already does it, since it's really the exact same thing - just provided in a second Secret so that everything can be mounted at once.

Sep 26 '24 19:09 andrewazores

That sounds right to me. cert-manager adds a copy of the CA certificate into each secret it creates to make a certificate chain. When using the agent auto-configuration, we'd set up the agent with cryostat.agent.webclient.tls.truststore.certs and cryostat.agent.webserver.tls.cert entries. These would reference the same certificate secret, since the cryostat-sample-agent-tls secret contains a private key, a certificate with both client and server key usages, and the CA certificate.

I was also thinking we may want to remove support for the agent to use the OAuth Proxy, IIRC, this would allow us to remove the openshift-delegate-urls arguments and allow us to remove some cluster level permissions for TokenReviews and SARs.

Sep 26 '24 19:09 ebaron

I was also thinking we may want to remove support for the agent to use the OAuth Proxy, IIRC, this would allow us to remove the openshift-delegate-urls arguments and allow us to remove some cluster level permissions for TokenReviews and SARs.

Hmm... the Agent does that by just passing the Authorization: Bearer abcd1234 header, so I think removing support on the OAuth proxy side means generally removing support for authentication/authorization using this mechanism. We don't directly use it anywhere else, but it can certainly be handy during development or for troubleshooting since this also allows us to use curl/HTTPie/etc. directly on the Route, or on the Service from another Pod within the cluster.

It'd still be possible to use curl using oc exec on one of the Cryostat Pods behind the proxy so that curl is talking directly to Cryostat, so it doesn't become impossible, just somewhat more complicated. This also depends on the assumption that one of those Pods will have curl (or a similar utility) in its base image, which may not be true forever.

I think it's worth considering anyway. Just wanted to highlight that removing that support doesn't affect only the Agent.

Sep 26 '24 19:09 andrewazores

/build_test

Oct 04 '24 17:10 ebaron

/build_test : At least one test failed ❌. View Actions Run.

Oct 04 '24 17:10 github-actions[bot]

Image:      ghcr.io/cryostatio/cryostat-operator-scorecard:pr-957-1bdfc3e61ab7fd4e7e7759f0007a09ad27efb057
Entrypoint: [cryostat-scorecard-tests cryostat-recording]
Labels:
	"suite":"cryostat"
	"test":"cryostat-recording"
Results:
	State: fail

	Errors:
		invalid character 'p' looking for beginning of value
	Log:
		panic: runtime error: invalid memory address or nil pointer dereference
		[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1646604]
		
		goroutine 1 [running]:
		github.com/cryostatio/cryostat-operator/internal/test/scorecard.(*RecordingClient).Create(0xc0001981b8, {0x1d0d010, 0x2a9aca0}, 0xc00017c7a0?, 0xc00010f9e0)
			/workspace/internal/test/scorecard/clients.go:338 +0x64
		github.com/cryostatio/cryostat-operator/internal/test/scorecard.CryostatRecordingTest(0x23?, {0xc000046044, 0x1b}, 0x0)
			/workspace/internal/test/scorecard/tests.go:200 +0xc05
		main.runTests({0xc0001b4010?, 0xa?, 0x1b?}, 0xc0000929c0, {0xc000046044, 0x1b}, 0x0)
			/workspace/internal/images/custom-scorecard-tests/main.go:129 +0x8aa
		main.main()
			/workspace/internal/images/custom-scorecard-tests/main.go:73 +0x458

This probably has to do with the V4 API changes.

Oct 04 '24 17:10 andrewazores

Image:      ghcr.io/cryostatio/cryostat-operator-scorecard:pr-957-1bdfc3e61ab7fd4e7e7759f0007a09ad27efb057
Entrypoint: [cryostat-scorecard-tests cryostat-recording]
Labels:
	"suite":"cryostat"
	"test":"cryostat-recording"
Results:
	State: fail

	Errors:
		invalid character 'p' looking for beginning of value
	Log:
		panic: runtime error: invalid memory address or nil pointer dereference
		[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1646604]
		
		goroutine 1 [running]:
		github.com/cryostatio/cryostat-operator/internal/test/scorecard.(*RecordingClient).Create(0xc0001981b8, {0x1d0d010, 0x2a9aca0}, 0xc00017c7a0?, 0xc00010f9e0)
			/workspace/internal/test/scorecard/clients.go:338 +0x64
		github.com/cryostatio/cryostat-operator/internal/test/scorecard.CryostatRecordingTest(0x23?, {0xc000046044, 0x1b}, 0x0)
			/workspace/internal/test/scorecard/tests.go:200 +0xc05
		main.runTests({0xc0001b4010?, 0xa?, 0x1b?}, 0xc0000929c0, {0xc000046044, 0x1b}, 0x0)
			/workspace/internal/images/custom-scorecard-tests/main.go:129 +0x8aa
		main.main()
			/workspace/internal/images/custom-scorecard-tests/main.go:73 +0x458

This probably has to do with the V4 API changes.

Looks to me like the target was nil. Does the create target response no longer look like this?

"data": {
  "result": {
    "id": 123,
    "connectUrl": "foo",
    "alias": "bar"
  }
}

Oct 04 '24 18:10 ebaron

That'd do it. The .data.result JSON layering format has been removed since it was never really useful in the way I intended back when I introduced it in 2.0, so it's gone in 4.0.

$ https -v --auth=user:pass :8443/api/v4/targets connectUrl=reports:11224 alias=reports
POST /api/v4/targets HTTP/1.1
Accept: application/json, */*;q=0.5
Accept-Encoding: gzip, deflate, br
Authorization: Basic dXNlcjpwYXNz
Connection: keep-alive
Content-Length: 51
Content-Type: application/json
Host: localhost:8443
User-Agent: HTTPie/3.2.2

{
    "alias": "reports",
    "connectUrl": "reports:11224"
}


HTTP/1.1 201 Created
Content-Length: 257
Content-Type: application/json;charset=UTF-8
Date: Fri, 04 Oct 2024 18:43:40 GMT
Gap-Auth: user
Location: https://localhost:8443/api/v4/targets/1

{
    "agent": false,
    "alias": "reports",
    "annotations": {
        "cryostat": [
            {
                "key": "REALM",
                "value": "Custom Targets"
            }
        ],
        "platform": []
    },
    "connectUrl": "service:jmx:rmi:///jndi/rmi://reports:11224/jmxrmi",
    "id": 1,
    "jvmId": "XaWFeOExJ5-CiPQioXregbBEf7SU9Rn5fgFOYhEIkXU=",
    "labels": []
}

Oct 04 '24 18:10 andrewazores

/build_test

Oct 04 '24 18:10 ebaron

/build_test completed successfully ✅. View Actions Run.

Oct 04 '24 19:10 github-actions[bot]

@andrewazores looks like that last commit fixed it, mind approving again?

Oct 04 '24 19:10 ebaron

cryostat-operator
cryostat-operator copied to clipboard

feat(agent): add nginx proxy for agent communication

Welcome to Cryostat! 👋

Before contributing, make sure you have:

Description of the change:

Motivation for the change:

How to manually test:

Add key/certs to pod

Check the proxy

Disable cert-manager

Re-enable cert-manager

Re-create the Cryostat CR (and its CA)

cryostat-operator cryostat-operator copied to clipboard

feat(agent): add nginx proxy for agent communication

Welcome to Cryostat! 👋

Before contributing, make sure you have:

Description of the change:

Motivation for the change:

How to manually test:

Add key/certs to pod

Check the proxy

Disable cert-manager

Re-enable cert-manager

Re-create the Cryostat CR (and its CA)

cryostat-operator
cryostat-operator copied to clipboard