Fix OpenSslCertificateException error code validation

Motivation:

In OpenSslCertificateException we tried to validate the supplied error code but did not correctly account for all different valid error codes and so threw an IllegalArgumentException.

Modifications:

- Fix validation by updating to latest netty-tcnative and use CertificateVerifier.isValid
- Add unit tests

Result:

Validation of error code works as expected.
This commit is contained in:
Norman Maurer 2017-03-29 11:20:31 +02:00 committed by Scott Mitchell
parent 00bf06e97b
commit 4bcfa07a7d
4 changed files with 56 additions and 8 deletions

View File

@ -21,7 +21,7 @@ import java.security.cert.CertificateException;
/** /**
* A special {@link CertificateException} which allows to specify which error code is included in the * A special {@link CertificateException} which allows to specify which error code is included in the
* SSL Record. This only work when {@link SslProvider#OPENSSL} is used. * SSL Record. This only work when {@link SslProvider#OPENSSL} or {@link SslProvider#OPENSSL_REFCNT} is used.
*/ */
public final class OpenSslCertificateException extends CertificateException { public final class OpenSslCertificateException extends CertificateException {
private static final long serialVersionUID = 5542675253797129798L; private static final long serialVersionUID = 5542675253797129798L;
@ -63,17 +63,16 @@ public final class OpenSslCertificateException extends CertificateException {
} }
/** /**
* Return the <a href="https://www.openssl.org/docs/manmaster/apps/verify.html">error code</a> to use. * Return the <a href="https://www.openssl.org/docs/man1.0.2/apps/verify.html">error code</a> to use.
*/ */
public int errorCode() { public int errorCode() {
return errorCode; return errorCode;
} }
private static int checkErrorCode(int errorCode) { private static int checkErrorCode(int errorCode) {
if (errorCode < CertificateVerifier.X509_V_OK || errorCode > CertificateVerifier.X509_V_ERR_DANE_NO_MATCH) { if (!CertificateVerifier.isValid(errorCode)) {
throw new IllegalArgumentException("errorCode must be " + CertificateVerifier.X509_V_OK + " => " throw new IllegalArgumentException("errorCode '" + errorCode +
+ CertificateVerifier.X509_V_ERR_DANE_NO_MATCH + "' invalid, see https://www.openssl.org/docs/man1.0.2/apps/verify.html.");
". See https://www.openssl.org/docs/manmaster/apps/verify.html .");
} }
return errorCode; return errorCode;
} }

View File

@ -615,7 +615,7 @@ public abstract class ReferenceCountedOpenSslContext extends SslContext implemen
return refCnt.release(decrement); return refCnt.release(decrement);
} }
abstract static class AbstractCertificateVerifier implements CertificateVerifier { abstract static class AbstractCertificateVerifier extends CertificateVerifier {
private final OpenSslEngineMap engineMap; private final OpenSslEngineMap engineMap;
AbstractCertificateVerifier(OpenSslEngineMap engineMap) { AbstractCertificateVerifier(OpenSslEngineMap engineMap) {

View File

@ -0,0 +1,49 @@
/*
* Copyright 2017 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.internal.tcnative.CertificateVerifier;
import org.junit.Assert;
import org.junit.Assume;
import org.junit.BeforeClass;
import org.junit.Test;
import java.lang.reflect.Field;
public class OpenSslCertificateExceptionTest {
@BeforeClass
public static void assumeOpenSsl() {
Assume.assumeTrue(OpenSsl.isAvailable());
}
@Test
public void testValidErrorCode() throws Exception {
Field[] fields = CertificateVerifier.class.getFields();
for (Field field : fields) {
if (field.isAccessible()) {
int errorCode = field.getInt(null);
OpenSslCertificateException exception = new OpenSslCertificateException(errorCode);
Assert.assertEquals(errorCode, exception.errorCode());
}
}
}
@Test(expected = IllegalArgumentException.class)
public void testNonValidErrorCode() {
new OpenSslCertificateException(Integer.MIN_VALUE);
}
}

View File

@ -242,7 +242,7 @@
<!-- Fedora-"like" systems. This is currently only used for the netty-tcnative dependency --> <!-- Fedora-"like" systems. This is currently only used for the netty-tcnative dependency -->
<os.detection.classifierWithLikes>fedora</os.detection.classifierWithLikes> <os.detection.classifierWithLikes>fedora</os.detection.classifierWithLikes>
<tcnative.artifactId>netty-tcnative</tcnative.artifactId> <tcnative.artifactId>netty-tcnative</tcnative.artifactId>
<tcnative.version>2.0.0.Final</tcnative.version> <tcnative.version>2.0.1.Final-SNAPSHOT</tcnative.version>
<tcnative.classifier>${os.detected.classifier}</tcnative.classifier> <tcnative.classifier>${os.detected.classifier}</tcnative.classifier>
<conscrypt.groupId>org.conscrypt</conscrypt.groupId> <conscrypt.groupId>org.conscrypt</conscrypt.groupId>
<conscrypt.artifactId>conscrypt-openjdk-uber</conscrypt.artifactId> <conscrypt.artifactId>conscrypt-openjdk-uber</conscrypt.artifactId>