From 34d4fa002c738c7a6ec1fdab4e12e748c8967f92 Mon Sep 17 00:00:00 2001
From: Peter Amstutz <pamstutz@veritasgenetics.com>
Date: Fri, 9 Mar 2018 16:57:42 -0500
Subject: [PATCH] 13135: Handle secrets in file literals.

Also check for secrets leaking into command line or environment, and
fail if detected.

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz@veritasgenetics.com>
---
 sdk/cwl/arvados_cwl/__init__.py     |  2 +-
 sdk/cwl/arvados_cwl/arvcontainer.py | 27 ++++++++++++++++++++++-----
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/sdk/cwl/arvados_cwl/__init__.py b/sdk/cwl/arvados_cwl/__init__.py
index 5b8eac59c8..5c5f5502d0 100644
--- a/sdk/cwl/arvados_cwl/__init__.py
+++ b/sdk/cwl/arvados_cwl/__init__.py
@@ -354,7 +354,7 @@ class ArvCwlRunner(object):
         make_fs_access = kwargs.get("make_fs_access") or partial(CollectionFsAccess,
                                                                  collection_cache=self.collection_cache)
         self.fs_access = make_fs_access(kwargs["basedir"])
-
+        self.secret_store = kwargs.get("secret_store")
 
         self.trash_intermediate = kwargs["trash_intermediate"]
         if self.trash_intermediate and self.work_api != "containers":
diff --git a/sdk/cwl/arvados_cwl/arvcontainer.py b/sdk/cwl/arvados_cwl/arvcontainer.py
index eeadfbdc9e..c90e79e4ac 100644
--- a/sdk/cwl/arvados_cwl/arvcontainer.py
+++ b/sdk/cwl/arvados_cwl/arvcontainer.py
@@ -54,6 +54,12 @@ class ArvadosContainer(object):
         }
         runtime_constraints = {}
 
+        if self.arvrunner.secret_store.has_secret(self.command_line):
+            raise WorkflowException("Secret material leaked on command line, only file literals may contain secrets")
+
+        if self.arvrunner.secret_store.has_secret(self.environment):
+            raise WorkflowException("Secret material leaked in environment, only file literals may contain secrets")
+
         resources = self.builder.resources
         if resources is not None:
             runtime_constraints["vcpus"] = resources.get("cores", 1)
@@ -69,6 +75,7 @@ class ArvadosContainer(object):
                 "capacity": resources.get("tmpdirSize", 0) * 2**20
             }
         }
+        secret_mounts = {}
         scheduling_parameters = {}
 
         rf = [self.pathmapper.mapper(f) for f in self.pathmapper.referenced_files]
@@ -119,8 +126,14 @@ class ArvadosContainer(object):
                                 source, path = self.arvrunner.fs_access.get_collection(p.resolved)
                                 vwd.copy(path, p.target, source_collection=source)
                         elif p.type == "CreateFile":
-                            with vwd.open(p.target, "w") as n:
-                                n.write(p.resolved.encode("utf-8"))
+                            if self.arvrunner.secret_store.has_secret(p.resolved):
+                                secret_mounts["%s/%s" % (self.outdir, p.target)] = {
+                                    "kind": "text",
+                                    "content": self.arvrunner.secret_store.retrieve(p.resolved)
+                                }
+                            else:
+                                with vwd.open(p.target, "w") as n:
+                                    n.write(p.resolved.encode("utf-8"))
 
                 def keepemptydirs(p):
                     if isinstance(p, arvados.collection.RichCollectionBase):
@@ -136,7 +149,7 @@ class ArvadosContainer(object):
                     vwd.save_new()
 
                 for f, p in generatemapper.items():
-                    if not p.target:
+                    if not p.target or self.arvrunner.secret_store.has_secret(p.resolved):
                         continue
                     mountpoint = "%s/%s" % (self.outdir, p.target)
                     mounts[mountpoint] = {"kind": "collection",
@@ -201,10 +214,14 @@ class ArvadosContainer(object):
             self.output_ttl = self.arvrunner.intermediate_output_ttl
 
         if self.output_ttl < 0:
-            raise WorkflowError("Invalid value %d for output_ttl, cannot be less than zero" % container_request["output_ttl"])
+            raise WorkflowException("Invalid value %d for output_ttl, cannot be less than zero" % container_request["output_ttl"])
+
+        # for testing only!
+        mounts.update(secret_mounts)
 
         container_request["output_ttl"] = self.output_ttl
         container_request["mounts"] = mounts
+        container_request["secret_mounts"] = secret_mounts
         container_request["runtime_constraints"] = runtime_constraints
         container_request["scheduling_parameters"] = scheduling_parameters
 
@@ -311,11 +328,11 @@ class RunnerContainer(Runner):
         for param in sorted(self.job_order.keys()):
             if self.secret_store.has_secret(self.job_order[param]):
                 mnt = "/secrets/s%d" % len(secret_mounts)
-                self.job_order[param] = {"$include": mnt}
                 secret_mounts[mnt] = {
                     "kind": "text",
                     "content": self.secret_store.retrieve(self.job_order[param])
                 }
+                self.job_order[param] = {"$include": mnt}
 
         container_req = {
             "owner_uuid": self.arvrunner.project_uuid,
-- 
2.30.2