Merge pull request #689 from clj-commons/enable-tls-endpoint-identification-by-default

Enable TLS endpoint identification by default (breaking)

Author: KingMob

src/aleph/http.clj

@@ -120,6 +120,7 @@
    Param key                   | Description
    | ---                       | ---
    | `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
+   | `ssl-endpoint-id-alg`     | the name of the algorithm to use for SSL endpoint identification (see https://docs.oracle.com/en/java/javase/17/docs/specs/security/standard-names.html#endpoint-identification-algorithms), defaults to \"HTTPS\". Only used for HTTPS connections. Pass `nil` to disable endpoint identification.
    | `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.
@@ -229,6 +230,7 @@
    | `raw-stream?`         | if `true`, the connection will emit raw `io.netty.buffer.ByteBuf` objects rather than strings or byte-arrays.  This will minimize copying, but means that care must be taken with Netty's buffer reference counting.  Only recommended for advanced users.
    | `insecure?`           | if `true`, the certificates for `wss://` will be ignored.
    | `ssl-context`         | an `io.netty.handler.ssl.SslContext` object, only required if a custom context is required
+   | `ssl-endpoint-id-alg` | the name of the algorithm to use for SSL endpoint identification (see https://docs.oracle.com/en/java/javase/17/docs/specs/security/standard-names.html#endpoint-identification-algorithms), defaults to \"HTTPS\". Only used for WSS connections. Pass `nil` to disable endpoint identification.
    | `extensions?`         | if `true`, the websocket extensions will be supported.
    | `sub-protocols`       | a string with a comma seperated list of supported sub-protocols.
    | `headers`             | the headers that should be included in the handshake

src/aleph/http/client.clj

@@ -683,14 +683,14 @@
 
    Can't use an ApnHandler/ApplicationProtocolNegotiationHandler here,
    because it's tricky to run Manifold code on Netty threads."
-  [{:keys [ssl? remote-address ssl-context]}]
+  [{:keys [ssl? remote-address ssl-context ssl-endpoint-id-alg]}]
   (fn pipeline-builder
     [^ChannelPipeline pipeline]
     (when ssl?
       (do
         (.addLast pipeline
                   "ssl-handler"
-                  (netty/ssl-handler (.channel pipeline) ssl-context remote-address))
+                  (netty/ssl-handler (.channel pipeline) ssl-context remote-address ssl-endpoint-id-alg))
         (.addLast pipeline
                   "pause-handler"
                   ^ChannelHandler (netty/pause-handler))))))
@@ -954,6 +954,7 @@
            keep-alive?
            insecure?
            ssl-context
+           ssl-endpoint-id-alg
            response-buffer-size
            epoll?
            transport
@@ -966,6 +967,7 @@
            bootstrap-transform  identity
            pipeline-transform   identity
            keep-alive?          true
+           ssl-endpoint-id-alg  netty/default-ssl-endpoint-id-alg
            response-buffer-size 65536
            epoll?               false
            name-resolver        :default
@@ -999,6 +1001,7 @@
                            (assoc opts
                                   :ssl? ssl?
                                   :ssl-context ssl-context
+                                  :ssl-endpoint-id-alg ssl-endpoint-id-alg
                                   :remote-address remote-address
                                   :raw-stream? raw-stream?
                                   :response-buffer-size response-buffer-size

src/aleph/http/websocket/client.clj

@@ -232,6 +232,7 @@
    {:keys [raw-stream?
            insecure?
            ssl-context
+           ssl-endpoint-id-alg
            headers
            local-address
            bootstrap-transform
@@ -248,6 +249,7 @@
            pipeline-transform  identity
            raw-stream?         false
            epoll?              false
+           ssl-endpoint-id-alg netty/default-ssl-endpoint-id-alg
            sub-protocols       nil
            extensions?         false
            max-frame-payload   65536
@@ -294,7 +296,7 @@
                            (when ssl?
                              (.addLast p
                                        "ssl-handler"
-                                       (netty/ssl-handler (.channel p) ssl-context remote-address)))
+                                       (netty/ssl-handler (.channel p) ssl-context remote-address ssl-endpoint-id-alg)))
                            (.addLast p "http-client" (HttpClientCodec.))
                            (.addLast p "aggregator" (HttpObjectAggregator. 16384))
                            (.addLast p "websocket-frame-aggregator" (WebSocketFrameAggregator. max-frame-size))

src/aleph/netty.clj

@@ -1078,10 +1078,12 @@
 
 (defn self-signed-ssl-context
   "A self-signed SSL context for servers."
-  []
-  (let [cert (SelfSignedCertificate.)]
-    (ssl-server-context {:private-key       (.privateKey cert)
-                         :certificate-chain (.certificate cert)})))
+  ([]
+   (self-signed-ssl-context "localhost"))
+  ([hostname]
+   (let [cert (SelfSignedCertificate. hostname)]
+     (ssl-server-context {:private-key       (.privateKey cert)
+                          :certificate-chain (.certificate cert)}))))
 
 (defn insecure-ssl-client-context
   "An insure SSL context for servers."
@@ -1374,33 +1376,47 @@ initialize an DnsAddressResolverGroup instance.
   anyway."
   [_])
 
+(def ^:const ^:no-doc default-ssl-endpoint-id-alg "HTTPS")
+
 (defn ssl-handler
   "Generates a new SslHandler for the given SslContext.
 
-   The 2-arity version is for the server.
-   The 3-arity version is for the client. The `remote-address` must be provided"
-  (^ChannelHandler
+   The 2-ary version is for servers.
+
+   The 3- and 4-ary version are for clients.
+   For these, the `remote-address` must be provided.
+   The `ssl-endpoint-id-alg` is the name of the algorithm to use for endpoint identification (see https://docs.oracle.com/en/java/javase/17/docs/specs/security/standard-names.html#endpoint-identification-algorithms). It defaults to \"HTTPS\" in the 3-ary version which is a reasonable default for non-HTTPS uses, too. Pass `nil` to disable endpoint identification."
+  (^SslHandler
    [^Channel ch ^SslContext ssl-ctx]
    (.newHandler ssl-ctx
                 (.alloc ch)))
-  (^ChannelHandler
+  (^SslHandler
    [^Channel ch ^SslContext ssl-ctx ^InetSocketAddress remote-address]
-   (.newHandler ssl-ctx
-                (.alloc ch)
-                (.getHostName ^InetSocketAddress remote-address)
-                (.getPort ^InetSocketAddress remote-address))))
+   (ssl-handler ch ssl-ctx remote-address default-ssl-endpoint-id-alg))
+  (^SslHandler
+   [^Channel ch ^SslContext ssl-ctx ^InetSocketAddress remote-address ssl-endpoint-id-alg]
+   (let [ssl-handler (.newHandler ssl-ctx
+                                  (.alloc ch)
+                                  (.getHostName ^InetSocketAddress remote-address)
+                                  (.getPort ^InetSocketAddress remote-address))]
+     (when ssl-endpoint-id-alg
+       (let [ssl-engine (.engine ssl-handler)
+             ssl-params (.getSSLParameters ssl-engine)]
+         (.setEndpointIdentificationAlgorithm ssl-params ssl-endpoint-id-alg)
+         (.setSSLParameters ssl-engine ssl-params)))
+     ssl-handler)))
 
 (defn- add-ssl-handler-to-pipeline-builder
   "Adds an `SslHandler` to the pipeline builder."
   ([pipeline-builder ssl-ctx]
-   (add-ssl-handler-to-pipeline-builder pipeline-builder ssl-ctx nil))
-  ([pipeline-builder ssl-ctx remote-address]
+   (add-ssl-handler-to-pipeline-builder pipeline-builder ssl-ctx nil nil))
+  ([pipeline-builder ssl-ctx remote-address ssl-endpoint-id-alg]
    (fn [^ChannelPipeline p]
      (.addFirst p
                 "ssl-handler"
                 (let [ch (.channel p)]
                   (if remote-address
-                    (ssl-handler ch ssl-ctx remote-address)
+                    (ssl-handler ch ssl-ctx remote-address ssl-endpoint-id-alg)
                     (ssl-handler ch ssl-ctx))))
      (pipeline-builder p))))
 
@@ -1471,6 +1487,22 @@ initialize an DnsAddressResolverGroup instance.
   ([pipeline-builder
     ssl-context
     bootstrap-transform
+    remote-address
+    local-address
+    epoll?
+    name-resolver]
+   (create-client pipeline-builder
+                  ssl-context
+                  default-ssl-endpoint-id-alg
+                  bootstrap-transform
+                  remote-address
+                  local-address
+                  epoll?
+                  name-resolver))
+  ([pipeline-builder
+    ssl-context
+    ssl-endpoint-id-alg
+    bootstrap-transform
     ^SocketAddress remote-address
     ^SocketAddress local-address
     epoll?
@@ -1480,7 +1512,7 @@ initialize an DnsAddressResolverGroup instance.
                        (coerce-ssl-client-context ssl-context))
 
          pipeline-builder (if ssl-context
-                            (add-ssl-handler-to-pipeline-builder pipeline-builder ssl-context remote-address)
+                            (add-ssl-handler-to-pipeline-builder pipeline-builder ssl-context remote-address ssl-endpoint-id-alg)
                             pipeline-builder)]
      (create-client-chan {:pipeline-builder    pipeline-builder
                           :bootstrap-transform bootstrap-transform

src/aleph/tcp.clj

@@ -162,14 +162,16 @@
    | `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` or a map of SSL context options (see `aleph.netty/ssl-client-context` for more details) to use. Defers to `ssl?` and `insecure?` configuration if omitted.
+   | `ssl-endpoint-id-alg` | the name of the algorithm to use for SSL endpoint identification (see https://docs.oracle.com/en/java/javase/17/docs/specs/security/standard-names.html#endpoint-identification-algorithms), defaults to \"HTTPS\" which is a reasonable default for non-HTTPS uses, too. Only used by SSL connections. Pass `nil` to disable endpoint identification.
    | `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.
    | `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.
    | `transport`           | the transport to use, one of `:nio`, `:epoll`, `:kqueue` or `:io-uring` (defaults to `:nio`)."
-  [{:keys [host port remote-address local-address ssl-context ssl? insecure? pipeline-transform bootstrap-transform epoll? transport]
-    :or {bootstrap-transform identity
+  [{:keys [host port remote-address local-address ssl-context ssl-endpoint-id-alg ssl? insecure? pipeline-transform bootstrap-transform epoll? transport]
+    :or {ssl-endpoint-id-alg netty/default-ssl-endpoint-id-alg
+         bootstrap-transform identity
          epoll? false}
     :as options}]
   (let [[s ^ChannelHandler handler] (client-channel-handler options)
@@ -185,7 +187,7 @@
                            (when ssl?
                              (.addLast pipeline
                                        "ssl-handler"
-                                       (netty/ssl-handler (.channel pipeline) ssl-context remote-address)))
+                                       (netty/ssl-handler (.channel pipeline) ssl-context remote-address ssl-endpoint-id-alg)))
                            (.addLast pipeline "handler" handler)
                            (when pipeline-transform
                              (pipeline-transform pipeline)))]

test/aleph/http_test.clj

@@ -186,9 +186,16 @@
     (apply concat)
     (partition 2)))
 
+(def default-ssl-server-options
+  {:port port
+   :ssl-context ssl/server-ssl-context})
+
+(def default-ssl-client-options
+  {:ssl-context ssl/client-ssl-context-opts})
+
 (defmacro with-server [server & body]
   `(let [server# ~server]
-     (binding [*pool* (http/connection-pool {:connection-options (merge *connection-options* {:insecure? true})})]
+     (binding [*pool* (http/connection-pool {:connection-options *connection-options*})]
        (try
          ~@body
          (finally
@@ -207,7 +214,7 @@
      (with-server (http/start-server ~handler {:port port :compression-level 3 :shutdown-timeout 0})
        ~@body)))
 
-(def default-ssl-options {:port port, :ssl-context (netty/self-signed-ssl-context)})
+
 
 (defmacro with-handler-options
   [handler options & body]
@@ -247,14 +254,15 @@
 
 
 (deftest test-ssl-response-formats
-  (with-handler-options basic-handler default-ssl-options
-    (doseq [[path result] expected-results]
-      (is
-       (= result
-          (bs/to-string
-           (:body
-            @(http-get (str "https://localhost:" port "/" path)))))
-       (str path "path failed")))))
+  (binding [*connection-options* default-ssl-client-options]
+    (with-handler-options basic-handler default-ssl-server-options
+      (doseq [[path result] expected-results]
+        (is
+         (= result
+            (bs/to-string
+             (:body
+              @(http-get (str "https://localhost:" port "/" path)))))
+         (str path "path failed"))))))
 
 (deftest test-files
   (let [client-url (str "http://localhost:" port)]
@@ -269,49 +277,69 @@
                                 {:body (io/file "test/file.txt")}))))))))
 
 (deftest test-ssl-files
-  (let [client-url (str "https://localhost:" port)
-        client-options {:connection-options {:ssl-context ssl/client-ssl-context}}
-        client-pool    (http/connection-pool client-options)]
-    (with-handler-options identity (merge default-ssl-options {:ssl-context ssl/server-ssl-context})
-      (is (str/blank?
-           (bs/to-string
-            (:body @(http-put client-url
-                              {:body (io/file "test/empty.txt")
-                               :pool client-pool})))))
-      (is (= (slurp "test/file.txt" :encoding "UTF-8")
+  (binding [*connection-options* default-ssl-client-options]
+    (let [client-url (str "https://localhost:" port)]
+      (with-handler-options identity default-ssl-server-options
+        (is (str/blank?
              (bs/to-string
               (:body @(http-put client-url
-                                {:body (io/file "test/file.txt")
-                                 :pool client-pool}))))))))
+                                {:body (io/file "test/empty.txt")})))))
+        (is (= (slurp "test/file.txt" :encoding "UTF-8")
+               (bs/to-string
+                (:body @(http-put client-url
+                                  {:body (io/file "test/file.txt")})))))))))
 
 (defn ssl-session-capture-handler [ssl-session-atom]
   (fn [req]
     (reset! ssl-session-atom (http.core/ring-request-ssl-session req))
     {:status 200 :body "ok"}))
 
 (deftest test-ssl-session-access
-  (let [ssl-session (atom nil)]
-    (with-handler-options
-      (ssl-session-capture-handler ssl-session)
-      default-ssl-options
-      (is (= 200 (:status @(http-get (str "https://localhost:" port)))))
-      (is (some? @ssl-session))
-      (when-let [^SSLSession s @ssl-session]
-        (is (.isValid s))
-        (is (not (str/includes? "NULL" (.getCipherSuite s))))))))
+  (binding [*connection-options* default-ssl-client-options]
+    (let [ssl-session (atom nil)]
+      (with-handler-options
+        (ssl-session-capture-handler ssl-session)
+        default-ssl-server-options
+        (is (= 200 (:status @(http-get (str "https://localhost:" port)))))
+        (is (some? @ssl-session))
+        (when-let [^SSLSession s @ssl-session]
+          (is (.isValid s))
+          (is (not (str/includes? "NULL" (.getCipherSuite s)))))))))
 
 (deftest test-ssl-with-plain-client-request
-  (let [ssl-session (atom nil)]
-    (with-handler-options
-      (ssl-session-capture-handler ssl-session)
-      default-ssl-options
-      ;; Note the intentionally wrong "http" scheme here
-      (is (some-> (http-get (str "http://localhost:" port))
-                  (d/catch identity)
-                  deref
-                  ex-message
-                  (str/includes? "connection was closed")))
-      (is (nil? @ssl-session)))))
+  (binding [*connection-options* default-ssl-client-options]
+    (let [ssl-session (atom nil)]
+      (with-handler-options
+        (ssl-session-capture-handler ssl-session)
+        default-ssl-server-options
+        ;; Note the intentionally wrong "http" scheme here
+        (is (some-> (http-get (str "http://localhost:" port))
+                    (d/catch identity)
+                    deref
+                    ex-message
+                    (str/includes? "connection was closed")))
+        (is (nil? @ssl-session))))))
+
+(deftest test-ssl-endpoint-identification
+  (binding [*connection-options* {:ssl-context ssl/wrong-hostname-client-ssl-context-opts}]
+    (let [ssl-session (atom nil)]
+      (with-handler-options
+        (ssl-session-capture-handler ssl-session)
+        (assoc default-ssl-server-options :ssl-context ssl/wrong-hostname-server-ssl-context-opts)
+        (is (thrown-with-msg? javax.net.ssl.SSLHandshakeException
+                              #"^No name matching localhost found$"
+                              @(http-get (str "https://localhost:" port))))
+        (is (nil? @ssl-session))))))
+
+(deftest test-disabling-ssl-endpoint-identification
+  (binding [*connection-options* {:ssl-context ssl/wrong-hostname-client-ssl-context-opts
+                                  :ssl-endpoint-id-alg nil}]
+    (let [ssl-session (atom nil)]
+      (with-handler-options
+        (ssl-session-capture-handler ssl-session)
+        (assoc default-ssl-server-options :ssl-context ssl/wrong-hostname-server-ssl-context-opts)
+        (is (= 200 (:status @(http-get (str "https://localhost:" port)))))
+        (is (some? @ssl-session))))))
 
 (deftest test-invalid-body
   (let [client-url (str "http://localhost:" port)]

test/aleph/ssl.clj

@@ -8,6 +8,7 @@
       ApplicationProtocolConfig$SelectedListenerFailureBehavior
       ApplicationProtocolConfig$SelectorFailureBehavior
       ApplicationProtocolNames)
+    (io.netty.handler.ssl.util SelfSignedCertificate)
     (java.io ByteArrayInputStream)
     (java.security KeyFactory PrivateKey)
     (java.security.cert CertificateFactory X509Certificate)
@@ -75,3 +76,14 @@
 
 (def client-ssl-context
   (netty/ssl-client-context client-ssl-context-opts))
+
+(def wrong-hostname-cert
+  (SelfSignedCertificate. "some-random-hostname"))
+
+(def wrong-hostname-server-ssl-context-opts
+  {:private-key       (.privateKey wrong-hostname-cert)
+   :certificate-chain (.certificate wrong-hostname-cert)})
+
+(def wrong-hostname-client-ssl-context-opts
+  (assoc client-ssl-context-opts
+         :trust-store (.certificate wrong-hostname-cert)))