From 78d20099fd39d3f3834ce7e6349d05a3b0b1dbee Mon Sep 17 00:00:00 2001
From: Rodric Rabbah <rodric@gmail.com>
Date: Sun, 8 Mar 2020 12:09:19 -0400
Subject: [PATCH 01/12] Review notes and refactoring. No intended semantic
 change.

---
 .../openwhisk/core/entity/Parameter.scala     | 56 ++++++++-----------
 .../core/entity/ParameterEncryption.scala     | 26 +++++----
 .../test/ParameterEncryptionTests.scala       | 32 +++++------
 3 files changed, 52 insertions(+), 62 deletions(-)

diff --git a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala
index 89494fd7351..27489c016b0 100644
--- a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala
+++ b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala
@@ -46,13 +46,6 @@ protected[core] class Parameters protected[entity] (private val params: Map[Para
       .foldLeft(0 B)(_ + _)
   }
 
-  protected[entity] def +(p: (ParameterName, ParameterValue)) = {
-
-    Option(p) map { p =>
-      new Parameters(params + (p._1 -> p._2))
-    } getOrElse this
-  }
-
   protected[entity] def +(p: ParameterName, v: ParameterValue) = {
     new Parameters(params + (p -> v))
   }
@@ -86,26 +79,20 @@ protected[core] class Parameters protected[entity] (private val params: Map[Para
   }
   protected[core] def toJsArray = {
     JsArray(params map { p =>
-      val init = p._2.init match {
-        case true => Some("init" -> p._2.init.toJson)
-        case _    => None
-      }
-      val encrypt = p._2.encryption match {
-        case (JsNull) => None
-        case _        => Some("encryption" -> p._2.encryption)
-      }
+      val init = if (p._2.init) Some("init" -> JsTrue) else None
+      val encrypt = p._2.encryption.map(e => ("encryption" -> JsString(e)))
+
       // Have do use this slightly strange construction to get the json object order identical.
-      JsObject(ListMap() ++ encrypt ++ init ++ Map("key" -> p._1.name.toJson, "value" -> p._2.value.toJson))
+      JsObject(ListMap.empty ++ encrypt ++ init ++ Map("key" -> p._1.name.toJson, "value" -> p._2.value))
     } toSeq: _*)
   }
 
   protected[core] def toJsObject =
     JsObject(params.map(p => {
       val newValue =
-        if (p._2.encryption == JsNull)
-          p._2.value.toJson
-        else
-          JsObject("value" -> p._2.value.toJson, "encryption" -> p._2.encryption, "init" -> p._2.init.toJson)
+        p._2.encryption
+          .map(e => JsObject("value" -> p._2.value, "init" -> p._2.init.toJson, "encryption" -> e.toJson))
+          .getOrElse(p._2.value)
       (p._1.name, newValue)
     }))
 
@@ -175,11 +162,11 @@ protected[entity] class ParameterName protected[entity] (val name: String) exten
  *
  * @param v the value of the parameter, may be null
  * @param init if true, this parameter value is only offered to the action during initialization
- * @param encryptionDetails the name of the encrypter used to store the parameter.
+ * @param encryption the name of the encryption algorithm used to store the parameter or none (plain text)
  */
 protected[entity] case class ParameterValue protected[entity] (private val v: JsValue,
                                                                val init: Boolean,
-                                                               val encryptionDetails: Option[JsString] = None) {
+                                                               val encryption: Option[String] = None) {
 
   /** @return JsValue if defined else JsNull. */
   protected[entity] def value = Option(v) getOrElse JsNull
@@ -187,9 +174,6 @@ protected[entity] case class ParameterValue protected[entity] (private val v: Js
   /** @return true iff value is not JsNull. */
   protected[entity] def isDefined = value != JsNull
 
-  /** @return JsValue if defined else JsNull. */
-  protected[entity] def encryption = encryptionDetails getOrElse JsNull
-
   /**
    * The size of the ParameterValue entity as ByteSize.
    */
@@ -263,7 +247,7 @@ protected[core] object Parameters extends ArgNormalizer[Parameters] {
             val paramVal: ParameterValue = tuple._2 match {
               case o: JsObject =>
                 o.getFields("value", "init", "encryption") match {
-                  case Seq(v: JsValue, JsBoolean(i), e: JsString) =>
+                  case Seq(v: JsValue, JsBoolean(i), JsString(e)) =>
                     ParameterValue(v, i, Some(e))
                   case _ => ParameterValue(o, false, None)
                 }
@@ -297,22 +281,26 @@ protected[core] object Parameters extends ArgNormalizer[Parameters] {
           val JsObject(o) = value
           o.foreach(i =>
             i._2.asJsObject.getFields("value", "init", "encryption") match {
-              case Seq(v: JsValue, JsBoolean(init), e: JsValue) if e != JsNull =>
+              case Seq(v: JsValue, JsBoolean(init), JsString(e)) =>
+                val key = new ParameterName(i._1)
+                val value = ParameterValue(v, init, Some(e))
+                converted = converted + (key -> value)
+              case Seq(v: JsValue, JsBoolean(init)) =>
                 val key = new ParameterName(i._1)
-                val value = ParameterValue(v, init, Some(JsString(e.convertTo[String])))
+                val value = ParameterValue(v, init)
                 converted = converted + (key -> value)
-              case Seq(v: JsValue, JsBoolean(init), e: JsValue) =>
+              case Seq(v: JsValue, JsBoolean(init), JsNull) =>
                 val key = new ParameterName(i._1)
-                val value = ParameterValue(v, init, None)
+                val value = ParameterValue(v, init)
                 converted = converted + (key -> value)
           })
           if (converted.size == 0) {
-            deserializationError("parameters malformed no parameters available: " + value.toString())
+            deserializationError("parameters malformed no parameters available: " + value.toString)
           } else {
             new Parameters(converted)
           }
         } getOrElse deserializationError(
-          "parameters malformed could not read directly: " + (if (value != null) value.toString() else ""))
+          "parameters malformed could not read directly: " + (if (value != null) value.toString else ""))
       }
 
     /**
@@ -331,7 +319,7 @@ protected[core] object Parameters extends ArgNormalizer[Parameters] {
                 val key = new ParameterName(k)
                 val value = ParameterValue(v, false)
                 (key, value)
-              case Seq(JsString(k), v: JsValue, JsBoolean(i), e: JsString) =>
+              case Seq(JsString(k), v: JsValue, JsBoolean(i), JsString(e)) =>
                 val key = new ParameterName(k)
                 val value = ParameterValue(v, i, Some(e))
                 (key, value)
@@ -339,7 +327,7 @@ protected[core] object Parameters extends ArgNormalizer[Parameters] {
                 val key = new ParameterName(k)
                 val value = ParameterValue(v, i)
                 (key, value)
-              case Seq(JsString(k), v: JsValue, e: JsString) if (i.asJsObject.fields.contains("encryption")) =>
+              case Seq(JsString(k), v: JsValue, JsString(e)) if (i.asJsObject.fields.contains("encryption")) =>
                 val key = new ParameterName(k)
                 val value = ParameterValue(v, false, Some(e))
                 (key, value)
diff --git a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/ParameterEncryption.scala b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/ParameterEncryption.scala
index 482579cbad2..df50989b1f8 100644
--- a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/ParameterEncryption.scala
+++ b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/ParameterEncryption.scala
@@ -26,7 +26,7 @@ import javax.crypto.spec.{GCMParameterSpec, SecretKeySpec}
 import org.apache.openwhisk.core.ConfigKeys
 import pureconfig.loadConfig
 import spray.json.DefaultJsonProtocol._
-import spray.json.{JsNull, JsString}
+import spray.json._
 import pureconfig.generic.auto._
 import spray.json._
 case class ParameterStorageConfig(current: String = "", aes128: String = "", aes256: String = "")
@@ -34,12 +34,13 @@ case class ParameterStorageConfig(current: String = "", aes128: String = "", aes
 object ParameterEncryption {
   private val storageConfigLoader = loadConfig[ParameterStorageConfig](ConfigKeys.parameterStorage)
   var storageConfig = storageConfigLoader.getOrElse(ParameterStorageConfig.apply())
+
   def lock(params: Parameters): Parameters = {
     val configuredEncryptors = new encrypters(storageConfig)
     new Parameters(
       params.getMap
         .map(({
-          case (paramName, paramValue) if paramValue.encryption == JsNull =>
+          case (paramName, paramValue) if paramValue.encryption.isEmpty =>
             paramName -> configuredEncryptors.getCurrentEncrypter().encrypt(paramValue)
           case (paramName, paramValue) => paramName -> paramValue
         })))
@@ -50,11 +51,11 @@ object ParameterEncryption {
       params.getMap
         .map(({
           case (paramName, paramValue)
-              if paramValue.encryption != JsNull && !configuredEncryptors
-                .getEncrypter(paramValue.encryption.convertTo[String])
+              if paramValue.encryption.isDefined && !configuredEncryptors
+                .getEncrypter(paramValue.encryption.get)
                 .isEmpty =>
             paramName -> configuredEncryptors
-              .getEncrypter(paramValue.encryption.convertTo[String])
+              .getEncrypter(paramValue.encryption.get)
               .get
               .decrypt(paramValue)
           case (paramName, paramValue) => paramName -> paramValue
@@ -69,22 +70,25 @@ private trait encrypter {
 }
 
 private class encrypters(val storageConfig: ParameterStorageConfig) {
-  private val availableEncrypters = Map("" -> new NoopCrypt()) ++
+  val noop = new NoopCrypt()
+
+  private val availableEncrypters = Map.empty ++
     (if (!storageConfig.aes256.isEmpty) Some(Aes256.name -> new Aes256(getKeyBytes(storageConfig.aes256))) else None) ++
     (if (!storageConfig.aes128.isEmpty) Some(Aes128.name -> new Aes128(getKeyBytes(storageConfig.aes128))) else None)
 
   protected[entity] def getCurrentEncrypter(): encrypter = {
-    availableEncrypters.get(ParameterEncryption.storageConfig.current).get
+    availableEncrypters.get(ParameterEncryption.storageConfig.current).getOrElse(noop)
   }
-  protected[entity] def getEncrypter(name: String) = {
+
+  protected[entity] def getEncrypter(name: String): Option[encrypter] = {
     availableEncrypters.get(name)
   }
 
   def getKeyBytes(key: String): Array[Byte] = {
     if (key.length == 0) {
-      Array[Byte]()
+      Array.empty
     } else {
-      Base64.getDecoder().decode(key)
+      Base64.getDecoder.decode(key)
     }
   }
 }
@@ -112,7 +116,7 @@ private trait AesEncryption extends encrypter {
     byteBuffer.put(iv)
     byteBuffer.put(cipherText)
     val cipherMessage = byteBuffer.array
-    ParameterValue(JsString(Base64.getEncoder.encodeToString(cipherMessage)), value.init, Some(JsString(name)))
+    ParameterValue(JsString(Base64.getEncoder.encodeToString(cipherMessage)), value.init, Some(name))
   }
 
   def decrypt(value: ParameterValue): ParameterValue = {
diff --git a/tests/src/test/scala/org/apache/openwhisk/core/entity/test/ParameterEncryptionTests.scala b/tests/src/test/scala/org/apache/openwhisk/core/entity/test/ParameterEncryptionTests.scala
index c6aec18c3a0..56d629cab4e 100644
--- a/tests/src/test/scala/org/apache/openwhisk/core/entity/test/ParameterEncryptionTests.scala
+++ b/tests/src/test/scala/org/apache/openwhisk/core/entity/test/ParameterEncryptionTests.scala
@@ -102,9 +102,7 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
     ParameterEncryption.storageConfig = new ParameterStorageConfig("aes-128", "ra1V6AfOYAv0jCzEdufIFA==")
     val locked = ParameterEncryption.lock(parameters)
 
-    val unlockedParam = new ParameterValue(JsString("test-plain"), false)
-    val mixedParams =
-      locked.merge(Some((new Parameters(Map.empty) + (new ParameterName("plain") -> unlockedParam)).toJsObject))
+    val mixedParams = locked.merge(Some(Parameters("plain", "test-plain").toJsObject))
     val params = Parameters.readMergedList(mixedParams.get)
     params.get("one").get shouldBe locked.get("one").get
     params.get("two").get shouldBe locked.get("two").get
@@ -116,11 +114,11 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
   it should "correctly mark the encrypted parameters after lock" in {
     ParameterEncryption.storageConfig = new ParameterStorageConfig("aes-128", "ra1V6AfOYAv0jCzEdufIFA==")
     val locked = ParameterEncryption.lock(parameters)
-    locked.getMap.map(({
+    locked.getMap.foreach {
       case (_, paramValue) =>
-        paramValue.encryption.convertTo[String] shouldBe "aes-128"
+        paramValue.encryption shouldBe Some("aes-128")
         paramValue.value.convertTo[String] should not be "secret"
-    }))
+    }
   }
 
   it should "serialize to json correctly" in {
@@ -137,14 +135,14 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
     val locked = ParameterEncryption.lock(parameters)
     locked.getMap.map(({
       case (_, paramValue) =>
-        paramValue.encryption.convertTo[String] shouldBe "aes-128"
+        paramValue.encryption shouldBe Some("aes-128")
         paramValue.value.convertTo[String] should not be "secret"
     }))
 
     val unlocked = ParameterEncryption.unlock(locked)
     unlocked.getMap.map(({
       case (_, paramValue) =>
-        paramValue.encryption shouldBe JsNull
+        paramValue.encryption shouldBe empty
         paramValue.value.convertTo[String] shouldBe "secret"
     }))
   }
@@ -157,14 +155,14 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
     val locked = ParameterEncryption.lock(complexParam)
     locked.getMap.map(({
       case (_, paramValue) =>
-        paramValue.encryption.convertTo[String] shouldBe "aes-128"
+        paramValue.encryption shouldBe Some("aes-128")
         paramValue.value.convertTo[String] should not be "secret"
     }))
 
     val unlocked = ParameterEncryption.unlock(locked)
     unlocked.getMap.map(({
       case (_, paramValue) =>
-        paramValue.encryption shouldBe JsNull
+        paramValue.encryption shouldBe empty
         paramValue.value shouldBe obj
     }))
   }
@@ -176,14 +174,14 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
     val locked = ParameterEncryption.lock(multiline)
     locked.getMap.map(({
       case (_, paramValue) =>
-        paramValue.encryption.convertTo[String] shouldBe "aes-128"
+        paramValue.encryption shouldBe Some("aes-128")
         paramValue.value.convertTo[String] should not be "secret"
     }))
 
     val unlocked = ParameterEncryption.unlock(locked)
     unlocked.getMap.map(({
       case (_, paramValue) =>
-        paramValue.encryption shouldBe JsNull
+        paramValue.encryption shouldBe empty
         paramValue.value.convertTo[String] shouldBe lines
     }))
   }
@@ -195,14 +193,14 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
       val locked = ParameterEncryption.lock(parameters)
       locked.getMap.map(({
         case (_, paramValue) =>
-          paramValue.encryption.convertTo[String] shouldBe "aes-256"
+          paramValue.encryption shouldBe Some("aes-256")
           paramValue.value.convertTo[String] should not be "secret"
       }))
 
       val unlocked = ParameterEncryption.unlock(locked)
       unlocked.getMap.map(({
         case (_, paramValue) =>
-          paramValue.encryption shouldBe JsNull
+          paramValue.encryption shouldBe empty
           paramValue.value.convertTo[String] shouldBe "secret"
       }))
     } catch {
@@ -216,7 +214,7 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
       val locked = ParameterEncryption.lock(parameters)
       locked.getMap.map(({
         case (_, paramValue) =>
-          paramValue.encryption.convertTo[String] shouldBe "aes-128"
+          paramValue.encryption shouldBe Some("aes-128")
           paramValue.value.convertTo[String] should not be "secret"
       }))
 
@@ -229,10 +227,10 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
       val unlocked = ParameterEncryption.unlock(toDecrypt)
       unlocked.getMap.map(({
         case (_, paramValue) =>
-          paramValue.encryption shouldBe JsNull
+          paramValue.encryption shouldBe empty
           paramValue.value.convertTo[String] shouldBe "secret"
       }))
-      unlocked.toJsObject should not be JsNull
+      unlocked.toJsObject shouldBe JsObject("one" -> "secret".toJson, "two" -> "secret".toJson)
     } catch {
       case e: InvalidAlgorithmParameterException =>
         cancel(e.toString)

From 5342b900d7767984fe76d08af2fddd929cd94dca Mon Sep 17 00:00:00 2001
From: Rodric Rabbah <rodric@gmail.com>
Date: Sun, 8 Mar 2020 12:35:41 -0400
Subject: [PATCH 02/12] Remove 'strange construction' of param as json.

Simplify expression.
---
 .../scala/org/apache/openwhisk/core/entity/Parameter.scala | 3 +--
 .../openwhisk/core/containerpool/ContainerProxy.scala      | 7 ++-----
 .../apache/openwhisk/core/entity/test/SchemaTests.scala    | 2 +-
 3 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala
index 27489c016b0..27321af04c2 100644
--- a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala
+++ b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala
@@ -82,8 +82,7 @@ protected[core] class Parameters protected[entity] (private val params: Map[Para
       val init = if (p._2.init) Some("init" -> JsTrue) else None
       val encrypt = p._2.encryption.map(e => ("encryption" -> JsString(e)))
 
-      // Have do use this slightly strange construction to get the json object order identical.
-      JsObject(ListMap.empty ++ encrypt ++ init ++ Map("key" -> p._1.name.toJson, "value" -> p._2.value))
+      JsObject(Map("key" -> p._1.name.toJson, "value" -> p._2.value) ++ init ++ encrypt)
     } toSeq: _*)
   }
 
diff --git a/core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerProxy.scala b/core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerProxy.scala
index eebcc7c9f95..7174f8c405e 100644
--- a/core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerProxy.scala
+++ b/core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerProxy.scala
@@ -774,11 +774,8 @@ class ContainerProxy(factory: (TransactionId,
   def initializeAndRun(container: Container, job: Run, reschedule: Boolean = false)(
     implicit tid: TransactionId): Future[WhiskActivation] = {
     val actionTimeout = job.action.limits.timeout.duration
-    val unlockedContent = job.msg.content match {
-      case Some(js) => {
-        Some(ParameterEncryption.unlock(Parameters.readMergedList(js)).toJsObject)
-      }
-      case _ => job.msg.content
+    val unlockedContent = job.msg.content.map { js =>
+      ParameterEncryption.unlock(Parameters.readMergedList(js)).toJsObject
     }
 
     val (env, parameters) = ContainerProxy.partitionArguments(unlockedContent, job.msg.initArgs)
diff --git a/tests/src/test/scala/org/apache/openwhisk/core/entity/test/SchemaTests.scala b/tests/src/test/scala/org/apache/openwhisk/core/entity/test/SchemaTests.scala
index 7b6cad4c1d9..ccab9dff6cc 100644
--- a/tests/src/test/scala/org/apache/openwhisk/core/entity/test/SchemaTests.scala
+++ b/tests/src/test/scala/org/apache/openwhisk/core/entity/test/SchemaTests.scala
@@ -695,7 +695,7 @@ class SchemaTests extends FlatSpec with BeforeAndAfter with ExecHelpers with Mat
     val json = Seq[JsValue](
       JsArray(JsObject("key" -> "k".toJson, "value" -> "v".toJson)),
       JsArray(JsObject("key" -> "k".toJson, "value" -> "v".toJson, "init" -> JsFalse)),
-      JsArray(JsObject("key" -> "k".toJson, "value" -> "v".toJson, "init" -> JsTrue)))
+      JsArray(JsObject(Map("key" -> "k".toJson, "value" -> "v".toJson, "init" -> JsTrue))))
 
     val params = json.map { p =>
       Parameters.serdes.read(p)

From 5a7db0b78c0b3d054bbeef7f2ba3f58e3c096e61 Mon Sep 17 00:00:00 2001
From: Rodric Rabbah <rodric@gmail.com>
Date: Sun, 8 Mar 2020 17:18:21 -0400
Subject: [PATCH 03/12] Remove unnecessary cons of Encrypter class.

---
 .../core/entity/ParameterEncryption.scala     | 112 +++++++++---------
 .../test/ParameterEncryptionTests.scala       |  23 ++--
 2 files changed, 70 insertions(+), 65 deletions(-)

diff --git a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/ParameterEncryption.scala b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/ParameterEncryption.scala
index df50989b1f8..9240f78ff2a 100644
--- a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/ParameterEncryption.scala
+++ b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/ParameterEncryption.scala
@@ -29,68 +29,77 @@ import spray.json.DefaultJsonProtocol._
 import spray.json._
 import pureconfig.generic.auto._
 import spray.json._
-case class ParameterStorageConfig(current: String = "", aes128: String = "", aes256: String = "")
 
-object ParameterEncryption {
-  private val storageConfigLoader = loadConfig[ParameterStorageConfig](ConfigKeys.parameterStorage)
-  var storageConfig = storageConfigLoader.getOrElse(ParameterStorageConfig.apply())
+protected[core] case class ParameterStorageConfig(current: String = ParameterEncryption.NO_ENCRYPTION,
+                                                  aes128: Option[String] = None,
+                                                  aes256: Option[String] = None)
 
-  def lock(params: Parameters): Parameters = {
-    val configuredEncryptors = new encrypters(storageConfig)
-    new Parameters(
-      params.getMap
-        .map(({
-          case (paramName, paramValue) if paramValue.encryption.isEmpty =>
-            paramName -> configuredEncryptors.getCurrentEncrypter().encrypt(paramValue)
-          case (paramName, paramValue) => paramName -> paramValue
-        })))
-  }
-  def unlock(params: Parameters): Parameters = {
-    val configuredEncryptors = new encrypters(storageConfig)
-    new Parameters(
-      params.getMap
-        .map(({
-          case (paramName, paramValue)
-              if paramValue.encryption.isDefined && !configuredEncryptors
-                .getEncrypter(paramValue.encryption.get)
-                .isEmpty =>
-            paramName -> configuredEncryptors
-              .getEncrypter(paramValue.encryption.get)
-              .get
-              .decrypt(paramValue)
-          case (paramName, paramValue) => paramName -> paramValue
-        })))
-  }
-}
+protected[core] object ParameterEncryption {
 
-private trait encrypter {
-  def encrypt(p: ParameterValue): ParameterValue
-  def decrypt(p: ParameterValue): ParameterValue
-  val name: String
-}
+  val NO_ENCRYPTION = "noop"
 
-private class encrypters(val storageConfig: ParameterStorageConfig) {
-  val noop = new NoopCrypt()
+  private val noop = new NoopCrypt()
 
-  private val availableEncrypters = Map.empty ++
-    (if (!storageConfig.aes256.isEmpty) Some(Aes256.name -> new Aes256(getKeyBytes(storageConfig.aes256))) else None) ++
-    (if (!storageConfig.aes128.isEmpty) Some(Aes128.name -> new Aes128(getKeyBytes(storageConfig.aes128))) else None)
+  private var defaultEncryptor: encrypter = noop
+  private var encryptors: Map[String, encrypter] = Map.empty
 
-  protected[entity] def getCurrentEncrypter(): encrypter = {
-    availableEncrypters.get(ParameterEncryption.storageConfig.current).getOrElse(noop)
+  {
+    val configLoader = loadConfig[ParameterStorageConfig](ConfigKeys.parameterStorage)
+    val config = configLoader.getOrElse(ParameterStorageConfig(noop.name))
+    initialize(config)
   }
 
-  protected[entity] def getEncrypter(name: String): Option[encrypter] = {
-    availableEncrypters.get(name)
+  def initialize(config: ParameterStorageConfig): Unit = {
+    val availableEncrypters = Map(noop.name -> noop) ++
+      config.aes128.map(k => Aes128.name -> new Aes128(getKeyBytes(k))) ++
+      config.aes256.map(k => Aes256.name -> new Aes256(getKeyBytes(k)))
+
+    // should this succeed if the given current scheme does not exist?
+    defaultEncryptor = availableEncrypters.get(config.current).getOrElse(noop)
+    encryptors = availableEncrypters
   }
 
-  def getKeyBytes(key: String): Array[Byte] = {
+  private def getKeyBytes(key: String): Array[Byte] = {
     if (key.length == 0) {
       Array.empty
     } else {
       Base64.getDecoder.decode(key)
     }
   }
+
+  /**
+   * Encrypts any parameters that are not yet encoded.
+   *
+   * @param params the parameters to encode
+   * @return parameters with all values encrypted
+   */
+  def lock(params: Parameters): Parameters = {
+    new Parameters(params.getMap.map {
+      case (paramName, paramValue) if paramValue.encryption.isEmpty =>
+        paramName -> defaultEncryptor.encrypt(paramValue)
+      case p => p
+    })
+  }
+
+  /**
+   * Decodes parameters. If the encryption scheme for a parameter is not recognized, it is not modified.
+   *
+   * @param params the parameters to decode
+   * @return parameters will all values decoded (where scheme is known)
+   */
+  def unlock(params: Parameters): Parameters = {
+    new Parameters(params.getMap.map {
+      case (paramName, paramValue) if paramValue.encryption.nonEmpty =>
+        paramName -> encryptors(paramValue.encryption.getOrElse(noop.name)).decrypt(paramValue)
+      case p => p
+    })
+  }
+}
+
+private trait encrypter {
+  def encrypt(p: ParameterValue): ParameterValue
+  def decrypt(p: ParameterValue): ParameterValue
+  val name: String
 }
 
 private trait AesEncryption extends encrypter {
@@ -156,12 +165,7 @@ private case class Aes256(val key: Array[Byte], val ivLen: Int = 128, val name:
     with encrypter
 
 private class NoopCrypt extends encrypter {
-  val name = ""
-  def encrypt(p: ParameterValue): ParameterValue = {
-    p
-  }
-
-  def decrypt(p: ParameterValue): ParameterValue = {
-    p
-  }
+  val name = ParameterEncryption.NO_ENCRYPTION
+  def encrypt(p: ParameterValue) = p
+  def decrypt(p: ParameterValue) = p
 }
diff --git a/tests/src/test/scala/org/apache/openwhisk/core/entity/test/ParameterEncryptionTests.scala b/tests/src/test/scala/org/apache/openwhisk/core/entity/test/ParameterEncryptionTests.scala
index 56d629cab4e..9f3822c141d 100644
--- a/tests/src/test/scala/org/apache/openwhisk/core/entity/test/ParameterEncryptionTests.scala
+++ b/tests/src/test/scala/org/apache/openwhisk/core/entity/test/ParameterEncryptionTests.scala
@@ -29,7 +29,7 @@ import spray.json._
 class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfter {
 
   after {
-    ParameterEncryption.storageConfig = new ParameterStorageConfig("")
+    ParameterEncryption.initialize(ParameterStorageConfig())
   }
 
   val parameters = new Parameters(
@@ -99,7 +99,7 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
   }
 
   it should "read the merged message payload from kafka into parameters" in {
-    ParameterEncryption.storageConfig = new ParameterStorageConfig("aes-128", "ra1V6AfOYAv0jCzEdufIFA==")
+    ParameterEncryption.initialize(ParameterStorageConfig("aes-128", aes128 = Some("ra1V6AfOYAv0jCzEdufIFA==")))
     val locked = ParameterEncryption.lock(parameters)
 
     val mixedParams = locked.merge(Some(Parameters("plain", "test-plain").toJsObject))
@@ -112,7 +112,7 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
 
   behavior of "AesParameterEncryption"
   it should "correctly mark the encrypted parameters after lock" in {
-    ParameterEncryption.storageConfig = new ParameterStorageConfig("aes-128", "ra1V6AfOYAv0jCzEdufIFA==")
+    ParameterEncryption.initialize(ParameterStorageConfig("aes-128", aes128 = Some("ra1V6AfOYAv0jCzEdufIFA==")))
     val locked = ParameterEncryption.lock(parameters)
     locked.getMap.foreach {
       case (_, paramValue) =>
@@ -124,14 +124,14 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
   it should "serialize to json correctly" in {
     val output =
       """\Q{"one":{"encryption":"aes-128","init":false,"value":"\E.*\Q"},"two":{"encryption":"aes-128","init":true,"value":"\E.*\Q"}}""".stripMargin.r
-    ParameterEncryption.storageConfig = new ParameterStorageConfig("aes-128", "ra1V6AfOYAv0jCzEdufIFA==")
+    ParameterEncryption.initialize(ParameterStorageConfig("aes-128", aes128 = Some("ra1V6AfOYAv0jCzEdufIFA==")))
     val locked = ParameterEncryption.lock(parameters)
     val dbString = locked.toJsObject.toString
     dbString should fullyMatch regex output
   }
 
   it should "correctly decrypted encrypted values" in {
-    ParameterEncryption.storageConfig = new ParameterStorageConfig("aes-128", "ra1V6AfOYAv0jCzEdufIFA==")
+    ParameterEncryption.initialize(ParameterStorageConfig("aes-128", aes128 = Some("ra1V6AfOYAv0jCzEdufIFA==")))
     val locked = ParameterEncryption.lock(parameters)
     locked.getMap.map(({
       case (_, paramValue) =>
@@ -147,7 +147,7 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
     }))
   }
   it should "correctly decrypted encrypted JsObject values" in {
-    ParameterEncryption.storageConfig = new ParameterStorageConfig("aes-128", "ra1V6AfOYAv0jCzEdufIFA==")
+    ParameterEncryption.initialize(ParameterStorageConfig("aes-128", aes128 = Some("ra1V6AfOYAv0jCzEdufIFA==")))
     val obj = Map("key" -> "xyz".toJson, "value" -> "v1".toJson).toJson
 
     val complexParam = new Parameters(Map(new ParameterName("one") -> new ParameterValue(obj, false)))
@@ -167,7 +167,7 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
     }))
   }
   it should "correctly decrypted encrypted multiline values" in {
-    ParameterEncryption.storageConfig = new ParameterStorageConfig("aes-128", "ra1V6AfOYAv0jCzEdufIFA==")
+    ParameterEncryption.initialize(ParameterStorageConfig("aes-128", aes128 = Some("ra1V6AfOYAv0jCzEdufIFA==")))
     val lines = "line1\nline2\nline3\nline4"
     val multiline = new Parameters(Map(new ParameterName("one") -> new ParameterValue(JsString(lines), false)))
 
@@ -187,8 +187,8 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
   }
   // Not sure having cancelled tests is a good idea either, need to work on aes256 packaging.
   it should "work if with aes256 if policy allows it" in {
-    ParameterEncryption.storageConfig =
-      new ParameterStorageConfig("aes-256", "", "j5rLzhtxwzPyUVUy8/p8XJmBoKeDoSzNJP1SITJEY9E=")
+    ParameterEncryption.initialize(
+      ParameterStorageConfig("aes-256", aes256 = Some("j5rLzhtxwzPyUVUy8/p8XJmBoKeDoSzNJP1SITJEY9E=")))
     try {
       val locked = ParameterEncryption.lock(parameters)
       locked.getMap.map(({
@@ -209,7 +209,7 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
     }
   }
   it should "support reverting back to Noop encryption" in {
-    ParameterEncryption.storageConfig = new ParameterStorageConfig("aes-128", "ra1V6AfOYAv0jCzEdufIFA==", "")
+    ParameterEncryption.initialize(ParameterStorageConfig("aes-128", aes128 = Some("ra1V6AfOYAv0jCzEdufIFA==")))
     try {
       val locked = ParameterEncryption.lock(parameters)
       locked.getMap.map(({
@@ -220,7 +220,8 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
 
       val lockedJson = locked.toJsObject
 
-      ParameterEncryption.storageConfig = new ParameterStorageConfig("", "ra1V6AfOYAv0jCzEdufIFA==", "")
+      // current mode defaults to noop
+      ParameterEncryption.initialize(ParameterStorageConfig(aes128 = Some("ra1V6AfOYAv0jCzEdufIFA==")))
 
       val toDecrypt = Parameters.serdes.read(lockedJson)
 

From e70abe79cf35a21f64f77a0f0517d3618c502e13 Mon Sep 17 00:00:00 2001
From: Rodric Rabbah <rodric@gmail.com>
Date: Sun, 8 Mar 2020 17:28:35 -0400
Subject: [PATCH 04/12] Refactoring of encryptor names.

---
 .../core/entity/ParameterEncryption.scala     | 49 ++++++++-----------
 1 file changed, 21 insertions(+), 28 deletions(-)

diff --git a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/ParameterEncryption.scala b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/ParameterEncryption.scala
index 9240f78ff2a..1f2762fe480 100644
--- a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/ParameterEncryption.scala
+++ b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/ParameterEncryption.scala
@@ -37,11 +37,15 @@ protected[core] case class ParameterStorageConfig(current: String = ParameterEnc
 protected[core] object ParameterEncryption {
 
   val NO_ENCRYPTION = "noop"
+  val AES128_ENCRYPTION = "aes-128"
+  val AES256_ENCRYPTION = "aes-256"
 
-  private val noop = new NoopCrypt()
+  private val noop = new Encrypter {
+    override val name = NO_ENCRYPTION
+  }
 
-  private var defaultEncryptor: encrypter = noop
-  private var encryptors: Map[String, encrypter] = Map.empty
+  private var defaultEncryptor: Encrypter = noop
+  private var encryptors: Map[String, Encrypter] = Map.empty
 
   {
     val configLoader = loadConfig[ParameterStorageConfig](ConfigKeys.parameterStorage)
@@ -51,8 +55,8 @@ protected[core] object ParameterEncryption {
 
   def initialize(config: ParameterStorageConfig): Unit = {
     val availableEncrypters = Map(noop.name -> noop) ++
-      config.aes128.map(k => Aes128.name -> new Aes128(getKeyBytes(k))) ++
-      config.aes256.map(k => Aes256.name -> new Aes256(getKeyBytes(k)))
+      config.aes128.map(k => AES128_ENCRYPTION -> new Aes128(getKeyBytes(k))) ++
+      config.aes256.map(k => AES256_ENCRYPTION -> new Aes256(getKeyBytes(k)))
 
     // should this succeed if the given current scheme does not exist?
     defaultEncryptor = availableEncrypters.get(config.current).getOrElse(noop)
@@ -96,22 +100,21 @@ protected[core] object ParameterEncryption {
   }
 }
 
-private trait encrypter {
-  def encrypt(p: ParameterValue): ParameterValue
-  def decrypt(p: ParameterValue): ParameterValue
+private trait Encrypter {
   val name: String
+  def encrypt(p: ParameterValue): ParameterValue = p
+  def decrypt(p: ParameterValue): ParameterValue = p
 }
 
-private trait AesEncryption extends encrypter {
+private trait AesEncryption extends Encrypter {
   val key: Array[Byte]
   val ivLen: Int
   val name: String
   private val tLen = 128
   private val secretKey = new SecretKeySpec(key, "AES")
-
   private val secureRandom = new SecureRandom()
 
-  def encrypt(value: ParameterValue): ParameterValue = {
+  override def encrypt(value: ParameterValue): ParameterValue = {
     val iv = new Array[Byte](ivLen)
     secureRandom.nextBytes(iv)
     val gcmSpec = new GCMParameterSpec(tLen, iv)
@@ -128,7 +131,7 @@ private trait AesEncryption extends encrypter {
     ParameterValue(JsString(Base64.getEncoder.encodeToString(cipherMessage)), value.init, Some(name))
   }
 
-  def decrypt(value: ParameterValue): ParameterValue = {
+  override def decrypt(value: ParameterValue): ParameterValue = {
     val cipherMessage = value.value.convertTo[String].getBytes(StandardCharsets.UTF_8)
     val byteBuffer = ByteBuffer.wrap(Base64.getDecoder.decode(cipherMessage))
     val ivLength = byteBuffer.getInt
@@ -150,22 +153,12 @@ private trait AesEncryption extends encrypter {
 
 }
 
-private object Aes128 {
-  val name: String = "aes-128"
+private case class Aes128(override val key: Array[Byte]) extends AesEncryption with Encrypter {
+  override val name = ParameterEncryption.AES128_ENCRYPTION
+  override val ivLen = 12
 }
-private case class Aes128(val key: Array[Byte], val ivLen: Int = 12, val name: String = Aes128.name)
-    extends AesEncryption
-    with encrypter
 
-private object Aes256 {
-  val name: String = "aes-256"
-}
-private case class Aes256(val key: Array[Byte], val ivLen: Int = 128, val name: String = Aes256.name)
-    extends AesEncryption
-    with encrypter
-
-private class NoopCrypt extends encrypter {
-  val name = ParameterEncryption.NO_ENCRYPTION
-  def encrypt(p: ParameterValue) = p
-  def decrypt(p: ParameterValue) = p
+private case class Aes256(override val key: Array[Byte]) extends AesEncryption with Encrypter {
+  override val name = ParameterEncryption.AES256_ENCRYPTION
+  override val ivLen = 128
 }

From bf72952c967494372f02df3c6056220004be365b Mon Sep 17 00:00:00 2001
From: Rodric Rabbah <rodric@gmail.com>
Date: Sun, 8 Mar 2020 18:57:56 -0400
Subject: [PATCH 05/12] Move lock/unlock to Parameters. Refactor tests.

---
 .../openwhisk/core/entity/Parameter.scala     |  30 ++++
 .../core/entity/ParameterEncryption.scala     |  91 ++++------
 .../openwhisk/core/entity/WhiskAction.scala   |   7 +-
 .../openwhisk/core/entity/WhiskPackage.scala  |   2 +-
 .../core/containerpool/ContainerProxy.scala   |   4 +-
 .../test/ParameterEncryptionTests.scala       | 166 ++++++++++--------
 6 files changed, 165 insertions(+), 135 deletions(-)

diff --git a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala
index 27321af04c2..53dd3fef366 100644
--- a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala
+++ b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala
@@ -130,6 +130,36 @@ protected[core] class Parameters protected[entity] (private val params: Map[Para
       case _            => true
     } getOrElse valueForNonExistent
   }
+
+  /**
+   * Encrypts any parameters that are not yet encoded.
+   *
+   * @return parameters with all values encrypted
+   */
+  def lock(encoder: Option[Encrypter] = None): Parameters = {
+    new Parameters(params.map {
+      case (paramName, paramValue) if paramValue.encryption.isEmpty =>
+        paramName -> encoder.getOrElse(ParameterEncryption.singleton.default).encrypt(paramValue)
+      case p => p
+    })
+  }
+
+  /**
+   * Decodes parameters. If the encryption scheme for a parameter is not recognized, it is not modified.
+   *
+   * @return parameters will all values decoded (where scheme is known)
+   */
+  def unlock(decoder: Option[ParameterEncryption] = None): Parameters = {
+    new Parameters(params.map {
+      case (paramName, paramValue) if paramValue.encryption.nonEmpty =>
+        paramName -> decoder
+          .getOrElse(ParameterEncryption.singleton)
+          .encryptor(paramValue.encryption)
+          .decrypt(paramValue)
+      case p => p
+    })
+  }
+
 }
 
 /**
diff --git a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/ParameterEncryption.scala b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/ParameterEncryption.scala
index 1f2762fe480..3bf77eb6e50 100644
--- a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/ParameterEncryption.scala
+++ b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/ParameterEncryption.scala
@@ -34,85 +34,70 @@ protected[core] case class ParameterStorageConfig(current: String = ParameterEnc
                                                   aes128: Option[String] = None,
                                                   aes256: Option[String] = None)
 
+protected[core] case class ParameterEncryption(default: Encrypter, encryptors: Map[String, Encrypter]) {
+
+  /**
+   * Gets the default encryptor or the encryptor for the given scheme name.
+   *
+   * @param name optionnal name of the encryption algorithm (defaults to current from last configuration)
+   * @return the encryptor if there is one else no-op encryptor
+   */
+  def encryptor(name: Option[String] = None): Encrypter = {
+    encryptors.get(name.getOrElse(default.name)).getOrElse(ParameterEncryption.noop)
+  }
+
+}
+
 protected[core] object ParameterEncryption {
 
   val NO_ENCRYPTION = "noop"
   val AES128_ENCRYPTION = "aes-128"
   val AES256_ENCRYPTION = "aes-256"
 
-  private val noop = new Encrypter {
+  val noop = new Encrypter {
     override val name = NO_ENCRYPTION
   }
 
-  private var defaultEncryptor: Encrypter = noop
-  private var encryptors: Map[String, Encrypter] = Map.empty
-
-  {
+  val singleton: ParameterEncryption = {
     val configLoader = loadConfig[ParameterStorageConfig](ConfigKeys.parameterStorage)
     val config = configLoader.getOrElse(ParameterStorageConfig(noop.name))
-    initialize(config)
+    ParameterEncryption(config)
   }
 
-  def initialize(config: ParameterStorageConfig): Unit = {
-    val availableEncrypters = Map(noop.name -> noop) ++
-      config.aes128.map(k => AES128_ENCRYPTION -> new Aes128(getKeyBytes(k))) ++
-      config.aes256.map(k => AES256_ENCRYPTION -> new Aes256(getKeyBytes(k)))
+  def apply(config: ParameterStorageConfig): ParameterEncryption = {
+    val availableEncoders = Map(noop.name -> noop) ++
+      config.aes128.map(k => AES128_ENCRYPTION -> new Aes128(k)) ++
+      config.aes256.map(k => AES256_ENCRYPTION -> new Aes256(k))
+
+    val defaultEncoder: Encrypter = availableEncoders.get(config.current).getOrElse(noop)
 
-    // should this succeed if the given current scheme does not exist?
-    defaultEncryptor = availableEncrypters.get(config.current).getOrElse(noop)
-    encryptors = availableEncrypters
+    ParameterEncryption(defaultEncoder, availableEncoders)
   }
+}
+
+protected[core] trait Encrypter {
+  val name: String
+  def encrypt(p: ParameterValue): ParameterValue = p
+  def decrypt(p: ParameterValue): ParameterValue = p
+}
 
-  private def getKeyBytes(key: String): Array[Byte] = {
+protected[core] object Encrypter {
+  protected[entity] def getKeyBytes(key: String): Array[Byte] = {
     if (key.length == 0) {
       Array.empty
     } else {
       Base64.getDecoder.decode(key)
     }
   }
-
-  /**
-   * Encrypts any parameters that are not yet encoded.
-   *
-   * @param params the parameters to encode
-   * @return parameters with all values encrypted
-   */
-  def lock(params: Parameters): Parameters = {
-    new Parameters(params.getMap.map {
-      case (paramName, paramValue) if paramValue.encryption.isEmpty =>
-        paramName -> defaultEncryptor.encrypt(paramValue)
-      case p => p
-    })
-  }
-
-  /**
-   * Decodes parameters. If the encryption scheme for a parameter is not recognized, it is not modified.
-   *
-   * @param params the parameters to decode
-   * @return parameters will all values decoded (where scheme is known)
-   */
-  def unlock(params: Parameters): Parameters = {
-    new Parameters(params.getMap.map {
-      case (paramName, paramValue) if paramValue.encryption.nonEmpty =>
-        paramName -> encryptors(paramValue.encryption.getOrElse(noop.name)).decrypt(paramValue)
-      case p => p
-    })
-  }
-}
-
-private trait Encrypter {
-  val name: String
-  def encrypt(p: ParameterValue): ParameterValue = p
-  def decrypt(p: ParameterValue): ParameterValue = p
 }
 
-private trait AesEncryption extends Encrypter {
+protected[core] trait AesEncryption extends Encrypter {
   val key: Array[Byte]
   val ivLen: Int
   val name: String
   private val tLen = 128
-  private val secretKey = new SecretKeySpec(key, "AES")
   private val secureRandom = new SecureRandom()
+  private lazy val secretKey = new SecretKeySpec(key, "AES")
 
   override def encrypt(value: ParameterValue): ParameterValue = {
     val iv = new Array[Byte](ivLen)
@@ -153,12 +138,14 @@ private trait AesEncryption extends Encrypter {
 
 }
 
-private case class Aes128(override val key: Array[Byte]) extends AesEncryption with Encrypter {
+protected[core] class Aes128(val k: String) extends AesEncryption with Encrypter {
+  override val key = Encrypter.getKeyBytes(k)
   override val name = ParameterEncryption.AES128_ENCRYPTION
   override val ivLen = 12
 }
 
-private case class Aes256(override val key: Array[Byte]) extends AesEncryption with Encrypter {
+protected[core] class Aes256(val k: String) extends AesEncryption with Encrypter {
+  override val key = Encrypter.getKeyBytes(k)
   override val name = ParameterEncryption.AES256_ENCRYPTION
   override val ivLen = 128
 }
diff --git a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala
index 6dd60a1f12a..85074989346 100644
--- a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala
+++ b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala
@@ -384,7 +384,7 @@ object WhiskAction extends DocumentFactory[WhiskAction] with WhiskEntityQueries[
       val stream = new ByteArrayInputStream(bytes)
       super.putAndAttach(
         db,
-        doc.copy(parameters = ParameterEncryption.lock(doc.parameters)).revision[WhiskAction](doc.rev),
+        doc.copy(parameters = doc.parameters.lock()).revision[WhiskAction](doc.rev),
         attachmentUpdater,
         attachmentType,
         stream,
@@ -404,10 +404,7 @@ object WhiskAction extends DocumentFactory[WhiskAction] with WhiskEntityQueries[
         case exec @ BlackBoxExec(_, Some(Inline(code)), _, _, binary) =>
           putWithAttachment(code, binary, exec)
         case _ =>
-          super.put(
-            db,
-            doc.copy(parameters = ParameterEncryption.lock(doc.parameters)).revision[WhiskAction](doc.rev),
-            old)
+          super.put(db, doc.copy(parameters = doc.parameters.lock()).revision[WhiskAction](doc.rev), old)
       }
     } match {
       case Success(f) => f
diff --git a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskPackage.scala b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskPackage.scala
index ec9e0ecadb9..8ded69ed54e 100644
--- a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskPackage.scala
+++ b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskPackage.scala
@@ -205,7 +205,7 @@ object WhiskPackage
   override def put[A >: WhiskPackage](db: ArtifactStore[A], doc: WhiskPackage, old: Option[WhiskPackage])(
     implicit transid: TransactionId,
     notifier: Option[CacheChangeNotification]): Future[DocInfo] = {
-    super.put(db, doc.copy(parameters = ParameterEncryption.lock(doc.parameters)).revision[WhiskPackage](doc.rev), old)
+    super.put(db, doc.copy(parameters = doc.parameters.lock()).revision[WhiskPackage](doc.rev), old)
   }
 }
 
diff --git a/core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerProxy.scala b/core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerProxy.scala
index 7174f8c405e..67cdf433a7b 100644
--- a/core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerProxy.scala
+++ b/core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerProxy.scala
@@ -774,9 +774,7 @@ class ContainerProxy(factory: (TransactionId,
   def initializeAndRun(container: Container, job: Run, reschedule: Boolean = false)(
     implicit tid: TransactionId): Future[WhiskActivation] = {
     val actionTimeout = job.action.limits.timeout.duration
-    val unlockedContent = job.msg.content.map { js =>
-      ParameterEncryption.unlock(Parameters.readMergedList(js)).toJsObject
-    }
+    val unlockedContent = job.msg.content.map(Parameters.readMergedList(_).unlock().toJsObject)
 
     val (env, parameters) = ContainerProxy.partitionArguments(unlockedContent, job.msg.initArgs)
 
diff --git a/tests/src/test/scala/org/apache/openwhisk/core/entity/test/ParameterEncryptionTests.scala b/tests/src/test/scala/org/apache/openwhisk/core/entity/test/ParameterEncryptionTests.scala
index 9f3822c141d..bb9cfb88445 100644
--- a/tests/src/test/scala/org/apache/openwhisk/core/entity/test/ParameterEncryptionTests.scala
+++ b/tests/src/test/scala/org/apache/openwhisk/core/entity/test/ParameterEncryptionTests.scala
@@ -28,9 +28,17 @@ import spray.json._
 @RunWith(classOf[JUnitRunner])
 class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfter {
 
-  after {
-    ParameterEncryption.initialize(ParameterStorageConfig())
-  }
+  val noop = Some( // default is no-op but key is available to decodde aes128 encoded params
+    ParameterEncryption(ParameterStorageConfig(aes128 = Some("ra1V6AfOYAv0jCzEdufIFA=="), aes256 = Some(""))))
+
+  val aes128decoder = Some(
+    ParameterEncryption(ParameterStorageConfig("aes-128", aes128 = Some("ra1V6AfOYAv0jCzEdufIFA=="))))
+  val aes128encoder = Some(aes128decoder.get.default)
+
+  val aes256decoder = Some(
+    ParameterEncryption(
+      ParameterStorageConfig("aes-256", aes256 = Some("j5rLzhtxwzPyUVUy8/p8XJmBoKeDoSzNJP1SITJEY9E="))))
+  val aes256encoder = Some(aes256decoder.get.default)
 
   val parameters = new Parameters(
     Map(
@@ -38,6 +46,7 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
       new ParameterName("two") -> new ParameterValue("secret".toJson, true)))
 
   behavior of "Parameters"
+
   it should "handle complex objects in param body" in {
     val input =
       """
@@ -54,11 +63,26 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
         |    "a": "A"
         |}
         |""".stripMargin
-    val ps = Parameters.readMergedList(input.parseJson)
-    ps.get("a").get.convertTo[String] shouldBe "A"
+
+    val p = Parameters.readMergedList(input.parseJson)
+    p.get("a").get.convertTo[String] shouldBe "A"
   }
 
-  it should "handle decryption json objects" in {
+  it should "handle decryption of json objects" in {
+    val originalValue =
+      """
+        |{
+        |"paramName1":{"init":false,"value":"from-action"},
+        |"paramName2":{"init":false,"value":"from-pack"}
+        |}
+        |""".stripMargin
+
+    val p = Parameters.serdes.read(originalValue.parseJson)
+    p.get("paramName1").get.convertTo[String] shouldBe "from-action"
+    p.get("paramName2").get.convertTo[String] shouldBe "from-pack"
+  }
+
+  it should "handle decryption of json objects with null field" in {
     val originalValue =
       """
         |{
@@ -66,12 +90,13 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
         |"paramName2":{"encryption":null,"init":false,"value":"from-pack"}
         |}
         |""".stripMargin
-    val ps = Parameters.serdes.read(originalValue.parseJson)
-    ps.get("paramName1").get.convertTo[String] shouldBe "from-action"
-    ps.get("paramName2").get.convertTo[String] shouldBe "from-pack"
+
+    val p = Parameters.serdes.read(originalValue.parseJson)
+    p.get("paramName1").get.convertTo[String] shouldBe "from-action"
+    p.get("paramName2").get.convertTo[String] shouldBe "from-pack"
   }
 
-  it should "drop encryption payload when no longer encrypted" in {
+  it should "drop encryption of payload when no longer encrypted" in {
     val originalValue =
       """
         |{
@@ -79,11 +104,11 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
         |"paramName2":{"encryption":null,"init":false,"value":"from-action"}
         |}
         |""".stripMargin
-    val ps = Parameters.serdes.read(originalValue.parseJson)
-    val o = ps.toJsObject
-    o.fields.map((tuple: (String, JsValue)) => {
-      tuple._2.convertTo[String] shouldBe "from-action"
-    })
+
+    val p = Parameters.serdes.read(originalValue.parseJson)
+    p.toJsObject.fields.foreach {
+      case tuple => tuple._2.convertTo[String] shouldBe "from-action"
+    }
   }
 
   it should "read the merged unencrypted parameters during mixed storage" in {
@@ -91,17 +116,15 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
       """
         |{"name":"from-action","other":"from-action"}
         |""".stripMargin
-    val ps = Parameters.readMergedList(originalValue.parseJson)
-    val o = ps.toJsObject
-    o.fields.map((tuple: (String, JsValue)) => {
-      tuple._2.convertTo[String] shouldBe "from-action"
-    })
+
+    val p = Parameters.readMergedList(originalValue.parseJson)
+    p.toJsObject.fields.foreach {
+      case tuple => tuple._2.convertTo[String] shouldBe "from-action"
+    }
   }
 
   it should "read the merged message payload from kafka into parameters" in {
-    ParameterEncryption.initialize(ParameterStorageConfig("aes-128", aes128 = Some("ra1V6AfOYAv0jCzEdufIFA==")))
-    val locked = ParameterEncryption.lock(parameters)
-
+    val locked = parameters.lock(aes128encoder)
     val mixedParams = locked.merge(Some(Parameters("plain", "test-plain").toJsObject))
     val params = Parameters.readMergedList(mixedParams.get)
     params.get("one").get shouldBe locked.get("one").get
@@ -111,9 +134,9 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
   }
 
   behavior of "AesParameterEncryption"
+
   it should "correctly mark the encrypted parameters after lock" in {
-    ParameterEncryption.initialize(ParameterStorageConfig("aes-128", aes128 = Some("ra1V6AfOYAv0jCzEdufIFA==")))
-    val locked = ParameterEncryption.lock(parameters)
+    val locked = parameters.lock(aes128encoder)
     locked.getMap.foreach {
       case (_, paramValue) =>
         paramValue.encryption shouldBe Some("aes-128")
@@ -124,113 +147,107 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
   it should "serialize to json correctly" in {
     val output =
       """\Q{"one":{"encryption":"aes-128","init":false,"value":"\E.*\Q"},"two":{"encryption":"aes-128","init":true,"value":"\E.*\Q"}}""".stripMargin.r
-    ParameterEncryption.initialize(ParameterStorageConfig("aes-128", aes128 = Some("ra1V6AfOYAv0jCzEdufIFA==")))
-    val locked = ParameterEncryption.lock(parameters)
+    val locked = parameters.lock(aes128encoder)
     val dbString = locked.toJsObject.toString
     dbString should fullyMatch regex output
   }
 
-  it should "correctly decrypted encrypted values" in {
-    ParameterEncryption.initialize(ParameterStorageConfig("aes-128", aes128 = Some("ra1V6AfOYAv0jCzEdufIFA==")))
-    val locked = ParameterEncryption.lock(parameters)
-    locked.getMap.map(({
+  it should "correctly decrypt encrypted values" in {
+    val locked = parameters.lock(aes128encoder)
+
+    locked.getMap.foreach {
       case (_, paramValue) =>
         paramValue.encryption shouldBe Some("aes-128")
         paramValue.value.convertTo[String] should not be "secret"
-    }))
+    }
 
-    val unlocked = ParameterEncryption.unlock(locked)
-    unlocked.getMap.map(({
+    val unlocked = locked.unlock(aes128decoder)
+    unlocked.getMap.foreach {
       case (_, paramValue) =>
         paramValue.encryption shouldBe empty
         paramValue.value.convertTo[String] shouldBe "secret"
-    }))
+    }
   }
-  it should "correctly decrypted encrypted JsObject values" in {
-    ParameterEncryption.initialize(ParameterStorageConfig("aes-128", aes128 = Some("ra1V6AfOYAv0jCzEdufIFA==")))
+  it should "correctly decrypt encrypted JsObject values" in {
     val obj = Map("key" -> "xyz".toJson, "value" -> "v1".toJson).toJson
-
     val complexParam = new Parameters(Map(new ParameterName("one") -> new ParameterValue(obj, false)))
 
-    val locked = ParameterEncryption.lock(complexParam)
-    locked.getMap.map(({
+    val locked = complexParam.lock(aes128encoder)
+    locked.getMap.foreach {
       case (_, paramValue) =>
         paramValue.encryption shouldBe Some("aes-128")
         paramValue.value.convertTo[String] should not be "secret"
-    }))
+    }
 
-    val unlocked = ParameterEncryption.unlock(locked)
-    unlocked.getMap.map(({
+    val unlocked = locked.unlock(aes128decoder)
+    unlocked.getMap.foreach {
       case (_, paramValue) =>
         paramValue.encryption shouldBe empty
         paramValue.value shouldBe obj
-    }))
+    }
   }
-  it should "correctly decrypted encrypted multiline values" in {
-    ParameterEncryption.initialize(ParameterStorageConfig("aes-128", aes128 = Some("ra1V6AfOYAv0jCzEdufIFA==")))
+  it should "correctly decrypt encrypted multiline values" in {
     val lines = "line1\nline2\nline3\nline4"
     val multiline = new Parameters(Map(new ParameterName("one") -> new ParameterValue(JsString(lines), false)))
 
-    val locked = ParameterEncryption.lock(multiline)
-    locked.getMap.map(({
+    val locked = multiline.lock(aes128encoder)
+    locked.getMap.foreach {
       case (_, paramValue) =>
         paramValue.encryption shouldBe Some("aes-128")
         paramValue.value.convertTo[String] should not be "secret"
-    }))
+    }
 
-    val unlocked = ParameterEncryption.unlock(locked)
-    unlocked.getMap.map(({
+    val unlocked = locked.unlock(aes128decoder)
+    unlocked.getMap.foreach {
       case (_, paramValue) =>
         paramValue.encryption shouldBe empty
         paramValue.value.convertTo[String] shouldBe lines
-    }))
+    }
   }
+
   // Not sure having cancelled tests is a good idea either, need to work on aes256 packaging.
   it should "work if with aes256 if policy allows it" in {
-    ParameterEncryption.initialize(
-      ParameterStorageConfig("aes-256", aes256 = Some("j5rLzhtxwzPyUVUy8/p8XJmBoKeDoSzNJP1SITJEY9E=")))
     try {
-      val locked = ParameterEncryption.lock(parameters)
-      locked.getMap.map(({
+      val locked = parameters.lock(aes256encoder)
+      locked.getMap.foreach {
         case (_, paramValue) =>
           paramValue.encryption shouldBe Some("aes-256")
           paramValue.value.convertTo[String] should not be "secret"
-      }))
+      }
 
-      val unlocked = ParameterEncryption.unlock(locked)
-      unlocked.getMap.map(({
+      val unlocked = locked.unlock(noop)
+      unlocked.getMap.foreach {
         case (_, paramValue) =>
           paramValue.encryption shouldBe empty
           paramValue.value.convertTo[String] shouldBe "secret"
-      }))
+      }
     } catch {
       case e: InvalidAlgorithmParameterException =>
         cancel(e.toString)
     }
   }
+
   it should "support reverting back to Noop encryption" in {
-    ParameterEncryption.initialize(ParameterStorageConfig("aes-128", aes128 = Some("ra1V6AfOYAv0jCzEdufIFA==")))
     try {
-      val locked = ParameterEncryption.lock(parameters)
-      locked.getMap.map(({
+      val locked = parameters.lock(aes128encoder)
+      locked.getMap.foreach {
         case (_, paramValue) =>
           paramValue.encryption shouldBe Some("aes-128")
           paramValue.value.convertTo[String] should not be "secret"
-      }))
+      }
 
       val lockedJson = locked.toJsObject
 
-      // current mode defaults to noop
-      ParameterEncryption.initialize(ParameterStorageConfig(aes128 = Some("ra1V6AfOYAv0jCzEdufIFA==")))
-
       val toDecrypt = Parameters.serdes.read(lockedJson)
 
-      val unlocked = ParameterEncryption.unlock(toDecrypt)
-      unlocked.getMap.map(({
+      // defaults to no-op
+      val unlocked = toDecrypt.unlock(noop)
+      unlocked.getMap.foreach {
         case (_, paramValue) =>
           paramValue.encryption shouldBe empty
           paramValue.value.convertTo[String] shouldBe "secret"
-      }))
+      }
+
       unlocked.toJsObject shouldBe JsObject("one" -> "secret".toJson, "two" -> "secret".toJson)
     } catch {
       case e: InvalidAlgorithmParameterException =>
@@ -238,12 +255,13 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
     }
   }
 
-  behavior of "NoopEncryption"
+  behavior of "No-op Encryption"
+
   it should "not mark parameters as encrypted" in {
-    val locked = ParameterEncryption.lock(parameters)
-    locked.getMap.map(({
+    val locked = parameters.lock(Some(ParameterEncryption.noop))
+    locked.getMap.foreach {
       case (_, paramValue) =>
         paramValue.value.convertTo[String] shouldBe "secret"
-    }))
+    }
   }
 }

From 31ab9353d263e46e3b6a5eb20c8fcb6c0ee0ad39 Mon Sep 17 00:00:00 2001
From: Rodric Rabbah <rodric@gmail.com>
Date: Mon, 9 Mar 2020 16:05:15 -0400
Subject: [PATCH 06/12] Partition params into locked and unlocked sets.

---
 .../openwhisk/core/connector/Message.scala    |   3 +-
 .../openwhisk/core/entity/Parameter.scala     | 103 +++++------------
 .../core/entity/ParameterEncryption.scala     |  18 ++-
 .../openwhisk/core/entity/WhiskAction.scala   |  11 +-
 .../openwhisk/core/entity/WhiskPackage.scala  |   5 +-
 .../controller/actions/PrimitiveActions.scala |   1 +
 .../loadBalancer/InvokerSupervision.scala     |   3 +-
 .../core/containerpool/ContainerProxy.scala   |  22 +++-
 .../test/ContainerPoolTests.scala             |   3 +-
 .../test/ContainerProxyTests.scala            |   3 +-
 .../test/ParameterEncryptionTests.scala       | 105 ++++++------------
 .../test/InvokerSupervisionTests.scala        |   3 +-
 .../ShardingContainerPoolBalancerTests.scala  |   3 +-
 13 files changed, 123 insertions(+), 160 deletions(-)

diff --git a/common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala b/common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala
index 5ca2a71be8c..c86426d4894 100644
--- a/common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala
+++ b/common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala
@@ -57,6 +57,7 @@ case class ActivationMessage(override val transid: TransactionId,
                              blocking: Boolean,
                              content: Option[JsObject],
                              initArgs: Set[String] = Set.empty,
+                             lockedArgs: Map[String, String] = Map.empty,
                              cause: Option[ActivationId] = None,
                              traceContext: Option[Map[String, String]] = None)
     extends Message {
@@ -171,7 +172,7 @@ object ActivationMessage extends DefaultJsonProtocol {
   def parse(msg: String) = Try(serdes.read(msg.parseJson))
 
   private implicit val fqnSerdes = FullyQualifiedEntityName.serdes
-  implicit val serdes = jsonFormat11(ActivationMessage.apply)
+  implicit val serdes = jsonFormat12(ActivationMessage.apply)
 }
 
 object CombinedCompletionAndResultMessage extends DefaultJsonProtocol {
diff --git a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala
index 53dd3fef366..d323729b0a3 100644
--- a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala
+++ b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala
@@ -21,7 +21,6 @@ import org.apache.openwhisk.core.entity.size.{SizeInt, SizeString}
 import spray.json.DefaultJsonProtocol._
 import spray.json._
 
-import scala.collection.immutable.ListMap
 import scala.language.postfixOps
 import scala.util.{Failure, Success, Try}
 
@@ -74,9 +73,17 @@ protected[core] class Parameters protected[entity] (private val params: Map[Para
     params.keySet filter (params(_).init) map (_.name)
   }
 
+  /** Gets list all locked (encrypted) parameters. */
+  protected[core] def lockedParameters: Map[String, String] = {
+    params.collect {
+      case p if p._2.encryption.isDefined => (p._1.name -> p._2.encryption.get)
+    }
+  }
+
   protected[core] def getMap = {
     params
   }
+
   protected[core] def toJsArray = {
     JsArray(params map { p =>
       val init = if (p._2.init) Some("init" -> JsTrue) else None
@@ -86,14 +93,7 @@ protected[core] class Parameters protected[entity] (private val params: Map[Para
     } toSeq: _*)
   }
 
-  protected[core] def toJsObject =
-    JsObject(params.map(p => {
-      val newValue =
-        p._2.encryption
-          .map(e => JsObject("value" -> p._2.value, "init" -> p._2.init.toJson, "encryption" -> e.toJson))
-          .getOrElse(p._2.value)
-      (p._1.name, newValue)
-    }))
+  protected[core] def toJsObject = JsObject(params.map(p => (p._1.name -> p._2.value.toJson)))
 
   override def toString = toJsArray.compactPrint
 
@@ -134,12 +134,13 @@ protected[core] class Parameters protected[entity] (private val params: Map[Para
   /**
    * Encrypts any parameters that are not yet encoded.
    *
+   * @param encoder the encoder to transform parameter values with
    * @return parameters with all values encrypted
    */
-  def lock(encoder: Option[Encrypter] = None): Parameters = {
+  def lock(encoder: Encrypter): Parameters = {
     new Parameters(params.map {
       case (paramName, paramValue) if paramValue.encryption.isEmpty =>
-        paramName -> encoder.getOrElse(ParameterEncryption.singleton.default).encrypt(paramValue)
+        paramName -> encoder.encrypt(paramValue)
       case p => p
     })
   }
@@ -147,15 +148,13 @@ protected[core] class Parameters protected[entity] (private val params: Map[Para
   /**
    * Decodes parameters. If the encryption scheme for a parameter is not recognized, it is not modified.
    *
+   * @param decoder the decoder to use to transform locked values
    * @return parameters will all values decoded (where scheme is known)
    */
-  def unlock(decoder: Option[ParameterEncryption] = None): Parameters = {
+  def unlock(decoder: ParameterEncryption): Parameters = {
     new Parameters(params.map {
-      case (paramName, paramValue) if paramValue.encryption.nonEmpty =>
-        paramName -> decoder
-          .getOrElse(ParameterEncryption.singleton)
-          .encryptor(paramValue.encryption)
-          .decrypt(paramValue)
+      case (paramName, paramValue) if paramValue.encryption.isDefined =>
+        paramName -> decoder.encryptor(paramValue.encryption).decrypt(paramValue)
       case p => p
     })
   }
@@ -265,29 +264,6 @@ protected[core] object Parameters extends ArgNormalizer[Parameters] {
     ParameterValue(Option(v).getOrElse(JsNull), false, None))
   }
 
-  def readMergedList(value: JsValue): Parameters =
-    Try {
-
-      val JsObject(obj) = value
-      new Parameters(
-        obj
-          .map((tuple: (String, JsValue)) => {
-            val key = new ParameterName(tuple._1)
-            val paramVal: ParameterValue = tuple._2 match {
-              case o: JsObject =>
-                o.getFields("value", "init", "encryption") match {
-                  case Seq(v: JsValue, JsBoolean(i), JsString(e)) =>
-                    ParameterValue(v, i, Some(e))
-                  case _ => ParameterValue(o, false, None)
-                }
-              case v: JsValue => ParameterValue(v, false, None)
-            }
-            (key, paramVal)
-          })
-          .toMap)
-    } getOrElse deserializationError(
-      "parameters malformed, could not get a JsObject from: " + (if (value != null) value.toString() else ""))
-
   override protected[core] implicit val serdes = new RootJsonFormat[Parameters] {
     def write(p: Parameters) = p.toJsArray
 
@@ -298,39 +274,12 @@ protected[core] object Parameters extends ArgNormalizer[Parameters] {
      * @param parameters the JSON representation of an parameter array
      * @return Parameters instance if parameters conforms to schema
      */
-    def read(value: JsValue) =
-      Try {
-        val JsArray(params) = value
-        params
-      } flatMap {
-        read(_)
-      } getOrElse {
-        Try {
-          var converted = new ListMap[ParameterName, ParameterValue]()
-          val JsObject(o) = value
-          o.foreach(i =>
-            i._2.asJsObject.getFields("value", "init", "encryption") match {
-              case Seq(v: JsValue, JsBoolean(init), JsString(e)) =>
-                val key = new ParameterName(i._1)
-                val value = ParameterValue(v, init, Some(e))
-                converted = converted + (key -> value)
-              case Seq(v: JsValue, JsBoolean(init)) =>
-                val key = new ParameterName(i._1)
-                val value = ParameterValue(v, init)
-                converted = converted + (key -> value)
-              case Seq(v: JsValue, JsBoolean(init), JsNull) =>
-                val key = new ParameterName(i._1)
-                val value = ParameterValue(v, init)
-                converted = converted + (key -> value)
-          })
-          if (converted.size == 0) {
-            deserializationError("parameters malformed no parameters available: " + value.toString)
-          } else {
-            new Parameters(converted)
-          }
-        } getOrElse deserializationError(
-          "parameters malformed could not read directly: " + (if (value != null) value.toString else ""))
+    def read(value: JsValue): Parameters = {
+      value match {
+        case JsArray(params) => read(params).getOrElse(deserializationError("parameters malformed!"))
+        case _               => deserializationError("parameters malformed!")
       }
+    }
 
     /**
      * Gets parameters as a Parameters instances.
@@ -348,15 +297,19 @@ protected[core] object Parameters extends ArgNormalizer[Parameters] {
                 val key = new ParameterName(k)
                 val value = ParameterValue(v, false)
                 (key, value)
+              case Seq(JsString(k), v: JsValue, JsBoolean(i)) =>
+                val key = new ParameterName(k)
+                val value = ParameterValue(v, i)
+                (key, value)
               case Seq(JsString(k), v: JsValue, JsBoolean(i), JsString(e)) =>
                 val key = new ParameterName(k)
                 val value = ParameterValue(v, i, Some(e))
                 (key, value)
-              case Seq(JsString(k), v: JsValue, JsBoolean(i)) =>
+              case Seq(JsString(k), v: JsValue, JsBoolean(i), JsNull) =>
                 val key = new ParameterName(k)
-                val value = ParameterValue(v, i)
+                val value = ParameterValue(v, i, None)
                 (key, value)
-              case Seq(JsString(k), v: JsValue, JsString(e)) if (i.asJsObject.fields.contains("encryption")) =>
+              case Seq(JsString(k), v: JsValue, JsString(e)) =>
                 val key = new ParameterName(k)
                 val value = ParameterValue(v, false, Some(e))
                 (key, value)
diff --git a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/ParameterEncryption.scala b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/ParameterEncryption.scala
index 3bf77eb6e50..271328e325a 100644
--- a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/ParameterEncryption.scala
+++ b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/ParameterEncryption.scala
@@ -46,6 +46,10 @@ protected[core] case class ParameterEncryption(default: Encrypter, encryptors: M
     encryptors.get(name.getOrElse(default.name)).getOrElse(ParameterEncryption.noop)
   }
 
+  def encryptor(name: String): Encrypter = {
+    encryptors.get(name).getOrElse(ParameterEncryption.noop)
+  }
+
 }
 
 protected[core] object ParameterEncryption {
@@ -79,6 +83,7 @@ protected[core] trait Encrypter {
   val name: String
   def encrypt(p: ParameterValue): ParameterValue = p
   def decrypt(p: ParameterValue): ParameterValue = p
+  def decrypt(v: JsString): JsValue = v
 }
 
 protected[core] object Encrypter {
@@ -116,8 +121,15 @@ protected[core] trait AesEncryption extends Encrypter {
     ParameterValue(JsString(Base64.getEncoder.encodeToString(cipherMessage)), value.init, Some(name))
   }
 
-  override def decrypt(value: ParameterValue): ParameterValue = {
-    val cipherMessage = value.value.convertTo[String].getBytes(StandardCharsets.UTF_8)
+  override def decrypt(p: ParameterValue): ParameterValue = {
+    p.value match {
+      case s: JsString => p.copy(v = decrypt(s), encryption = None)
+      case _           => p
+    }
+  }
+
+  override def decrypt(value: JsString): JsValue = {
+    val cipherMessage = value.convertTo[String].getBytes(StandardCharsets.UTF_8)
     val byteBuffer = ByteBuffer.wrap(Base64.getDecoder.decode(cipherMessage))
     val ivLength = byteBuffer.getInt
     if (ivLength != ivLen) {
@@ -133,7 +145,7 @@ protected[core] trait AesEncryption extends Encrypter {
     cipher.init(Cipher.DECRYPT_MODE, secretKey, gcmSpec)
     val plainTextBytes = cipher.doFinal(cipherText)
     val plainText = new String(plainTextBytes, StandardCharsets.UTF_8)
-    ParameterValue(plainText.parseJson, value.init)
+    plainText.parseJson
   }
 
 }
diff --git a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala
index 85074989346..e61b47d97fb 100644
--- a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala
+++ b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala
@@ -384,7 +384,9 @@ object WhiskAction extends DocumentFactory[WhiskAction] with WhiskEntityQueries[
       val stream = new ByteArrayInputStream(bytes)
       super.putAndAttach(
         db,
-        doc.copy(parameters = doc.parameters.lock()).revision[WhiskAction](doc.rev),
+        doc
+          .copy(parameters = doc.parameters.lock(ParameterEncryption.singleton.default))
+          .revision[WhiskAction](doc.rev),
         attachmentUpdater,
         attachmentType,
         stream,
@@ -404,7 +406,12 @@ object WhiskAction extends DocumentFactory[WhiskAction] with WhiskEntityQueries[
         case exec @ BlackBoxExec(_, Some(Inline(code)), _, _, binary) =>
           putWithAttachment(code, binary, exec)
         case _ =>
-          super.put(db, doc.copy(parameters = doc.parameters.lock()).revision[WhiskAction](doc.rev), old)
+          super.put(
+            db,
+            doc
+              .copy(parameters = doc.parameters.lock(ParameterEncryption.singleton.default))
+              .revision[WhiskAction](doc.rev),
+            old)
       }
     } match {
       case Success(f) => f
diff --git a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskPackage.scala b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskPackage.scala
index 8ded69ed54e..537c2fe440b 100644
--- a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskPackage.scala
+++ b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskPackage.scala
@@ -205,7 +205,10 @@ object WhiskPackage
   override def put[A >: WhiskPackage](db: ArtifactStore[A], doc: WhiskPackage, old: Option[WhiskPackage])(
     implicit transid: TransactionId,
     notifier: Option[CacheChangeNotification]): Future[DocInfo] = {
-    super.put(db, doc.copy(parameters = doc.parameters.lock()).revision[WhiskPackage](doc.rev), old)
+    super.put(
+      db,
+      doc.copy(parameters = doc.parameters.lock(ParameterEncryption.singleton.default)).revision[WhiskPackage](doc.rev),
+      old)
   }
 }
 
diff --git a/core/controller/src/main/scala/org/apache/openwhisk/core/controller/actions/PrimitiveActions.scala b/core/controller/src/main/scala/org/apache/openwhisk/core/controller/actions/PrimitiveActions.scala
index 1ac11515bc8..64ef0e8d79d 100644
--- a/core/controller/src/main/scala/org/apache/openwhisk/core/controller/actions/PrimitiveActions.scala
+++ b/core/controller/src/main/scala/org/apache/openwhisk/core/controller/actions/PrimitiveActions.scala
@@ -179,6 +179,7 @@ protected[actions] trait PrimitiveActions {
       waitForResponse.isDefined,
       args,
       action.parameters.initParameters,
+      action.parameters.lockedParameters,
       cause = cause,
       WhiskTracerProvider.tracer.getTraceContext(transid))
 
diff --git a/core/controller/src/main/scala/org/apache/openwhisk/core/loadBalancer/InvokerSupervision.scala b/core/controller/src/main/scala/org/apache/openwhisk/core/loadBalancer/InvokerSupervision.scala
index e1b7c64e548..5a9d36707db 100644
--- a/core/controller/src/main/scala/org/apache/openwhisk/core/loadBalancer/InvokerSupervision.scala
+++ b/core/controller/src/main/scala/org/apache/openwhisk/core/loadBalancer/InvokerSupervision.scala
@@ -416,7 +416,8 @@ class InvokerActor(invokerInstance: InvokerInstanceId, controllerInstance: Contr
         rootControllerIndex = controllerInstance,
         blocking = false,
         content = None,
-        initArgs = Set.empty)
+        initArgs = Set.empty,
+        lockedArgs = Map.empty)
 
       context.parent ! ActivationRequest(activationMessage, invokerInstance)
     }
diff --git a/core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerProxy.scala b/core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerProxy.scala
index 67cdf433a7b..319b9eb9def 100644
--- a/core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerProxy.scala
+++ b/core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerProxy.scala
@@ -774,9 +774,10 @@ class ContainerProxy(factory: (TransactionId,
   def initializeAndRun(container: Container, job: Run, reschedule: Boolean = false)(
     implicit tid: TransactionId): Future[WhiskActivation] = {
     val actionTimeout = job.action.limits.timeout.duration
-    val unlockedContent = job.msg.content.map(Parameters.readMergedList(_).unlock().toJsObject)
+    val unlockedArgs =
+      ContainerProxy.unlockArguments(job.msg.content, job.msg.lockedArgs, ParameterEncryption.singleton)
 
-    val (env, parameters) = ContainerProxy.partitionArguments(unlockedContent, job.msg.initArgs)
+    val (env, parameters) = ContainerProxy.partitionArguments(unlockedArgs, job.msg.initArgs)
 
     val environment = Map(
       "namespace" -> job.msg.user.namespace.name.toJson,
@@ -1089,6 +1090,23 @@ object ContainerProxy {
         (env, JsObject(args))
     }
   }
+
+  def unlockArguments(content: Option[JsObject],
+                      lockedArgs: Map[String, String],
+                      decoder: ParameterEncryption): Option[JsObject] = {
+    content.map {
+      case JsObject(fields) =>
+        JsObject(fields.map {
+          case (k, o: JsObject) =>
+            o.getFields("value", "encryption") match {
+              case Seq(s: JsString, JsString(e)) => (k -> decoder.encryptor(e).decrypt(s))
+              case Seq(v: JsValue, JsNull)       => (k -> v)
+            }
+
+          case p => p
+        })
+    }
+  }
 }
 
 object TCPPingClient {
diff --git a/tests/src/test/scala/org/apache/openwhisk/core/containerpool/test/ContainerPoolTests.scala b/tests/src/test/scala/org/apache/openwhisk/core/containerpool/test/ContainerPoolTests.scala
index 8c2fea5f17a..63930fef799 100644
--- a/tests/src/test/scala/org/apache/openwhisk/core/containerpool/test/ContainerPoolTests.scala
+++ b/tests/src/test/scala/org/apache/openwhisk/core/containerpool/test/ContainerPoolTests.scala
@@ -87,7 +87,8 @@ class ContainerPoolTests
       ControllerInstanceId("0"),
       blocking = false,
       content = None,
-      initArgs = Set.empty)
+      initArgs = Set.empty,
+      lockedArgs = Map.empty)
     Run(action, message)
   }
 
diff --git a/tests/src/test/scala/org/apache/openwhisk/core/containerpool/test/ContainerProxyTests.scala b/tests/src/test/scala/org/apache/openwhisk/core/containerpool/test/ContainerProxyTests.scala
index 968a5ea36bb..d22fce8367c 100644
--- a/tests/src/test/scala/org/apache/openwhisk/core/containerpool/test/ContainerProxyTests.scala
+++ b/tests/src/test/scala/org/apache/openwhisk/core/containerpool/test/ContainerProxyTests.scala
@@ -125,7 +125,8 @@ class ContainerProxyTests
     ControllerInstanceId("0"),
     blocking = false,
     content = Some(activationArguments),
-    initArgs = Set("ENV_VAR"))
+    initArgs = Set("ENV_VAR"),
+    lockedArgs = Map.empty)
 
   /*
    * Helpers for assertions and actor lifecycles
diff --git a/tests/src/test/scala/org/apache/openwhisk/core/entity/test/ParameterEncryptionTests.scala b/tests/src/test/scala/org/apache/openwhisk/core/entity/test/ParameterEncryptionTests.scala
index bb9cfb88445..a200364b932 100644
--- a/tests/src/test/scala/org/apache/openwhisk/core/entity/test/ParameterEncryptionTests.scala
+++ b/tests/src/test/scala/org/apache/openwhisk/core/entity/test/ParameterEncryptionTests.scala
@@ -28,17 +28,17 @@ import spray.json._
 @RunWith(classOf[JUnitRunner])
 class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfter {
 
-  val noop = Some( // default is no-op but key is available to decodde aes128 encoded params
-    ParameterEncryption(ParameterStorageConfig(aes128 = Some("ra1V6AfOYAv0jCzEdufIFA=="), aes256 = Some(""))))
+  val k128 = "ra1V6AfOYAv0jCzEdufIFA=="
+  val k256 = "j5rLzhtxwzPyUVUy8/p8XJmBoKeDoSzNJP1SITJEY9E="
 
-  val aes128decoder = Some(
-    ParameterEncryption(ParameterStorageConfig("aes-128", aes128 = Some("ra1V6AfOYAv0jCzEdufIFA=="))))
-  val aes128encoder = Some(aes128decoder.get.default)
+  // default is no-op but keys are available to decode encoded params
+  val noop = ParameterEncryption(ParameterStorageConfig(aes128 = Some(k128), aes256 = Some(k256)))
 
-  val aes256decoder = Some(
-    ParameterEncryption(
-      ParameterStorageConfig("aes-256", aes256 = Some("j5rLzhtxwzPyUVUy8/p8XJmBoKeDoSzNJP1SITJEY9E="))))
-  val aes256encoder = Some(aes256decoder.get.default)
+  val aes128decoder = ParameterEncryption(ParameterStorageConfig("aes-128", aes128 = Some(k128)))
+  val aes128encoder = aes128decoder.default
+
+  val aes256decoder = ParameterEncryption(ParameterStorageConfig("aes-256", aes256 = Some(k256)))
+  val aes256encoder = aes256decoder.default
 
   val parameters = new Parameters(
     Map(
@@ -47,34 +47,13 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
 
   behavior of "Parameters"
 
-  it should "handle complex objects in param body" in {
-    val input =
-      """
-        |{
-        |    "__ow_headers": {
-        |        "accept": "*/*",
-        |        "accept-encoding": "gzip, deflate",
-        |        "host": "controllers",
-        |        "user-agent": "Apache-HttpClient/4.5.5 (Java/1.8.0_212)",
-        |        "x-request-id": "fd2263668266da5a5433109076191d95"
-        |    },
-        |    "__ow_method": "get",
-        |    "__ow_path": "/a",
-        |    "a": "A"
-        |}
-        |""".stripMargin
-
-    val p = Parameters.readMergedList(input.parseJson)
-    p.get("a").get.convertTo[String] shouldBe "A"
-  }
-
   it should "handle decryption of json objects" in {
     val originalValue =
       """
-        |{
-        |"paramName1":{"init":false,"value":"from-action"},
-        |"paramName2":{"init":false,"value":"from-pack"}
-        |}
+        |[
+        |  { "key": "paramName1", "init": false, "value": "from-action" },
+        |  { "key": "paramName2", "init": false, "value": "from-pack" }
+        |]
         |""".stripMargin
 
     val p = Parameters.serdes.read(originalValue.parseJson)
@@ -85,10 +64,10 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
   it should "handle decryption of json objects with null field" in {
     val originalValue =
       """
-        |{
-        |"paramName1":{"encryption":null,"init":false,"value":"from-action"},
-        |"paramName2":{"encryption":null,"init":false,"value":"from-pack"}
-        |}
+        |[
+        |  { "key": "paramName1", "encryption":null, "init": false, "value": "from-action" },
+        |  { "key": "paramName2", "encryption":null, "init": false, "value": "from-pack" }
+        |]
         |""".stripMargin
 
     val p = Parameters.serdes.read(originalValue.parseJson)
@@ -96,41 +75,27 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
     p.get("paramName2").get.convertTo[String] shouldBe "from-pack"
   }
 
-  it should "drop encryption of payload when no longer encrypted" in {
+  it should "drop encryption propery when no longer encrypted" in {
     val originalValue =
       """
-        |{
-        |"paramName1":{"encryption":null,"init":false,"value":"from-action"},
-        |"paramName2":{"encryption":null,"init":false,"value":"from-action"}
-        |}
+        |[
+        |  { "key": "paramName1", "encryption":null, "init": false, "value": "from-action" },
+        |  { "key": "paramName2", "encryption":null, "init": false, "value": "from-pack" }
+        |]
         |""".stripMargin
 
     val p = Parameters.serdes.read(originalValue.parseJson)
-    p.toJsObject.fields.foreach {
-      case tuple => tuple._2.convertTo[String] shouldBe "from-action"
-    }
-  }
-
-  it should "read the merged unencrypted parameters during mixed storage" in {
-    val originalValue =
-      """
-        |{"name":"from-action","other":"from-action"}
-        |""".stripMargin
-
-    val p = Parameters.readMergedList(originalValue.parseJson)
-    p.toJsObject.fields.foreach {
-      case tuple => tuple._2.convertTo[String] shouldBe "from-action"
-    }
+    Parameters.serdes.write(p).compactPrint should not include "encryption"
   }
 
   it should "read the merged message payload from kafka into parameters" in {
     val locked = parameters.lock(aes128encoder)
     val mixedParams = locked.merge(Some(Parameters("plain", "test-plain").toJsObject))
-    val params = Parameters.readMergedList(mixedParams.get)
-    params.get("one").get shouldBe locked.get("one").get
-    params.get("two").get shouldBe locked.get("two").get
-    params.get("two").get should not be locked.get("one").get
-    params.get("plain").get shouldBe JsString("test-plain")
+    mixedParams shouldBe defined
+    mixedParams.get.fields("one") shouldBe locked.get("one").get
+    mixedParams.get.fields("two") shouldBe locked.get("two").get
+    mixedParams.get.fields("two") should not be locked.get("one").get
+    mixedParams.get.fields("plain") shouldBe JsString("test-plain")
   }
 
   behavior of "AesParameterEncryption"
@@ -145,11 +110,9 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
   }
 
   it should "serialize to json correctly" in {
-    val output =
-      """\Q{"one":{"encryption":"aes-128","init":false,"value":"\E.*\Q"},"two":{"encryption":"aes-128","init":true,"value":"\E.*\Q"}}""".stripMargin.r
     val locked = parameters.lock(aes128encoder)
-    val dbString = locked.toJsObject.toString
-    dbString should fullyMatch regex output
+    locked.toJsObject.toString should fullyMatch regex """\Q{"one":"\E.*\Q","two":"\E.*\Q"}""".stripMargin.r
+    locked.lockedParameters shouldBe Map("one" -> "aes-128", "two" -> "aes-128")
   }
 
   it should "correctly decrypt encrypted values" in {
@@ -168,6 +131,7 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
         paramValue.value.convertTo[String] shouldBe "secret"
     }
   }
+
   it should "correctly decrypt encrypted JsObject values" in {
     val obj = Map("key" -> "xyz".toJson, "value" -> "v1".toJson).toJson
     val complexParam = new Parameters(Map(new ParameterName("one") -> new ParameterValue(obj, false)))
@@ -236,9 +200,8 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
           paramValue.value.convertTo[String] should not be "secret"
       }
 
-      val lockedJson = locked.toJsObject
-
-      val toDecrypt = Parameters.serdes.read(lockedJson)
+      val lockedJson = Parameters.serdes.write(locked).compactPrint
+      val toDecrypt = Parameters.serdes.read(lockedJson.parseJson)
 
       // defaults to no-op
       val unlocked = toDecrypt.unlock(noop)
@@ -258,7 +221,7 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
   behavior of "No-op Encryption"
 
   it should "not mark parameters as encrypted" in {
-    val locked = parameters.lock(Some(ParameterEncryption.noop))
+    val locked = parameters.lock(ParameterEncryption.noop)
     locked.getMap.foreach {
       case (_, paramValue) =>
         paramValue.value.convertTo[String] shouldBe "secret"
diff --git a/tests/src/test/scala/org/apache/openwhisk/core/loadBalancer/test/InvokerSupervisionTests.scala b/tests/src/test/scala/org/apache/openwhisk/core/loadBalancer/test/InvokerSupervisionTests.scala
index aa07914588e..1cf73834b82 100644
--- a/tests/src/test/scala/org/apache/openwhisk/core/loadBalancer/test/InvokerSupervisionTests.scala
+++ b/tests/src/test/scala/org/apache/openwhisk/core/loadBalancer/test/InvokerSupervisionTests.scala
@@ -195,7 +195,8 @@ class InvokerSupervisionTests
       rootControllerIndex = ControllerInstanceId("0"),
       blocking = false,
       content = None,
-      initArgs = Set.empty)
+      initArgs = Set.empty,
+      lockedArgs = Map.empty)
     val msg = ActivationRequest(activationMessage, invokerInstance)
 
     supervisor ! msg
diff --git a/tests/src/test/scala/org/apache/openwhisk/core/loadBalancer/test/ShardingContainerPoolBalancerTests.scala b/tests/src/test/scala/org/apache/openwhisk/core/loadBalancer/test/ShardingContainerPoolBalancerTests.scala
index dab21aa0e9a..11fee6d002b 100644
--- a/tests/src/test/scala/org/apache/openwhisk/core/loadBalancer/test/ShardingContainerPoolBalancerTests.scala
+++ b/tests/src/test/scala/org/apache/openwhisk/core/loadBalancer/test/ShardingContainerPoolBalancerTests.scala
@@ -508,7 +508,8 @@ class ShardingContainerPoolBalancerTests
         ControllerInstanceId("0"),
         blocking = false,
         content = None,
-        initArgs = Set.empty)
+        initArgs = Set.empty,
+        lockedArgs = Map.empty)
 
       //send activation to loadbalancer
       aid -> balancer.publish(actionMetaData.toExecutableWhiskAction.get, msg)

From 3705c4eca0275fe4b8e47053fa107fee87f65cab Mon Sep 17 00:00:00 2001
From: Rodric Rabbah <rodric@gmail.com>
Date: Mon, 9 Mar 2020 16:14:02 -0400
Subject: [PATCH 07/12] Remove getter, make field protected/accessible for
 test.

---
 .../openwhisk/core/entity/Parameter.scala     | 11 +++-----
 .../test/ParameterEncryptionTests.scala       | 25 ++++++++++---------
 2 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala
index d323729b0a3..ab494447357 100644
--- a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala
+++ b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala
@@ -17,12 +17,13 @@
 
 package org.apache.openwhisk.core.entity
 
-import org.apache.openwhisk.core.entity.size.{SizeInt, SizeString}
+import scala.util.{Failure, Success, Try}
 import spray.json.DefaultJsonProtocol._
 import spray.json._
 
 import scala.language.postfixOps
-import scala.util.{Failure, Success, Try}
+import org.apache.openwhisk.core.entity.size.SizeInt
+import org.apache.openwhisk.core.entity.size.SizeString
 
 /**
  * Parameters is a key-value map from parameter names to parameter values. The value of a
@@ -31,7 +32,7 @@ import scala.util.{Failure, Success, Try}
  * @param key the parameter name, assured to be non-null because it is a value
  * @param value the parameter value, assured to be non-null because it is a value
  */
-protected[core] class Parameters protected[entity] (private val params: Map[ParameterName, ParameterValue])
+protected[core] class Parameters protected[entity] (protected[entity] val params: Map[ParameterName, ParameterValue])
     extends AnyVal {
 
   /**
@@ -80,10 +81,6 @@ protected[core] class Parameters protected[entity] (private val params: Map[Para
     }
   }
 
-  protected[core] def getMap = {
-    params
-  }
-
   protected[core] def toJsArray = {
     JsArray(params map { p =>
       val init = if (p._2.init) Some("init" -> JsTrue) else None
diff --git a/tests/src/test/scala/org/apache/openwhisk/core/entity/test/ParameterEncryptionTests.scala b/tests/src/test/scala/org/apache/openwhisk/core/entity/test/ParameterEncryptionTests.scala
index a200364b932..14cdab49f23 100644
--- a/tests/src/test/scala/org/apache/openwhisk/core/entity/test/ParameterEncryptionTests.scala
+++ b/tests/src/test/scala/org/apache/openwhisk/core/entity/test/ParameterEncryptionTests.scala
@@ -102,7 +102,8 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
 
   it should "correctly mark the encrypted parameters after lock" in {
     val locked = parameters.lock(aes128encoder)
-    locked.getMap.foreach {
+
+    locked.params.foreach {
       case (_, paramValue) =>
         paramValue.encryption shouldBe Some("aes-128")
         paramValue.value.convertTo[String] should not be "secret"
@@ -118,14 +119,14 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
   it should "correctly decrypt encrypted values" in {
     val locked = parameters.lock(aes128encoder)
 
-    locked.getMap.foreach {
+    locked.params.foreach {
       case (_, paramValue) =>
         paramValue.encryption shouldBe Some("aes-128")
         paramValue.value.convertTo[String] should not be "secret"
     }
 
     val unlocked = locked.unlock(aes128decoder)
-    unlocked.getMap.foreach {
+    unlocked.params.foreach {
       case (_, paramValue) =>
         paramValue.encryption shouldBe empty
         paramValue.value.convertTo[String] shouldBe "secret"
@@ -137,14 +138,14 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
     val complexParam = new Parameters(Map(new ParameterName("one") -> new ParameterValue(obj, false)))
 
     val locked = complexParam.lock(aes128encoder)
-    locked.getMap.foreach {
+    locked.params.foreach {
       case (_, paramValue) =>
         paramValue.encryption shouldBe Some("aes-128")
         paramValue.value.convertTo[String] should not be "secret"
     }
 
     val unlocked = locked.unlock(aes128decoder)
-    unlocked.getMap.foreach {
+    unlocked.params.foreach {
       case (_, paramValue) =>
         paramValue.encryption shouldBe empty
         paramValue.value shouldBe obj
@@ -155,14 +156,14 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
     val multiline = new Parameters(Map(new ParameterName("one") -> new ParameterValue(JsString(lines), false)))
 
     val locked = multiline.lock(aes128encoder)
-    locked.getMap.foreach {
+    locked.params.foreach {
       case (_, paramValue) =>
         paramValue.encryption shouldBe Some("aes-128")
         paramValue.value.convertTo[String] should not be "secret"
     }
 
     val unlocked = locked.unlock(aes128decoder)
-    unlocked.getMap.foreach {
+    unlocked.params.foreach {
       case (_, paramValue) =>
         paramValue.encryption shouldBe empty
         paramValue.value.convertTo[String] shouldBe lines
@@ -173,14 +174,14 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
   it should "work if with aes256 if policy allows it" in {
     try {
       val locked = parameters.lock(aes256encoder)
-      locked.getMap.foreach {
+      locked.params.foreach {
         case (_, paramValue) =>
           paramValue.encryption shouldBe Some("aes-256")
           paramValue.value.convertTo[String] should not be "secret"
       }
 
       val unlocked = locked.unlock(noop)
-      unlocked.getMap.foreach {
+      unlocked.params.foreach {
         case (_, paramValue) =>
           paramValue.encryption shouldBe empty
           paramValue.value.convertTo[String] shouldBe "secret"
@@ -194,7 +195,7 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
   it should "support reverting back to Noop encryption" in {
     try {
       val locked = parameters.lock(aes128encoder)
-      locked.getMap.foreach {
+      locked.params.foreach {
         case (_, paramValue) =>
           paramValue.encryption shouldBe Some("aes-128")
           paramValue.value.convertTo[String] should not be "secret"
@@ -205,7 +206,7 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
 
       // defaults to no-op
       val unlocked = toDecrypt.unlock(noop)
-      unlocked.getMap.foreach {
+      unlocked.params.foreach {
         case (_, paramValue) =>
           paramValue.encryption shouldBe empty
           paramValue.value.convertTo[String] shouldBe "secret"
@@ -222,7 +223,7 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
 
   it should "not mark parameters as encrypted" in {
     val locked = parameters.lock(ParameterEncryption.noop)
-    locked.getMap.foreach {
+    locked.params.foreach {
       case (_, paramValue) =>
         paramValue.value.convertTo[String] shouldBe "secret"
     }

From 12ba958d76ed2dc1f8b4a1780b51e3cbed717e7c Mon Sep 17 00:00:00 2001
From: Rodric Rabbah <rodric@gmail.com>
Date: Mon, 9 Mar 2020 16:20:24 -0400
Subject: [PATCH 08/12] Comments.

---
 ansible/group_vars/all                        |  4 +-
 .../scala/src/main/resources/application.conf | 14 ++--
 .../openwhisk/core/entity/Parameter.scala     | 65 +++++++++----------
 tests/src/test/resources/application.conf.j2  |  2 +-
 4 files changed, 43 insertions(+), 42 deletions(-)

diff --git a/ansible/group_vars/all b/ansible/group_vars/all
index 0c54c92243f..342ad1d7938 100644
--- a/ansible/group_vars/all
+++ b/ansible/group_vars/all
@@ -49,8 +49,8 @@ whisk:
   version:
     date: "{{ansible_date_time.iso8601}}"
   feature_flags:
-    require_api_key_annotation: "{{ require_api_key_annotation | default(true) | lower  }}"
-    require_response_payload: "{{ require_response_payload | default(true) | lower  }}"
+    require_api_key_annotation: "{{ require_api_key_annotation | default(true) | lower }}"
+    require_response_payload: "{{ require_response_payload | default(true) | lower }}"
 
 ##
 # configuration parameters related to support runtimes (see org.apache.openwhisk.core.entity.ExecManifest for schema of the manifest).
diff --git a/common/scala/src/main/resources/application.conf b/common/scala/src/main/resources/application.conf
index 50a748a5db5..24b3b2bfbf6 100644
--- a/common/scala/src/main/resources/application.conf
+++ b/common/scala/src/main/resources/application.conf
@@ -567,13 +567,17 @@ whisk {
     # it will slowly migrate all the actions that have been 'updated' to use encrypted parameters but going back would
     # require a currently non-existing migration step.
     parameter-storage {
-        # Base64 encoded 256 bit key
-        #aes-256 = ""
-        # Base64 encoded 128 bit key
-        #aes-128 = ""
         # The current algorithm to use for parameter encryption, this can be changed but you have to leave all the keys
         # configured for any algorithm you used previously.
-        #current = "aes-128|aes-256"
+        # Allowed values:
+        #   "noop"    -> no op/no encryption
+        #   "aes-128" -> AES with 128 bit key (given as base64 encoded string)
+        #   "aes-256" -> AES with 256 bit key (given as base64 encoded string)
+        current = "noop"
+        # Base64 encoded 128 bit key
+        #aes-128 = ""
+        # Base64 encoded 256 bit key
+        #aes-256 = ""
     }
 }
 #placeholder for test overrides so that tests can override defaults in application.conf (todo: move all defaults to reference.conf)
diff --git a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala
index ab494447357..e4775ed3aa4 100644
--- a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala
+++ b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala
@@ -64,17 +64,17 @@ protected[core] class Parameters protected[entity] (protected[entity] val params
     Try(new Parameters(params - new ParameterName(p))) getOrElse this
   }
 
-  /** Gets list all defined parameters. */
+  /** Gets set of all defined parameters. */
   protected[core] def definedParameters: Set[String] = {
     params.keySet filter (params(_).isDefined) map (_.name)
   }
 
-  /** Gets list all defined parameters. */
+  /** Gets set of all defined parameters. */
   protected[core] def initParameters: Set[String] = {
     params.keySet filter (params(_).init) map (_.name)
   }
 
-  /** Gets list all locked (encrypted) parameters. */
+  /** Gets map of all locked (encrypted) parameters. */
   protected[core] def lockedParameters: Map[String, String] = {
     params.collect {
       case p if p._2.encryption.isDefined => (p._1.name -> p._2.encryption.get)
@@ -217,8 +217,8 @@ protected[core] object Parameters extends ArgNormalizer[Parameters] {
    * Creates a parameter tuple from a pair of strings.
    * A convenience method for tests.
    *
-   * @param p the parameter name
-   * @param v the parameter value
+   * @param p    the parameter name
+   * @param v    the parameter value
    * @param init the parameter is for initialization
    * @return (ParameterName, ParameterValue)
    * @throws IllegalArgumentException if key is not defined
@@ -233,8 +233,8 @@ protected[core] object Parameters extends ArgNormalizer[Parameters] {
   /**
    * Creates a parameter tuple from a parameter name and JsValue.
    *
-   * @param p the parameter name
-   * @param v the parameter value
+   * @param p    the parameter name
+   * @param v    the parameter value
    * @param init the parameter is for initialization
    * @return (ParameterName, ParameterValue)
    * @throws IllegalArgumentException if key is not defined
@@ -286,33 +286,30 @@ protected[core] object Parameters extends ArgNormalizer[Parameters] {
      * @return Parameters instance if parameters conforms to schema
      */
     def read(params: Vector[JsValue]) = Try {
-      new Parameters(
-        params
-          .map(i => {
-            i.asJsObject.getFields("key", "value", "init", "encryption") match {
-              case Seq(JsString(k), v: JsValue) =>
-                val key = new ParameterName(k)
-                val value = ParameterValue(v, false)
-                (key, value)
-              case Seq(JsString(k), v: JsValue, JsBoolean(i)) =>
-                val key = new ParameterName(k)
-                val value = ParameterValue(v, i)
-                (key, value)
-              case Seq(JsString(k), v: JsValue, JsBoolean(i), JsString(e)) =>
-                val key = new ParameterName(k)
-                val value = ParameterValue(v, i, Some(e))
-                (key, value)
-              case Seq(JsString(k), v: JsValue, JsBoolean(i), JsNull) =>
-                val key = new ParameterName(k)
-                val value = ParameterValue(v, i, None)
-                (key, value)
-              case Seq(JsString(k), v: JsValue, JsString(e)) =>
-                val key = new ParameterName(k)
-                val value = ParameterValue(v, false, Some(e))
-                (key, value)
-            }
-          })
-          .toMap)
+      new Parameters(params.map {
+        _.asJsObject.getFields("key", "value", "init", "encryption") match {
+          case Seq(JsString(k), v: JsValue) =>
+            val key = new ParameterName(k)
+            val value = ParameterValue(v, false)
+            (key, value)
+          case Seq(JsString(k), v: JsValue, JsBoolean(i)) =>
+            val key = new ParameterName(k)
+            val value = ParameterValue(v, i)
+            (key, value)
+          case Seq(JsString(k), v: JsValue, JsBoolean(i), JsString(e)) =>
+            val key = new ParameterName(k)
+            val value = ParameterValue(v, i, Some(e))
+            (key, value)
+          case Seq(JsString(k), v: JsValue, JsBoolean(i), JsNull) =>
+            val key = new ParameterName(k)
+            val value = ParameterValue(v, i, None)
+            (key, value)
+          case Seq(JsString(k), v: JsValue, JsString(e)) =>
+            val key = new ParameterName(k)
+            val value = ParameterValue(v, false, Some(e))
+            (key, value)
+        }
+      }.toMap)
     }
   }
 }
diff --git a/tests/src/test/resources/application.conf.j2 b/tests/src/test/resources/application.conf.j2
index 5f61695a559..ea10e6371d5 100644
--- a/tests/src/test/resources/application.conf.j2
+++ b/tests/src/test/resources/application.conf.j2
@@ -95,7 +95,7 @@ whisk {
     }
 
     parameter-storage {
-        key = ""
+        current = "noop"
     }
     
     elasticsearch {

From 78c4f6aa81c84dce36206eb30a76895fdb4bbcff Mon Sep 17 00:00:00 2001
From: Rodric Rabbah <rodric@gmail.com>
Date: Mon, 9 Mar 2020 17:01:45 -0400
Subject: [PATCH 09/12] Revert changes to test suite.

---
 .../controller/test/ActionsApiTests.scala     | 65 ++++++++++---------
 1 file changed, 33 insertions(+), 32 deletions(-)

diff --git a/tests/src/test/scala/org/apache/openwhisk/core/controller/test/ActionsApiTests.scala b/tests/src/test/scala/org/apache/openwhisk/core/controller/test/ActionsApiTests.scala
index 391712227f8..9a1a96e7227 100644
--- a/tests/src/test/scala/org/apache/openwhisk/core/controller/test/ActionsApiTests.scala
+++ b/tests/src/test/scala/org/apache/openwhisk/core/controller/test/ActionsApiTests.scala
@@ -19,28 +19,29 @@ package org.apache.openwhisk.core.controller.test
 
 import java.time.Instant
 
-import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport.{sprayJsonMarshaller, sprayJsonUnmarshaller}
+import scala.concurrent.duration.DurationInt
+import scala.language.postfixOps
+import org.junit.runner.RunWith
+import org.scalatest.junit.JUnitRunner
 import akka.http.scaladsl.model.StatusCodes._
-import akka.http.scaladsl.model.headers.RawHeader
+import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport.sprayJsonMarshaller
+import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport.sprayJsonUnmarshaller
 import akka.http.scaladsl.server.Route
-import org.apache.commons.lang3.StringUtils
-import org.apache.openwhisk.core.connector.ActivationMessage
+import spray.json._
+import spray.json.DefaultJsonProtocol._
 import org.apache.openwhisk.core.controller.WhiskActionsApi
-import org.apache.openwhisk.core.database.UserContext
-import org.apache.openwhisk.core.entitlement.Collection
-import org.apache.openwhisk.core.entity.Attachments.Inline
 import org.apache.openwhisk.core.entity._
 import org.apache.openwhisk.core.entity.size._
+import org.apache.openwhisk.core.entitlement.Collection
+import org.apache.openwhisk.http.ErrorResponse
+import org.apache.openwhisk.http.Messages
+import org.apache.openwhisk.core.database.UserContext
+import akka.http.scaladsl.model.headers.RawHeader
+import org.apache.commons.lang3.StringUtils
+import org.apache.openwhisk.core.connector.ActivationMessage
+import org.apache.openwhisk.core.entity.Attachments.Inline
 import org.apache.openwhisk.core.entity.test.ExecHelpers
-import org.apache.openwhisk.http.{ErrorResponse, Messages}
-import org.junit.runner.RunWith
-import org.scalatest.junit.JUnitRunner
 import org.scalatest.{FlatSpec, Matchers}
-import spray.json.DefaultJsonProtocol._
-import spray.json._
-
-import scala.concurrent.duration.DurationInt
-import scala.language.postfixOps
 
 /**
  * Tests Actions API.
@@ -223,22 +224,22 @@ class ActionsApiTests extends ControllerTestCommon with WhiskActionsApi {
     }
   }
 
-//  it should "ignore updated field when updating action" in {
-//    implicit val tid = transid()
-//
-//    val action = WhiskAction(namespace, aname(), jsDefault(""))
-//    val dummyUpdated = WhiskEntity.currentMillis().toEpochMilli
-//
-//    val content = JsObject(
-//      "exec" -> JsObject("code" -> "".toJson, "kind" -> action.exec.kind.toJson),
-//      "updated" -> dummyUpdated.toJson)
-//
-//    Put(s"$collectionPath/${action.name}", content) ~> Route.seal(routes(creds)) ~> check {
-//      status should be(OK)
-//      val response = responseAs[WhiskAction]
-//      response.updated.toEpochMilli should be > dummyUpdated
-//    }
-//  }
+  it should "ignore updated field when updating action" in {
+    implicit val tid = transid()
+
+    val action = WhiskAction(namespace, aname(), jsDefault(""))
+    val dummyUpdated = WhiskEntity.currentMillis().toEpochMilli
+
+    val content = JsObject(
+      "exec" -> JsObject("code" -> "".toJson, "kind" -> action.exec.kind.toJson),
+      "updated" -> dummyUpdated.toJson)
+
+    Put(s"$collectionPath/${action.name}", content) ~> Route.seal(routes(creds)) ~> check {
+      status should be(OK)
+      val response = responseAs[WhiskAction]
+      response.updated.toEpochMilli should be > dummyUpdated
+    }
+  }
 
   def getExecPermutations() = {
     implicit val tid = transid()
@@ -1702,9 +1703,9 @@ class ActionsApiTests extends ControllerTestCommon with WhiskActionsApi {
 
 @RunWith(classOf[JUnitRunner])
 class WhiskActionsApiTests extends FlatSpec with Matchers with ExecHelpers {
+  import WhiskActionsApi.amendAnnotations
   import Annotations.ProvideApiKeyAnnotationName
   import WhiskAction.execFieldName
-  import WhiskActionsApi.amendAnnotations
 
   val baseParams = Parameters("a", JsString("A")) ++ Parameters("b", JsString("B"))
   val keyTruthyAnnotation = Parameters(ProvideApiKeyAnnotationName, JsTrue)

From af0bc080d176a1e1885f59fcf366bc0116943a3e Mon Sep 17 00:00:00 2001
From: Rodric Rabbah <rodric@gmail.com>
Date: Mon, 9 Mar 2020 19:47:37 -0400
Subject: [PATCH 10/12] Exclude overriden parameters from decryption.

---
 .../org/apache/openwhisk/core/entity/Parameter.scala  |  8 +++++---
 .../core/controller/actions/PrimitiveActions.scala    |  2 +-
 .../openwhisk/core/containerpool/ContainerProxy.scala | 10 +++-------
 .../core/entity/test/ParameterEncryptionTests.scala   | 11 ++++++++++-
 4 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala
index e4775ed3aa4..17b19625786 100644
--- a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala
+++ b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala
@@ -74,10 +74,12 @@ protected[core] class Parameters protected[entity] (protected[entity] val params
     params.keySet filter (params(_).init) map (_.name)
   }
 
-  /** Gets map of all locked (encrypted) parameters. */
-  protected[core] def lockedParameters: Map[String, String] = {
+  /**
+   * Gets map of all locked (encrypted) parameters, excluding parameters from given set.
+   */
+  protected[core] def lockedParameters(exclude: Set[String] = Set.empty): Map[String, String] = {
     params.collect {
-      case p if p._2.encryption.isDefined => (p._1.name -> p._2.encryption.get)
+      case p if p._2.encryption.isDefined && !exclude.contains(p._1.name) => (p._1.name -> p._2.encryption.get)
     }
   }
 
diff --git a/core/controller/src/main/scala/org/apache/openwhisk/core/controller/actions/PrimitiveActions.scala b/core/controller/src/main/scala/org/apache/openwhisk/core/controller/actions/PrimitiveActions.scala
index 64ef0e8d79d..621a10e2f19 100644
--- a/core/controller/src/main/scala/org/apache/openwhisk/core/controller/actions/PrimitiveActions.scala
+++ b/core/controller/src/main/scala/org/apache/openwhisk/core/controller/actions/PrimitiveActions.scala
@@ -179,7 +179,7 @@ protected[actions] trait PrimitiveActions {
       waitForResponse.isDefined,
       args,
       action.parameters.initParameters,
-      action.parameters.lockedParameters,
+      action.parameters.lockedParameters(payload.map(_.fields.keySet).getOrElse(Set.empty)),
       cause = cause,
       WhiskTracerProvider.tracer.getTraceContext(transid))
 
diff --git a/core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerProxy.scala b/core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerProxy.scala
index 319b9eb9def..d84947b5f5b 100644
--- a/core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerProxy.scala
+++ b/core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerProxy.scala
@@ -754,6 +754,7 @@ class ContainerProxy(factory: (TransactionId,
     }
     hpa ! HealthPingEnabled(true)
   }
+
   private def disableHealthPing() = {
     healthPingActor.foreach(_ ! HealthPingEnabled(false))
   }
@@ -1097,13 +1098,8 @@ object ContainerProxy {
     content.map {
       case JsObject(fields) =>
         JsObject(fields.map {
-          case (k, o: JsObject) =>
-            o.getFields("value", "encryption") match {
-              case Seq(s: JsString, JsString(e)) => (k -> decoder.encryptor(e).decrypt(s))
-              case Seq(v: JsValue, JsNull)       => (k -> v)
-            }
-
-          case p => p
+          case (k, v: JsString) if lockedArgs.contains(k) => (k -> decoder.encryptor(lockedArgs(k)).decrypt(v))
+          case p                                          => p
         })
     }
   }
diff --git a/tests/src/test/scala/org/apache/openwhisk/core/entity/test/ParameterEncryptionTests.scala b/tests/src/test/scala/org/apache/openwhisk/core/entity/test/ParameterEncryptionTests.scala
index 14cdab49f23..090e5cfa60c 100644
--- a/tests/src/test/scala/org/apache/openwhisk/core/entity/test/ParameterEncryptionTests.scala
+++ b/tests/src/test/scala/org/apache/openwhisk/core/entity/test/ParameterEncryptionTests.scala
@@ -113,7 +113,16 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
   it should "serialize to json correctly" in {
     val locked = parameters.lock(aes128encoder)
     locked.toJsObject.toString should fullyMatch regex """\Q{"one":"\E.*\Q","two":"\E.*\Q"}""".stripMargin.r
-    locked.lockedParameters shouldBe Map("one" -> "aes-128", "two" -> "aes-128")
+    locked.lockedParameters() shouldBe Map("one" -> "aes-128", "two" -> "aes-128")
+  }
+
+  it should "serialize to json correctly when a locked parameter is overriden" in {
+    val locked = parameters.lock(aes128encoder)
+    locked
+      .merge(Some(JsObject("one" -> JsString("override"))))
+      .get
+      .compactPrint should fullyMatch regex """\Q{"one":"override","two":"\E.*\Q"}""".stripMargin.r
+    locked.lockedParameters(Set("one")) shouldBe Map("two" -> "aes-128")
   }
 
   it should "correctly decrypt encrypted values" in {

From 1020e256ff1e2f4010af55256073c76635625494 Mon Sep 17 00:00:00 2001
From: Rodric Rabbah <rodric@gmail.com>
Date: Tue, 10 Mar 2020 09:43:23 -0400
Subject: [PATCH 11/12] Tighten tests.

Add test for unlocking args in container proxy.
---
 .../scala/src/main/resources/application.conf |  8 +++----
 .../openwhisk/core/entity/Parameter.scala     | 23 +++++++++++-------
 .../core/entity/ParameterEncryption.scala     | 20 ++++++++--------
 .../openwhisk/core/entity/WhiskAction.scala   |  1 +
 .../openwhisk/core/entity/WhiskPackage.scala  |  2 ++
 tests/src/test/resources/application.conf.j2  |  2 +-
 .../test/ContainerProxyTests.scala            | 12 ++++++++++
 .../test/ParameterEncryptionTests.scala       | 24 ++++++++++++++++++-
 8 files changed, 67 insertions(+), 25 deletions(-)

diff --git a/common/scala/src/main/resources/application.conf b/common/scala/src/main/resources/application.conf
index 24b3b2bfbf6..ea96e92709a 100644
--- a/common/scala/src/main/resources/application.conf
+++ b/common/scala/src/main/resources/application.conf
@@ -570,10 +570,10 @@ whisk {
         # The current algorithm to use for parameter encryption, this can be changed but you have to leave all the keys
         # configured for any algorithm you used previously.
         # Allowed values:
-        #   "noop"    -> no op/no encryption
-        #   "aes-128" -> AES with 128 bit key (given as base64 encoded string)
-        #   "aes-256" -> AES with 256 bit key (given as base64 encoded string)
-        current = "noop"
+        #   "off|noop" -> no op/no encryption
+        #   "aes-128"  -> AES with 128 bit key (given as base64 encoded string)
+        #   "aes-256"  -> AES with 256 bit key (given as base64 encoded string)
+        current = "off"
         # Base64 encoded 128 bit key
         #aes-128 = ""
         # Base64 encoded 256 bit key
diff --git a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala
index 17b19625786..9470191b4a7 100644
--- a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala
+++ b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala
@@ -136,12 +136,16 @@ protected[core] class Parameters protected[entity] (protected[entity] val params
    * @param encoder the encoder to transform parameter values with
    * @return parameters with all values encrypted
    */
-  def lock(encoder: Encrypter): Parameters = {
-    new Parameters(params.map {
-      case (paramName, paramValue) if paramValue.encryption.isEmpty =>
-        paramName -> encoder.encrypt(paramValue)
-      case p => p
-    })
+  def lock(encoder: Option[Encrypter] = None): Parameters = {
+    encoder
+      .map { coder =>
+        new Parameters(params.map {
+          case (paramName, paramValue) if paramValue.encryption.isEmpty =>
+            paramName -> coder.encrypt(paramValue)
+          case p => p
+        })
+      }
+      .getOrElse(this)
   }
 
   /**
@@ -152,9 +156,10 @@ protected[core] class Parameters protected[entity] (protected[entity] val params
    */
   def unlock(decoder: ParameterEncryption): Parameters = {
     new Parameters(params.map {
-      case (paramName, paramValue) if paramValue.encryption.isDefined =>
-        paramName -> decoder.encryptor(paramValue.encryption).decrypt(paramValue)
-      case p => p
+      case p @ (paramName, paramValue) =>
+        paramValue.encryption
+          .map(paramName -> decoder.encryptor(_).decrypt(paramValue))
+          .getOrElse(p)
     })
   }
 
diff --git a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/ParameterEncryption.scala b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/ParameterEncryption.scala
index 271328e325a..2169eaf3ff6 100644
--- a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/ParameterEncryption.scala
+++ b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/ParameterEncryption.scala
@@ -34,18 +34,14 @@ protected[core] case class ParameterStorageConfig(current: String = ParameterEnc
                                                   aes128: Option[String] = None,
                                                   aes256: Option[String] = None)
 
-protected[core] case class ParameterEncryption(default: Encrypter, encryptors: Map[String, Encrypter]) {
+protected[core] class ParameterEncryption(val default: Option[Encrypter], encryptors: Map[String, Encrypter]) {
 
   /**
-   * Gets the default encryptor or the encryptor for the given scheme name.
+   * Gets the coder for the given scheme name.
    *
-   * @param name optionnal name of the encryption algorithm (defaults to current from last configuration)
-   * @return the encryptor if there is one else no-op encryptor
+   * @param name the name of the encryption algorithm (defaults to current from last configuration)
+   * @return the coder if there is one else no-op encryptor
    */
-  def encryptor(name: Option[String] = None): Encrypter = {
-    encryptors.get(name.getOrElse(default.name)).getOrElse(ParameterEncryption.noop)
-  }
-
   def encryptor(name: String): Encrypter = {
     encryptors.get(name).getOrElse(ParameterEncryption.noop)
   }
@@ -73,9 +69,13 @@ protected[core] object ParameterEncryption {
       config.aes128.map(k => AES128_ENCRYPTION -> new Aes128(k)) ++
       config.aes256.map(k => AES256_ENCRYPTION -> new Aes256(k))
 
-    val defaultEncoder: Encrypter = availableEncoders.get(config.current).getOrElse(noop)
+    val current = config.current.toLowerCase match {
+      case "" | "off" | NO_ENCRYPTION => NO_ENCRYPTION
+      case s                          => s
+    }
 
-    ParameterEncryption(defaultEncoder, availableEncoders)
+    val defaultEncoder: Encrypter = availableEncoders.get(current).getOrElse(noop)
+    new ParameterEncryption(Option(defaultEncoder).filter(_ != noop), availableEncoders)
   }
 }
 
diff --git a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala
index e61b47d97fb..4370a1781e3 100644
--- a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala
+++ b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala
@@ -348,6 +348,7 @@ case class ExecutableWhiskActionMetaData(namespace: EntityPath,
 
 object WhiskAction extends DocumentFactory[WhiskAction] with WhiskEntityQueries[WhiskAction] with DefaultJsonProtocol {
   import WhiskActivation.instantSerdes
+
   val execFieldName = "exec"
   val requireWhiskAuthHeader = "x-require-whisk-auth"
 
diff --git a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskPackage.scala b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskPackage.scala
index 537c2fe440b..3acac54c080 100644
--- a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskPackage.scala
+++ b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskPackage.scala
@@ -198,9 +198,11 @@ object WhiskPackage
     }
     jsonFormat8(WhiskPackage.apply)
   }
+
   override val cacheEnabled = true
 
   lazy val publicPackagesView: View = WhiskQueries.entitiesView(collection = s"$collectionName-public")
+
   // overriden to store encrypted parameters.
   override def put[A >: WhiskPackage](db: ArtifactStore[A], doc: WhiskPackage, old: Option[WhiskPackage])(
     implicit transid: TransactionId,
diff --git a/tests/src/test/resources/application.conf.j2 b/tests/src/test/resources/application.conf.j2
index ea10e6371d5..e2eaa0f36de 100644
--- a/tests/src/test/resources/application.conf.j2
+++ b/tests/src/test/resources/application.conf.j2
@@ -95,7 +95,7 @@ whisk {
     }
 
     parameter-storage {
-        current = "noop"
+        current = "off"
     }
     
     elasticsearch {
diff --git a/tests/src/test/scala/org/apache/openwhisk/core/containerpool/test/ContainerProxyTests.scala b/tests/src/test/scala/org/apache/openwhisk/core/containerpool/test/ContainerProxyTests.scala
index d22fce8367c..dc8a33f0ee3 100644
--- a/tests/src/test/scala/org/apache/openwhisk/core/containerpool/test/ContainerProxyTests.scala
+++ b/tests/src/test/scala/org/apache/openwhisk/core/containerpool/test/ContainerProxyTests.scala
@@ -297,6 +297,18 @@ class ContainerProxyTests
     }
   }
 
+  it should "unlock arguments" in {
+    val k128 = "ra1V6AfOYAv0jCzEdufIFA=="
+    val coder = ParameterEncryption(ParameterStorageConfig("aes-128", aes128 = Some(k128)))
+    val locker = Some(coder.encryptor("aes-128"))
+
+    val param = Parameters("a", "abc").lock(locker).merge(Some(JsObject("b" -> JsString("xyz"))))
+    param.get.compactPrint should not include "abc"
+    ContainerProxy.unlockArguments(param, Map("a" -> "aes-128"), coder) shouldBe Some {
+      JsObject("a" -> JsString("abc"), "b" -> JsString("xyz"))
+    }
+  }
+
   /*
    * SUCCESSFUL CASES
    */
diff --git a/tests/src/test/scala/org/apache/openwhisk/core/entity/test/ParameterEncryptionTests.scala b/tests/src/test/scala/org/apache/openwhisk/core/entity/test/ParameterEncryptionTests.scala
index 090e5cfa60c..8fe2a5d51a8 100644
--- a/tests/src/test/scala/org/apache/openwhisk/core/entity/test/ParameterEncryptionTests.scala
+++ b/tests/src/test/scala/org/apache/openwhisk/core/entity/test/ParameterEncryptionTests.scala
@@ -45,6 +45,16 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
       new ParameterName("one") -> new ParameterValue("secret".toJson, false),
       new ParameterName("two") -> new ParameterValue("secret".toJson, true)))
 
+  behavior of "ParameterEncryption"
+
+  it should "not have a default coder when turned off" in {
+    ParameterEncryption(ParameterStorageConfig("")).default shouldBe empty
+    ParameterEncryption(ParameterStorageConfig("off")).default shouldBe empty
+    ParameterEncryption(ParameterStorageConfig("noop")).default shouldBe empty
+    ParameterEncryption(ParameterStorageConfig("OFF")).default shouldBe empty
+    ParameterEncryption(ParameterStorageConfig("NOOP")).default shouldBe empty
+  }
+
   behavior of "Parameters"
 
   it should "handle decryption of json objects" in {
@@ -59,6 +69,10 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
     val p = Parameters.serdes.read(originalValue.parseJson)
     p.get("paramName1").get.convertTo[String] shouldBe "from-action"
     p.get("paramName2").get.convertTo[String] shouldBe "from-pack"
+    p.params.foreach {
+      case (_, paramValue) =>
+        paramValue.encryption shouldBe empty
+    }
   }
 
   it should "handle decryption of json objects with null field" in {
@@ -73,6 +87,10 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
     val p = Parameters.serdes.read(originalValue.parseJson)
     p.get("paramName1").get.convertTo[String] shouldBe "from-action"
     p.get("paramName2").get.convertTo[String] shouldBe "from-pack"
+    p.params.foreach {
+      case (_, paramValue) =>
+        paramValue.encryption shouldBe empty
+    }
   }
 
   it should "drop encryption propery when no longer encrypted" in {
@@ -86,6 +104,10 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
 
     val p = Parameters.serdes.read(originalValue.parseJson)
     Parameters.serdes.write(p).compactPrint should not include "encryption"
+    p.params.foreach {
+      case (_, paramValue) =>
+        paramValue.encryption shouldBe empty
+    }
   }
 
   it should "read the merged message payload from kafka into parameters" in {
@@ -231,7 +253,7 @@ class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfte
   behavior of "No-op Encryption"
 
   it should "not mark parameters as encrypted" in {
-    val locked = parameters.lock(ParameterEncryption.noop)
+    val locked = parameters.lock()
     locked.params.foreach {
       case (_, paramValue) =>
         paramValue.value.convertTo[String] shouldBe "secret"

From 4c5ffa92ce86a1dd3c72c2dbb935c1f5057a3d4c Mon Sep 17 00:00:00 2001
From: Rodric Rabbah <rodric@gmail.com>
Date: Wed, 11 Mar 2020 22:22:13 -0400
Subject: [PATCH 12/12] Fix test.

---
 .../openwhisk/core/entity/Parameter.scala     | 47 ++++++++++---------
 1 file changed, 25 insertions(+), 22 deletions(-)

diff --git a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala
index 9470191b4a7..4a66867903e 100644
--- a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala
+++ b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala
@@ -294,28 +294,31 @@ protected[core] object Parameters extends ArgNormalizer[Parameters] {
      */
     def read(params: Vector[JsValue]) = Try {
       new Parameters(params.map {
-        _.asJsObject.getFields("key", "value", "init", "encryption") match {
-          case Seq(JsString(k), v: JsValue) =>
-            val key = new ParameterName(k)
-            val value = ParameterValue(v, false)
-            (key, value)
-          case Seq(JsString(k), v: JsValue, JsBoolean(i)) =>
-            val key = new ParameterName(k)
-            val value = ParameterValue(v, i)
-            (key, value)
-          case Seq(JsString(k), v: JsValue, JsBoolean(i), JsString(e)) =>
-            val key = new ParameterName(k)
-            val value = ParameterValue(v, i, Some(e))
-            (key, value)
-          case Seq(JsString(k), v: JsValue, JsBoolean(i), JsNull) =>
-            val key = new ParameterName(k)
-            val value = ParameterValue(v, i, None)
-            (key, value)
-          case Seq(JsString(k), v: JsValue, JsString(e)) =>
-            val key = new ParameterName(k)
-            val value = ParameterValue(v, false, Some(e))
-            (key, value)
-        }
+        case o @ JsObject(fields) =>
+          o.getFields("key", "value", "init", "encryption") match {
+            case Seq(JsString(k), v: JsValue) if fields.contains("value") =>
+              val key = new ParameterName(k)
+              val value = ParameterValue(v, false)
+              (key, value)
+            case Seq(JsString(k), v: JsValue, JsBoolean(i)) =>
+              val key = new ParameterName(k)
+              val value = ParameterValue(v, i)
+              (key, value)
+            case Seq(JsString(k), v: JsValue, JsBoolean(i), JsString(e)) =>
+              val key = new ParameterName(k)
+              val value = ParameterValue(v, i, Some(e))
+              (key, value)
+            case Seq(JsString(k), v: JsValue, JsBoolean(i), JsNull) =>
+              val key = new ParameterName(k)
+              val value = ParameterValue(v, i, None)
+              (key, value)
+            case Seq(JsString(k), v: JsValue, JsString(e))
+                if fields.contains("value") && fields.contains("encryption") =>
+              val key = new ParameterName(k)
+              val value = ParameterValue(v, false, Some(e))
+              (key, value)
+          }
+        case _ => deserializationError("invalid parameter")
       }.toMap)
     }
   }