From 5ff6b579403eacb6288f50cd0d5fb5641c1aa276 Mon Sep 17 00:00:00 2001
From: Norman Maurer <norman_maurer@apple.com>
Date: Wed, 5 Sep 2018 20:33:40 +0200
Subject: [PATCH] =?UTF-8?q?PemPrivateKey.toPem(...)=20should=20throw=20Ill?=
 =?UTF-8?q?egalArgumentException=20when=20P=E2=80=A6=20(#8253)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* PemPrivateKey.toPem(...) should throw IllegalArgumentException when PrivateKey which does not support encoding is used.

Motivation:

At the moment when a PrivateKey is used that does not support encoding we throw a NPE when trying to convert the key. We should better throw an IllegalArgumentException with the details about what key we tried to encode.

Modifications:

- Check if PrivateKey.getEncoded() returns null and if so throw an IllegalArgumentException
- Add unit test.

Result:

Better handling of non-supported PrivateKey implementations.
---
 .../io/netty/handler/ssl/PemPrivateKey.java   |  7 +++++-
 .../io/netty/handler/ssl/PemEncodedTest.java  | 23 +++++++++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/handler/src/main/java/io/netty/handler/ssl/PemPrivateKey.java b/handler/src/main/java/io/netty/handler/ssl/PemPrivateKey.java
index e7bfd12392..46145a04b1 100644
--- a/handler/src/main/java/io/netty/handler/ssl/PemPrivateKey.java
+++ b/handler/src/main/java/io/netty/handler/ssl/PemPrivateKey.java
@@ -60,7 +60,12 @@ public final class PemPrivateKey extends AbstractReferenceCounted implements Pri
             return ((PemEncoded) key).retain();
         }
 
-        ByteBuf encoded = Unpooled.wrappedBuffer(key.getEncoded());
+        byte[] bytes = key.getEncoded();
+        if (bytes == null) {
+            throw new IllegalArgumentException(key.getClass().getName() + " does not support encoding");
+        }
+
+        ByteBuf encoded = Unpooled.wrappedBuffer(bytes);
         try {
             ByteBuf base64 = SslUtils.toBase64(allocator, encoded);
             try {
diff --git a/handler/src/test/java/io/netty/handler/ssl/PemEncodedTest.java b/handler/src/test/java/io/netty/handler/ssl/PemEncodedTest.java
index 793f772287..b2531eb8f6 100644
--- a/handler/src/test/java/io/netty/handler/ssl/PemEncodedTest.java
+++ b/handler/src/test/java/io/netty/handler/ssl/PemEncodedTest.java
@@ -24,7 +24,10 @@ import static org.junit.Assume.assumeTrue;
 import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.FileInputStream;
+import java.security.PrivateKey;
 
+import io.netty.buffer.Unpooled;
+import io.netty.buffer.UnpooledByteBufAllocator;
 import org.junit.Test;
 
 import io.netty.handler.ssl.util.SelfSignedCertificate;
@@ -69,6 +72,26 @@ public class PemEncodedTest {
         }
     }
 
+    @Test(expected = IllegalArgumentException.class)
+    public void testEncodedReturnsNull() throws Exception {
+        PemPrivateKey.toPEM(UnpooledByteBufAllocator.DEFAULT, true, new PrivateKey() {
+            @Override
+            public String getAlgorithm() {
+                return null;
+            }
+
+            @Override
+            public String getFormat() {
+                return null;
+            }
+
+            @Override
+            public byte[] getEncoded() {
+                return null;
+            }
+        });
+    }
+
     private static void assertRelease(PemEncoded encoded) {
         assertTrue(encoded.release());
     }