[GitHub] [openwhisk] vrann commented on a change in pull request #5065: Upgrade to Akka 2.6.11

[GitBox <gi...@apache.org>]

vrann commented on a change in pull request #5065:
URL: https://github.com/apache/openwhisk/pull/5065#discussion_r586984919



##########
File path: common/scala/src/main/scala/org/apache/openwhisk/common/Https.scala
##########
@@ -35,8 +33,16 @@ object Https {
     cs
   }
 
-  def connectionContext(httpsConfig: HttpsConfig, sslConfig: Option[AkkaSSLConfig] = None) = {
+  def httpsInsecureClient(context: SSLContext): HttpsConnectionContext =
+    ConnectionContext.httpsClient((host, port) => {
+      val engine = context.createSSLEngine(host, port)
+      engine.setUseClientMode(true)
+      // WARNING: this creates an SSL Engine without enabling endpoint identification/verification procedures
+      // Disabling host name verification is a very bad idea, please don't unless you have a very good reason to.

Review comment:
       This comment and the implementation is taken from the akka 2.6.x documentation on how to disable host name verification for ssl. This is what openwhisk was doing in couple of places including tests, but having the old SSLConfig class deprecated it had to be re-implemented. So I figured it, I'd rather keep comment to discourage using this approach anywhere in production.
   
   Do you suggest adding more explanation on why is it needed to the code? Sure, I can do that.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org