Prevent NPE when attempting to set client key material with no alias (#8378)

Motivation:

It is possible that a client is unable to locate a certificate alias given the list of issuers and key types. In this case the X509KeyManager
will return a null which when past to the OpenSslKeyMaterialProvider implementation may produce a NPE. If no matching alias could be found we should not
call OpenSslKeyMaterialProvider at all which is also consistent what OpenJDK does.

Modifications:

- Add null check before calling OpenSslKeyMaterialProvider
- Add unit test.

Result:

No more NPE caused by passing null as client alias.
This commit is contained in:
Norman Maurer 2018-10-12 09:27:46 +02:00 committed by GitHub
parent 5b3b8db07f
commit aae7cdca96
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 89 additions and 1 deletions

View File

@ -83,8 +83,13 @@ final class OpenSslKeyMaterialManager {
void setKeyMaterialClientSide(ReferenceCountedOpenSslEngine engine, String[] keyTypes,
X500Principal[] issuer) throws SSLException {
String alias = chooseClientAlias(engine, keyTypes, issuer);
// Only try to set the keymaterial if we have a match. This is also consistent with what OpenJDK does:
// http://hg.openjdk.java.net/jdk/jdk11/file/76072a077ee1/
// src/java.base/share/classes/sun/security/ssl/CertificateRequest.java#l362
if (alias != null) {
setKeyMaterial(engine, alias);
}
}
private void setKeyMaterial(ReferenceCountedOpenSslEngine engine, String alias) throws SSLException {
OpenSslKeyMaterial keyMaterial = null;

View File

@ -0,0 +1,83 @@
/*
* 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 io.netty.buffer.ByteBufAllocator;
import io.netty.buffer.UnpooledByteBufAllocator;
import io.netty.util.internal.EmptyArrays;
import org.junit.Assert;
import org.junit.Assume;
import org.junit.Test;
import javax.net.ssl.SSLException;
import javax.net.ssl.X509ExtendedKeyManager;
import java.net.Socket;
import java.security.Principal;
import java.security.PrivateKey;
import java.security.cert.X509Certificate;
public class OpenSslKeyMaterialManagerTest {
@Test
public void testChooseClientAliasReturnsNull() throws SSLException {
Assume.assumeTrue(OpenSsl.isAvailable());
X509ExtendedKeyManager keyManager = new X509ExtendedKeyManager() {
@Override
public String[] getClientAliases(String s, Principal[] principals) {
return EmptyArrays.EMPTY_STRINGS;
}
@Override
public String chooseClientAlias(String[] strings, Principal[] principals, Socket socket) {
return null;
}
@Override
public String[] getServerAliases(String s, Principal[] principals) {
return EmptyArrays.EMPTY_STRINGS;
}
@Override
public String chooseServerAlias(String s, Principal[] principals, Socket socket) {
return null;
}
@Override
public X509Certificate[] getCertificateChain(String s) {
return EmptyArrays.EMPTY_X509_CERTIFICATES;
}
@Override
public PrivateKey getPrivateKey(String s) {
return null;
}
};
OpenSslKeyMaterialManager manager = new OpenSslKeyMaterialManager(
new OpenSslKeyMaterialProvider(keyManager, null) {
@Override
OpenSslKeyMaterial chooseKeyMaterial(ByteBufAllocator allocator, String alias) throws Exception {
Assert.fail("Should not be called when alias is null");
return null;
}
});
SslContext context = SslContextBuilder.forClient().sslProvider(SslProvider.OPENSSL).build();
OpenSslEngine engine =
(OpenSslEngine) context.newEngine(UnpooledByteBufAllocator.DEFAULT);
manager.setKeyMaterialClientSide(engine, EmptyArrays.EMPTY_STRINGS, null);
}
}