Skip to content

Commit

Permalink
apacheGH-39014: [Java] Add default truststore along with KeychainStor…
Browse files Browse the repository at this point in the history
…e when on Mac system (apache#39235)

### Rationale for this change
As described in apache#39014, when using the system TrustStore on Mac, the certificates returned do not include Root CAs trusted by the system. This change adds the default KeyStore instance along with the KeyChainStore to include trusted Root CAs. The reason we add the default KeyStore instance is because there is no easy way to get the certificates from the System Roots keychain. 

### What changes are included in this PR?

I've updated ClientAuthenticationUtils to get the default KeyStore instance when the operating system is macOS and have updated the tests to include this change. 

### Are these changes tested?

See changes made in ClientAuthenticationUtilsTest.java.

### Are there any user-facing changes?

No

* Closes: apache#39014

Authored-by: Ravjot Brar <ravjot.brar@dremio.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
  • Loading branch information
ravjotbrar authored and clayburn committed Jan 23, 2024
1 parent 3b13ba4 commit 5e1e3b2
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,16 @@ static KeyStore getKeyStoreInstance(String instance)
return keyStore;
}

@VisibleForTesting
static KeyStore getDefaultKeyStoreInstance(String password)
throws KeyStoreException, CertificateException, NoSuchAlgorithmException, IOException {
try (InputStream fileInputStream = getKeystoreInputStream()) {
KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType());
keyStore.load(fileInputStream, password == null ? null : password.toCharArray());
return keyStore;
}
}

static String getOperatingSystem() {
return System.getProperty("os.name");
}
Expand Down Expand Up @@ -156,16 +166,9 @@ public static InputStream getCertificateInputStreamFromSystem(String password) t
keyStoreList.add(getKeyStoreInstance("Windows-MY"));
} else if (isMac()) {
keyStoreList.add(getKeyStoreInstance("KeychainStore"));
keyStoreList.add(getDefaultKeyStoreInstance(password));
} else {
try (InputStream fileInputStream = getKeystoreInputStream()) {
KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType());
if (password == null) {
keyStore.load(fileInputStream, null);
} else {
keyStore.load(fileInputStream, password.toCharArray());
}
keyStoreList.add(keyStore);
}
keyStoreList.add(getDefaultKeyStoreInstance(password));
}

return getCertificatesInputStream(keyStoreList);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,33 @@ public void testGetKeyStoreInstance() throws IOException,
}
}

@Test
public void testGetDefaultKeyStoreInstancePassword() throws IOException,
KeyStoreException, CertificateException, NoSuchAlgorithmException {
try (MockedStatic<KeyStore> keyStoreMockedStatic = Mockito.mockStatic(KeyStore.class)) {

keyStoreMockedStatic
.when(() -> ClientAuthenticationUtils.getDefaultKeyStoreInstance("changeit"))
.thenReturn(keyStoreMock);
KeyStore receiveKeyStore = ClientAuthenticationUtils.getDefaultKeyStoreInstance("changeit");
Assert.assertEquals(receiveKeyStore, keyStoreMock);
}
}

@Test
public void testGetDefaultKeyStoreInstanceNoPassword() throws IOException,
KeyStoreException, CertificateException, NoSuchAlgorithmException {
try (MockedStatic<KeyStore> keyStoreMockedStatic = Mockito.mockStatic(KeyStore.class)) {

keyStoreMockedStatic
.when(() -> ClientAuthenticationUtils.getDefaultKeyStoreInstance(null))
.thenReturn(keyStoreMock);
KeyStore receiveKeyStore = ClientAuthenticationUtils.getDefaultKeyStoreInstance(null);
Assert.assertEquals(receiveKeyStore, keyStoreMock);
}
}


@Test
public void testGetCertificateInputStreamFromMacSystem() throws IOException,
KeyStoreException, CertificateException, NoSuchAlgorithmException {
Expand All @@ -90,11 +117,18 @@ public void testGetCertificateInputStreamFromMacSystem() throws IOException,
keyStoreMockedStatic.when(() -> ClientAuthenticationUtils
.getKeyStoreInstance("KeychainStore"))
.thenReturn(keyStoreMock);
keyStoreMockedStatic.when(() -> ClientAuthenticationUtils
.getDefaultKeyStoreInstance("changeit"))
.thenReturn(keyStoreMock);
clientAuthenticationUtilsMockedStatic
.when(ClientAuthenticationUtils::getKeystoreInputStream)
.thenCallRealMethod();
keyStoreMockedStatic.when(KeyStore::getDefaultType).thenCallRealMethod();
keyStoreMockedStatic.when(() -> ClientAuthenticationUtils
.getCertificatesInputStream(Mockito.any()))
.thenReturn(mock);

InputStream inputStream = ClientAuthenticationUtils.getCertificateInputStreamFromSystem("test");
InputStream inputStream = ClientAuthenticationUtils.getCertificateInputStreamFromSystem("changeit");
Assert.assertEquals(inputStream, mock);
}
}
Expand Down Expand Up @@ -136,9 +170,11 @@ public void testGetCertificateInputStreamFromLinuxSystem() throws IOException,

setOperatingSystemMock(clientAuthenticationUtilsMockedStatic, false, false);
keyStoreMockedStatic.when(() -> ClientAuthenticationUtils
.getCertificatesInputStream(Mockito.any()))
.getCertificatesInputStream(Mockito.any()))
.thenReturn(mock);

keyStoreMockedStatic.when(() -> ClientAuthenticationUtils
.getDefaultKeyStoreInstance(Mockito.any()))
.thenReturn(keyStoreMock);
clientAuthenticationUtilsMockedStatic
.when(ClientAuthenticationUtils::getKeystoreInputStream)
.thenCallRealMethod();
Expand Down

0 comments on commit 5e1e3b2

Please sign in to comment.