Skip to content

Commit

Permalink
fix: adds support for read-only system truststores
Browse files Browse the repository at this point in the history
closes #5316
  • Loading branch information
shawkins authored and manusa committed Oct 4, 2023
1 parent 03b84b2 commit dd457f8
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
* Fix #5423: OkHttpClientImpl supports setting request method for empty payload requests

#### Improvements
* Fix #5316: support read-only system KeyStores with Kube CA Certs
* Fix #5327: added proactive shutdown of informers on client close
* Fix #5432: [java-generator] Add the possibility to always emit `additionalProperties` on generated POJOs
* Fix #5410: [crd-generator] added support for `default`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.io.ByteArrayInputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
Expand All @@ -48,6 +49,7 @@
import java.security.spec.RSAPrivateCrtKeySpec;
import java.util.Base64;
import java.util.Collection;
import java.util.Collections;
import java.util.concurrent.Callable;
import java.util.stream.Collectors;

Expand All @@ -73,15 +75,16 @@ public static ByteArrayInputStream getInputStreamFromDataOrFile(String data, Str
public static KeyStore createTrustStore(String caCertData, String caCertFile, String trustStoreFile,
String trustStorePassphrase) throws IOException, CertificateException, KeyStoreException, NoSuchAlgorithmException {
ByteArrayInputStream pemInputStream = getInputStreamFromDataOrFile(caCertData, caCertFile);
return createTrustStore(pemInputStream, trustStoreFile,

KeyStore trustStore = loadTrustStore(trustStoreFile,
getPassphrase(TRUST_STORE_PASSWORD_SYSTEM_PROPERTY, trustStorePassphrase));
}

private static KeyStore createTrustStore(ByteArrayInputStream pemInputStream, String trustStoreFile,
char[] trustStorePassphrase)
throws IOException, CertificateException, KeyStoreException, NoSuchAlgorithmException {
return mergePemCertsIntoTrustStore(pemInputStream, trustStore, true);
}

final String trustStoreType = System.getProperty(TRUST_STORE_TYPE_SYSTEM_PROPERTY, KeyStore.getDefaultType());
static KeyStore loadTrustStore(String trustStoreFile, char[] trustStorePassphrase)
throws KeyStoreException, IOException, NoSuchAlgorithmException, CertificateException, FileNotFoundException {
String trustStoreType = System.getProperty(TRUST_STORE_TYPE_SYSTEM_PROPERTY, KeyStore.getDefaultType());
KeyStore trustStore = KeyStore.getInstance(trustStoreType);

if (Utils.isNotNullOrEmpty(trustStoreFile)) {
Expand All @@ -91,19 +94,45 @@ private static KeyStore createTrustStore(ByteArrayInputStream pemInputStream, St
} else {
loadDefaultTrustStoreFile(trustStore, trustStorePassphrase);
}
return trustStore;
}

static KeyStore mergePemCertsIntoTrustStore(ByteArrayInputStream pemInputStream, KeyStore trustStore, boolean first)
throws CertificateException, KeyStoreException {
CertificateFactory certFactory = CertificateFactory.getInstance("X509");
while (pemInputStream.available() > 0) {
X509Certificate cert;
try {
X509Certificate cert = (X509Certificate) certFactory.generateCertificate(pemInputStream);
String alias = cert.getSubjectX500Principal().getName() + "_" + cert.getSerialNumber().toString(16);
trustStore.setCertificateEntry(alias, cert);
cert = (X509Certificate) certFactory.generateCertificate(pemInputStream);
} catch (CertificateException e) {
if (pemInputStream.available() > 0) {
// any remaining input means there is an actual problem with the key contents or file format
throw e;
}
LOG.debug("The trailing entry generated a certificate exception. More than likely the contents end with comments.", e);
break;
}
try {
String alias = cert.getSubjectX500Principal().getName() + "_" + cert.getSerialNumber().toString(16);
trustStore.setCertificateEntry(alias, cert);
first = false;
} catch (KeyStoreException e) {
if (first) {
// could be that the store type is not writable, rather than some elaborate check for read-only
// we'll simply try again with a well supported type
pemInputStream.reset();
KeyStore writableStore = KeyStore.getInstance("PKCS12");
try {
writableStore.load(null, null); // initialize the instance
} catch (NoSuchAlgorithmException | CertificateException | IOException e1) {
throw e; // not usable, just give up
}
for (String alias : Collections.list(trustStore.aliases())) {
writableStore.setCertificateEntry(alias, trustStore.getCertificate(alias));
}
return mergePemCertsIntoTrustStore(pemInputStream, writableStore, false);
}
throw e;
}
}
return trustStore;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;

import java.io.File;
import java.io.IOException;
Expand All @@ -36,6 +37,7 @@
import java.util.Properties;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertNotSame;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
Expand Down Expand Up @@ -68,6 +70,21 @@ public void resetSystemPropertiesBack() {
System.setProperties(systemProperties);
}

@Test
void handleReadOnlyJavaTrustStore() throws Exception {
KeyStore system = CertUtils.loadTrustStore(null, "changeit".toCharArray());
KeyStore trustStore = Mockito.spy(system);
Mockito.doThrow(KeyStoreException.class).when(trustStore).setCertificateEntry(Mockito.anyString(), Mockito.any());
KeyStore result = CertUtils.mergePemCertsIntoTrustStore(
CertUtils.getInputStreamFromDataOrFile(null, "src/test/resources/ssl/multiple-certs.pem"), trustStore, true);

assertNotSame(trustStore, result);
assertThat(Collections.list(result.aliases()))
.hasSizeGreaterThanOrEqualTo(2)
.satisfiesOnlyOnce(alias -> assertThat(alias).contains("openshift-signer"))
.satisfiesOnlyOnce(alias -> assertThat(alias).contains("openshift-service-serving-signer"));
}

@Test
void loadingMultipleCertsFromSameFile() throws Exception {
KeyStore ts = CertUtils.createTrustStore(
Expand Down

0 comments on commit dd457f8

Please sign in to comment.