From 10355da3b7a33e2af7d508793487db59d2ef1307 Mon Sep 17 00:00:00 2001 From: dan mcweeney Date: Fri, 24 Jan 2020 10:59:20 -0500 Subject: [PATCH] Tighten up types for encryption metadata. --- .../scala/src/main/resources/application.conf | 10 ++- .../openwhisk/core/entity/Parameter.scala | 39 ++++++---- .../core/entity/ParameterEncryption.scala | 78 +++++++++++-------- .../test/ParameterEncryptionTests.scala | 70 ++++++++++------- 4 files changed, 116 insertions(+), 81 deletions(-) diff --git a/common/scala/src/main/resources/application.conf b/common/scala/src/main/resources/application.conf index 724acd84f78..4106940bfce 100644 --- a/common/scala/src/main/resources/application.conf +++ b/common/scala/src/main/resources/application.conf @@ -481,13 +481,17 @@ whisk { # In general this setting should be left to its default disabled state retry-no-http-response-exception = false } - # Support key sizes of: 0, 16, 32. Which correspond to, no encryption, AES128 and AES256. For AES256 your JVM must be - # capable of that size key. # Enabling this will start to encrypt all default parameters for actions and packages. Be careful using this as # 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 { - key = "" + # Base64 encoded 256 bit key + #aes256 = "" + # Base64 encoded 128 bit key + #aes128 = "" + # 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 = "aes128|aes256" } } #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 b7b4c72055b..8071246c4c6 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 @@ -92,7 +92,7 @@ protected[core] class Parameters protected[entity] (private val params: Map[Para } val encrypt = p._2.encryption match { case (JsNull) => None - case _ => Some("encryption" -> p._2.encryption.toJson) + case _ => Some("encryption" -> p._2.encryption) } // 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)) @@ -105,7 +105,7 @@ protected[core] class Parameters protected[entity] (private val params: Map[Para if (p._2.encryption == JsNull) p._2.value.toJson else - JsObject("value" -> p._2.value.toJson, "encryption" -> p._2.encryption.toJson, "init" -> p._2.init.toJson) + JsObject("value" -> p._2.value.toJson, "encryption" -> p._2.encryption, "init" -> p._2.init.toJson) (p._1.name, newValue) })) @@ -148,7 +148,7 @@ protected[core] class Parameters protected[entity] (private val params: Map[Para /** * A ParameterName is a parameter name for an action or trigger to bind to its environment. - * It wraps a normalized string as a valueread type. + * It wraps a normalized string as a value type. * * It is a value type (hence == is .equals, immutable and cannot be assigned null). * The constructor is private so that argument requirements are checked and normalized @@ -175,10 +175,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. */ protected[entity] case class ParameterValue protected[entity] (private val v: JsValue, val init: Boolean, - val e: JsValue = JsNull) { + val encryptionDetails: Option[JsString] = None) { /** @return JsValue if defined else JsNull. */ protected[entity] def value = Option(v) getOrElse JsNull @@ -187,7 +188,7 @@ protected[entity] case class ParameterValue protected[entity] (private val v: Js protected[entity] def isDefined = value != JsNull /** @return JsValue if defined else JsNull. */ - protected[entity] def encryption = Option(e).getOrElse(JsNull) + protected[entity] def encryption = encryptionDetails getOrElse JsNull /** * The size of the ParameterValue entity as ByteSize. @@ -217,7 +218,7 @@ protected[core] object Parameters extends ArgNormalizer[Parameters] { protected[core] def apply(p: String, v: String, init: Boolean = false): Parameters = { require(p != null && p.trim.nonEmpty, "key undefined") Parameters() + (new ParameterName(ArgNormalizer.trim(p)), - ParameterValue(Option(v).map(_.trim.toJson).getOrElse(JsNull), init, JsNull)) + ParameterValue(Option(v).map(_.trim.toJson).getOrElse(JsNull), init, None)) } /** @@ -233,7 +234,7 @@ protected[core] object Parameters extends ArgNormalizer[Parameters] { protected[core] def apply(p: String, v: JsValue, init: Boolean): Parameters = { require(p != null && p.trim.nonEmpty, "key undefined") Parameters() + (new ParameterName(ArgNormalizer.trim(p)), - ParameterValue(Option(v).getOrElse(JsNull), init, JsNull)) + ParameterValue(Option(v).getOrElse(JsNull), init, None)) } /** @@ -248,7 +249,7 @@ protected[core] object Parameters extends ArgNormalizer[Parameters] { protected[core] def apply(p: String, v: JsValue): Parameters = { require(p != null && p.trim.nonEmpty, "key undefined") Parameters() + (new ParameterName(ArgNormalizer.trim(p)), - ParameterValue(Option(v).getOrElse(JsNull), false, JsNull)) + ParameterValue(Option(v).getOrElse(JsNull), false, None)) } def readMergedList(value: JsValue): Parameters = @@ -261,11 +262,11 @@ 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: JsValue) => - ParameterValue(v, i, e) - case _ => ParameterValue(o, false, JsNull) + case Seq(v: JsValue, JsBoolean(i), e: JsString) => + ParameterValue(v, i, Some(e)) + case _ => ParameterValue(o, false, None) } - case v: JsValue => ParameterValue(v, false, JsNull) + case v: JsValue => ParameterValue(v, false, None) } (key, paramVal) }) @@ -295,9 +296,13 @@ 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 => + val key = new ParameterName(i._1) + val value = ParameterValue(v, init, Some(JsString(e.toString()))) + converted = converted + (key -> value) case Seq(v: JsValue, JsBoolean(init), e: JsValue) => val key = new ParameterName(i._1) - val value = ParameterValue(v, init, e) + val value = ParameterValue(v, init, None) converted = converted + (key -> value) }) if (converted.size == 0) { @@ -325,17 +330,17 @@ 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: JsValue) => + case Seq(JsString(k), v: JsValue, JsBoolean(i), e: JsString) => val key = new ParameterName(k) - val value = ParameterValue(v, i, e) + val value = ParameterValue(v, i, Some(e)) (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, e: JsValue) if (i.asJsObject.fields.contains("encryption")) => + case Seq(JsString(k), v: JsValue, e: JsString) if (i.asJsObject.fields.contains("encryption")) => val key = new ParameterName(k) - val value = ParameterValue(v, false, e) + val value = ParameterValue(v, false, None) (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 c6e2e770ae3..d9b74959620 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,55 +29,66 @@ import spray.json.DefaultJsonProtocol._ import spray.json.{JsNull, JsString} import pureconfig.generic.auto._ -private trait encrypter { - def encrypt(p: ParameterValue): ParameterValue - def decrypt(p: ParameterValue): ParameterValue - val name: String -} - -case class ParameterStorageConfig(key: String = "") { - def getKeyBytes(): Array[Byte] = { - if (key.length == 0) { - Array[Byte]() - } else { - Base64.getDecoder().decode(key) - } - } - -} +case class ParameterStorageConfig(current: String = "", aes128: String = "", aes256: String = "") object ParameterEncryption { private val storageConfigLoader = loadConfig[ParameterStorageConfig](ConfigKeys.parameterStorage) var storageConfig = storageConfigLoader.getOrElse(ParameterStorageConfig.apply()) - private def enc = storageConfig.getKeyBytes.length match { - case 16 => new Aes128(storageConfig.getKeyBytes) - case 32 => new Aes256(storageConfig.getKeyBytes) - case 0 => new NoopCrypt - case _ => - throw new IllegalArgumentException( - s"Only 0, 16 and 32 characters support for key size but instead got ${storageConfig.getKeyBytes.length}") - } def lock(params: Parameters): Parameters = { + val configuredEncryptors = new encrypters(storageConfig) new Parameters( params.getMap .map(({ case (paramName, paramValue) if paramValue.encryption == JsNull => - paramName -> enc.encrypt(paramValue) + 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 != JsNull && paramValue.encryption.convertTo[String] == enc.name => - paramName -> enc.decrypt(paramValue) + if paramValue.encryption != JsNull && !configuredEncryptors + .getEncrypter(paramValue.encryption.convertTo[String]) + .isEmpty => + paramName -> configuredEncryptors + .getEncrypter(paramValue.encryption.convertTo[String]) + .get + .decrypt(paramValue) case (paramName, paramValue) => paramName -> paramValue }))) } } +private trait encrypter { + def encrypt(p: ParameterValue): ParameterValue + def decrypt(p: ParameterValue): ParameterValue + val name: String +} + +private class encrypters(val storageConfig: ParameterStorageConfig) { + private val availableEncrypters = Map("" -> new NoopCrypt()) ++ + (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 + } + protected[entity] def getEncrypter(name: String) = { + availableEncrypters.get(name) + } + + def getKeyBytes(key: String): Array[Byte] = { + if (key.length == 0) { + Array[Byte]() + } else { + Base64.getDecoder().decode(key) + } + } +} + private trait AesEncryption extends encrypter { val key: Array[Byte] val ivLen: Int @@ -88,7 +99,6 @@ private trait AesEncryption extends encrypter { private val secureRandom = new SecureRandom() def encrypt(value: ParameterValue): ParameterValue = { - val iv = new Array[Byte](ivLen) secureRandom.nextBytes(iv) val gcmSpec = new GCMParameterSpec(tLen, iv) @@ -102,7 +112,7 @@ private trait AesEncryption extends encrypter { byteBuffer.put(iv) byteBuffer.put(cipherText) val cipherMessage = byteBuffer.array - ParameterValue(JsString(Base64.getEncoder.encodeToString(cipherMessage)), value.init, JsString(name)) + ParameterValue(JsString(Base64.getEncoder.encodeToString(cipherMessage)), value.init, Some(JsString(name))) } def decrypt(value: ParameterValue): ParameterValue = { @@ -127,11 +137,17 @@ private trait AesEncryption extends encrypter { } -private case class Aes128(val key: Array[Byte], val ivLen: Int = 12, val name: String = "aes128") +private object Aes128 { + val name: String = "aes128" +} +private case class Aes128(val key: Array[Byte], val ivLen: Int = 12, val name: String = Aes128.name) extends AesEncryption with encrypter -private case class Aes256(val key: Array[Byte], val ivLen: Int = 128, val name: String = "aes256") +private object Aes256 { + val name: String = "aes256" +} +private case class Aes256(val key: Array[Byte], val ivLen: Int = 128, val name: String = Aes256.name) extends AesEncryption with encrypter 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 2cfceb05d91..3b2312ea773 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 @@ -16,6 +16,8 @@ */ package org.apache.openwhisk.core.entity.test +import java.security.InvalidAlgorithmParameterException + import org.apache.openwhisk.core.entity._ import org.junit.runner.RunWith import org.scalatest.junit.JUnitRunner @@ -24,7 +26,7 @@ import spray.json.DefaultJsonProtocol._ import spray.json._ @RunWith(classOf[JUnitRunner]) -class ParameterEncryptionTests extends FlatSpec with ExecHelpers with Matchers with BeforeAndAfter { +class ParameterEncryptionTests extends FlatSpec with Matchers with BeforeAndAfter { after { ParameterEncryption.storageConfig = new ParameterStorageConfig("") @@ -33,7 +35,7 @@ class ParameterEncryptionTests extends FlatSpec with ExecHelpers with Matchers w val parameters = new Parameters( Map( new ParameterName("one") -> new ParameterValue(JsString("secret"), false), - new ParameterName("two") -> new ParameterValue(JsString("secret"), false))) + new ParameterName("two") -> new ParameterValue(JsString("secret"), true))) behavior of "Parameters" it should "handle complex objects in param body" in { @@ -97,7 +99,7 @@ class ParameterEncryptionTests extends FlatSpec with ExecHelpers with Matchers w } it should "read the merged message payload from kafka into parameters" in { - ParameterEncryption.storageConfig = new ParameterStorageConfig("ra1V6AfOYAv0jCzEdufIFA==") + ParameterEncryption.storageConfig = new ParameterStorageConfig("aes128", "ra1V6AfOYAv0jCzEdufIFA==") val locked = ParameterEncryption.lock(parameters) val unlockedParam = new ParameterValue(JsString("test-plain"), false) @@ -112,7 +114,7 @@ class ParameterEncryptionTests extends FlatSpec with ExecHelpers with Matchers w behavior of "AesParameterEncryption" it should "correctly mark the encrypted parameters after lock" in { - ParameterEncryption.storageConfig = new ParameterStorageConfig("ra1V6AfOYAv0jCzEdufIFA==") + ParameterEncryption.storageConfig = new ParameterStorageConfig("aes128", "ra1V6AfOYAv0jCzEdufIFA==") val locked = ParameterEncryption.lock(parameters) locked.getMap.map(({ case (_, paramValue) => @@ -121,8 +123,17 @@ class ParameterEncryptionTests extends FlatSpec with ExecHelpers with Matchers w })) } + it should "serialize to json correctly" in { + val output = + """\Q{"one":{"encryption":"aes128","init":false,"value":"\E.*\Q"},"two":{"encryption":"aes128","init":true,"value":"\E.*\Q"}}""".stripMargin.r + ParameterEncryption.storageConfig = new ParameterStorageConfig("aes128", "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("ra1V6AfOYAv0jCzEdufIFA==") + ParameterEncryption.storageConfig = new ParameterStorageConfig("aes128", "ra1V6AfOYAv0jCzEdufIFA==") val locked = ParameterEncryption.lock(parameters) locked.getMap.map(({ case (_, paramValue) => @@ -139,36 +150,35 @@ class ParameterEncryptionTests extends FlatSpec with ExecHelpers with Matchers w } // 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("j5rLzhtxwzPyUVUy8/p8XJmBoKeDoSzNJP1SITJEY9E=") -// try { -// val locked = ParameterEncryption.lock(parameters) -// locked.getMap.map(({ -// case (_, paramValue) => -// paramValue.encryption.convertTo[String] shouldBe "aes256" -// paramValue.value.convertTo[String] should not be "secret" -// })) -// -// val unlocked = ParameterEncryption.unlock(locked) -// unlocked.getMap.map(({ -// case (_, paramValue) => -// paramValue.encryption shouldBe JsNull -// paramValue.value.convertTo[String] shouldBe "secret" -// })) -// } catch { -// case e: InvalidAlgorithmParameterException => -// cancel(e) -// } -// } + it should "work if with aes256 if policy allows it" in { + ParameterEncryption.storageConfig = + new ParameterStorageConfig("aes256", "", "j5rLzhtxwzPyUVUy8/p8XJmBoKeDoSzNJP1SITJEY9E=") + try { + val locked = ParameterEncryption.lock(parameters) + locked.getMap.map(({ + case (_, paramValue) => + paramValue.encryption.convertTo[String] shouldBe "aes256" + paramValue.value.convertTo[String] should not be "secret" + })) + + val unlocked = ParameterEncryption.unlock(locked) + unlocked.getMap.map(({ + case (_, paramValue) => + paramValue.encryption shouldBe JsNull + paramValue.value.convertTo[String] shouldBe "secret" + })) + } catch { + case e: InvalidAlgorithmParameterException => + cancel(e) + } + } behavior of "NoopEncryption" it should "not mark parameters as encrypted" in { - val unlock = ParameterEncryption.unlock(parameters) - unlock.getMap.map(({ + val locked = ParameterEncryption.lock(parameters) + locked.getMap.map(({ case (_, paramValue) => - paramValue.encryption shouldBe JsNull paramValue.value.convertTo[String] shouldBe "secret" })) } - }