WX-808 Host allowlist for HTTP imports

core/src/main/resources/reference.conf

@@ -439,6 +439,10 @@ engine {
 languages {
   default: WDL
   WDL {
+    http-allow-list {
+      enabled: false
+      allowed-http-hosts: []
+    }
     versions {
       default: "draft-2"
       "draft-2" {

languageFactories/language-factory-core/src/main/scala/cromwell/languages/util/ImportResolver.scala

@@ -9,12 +9,14 @@ import cats.syntax.either._
 import cats.syntax.validated._
 import com.softwaremill.sttp._
 import com.softwaremill.sttp.asynchttpclient.cats.AsyncHttpClientCatsBackend
+import com.typesafe.config.ConfigFactory
 import common.Checked
 import common.transforms.CheckedAtoB
 import common.validation.ErrorOr._
 import common.validation.Checked._
 import common.validation.Validation._
 import cromwell.core.path.{DefaultPathBuilder, Path}
+import net.ceedubs.ficus.Ficus._
 
 import java.nio.file.{Path => NioPath}
 import java.security.MessageDigest
@@ -157,7 +159,9 @@ object ImportResolver {
     }
   }
 
-  case class HttpResolver(relativeTo: Option[String] = None, headers: Map[String, String] = Map.empty) extends ImportResolver {
+  case class HttpResolver(relativeTo: Option[String],
+                          headers: Map[String, String],
+                          hostAllowlist: Option[List[String]]) extends ImportResolver {
     import HttpResolver._
 
     override def name: String = relativeTo match {
@@ -169,7 +173,7 @@ object ImportResolver {
     def newResolverList(newRoot: String): List[ImportResolver] = {
       val rootWithoutFilename = newRoot.split('/').init.mkString("", "/", "/")
       List(
-        HttpResolver(relativeTo = Some(canonicalize(rootWithoutFilename)), headers)
+        HttpResolver(relativeTo = Some(canonicalize(rootWithoutFilename)), headers, hostAllowlist)
       )
     }
 
@@ -183,18 +187,20 @@ object ImportResolver {
         else "Relative path".invalidNelCheck
     }
 
+    def isAllowed(uri: Uri): Boolean = hostAllowlist match {
+      case Some(hosts) => hosts.contains(uri.host)
+      case None => true
+    }
+
     override def innerResolver(str: String, currentResolvers: List[ImportResolver]): Checked[ResolvedImportBundle] = {
-      pathToLookup(str) flatMap { toLookup =>
+      pathToLookup(str) flatMap { toLookup: WorkflowSource =>
         (Try {
-          implicit val sttpBackend = HttpResolver.sttpBackend()
-          val responseIO: IO[Response[String]] = sttp.get(uri"$toLookup").headers(headers).send()
-
-          // temporary situation to get functionality working before
-          // starting in on async-ifying the entire WdlNamespace flow
-          val result: Checked[String] = Await.result(responseIO.unsafeToFuture(), 15.seconds).body.leftMap { e => NonEmptyList(e.toString.trim, List.empty) }
+          val uri: Uri = uri"$toLookup"
 
-          result map {
-            ResolvedImportBundle(_, newResolverList(toLookup), ResolvedImportRecord(toLookup))
+          if (isAllowed(uri)) {
+            getUri(toLookup)
+          } else {
+            s"Disallowed domain in URI. ${uri.toString()}".invalidNelCheck
           }
         } match {
           case Success(result) => result
@@ -203,6 +209,19 @@ object ImportResolver {
       }
     }
 
+    private def getUri(toLookup: WorkflowSource): Either[NonEmptyList[WorkflowSource], ResolvedImportBundle] = {
+      implicit val sttpBackend = HttpResolver.sttpBackend()
+      val responseIO: IO[Response[WorkflowSource]] = sttp.get(uri"$toLookup").headers(headers).send()
+
+      // temporary situation to get functionality working before
+      // starting in on async-ifying the entire WdlNamespace flow
+      val result: Checked[WorkflowSource] = Await.result(responseIO.unsafeToFuture(), 15.seconds).body.leftMap { e => NonEmptyList(e.toString.trim, List.empty) }
+
+      result map {
+        ResolvedImportBundle(_, newResolverList(toLookup), ResolvedImportRecord(toLookup))
+      }
+    }
+
     override def cleanupIfNecessary(): ErrorOr[Unit] = ().validNel
 
     override def hashKey: ErrorOr[String] = relativeTo.toString.md5Sum.validNel
@@ -213,6 +232,18 @@ object ImportResolver {
     import common.util.IntrospectableLazy
     import common.util.IntrospectableLazy._
 
+    def apply(relativeTo: Option[String] = None,
+              headers: Map[String, String] = Map.empty): HttpResolver = {
+      val config = ConfigFactory.load().getConfig("languages.WDL.http-allow-list")
+      val allowListEnabled = config.as[Option[Boolean]]("enabled").getOrElse(false)
+      val allowList: Option[List[String]] =
+        if (allowListEnabled)
+          config.as[Option[List[String]]]("allowed-http-hosts")
+        else None
+
+      new HttpResolver(relativeTo, headers, allowList)
+    }
+
     val sttpBackend: IntrospectableLazy[SttpBackend[IO, Nothing]] = lazily {
       // 2.13 Beginning with sttp 1.6.x a `ContextShift` parameter is now required to construct an
       // `AsyncHttpClientCatsBackend`. There may be a more appropriate choice for backing this than the global

languageFactories/language-factory-core/src/test/scala/cromwell/languages/util/ImportResolverSpec.scala

@@ -1,7 +1,8 @@
 package cromwell.languages.util
 
-import java.nio.file.{Files, Paths}
+import com.softwaremill.sttp._
 
+import java.nio.file.{Files, Paths}
 import common.assertion.CromwellTimeoutSpec
 import common.assertion.ErrorOrAssertions._
 import cromwell.core.WorkflowId
@@ -41,13 +42,13 @@ class ImportResolverSpec extends AnyFlatSpec with CromwellTimeoutSpec with Match
   }
 
   it should "resolve a path from no initial root" in {
-    val resolver = HttpResolver()
+    val resolver = HttpResolver(None, Map.empty, None)
     val toResolve = resolver.pathToLookup("http://abc.com:8000/blah1/blah2.wdl")
     toResolve shouldBeValid "http://abc.com:8000/blah1/blah2.wdl"
   }
 
   it should "resolve a path and store the import in ResolvedImportRecord" in {
-    val resolver = HttpResolver()
+    val resolver = HttpResolver(None, Map.empty, None)
     val importUri = "https://raw.githubusercontent.com/broadinstitute/cromwell/develop/centaur/src/main/resources/standardTestCases/hello/hello.wdl"
     val resolvedBundle = resolver.innerResolver(importUri, List(resolver))
 
@@ -57,10 +58,42 @@ class ImportResolverSpec extends AnyFlatSpec with CromwellTimeoutSpec with Match
     }
   }
 
+  behavior of "HttpResolver with allowList"
+
+  val allowList = Option(List("my.favorite.wdls.com", "anotherwdlsite.org"))
+  val pathEnd = "bob/loblaw/blah/blah.wdl"
+
+  it should "allow any import when there is no allow list" in {
+    val resolver = HttpResolver(None, Map.empty, None)
+    resolver.isAllowed(uri"https://my.favorite.wdls.com/$pathEnd") shouldBe true
+    resolver.isAllowed(uri"http://some-garbage.whatever.eu/$pathEnd") shouldBe true
+    resolver.isAllowed(uri"localhost:8080/my/secrets") shouldBe true
+  }
+
+  it should "allow any import that's on the allow list" in {
+    val resolver = HttpResolver(None, Map.empty, allowList)
+    resolver.isAllowed(uri"https://my.favorite.wdls.com/$pathEnd") shouldBe true
+    resolver.isAllowed(uri"http://anotherwdlsite.org/$pathEnd") shouldBe true
+    resolver.isAllowed(uri"https://yetanotherwdlsite.org/$pathEnd") shouldBe false
+    resolver.isAllowed(uri"https://FOO.my.favorite.wdls.com/$pathEnd") shouldBe false
+    resolver.isAllowed(uri"https://wdls.com/$pathEnd") shouldBe false
+    resolver.isAllowed(uri"localhost:8080/my/secrets") shouldBe false
+  }
+
+  it should "allow nothing with an empty allow list" in {
+    val resolver = HttpResolver(None, Map.empty, Option(List.empty))
+    resolver.isAllowed(uri"https://my.favorite.wdls.com/$pathEnd") shouldBe false
+    resolver.isAllowed(uri"http://anotherwdlsite.org/$pathEnd") shouldBe false
+    resolver.isAllowed(uri"https://yetanotherwdlsite.org/$pathEnd") shouldBe false
+    resolver.isAllowed(uri"https://FOO.my.favorite.wdls.com/$pathEnd") shouldBe false
+    resolver.isAllowed(uri"https://wdls.com/$pathEnd") shouldBe false
+    resolver.isAllowed(uri"localhost:8080/my/secrets") shouldBe false
+  }
+
   behavior of "HttpResolver with a 'relativeTo' value"
 
-  val relativeHttpResolver = HttpResolver(relativeTo = Some("http://abc.com:8000/blah1/blah2/"))
-  val relativeToGithubHttpResolver = HttpResolver(relativeTo = Some(relativeToGithubRoot))
+  val relativeHttpResolver = HttpResolver(relativeTo = Some("http://abc.com:8000/blah1/blah2/"), Map.empty, None)
+  val relativeToGithubHttpResolver = HttpResolver(relativeTo = Some(relativeToGithubRoot), Map.empty, None)
 
   it should "resolve an absolute path from a different initial root" in {
     val pathToLookup = relativeHttpResolver.pathToLookup("http://def.org:8080/blah3.wdl")

languageFactories/wdl-draft2/src/test/scala/languages.wdl.draft2/NamespaceCacheSpec.scala

@@ -61,7 +61,7 @@ class NamespaceCacheSpec extends AnyFlatSpec with CromwellTimeoutSpec with Befor
     )
 
     var lookupCount = 0
-    val countingResolver = new HttpResolver() {
+    val countingResolver = new HttpResolver(None, Map.empty, None) {
       override def pathToLookup(str: String): Checked[String] = {
         lookupCount = lookupCount + 1
         super.pathToLookup(str)