Don't cache key material if sun.security.ssl.X509KeyManagerImpl is used (#9762)

Motivation:

sun.security.ssl.X509KeyManagerImpl will not use "stable" aliases and so aliases may be changed during invocations. This means caching is useless. Because of this we should disable the cache if its used.

Modifications:

- Disable caching if sun.security.ssl.X509KeyManagerImpl is used
- Add tests

Result:

More protection against https://github.com/netty/netty/issues/9747.
This commit is contained in:
Norman Maurer 2019-11-07 15:12:34 +01:00 committed by GitHub
parent 394e7f0287
commit c35a80d6f7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 32 additions and 2 deletions

View File

@ -21,6 +21,7 @@ import javax.net.ssl.KeyManager;
import javax.net.ssl.KeyManagerFactory;
import javax.net.ssl.KeyManagerFactorySpi;
import javax.net.ssl.ManagerFactoryParameters;
import javax.net.ssl.X509ExtendedKeyManager;
import javax.net.ssl.X509KeyManager;
import java.security.InvalidAlgorithmParameterException;
import java.security.KeyStore;
@ -67,7 +68,13 @@ public final class OpenSslCachingX509KeyManagerFactory extends KeyManagerFactory
this.maxCachedEntries = ObjectUtil.checkPositive(maxCachedEntries, "maxCachedEntries");
}
OpenSslCachingKeyMaterialProvider newProvider(String password) {
OpenSslKeyMaterialProvider newProvider(String password) {
X509KeyManager keyManager = ReferenceCountedOpenSslContext.chooseX509KeyManager(getKeyManagers());
if ("sun.security.ssl.X509KeyManagerImpl".equals(keyManager.getClass().getName())) {
// Don't do caching if X509KeyManagerImpl is used as the returned aliases are not stable and will change
// between invocations.
return new OpenSslKeyMaterialProvider(keyManager, password);
}
return new OpenSslCachingKeyMaterialProvider(
ReferenceCountedOpenSslContext.chooseX509KeyManager(getKeyManagers()), password, maxCachedEntries);
}

View File

@ -16,6 +16,7 @@
package io.netty.handler.ssl;
import io.netty.buffer.UnpooledByteBufAllocator;
import org.hamcrest.CoreMatchers;
import org.junit.Assert;
import org.junit.Test;
@ -67,4 +68,22 @@ public class OpenSslCachingKeyMaterialProviderTest extends OpenSslKeyMaterialPro
assertEquals(0, material.refCnt());
assertEquals(0, material2.refCnt());
}
@Test
public void testCacheForSunX509() throws Exception {
OpenSslCachingX509KeyManagerFactory factory = new OpenSslCachingX509KeyManagerFactory(
super.newKeyManagerFactory("SunX509"));
OpenSslKeyMaterialProvider provider = factory.newProvider(PASSWORD);
assertThat(provider,
CoreMatchers.<OpenSslKeyMaterialProvider>instanceOf(OpenSslCachingKeyMaterialProvider.class));
}
@Test
public void testNotCacheForX509() throws Exception {
OpenSslCachingX509KeyManagerFactory factory = new OpenSslCachingX509KeyManagerFactory(
super.newKeyManagerFactory("PKIX"));
OpenSslKeyMaterialProvider provider = factory.newProvider(PASSWORD);
assertThat(provider, CoreMatchers.not(
CoreMatchers.<OpenSslKeyMaterialProvider>instanceOf(OpenSslCachingKeyMaterialProvider.class)));
}
}

View File

@ -46,12 +46,16 @@ public class OpenSslKeyMaterialProviderTest {
}
protected KeyManagerFactory newKeyManagerFactory() throws Exception {
return newKeyManagerFactory(KeyManagerFactory.getDefaultAlgorithm());
}
protected KeyManagerFactory newKeyManagerFactory(String algorithm) throws Exception {
char[] password = PASSWORD.toCharArray();
final KeyStore keystore = KeyStore.getInstance("PKCS12");
keystore.load(getClass().getResourceAsStream("mutual_auth_server.p12"), password);
KeyManagerFactory kmf =
KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
KeyManagerFactory.getInstance(algorithm);
kmf.init(keystore, password);
return kmf;
}