search

redhat-developer / intellij-common

4 stars 11 forks source link

IDEATrustManager does not prompt user to accept/refuse unknown certificates #212

Closed adietish closed 5 months ago

adietish commented 5 months ago

In #206 we moved the IDEATrustManager from intellij-kubernetes to commons. This move did not include the CompositeX509ExtendedTrustManager that was present in the original but used the one in https://github.com/Hakky54/sslcontext-kickstart instead (that the manager in intellij-kubernetes was based on). With this implementation intellij-kubernetes wasn't asking the user any more, whether he wants to accept/refuse an unknown certificate

Hakky54 commented 5 months ago

Hi Andre, I noticed this issue and the usage of the IDEATrustManager. The library has an InflatableX509ExtendedTrustManager which provides the same feature as the IDEATrustManager. It can prompt the user whether to trust the new certificate. Wouldn't that be an option for you guys instead of having a custom CompositeX509ExtendedTrustManager and IDEATrustManager?

I am just jumping in here out of curiosity

adietish commented 5 months ago

Hi @Hakky54 Thanks for the pointer. We were obviously using the wrong class. We'd benefit from using your lib by using your well tested code. The drawback is to add further 200k to our lib. I guess it's worth it given the small size. Trying 😄

adietish commented 5 months ago

@Hakky54: Guess I misunderstood you. IDEATrustManager is invoking the jetbrains IDEA manager when prompting and thus integrating the jetbrains IDEA preferences for certificates. There's no way replacing this to nicely integrate into IDEA.

Hakky54 commented 5 months ago

Ahh, I get it. I didn't know something like that exists, thank you for giving some context. This is pretty interesting, I am bookmarking it 😄

adietish commented 5 months ago

@Hakky54 our impl is a bit hacky because there's no extension point nor API to smoothly integrate into jetbrains IDEA in this area. We replace the trust manager in CertificateManager.getInstance().getTrustManager() (the jetbrains class) with a wrapper (CompositeX509ExtendedTrustManager). Nevertheless, the hacky bits are limited to a single method. Crossing fingers jetbrains do offer "official" means at some point.

Hakky54 commented 5 months ago

Yes, I just checked and I see that you replace the mySystemManager class field of the ConfirmingTrustManager using reflection with your own TrustManager. Very clever. The latest version of ConfirmingTrustManager has an add method, that might be useful:

https://github.com/JetBrains/intellij-community/blob/d9d260a048ef2105e801d18ffbca87c108b0a187/platform/platform-api/src/com/intellij/util/net/ssl/ConfirmingTrustManager.java#L154

I think if you use it in that way you don't need the X509CompositeTrustManager at all. You can then just append your custom trustmanager. what do you think?

Sorry to bother you with my questions, I am just curious but I am stepping out of this issue so I won't take your focus away 😄

adietish commented 5 months ago

@Hakky54 thanks for the pointers. Nice find actually, thanks! Not sure why I didnt use it back when I implemented it as it seems to exist for quite some years. Looking into it. Not annoyed by any of your comments at all. Enjoying the true FOSS exchange :)

ps. we aim at supporting a wider range of IDEAs and move up the lower bound as new releases are published. We're currently at IC-2022.1+ which seems to offer this method.

adietish commented 5 months ago

trying to add the kubernetes-client certificates to the MutableTrustManager within jetbrains ConfirmingTrustManager is not working for me. MutableTrustManager spits an exception when it is initialized before my certificate would be added to it. 

java.io.IOException: Short read of DER length
    at java.base/sun.security.util.DerInputStream.getLength(DerInputStream.java:584)
    at java.base/sun.security.util.DerValue.init(DerValue.java:383)
    at java.base/sun.security.util.DerValue.<init>(DerValue.java:324)
    at java.base/sun.security.util.DerValue.<init>(DerValue.java:337)
    at java.base/sun.security.pkcs12.PKCS12KeyStore.engineLoad(PKCS12KeyStore.java:1973)
    at java.base/sun.security.util.KeyStoreDelegator.engineLoad(KeyStoreDelegator.java:222)
    at java.base/java.security.KeyStore.load(KeyStore.java:1479)
    at com.intellij.util.net.ssl.ConfirmingTrustManager$MutableTrustManager.createKeyStore(ConfirmingTrustManager.java:217)
    at com.intellij.util.net.ssl.ConfirmingTrustManager$MutableTrustManager.<init>(ConfirmingTrustManager.java:192)

I did not dig further trying to understand what's wrong here. I'll stick to my current hacky implementation and revisit this once we move up the min supported IDEA version.