Skip to content

Commit faaa15c

Browse files
Zgoda91Zgoda91
committed
Changed polling mechanism for certificate files
1 parent c36d730 commit faaa15c

File tree

9 files changed

+101
-218
lines changed

9 files changed

+101
-218
lines changed

‎api/src/main/java/io/grpc/TlsChannelCredentials.java

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import java.util.Collections;
2727
import java.util.EnumSet;
2828
import java.util.List;
29-
import java.util.Map;
3029
import java.util.Set;
3130
import javax.net.ssl.KeyManager;
3231
import javax.net.ssl.TrustManager;
@@ -50,7 +49,6 @@ public static ChannelCredentials create() {
5049
private final List<KeyManager> keyManagers;
5150
private final byte[] rootCertificates;
5251
private final List<TrustManager> trustManagers;
53-
private final Map<String, ?> customCertificatesConfig;
5452

5553
TlsChannelCredentials(Builder builder) {
5654
fakeFeature = builder.fakeFeature;
@@ -60,7 +58,6 @@ public static ChannelCredentials create() {
6058
keyManagers = builder.keyManagers;
6159
rootCertificates = builder.rootCertificates;
6260
trustManagers = builder.trustManagers;
63-
customCertificatesConfig = builder.customCertificatesConfig;
6461
}
6562

6663
/**
@@ -124,21 +121,6 @@ public List<TrustManager> getTrustManagers() {
124121
return trustManagers;
125122
}
126123

127-
/**
128-
* Returns custom certificates config. It contains following entries:
129-
*
130-
* <ul>
131-
* <li>{@code "ca_certificate_file"} key containing the path to the root certificate file</li>
132-
* <li>{@code "certificate_file"} key containing the path to the identity certificate file</li>
133-
* <li>{@code "private_key_file"} key containing the path to the private key</li>
134-
* <li>{@code "refresh_interval"} key specifying the frequency of updates to the above
135-
* files</li>
136-
* </ul>
137-
*/
138-
public Map<String, ?> getCustomCertificatesConfig() {
139-
return customCertificatesConfig;
140-
}
141-
142124
/**
143125
* Returns an empty set if this credential can be adequately understood via
144126
* the features listed, otherwise returns a hint of features that are lacking
@@ -246,7 +228,6 @@ public static final class Builder {
246228
private List<KeyManager> keyManagers;
247229
private byte[] rootCertificates;
248230
private List<TrustManager> trustManagers;
249-
private Map<String, ?> customCertificatesConfig;
250231

251232
private Builder() {}
252233

@@ -374,11 +355,6 @@ public Builder trustManager(TrustManager... trustManagers) {
374355
return this;
375356
}
376357

377-
public Builder customCertificatesConfig(Map<String, ?> customCertificatesConfig) {
378-
this.customCertificatesConfig = customCertificatesConfig;
379-
return this;
380-
}
381-
382358
private void clearTrustManagers() {
383359
this.rootCertificates = null;
384360
this.trustManagers = null;

‎xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import com.google.common.collect.ImmutableMap;
2222
import io.grpc.Internal;
2323
import io.grpc.InternalLogId;
24-
import io.grpc.TlsChannelCredentials;
2524
import io.grpc.internal.GrpcUtil;
2625
import io.grpc.internal.GrpcUtil.GrpcBuildVersion;
2726
import io.grpc.internal.JsonParser;
@@ -163,9 +162,9 @@ protected BootstrapInfo.Builder bootstrapBuilder(Map<String, ?> rawData)
163162
builder.node(nodeBuilder.build());
164163

165164
Map<String, ?> certProvidersBlob = JsonUtil.getObject(rawData, "certificate_providers");
166-
Map<String, CertificateProviderInfo> certProviders = new HashMap<>();
167165
if (certProvidersBlob != null) {
168166
logger.log(XdsLogLevel.INFO, "Configured with {0} cert providers", certProvidersBlob.size());
167+
Map<String, CertificateProviderInfo> certProviders = new HashMap<>(certProvidersBlob.size());
169168
for (String name : certProvidersBlob.keySet()) {
170169
Map<String, ?> valueMap = JsonUtil.getObject(certProvidersBlob, name);
171170
String pluginName =
@@ -176,21 +175,6 @@ protected BootstrapInfo.Builder bootstrapBuilder(Map<String, ?> rawData)
176175
CertificateProviderInfo.create(pluginName, config);
177176
certProviders.put(name, certificateProviderInfo);
178177
}
179-
}
180-
181-
for (ServerInfo serverInfo : servers) {
182-
Object creds = serverInfo.implSpecificConfig();
183-
if (creds instanceof TlsChannelCredentials) {
184-
Map<String, ?> config = ((TlsChannelCredentials)creds).getCustomCertificatesConfig();
185-
if (config != null) {
186-
CertificateProviderInfo certificateProviderInfo =
187-
CertificateProviderInfo.create("file_watcher", config);
188-
certProviders.put("mtls_channel_creds_identity_certs", certificateProviderInfo);
189-
}
190-
}
191-
}
192-
193-
if (!certProviders.isEmpty()) {
194178
builder.certProviders(certProviders);
195179
}
196180

‎xds/src/main/java/io/grpc/xds/internal/TlsXdsCredentialsProvider.java

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,10 @@
1919
import io.grpc.ChannelCredentials;
2020
import io.grpc.TlsChannelCredentials;
2121
import io.grpc.internal.JsonUtil;
22+
import io.grpc.util.AdvancedTlsX509KeyManager;
23+
import io.grpc.util.AdvancedTlsX509TrustManager;
2224
import io.grpc.xds.XdsCredentialsProvider;
2325
import java.io.File;
24-
import java.io.IOException;
2526
import java.util.Map;
2627
import java.util.logging.Level;
2728
import java.util.logging.Logger;
@@ -33,6 +34,9 @@
3334
public final class TlsXdsCredentialsProvider extends XdsCredentialsProvider {
3435
private static final Logger logger = Logger.getLogger(TlsXdsCredentialsProvider.class.getName());
3536
private static final String CREDS_NAME = "tls";
37+
private static final String CERT_FILE_KEY = "certificate_file";
38+
private static final String KEY_FILE_KEY = "private_key_file";
39+
private static final String ROOT_FILE_KEY = "ca_certificate_file";
3640

3741
@Override
3842
protected ChannelCredentials newChannelCredentials(Map<String, ?> jsonConfig) {
@@ -43,24 +47,28 @@ protected ChannelCredentials newChannelCredentials(Map<String, ?> jsonConfig) {
4347
}
4448

4549
// use trust certificate file path from bootstrap config if provided; else use system default
46-
String rootCertPath = JsonUtil.getString(jsonConfig, "ca_certificate_file");
50+
String rootCertPath = JsonUtil.getString(jsonConfig, ROOT_FILE_KEY);
4751
if (rootCertPath != null) {
4852
try {
49-
builder.trustManager(new File(rootCertPath));
50-
} catch (IOException e) {
53+
AdvancedTlsX509TrustManager trustManager = AdvancedTlsX509TrustManager.newBuilder().build();
54+
trustManager.updateTrustCredentials(new File(rootCertPath));
55+
builder.trustManager(trustManager);
56+
} catch (Exception e) {
5157
logger.log(Level.WARNING, "Unable to read root certificates", e);
5258
return null;
5359
}
5460
}
5561

5662
// use certificate chain and private key file paths from bootstrap config if provided. Mind that
5763
// both JSON values must be either set (mTLS case) or both unset (TLS case)
58-
String certChainPath = JsonUtil.getString(jsonConfig, "certificate_file");
59-
String privateKeyPath = JsonUtil.getString(jsonConfig, "private_key_file");
64+
String certChainPath = JsonUtil.getString(jsonConfig, CERT_FILE_KEY);
65+
String privateKeyPath = JsonUtil.getString(jsonConfig, KEY_FILE_KEY);
6066
if (certChainPath != null && privateKeyPath != null) {
6167
try {
62-
builder.keyManager(new File(certChainPath), new File(privateKeyPath));
63-
} catch (IOException e) {
68+
AdvancedTlsX509KeyManager keyManager = new AdvancedTlsX509KeyManager();
69+
keyManager.updateIdentityCredentials(new File(certChainPath), new File(privateKeyPath));
70+
builder.keyManager(keyManager);
71+
} catch (Exception e) {
6472
logger.log(Level.WARNING, "Unable to read certificate chain or private key", e);
6573
return null;
6674
}
@@ -69,11 +77,6 @@ protected ChannelCredentials newChannelCredentials(Map<String, ?> jsonConfig) {
6977
return null;
7078
}
7179

72-
// save json config when custom certificate paths were provided in a bootstrap
73-
if (rootCertPath != null || certChainPath != null) {
74-
builder.customCertificatesConfig(jsonConfig);
75-
}
76-
7780
return builder.build();
7881
}
7982

‎xds/src/main/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProvider.java

Lines changed: 20 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -73,12 +73,10 @@ final class FileWatcherCertificateProvider extends CertificateProvider implement
7373
this.scheduledExecutorService =
7474
checkNotNull(scheduledExecutorService, "scheduledExecutorService");
7575
this.timeProvider = checkNotNull(timeProvider, "timeProvider");
76-
checkArgument(((certFile != null) == (keyFile != null)),
77-
"certFile and keyFile must be both set or both unset");
78-
this.certFile = certFile == null ? null : Paths.get(certFile);
79-
this.keyFile = keyFile == null ? null : Paths.get(keyFile);
80-
checkArgument((trustFile != null || spiffeTrustMapFile != null || keyFile != null),
81-
"must be watching either root or identity certificates");
76+
this.certFile = Paths.get(checkNotNull(certFile, "certFile"));
77+
this.keyFile = Paths.get(checkNotNull(keyFile, "keyFile"));
78+
checkArgument((trustFile != null || spiffeTrustMapFile != null),
79+
"either trustFile or spiffeTrustMapFile must be present");
8280
if (spiffeTrustMapFile != null) {
8381
this.spiffeTrustMapFile = Paths.get(spiffeTrustMapFile);
8482
this.trustFile = null;
@@ -115,26 +113,23 @@ private synchronized void scheduleNextRefreshCertificate(long delayInSeconds) {
115113
void checkAndReloadCertificates() {
116114
try {
117115
try {
118-
if (certFile != null && keyFile != null) {
119-
FileTime currentCertTime = Files.getLastModifiedTime(certFile);
120-
FileTime currentKeyTime = Files.getLastModifiedTime(keyFile);
121-
if (!currentCertTime.equals(lastModifiedTimeCert)
122-
&& !currentKeyTime.equals(lastModifiedTimeKey)) {
123-
byte[] certFileContents = Files.readAllBytes(certFile);
124-
byte[] keyFileContents = Files.readAllBytes(keyFile);
125-
FileTime currentCertTime2 = Files.getLastModifiedTime(certFile);
126-
FileTime currentKeyTime2 = Files.getLastModifiedTime(keyFile);
127-
if (currentCertTime2.equals(currentCertTime)
128-
&& currentKeyTime2.equals(currentKeyTime)) {
129-
try (ByteArrayInputStream certStream = new ByteArrayInputStream(certFileContents);
130-
ByteArrayInputStream keyStream = new ByteArrayInputStream(keyFileContents)) {
131-
PrivateKey privateKey = CertificateUtils.getPrivateKey(keyStream);
132-
X509Certificate[] certs = CertificateUtils.toX509Certificates(certStream);
133-
getWatcher().updateCertificate(privateKey, Arrays.asList(certs));
134-
}
135-
lastModifiedTimeCert = currentCertTime;
136-
lastModifiedTimeKey = currentKeyTime;
116+
FileTime currentCertTime = Files.getLastModifiedTime(certFile);
117+
FileTime currentKeyTime = Files.getLastModifiedTime(keyFile);
118+
if (!currentCertTime.equals(lastModifiedTimeCert)
119+
&& !currentKeyTime.equals(lastModifiedTimeKey)) {
120+
byte[] certFileContents = Files.readAllBytes(certFile);
121+
byte[] keyFileContents = Files.readAllBytes(keyFile);
122+
FileTime currentCertTime2 = Files.getLastModifiedTime(certFile);
123+
FileTime currentKeyTime2 = Files.getLastModifiedTime(keyFile);
124+
if (currentCertTime2.equals(currentCertTime) && currentKeyTime2.equals(currentKeyTime)) {
125+
try (ByteArrayInputStream certStream = new ByteArrayInputStream(certFileContents);
126+
ByteArrayInputStream keyStream = new ByteArrayInputStream(keyFileContents)) {
127+
PrivateKey privateKey = CertificateUtils.getPrivateKey(keyStream);
128+
X509Certificate[] certs = CertificateUtils.toX509Certificates(certStream);
129+
getWatcher().updateCertificate(privateKey, Arrays.asList(certs));
137130
}
131+
lastModifiedTimeCert = currentCertTime;
132+
lastModifiedTimeKey = currentKeyTime;
138133
}
139134
}
140135
} catch (Throwable t) {

‎xds/src/main/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProviderProvider.java

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package io.grpc.xds.internal.security.certprovider;
1818

1919
import static com.google.common.base.Preconditions.checkArgument;
20+
import static com.google.common.base.Preconditions.checkNotNull;
2021

2122
import com.google.common.annotations.VisibleForTesting;
2223
import com.google.common.util.concurrent.ThreadFactoryBuilder;
@@ -93,27 +94,30 @@ public CertificateProvider createCertificateProvider(
9394
timeProvider);
9495
}
9596

97+
private static String checkForNullAndGet(Map<String, ?> map, String key) {
98+
return checkNotNull(JsonUtil.getString(map, key), "'" + key + "' is required in the config");
99+
}
100+
96101
private static Config validateAndTranslateConfig(Object config) {
97102
checkArgument(config instanceof Map, "Only Map supported for config");
98103
@SuppressWarnings("unchecked") Map<String, ?> map = (Map<String, ?>)config;
99104

100105
Config configObj = new Config();
101-
configObj.certFile = JsonUtil.getString(map, CERT_FILE_KEY);
102-
configObj.keyFile = JsonUtil.getString(map, KEY_FILE_KEY);
103-
if ((configObj.certFile != null) != (configObj.keyFile != null)) {
104-
throw new NullPointerException(
105-
String.format("'%s' and '%s' must be both set or both unset",
106-
CERT_FILE_KEY, KEY_FILE_KEY));
107-
}
108-
if (!map.containsKey(ROOT_FILE_KEY)
109-
&& !map.containsKey(CERT_FILE_KEY)
110-
&& (!enableSpiffe || !map.containsKey(SPIFFE_TRUST_MAP_FILE_KEY))) {
111-
throw new NullPointerException("must be watching either root or identity certificates");
112-
}
113-
if (enableSpiffe && map.containsKey(SPIFFE_TRUST_MAP_FILE_KEY)) {
114-
configObj.spiffeTrustMapFile = JsonUtil.getString(map, SPIFFE_TRUST_MAP_FILE_KEY);
106+
configObj.certFile = checkForNullAndGet(map, CERT_FILE_KEY);
107+
configObj.keyFile = checkForNullAndGet(map, KEY_FILE_KEY);
108+
if (enableSpiffe) {
109+
if (!map.containsKey(ROOT_FILE_KEY) && !map.containsKey(SPIFFE_TRUST_MAP_FILE_KEY)) {
110+
throw new NullPointerException(
111+
String.format("either '%s' or '%s' is required in the config",
112+
ROOT_FILE_KEY, SPIFFE_TRUST_MAP_FILE_KEY));
113+
}
114+
if (map.containsKey(SPIFFE_TRUST_MAP_FILE_KEY)) {
115+
configObj.spiffeTrustMapFile = JsonUtil.getString(map, SPIFFE_TRUST_MAP_FILE_KEY);
116+
} else {
117+
configObj.rootFile = JsonUtil.getString(map, ROOT_FILE_KEY);
118+
}
115119
} else {
116-
configObj.rootFile = JsonUtil.getString(map, ROOT_FILE_KEY);
120+
configObj.rootFile = checkForNullAndGet(map, ROOT_FILE_KEY);
117121
}
118122
String refreshIntervalString = JsonUtil.getString(map, REFRESH_INTERVAL_KEY);
119123
if (refreshIntervalString != null) {

‎xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,11 @@
3232
import io.grpc.internal.GrpcUtil;
3333
import io.grpc.internal.GrpcUtil.GrpcBuildVersion;
3434
import io.grpc.internal.testing.TestUtils;
35+
import io.grpc.util.AdvancedTlsX509KeyManager;
36+
import io.grpc.util.AdvancedTlsX509TrustManager;
3537
import io.grpc.xds.client.Bootstrapper;
3638
import io.grpc.xds.client.Bootstrapper.AuthorityInfo;
3739
import io.grpc.xds.client.Bootstrapper.BootstrapInfo;
38-
import io.grpc.xds.client.Bootstrapper.CertificateProviderInfo;
3940
import io.grpc.xds.client.Bootstrapper.ServerInfo;
4041
import io.grpc.xds.client.BootstrapperImpl;
4142
import io.grpc.xds.client.CommonBootstrapperTestUtils;
@@ -45,6 +46,8 @@
4546
import java.io.IOException;
4647
import java.util.List;
4748
import java.util.Map;
49+
import javax.net.ssl.KeyManager;
50+
import javax.net.ssl.TrustManager;
4851
import org.junit.After;
4952
import org.junit.Assert;
5053
import org.junit.Before;
@@ -934,21 +937,16 @@ public void parseTlsChannelCredentialsWithCustomCertificatesConfig()
934937
ServerInfo serverInfo = Iterables.getOnlyElement(info.servers());
935938
assertThat(serverInfo.target()).isEqualTo(SERVER_URI);
936939
assertThat(serverInfo.implSpecificConfig()).isInstanceOf(TlsChannelCredentials.class);
937-
assertThat(info.node()).isEqualTo(getNodeBuilder().build());
938940

939-
assertThat(info.certProviders()).hasSize(1);
940-
ImmutableMap<String, CertificateProviderInfo> certProviderInfo = info.certProviders();
941-
assertThat(certProviderInfo.keySet()).containsExactly("mtls_channel_creds_identity_certs");
942-
CertificateProviderInfo mtlsChannelCredCertProviderInfo =
943-
certProviderInfo.get("mtls_channel_creds_identity_certs");
944-
assertThat(mtlsChannelCredCertProviderInfo.config().keySet())
945-
.containsExactly("ca_certificate_file", "certificate_file", "private_key_file");
946-
assertThat(mtlsChannelCredCertProviderInfo.config().get("ca_certificate_file"))
947-
.isEqualTo(rootCertPath);
948-
assertThat(mtlsChannelCredCertProviderInfo.config().get("certificate_file"))
949-
.isEqualTo(certChainPath);
950-
assertThat(mtlsChannelCredCertProviderInfo.config().get("private_key_file"))
951-
.isEqualTo(privateKeyPath);
941+
TlsChannelCredentials tlsChannelCredentials =
942+
(TlsChannelCredentials) serverInfo.implSpecificConfig();
943+
assertThat(tlsChannelCredentials.getKeyManagers()).hasSize(1);
944+
KeyManager keyManager = Iterables.getOnlyElement(tlsChannelCredentials.getKeyManagers());
945+
assertThat(keyManager).isInstanceOf(AdvancedTlsX509KeyManager.class);
946+
assertThat(tlsChannelCredentials.getTrustManagers()).hasSize(1);
947+
TrustManager trustManager = Iterables.getOnlyElement(tlsChannelCredentials.getTrustManagers());
948+
assertThat(trustManager).isInstanceOf(AdvancedTlsX509TrustManager.class);
949+
952950
}
953951

954952
private static BootstrapperImpl.FileReader createFileReader(

0 commit comments

Comments
 (0)