diff --git a/core/src/main/resources/reference.conf b/core/src/main/resources/reference.conf index c58eda5bc9f..8034409dbf5 100644 --- a/core/src/main/resources/reference.conf +++ b/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" { diff --git a/languageFactories/language-factory-core/src/main/scala/cromwell/languages/util/ImportResolver.scala b/languageFactories/language-factory-core/src/main/scala/cromwell/languages/util/ImportResolver.scala index 01c5db23084..f7fd70cfc56 100644 --- a/languageFactories/language-factory-core/src/main/scala/cromwell/languages/util/ImportResolver.scala +++ b/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 diff --git a/languageFactories/language-factory-core/src/test/scala/cromwell/languages/util/ImportResolverSpec.scala b/languageFactories/language-factory-core/src/test/scala/cromwell/languages/util/ImportResolverSpec.scala index 26249b02da1..813a688a573 100644 --- a/languageFactories/language-factory-core/src/test/scala/cromwell/languages/util/ImportResolverSpec.scala +++ b/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") diff --git a/languageFactories/wdl-draft2/src/test/scala/languages.wdl.draft2/NamespaceCacheSpec.scala b/languageFactories/wdl-draft2/src/test/scala/languages.wdl.draft2/NamespaceCacheSpec.scala index df87f2fa644..b96cfa5c2f6 100644 --- a/languageFactories/wdl-draft2/src/test/scala/languages.wdl.draft2/NamespaceCacheSpec.scala +++ b/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)