in relatation to feature 3595#

[jwojcie]

Describe the bug
I'm afraid that feature in #3595 is not going to work properly. I tested locally that changed piece of code on my FIPS hardened Java, which means that apart from other stuff I have this in java.security:
security.provider.1=org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider C:DEFRND[SHA256];ENABLE{ALL}; security.provider.2=org.bouncycastle.jsse.provider.BouncyCastleJsseProvider fips:BCFIPS
security.provider.3=SUN

So I have 3 providers, nothing more or less (that's a configuration recommended for env compliant with FIPS 140). I used this small code snippet:

`public static void listProviders() {

System.out.println("Kubernetes fix: +++++++++++++++++++++++++++++");
ServiceLoader<Provider> services = ServiceLoader.load(java.security.Provider.class);
for (Provider service : services) {
    System.out.println("Found security provider: " + service.getName());
    //Security.addProvider(service);
}

System.out.println("Actual providers: +++++++++++++++++++++++++++++++");
for (Provider provider : Security.getProviders()) {
    System.out.println("Provider: " + provider.getName() + ", Version: " + provider.getVersion());
}

System.out.println("Just for reference: +++++++++++++++++++++++++++++++");
Provider provider = new org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider();
System.out.println("Provider: " + provider.getName() + ", Version: " + provider.getVersion());

}`

And the result is kind of surprising:
Kubernetes fix: +++++++++++++++++++++++++++++ Found security provider: SunPCSC Found security provider: SunEC Found security provider: SunPKCS11 Found security provider: SunMSCAPI Found security provider: JdkSASL Found security provider: XMLDSig Found security provider: SunJGSS Found security provider: SunSASL Found security provider: JdkLDAP Actual providers: +++++++++++++++++++++++++++++++ Provider: BCFIPS, Version: 2.0 Provider: BCJSSE, Version: 2.0019 Provider: SUN, Version: 11.0 Just for reference: +++++++++++++++++++++++++++++++ Provider: BCFIPS, Version: 2.0

Which makes me think that the fix you made is not going to work because that loop is returning something else than expected, or did I miss something here?

In that context may I interest you in the netty solution to the problem?
https://github.com/netty/netty/blob/4.1/handler/src/main/java/io/netty/handler/ssl/util/BouncyCastleSelfSignedCertGenerator.java#L44

Client Version
actual client wasn't tested, just a part of code from the feature change

Kubernetes Version
not aplicable

Java Version
11

To Reproduce
In description.

Expected behavior
kubernetes client should fall back to org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider in the absence of non-compliant BC library.

KubeConfig
not applicable

Server (please complete the following information):
tested on Windows

Additional context
#3595

[brendandburns]

This loop is just looking through your classpath, so I think that you must have these on your classpath for some reason.

[jwojcie]

I guess you're right, I think those are coming from the Java distribution itself. But in this case imho it adds salt to the wound here, because it ignores the order and constraints of java.security file which it should not do. I think the proper implementation would be to use:
for (Provider provider : Security.getProviders()) {
which would return:

• only those providers which are listed in the java.security
• in the order which is set there - because that order is very significant in the FIPS 140 context - so for example in my setting it musn't be SUN - which is there on the third place only for some technical, not security related hashing. All security related stuff must be processed via BC FIPS, otherwise it is not compliant.

Well I guess, that piece of code is redundant if java.security is configured properly.

Anyway, it seems Security.addProvider(service); adds the provider to the end of the list. So, while not ideal (because I would not like list of providers to be populated with whole bunch of non compliant providers found is the classpath somewhere) I guess it may work in the meantime, so at least kubernetes-client doesn't blow up. But from compliance point of view it is an issue - because the system will let to exercise some cryptographic method which BC FIPS will deny, but some random provider down the list will happily perform. So it kind of defies the purpose of all those BC FIPS validations.

I think in the long run it would be great if kubernetes-client didn't play with providers if those are configured via java.security file.

Can you tell me when the current fix can be available in the release?

[brendandburns]

We're happy to take PRs to improve this logic. I will definitely admit that I'm not sure what the 100% right answer is here. Our goals are to have something that works for most people (who are likely non-FIPS) easily, and make it feasible for people who have regulated environments to achieve what they need.

If you have suggestions for improving the code, please send PRs.

This code should be released in 22.0.0 which should be in the next couple of weeks.

[jwojcie]

Thank you for the release date info. Imho it would be best to just remove that piece of code and rely on java.security settings. Keeping the java security configuration transparent/lously coupled with actual Security Provider implementations was a goal of JCA. Best practice is to refrain from any direct imports from any implementations of Security Provider, so it can be easily switched if there is a need for it - like for the compliance purpose in my case.

I'm not sure though why there was a need to programatically add the BC provider in the first place in this library.

[brendandburns]

The goal for the direct reference was to make the default experience automatic. If you loosly couple and there's no crypto present in the classpath, it's hard for the easy case to work simply.

[jwojcie]

Understood, however I think it is no longer relevant after current change. The reason is that typical java distribution has some default java.security with some default settings like:

security.provider.1=SUN
security.provider.2=SunRsaSign
security.provider.3=SunEC
security.provider.4=SunJSSE
security.provider.5=SunJCE
security.provider.6=SunJGSS
security.provider.7=SunSASL
security.provider.8=XMLDSig
security.provider.9=SunPCSC
security.provider.10=JdkLDAP
security.provider.11=JdkSASL
security.provider.12=SunMSCAPI
security.provider.13=SunPKCS11

I think this new code snippet for default/out of the box java installations is just doing nothing really. But it is going to mess up a bit those environments where java.security configuration was changed on purpose.
So it is not needed for default experience, and problematic for those who consciously set up their java security settings.
When I test it on jdk 11 out of the box distribution with that:

public static void kubernetesFix() {
        System.out.println("Providers out of the box: +++++++++++++++++++++++++++++++");
        for (Provider provider : Security.getProviders()) {
            System.out.println("Provider: " + provider.getName() + ", Version: " + provider.getVersion());
        }

        System.out.println("Kubernetes fix: +++++++++++++++++++++++++++++");
        ServiceLoader<Provider> services = ServiceLoader.load(java.security.Provider.class);
        for (Provider service : services) {
            System.out.println("Found security provider: " + service.getName());
            Security.addProvider(service);
        }

        System.out.println("Providers after fix: +++++++++++++++++++++++++++++++");
        for (Provider provider : Security.getProviders()) {
            System.out.println("Provider: " + provider.getName() + ", Version: " + provider.getVersion());
        }
    }

I have that result:

Providers out of the box: +++++++++++++++++++++++++++++++
Provider: SUN, Version: 11.0
Provider: SunRsaSign, Version: 11.0
Provider: SunEC, Version: 11.0
Provider: SunJSSE, Version: 11.0
Provider: SunJCE, Version: 11.0
Provider: SunJGSS, Version: 11.0
Provider: SunSASL, Version: 11.0
Provider: XMLDSig, Version: 11.0
Provider: SunPCSC, Version: 11.0
Provider: JdkLDAP, Version: 11.0
Provider: JdkSASL, Version: 11.0
Provider: SunMSCAPI, Version: 11.0
Provider: SunPKCS11, Version: 11.0
Kubernetes fix: +++++++++++++++++++++++++++++
Found security provider: SunMSCAPI
Found security provider: SunPKCS11
Found security provider: SunEC
Found security provider: SunPCSC
Found security provider: SunJGSS
Found security provider: XMLDSig
Found security provider: JdkSASL
Found security provider: SunSASL
Found security provider: JdkLDAP
Providers after fix: +++++++++++++++++++++++++++++++
Provider: SUN, Version: 11.0
Provider: SunRsaSign, Version: 11.0
Provider: SunEC, Version: 11.0
Provider: SunJSSE, Version: 11.0
Provider: SunJCE, Version: 11.0
Provider: SunJGSS, Version: 11.0
Provider: SunSASL, Version: 11.0
Provider: XMLDSig, Version: 11.0
Provider: SunPCSC, Version: 11.0
Provider: JdkLDAP, Version: 11.0
Provider: JdkSASL, Version: 11.0
Provider: SunMSCAPI, Version: 11.0
Provider: SunPKCS11, Version: 11.0

As you can see nothing change. Providers are not added because they are already there instantiated by JVM based on out of the box java.security settings.