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()