Repository: netty/netty
Issue: SslContextBuilder has conflicting conditions in keyManager(...)
Labels: defect

The `keyManager(File, ...)` and `keyManager(KeyManagerFactory)` methods have conflicting conditions for the server SslContextBuilder.

Or SslContextBuilder is missing an argument-less `forServer()` method.

```
SslContextBuilder.forServer(null, null)
    .keyManager(myKeyManagerFactory)
    .build();
```

```
java.lang.NullPointerException: keyCertChainFile required for servers
    at io.netty.util.internal.ObjectUtil.checkNotNull(ObjectUtil.java:31)
    at io.netty.handler.ssl.SslContextBuilder.keyManager(SslContextBuilder.java:129)
    at io.netty.handler.ssl.SslContextBuilder.keyManager(SslContextBuilder.java:115)
    at io.netty.handler.ssl.SslContextBuilder.forServer(SslContextBuilder.java:44)
```


Issue comments:
@ejona86 can you check ?

There should be a `SslContextBuilder.forServer(KeyManagerFactory)` but it is missing. Workaround is to provide any `File` (doesn't need to exist) to `forServer()` and then change the configuration.

```
SslContextBuilder.forServer(new File("foo"), new File("foo"))
    .keyManager(myKeyManagerFactory)
    .build();
```

I'll create a PR to add the missing method.

@ejona86 :beers: on me ;)

@normanmaurer, I have discovered the method of getting :beers:: introduce bugs, then fix the bugs. :-)

@ejona86 lol


Pull request review comments:

Patch that resolved the issue:
diff --git a/handler/src/main/java/io/netty/handler/ssl/SslContext.java b/handler/src/main/java/io/netty/handler/ssl/SslContext.java
index 890b36242bf..0865890f405 100644
--- a/handler/src/main/java/io/netty/handler/ssl/SslContext.java
+++ b/handler/src/main/java/io/netty/handler/ssl/SslContext.java
@@ -76,7 +76,7 @@
  * <pre>
  * // In your {@link ChannelInitializer}:
  * {@link ChannelPipeline} p = channel.pipeline();
- * {@link SslContext} sslCtx = {@link #newBuilderForClient() SslContext.newBuilderForClient()}.build();
+ * {@link SslContext} sslCtx = {@link SslContextBuilder#forClient() SslContextBuilder.forClient()}.build();
  * p.addLast("ssl", {@link #newEngine(ByteBufAllocator, String, int) sslCtx.newEngine(channel.alloc(), host, port)});
  * ...
  * </pre>
diff --git a/handler/src/main/java/io/netty/handler/ssl/SslContextBuilder.java b/handler/src/main/java/io/netty/handler/ssl/SslContextBuilder.java
index fcc840e6ca8..a0d3caf45d0 100644
--- a/handler/src/main/java/io/netty/handler/ssl/SslContextBuilder.java
+++ b/handler/src/main/java/io/netty/handler/ssl/SslContextBuilder.java
@@ -39,6 +39,7 @@ public static SslContextBuilder forClient() {
      *
      * @param keyCertChainFile an X.509 certificate chain file in PEM format
      * @param keyFile a PKCS#8 private key file in PEM format
+     * @see #keyManager(File, File)
      */
     public static SslContextBuilder forServer(File keyCertChainFile, File keyFile) {
         return new SslContextBuilder(true).keyManager(keyCertChainFile, keyFile);
@@ -51,12 +52,23 @@ public static SslContextBuilder forServer(File keyCertChainFile, File keyFile) {
      * @param keyFile a PKCS#8 private key file in PEM format
      * @param keyPassword the password of the {@code keyFile}, or {@code null} if it's not
      *     password-protected
+     * @see #keyManager(File, File, String)
      */
     public static SslContextBuilder forServer(
             File keyCertChainFile, File keyFile, String keyPassword) {
         return new SslContextBuilder(true).keyManager(keyCertChainFile, keyFile, keyPassword);
     }
 
+    /**
+     * Creates a builder for new server-side {@link SslContext}.
+     *
+     * @param keyManagerFactory non-{@code null} factory for server's private key
+     * @see #keyManager(KeyManagerFactory)
+     */
+    public static SslContextBuilder forServer(KeyManagerFactory keyManagerFactory) {
+        return new SslContextBuilder(true).keyManager(keyManagerFactory);
+    }
+
     private final boolean forServer;
     private SslProvider provider;
     private File trustCertChainFile;


Test patch: