From 58348eb56dd2a3eb5c88e0724703949c9f04df7b Mon Sep 17 00:00:00 2001 From: geoffjentry Date: Tue, 11 Jul 2017 13:31:20 -0400 Subject: [PATCH 1/3] Add restrict-metadata-access --- core/src/main/resources/reference.conf | 4 ++ .../cromwell/backend/impl/jes/JesAttributes.scala | 10 ++- .../backend/impl/jes/JesConfiguration.scala | 2 +- .../backend/impl/jes/JesInitializationActor.scala | 37 ++++++----- .../jes/authentication/JesVMAuthentication.scala | 8 +-- .../backend/impl/jes/JesAttributesSpec.scala | 9 +++ .../impl/jes/JesInitializationActorSpec.scala | 73 +++++++++++++++++++--- 7 files changed, 114 insertions(+), 29 deletions(-) diff --git a/core/src/main/resources/reference.conf b/core/src/main/resources/reference.conf index 5439efbdf..ece979929 100644 --- a/core/src/main/resources/reference.conf +++ b/core/src/main/resources/reference.conf @@ -498,6 +498,10 @@ backend { # # # Endpoint for APIs, no reason to change this unless directed by Google. # endpoint-url = "https://genomics.googleapis.com/" + # + # # Restrict access to VM metadata. Useful in cases when untrusted containers are running under a service + # # account not owned by the submitting user + # restrict-metadata-access = false # } # # filesystems { diff --git a/supportedBackends/jes/src/main/scala/cromwell/backend/impl/jes/JesAttributes.scala b/supportedBackends/jes/src/main/scala/cromwell/backend/impl/jes/JesAttributes.scala index 3bd1a2c04..73d681812 100644 --- a/supportedBackends/jes/src/main/scala/cromwell/backend/impl/jes/JesAttributes.scala +++ b/supportedBackends/jes/src/main/scala/cromwell/backend/impl/jes/JesAttributes.scala @@ -23,6 +23,7 @@ import scala.collection.JavaConverters._ case class JesAttributes(project: String, computeServiceAccount: String, auths: JesAuths, + restrictMetadataAccess: Boolean, executionBucket: String, endpointUrl: URL, maxPollingInterval: Int, @@ -45,6 +46,7 @@ object JesAttributes { "genomics", "filesystems", "genomics.auth", + "genomics.restrict-metadata-access", "genomics.endpoint-url", "filesystems.gcs.auth", "filesystems.gcs.caching.duplication-strategy", @@ -76,6 +78,7 @@ object JesAttributes { val maxPollingInterval: Int = backendConfig.as[Option[Int]]("maximum-polling-interval").getOrElse(600) val computeServiceAccount: String = backendConfig.as[Option[String]]("genomics.compute-service-account").getOrElse("default") val genomicsAuthName: ErrorOr[String] = validate { backendConfig.as[String]("genomics.auth") } + val genomicsRestrictMetadataAccess: ErrorOr[Boolean] = validate { backendConfig.as[Option[Boolean]]("genomics.restrict-metadata-access").getOrElse(false) } val gcsFilesystemAuthName: ErrorOr[String] = validate { backendConfig.as[String]("filesystems.gcs.auth") } val qpsValidation = validateQps(backendConfig) val duplicationStrategy = validate { backendConfig.as[Option[String]]("filesystems.gcs.caching.duplication-strategy").getOrElse("copy") match { @@ -85,10 +88,11 @@ object JesAttributes { } } - (project |@| executionBucket |@| endpointUrl |@| genomicsAuthName |@| gcsFilesystemAuthName |@| qpsValidation |@| duplicationStrategy).tupled flatMap { - case (p, b, u, genomicsName, gcsName, qps, cachingStrategy) => + (project |@| executionBucket |@| endpointUrl |@| genomicsAuthName |@| genomicsRestrictMetadataAccess |@| gcsFilesystemAuthName |@| + qpsValidation |@| duplicationStrategy).tupled flatMap { + case (p, b, u, genomicsName, restrictMetadata, gcsName, qps, cachingStrategy) => (googleConfig.auth(genomicsName) |@| googleConfig.auth(gcsName)) map { case (genomicsAuth, gcsAuth) => - JesAttributes(p, computeServiceAccount, JesAuths(genomicsAuth, gcsAuth), b, u, maxPollingInterval, qps, cachingStrategy) + JesAttributes(p, computeServiceAccount, JesAuths(genomicsAuth, gcsAuth), restrictMetadata, b, u, maxPollingInterval, qps, cachingStrategy) } } match { case Valid(r) => r diff --git a/supportedBackends/jes/src/main/scala/cromwell/backend/impl/jes/JesConfiguration.scala b/supportedBackends/jes/src/main/scala/cromwell/backend/impl/jes/JesConfiguration.scala index f92a5f818..f0e863da6 100644 --- a/supportedBackends/jes/src/main/scala/cromwell/backend/impl/jes/JesConfiguration.scala +++ b/supportedBackends/jes/src/main/scala/cromwell/backend/impl/jes/JesConfiguration.scala @@ -17,6 +17,6 @@ class JesConfiguration(val configurationDescriptor: BackendConfigurationDescript val gcsPathBuilderFactory = GcsPathBuilderFactory(jesAuths.gcs, googleConfig.applicationName) val genomicsFactory = GenomicsFactory(googleConfig.applicationName, jesAuths.genomics, jesAttributes.endpointUrl) val dockerCredentials = BackendDockerConfiguration.build(configurationDescriptor.backendConfig).dockerCredentials map JesDockerCredentials.apply - val needAuthFileUpload = jesAuths.gcs.requiresAuthFile || dockerCredentials.isDefined + val needAuthFileUpload = jesAuths.gcs.requiresAuthFile || dockerCredentials.isDefined || jesAttributes.restrictMetadataAccess val qps = jesAttributes.qps } diff --git a/supportedBackends/jes/src/main/scala/cromwell/backend/impl/jes/JesInitializationActor.scala b/supportedBackends/jes/src/main/scala/cromwell/backend/impl/jes/JesInitializationActor.scala index 890bea568..b5097cc7a 100644 --- a/supportedBackends/jes/src/main/scala/cromwell/backend/impl/jes/JesInitializationActor.scala +++ b/supportedBackends/jes/src/main/scala/cromwell/backend/impl/jes/JesInitializationActor.scala @@ -6,13 +6,13 @@ import akka.actor.ActorRef import com.google.api.services.genomics.Genomics import com.google.auth.Credentials import com.google.cloud.storage.contrib.nio.CloudStorageOptions -import cromwell.backend.impl.jes.authentication.{GcsLocalizing, JesAuthInformation, JesDockerCredentials} +import cromwell.backend.impl.jes.authentication.{GcsLocalizing, JesAuthObject, JesDockerCredentials} import cromwell.backend.standard.{StandardInitializationActor, StandardInitializationActorParams, StandardValidatedRuntimeAttributesBuilder} import cromwell.backend.{BackendConfigurationDescriptor, BackendInitializationData, BackendWorkflowDescriptor} import cromwell.core.io.AsyncIo import cromwell.filesystems.gcs.auth.{ClientSecrets, GoogleAuthMode} import cromwell.filesystems.gcs.batch.GcsBatchCommandBuilder -import spray.json.JsObject +import spray.json.{JsObject, JsTrue} import wdl4s.TaskCall import scala.concurrent.Future @@ -43,7 +43,7 @@ class JesInitializationActor(jesParams: JesInitializationActorParams) // From the gcs auth and the workflow options, optionally builds a GcsLocalizing that contains // the information (client Id/Secrets + refresh token) that will be uploaded to Gcs before the workflow start - private[jes] lazy val refreshTokenAuth: Option[JesAuthInformation] = { + private[jes] lazy val refreshTokenAuth: Option[JesAuthObject] = { for { clientSecrets <- List(jesConfiguration.jesAttributes.auths.gcs) collectFirst { case s: ClientSecrets => s } token <- workflowDescriptor.workflowOptions.get(GoogleAuthMode.RefreshTokenOptionKey).toOption @@ -75,8 +75,8 @@ class JesInitializationActor(jesParams: JesInitializationActorParams) } yield JesBackendInitializationData(jesWorkflowPaths, runtimeAttributesBuilder, jesConfiguration, gcsCreds, genomicsFactory) override def beforeAll(): Future[Option[BackendInitializationData]] = { - def fileUpload(paths: JesWorkflowPaths, dockerCredentials: Option[JesDockerCredentials]): Future[Unit] = { - writeAuthenticationFile(paths, dockerCredentials) recoverWith { + def fileUpload(paths: JesWorkflowPaths): Future[Unit] = { + writeAuthenticationFile(paths, jesConfiguration.jesAttributes.restrictMetadataAccess, jesConfiguration.dockerCredentials) recoverWith { case failure => Future.failed(new IOException("Failed to upload authentication file", failure)) } } @@ -84,25 +84,34 @@ class JesInitializationActor(jesParams: JesInitializationActorParams) for { paths <- workflowPaths _ = publishWorkflowRoot(paths.workflowRoot.pathAsString) - _ <- if (jesConfiguration.needAuthFileUpload) fileUpload(paths, jesConfiguration.dockerCredentials) else Future.successful(()) + _ <- if (jesConfiguration.needAuthFileUpload) fileUpload(paths) else Future.successful(()) data <- initializationData } yield Option(data) } - private def writeAuthenticationFile(workflowPath: JesWorkflowPaths, dockerCredentials: Option[JesDockerCredentials]): Future[Unit] = { - generateAuthJson(dockerCredentials, refreshTokenAuth) map { content => + private def writeAuthenticationFile(workflowPath: JesWorkflowPaths, + restrictMetadataAccess: Boolean, + dockerCredentials: Option[JesDockerCredentials]): Future[Unit] = { + val authObjects = List(dockerCredentials, refreshTokenAuth).flatten + generateAuthJson(authObjects, restrictMetadataAccess) map { content => val path = workflowPath.gcsAuthFilePath workflowLogger.info(s"Creating authentication file for workflow ${workflowDescriptor.id} at \n $path") writeAsync(path, content, Seq(CloudStorageOptions.withMimeType("application/json"))) } getOrElse Future.successful(()) } - def generateAuthJson(authInformation: Option[JesAuthInformation]*): Option[String] = { - authInformation.flatten map { _.toMap } match { - case Nil => None - case jsons => - val authsValues = jsons.reduce(_ ++ _) mapValues JsObject.apply - Option(JsObject("auths" -> JsObject(authsValues)).prettyPrint) + def generateAuthJson(authObjects: List[JesAuthObject], restrictMetadataAccess: Boolean): Option[String] = { + def generateAuthObject(): Map[String, JsObject] = { + if (authObjects.nonEmpty) { + val authObjectMaps = authObjects map { _.toMap } + Map("auths" -> JsObject(authObjectMaps.reduce(_ ++ _) map { case (k, v) => k -> JsObject.apply(v) })) + } else Map.empty } + + val authMap = generateAuthObject() + val jsonMap = if (restrictMetadataAccess) authMap ++ Map("restrictMetadataAccess" -> JsTrue) else authMap + + if (jsonMap.nonEmpty) Option(JsObject(jsonMap).prettyPrint) + else None } } diff --git a/supportedBackends/jes/src/main/scala/cromwell/backend/impl/jes/authentication/JesVMAuthentication.scala b/supportedBackends/jes/src/main/scala/cromwell/backend/impl/jes/authentication/JesVMAuthentication.scala index 9c92b380a..0f6a259a3 100644 --- a/supportedBackends/jes/src/main/scala/cromwell/backend/impl/jes/authentication/JesVMAuthentication.scala +++ b/supportedBackends/jes/src/main/scala/cromwell/backend/impl/jes/authentication/JesVMAuthentication.scala @@ -5,10 +5,10 @@ import cromwell.filesystems.gcs.auth.ClientSecrets import spray.json.{JsString, JsValue} /** - * Interface for Authentication information that can be included in the json file uploaded to GCS + * Interface for Authentication information that can be included as a json object in the file uploaded to GCS * upon workflow creation and used in the VM. */ -sealed trait JesAuthInformation { +sealed trait JesAuthObject { def context: String def map: Map[String, JsValue] @@ -18,7 +18,7 @@ sealed trait JesAuthInformation { /** * Authentication information for data (de)localization as the user. */ -case class GcsLocalizing(clientSecrets: ClientSecrets, token: String) extends JesAuthInformation { +case class GcsLocalizing(clientSecrets: ClientSecrets, token: String) extends JesAuthObject { override val context = "boto" override val map = Map( "client_id" -> JsString(clientSecrets.clientId), @@ -35,7 +35,7 @@ object JesDockerCredentials { /** * Authentication information to pull docker images as the user. */ -class JesDockerCredentials(account: String, token: String) extends DockerCredentials(account, token) with JesAuthInformation { +class JesDockerCredentials(account: String, token: String) extends DockerCredentials(account, token) with JesAuthObject { override val context = "docker" override val map = Map( "account" -> JsString(account), diff --git a/supportedBackends/jes/src/test/scala/cromwell/backend/impl/jes/JesAttributesSpec.scala b/supportedBackends/jes/src/test/scala/cromwell/backend/impl/jes/JesAttributesSpec.scala index 1145cddbc..034faa8ca 100644 --- a/supportedBackends/jes/src/test/scala/cromwell/backend/impl/jes/JesAttributesSpec.scala +++ b/supportedBackends/jes/src/test/scala/cromwell/backend/impl/jes/JesAttributesSpec.scala @@ -27,6 +27,7 @@ class JesAttributesSpec extends FlatSpec with Matchers { jesAttributes.executionBucket should be("gs://myBucket") jesAttributes.maxPollingInterval should be(600) jesAttributes.computeServiceAccount should be("default") + jesAttributes.restrictMetadataAccess should be(false) } it should "parse correct preemptible config" taggedAs IntegrationTest in { @@ -46,6 +47,14 @@ class JesAttributesSpec extends FlatSpec with Matchers { jesAttributes.computeServiceAccount should be("testing") } + it should "parse restrict-metadata-access" taggedAs IntegrationTest in { + val backendConfig = ConfigFactory.parseString(configString(genomics = """restrict-metadata-access = true """)) + + val jesAttributes = JesAttributes(googleConfig, backendConfig) + jesAttributes.restrictMetadataAccess should be(true) + + } + it should "not parse invalid config" taggedAs IntegrationTest in { val nakedConfig = ConfigFactory.parseString( diff --git a/supportedBackends/jes/src/test/scala/cromwell/backend/impl/jes/JesInitializationActorSpec.scala b/supportedBackends/jes/src/test/scala/cromwell/backend/impl/jes/JesInitializationActorSpec.scala index d9970d368..39007ae70 100644 --- a/supportedBackends/jes/src/test/scala/cromwell/backend/impl/jes/JesInitializationActorSpec.scala +++ b/supportedBackends/jes/src/test/scala/cromwell/backend/impl/jes/JesInitializationActorSpec.scala @@ -7,7 +7,7 @@ import akka.testkit._ import com.typesafe.config.{Config, ConfigFactory} import cromwell.backend.BackendWorkflowInitializationActor.{InitializationFailed, InitializationSuccess, Initialize} import cromwell.backend.async.RuntimeAttributeValidationFailures -import cromwell.backend.impl.jes.authentication.GcsLocalizing +import cromwell.backend.impl.jes.authentication.{GcsLocalizing, JesAuthObject} import cromwell.backend.{BackendConfigurationDescriptor, BackendSpec, BackendWorkflowDescriptor} import cromwell.core.Dispatcher.BackendDispatcher import cromwell.core.Tags.IntegrationTest @@ -28,6 +28,7 @@ class JesInitializationActorSpec extends TestKitSuite("JesInitializationActorSpe val Timeout: FiniteDuration = 5.second.dilated import BackendSpec._ + import JesInitializationActorSpec._ val HelloWorld: String = s""" @@ -249,9 +250,9 @@ class JesInitializationActorSpec extends TestKitSuite("JesInitializationActorSpe val TestingBits(actorRef, _) = buildJesInitializationTestingBits() val actor = actorRef.underlyingActor - actor.generateAuthJson(None, None) should be(empty) + actor.generateAuthJson(flattenAuthOptions(None, None), false) should be(empty) - val authJsonOption = actor.generateAuthJson(None, None) + val authJsonOption = actor.generateAuthJson(flattenAuthOptions(None, None), false) authJsonOption should be(empty) actorRef.stop() @@ -263,7 +264,7 @@ class JesInitializationActorSpec extends TestKitSuite("JesInitializationActorSpe val TestingBits(actorRef, jesConfiguration) = buildJesInitializationTestingBits() val actor = actorRef.underlyingActor - val authJsonOption = actor.generateAuthJson(jesConfiguration.dockerCredentials, None) + val authJsonOption = actor.generateAuthJson(flattenAuthOptions(jesConfiguration.dockerCredentials, None), false) authJsonOption shouldNot be(empty) authJsonOption.get should be( normalize( @@ -289,7 +290,7 @@ class JesInitializationActorSpec extends TestKitSuite("JesInitializationActorSpe val actor = actorRef.underlyingActor val gcsUserAuth = Option(GcsLocalizing(SimpleClientSecrets("myclientid", "myclientsecret"), "mytoken")) - val authJsonOption = actor.generateAuthJson(None, gcsUserAuth) + val authJsonOption = actor.generateAuthJson(flattenAuthOptions(None, gcsUserAuth), false) authJsonOption shouldNot be(empty) authJsonOption.get should be( normalize( @@ -316,7 +317,7 @@ class JesInitializationActorSpec extends TestKitSuite("JesInitializationActorSpe val actor = actorRef.underlyingActor val gcsUserAuth = Option(GcsLocalizing(SimpleClientSecrets("myclientid", "myclientsecret"), "mytoken")) - val authJsonOption = actor.generateAuthJson(jesConfiguration.dockerCredentials, gcsUserAuth) + val authJsonOption = actor.generateAuthJson(flattenAuthOptions(jesConfiguration.dockerCredentials, gcsUserAuth), false) authJsonOption shouldNot be(empty) authJsonOption.get should be( normalize( @@ -340,7 +341,65 @@ class JesInitializationActorSpec extends TestKitSuite("JesInitializationActorSpe actorRef.stop() } - private def normalize(str: String) = { + it should "generate the correct json content for a docker token, a refresh token, and restrictMetadataAccess" in { + EncryptionSpec.assumeAes256Cbc() + + val TestingBits(actorRef, jesConfiguration) = buildJesInitializationTestingBits() + val actor = actorRef.underlyingActor + + val gcsUserAuth = Option(GcsLocalizing(SimpleClientSecrets("myclientid", "myclientsecret"), "mytoken")) + val authJsonOption = actor.generateAuthJson(flattenAuthOptions(jesConfiguration.dockerCredentials, gcsUserAuth), true) + authJsonOption shouldNot be(empty) + authJsonOption.get should be( + normalize( + """ + |{ + | "auths": { + | "docker": { + | "account": "my@docker.account", + | "token": "mydockertoken" + | }, + | "boto": { + | "client_id": "myclientid", + | "client_secret": "myclientsecret", + | "refresh_token": "mytoken" + | } + | }, + | "restrictMetadataAccess": true + |} + """.stripMargin) + ) + + actorRef.stop() + } + + it should "generate the correct json content for just restrictMetadataAccess" in { + EncryptionSpec.assumeAes256Cbc() + + val TestingBits(actorRef, jesConfiguration) = buildJesInitializationTestingBits() + val actor = actorRef.underlyingActor + + val authJsonOption = actor.generateAuthJson(flattenAuthOptions(None, None), true) + authJsonOption shouldNot be(empty) + authJsonOption.get should be( + normalize( + """ + |{ + | "restrictMetadataAccess": true + |} + """.stripMargin) + ) + + actorRef.stop() + } +} + +object JesInitializationActorSpec { + def normalize(str: String) = { str.parseJson.prettyPrint } + + def flattenAuthOptions(options: Option[JesAuthObject]*): List[JesAuthObject] = { + options.toList.flatten + } } From e5da338db807eb8be56380839b84ea5852ccb052 Mon Sep 17 00:00:00 2001 From: geoffjentry Date: Tue, 11 Jul 2017 15:42:05 -0400 Subject: [PATCH 2/3] oops --- .../scala/cromwell/backend/impl/jes/JesInitializationActorSpec.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/supportedBackends/jes/src/test/scala/cromwell/backend/impl/jes/JesInitializationActorSpec.scala b/supportedBackends/jes/src/test/scala/cromwell/backend/impl/jes/JesInitializationActorSpec.scala index 39007ae70..2b879c97f 100644 --- a/supportedBackends/jes/src/test/scala/cromwell/backend/impl/jes/JesInitializationActorSpec.scala +++ b/supportedBackends/jes/src/test/scala/cromwell/backend/impl/jes/JesInitializationActorSpec.scala @@ -376,7 +376,7 @@ class JesInitializationActorSpec extends TestKitSuite("JesInitializationActorSpe it should "generate the correct json content for just restrictMetadataAccess" in { EncryptionSpec.assumeAes256Cbc() - val TestingBits(actorRef, jesConfiguration) = buildJesInitializationTestingBits() + val TestingBits(actorRef, _) = buildJesInitializationTestingBits() val actor = actorRef.underlyingActor val authJsonOption = actor.generateAuthJson(flattenAuthOptions(None, None), true) From 9c1d6f71719e4c608a6dda4c0d019f468e652709 Mon Sep 17 00:00:00 2001 From: geoffjentry Date: Wed, 12 Jul 2017 13:58:30 -0400 Subject: [PATCH 3/3] code review --- .../src/test/scala/cromwell/backend/impl/jes/JesAttributesSpec.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/supportedBackends/jes/src/test/scala/cromwell/backend/impl/jes/JesAttributesSpec.scala b/supportedBackends/jes/src/test/scala/cromwell/backend/impl/jes/JesAttributesSpec.scala index 034faa8ca..695305781 100644 --- a/supportedBackends/jes/src/test/scala/cromwell/backend/impl/jes/JesAttributesSpec.scala +++ b/supportedBackends/jes/src/test/scala/cromwell/backend/impl/jes/JesAttributesSpec.scala @@ -48,7 +48,7 @@ class JesAttributesSpec extends FlatSpec with Matchers { } it should "parse restrict-metadata-access" taggedAs IntegrationTest in { - val backendConfig = ConfigFactory.parseString(configString(genomics = """restrict-metadata-access = true """)) + val backendConfig = ConfigFactory.parseString(configString(genomics = "restrict-metadata-access = true")) val jesAttributes = JesAttributes(googleConfig, backendConfig) jesAttributes.restrictMetadataAccess should be(true)