Skip to content

Commit

Permalink
[common][server][fc] Fix trust store handling for Grpc with SSL (#816)
Browse files Browse the repository at this point in the history
Currently, "GrpcSslUtils#getTrustManagers" loads the trust store but expects the trust store to be of the same type as the key store. While this is true in our tests, it might not always be true in practice. This PR fixes this by handling trust store types explicitly
  • Loading branch information
nisargthakkar authored Jan 12, 2024
1 parent d788799 commit 87b957d
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import static com.linkedin.venice.CommonConfigKeys.SSL_KEYSTORE_TYPE;
import static com.linkedin.venice.CommonConfigKeys.SSL_TRUSTSTORE_LOCATION;
import static com.linkedin.venice.CommonConfigKeys.SSL_TRUSTSTORE_PASSWORD;
import static com.linkedin.venice.CommonConfigKeys.SSL_TRUSTSTORE_TYPE;

import java.util.Properties;

Expand All @@ -15,6 +16,7 @@ public class SSLConfig {
private String _keyStorePassword = "";
private String _keyStoreType = "jks";
private String _keyStoreFilePath = "";
private String _trustStoreType = "jks";
private String _trustStoreFilePath = "";
private String _trustStoreFilePassword = "";
private boolean _sslEnabled = false;
Expand Down Expand Up @@ -69,6 +71,14 @@ public String getKeyStoreType() {
return _keyStoreType;
}

public void setTrustStoreType(String trustStoreType) {
_trustStoreType = trustStoreType;
}

public String getTrustStoreType() {
return _trustStoreType;
}

public void setSslEnabled(boolean sslEnabled) {
_sslEnabled = sslEnabled;
}
Expand Down Expand Up @@ -101,6 +111,7 @@ public static SSLConfig buildConfig(Properties sslProperties) {
config.setSslEnabled(Boolean.valueOf(sslProperties.getProperty(SSL_ENABLED)));
config.setKeyStoreType(sslProperties.getProperty(SSL_KEYSTORE_TYPE));
config.setKeyStoreFilePath(sslProperties.getProperty(SSL_KEYSTORE_LOCATION));
config.setTrustStoreType(sslProperties.getProperty(SSL_TRUSTSTORE_TYPE));
config.setTrustStoreFilePath(sslProperties.getProperty(SSL_TRUSTSTORE_LOCATION));
config.setKeyStorePassword(sslProperties.getProperty(SSL_KEYSTORE_PASSWORD));
config.setTrustStoreFilePassword(sslProperties.getProperty(SSL_TRUSTSTORE_PASSWORD));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,23 +46,6 @@ public class SslUtils {
public static final String LOCAL_PASSWORD = "dev_pass";
public static final String LOCAL_KEYSTORE_JKS = "localhost.jks";

/**
* This function should be used in test cases only.
*
* @return an instance of {@link SSLConfig} with local SSL config.
*/
public static SSLConfig getLocalSslConfig() {
String keyStorePath = getPathForResource(LOCAL_KEYSTORE_JKS);
SSLConfig sslConfig = new SSLConfig();
sslConfig.setKeyStoreFilePath(keyStorePath);
sslConfig.setKeyStorePassword(LOCAL_PASSWORD);
sslConfig.setKeyStoreType("JKS");
sslConfig.setTrustStoreFilePath(keyStorePath);
sslConfig.setTrustStoreFilePassword(LOCAL_PASSWORD);
sslConfig.setSslEnabled(true);
return sslConfig;
}

/**
* This function should be used in test cases only.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ public com.linkedin.venice.security.SSLConfig getSSLConfig() {
config.setKeyStoreType(sslKeyStoreType);
config.setTrustStoreFilePassword(sslTrustStorePassword);
config.setTrustStoreFilePath(sslTrustStoreLocation);
config.setTrustStoreType(sslTrustStoreType);

return config;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public static KeyManager[] getKeyManagers(SSLFactory sslFactory, String algorith

public static TrustManager[] getTrustManagers(SSLFactory sslFactory, String algorithm)
throws CertificateException, KeyStoreException, IOException, NoSuchAlgorithmException {
KeyStore trustStore = loadTrustStore(sslFactory, sslFactory.getSSLConfig().getKeyStoreType());
KeyStore trustStore = loadTrustStore(sslFactory, sslFactory.getSSLConfig().getTrustStoreType());
return createTrustManagers(trustStore, algorithm);
}

Expand Down

0 comments on commit 87b957d

Please sign in to comment.