From 4bcfa07a7d8baae64998c97d4e35e3333dbbd727 Mon Sep 17 00:00:00 2001
From: Norman Maurer <norman_maurer@apple.com>
Date: Wed, 29 Mar 2017 11:20:31 +0200
Subject: [PATCH] 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.
---
 .../ssl/OpenSslCertificateException.java      | 11 ++---
 .../ssl/ReferenceCountedOpenSslContext.java   |  2 +-
 .../ssl/OpenSslCertificateExceptionTest.java  | 49 +++++++++++++++++++
 pom.xml                                       |  2 +-
 4 files changed, 56 insertions(+), 8 deletions(-)
 create mode 100644 handler/src/test/java/io/netty/handler/ssl/OpenSslCertificateExceptionTest.java

diff --git a/handler/src/main/java/io/netty/handler/ssl/OpenSslCertificateException.java b/handler/src/main/java/io/netty/handler/ssl/OpenSslCertificateException.java
index c16018666a..4672d00787 100644
--- a/handler/src/main/java/io/netty/handler/ssl/OpenSslCertificateException.java
+++ b/handler/src/main/java/io/netty/handler/ssl/OpenSslCertificateException.java
@@ -21,7 +21,7 @@ import java.security.cert.CertificateException;
 
 /**
  * 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 {
     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() {
         return errorCode;
     }
 
     private static int checkErrorCode(int errorCode) {
-        if (errorCode < CertificateVerifier.X509_V_OK || errorCode > CertificateVerifier.X509_V_ERR_DANE_NO_MATCH) {
-            throw new IllegalArgumentException("errorCode must be " + CertificateVerifier.X509_V_OK + " => "
-                    + CertificateVerifier.X509_V_ERR_DANE_NO_MATCH +
-                    ". See https://www.openssl.org/docs/manmaster/apps/verify.html .");
+        if (!CertificateVerifier.isValid(errorCode)) {
+            throw new IllegalArgumentException("errorCode '" + errorCode +
+                    "' invalid, see https://www.openssl.org/docs/man1.0.2/apps/verify.html.");
         }
         return errorCode;
     }
diff --git a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslContext.java b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslContext.java
index 83c5af7120..b35bc17c11 100644
--- a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslContext.java
+++ b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslContext.java
@@ -615,7 +615,7 @@ public abstract class ReferenceCountedOpenSslContext extends SslContext implemen
         return refCnt.release(decrement);
     }
 
-    abstract static class AbstractCertificateVerifier implements CertificateVerifier {
+    abstract static class AbstractCertificateVerifier extends CertificateVerifier {
         private final OpenSslEngineMap engineMap;
 
         AbstractCertificateVerifier(OpenSslEngineMap engineMap) {
diff --git a/handler/src/test/java/io/netty/handler/ssl/OpenSslCertificateExceptionTest.java b/handler/src/test/java/io/netty/handler/ssl/OpenSslCertificateExceptionTest.java
new file mode 100644
index 0000000000..229e853cd2
--- /dev/null
+++ b/handler/src/test/java/io/netty/handler/ssl/OpenSslCertificateExceptionTest.java
@@ -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);
+    }
+}
diff --git a/pom.xml b/pom.xml
index 9fedce841c..55183c41d2 100644
--- a/pom.xml
+++ b/pom.xml
@@ -242,7 +242,7 @@
     <!-- Fedora-"like" systems. This is currently only used for the netty-tcnative dependency -->
     <os.detection.classifierWithLikes>fedora</os.detection.classifierWithLikes>
     <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>
     <conscrypt.groupId>org.conscrypt</conscrypt.groupId>
     <conscrypt.artifactId>conscrypt-openjdk-uber</conscrypt.artifactId>