Skip to content

Commit

Permalink
Tighten up handling of a read of the params.
Browse files Browse the repository at this point in the history 
Broke reading the kafka protocol into a new method to keep the
strict parsing of the scheme intact.
Use only base64 encoded keys.
  • Loading branch information
@mcdan
mcdan committed Jan 9, 2020
1 parent da7bd79 commit 3f324e2
Show file tree
Hide file tree
Showing 5 changed files with 195 additions and 106 deletions.
107 changes: 65 additions & 42 deletions common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ 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}

Expand Down Expand Up @@ -93,7 +94,8 @@ protected[core] class Parameters protected[entity] (private val params: Map[Para
case (JsNull) => None
case _ => Some("encryption" -> p._2.encryption.toJson)
}
JsObject(Map("key" -> p._1.name.toJson, "value" -> p._2.value.toJson) ++ init ++ encrypt)
// 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))
} toSeq: _*)
}

Expand Down Expand Up @@ -146,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 value type.
* It wraps a normalized string as a valueread 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
Expand Down Expand Up @@ -215,7 +217,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))
ParameterValue(Option(v).map(_.trim.toJson).getOrElse(JsNull), init, JsNull))
}

/**
Expand All @@ -231,7 +233,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))
ParameterValue(Option(v).getOrElse(JsNull), init, JsNull))
}

/**
Expand All @@ -246,9 +248,30 @@ 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))
ParameterValue(Option(v).getOrElse(JsNull), false, JsNull))
}

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), e: JsValue) =>
ParameterValue(v, i, e)
case _ => ParameterValue(o, false, JsNull)
}
case v: JsValue => ParameterValue(v, false, JsNull)
}
(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

Expand All @@ -266,25 +289,22 @@ protected[core] object Parameters extends ArgNormalizer[Parameters] {
} flatMap {
read(_)
} getOrElse {
// Used when the container proxy is reading back a merged version of the params.
Try {
var converted = Map[ParameterName, ParameterValue]()
new Parameters(value.asJsObject.fields.map { item: (String, JsValue) =>
{
item._2 match {
case JsString(s) => (new ParameterName(item._1), new ParameterValue(JsString(s), false))
case _ => {
item._2.asJsObject.getFields("value", "init", "encryption") match {
case Seq(v: JsValue, JsBoolean(i), e: JsValue) =>
(new ParameterName(item._1), new ParameterValue(v, i, e))
}
}
}
}
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), e: JsValue) =>
val key = new ParameterName(i._1)
val value = ParameterValue(v, init, e)
converted = converted + (key -> value)
})
} getOrElse {
deserializationError("parameters malformed!")
}
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 ""))
}

/**
Expand All @@ -295,26 +315,29 @@ 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 {
_.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), e: JsValue) =>
val key = new ParameterName(k)
val value = ParameterValue(v, i, e)
(key, value)
case Seq(JsString(k), v: JsValue, e: JsValue) =>
val key = new ParameterName(k)
val value = ParameterValue(v, false, e)
(key, value)
case Seq(JsString(k), v: JsValue, JsBoolean(i)) =>
val key = new ParameterName(k)
val value = ParameterValue(v, i)
(key, value)
}
} toMap)
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), e: JsValue) =>
val key = new ParameterName(k)
val value = ParameterValue(v, i, 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")) =>
val key = new ParameterName(k)
val value = ParameterValue(v, false, e)
(key, value)
}
})
.toMap)
}
}
}
31 changes: 20 additions & 11 deletions common/scala/src/main/scala/org/apache/openwhisk/core/entity/ParameterEncryption.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,28 @@ private trait encrypter {
val name: String
}

case class ParameterStorageConfig(key: String = "")
case class ParameterStorageConfig(key: String = "") {
def getKeyBytes(): Array[Byte] = {
if (key.length == 0) {
Array[Byte]()
} else {
Base64.getDecoder().decode(key)
}
}

object ParameterEncryption {
}

object ParameterEncryption {
private val storageConfigLoader = loadConfig[ParameterStorageConfig](ConfigKeys.parameterStorage)
var storageConfig = storageConfigLoader.getOrElse(ParameterStorageConfig.apply())
private val enc: encrypter = storageConfig.key.length match {
case 16 => new Aes128(storageConfig.key)
case 32 => new Aes256(storageConfig.key)
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("Only 0, 16 and 32 characters support for key size.")
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 = {
new Parameters(
params.getMap
Expand All @@ -69,11 +78,11 @@ object ParameterEncryption {
}

private trait AesEncryption extends encrypter {
val key: String
val key: Array[Byte]
val ivLen: Int
val name: String
private val tLen = key.length * 8
private val secretKey = new SecretKeySpec(key.getBytes(StandardCharsets.UTF_8), "AES")
private val secretKey = new SecretKeySpec(key, "AES")

private val secureRandom = new SecureRandom()

Expand Down Expand Up @@ -117,11 +126,11 @@ private trait AesEncryption extends encrypter {

}

private case class Aes128(val key: String, val ivLen: Int = 12, val name: String = "aes128")
private case class Aes128(val key: Array[Byte], val ivLen: Int = 12, val name: String = "aes128")
extends AesEncryption
with encrypter

private case class Aes256(val key: String, val ivLen: Int = 128, val name: String = "aes256")
private case class Aes256(val key: Array[Byte], val ivLen: Int = 128, val name: String = "aes256")
extends AesEncryption
with encrypter

Expand Down
2 changes: 1 addition & 1 deletion core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerProxy.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,7 @@ class ContainerProxy(factory: (TransactionId,
val actionTimeout = job.action.limits.timeout.duration
val unlockedContent = job.msg.content match {
case Some(js) => {
Some(ParameterEncryption.unlock(Parameters.serdes.read(js)).toJsObject)
Some(ParameterEncryption.unlock(Parameters.readMergedList(js)).toJsObject)
}
case _ => job.msg.content
}
Expand Down
65 changes: 32 additions & 33 deletions tests/src/test/scala/org/apache/openwhisk/core/controller/test/ActionsApiTests.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,29 +19,28 @@ package org.apache.openwhisk.core.controller.test

import java.time.Instant

import scala.concurrent.duration.DurationInt
import scala.language.postfixOps
import org.junit.runner.RunWith
import org.scalatest.junit.JUnitRunner
import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport.{sprayJsonMarshaller, sprayJsonUnmarshaller}
import akka.http.scaladsl.model.StatusCodes._
import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport.sprayJsonMarshaller
import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport.sprayJsonUnmarshaller
import akka.http.scaladsl.server.Route
import spray.json._
import spray.json.DefaultJsonProtocol._
import org.apache.openwhisk.core.controller.WhiskActionsApi
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 akka.http.scaladsl.server.Route
import org.apache.commons.lang3.StringUtils
import org.apache.openwhisk.core.connector.ActivationMessage
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.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.
Expand Down Expand Up @@ -224,22 +223,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()
Expand Down Expand Up @@ -1703,9 +1702,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)
Expand Down
Loading

0 comments on commit 3f324e2

Please sign in to comment.