Conversation
Signed-off-by: see-quick <maros.orsak159@gmail.com>
|
/gha run pipeline=regression |
|
⏳ System test verification started: link The following 6 job(s) will be executed:
Tests will start after successful build completion. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12497 +/- ##
============================================
+ Coverage 74.99% 75.02% +0.02%
+ Complexity 6651 6437 -214
============================================
Files 373 373
Lines 25367 24881 -486
Branches 3376 3205 -171
============================================
- Hits 19025 18666 -359
+ Misses 4956 4901 -55
+ Partials 1386 1314 -72 🚀 New features to boost your workflow:
|
|
🎉 System test verification passed: link |
Signed-off-by: see-quick <maros.orsak159@gmail.com>
Signed-off-by: see-quick <maros.orsak159@gmail.com>
katheris
left a comment
There was a problem hiding this comment.
Thanks @see-quick I've had a look through. Generally the tests look good but I've added some comments around the checks being made. Also I think we should be explicitly testing the case where the user provides a CA that is multistage, but Strimzi still issues the certificates, rather than just the cases of a custom listener certificate, since those are different code paths.
systemtest/src/test/java/io/strimzi/systemtest/security/custom/CustomCaChainST.java
Outdated
Show resolved
Hide resolved
| KubeResourceManager.get().kubeClient().getClient().secrets().inNamespace(testStorage.getNamespaceName()).withName(brokerPodName).get(), | ||
| brokerPodName + ".crt"); | ||
| LOGGER.info("Broker certificate chain contains {} certificates", brokerChain.size()); | ||
| assertThat("Broker certificate chain should contain at least the leaf cert", brokerChain.size(), is(greaterThanOrEqualTo(1))); |
There was a problem hiding this comment.
This seems to only check that the leaf cert is presented, and not all three
There was a problem hiding this comment.
I think it should be 4? RootCA -> ImCA -> LeafCA -> broker cert? I will use definitely stronger condition is(4) and not this weak greaterThanOrEqualTo.
Or do we want RootCA -> ImCA -> broker cert?
systemtest/src/test/java/io/strimzi/systemtest/security/custom/CustomCaChainST.java
Outdated
Show resolved
Hide resolved
systemtest/src/test/java/io/strimzi/systemtest/security/custom/CustomCaChainST.java
Outdated
Show resolved
Hide resolved
systemtest/src/test/java/io/strimzi/systemtest/security/custom/CustomCaChainST.java
Outdated
Show resolved
Hide resolved
…eaks Signed-off-by: see-quick <maros.orsak159@gmail.com>
development-docs/systemtests/io.strimzi.systemtest.security.custom.CustomCaChainST.md
Show resolved
Hide resolved
| | 1. | Generate a custom CA chain: Root -> Intermediate -> Leaf. | CA chain is generated. | | ||
| | 2. | Generate a separate foreign Root CA. | Foreign CA is generated. | | ||
| | 3. | Generate four user certificates signed by Leaf CA, Intermediate CA, Root CA, and foreign CA respectively. | User certificates are generated. | | ||
| | 4. | Deploy user cert secrets and a CA trust secret containing only the Leaf CA cert. | Secrets are created in the namespace. | |
There was a problem hiding this comment.
This should be generated by the User Operator, or?
There was a problem hiding this comment.
I am not sure I understand, the point of this test case is verify broker side trust on custom listener. If I let UO to generarate these certs, then all will be signed by the same Clients CA and I would not be able to test negative cases.
Like here UO and CO should not have any involvement in this custom trust as I define custom auth listener which would have LeafCa. Like If I can not use UO generated user certs here as UO will sign it with the ClientsCA not with my LeafCA. My custom broker lsitener trust only the LeafCA so UO generated cert (which is signed by ClientsCA) would be rejected. Or am I wrong?
There was a problem hiding this comment.
The point I was trying to make is that I do not understand what Deploy user cert secrets means. User Secrets are normally created by the User Operator. Or what user secrets are you talking about here? So perhaps what you do is the right thing, but the description is not clear?
There was a problem hiding this comment.
Ah, I see ... by user cert secrets I meant client certificates secrets ... but I can see it's a bit confusing as I get your point.
It think maybe I can improve this Deploy user cert secrets ... wth Deploy external client certificate secrets (each signed by a different CA) ...?
| | 2. | Generate a separate foreign Root CA. | Foreign CA is generated. | | ||
| | 3. | Generate four user certificates signed by Leaf CA, Intermediate CA, Root CA, and foreign CA respectively. | User certificates are generated. | | ||
| | 4. | Deploy user cert secrets and a CA trust secret containing only the Leaf CA cert. | Secrets are created in the namespace. | | ||
| | 5. | Deploy Kafka with a custom listener (port 9122) configured with ssl.client.auth=required and PEM truststore pointing to the Leaf CA. | Kafka cluster is ready with custom listener. | |
There was a problem hiding this comment.
Should this be custom Clients CA instead?
There was a problem hiding this comment.
You mean just changing wording from Leaf CA with custom Clients CA? If so, I can do it for sure.
There was a problem hiding this comment.
You talk abotu custom listener (port 9122) configured with ssl.client.auth=required and PEM truststore pointing to the Leaf CA, but these seem to be 3 different things and all are confusing:
- Custom listener -> You mean
type: custom? Or user-configured? Not clear. ssl.client.auth=required-> that wouldtype: tlsauthentication? Or are you talking about this: https://strimzi.io/docs/operators/latest/full/configuring.html#configuring_customized_tls_client_authentication?- And so on
Again, maybe you do the right thing (to be honest, I did not get to the code part yet), but the description is not clear enough.
There was a problem hiding this comment.
Custom listener -> You mean type: custom? Or user-configured? Not clear.
It's an internal listener with name custom (i.e., user-configured). Maybe I should re-word it a bit better ... ?
ssl.client.auth=required -> that would type: tls authentication? Or are you talking about this: https://strimzi.io/docs/operators/latest/full/configuring.html#configuring_customized_tls_client_authentication?
Yes
| | 4. | Deploy user cert secrets and a CA trust secret containing only the Leaf CA cert. | Secrets are created in the namespace. | | ||
| | 5. | Deploy Kafka with a custom listener (port 9122) configured with ssl.client.auth=required and PEM truststore pointing to the Leaf CA. | Kafka cluster is ready with custom listener. | | ||
| | 6. | Verify that the user with a Leaf-CA-signed cert can produce and consume messages. | Messages are transmitted successfully. | | ||
| | 7. | Verify that users with Intermediate-CA, Root-CA, and foreign-CA-signed certs are rejected. | Producer/consumer time out due to TLS handshake failure. | |
There was a problem hiding this comment.
This is a godo test. I wonder if should try to test the same on the internal ports as well (9091).
There was a problem hiding this comment.
This is a godo test. I wonder if should try to test the same on the internal ports as well (9091).
If you think it make sense and it would add value but I am not sure how much code / addiional coverage it would do. I mean for me it would be just adding one test case (when I tried on my machine it tooks like 6 minutes so it should be fine)
There was a problem hiding this comment.
I think this has value. There is a separate code path, and the internal listeners are a big part of GHSA-2qwx-rq6j-8r6j. So I think this is worth 6 minutes of test time. You would need to add custom network policy to allow access to 9091 or 9090 from your clients. But otherwise it might be the same idea as for the Clients CA.
There was a problem hiding this comment.
I see, but I would also need to use ClusterCA signed client as Port 9091 (i.e., replication listener) trusts only the ClusterCA for client authentiufication.
You would need to add custom network policy to allow access to 9091 or 9090 from your clients.
Yeah, I think I can use something like this:
NetworkPolicyUtils.allowNetworkPolicyAllIngressForMatchingLabel(testStorage.getNamespaceName(),
testStorage.getClusterName() + "-internal-port-access",
testStorage.getBrokerPoolSelector().getMatchLabels());
development-docs/systemtests/io.strimzi.systemtest.security.custom.CustomCaChainST.md
Outdated
Show resolved
Hide resolved
development-docs/systemtests/io.strimzi.systemtest.security.custom.CustomCaChainST.md
Outdated
Show resolved
Hide resolved
development-docs/systemtests/io.strimzi.systemtest.security.custom.CustomCaChainST.md
Show resolved
Hide resolved
| | 1. | Generate a custom CA chain: Root -> Intermediate -> Leaf. | CA chain is generated. | | ||
| | 2. | Generate a separate foreign Root CA. | Foreign CA is generated. | | ||
| | 3. | Generate four user certificates signed by Leaf CA, Intermediate CA, Root CA, and foreign CA respectively. | User certificates are generated. | | ||
| | 4. | Deploy user cert secrets and a CA trust secret containing only the Leaf CA cert. | Secrets are created in the namespace. | |
Signed-off-by: see-quick <maros.orsak159@gmail.com>
Signed-off-by: see-quick <maros.orsak159@gmail.com>
| | Step | Action | Result | | ||
| | - | - | - | | ||
| | 1. | Generate a custom CA chain: Root -> Intermediate -> Leaf. | CA chain is generated. | | ||
| | 2. | Deploy the full custom CA chain as Cluster CA and Clients CA secrets. | CA secrets are deployed. | | ||
| | 3. | Deploy Kafka cluster with custom CAs (generateCertificateAuthority: false) so that broker certificates are signed by the Leaf CA. | Kafka cluster is ready. | | ||
| | 4. | Create a NetworkPolicy allowing all ingress to Kafka broker pods so that test clients can reach port 9091. | NetworkPolicy is created. | | ||
| | 5. | Generate a client certificate signed by the Cluster CA Leaf and create a KafkaTopic. | Client certificate and KafkaTopic are created. | | ||
| | 6. | Create trust secrets with different chain levels: Root + Intermediate + Leaf, Root + Intermediate, Root only, Intermediate only, Leaf only. | Trust secrets are created. | | ||
| | 7. | For each trust secret, verify that clients can produce and consume messages on port 9091. | All five trust configurations succeed. | | ||
| | 8. | Create a trust secret with only a foreign Root CA and verify that clients cannot connect on port 9091. | Producer/consumer time out due to trust failure. | |
There was a problem hiding this comment.
The test here is not necessarily invalid. But its core should be something different ...
- Create a client cert signed by Leaf CA
- Try to connect -> should work (I guess you have this at step 5 more or less from the description)
- Create a client certificate signed by Root CA or Intermediate CA
- Try to connect -> has to fail (=> this seems to be missing)
There was a problem hiding this comment.
- Try to connect -> should work (I guess you have this at step 5 more or less from the description)
correct.
Try to connect -> has to fail (=> this seems to be missing)
Yes, it's missing I can add these two case.
Okay so trust loop with different trust secrets you think it not needed? I can simplify that test to just do:
- Generate custom CA chain
- Deploy custom CA chain as Cluster CA and Client CA secrets
- Deploy Kafka cluster where broker certificates are signed by Leaf CA
- Create NetworkPolicy allowing ...
- Generate client cert signed by Leaf CA
- Verify that it works
- Generate client cert signed by Root CA or IM CA
- Verify it doesn't work
WDYT?
| | 2. | Generate a separate foreign Root CA. | Foreign CA is generated. | | ||
| | 3. | Generate four user certificates signed by Leaf CA, Intermediate CA, Root CA, and foreign CA respectively. | User certificates are generated. | | ||
| | 4. | Deploy external client certificate secrets (each signed by a different CA) and a broker-side CA trust secret containing only the Leaf CA cert. | Secrets are created in the namespace. | | ||
| | 5. | Deploy Kafka with an internal TLS listener using Custom TLS client authentication, with the broker truststore containing only the Leaf CA cert. | Kafka cluster is ready with the configured listener. | |
There was a problem hiding this comment.
I think this is still wrong. We want to test how we handle the Clients CA and how we establish trust from it. That was the CVE we had. So You need to use the CA from step one as a custom Clients CA and authenticate against it. If you use custom settings, you test how you configured it yourself. not how the operator does it.
There was a problem hiding this comment.
Okay, so here I would not use manual truststore configuration.
I would basically do the same as https://github.com/strimzi/strimzi-kafka-operator/pull/12497/changes/BASE..5a960e22b8448bd1ea038355b03a2ecbb48e18ac#r2959611701 but instead using internal port I would use the classic one for the clients. Right?
Type of change
Description
This PR adds a new test suite
CustomCaChainSTcovering user-defined listener using a multi-stage custom CA (i.e., #12364) plus CVE-2026-27133 and CVE-2026-27134.Checklist