From 795fa8aef170b19a5b1fcc9b36c3cc7cb8ffffb8 Mon Sep 17 00:00:00 2001
From: Norman Maurer <norman_maurer@apple.com>
Date: Mon, 29 Apr 2019 08:31:14 +0200
Subject: [PATCH] Throw SignatureException if OpenSslPrivateKeyMethod.* return
 null to prevent segfault (#9100)

Motivation:

While OpenSslPrivateKeyMethod.* should never return null we should still guard against it to prevent any possible segfault.

Modifications:

- Throw SignatureException if null is returned
- Add unit test

Result:

No segfault when user returns null.
---
 .../netty/handler/ssl/OpenSslPrivateKeyMethod.java  |  4 ++--
 .../handler/ssl/ReferenceCountedOpenSslContext.java | 12 ++++++++++--
 .../handler/ssl/OpenSslPrivateKeyMethodTest.java    | 13 ++++++++++++-
 3 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/handler/src/main/java/io/netty/handler/ssl/OpenSslPrivateKeyMethod.java b/handler/src/main/java/io/netty/handler/ssl/OpenSslPrivateKeyMethod.java
index 4b1da0b25c..d9fc877269 100644
--- a/handler/src/main/java/io/netty/handler/ssl/OpenSslPrivateKeyMethod.java
+++ b/handler/src/main/java/io/netty/handler/ssl/OpenSslPrivateKeyMethod.java
@@ -45,7 +45,7 @@ public interface OpenSslPrivateKeyMethod {
      * @param engine                the {@link SSLEngine}
      * @param signatureAlgorithm    the algorithm to use for signing
      * @param input                 the digest itself
-     * @return                      the signed data
+     * @return                      the signed data (must not be {@code null})
      * @throws Exception            thrown if an error is encountered during the signing
      */
     byte[] sign(SSLEngine engine, int signatureAlgorithm, byte[] input) throws Exception;
@@ -55,7 +55,7 @@ public interface OpenSslPrivateKeyMethod {
      *
      * @param engine                the {@link SSLEngine}
      * @param input                 the input which should be decrypted
-     * @return                      the decrypted data
+     * @return                      the decrypted data (must not be {@code null})
      * @throws Exception            thrown if an error is encountered during the decrypting
      */
     byte[] decrypt(SSLEngine engine, byte[] input) throws Exception;
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 415cee4004..5209daaa5e 100644
--- a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslContext.java
+++ b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslContext.java
@@ -35,6 +35,7 @@ import io.netty.util.internal.logging.InternalLoggerFactory;
 import java.security.AccessController;
 import java.security.PrivateKey;
 import java.security.PrivilegedAction;
+import java.security.SignatureException;
 import java.security.cert.CertPathValidatorException;
 import java.security.cert.Certificate;
 import java.security.cert.CertificateExpiredException;
@@ -917,7 +918,7 @@ public abstract class ReferenceCountedOpenSslContext extends SslContext implemen
         public byte[] sign(long ssl, int signatureAlgorithm, byte[] digest) throws Exception {
             ReferenceCountedOpenSslEngine engine = retrieveEngine(ssl);
             try {
-                return keyMethod.sign(engine, signatureAlgorithm, digest);
+                return verifyResult(keyMethod.sign(engine, signatureAlgorithm, digest));
             } catch (Exception e) {
                 engine.initHandshakeException(e);
                 throw e;
@@ -928,11 +929,18 @@ public abstract class ReferenceCountedOpenSslContext extends SslContext implemen
         public byte[] decrypt(long ssl, byte[] input) throws Exception {
             ReferenceCountedOpenSslEngine engine = retrieveEngine(ssl);
             try {
-                return keyMethod.decrypt(engine, input);
+                return verifyResult(keyMethod.decrypt(engine, input));
             } catch (Exception e) {
                 engine.initHandshakeException(e);
                 throw e;
             }
         }
+
+        private static byte[] verifyResult(byte[] result) throws SignatureException {
+            if (result == null) {
+                throw new SignatureException();
+            }
+            return result;
+        }
     }
 }
diff --git a/handler/src/test/java/io/netty/handler/ssl/OpenSslPrivateKeyMethodTest.java b/handler/src/test/java/io/netty/handler/ssl/OpenSslPrivateKeyMethodTest.java
index 81a030a057..0c65b4cd65 100644
--- a/handler/src/test/java/io/netty/handler/ssl/OpenSslPrivateKeyMethodTest.java
+++ b/handler/src/test/java/io/netty/handler/ssl/OpenSslPrivateKeyMethodTest.java
@@ -314,11 +314,22 @@ public class OpenSslPrivateKeyMethodTest {
     }
 
     @Test
-    public void testPrivateKeyMethodFails() throws Exception {
+    public void testPrivateKeyMethodFailsBecauseOfException() throws Exception {
+        testPrivateKeyMethodFails(false);
+    }
+
+    @Test
+    public void testPrivateKeyMethodFailsBecauseOfNull() throws Exception {
+        testPrivateKeyMethodFails(true);
+    }
+    private void testPrivateKeyMethodFails(final boolean returnNull) throws Exception {
         final SslContext sslServerContext = buildServerContext(new OpenSslPrivateKeyMethod() {
             @Override
             public byte[] sign(SSLEngine engine, int signatureAlgorithm, byte[] input) throws Exception {
                 assertThread();
+                if (returnNull) {
+                    return null;
+                }
                 throw new SignatureException();
             }