fix a flaky test in advanced TLS (#8474)

* fix a flaky test in advanced tls
This commit is contained in:
ZhenLian 2021-09-08 11:43:23 -07:00 committed by GitHub
parent 1f1396f3f0
commit fb00463001
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 47 additions and 20 deletions

View File

@ -23,12 +23,11 @@ import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.net.Socket;
import java.security.NoSuchAlgorithmException;
import java.security.GeneralSecurityException;
import java.security.Principal;
import java.security.PrivateKey;
import java.security.cert.CertificateException;
import java.security.cert.X509Certificate;
import java.security.spec.InvalidKeySpecException;
import java.util.Arrays;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
@ -107,8 +106,7 @@ public final class AdvancedTlsX509KeyManager extends X509ExtendedKeyManager {
* @param key the private key that is going to be used
* @param certs the certificate chain that is going to be used
*/
public void updateIdentityCredentials(PrivateKey key, X509Certificate[] certs)
throws CertificateException {
public void updateIdentityCredentials(PrivateKey key, X509Certificate[] certs) {
// TODO(ZhenLian): explore possibilities to do a crypto check here.
this.keyInfo = new KeyInfo(checkNotNull(key, "key"), checkNotNull(certs, "certs"));
}
@ -126,10 +124,16 @@ public final class AdvancedTlsX509KeyManager extends X509ExtendedKeyManager {
* @return an object that caller should close when the file refreshes are not needed
*/
public Closeable updateIdentityCredentialsFromFile(File keyFile, File certFile,
long period, TimeUnit unit, ScheduledExecutorService executor) {
long period, TimeUnit unit, ScheduledExecutorService executor) throws IOException,
GeneralSecurityException {
UpdateResult newResult = readAndUpdate(keyFile, certFile, 0, 0);
if (!newResult.success) {
throw new GeneralSecurityException(
"Files were unmodified before their initial update. Probably a bug.");
}
final ScheduledFuture<?> future =
executor.scheduleWithFixedDelay(
new LoadFilePathExecution(keyFile, certFile), 0, period, unit);
new LoadFilePathExecution(keyFile, certFile), period, period, unit);
return new Closeable() {
@Override public void close() {
future.cancel(false);
@ -170,8 +174,7 @@ public final class AdvancedTlsX509KeyManager extends X509ExtendedKeyManager {
this.currentKeyTime = newResult.keyTime;
this.currentCertTime = newResult.certTime;
}
} catch (CertificateException | IOException | NoSuchAlgorithmException
| InvalidKeySpecException e) {
} catch (IOException | GeneralSecurityException e) {
log.log(Level.SEVERE, "Failed refreshing private key and certificate chain from files. "
+ "Using previous ones", e);
}
@ -201,7 +204,7 @@ public final class AdvancedTlsX509KeyManager extends X509ExtendedKeyManager {
* @return the result of this update execution
*/
private UpdateResult readAndUpdate(File keyFile, File certFile, long oldKeyTime, long oldCertTime)
throws IOException, CertificateException, NoSuchAlgorithmException, InvalidKeySpecException {
throws IOException, GeneralSecurityException {
long newKeyTime = keyFile.lastModified();
long newCertTime = certFile.lastModified();
// We only update when both the key and the certs are updated.

View File

@ -21,6 +21,7 @@ import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.net.Socket;
import java.security.GeneralSecurityException;
import java.security.KeyStore;
import java.security.KeyStoreException;
import java.security.NoSuchAlgorithmException;
@ -124,8 +125,8 @@ public final class AdvancedTlsX509TrustManager extends X509ExtendedTrustManager
*
* @param trustCerts the trust certificates that are going to be used
*/
public void updateTrustCredentials(X509Certificate[] trustCerts) throws CertificateException,
KeyStoreException, NoSuchAlgorithmException, IOException {
public void updateTrustCredentials(X509Certificate[] trustCerts) throws IOException,
GeneralSecurityException {
KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType());
keyStore.load(null, null);
int i = 1;
@ -219,10 +220,15 @@ public final class AdvancedTlsX509TrustManager extends X509ExtendedTrustManager
* @return an object that caller should close when the file refreshes are not needed
*/
public Closeable updateTrustCredentialsFromFile(File trustCertFile, long period, TimeUnit unit,
ScheduledExecutorService executor) {
ScheduledExecutorService executor) throws IOException, GeneralSecurityException {
long updatedTime = readAndUpdate(trustCertFile, 0);
if (updatedTime == 0) {
throw new GeneralSecurityException(
"Files were unmodified before their initial update. Probably a bug.");
}
final ScheduledFuture<?> future =
executor.scheduleWithFixedDelay(
new LoadFilePathExecution(trustCertFile), 0, period, unit);
new LoadFilePathExecution(trustCertFile), period, period, unit);
return new Closeable() {
@Override public void close() {
future.cancel(false);
@ -243,8 +249,7 @@ public final class AdvancedTlsX509TrustManager extends X509ExtendedTrustManager
public void run() {
try {
this.currentTime = readAndUpdate(this.file, this.currentTime);
} catch (CertificateException | IOException | KeyStoreException
| NoSuchAlgorithmException e) {
} catch (IOException | GeneralSecurityException e) {
log.log(Level.SEVERE, "Failed refreshing trust CAs from file. Using previous CAs", e);
}
}
@ -259,7 +264,7 @@ public final class AdvancedTlsX509TrustManager extends X509ExtendedTrustManager
* @return oldTime if failed or the modified time is not changed, otherwise the new modified time
*/
private long readAndUpdate(File trustCertFile, long oldTime)
throws CertificateException, IOException, KeyStoreException, NoSuchAlgorithmException {
throws IOException, GeneralSecurityException {
long newTime = trustCertFile.lastModified();
if (newTime == oldTime) {
return oldTime;

View File

@ -45,6 +45,7 @@ import java.io.Closeable;
import java.io.File;
import java.io.IOException;
import java.net.Socket;
import java.security.GeneralSecurityException;
import java.security.NoSuchAlgorithmException;
import java.security.PrivateKey;
import java.security.cert.CertificateException;
@ -169,7 +170,6 @@ public class AdvancedTlsTest {
.clientAuth(ClientAuth.REQUIRE).build();
server = Grpc.newServerBuilderForPort(0, serverCredentials).addService(
new SimpleServiceImpl()).build().start();
TimeUnit.SECONDS.sleep(5);
// Create a client with the key manager and trust manager.
AdvancedTlsX509KeyManager clientKeyManager = new AdvancedTlsX509KeyManager();
clientKeyManager.updateIdentityCredentials(clientKey0, clientCert0);
@ -232,7 +232,6 @@ public class AdvancedTlsTest {
.clientAuth(ClientAuth.REQUIRE).build();
server = Grpc.newServerBuilderForPort(0, serverCredentials).addService(
new SimpleServiceImpl()).build().start();
TimeUnit.SECONDS.sleep(5);
AdvancedTlsX509KeyManager clientKeyManager = new AdvancedTlsX509KeyManager();
clientKeyManager.updateIdentityCredentials(clientKey0, clientCert0);
@ -307,7 +306,6 @@ public class AdvancedTlsTest {
.clientAuth(ClientAuth.REQUIRE).build();
server = Grpc.newServerBuilderForPort(0, serverCredentials).addService(
new SimpleServiceImpl()).build().start();
TimeUnit.SECONDS.sleep(5);
AdvancedTlsX509KeyManager clientKeyManager = new AdvancedTlsX509KeyManager();
clientKeyManager.updateIdentityCredentials(clientKey0, clientCert0);
@ -359,7 +357,6 @@ public class AdvancedTlsTest {
.clientAuth(ClientAuth.REQUIRE).build();
server = Grpc.newServerBuilderForPort(0, serverCredentials).addService(
new SimpleServiceImpl()).build().start();
TimeUnit.SECONDS.sleep(5);
// Create a client to connect.
AdvancedTlsX509KeyManager clientKeyManager = new AdvancedTlsX509KeyManager();
Closeable clientKeyShutdown = clientKeyManager.updateIdentityCredentialsFromFile(clientKey0File,
@ -391,6 +388,28 @@ public class AdvancedTlsTest {
clientTrustShutdown.close();
}
@Test
public void onFileReloadingKeyManagerBadInitialContentTest() throws Exception {
exceptionRule.expect(GeneralSecurityException.class);
AdvancedTlsX509KeyManager keyManager = new AdvancedTlsX509KeyManager();
// We swap the order of key and certificates to intentionally create an exception.
Closeable keyShutdown = keyManager.updateIdentityCredentialsFromFile(serverCert0File,
serverKey0File, 100, TimeUnit.MILLISECONDS, executor);
keyShutdown.close();
}
@Test
public void onFileReloadingTrustManagerBadInitialContentTest() throws Exception {
exceptionRule.expect(GeneralSecurityException.class);
AdvancedTlsX509TrustManager trustManager = AdvancedTlsX509TrustManager.newBuilder()
.setVerification(Verification.CERTIFICATE_ONLY_VERIFICATION)
.build();
// We pass in a key as the trust certificates to intentionally create an exception.
Closeable trustShutdown = trustManager.updateTrustCredentialsFromFile(serverKey0File,
100, TimeUnit.MILLISECONDS, executor);
trustShutdown.close();
}
@Test
public void keyManagerAliasesTest() throws Exception {
AdvancedTlsX509KeyManager km = new AdvancedTlsX509KeyManager();