-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
KubernetesClient should use provided SSLContext in config #5316
Comments
Just the SSLContext may not be enough. At least okhttp wants the trustmanager specified separately. And there may need to be some tweaks to the vertx direct usage of the SSLContext we're passing in as it doesn't seem to fully be working #5170 If the SSLUtils.sslContext method is good enough for your usage, would you be okay with supplying the keymanagers and trustManagers to the KubernetesClientBuilder as a way to by-pass the configuration driven approach? I realize these may ultimately need to go onto the Config if we want to keep the current Config.ensureHttps functioning the same way in this case. |
@shawkins Thanks for your response. Unfortunately, I won't be able to pass any trustmanager unless I build one explicitly and that's what I have been trying to do. Realized there is nothing I can set that will make it work because of the other ssl properties. These are our JVM system properties. Apparently we have our own keystore format implemented based on PEM certificates in the underlying JDK.
But here's an interesting point. The io.kubernetes works perfectly fine. It's only fabric8io client that is not working without the last |
Is the /security/cacerts.pem being picked up from the Config? Have you tried leaving the system properties javax.net.ssl.trustStore=/security/cacerts.pem and javax.net.ssl.trustStoreType=ROTKS and creating a config without the caCertFile
What specifically is happening there? |
@shawkins We cannot run without setting |
I will get back to you on DER related exception because I have to reproduce that problem. |
I'm saying leave those system properties set - that makes the /security/cacerts.pem the system default truststore. Then create a KubernetesClient Config that won't autoconfig the truststore: new new ConfigBuilder().withCaCertFile(null).build() - it should pick up the /security/cacerts.pem instead. |
I will try it & get back. Thanks for the suggestion. |
Ok, that's effectively
|
It should not be. It's picking up a certFile or data from the Config Line 168 in 9db6e48
Would it be possible to provide what is coming from the Config? Redacted, if needed values, of the parameters from Line 117 in 9db6e48
That will ensure we know exactly what the logic is acting on. |
Tried
|
This confirms you do need the caCert information coming from the Config (which is expected). It should also confirm that the base truststore is loading correctly via just the javax properties. As mentioned in the last comment, just to make sure we're fully reproducing your scenario - could provide all of the parameter values to the trustManagers call? |
ACK. I will try this and get back. |
By passing the
|
I am confused. The implementation doesn't like adding anything to a truststore. But without adding cacerts, the communication with k8s server was failing because of |
I think I see the problem: https://github.com/fabric8io/kubernetes-client/blob/main/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/internal/SSLUtils.java#L168 If both caCertFile & caCertData are null or empty, its also failing to add trustStoreFile. |
Again please provide the parameters being provided to the trustManagers call so that we can understand what all is being picked up by the auto-configuration. |
@shawkins here you go:
|
Is this what is picked up from just auto-configuration - that is no modification of the Config via withXXX methods? And just to confirm cacerts.pem as both certFile and the trustStoreFile refer to the same file which is also what you have set as javax.net.ssl.trustStore=/security/cacerts.pem ?
This is using withCaCertFile(null), correct? I'm confused that it is producing the same exception as the first case, because this should not call createTrustStore. I thought this should produce #5316 (comment) - my initial thought here was that having the appropriate javax.net.ssl.trustStoreType and javax.net.ssl.trustStore system properties set would have caused the existing logic to use that trustStore by default. I'll double check that. |
No. I set
Yes.
Sorry for the confusion. I mistyped setting
|
In summary,
|
Somewhere via the env or the kube config the auto configuration is picking up that there is BOTH a caCert and a trustStore that is the same file - that seems wrong. Depending on what certs it holds your cacerts.pem should be seen as one or the other, but not both. Are you setting the kubernetes.truststore.file env / system property? Next when the logic sees that there is a caCert it tries to create a merged trustStore - but that is failing due to the custom ROTKS keystore type (that should be picked up from your javax.net.ssl.trustStoreType system property). It seems like the logic could account for this by creating yet another trustStore, such as jks, and then put all the certs into that. But I'm not sure yet if that change is necessary.
In this scenario did you leave the system properties javax.net.ssl.trustStore and javax.net.ssl.trustStoreType set? I can confirm locally my expectation that this case will try to load the trust store by default from those properties. Please also set javax.net.debug=all to see what is happening. If you can see that the javax.net.ssl.trustStore is being used, then the problem is that you are supposed to be picking up another caCert file from the Config - like the one that would be associated with the serviceaccount - /var/run/secrets/kubernetes.io/serviceaccount/ca.crt - but the usage of other env / system properties is causing it not to be picked up. |
Do you think if I tried with |
Corrected my last comment to be the kubernetes.truststore.file property.
The issue is what you are doing with other system / env properties. In this case if you don't configure the fabric8 kubernetes client with an explicit trustStore (by not setting kubernetes.truststore.file or by using withTrustStore(null)), it will look for the default via the javax system properties - as long as you have set the truststore type to ROTKS that won't work. I think you need to take a step back and properly identify what trustStore(s) you have in play and what are their purpose. Also for any scenario discussed you should also include all relevant system / env properties. I'm having a hard time following why the auto configuration is picking up the values that it is, because it does not seem to match what you showed in #5316 (comment) |
@AKGarimella I have reviewed this issue again and believe the base case of what you are doing - that is have the system default truststore as read-only is something that we can support. I don't see a good workaround for you unless you take responsbility for creating a writable store type containing the ROTKS store entries. There is now a pr for this #5489 |
Is your enhancement related to a problem? Please describe
Fabric8 k8s client library always tries to create an SSLContext by itself using the truststore and keystore related parameters provided using Config object. However, in doing so, there are a lot of assumptions made reg. the runtime viz., format of the store files, whether or not pass phrases should exist, underlying security libraries etc.
In the past when we tried to use fabric8io client in our FIPS env., assuming
changeit
as the default password caused problem.(See #5126). Though this issue was addressed in later versions, we hit another problem because of PEM vs DER format assumptions made.It's turning out that basically the fabric8io k8s client is just not compatible with our FIPS compliant JDK runtime.
Describe the solution you'd like
Config object should allow user to pass an SSLContext. When SSLContext is provided in Config, fabric8 k8s client should simply use it AS IS by ignoring all other keystore/truststore related properties.
Config k8sClientConfig = new ConfigBuilder().withSSLContext(mySSLContext).build();
Describe alternatives you've considered
Temporarily, we had to rely on
kubernetes.trust.certificates=true
to bypass this whole problem.Additional context
No response
The text was updated successfully, but these errors were encountered: