Skip to content

Commit

Permalink
Add protect feature to avoid update or delete actions by mistake
Browse files Browse the repository at this point in the history 
- As more and more production system uses openwhisk,
Users will need some features to protect their service safe from mistake.
If we have this feature, user can protect their actions from updating or
deletion by mistake.
- Apply unix style permission management
- Make the action's downloadable for the shared user
  • Loading branch information
@ningyougang
ningyougang committed Sep 7, 2020
1 parent 66a9417 commit db02599
Show file tree
Hide file tree
Showing 6 changed files with 473 additions and 49 deletions.
53 changes: 53 additions & 0 deletions common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,16 @@ case class WhiskActionPut(exec: Option[Exec] = None,
case _ => this
} getOrElse this
}

protected[core] def getPermissions(): Option[String] = {
annotations match {
case Some(value) =>
value
.get(WhiskAction.permissionsFieldName)
.map(value => value.convertTo[String])
case None => None
}
}
}

abstract class WhiskActionLike(override val name: EntityName) extends WhiskEntity(name, "action") {
Expand Down Expand Up @@ -351,6 +361,49 @@ object WhiskAction extends DocumentFactory[WhiskAction] with WhiskEntityQueries[
val execFieldName = "exec"
val requireWhiskAuthHeader = "x-require-whisk-auth"

// annotation permission key name
val permissionsFieldName = "permissions"

val defaultPermissions = "rwxr-x"

// notes on users, just have 2 type users,
// 1. the action's owner
// 2. the user (not the owner) who used the shared action directly(e.g. get, invoke), we call it "the shared user"
//
// Notes on permission control
// 1. the owner has read(or download) permission on any situation, but for the shared user,
// in spite of has read permission on any situation, but can set it undownloadable or downloadable
// 2. the shared user can't update/delete the action on any situation.
// 3. the owner's permission can affect the shared user's permission, e.g
// if the owner is not given execute permission, the shared user can't have execute permission as well.
//
// Notes on permission values, include below permission value
// 1. permission code:rwxr-x: owner:read(yes)/write(yes)/execute(yes)|the shared action's user:download(yes)/write(no)/execute(yes), this is default
// 2. permission code:rwxr--: owner:read(yes)/write(yes)/execute(yes)|the shared action's user:download(yes)/write(no)/execute(no)
// 3. permission code:r-xr-x: owner:read(yes)/write(no)/execute(yes)|the shared action's user:download(yes)/write(no)/execute(yes)
// 4. permission code:r-xr--: owner:read(yes)/write(no)/execute(yes)|the shared action's user:download(yes)/write(no)/execute(no)
// 5. permission code:r--r--: owner:read(yes)/write(no)/execute(no)|the shared action's user:download(yes)/write(no)/execute(no)
// 6. permission code:rw-r--: owner:read(yes)/write(yes)/execute(no)|the shared action's user:download(yes)/write(no)/execute(no)
// 7. permission code:rwx--x: owner:read(yes)/write(yes)/execute(yes)|the shared action's user:download(no)/write(no)/execute(yes)
// 8. permission code:rwx---: owner:read(yes)/write(yes)/execute(yes)|the shared action's user:download(no)/write(no)/execute(no)
// 9. permission code:r-x--x: owner:read(yes)/write(no)/execute(yes)|the shared action's user:download(no)/write(no)/execute(yes)
// 10. permission code:r-x---: owner:read(yes)/write(no)/execute(yes)|the shared action's user:download(no)/write(no)/execute(no)
// 11. permission code:r-----: owner:read(yes)/write(no)/execute(no)|the shared action's user:download(no)/write(no)/execute(no)
// 12. permission code:rw----: owner:read(yes)/write(yes)/execute(no)|the shared action's user:download(no)/write(no)/execute(no)
val permissionList = List(
defaultPermissions,
"rwxr--",
"r-xr-x",
"r-xr--",
"r--r--",
"rw-r--",
"rwx--x",
"rwx---",
"r-x--x",
"r-x---",
"r-----",
"rw----")

override val collectionName = "actions"
override val cacheEnabled = true

Expand Down
9 changes: 1 addition & 8 deletions common/scala/src/main/scala/org/apache/openwhisk/http/ErrorResponse.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,23 +20,16 @@ package org.apache.openwhisk.http
import scala.concurrent.duration.Duration
import scala.concurrent.duration.FiniteDuration
import scala.util.Try

import akka.http.scaladsl.model.StatusCode
import akka.http.scaladsl.model.StatusCodes.Forbidden
import akka.http.scaladsl.model.StatusCodes.NotFound
import akka.http.scaladsl.model.MediaType
import akka.http.scaladsl.server.Directives
import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport.sprayJsonMarshaller
import akka.http.scaladsl.server.StandardRoute

import spray.json._

import org.apache.openwhisk.common.TransactionId
import org.apache.openwhisk.core.entity.SizeError
import org.apache.openwhisk.core.entity.ByteSize
import org.apache.openwhisk.core.entity.Exec
import org.apache.openwhisk.core.entity.ExecMetaDataBase
import org.apache.openwhisk.core.entity.ActivationId
import org.apache.openwhisk.core.entity.{ActivationId, ByteSize, Exec, ExecMetaDataBase, SizeError}

object Messages {

Expand Down
113 changes: 72 additions & 41 deletions core/controller/src/main/scala/org/apache/openwhisk/core/controller/Actions.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,9 +218,22 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with

onComplete(checkAdditionalPrivileges) {
case Success(_) =>
putEntity(WhiskAction, entityStore, entityName.toDocId, overwrite, update(user, request) _, () => {
make(user, entityName, request)
})
val operation = if (overwrite) "update" else "create"
onComplete(
entitlementProvider
.checkActionPermissions(
operation,
user,
entityStore,
entityName,
WhiskAction.get,
content.getPermissions())) {
case Success(_) =>
putEntity(WhiskAction, entityStore, entityName.toDocId, overwrite, update(user, request) _, () => {
make(user, entityName, request)
})
case Failure(f) => super.handleEntitlementFailure(f)
}
case Failure(f) =>
super.handleEntitlementFailure(f)
}
Expand All @@ -241,37 +254,43 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
*/
override def activate(user: Identity, entityName: FullyQualifiedEntityName, env: Option[Parameters])(
implicit transid: TransactionId) = {
parameter(
'blocking ? false,
'result ? false,
'timeout.as[FiniteDuration] ? controllerActivationConfig.maxWaitForBlockingActivation) {
(blocking, result, waitOverride) =>
entity(as[Option[JsObject]]) { payload =>
getEntity(WhiskActionMetaData.resolveActionAndMergeParameters(entityStore, entityName), Some {
act: WhiskActionMetaData =>
// resolve the action --- special case for sequences that may contain components with '_' as default package
val action = act.resolve(user.namespace)
onComplete(entitleReferencedEntitiesMetaData(user, Privilege.ACTIVATE, Some(action.exec))) {
case Success(_) =>
val actionWithMergedParams = env.map(action.inherit(_)) getOrElse action

// incoming parameters may not override final parameters (i.e., parameters with already defined values)
// on an action once its parameters are resolved across package and binding
val allowInvoke = payload
.map(_.fields.keySet.forall(key => !actionWithMergedParams.immutableParameters.contains(key)))
.getOrElse(true)
onComplete(
entitlementProvider
.checkActionPermissions("invoke", user, entityStore, entityName, WhiskAction.get)) {
case Success(_) =>
parameter(
'blocking ? false,
'result ? false,
'timeout.as[FiniteDuration] ? controllerActivationConfig.maxWaitForBlockingActivation) {
(blocking, result, waitOverride) =>
entity(as[Option[JsObject]]) { payload =>
getEntity(WhiskActionMetaData.resolveActionAndMergeParameters(entityStore, entityName), Some {
act: WhiskActionMetaData =>
// resolve the action --- special case for sequences that may contain components with '_' as default package
val action = act.resolve(user.namespace)
onComplete(entitleReferencedEntitiesMetaData(user, Privilege.ACTIVATE, Some(action.exec))) {
case Success(_) =>
val actionWithMergedParams = env.map(action.inherit(_)) getOrElse action

// incoming parameters may not override final parameters (i.e., parameters with already defined values)
// on an action once its parameters are resolved across package and binding
val allowInvoke = payload
.map(_.fields.keySet.forall(key => !actionWithMergedParams.immutableParameters.contains(key)))
.getOrElse(true)

if (allowInvoke) {
doInvoke(user, actionWithMergedParams, payload, blocking, waitOverride, result)
} else {
terminate(BadRequest, Messages.parametersNotAllowed)
}

if (allowInvoke) {
doInvoke(user, actionWithMergedParams, payload, blocking, waitOverride, result)
} else {
terminate(BadRequest, Messages.parametersNotAllowed)
case Failure(f) =>
super.handleEntitlementFailure(f)
}

case Failure(f) =>
super.handleEntitlementFailure(f)
}
})
})
}
}
case Failure(f) => super.handleEntitlementFailure(f)
}
}

Expand Down Expand Up @@ -333,11 +352,17 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
* - 500 Internal Server Error
*/
override def remove(user: Identity, entityName: FullyQualifiedEntityName)(implicit transid: TransactionId) = {
deleteEntity(WhiskAction, entityStore, entityName.toDocId, (a: WhiskAction) => Future.successful({}))
onComplete(
entitlementProvider
.checkActionPermissions("remove", user, entityStore, entityName, WhiskAction.get)) {
case Success(_) =>
deleteEntity(WhiskAction, entityStore, entityName.toDocId, (a: WhiskAction) => Future.successful({}))
case Failure(f) => super.handleEntitlementFailure(f)
}
}

/** Checks for package binding case. we don't want to allow get for a package binding in shared package */
private def fetchEntity(entityName: FullyQualifiedEntityName, env: Option[Parameters], code: Boolean)(
private def fetchEntity(user: Identity, entityName: FullyQualifiedEntityName, env: Option[Parameters], code: Boolean)(
implicit transid: TransactionId) = {
val resolvedPkg: Future[Either[String, FullyQualifiedEntityName]] = if (entityName.path.defaultPackage) {
Future.successful(Right(entityName))
Expand All @@ -357,13 +382,19 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
case Left(f) => terminate(Forbidden, f)
case Right(_) =>
if (code) {
getEntity(WhiskAction.resolveActionAndMergeParameters(entityStore, entityName), Some {
action: WhiskAction =>
val mergedAction = env map {
action inherit _
} getOrElse action
complete(OK, mergedAction)
})
onComplete(
entitlementProvider
.checkActionPermissions("download", user, entityStore, entityName, WhiskAction.get)) {
case Success(_) =>
getEntity(WhiskAction.resolveActionAndMergeParameters(entityStore, entityName), Some {
action: WhiskAction =>
val mergedAction = env map {
action inherit _
} getOrElse action
complete(OK, mergedAction)
})
case Failure(f) => super.handleEntitlementFailure(f)
}
} else {
getEntity(WhiskActionMetaData.resolveActionAndMergeParameters(entityStore, entityName), Some {
action: WhiskActionMetaData =>
Expand Down Expand Up @@ -396,7 +427,7 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
if (executeOnly && user.namespace.name != entityName.namespace) {
terminate(Forbidden, forbiddenGetAction(entityName.path.asString))
} else {
fetchEntity(entityName, env, code)
fetchEntity(user, entityName, env, code)
}
}
}
Expand Down
126 changes: 126 additions & 0 deletions core/controller/src/main/scala/org/apache/openwhisk/core/entitlement/Entitlement.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,12 @@ import org.apache.openwhisk.core.loadBalancer.{LoadBalancer, ShardingContainerPo
import org.apache.openwhisk.http.ErrorResponse
import org.apache.openwhisk.http.Messages
import org.apache.openwhisk.core.connector.MessagingProvider
import org.apache.openwhisk.core.database.ArtifactStore
import org.apache.openwhisk.spi.SpiLoader
import org.apache.openwhisk.spi.Spi

import spray.json.DefaultJsonProtocol._

object types {
type Entitlements = TrieMap[(Subject, String), Set[Privilege]]
}
Expand Down Expand Up @@ -231,6 +234,129 @@ protected[core] abstract class EntitlementProvider(
.getOrElse(Future.successful(()))
}

/**
* Checks if action operation(get/write/execute) whether feasible
*
* @param operation the action operation, e.g. get/write/execute
* @param user the user who get/write/execute the action
* @param entityStore store to write the action to
* @param entityName entityName
* @param permissions the passed permission code
* @return a promise that completes with success iff action operation is feasible
*/
protected[core] def checkActionPermissions(
operation: String,
user: Identity,
entityStore: ArtifactStore[WhiskEntity],
entityName: FullyQualifiedEntityName,
get: (ArtifactStore[WhiskEntity], DocId, DocRevision, Boolean) => Future[WhiskAction],
permissions: Option[String] = None)(implicit transid: TransactionId): Future[Unit] = {
if (operation == "create") {
permissions
.map { value =>
if (WhiskAction.permissionList.contains(value)) {
Future.successful(())
} else {
val errorInfo =
s"give error permission code: ${value}, available permission is in ${WhiskAction.permissionList}"
Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid))))
}
}
.getOrElse(Future.successful(()))
} else if (operation == "update") {
get(entityStore, entityName.toDocId, DocRevision.empty, true).flatMap { whiskAction =>
val currentPermissions = whiskAction.annotations
.get(WhiskAction.permissionsFieldName)
.map(value => value.convertTo[String])
.getOrElse(WhiskAction.defaultPermissions)

val errorInfo = s"have no permission to ${operation} this action"
permissions match {
case Some(value) =>
if (!WhiskAction.permissionList.contains(value)) {
val errorInfo =
s"give error permission code: ${value}, available permission is in ${WhiskAction.permissionList}"
Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid))))
} else {
val passedUpdatePermission = value.charAt(1)
if (passedUpdatePermission == 'w') { // make it to modifiable
Future.successful(())
} else {
val currentUpdatePermission = currentPermissions.charAt(1)
if (currentUpdatePermission == '-') {
Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid))))
} else {
Future.successful(())
}
}
}
case None =>
val currentUpdatePermission = currentPermissions.charAt(1)
if (currentUpdatePermission == '-') {
Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid))))
} else {
Future.successful(())
}
}
}
} else if (operation == "remove") {
get(entityStore, entityName.toDocId, DocRevision.empty, true).flatMap { whiskAction =>
val currentPermissions = whiskAction.annotations
.get(WhiskAction.permissionsFieldName)
.map(value => value.convertTo[String])
.getOrElse(WhiskAction.defaultPermissions)

val currentUpdatePermission = currentPermissions.charAt(1)
if (currentUpdatePermission == '-') {
val errorInfo = s"have no permission to ${operation} this action"
Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid))))
} else {
Future.successful(())
}
}
} else if (operation == "invoke") {
get(entityStore, entityName.toDocId, DocRevision.empty, true).flatMap { whiskAction =>
val currentPermissions = whiskAction.annotations
.get(WhiskAction.permissionsFieldName)
.map(value => value.convertTo[String])
.getOrElse(WhiskAction.defaultPermissions)

// the user who is owner by default
var currentExecutePermission = currentPermissions.charAt(2)
var errorInfo = s"have no permission to ${operation} this action"
if (user.namespace.name.asString != entityName.path.root.asString) { // the user who invoke the shared action
currentExecutePermission = currentPermissions.charAt(5)
errorInfo = s"have no permission to ${operation} this shared action"
}
if (currentExecutePermission == '-') { //have no permission
Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid))))
} else {
Future.successful(())
}
}
} else { // download the code
get(entityStore, entityName.toDocId, DocRevision.empty, true).flatMap { whiskAction =>
val currentPermissions = whiskAction.annotations
.get(WhiskAction.permissionsFieldName)
.map(value => value.convertTo[String])
.getOrElse(WhiskAction.defaultPermissions)

val errorInfo = s"have no permission to download this shared action"
val currentDownloadPermission = currentPermissions.charAt(3)
if (user.namespace.name.asString != entityName.path.root.asString) { // the shared user who download the action
if (currentDownloadPermission == '-') {
Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid))))
} else {
Future.successful(())
}
} else {
// the owner has download permission on any situation
Future.successful(())
}
}
}
}

/**
* Checks if a subject has the right to access a specific resource. The entitlement may be implicit,
* that is, inferred based on namespaces that a subject belongs to and the namespace of the
Expand Down
Loading

0 comments on commit db02599

Please sign in to comment.