Repository: clj-commons/aleph
Pull request #612 (2022-07-30 20:20:27)

Improved API for SSL context management (server and client)
This is a cherry-pick of dac2852c73c8ceeecaa15549c9de08dbcd530efd from the `1.0.0` adjusted to latest `master` and with some additions and corrections to docstrings.

As per https://github.com/clj-commons/aleph/pull/608#issuecomment-1200103587

DerGuteMoritz: > Regarding TLS, an improvement would be to support `javax.net.ssl SSLContext` out of the box.

@arnaudgeiser Seems like this should be doable via https://netty.io/4.1/api/io/netty/handler/ssl/JdkSslContext.html which one could then pass as `:ssl-context`. Some builtin support for this in Aleph would be more convenient, of course. Should be easy to do now by adding it to `coerce-ssl-context`. Ideally with a test case, of course... I suggest we make this a separate issue?
arnaudgeiser: > I suggest we make this a separate issue?

Yes, obviously.
Just surprised it wasn't supported already.
DerGuteMoritz: @KingMob FYI: I'll resolve the conflicts (and squash where appropriate) once you've approved my latest changes!
arnaudgeiser: I tried to rebase this PR on master but unfortunately I don't have the rights to perform commits on your branch @DerGuteMoritz.
Can I ask you to do so then I will merge?
DerGuteMoritz: @arnaudgeiser There you go! Only squashed the indentation commit after all.
DerGuteMoritz: Hold on a moment, I accidentlly clobbered Oleksii's original commit authorship, restoring ...

Patch:
diff --git a/src/aleph/http.clj b/src/aleph/http.clj
index 515beb6..8f3b2c5 100644
--- a/src/aleph/http.clj
+++ b/src/aleph/http.clj
@@ -35,7 +35,7 @@
    | `port` | the port the server will bind to.  If `0`, the server will bind to a random port.
    | `socket-address` |  a `java.net.SocketAddress` specifying both the port and interface to bind to.
    | `bootstrap-transform` | a function that takes an `io.netty.bootstrap.ServerBootstrap` object, which represents the server, and modifies it.
-   | `ssl-context` | an `io.netty.handler.ssl.SslContext` object if an SSL connection is desired |
+   | `ssl-context` | an `io.netty.handler.ssl.SslContext` object or a map of SSL context options (see `aleph.netty/ssl-server-context` for more details) if an SSL connection is desired |
    | `manual-ssl?` | set to `true` to indicate that SSL is active, but the caller is managing it (this implies `:ssl-context` is nil). For example, this can be used if you want to use configure SNI (perhaps in `:pipeline-transform`) to select the SSL context based on the client's indicated host name. |
    | `pipeline-transform` | a function that takes an `io.netty.channel.ChannelPipeline` object, which represents a connection, and modifies it.
    | `executor` | a `java.util.concurrent.Executor` which is used to handle individual requests.  To avoid this indirection you may specify `:none`, but in this case extreme care must be taken to avoid blocking operations on the handler's thread.
@@ -107,7 +107,7 @@
    the `connection-options` are a map describing behavior across all connections:
 
    |:---|:---
-   | `ssl-context` | an `io.netty.handler.ssl.SslContext` object, only required if a custom context is required
+   | `ssl-context` | an `io.netty.handler.ssl.SslContext` object or a map of SSL context options (see `aleph.netty/ssl-client-context` for more details), only required if a custom context is required
    | `local-address` | an optional `java.net.SocketAddress` describing which local interface should be used
    | `bootstrap-transform` | a function that takes an `io.netty.bootstrap.Bootstrap` object and modifies it.
    | `pipeline-transform` | a function that takes an `io.netty.channel.ChannelPipeline` object, which represents a connection, and modifies it.
diff --git a/src/aleph/netty.clj b/src/aleph/netty.clj
index 03604f3..de1d83e 100644
--- a/src/aleph/netty.clj
+++ b/src/aleph/netty.clj
@@ -34,12 +34,15 @@
      NioSocketChannel
      NioDatagramChannel]
     [io.netty.handler.ssl
+     ClientAuth
      SslContext
      SslContextBuilder
-     SslHandler]
+     SslHandler
+     SslProvider]
     [io.netty.handler.codec DecoderException]
     [io.netty.handler.ssl.util
-     SelfSignedCertificate InsecureTrustManagerFactory]
+     SelfSignedCertificate
+     InsecureTrustManagerFactory]
     [io.netty.resolver
      AddressResolverGroup
      NoopAddressResolverGroup
@@ -78,7 +81,10 @@
      LogLevel]
     [java.security.cert X509Certificate]
     [java.security PrivateKey]
-    [javax.net.ssl SSLHandshakeException]))
+    [javax.net.ssl
+     SSLHandshakeException
+     TrustManager
+     TrustManagerFactory]))
 
 ;;;
 
@@ -727,63 +733,230 @@
 
 ;;;
 
+(defn coerce-ssl-provider ^SslProvider [provider]
+  (case provider
+    :jdk SslProvider/JDK
+    :openssl SslProvider/OPENSSL
+    :openssl-refcnt SslProvider/OPENSSL_REFCNT))
+
+(let [cert-array-class (class (into-array X509Certificate []))]
+  (defn- add-ssl-trust-manager! ^SslContextBuilder [^SslContextBuilder builder trust-store]
+    (cond (instance? File trust-store)
+          (.trustManager builder ^File trust-store)
+          (instance? InputStream trust-store)
+          (.trustManager builder ^InputStream trust-store)
+          (instance? TrustManager trust-store)
+          (.trustManager builder ^TrustManager trust-store)
+          (instance? TrustManagerFactory trust-store)
+          (.trustManager builder ^TrustManagerFactory trust-store)
+          (instance? cert-array-class trust-store)
+          (.trustManager builder ^"[Ljava.security.cert.X509Certificate;" trust-store)
+          (sequential? trust-store)
+          (let [^"[Ljava.security.cert.X509Certificate;" trust-store' (into-array X509Certificate trust-store)]
+            (.trustManager builder trust-store'))
+          :else
+          (throw
+           (IllegalArgumentException.
+            "ssl context arguments invalid"))))
+
+  (defn ssl-client-context
+    "Creates a new client SSL context.
+     Keyword arguments are:
+     |:---|:----
+     | `private-key` | a `java.io.File`, `java.io.InputStream`, or `java.security.PrivateKey` containing the client-side private key.
+     | `certificate-chain` | a `java.io.File`, `java.io.InputStream`, sequence of `java.security.cert.X509Certificate`, or array of `java.security.cert.X509Certificate` containing the client's certificate chain.
+     | `private-key-password` | a string, the private key's password (optional).
+     | `trust-store` | a `java.io.File`, `java.io.InputStream`, array of `java.security.cert.X509Certificate`, `javax.net.ssl.TrustManager`, or a `javax.net.ssl.TrustManagerFactory` to initialize the context's trust manager.
+     | `ssl-provider` | `SslContext` implementation to use, on of `:jdk`, `:openssl` or `:openssl-refcnt`. Note, that when using OpenSSL based implementations, the library should be installed and linked properly.
+     | `ciphers` | a sequence of strings, the cipher suites to enable, in the order of preference.
+     | `protocols` | a sequence of strings, the TLS protocol versions to enable.
+     | `session-cache-size` | the size of the cache used for storing SSL session objects.
+     | `session-timeout` | the timeout for the cached SSL session objects, in seconds.
+     Note that if specified, the types of `private-key` and `certificate-chain` must be \"compatible\": either both input streams, both files, or a private key and an array of certificates."
+    ([] (ssl-client-context {}))
+    ([{:keys [private-key
+              ^String private-key-password
+              certificate-chain
+              trust-store
+              ssl-provider
+              ^Iterable ciphers
+              protocols
+              ^long session-cache-size
+              ^long session-timeout]}]
+     (let [^SslContextBuilder
+           builder (SslContextBuilder/forClient)
+
+           ^SslContextBuilder
+           builder (if (or private-key certificate-chain)
+                     (cond (and (instance? File private-key)
+                                (instance? File certificate-chain))
+                           (.keyManager builder
+                                        ^File certificate-chain
+                                        ^File private-key
+                                        private-key-password)
+                           (and (instance? InputStream private-key)
+                                (instance? InputStream certificate-chain))
+                           (.keyManager builder
+                                        ^InputStream certificate-chain
+                                        ^InputStream private-key
+                                        private-key-password)
+                           (and (instance? PrivateKey private-key)
+                                (instance? cert-array-class certificate-chain))
+                           (.keyManager builder
+                                        ^PrivateKey private-key
+                                        private-key-password
+                                        ^"[Ljava.security.cert.X509Certificate;" certificate-chain)
+                           (and (instance? PrivateKey private-key)
+                                (sequential? certificate-chain))
+                           (let [^"[Ljava.security.cert.X509Certificate;" certificate-chain' (into-array X509Certificate certificate-chain)]
+                             (.keyManager builder
+                                          ^PrivateKey private-key
+                                          private-key-password
+                                          certificate-chain'))
+                           :else
+                           (throw
+                            (IllegalArgumentException.
+                             "ssl context arguments invalid")))
+                     builder)
+
+           ^SslContextBuilder
+           builder (cond-> builder
+                     (some? trust-store)
+                     (add-ssl-trust-manager! trust-store)
+
+                     (some? ssl-provider)
+                     (.sslProvider (coerce-ssl-provider ssl-provider))
+
+                     (some? ciphers)
+                     (.ciphers ciphers)
+
+                     (some? protocols)
+                     (.protocols ^"[Ljava.lang.String;" (into-array String protocols))
+
+                     (some? session-cache-size)
+                     (.sessionCacheSize session-cache-size)
+
+                     (some? session-timeout)
+                     (.sessionTimeout session-timeout))]
+
+       (.build builder))))
+
+  (defn ssl-server-context
+    "Creates a new server SSL context.
+     Keyword arguments are:
+     |:---|:----
+     | `private-key` | a `java.io.File`, `java.io.InputStream`, or `java.security.PrivateKey` containing the server-side private key.
+     | `certificate-chain` | a `java.io.File`, `java.io.InputStream`, or array of `java.security.cert.X509Certificate` containing the server's certificate chain.
+     | `private-key-password` | a string, the private key's password (optional).
+     | `trust-store` | a `java.io.File`, `java.io.InputStream`, sequence of `java.security.cert.X509Certificate`,  array of `java.security.cert.X509Certificate`, `javax.net.ssl.TrustManager`, or a `javax.net.ssl.TrustManagerFactory` to initialize the context's trust manager.
+     | `ssl-provider` | `SslContext` implementation to use, on of `:jdk`, `:openssl` or `:openssl-refcnt`. Note, that when using OpenSSL based implementations, the library should be installed and linked properly.
+     | `ciphers` | a sequence of strings, the cipher suites to enable, in the order of preference.
+     | `protocols` | a sequence of strings, the TLS protocol versions to enable.
+     | `session-cache-size` | the size of the cache used for storing SSL session objects.
+     | `session-timeout` | the timeout for the cached SSL session objects, in seconds.
+     | `start-tls` | if the first write request shouldn't be encrypted.
+     | `client-auth` | the client authentication mode, one of `:none`, `:optional` or `:require`.
+     Note that if specified, the types of `private-key` and `certificate-chain` must be \"compatible\": either both input streams, both files, or a private key and an array of certificates."
+    ([] (ssl-server-context {}))
+    ([{:keys [private-key
+              ^String private-key-password
+              certificate-chain
+              trust-store
+              ssl-provider
+              ^Iterable ciphers
+              protocols
+              ^long session-cache-size
+              ^long session-timeout
+              start-tls
+              client-auth]}]
+     (let [^SslContextBuilder
+           b (cond (and (instance? File private-key)
+                        (instance? File certificate-chain))
+                   (SslContextBuilder/forServer ^File certificate-chain
+                                                ^File private-key
+                                                private-key-password)
+                   (and (instance? InputStream private-key)
+                        (instance? InputStream certificate-chain))
+                   (SslContextBuilder/forServer ^InputStream certificate-chain
+                                                ^InputStream private-key
+                                                private-key-password)
+                   (and (instance? PrivateKey private-key)
+                        (instance? cert-array-class certificate-chain))
+                   (SslContextBuilder/forServer ^PrivateKey private-key
+                                                private-key-password
+                                                ^"[Ljava.security.cert.X509Certificate;" certificate-chain)
+                   (and (instance? PrivateKey private-key)
+                        (sequential? certificate-chain))
+                   (let [^"[Ljava.security.cert.X509Certificate;" certificate-chain' (into-array X509Certificate certificate-chain)]
+                     (SslContextBuilder/forServer ^PrivateKey private-key
+                                                  private-key-password
+                                                  certificate-chain'))
+                   :else
+                   (throw
+                    (IllegalArgumentException.
+                     "ssl context arguments invalid")))
+
+           ^SslContextBuilder
+           b (cond-> b
+               (some? trust-store)
+               (add-ssl-trust-manager! trust-store)
+
+               (some? ssl-provider)
+               (.sslProvider (coerce-ssl-provider ssl-provider))
+
+               (some? ciphers)
+               (.ciphers ciphers)
+
+
+               (some? protocols)
+               (.protocols ^"[Ljava.lang.String;" (into-array String protocols))
+
+
+               (some? session-cache-size)
+               (.sessionCacheSize session-cache-size)
+
+               (some? session-timeout)
+               (.sessionTimeout session-timeout)
+
+               (some? start-tls)
+               (.startTls (boolean start-tls))
+
+               (some? client-auth)
+               (.clientAuth (case client-auth
+                              :none ClientAuth/NONE
+                              :optional ClientAuth/OPTIONAL
+                              :require ClientAuth/REQUIRE)))]
+       (.build b)))))
+
 (defn self-signed-ssl-context
   "A self-signed SSL context for servers."
   []
   (let [cert (SelfSignedCertificate.)]
-    (.build (SslContextBuilder/forServer (.certificate cert) (.privateKey cert)))))
+    (ssl-server-context {:private-key (.privateKey cert)
+                         :certificate-chain (.certificate cert)})))
 
 (defn insecure-ssl-client-context []
-  (-> (SslContextBuilder/forClient)
-      (.trustManager InsecureTrustManagerFactory/INSTANCE)
-      .build))
-
-(defn- check-ssl-args
-  [private-key certificate-chain]
-  (when-not
-    (or (and (instance? File private-key) (instance? File certificate-chain))
-      (and (instance? InputStream private-key) (instance? InputStream certificate-chain))
-      (and (instance? PrivateKey private-key) (instance? (class (into-array X509Certificate [])) certificate-chain)))
-    (throw (IllegalArgumentException. "ssl-client-context arguments invalid"))))
-
-(set! *warn-on-reflection* false)
-
-(defn ssl-client-context
-  "Creates a new client SSL context.
-
-  Keyword arguments are:
-
-  |:---|:----
-  | `private-key` | A `java.io.File`, `java.io.InputStream`, or `java.security.PrivateKey` containing the client-side private key.
-  | `certificate-chain` | A `java.io.File`, `java.io.InputStream`, or array of `java.security.cert.X509Certificate` containing the client's certificate chain.
-  | `private-key-password` | A string, the private key's password (optional).
-  | `trust-store` | A `java.io.File`, `java.io.InputStream`, array of `java.security.cert.X509Certificate`, or a `javax.net.ssl.TrustManagerFactory` to initialize the context's trust manager.
-
-  Note that if specified, the types of `private-key` and `certificate-chain` must be
-  \"compatible\": either both input streams, both files, or a private key and an array
-  of certificates."
-  ([] (ssl-client-context {}))
-  ([{:keys [private-key private-key-password certificate-chain trust-store]}]
-    (-> (SslContextBuilder/forClient)
-      (#(if (and private-key certificate-chain)
-          (do
-            (check-ssl-args private-key certificate-chain)
-            (if (instance? (class (into-array X509Certificate [])) certificate-chain)
-              (.keyManager %
-                private-key
-                private-key-password
-                certificate-chain)
-              (.keyManager %
-                certificate-chain
-                private-key
-                private-key-password)))
-          %))
-      (#(if trust-store
-          (.trustManager % trust-store)
-          %))
-      .build)))
-
-(set! *warn-on-reflection* true)
+  (ssl-client-context {:trust-store InsecureTrustManagerFactory/INSTANCE}))
+
+(defn- coerce-ssl-context [options->context ssl-context]
+  (cond
+    (instance? SslContext ssl-context)
+    ssl-context
+
+    ;; in future this option might be interesing
+    ;; for turning application config (e.g. ALPN)
+    ;; depending on the server's capabilities
+    (instance? SslContextBuilder ssl-context)
+    (.build ^SslContextBuilder ssl-context)
+
+    (map? ssl-context)
+    (options->context ssl-context)))
+
+(def ^:private  coerce-ssl-server-context
+  (partial coerce-ssl-context ssl-server-context))
+
+(def ^:private  coerce-ssl-client-context
+  (partial coerce-ssl-context ssl-client-context))
 
 (defn channel-ssl-session [^Channel ch]
   (some-> ch
@@ -975,7 +1148,7 @@ initialize an DnsAddressResolverGroup instance.
       epoll?
       nil))
   ([pipeline-builder
-    ^SslContext ssl-context
+    ssl-context
     bootstrap-transform
     ^SocketAddress remote-address
     ^SocketAddress local-address
@@ -988,6 +1161,10 @@ initialize an DnsAddressResolverGroup instance.
                     EpollSocketChannel
                     NioSocketChannel)
 
+          ^SslContext
+          ssl-context (when (some? ssl-context)
+                        (coerce-ssl-client-context ssl-context))
+
           pipeline-builder (if ssl-context
                              (fn [^ChannelPipeline p]
                                (.addLast p "ssl-handler"
@@ -1026,7 +1203,7 @@ initialize an DnsAddressResolverGroup instance.
 
 (defn start-server
   [pipeline-builder
-   ^SslContext ssl-context
+   ssl-context
    bootstrap-transform
    on-close
    ^SocketAddress socket-address
@@ -1053,6 +1230,10 @@ initialize an DnsAddressResolverGroup instance.
         transport
         (if epoll? :epoll :nio)
 
+        ^SslContext
+        ssl-context (when (some? ssl-context)
+                      (coerce-ssl-server-context ssl-context))
+
         pipeline-builder
         (if ssl-context
           (fn [^ChannelPipeline p]
diff --git a/src/aleph/tcp.clj b/src/aleph/tcp.clj
index b08828d..2becd63 100644
--- a/src/aleph/tcp.clj
+++ b/src/aleph/tcp.clj
@@ -88,7 +88,7 @@
    |:---|:-----
    | `port` | the port the server will bind to.  If `0`, the server will bind to a random port.
    | `socket-address` | a `java.net.SocketAddress` specifying both the port and interface to bind to.
-   | `ssl-context` | an `io.netty.handler.ssl.SslContext` object. If given, the server will only accept SSL connections and call the handler once the SSL session has been successfully established. If a self-signed certificate is all that's required, `(aleph.netty/self-signed-ssl-context)` will suffice.
+   | `ssl-context` | an `io.netty.handler.ssl.SslContext` object or a map of SSL context options (see `aleph.netty/ssl-server-context` for more details). If given, the server will only accept SSL connections and call the handler once the SSL session has been successfully established. If a self-signed certificate is all that's required, `(aleph.netty/self-signed-ssl-context)` will suffice.
    | `bootstrap-transform` | a function that takes an `io.netty.bootstrap.ServerBootstrap` object, which represents the server, and modifies it.
    | `pipeline-transform` | a function that takes an `io.netty.channel.ChannelPipeline` object, which represents a connection, and modifies it.
    | `raw-stream?` | if true, messages from the stream will be `io.netty.buffer.ByteBuf` objects rather than byte-arrays.  This will minimize copying, but means that care must be taken with Netty's buffer reference counting.  Only recommended for advanced users."
@@ -164,7 +164,7 @@
    | `port` | the port of the server.
    | `remote-address` | a `java.net.SocketAddress` specifying the server's address.
    | `local-address` | a `java.net.SocketAddress` specifying the local network interface to use.
-   | `ssl-context` | an explicit `io.netty.handler.ssl.SslHandler` to use. Defers to `ssl?` and `insecure?` configuration if omitted.
+   | `ssl-context` | an explicit `io.netty.handler.ssl.SslHandler` or a map of SSL context options (see `aleph.netty/ssl-server-context` for more details) to use. Defers to `ssl?` and `insecure?` configuration if omitted.
    | `ssl?` | if true, the client attempts to establish a secure connection with the server.
    | `insecure?` | if true, the client will ignore the server's certificate.
    | `bootstrap-transform` | a function that takes an `io.netty.bootstrap.Bootstrap` object, which represents the client, and modifies it.


Test patch:
diff --git a/test/aleph/ssl.clj b/test/aleph/ssl.clj
index 889c326..14ee2ad 100644
--- a/test/aleph/ssl.clj
+++ b/test/aleph/ssl.clj
@@ -44,14 +44,19 @@
 (def ^X509Certificate client-cert (gen-cert (read-string (slurp "test/client_cert.edn"))))
 (def client-key (gen-key 65537 (read-string (slurp "test/client_key.edn"))))
 
+(def server-ssl-context-opts
+  {:private-key server-key
+   :certificate-chain [server-cert]
+   :trust-store [ca-cert]
+   :client-auth :optional})
+
 (def server-ssl-context
-  (-> (SslContextBuilder/forServer server-key (into-array X509Certificate [server-cert]))
-      (.trustManager (into-array X509Certificate [ca-cert]))
-      (.clientAuth ClientAuth/OPTIONAL)
-      .build))
+  (netty/ssl-server-context server-ssl-context-opts))
+
+(def client-ssl-context-opts
+  {:private-key client-key
+   :certificate-chain [client-cert]
+   :trust-store [ca-cert]})
 
 (def client-ssl-context
-  (netty/ssl-client-context
-   {:private-key client-key
-    :certificate-chain (into-array X509Certificate [client-cert])
-    :trust-store (into-array X509Certificate [ca-cert])}))
+  (netty/ssl-client-context client-ssl-context-opts))
diff --git a/test/aleph/tcp_ssl_test.clj b/test/aleph/tcp_ssl_test.clj
index e17d180..785352f 100644
--- a/test/aleph/tcp_ssl_test.clj
+++ b/test/aleph/tcp_ssl_test.clj
@@ -33,6 +33,20 @@
         (is (= (.getSubjectDN ^X509Certificate ssl/client-cert)
                (.getSubjectDN ^X509Certificate (first (.getPeerCertificates @ssl-session)))))))))
 
+(deftest test-ssl-opts-echo
+  (let [ssl-session (atom nil)]
+    (with-server (tcp/start-server (ssl-echo-handler ssl-session)
+                                   {:port 10001
+                                    :ssl-context ssl/server-ssl-context-opts})
+      (let [c @(tcp/client {:host "localhost"
+                            :port 10001
+                            :ssl-context ssl/client-ssl-context-opts})]
+        (s/put! c "foo")
+        (is (= "foo" (bs/to-string @(s/take! c))))
+        (is (some? @ssl-session) "SSL session should be defined")
+        (is (= (.getSubjectDN ^X509Certificate ssl/client-cert)
+               (.getSubjectDN ^X509Certificate (first (.getPeerCertificates @ssl-session)))))))))
+
 (deftest test-failed-ssl-handshake
   (let [ssl-session (atom nil)]
     (with-server (tcp/start-server (ssl-echo-handler ssl-session)