Skip to content
This repository has been archived by the owner on Mar 16, 2024. It is now read-only.

Commit

Permalink
Make cluster-scoped permission encompass project-scoped
Browse files Browse the repository at this point in the history
After this change, if you are granted cluster-scoped permission, then
you are also grant project-scoped for any project.

This commit also includes some simplifications to the handler that
checks permissions.

Signed-off-by: Donnie Adams <donnie@acorn.io>
  • Loading branch information
thedadams committed Oct 4, 2023
1 parent d985d69 commit dac2a1d
Show file tree
Hide file tree
Showing 4 changed files with 182 additions and 83 deletions.
2 changes: 1 addition & 1 deletion pkg/apis/internal.acorn.io/v1/appspec.go
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ func (p PolicyRule) Grants(currentNamespace string, requested PolicyRule) bool {

for _, ns := range p.ResolveNamespaces(currentNamespace) {
for _, requestedNamespace := range requested.ResolveNamespaces(currentNamespace) {
if ns == requestedNamespace &&
if (ns == requestedNamespace || ns == "") &&
matches(p.Verbs, requested.Verbs, false) &&
matches(p.APIGroups, requested.APIGroups, false) &&
matches(p.Resources, requested.Resources, false) &&
Expand Down
102 changes: 102 additions & 0 deletions pkg/apis/internal.acorn.io/v1/appspec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,3 +149,105 @@ func TestSimplify(t *testing.T) {
},
}))
}

func TestGrantsAllClusterGrantsProject(t *testing.T) {
requested := []Permissions{
{
ServiceName: "foo",
Rules: []PolicyRule{
{
PolicyRule: rbacv1.PolicyRule{
Verbs: []string{"get"},
APIGroups: []string{"group-a"},
},
Scopes: []string{"project"},
},
},
},
}

granted := []Permissions{
{
ServiceName: "foo",
Rules: []PolicyRule{
{
PolicyRule: rbacv1.PolicyRule{
Verbs: []string{"get"},
APIGroups: []string{"group-a"},
},
Scopes: []string{"cluster"},
},
},
},
}
gotMissing, _ := GrantsAll("acorn", requested, granted)
assert.Equal(t, []Permissions(nil), gotMissing, "cluster permissions should grant project permissions")
}

func TestGrantsAllClusterGrantsNamespace(t *testing.T) {
requested := []Permissions{
{
ServiceName: "foo",
Rules: []PolicyRule{
{
PolicyRule: rbacv1.PolicyRule{
Verbs: []string{"get"},
APIGroups: []string{"group-a"},
},
Scopes: []string{"namespace:bar"},
},
},
},
}

granted := []Permissions{
{
ServiceName: "foo",
Rules: []PolicyRule{
{
PolicyRule: rbacv1.PolicyRule{
Verbs: []string{"get"},
APIGroups: []string{"group-a"},
},
Scopes: []string{"cluster"},
},
},
},
}
gotMissing, _ := GrantsAll("acorn", requested, granted)
assert.Equal(t, []Permissions(nil), gotMissing, "cluster permissions should grant namespace permissions")
}

func TestGrantsAllProjectNotGrantCluster(t *testing.T) {
requested := []Permissions{
{
ServiceName: "foo",
Rules: []PolicyRule{
{
PolicyRule: rbacv1.PolicyRule{
Verbs: []string{"get"},
APIGroups: []string{"group-a"},
},
Scopes: []string{"cluster"},
},
},
},
}

granted := []Permissions{
{
ServiceName: "foo",
Rules: []PolicyRule{
{
PolicyRule: rbacv1.PolicyRule{
Verbs: []string{"get"},
APIGroups: []string{"group-a"},
},
Scopes: []string{"project"},
},
},
},
}
gotMissing, _ := GrantsAll("acorn", requested, granted)
assert.Equal(t, requested, gotMissing, "project permissions should not grant cluster permissions")
}
159 changes: 78 additions & 81 deletions pkg/controller/permissions/permissions_check.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package permissions
import (
"errors"
"fmt"
"net/http"
"strings"

"github.com/acorn-io/baaah/pkg/router"
Expand Down Expand Up @@ -44,105 +43,103 @@ func CopyPromoteStagedAppImage(req router.Request, resp router.Response) error {
//
// b.1) granted by the user (as set in the app spec)
// b.2) authorized by the image role authorizations (if enabled)
func CheckPermissions(transport http.RoundTripper) router.HandlerFunc {
return func(req router.Request, _ router.Response) error {
app := req.Object.(*v1.AppInstance)

iraEnabled, err := config.GetFeature(req.Ctx, req.Client, profiles.FeatureImageRoleAuthorizations)
if err != nil {
return err
}
if !iraEnabled {
app.Status.Staged.ImagePermissionsDenied = nil
}
func CheckPermissions(req router.Request, _ router.Response) error {
app := req.Object.(*v1.AppInstance)

if app.Status.Staged.AppImage.ID == "" ||
app.Status.Staged.AppImage.Digest == app.Status.AppImage.Digest ||
app.Status.Staged.PermissionsObservedGeneration == app.Generation {
if enabled, err := config.GetFeature(req.Ctx, req.Client, profiles.FeatureImageAllowRules); err != nil {
return err
} else if !enabled {
app.Status.Staged.ImageAllowed = z.Pointer(true)
}
return nil
}
iraEnabled, err := config.GetFeature(req.Ctx, req.Client, profiles.FeatureImageRoleAuthorizations)
if err != nil {
return err
}
if !iraEnabled {
app.Status.Staged.ImagePermissionsDenied = nil
}

if err := checkImageAllowed(req.Ctx, req.Client, app); err != nil {
if app.Status.Staged.AppImage.ID == "" ||
app.Status.Staged.AppImage.Digest == app.Status.AppImage.Digest ||
app.Status.Staged.PermissionsObservedGeneration == app.Generation {
if enabled, err := config.GetFeature(req.Ctx, req.Client, profiles.FeatureImageAllowRules); err != nil {
return err
} else if !enabled {
app.Status.Staged.ImageAllowed = z.Pointer(true)
}
return nil
}

var (
appImage = app.Status.Staged.AppImage
imageName = appImage.ID
details = &apiv1.ImageDetails{
DeployArgs: app.Spec.DeployArgs,
Profiles: app.Spec.GetProfiles(app.Status.GetDevMode()),
IncludeNested: true,
}
)
if err := checkImageAllowed(req.Ctx, req.Client, app); err != nil {
return err
}

if !tags.IsLocalReference(imageName) {
ref, err := name.ParseReference(imageName)
if err != nil {
return err
}
imageName = ref.Context().Digest(appImage.Digest).String()
var (
appImage = app.Status.Staged.AppImage
imageName = appImage.ID
details = &apiv1.ImageDetails{
DeployArgs: app.Spec.DeployArgs,
Profiles: app.Spec.GetProfiles(app.Status.GetDevMode()),
IncludeNested: true,
}
)

err = req.Client.SubResource("details").Create(req.Ctx, uncached.Get(&apiv1.Image{
ObjectMeta: metav1.ObjectMeta{
Name: strings.ReplaceAll(imageName, "/", "+"),
Namespace: app.Namespace,
},
}), details)
if !tags.IsLocalReference(imageName) {
ref, err := name.ParseReference(imageName)
if err != nil {
return err
} else if details.GetParseError() != "" {
return errors.New(details.GetParseError())
}

if details.AppImage.Digest != appImage.Digest {
return fmt.Errorf("failed to lookup image [%s], resolved to digest [%s] but expected [%s]", imageName,
details.AppImage.Digest, appImage.Digest)
}
imageName = ref.Context().Digest(appImage.Digest).String()
}

// If iraEnabled, check if the Acorn images are authorized to request the defined permissions.
if iraEnabled {
imageName := appImage.Name
err = req.Client.SubResource("details").Create(req.Ctx, uncached.Get(&apiv1.Image{
ObjectMeta: metav1.ObjectMeta{
Name: strings.ReplaceAll(imageName, "/", "+"),
Namespace: app.Namespace,
},
}), details)
if err != nil {
return err
} else if details.GetParseError() != "" {
return errors.New(details.GetParseError())
}

// E.g. for child Acorns, the appImage.Name is the image ID, but we need the original image name (with registry/repo)
// to check for the signatures
if oi, ok := app.GetAnnotations()[labels.AcornOriginalImage]; ok {
imageName = oi
}
if details.AppImage.Digest != appImage.Digest {
return fmt.Errorf("failed to lookup image [%s], resolved to digest [%s] but expected [%s]", imageName,
details.AppImage.Digest, appImage.Digest)
}

authzPerms, err := imagerules.GetAuthorizedPermissions(req.Ctx, req.Client, app.Namespace, imageName, appImage.Digest)
if err != nil {
return err
}
// If iraEnabled, check if the Acorn images are authorized to request the defined permissions.
if iraEnabled {
imageName := appImage.Name

// Need to deepcopy here since otherwise we'd override the name in the original object which we still need for other authz checks
// For IRA Checks, we use the image name as the service name
copyWithName := func(perms []v1.Permissions, name string) []v1.Permissions {
nperms := make([]v1.Permissions, len(perms))
for i := range perms {
nperms[i] = perms[i].DeepCopy().Get()
nperms[i].ServiceName = name
}
return nperms
}
// E.g. for child Acorns, the appImage.Name is the image ID, but we need the original image name (with registry/repo)
// to check for the signatures
if oi, ok := app.GetAnnotations()[labels.AcornOriginalImage]; ok {
imageName = oi
}

denied, _ := v1.GrantsAll(app.Namespace, copyWithName(details.Permissions, imageName), authzPerms)
authzPerms, err := imagerules.GetAuthorizedPermissions(req.Ctx, req.Client, app.Namespace, imageName, appImage.Digest)
if err != nil {
return err
}

app.Status.Staged.ImagePermissionsDenied = denied
// Need to deepcopy here since otherwise we'd override the name in the original object which we still need for other authz checks
// For IRA Checks, we use the image name as the service name
copyWithName := func(perms []v1.Permissions, name string) []v1.Permissions {
nperms := make([]v1.Permissions, len(perms))
for i := range perms {
nperms[i] = perms[i].DeepCopy().Get()
nperms[i].ServiceName = name
}
return nperms
}

// This is checking if the user granted all permissions that the app requires
missing, _ := v1.GrantsAll(app.Namespace, details.GetPermissions(), app.Spec.GetPermissions())
app.Status.Staged.PermissionsObservedGeneration = app.Generation
app.Status.Staged.PermissionsChecked = true
app.Status.Staged.PermissionsMissing = missing
denied, _ := v1.GrantsAll(app.Namespace, copyWithName(details.Permissions, imageName), authzPerms)

return nil
app.Status.Staged.ImagePermissionsDenied = denied
}

// This is checking if the user granted all permissions that the app requires
missing, _ := v1.GrantsAll(app.Namespace, details.GetPermissions(), app.Spec.GetPermissions())
app.Status.Staged.PermissionsObservedGeneration = app.Generation
app.Status.Staged.PermissionsChecked = true
app.Status.Staged.PermissionsMissing = missing

return nil
}
2 changes: 1 addition & 1 deletion pkg/controller/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func routes(router *router.Router, cfg *rest.Config, registryTransport http.Roun
appRouter.HandlerFunc(appstatus.PrepareStatus)
appRouter.HandlerFunc(appdefinition.AssignNamespace)
appRouter.HandlerFunc(appdefinition.PullAppImage(registryTransport, recorder))
appRouter.HandlerFunc(permissions.CheckPermissions(registryTransport))
appRouter.HandlerFunc(permissions.CheckPermissions)
appRouter.HandlerFunc(permissions.CopyPromoteStagedAppImage)
appRouter.HandlerFunc(images.CreateImages)
appRouter.HandlerFunc(appdefinition.ParseAppImage)
Expand Down

0 comments on commit dac2a1d

Please sign in to comment.