Fixed CertUtils.handleOtherKeys() behavior with Cavium

[sfc-gh-jkowalski]

Description

Fixes #4509

This took a while to find the root cause:

The internal SPI fallback logic inside KeyFactory.generatePrivate() has the weird side effect of latching onto the LAST registered provider (which in our case was Cavium) after InvalidKeySpecException is thrown.

This choice is sticky for a single instance of KeyFactory and the fix for our issue is to get fresh KeyFactory instance when retrying.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• Feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change
• Chore (non-breaking change which doesn't affect codebase;
  test, version modification, documentation, etc.)

Checklist

• Code contributed by me aligns with current project license: Apache 2.0
• I Added CHANGELOG entry regarding this change
• I have implemented unit tests to cover my changes
• I have added/updated the javadocs and other documentation accordingly
• No new bugs, code smells, etc. in SonarCloud report
• I tested my code in Kubernetes
• I tested my code in OpenShift

[sfc-gh-dbrown]

Here is a simple repro showing that re-using the KeyFactory after an exception results in a different Provider than using a fresh KeyFactory:

import java.security.KeyFactory;
import java.security.Provider;
import java.security.Security;
import java.security.spec.InvalidKeySpecException;
import java.security.spec.PKCS8EncodedKeySpec;
import java.util.Base64;

public class Repro {
    public static void main(String[] args) throws Exception {
        System.out.println("List Providers that implement KeyFactory.RSA:");
        for (Provider p : Security.getProviders()) {
            if (p.getService("KeyFactory", "RSA") != null) {
                System.out.println(" " + p.getName());
            }
        }
        KeyFactory kf = KeyFactory.getInstance("RSA");
        // Uncommenting this will change behavior as calls to getProvider() pin the provider
        //System.out.println("Provider: " + kf.getProvider().getName());

        try {
            kf.generatePrivate(new PKCS8EncodedKeySpec(invalidPKCS8));
            System.out.println("success=bad");
        } catch (InvalidKeySpecException ex) {
            System.out.println("invalid key=good");
            var key = kf.generatePrivate(new PKCS8EncodedKeySpec(validPKCS8));
            System.out.println("Provider: " + kf.getProvider().getName());

            System.out.println("Do it again with a fresh KeyFactory and get a different Provider.");
            kf = KeyFactory.getInstance("RSA");
            key = kf.generatePrivate(new PKCS8EncodedKeySpec(validPKCS8));
            System.out.println("Provider: " + kf.getProvider().getName());
        }
    }

    private static final byte[] invalidPKCS8 = new byte[] {0,0,0};

    private static final byte[] validPKCS8 =
            Base64.getDecoder()
                    .decode(
                            "MIIBVgIBADANBgkqhkiG9w0BAQEFAASCAUAwggE8AgEAAkEAq7BFUpkGp3+LQmlQYx2eqzDV+xeG8kx/sQFV18S5JhzGeIJNA72wSeukEPojtqUyX2J0CciPBh7eqclQ2zpAswIDAQABAkAgisq4+zRdrzkwH1ITV1vpytnkO/NiHcnePQiOW0VUybPyHoGM/jf75C5xET7ZQpBe5kx5VHsPZj0CBb3b+wSRAiEA2mPWCBytosIU/ODRfq6EiV04lt6waE7I2uSPqIC20LcCIQDJQYIHQII+3YaPqyhGgqMexuuuGx+lDKD6/Fu/JwPb5QIhAKthiYcYKlL9h8bjDsQhZDUACPasjzdsDEdq8inDyLOFAiEAmCr/tZwA3qeAZoBzI10DGPIuoKXBd3nk/eBxPkaxlEECIQCNymjsoI7GldtujVnr1qT+3yedLfHKsrDVjIT3LsvTqw==");
}

[sfc-gh-jkowalski]

The style violation is pre-existing, not sure if I should be fixing it as part of the PR.

[shawkins]

@sfc-gh-jkowalski just run mvn spotless:apply then commit those changes. There was an outstanding issue / pr to bulk format the remaining files in the codebase, but it hasn't been committed.

[sfc-gh-jkowalski]

@shawkins done.

[sonarcloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

66.7% Coverage
0.0% Duplication

[sfc-gh-jkowalski]

So this style change now triggered code coverage warning. Could this PR be merged as-is or do you want me to do something about code coverage?

[shawkins]

LGTM

Don't worry about the coverage report, your core change is straight-forward.

[sfc-gh-jkowalski]

Is there anything blocking merging this PR?

[manusa]

The fix is brittle without a test that ensures this won't be reverted in the future.
A test would be good, but at least a comment pointing to the issue should be added here.