spring security acl doesn't compare bitwise permissions
Asked Answered
M

1

6

We've encountered an unexpected lack of bitwise permission checking in spring security. We'd like to confirm if this is the expected behaviour and if so, what the history is and/or rationale for this.

We're using the grails plugin spring-security-acl-1.1.1 which uses spring-security-acl 3.0.7.RELEASE.

A distilled scenario of what we are trying involves an object with ACL such that aclUtilService.readAcl(obj) returns granting roles:

PrincipalSid[sampleuser]; permission: BasePermission[...........................A....=16]
GrantedAuthoritySid[ROLE_RWD]; permission: CumulativePermission[............................D.WR=11]
GrantedAuthoritySid[ROLE_RW]; permission: CumulativePermission[..............................WR=3]
GrantedAuthoritySid[ROLE_R]; permission: BasePermission[...............................R=1]

Subsequently we expect to check for a single permission like WRITE and have it return true. However it appears this is not supported. For example, for a user who has all of the roles defined on the object above, the execution of:

READ?: ${aclUtilService.hasPermission(springSecurityService.authentication, obj, BasePermission.READ)}
WRITE?: ${aclUtilService.hasPermission(springSecurityService.authentication, obj, BasePermission.WRITE)}
DELETE?: ${aclUtilService.hasPermission(springSecurityService.authentication, obj, BasePermission.DELETE)}
READ-WRITE?: ${aclUtilService.hasPermission(springSecurityService.authentication, obj, new BasePermission(BasePermission.READ.getMask() | BasePermission.WRITE.getMask()))}

Returns output:

READ?: true
WRITE?: false
DELETE?: false
READ-WRITE?: true

Whereas we would expect all of these to have returned true. Having looked at the source we can see the permission is ultimately checked in AclImpl, which contains the line

if ((ace.getPermission().getMask() == p.getMask()) && ace.getSid().equals(sid)) {

Which explains why only the exact masks are matching.

Changing only this line is somewhat involved and we have found that in spring-security-acl 3.1 this code was refactored to allow for a permission granting strategy to be defined- https://jira.spring.io/browse/SEC-1166

However the default granting strategy still only checks the exact mask. So:

  • Why doesn't the default granting strategy check bitwise masks? What is the point of the CumulativePermission if this shouldn't be checked?
  • What is the preferred approach for handling bitwise permissions, are we configuring the permissions incorrectly or should we just look to implement bitwise permission checking (and in which case, what is the preferred way of doing this).

Thanks for any explanation or guidance.

Mentalist answered 22/9, 2014 at 10:33 Comment(2)
I've added a comment to jira.spring.io/browse/SEC-1166 that supposes olifernas solution to be adopted by Spring Security.Dismissal
Possible duplicate of this: https://mcmap.net/q/1013851/-how-do-i-use-the-mask-field-in-acl_entry-table-in-spring-security-3-1/5134722Jerome
E
3

This surprised me a lot when I was creating the plugin. It seemed very odd to pretend to use bit masking, and in the end limit yourself to only 32 permissions (although that should be enough for most apps). See this JIRA for Ben Alex's explantation: https://jira.spring.io/browse/SEC-1140

Expectoration answered 23/9, 2014 at 1:0 Comment(2)
Thanks Burt, SEC-1140 explains the situation and rationale behind it completely. It seems the common confusion comes from having a design which looks like masking is supported, yet the default behaviour does not take advantage of that. For anyone reading this in future, the options in this situation appear to be either: * Use many separate BasePermission entries instead of CumulativePermission. * Use at least spring-security-acl 3.1 and provide a custom comparison strategy.Mentalist
A small modification of Spring's DefaultPermissionGrantingStrategy that performs bit mask comparision. gist.github.com/oliverfernandez/36846fcdc03696a7b829 What do you think?Quenelle

© 2022 - 2024 — McMap. All rights reserved.