Enable client to get certificate from system keystore

[huw0]

Description

In #10482 support was added for the client to use the system truststore. This adds a further property for using the system keystore. This is helpful in corporate environments where PKI infrastructure is automatically provisioned by Active Directory.

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(x) Release notes are required, with the following suggested text:

# Client
* Enable client to get certificate from system keystore ({issue}`issuenumber`)

[huw0]

Notes for manual testing

For manual testing, a CA is required along with a server certificate and a user certificate.

Server

• Configure Trino server for certificate authentication and ensure that the CA is in the truststore

Client (Windows)

1. Open mmc
2. File -> Add/Remove Snap-in
3. Add the 'Certificates' Snap-in for 'My User Account'
4. In 'Personal -> Certificates' Import the certificate and key

If the CA is not already trusted, the above steps must be repeated but running as Administrator

1. Certificates Snap-in for 'Computer Account'
2. In 'Trusted Root Certification Authorities -> Certificates' import the CA cert.

When running the Trino client with the system keystore, a password prompt will appear in the UI if the key has been imported in 'high-security' mode (not the default).

Client (Mac)

1. Open the Keychain Access app
2. Use the login keychain
3. In the Certificates tab import the CA certificate
4. In the My Certificates tab import the user certificate and key

I found the process to be quite fiddly.

• This covers how to export PEM files with the correct legacy crypto to be imported into MacOS keychain
• Sometimes after importing, the certificate still wasn't accessible in Java. I'm not quite sure what solved it. Options I tried included...
  • Adjusting the trust settings on the certificate
  • Adjusting the permissions settings on the key
  • Removing entirely and then importing cert+key in the CLI security import ./my-cert.p12 -t priv -f pkcs12

This code will show whether the certificate has an accessible key:

KeyStore keyStore = KeyStore.getInstance("KeychainStore");
keyStore.load(null, null);

Enumeration<String> enumerator = keyStore.aliases();
while (enumerator.hasMoreElements()) {
    String alias = enumerator.nextElement();
    
    // This needs to return true!
    System.out.println(alias + " " + keyStore.isKeyEntry(alias));
}

When running the Trino client with the system keystore, a password prompt will appear in the UI for the system keystore. This requires administrator credentials and can be denied. A second prompt will then appear requesting permission to the user's login keystore.

[huw0]

This is my first PR against the main trino repo. I'm seeing some CI failures however I'm seeing the same on some other recent PRs?

[nineinchnick]

LGTM. If you suspect the CI test failures are unrelated, you can push an empty commit to run it again. Empty commits are discarded when PRs are merged.

[github-actions]

This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua

[mosabua]

Do we need to add something for Linux since that is actually our main supported operating system .. neither Windows or MacOS are ..

[huw0]

Initially I just added support for Windows. However I wanted to ensure feature parity in the client with the system trust store option so added MacOS too.

I'd be interested in Linux support as well, but as far as I know there isn't a standardised way to do this that is supported out of the box with JDK Providers?

I guess this is probably due to the proliferation of different Linux keystore implementations, some of which are provided by the different desktop environments and some by various auth tooling.

[mosabua]

Doc updates are fine.

@wendigo @electrum can this get onto your radar for review and potential merge.

[wendigo]

Can you rebase?

[wendigo]

@nineinchnick ptal :)

[nineinchnick]

LGTM, just some nitpicks.

[mosabua]

You want to push the fixes for the nits and merge @wendigo ?

[wendigo]

@mosabua let's do it!