Skip to content

Commit

Permalink
Fix possible NPE introduced by a137291 when using SslProvider.OPENSSL…
Browse files Browse the repository at this point in the history
… and init via files or OpenSslX509KeyManagerFactory (netty#8126)

Motivation:

a137291 introduced a way to get the most speed out of OpenSSL by not only caching keymaterial but pre-compute these. The problem was we missed to check for null before doing an instanceof check and then a cast which could lead to a NPE as we tried to cast null to Exception and throw it.

Modifications:

Add null check and unit test.

Result:

No more NPE when keymaterial was not found for requested alias.
  • Loading branch information
normanmaurer authored Jul 11, 2018
1 parent 8186c9a commit df08467
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -198,11 +198,14 @@ private static final class OpenSslPopulatedKeyMaterialProvider extends OpenSslKe
@Override
OpenSslKeyMaterial chooseKeyMaterial(ByteBufAllocator allocator, String alias) throws Exception {
Object value = materialMap.get(alias);
if (value == null) {
// There is no keymaterial for the requested alias, return null
return null;
}
if (value instanceof OpenSslKeyMaterial) {
return ((OpenSslKeyMaterial) value).retain();
} else {
throw (Exception) value;
}
throw (Exception) value;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import org.junit.Test;

import javax.net.ssl.KeyManagerFactory;
import javax.net.ssl.X509KeyManager;

import static org.junit.Assert.*;

Expand All @@ -32,25 +31,19 @@ protected KeyManagerFactory newKeyManagerFactory() throws Exception {
}

@Override
protected OpenSslKeyMaterialProvider newMaterialProvider(X509KeyManager manager, String password) {
return new OpenSslCachingKeyMaterialProvider(manager, password);
protected OpenSslKeyMaterialProvider newMaterialProvider(KeyManagerFactory factory, String password) {
return new OpenSslCachingKeyMaterialProvider(ReferenceCountedOpenSslContext.chooseX509KeyManager(
factory.getKeyManagers()), password);
}

@Override
protected void assertRelease(OpenSslKeyMaterial material) {
Assert.assertFalse(material.release());
}

@Override
protected Class<? extends OpenSslKeyMaterialProvider> providerClass() {
return OpenSslCachingKeyMaterialProvider.class;
}

@Test
public void testMaterialCached() throws Exception {
X509KeyManager manager = ReferenceCountedOpenSslContext.chooseX509KeyManager(
newKeyManagerFactory().getKeyManagers());
OpenSslKeyMaterialProvider provider = newMaterialProvider(manager, PASSWORD);
OpenSslKeyMaterialProvider provider = newMaterialProvider(newKeyManagerFactory(), PASSWORD);

OpenSslKeyMaterial material = provider.chooseKeyMaterial(UnpooledByteBufAllocator.DEFAULT, EXISTING_ALIAS);
assertNotNull(material);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import org.junit.Test;

import javax.net.ssl.KeyManagerFactory;
import javax.net.ssl.X509KeyManager;

import java.security.KeyStore;

Expand Down Expand Up @@ -49,12 +48,9 @@ protected KeyManagerFactory newKeyManagerFactory() throws Exception {
return kmf;
}

protected OpenSslKeyMaterialProvider newMaterialProvider(X509KeyManager manager, String password) {
return new OpenSslKeyMaterialProvider(manager, password);
}

protected Class<? extends OpenSslKeyMaterialProvider> providerClass() {
return OpenSslKeyMaterialProvider.class;
protected OpenSslKeyMaterialProvider newMaterialProvider(KeyManagerFactory factory, String password) {
return new OpenSslKeyMaterialProvider(ReferenceCountedOpenSslContext.chooseX509KeyManager(
factory.getKeyManagers()), password);
}

protected void assertRelease(OpenSslKeyMaterial material) {
Expand All @@ -63,9 +59,7 @@ protected void assertRelease(OpenSslKeyMaterial material) {

@Test
public void testChooseKeyMaterial() throws Exception {
X509KeyManager manager = ReferenceCountedOpenSslContext.chooseX509KeyManager(
newKeyManagerFactory().getKeyManagers());
OpenSslKeyMaterialProvider provider = newMaterialProvider(manager, PASSWORD);
OpenSslKeyMaterialProvider provider = newMaterialProvider(newKeyManagerFactory(), PASSWORD);
OpenSslKeyMaterial nonExistingMaterial = provider.chooseKeyMaterial(
UnpooledByteBufAllocator.DEFAULT, NON_EXISTING_ALIAS);
assertNull(nonExistingMaterial);
Expand All @@ -78,12 +72,4 @@ public void testChooseKeyMaterial() throws Exception {

provider.destroy();
}

@Test
public void testChooseTheCorrectProvider() throws Exception {
OpenSslKeyMaterialProvider provider = ReferenceCountedOpenSslContext.providerFor(
newKeyManagerFactory(), PASSWORD);
assertEquals(providerClass(), provider.getClass());
provider.destroy();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Copyright 2018 The Netty Project
*
* The Netty Project licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at:
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/
package io.netty.handler.ssl;

import javax.net.ssl.KeyManagerFactory;
import java.security.KeyStore;

public class OpenSslX509KeyManagerFactoryProviderTest extends OpenSslCachingKeyMaterialProviderTest {

@Override
protected KeyManagerFactory newKeyManagerFactory() throws Exception {
char[] password = PASSWORD.toCharArray();
final KeyStore keystore = KeyStore.getInstance("PKCS12");
keystore.load(getClass().getResourceAsStream("mutual_auth_server.p12"), password);

OpenSslX509KeyManagerFactory kmf = new OpenSslX509KeyManagerFactory();
kmf.init(keystore, password);
return kmf;
}

@Override
protected OpenSslKeyMaterialProvider newMaterialProvider(KeyManagerFactory kmf, String password) {
return ((OpenSslX509KeyManagerFactory) kmf).newProvider();
}
}

0 comments on commit df08467

Please sign in to comment.