This is an automated email from the ASF dual-hosted git repository.
tysonnorris pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-openwhisk.git
The following commit(s) were added to refs/heads/master by this push:
new cba3b7b Protect Package Bindings from containing circular references
(#4122)
cba3b7b is described below
commit cba3b7b3d815abe67efdcb7fb4ed63fa9cf788ac
Author: Andy Steed <andrewst...@gmail.com>
AuthorDate: Wed Nov 28 12:06:16 2018 -0800
Protect Package Bindings from containing circular references (#4122)
Use incoming package binding for update when calling checkBinding and add
protection to prevent following circular package bindings.
---
.../org/apache/openwhisk/http/ErrorResponse.scala | 1 +
.../openwhisk/core/controller/Packages.scala | 15 ++++++++++-----
.../core/entitlement/PackageCollection.scala | 10 +++++++++-
.../core/controller/test/PackagesApiTests.scala | 22 ++++++++++++++++++++++
4 files changed, 42 insertions(+), 6 deletions(-)
diff --git
a/common/scala/src/main/scala/org/apache/openwhisk/http/ErrorResponse.scala
b/common/scala/src/main/scala/org/apache/openwhisk/http/ErrorResponse.scala
index b14d338..a877630 100644
--- a/common/scala/src/main/scala/org/apache/openwhisk/http/ErrorResponse.scala
+++ b/common/scala/src/main/scala/org/apache/openwhisk/http/ErrorResponse.scala
@@ -112,6 +112,7 @@ object Messages {
val requestedBindingIsNotValid = "Cannot bind to a resource that is not a
package."
val notAllowedOnBinding = "Operation not permitted on package binding."
def packageNameIsReserved(name: String) = s"Package name '$name' is
reserved."
+ def packageBindingCircularReference(name: String) = s"Package binding
'$name' contains a circular reference."
/** Error messages for triggers */
def triggerWithInactiveRule(rule: String, action: String) = {
diff --git
a/core/controller/src/main/scala/org/apache/openwhisk/core/controller/Packages.scala
b/core/controller/src/main/scala/org/apache/openwhisk/core/controller/Packages.scala
index 6a07e5d..b1e555b 100644
---
a/core/controller/src/main/scala/org/apache/openwhisk/core/controller/Packages.scala
+++
b/core/controller/src/main/scala/org/apache/openwhisk/core/controller/Packages.scala
@@ -242,8 +242,8 @@ trait WhiskPackagesApi extends WhiskCollectionAPI with
ReferencedEntities {
val validateBinding = content.binding map { binding =>
wp.binding map {
// pre-existing entity is a binding, check that new binding is valid
- b =>
- checkBinding(b.fullyQualifiedName)
+ _ =>
+ checkBinding(binding.fullyQualifiedName)
} getOrElse {
// pre-existing entity is a package, cannot make it a binding
Future.failed(RejectRequest(Conflict,
Messages.packageCannotBecomeBinding))
@@ -299,9 +299,14 @@ trait WhiskPackagesApi extends WhiskCollectionAPI with
ReferencedEntities {
case b: Binding =>
val docid = b.fullyQualifiedName.toDocId
logging.debug(this, s"fetching package '$docid' for reference")
- getEntity(WhiskPackage.get(entityStore, docid), Some {
- mergePackageWithBinding(Some { wp }) _
- })
+ if (docid == wp.docid) {
+ logging.error(this, s"unexpected package binding refers to itself:
$docid")
+ terminate(UnprocessableEntity,
Messages.packageBindingCircularReference(b.fullyQualifiedName.toString))
+ } else {
+ getEntity(WhiskPackage.get(entityStore, docid), Some {
+ mergePackageWithBinding(Some { wp }) _
+ })
+ }
} getOrElse {
val pkg = ref map { _ inherit wp.parameters } getOrElse wp
logging.debug(this, s"fetching package actions in '${wp.fullPath}'")
diff --git
a/core/controller/src/main/scala/org/apache/openwhisk/core/entitlement/PackageCollection.scala
b/core/controller/src/main/scala/org/apache/openwhisk/core/entitlement/PackageCollection.scala
index 7dfb0c8..b53dcbc 100644
---
a/core/controller/src/main/scala/org/apache/openwhisk/core/entitlement/PackageCollection.scala
+++
b/core/controller/src/main/scala/org/apache/openwhisk/core/entitlement/PackageCollection.scala
@@ -98,7 +98,15 @@ class PackageCollection(entityStore: EntityStore)(implicit
logging: Logging) ext
val pkgOwner = namespaces.contains(binding.namespace.asString)
val pkgDocid = binding.docid
logging.debug(this, s"checking subject has privilege '$right' for
bound package '$pkgDocid'")
- checkPackageReadPermission(namespaces, pkgOwner, pkgDocid)
+ if (doc == pkgDocid) {
+ logging.error(this, s"unexpected package binding refers to itself:
$doc")
+ Future.failed(
+ RejectRequest(
+ UnprocessableEntity,
+
Messages.packageBindingCircularReference(binding.fullyQualifiedName.toString)))
+ } else {
+ checkPackageReadPermission(namespaces, pkgOwner, pkgDocid)
+ }
} else {
logging.debug(this, s"entitlement check on package binding, '$right'
allowed?: false")
Future.successful(false)
diff --git
a/tests/src/test/scala/org/apache/openwhisk/core/controller/test/PackagesApiTests.scala
b/tests/src/test/scala/org/apache/openwhisk/core/controller/test/PackagesApiTests.scala
index dd14397..8117993 100644
---
a/tests/src/test/scala/org/apache/openwhisk/core/controller/test/PackagesApiTests.scala
+++
b/tests/src/test/scala/org/apache/openwhisk/core/controller/test/PackagesApiTests.scala
@@ -712,6 +712,28 @@ class PackagesApiTests extends ControllerTestCommon with
WhiskPackagesApi {
}
}
+ it should "reject update package reference when new binding refers to
itself" in {
+ implicit val tid = transid()
+ // create package and valid reference binding to it
+ val provider = WhiskPackage(namespace, aname())
+ val reference = WhiskPackage(namespace, aname(), provider.bind)
+ put(entityStore, provider)
+ put(entityStore, reference)
+ // manipulate package reference such that it attempts to bind to itself
+ val content = WhiskPackagePut(Some(Binding(namespace.root,
reference.name)))
+ Put(s"$collectionPath/${reference.name}?overwrite=true", content) ~>
Route.seal(routes(creds)) ~> check {
+ status should be(BadRequest)
+ responseAs[ErrorResponse].error should
include(Messages.bindingCannotReferenceBinding)
+ }
+ // verify that the reference is still pointing to the original provider
+ Get(s"$collectionPath/${reference.name}") ~> Route.seal(routes(creds)) ~>
check {
+ status should be(OK)
+ val response = responseAs[WhiskPackage]
+ response should be(reference)
+ response.binding should be(provider.bind)
+ }
+ }
+
it should "reject update package reference when new binding refers to
private package in another namespace" in {
implicit val tid = transid()
val privateCreds = WhiskAuthHelpers.newIdentity()
