Skip to content

Commit f00bf45

Browse files
authored
Reuse FieldPermissionsCache in Role parsing (#95065)
When parsing role descriptors, we ensure that the FieldPermissions (`"field_security":{ "grant":[ ... ], "except":[ ... ] }`) are valid - that is that any patterns compile correctly, and the "except" is a subset of the "grant". However, the previous implementation would not use the FieldPermissionsCache for this, so it would compile (union, intersect & minimize) automatons every time a role was parsed. This was particularly an issue when parsing roles (from the security index) in the GET /_security/role/ endpoint. If there were a large number of roles with field level security the automaton parsing could have significant impact on the performance of this API. Backport of: #94931
1 parent 21329d4 commit f00bf45

File tree

9 files changed

+77
-5
lines changed

9 files changed

+77
-5
lines changed

docs/changelog/94931.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 94931
2+
summary: Reuse `FieldPermissionsCache` in Role parsing
3+
area: Authorization
4+
type: enhancement
5+
issues: []

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import org.elasticsearch.common.io.stream.StreamInput;
1717
import org.elasticsearch.common.io.stream.StreamOutput;
1818
import org.elasticsearch.common.io.stream.Writeable;
19+
import org.elasticsearch.common.settings.Settings;
1920
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
2021
import org.elasticsearch.core.Nullable;
2122
import org.elasticsearch.xcontent.NamedXContentRegistry;
@@ -26,7 +27,8 @@
2627
import org.elasticsearch.xcontent.XContentParser;
2728
import org.elasticsearch.xcontent.XContentType;
2829
import org.elasticsearch.xcontent.json.JsonXContent;
29-
import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissions;
30+
import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissionsCache;
31+
import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissionsDefinition;
3032
import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivilege;
3133
import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivileges;
3234
import org.elasticsearch.xpack.core.security.support.Validation;
@@ -59,6 +61,17 @@ public class RoleDescriptor implements ToXContentObject, Writeable {
5961
private final Map<String, Object> metadata;
6062
private final Map<String, Object> transientMetadata;
6163

64+
/**
65+
* Needed as a stop-gap measure because {@link FieldPermissionsCache} has state (settings) but we need to use one
66+
* within {@link #checkIfExceptFieldsIsSubsetOfGrantedFields(String, String[], String[])} which is static.
67+
* Eventually we want to move parsing away from this class to its own object that can have an internal cache field
68+
*/
69+
private static FieldPermissionsCache fieldPermissionsCache = new FieldPermissionsCache(Settings.EMPTY);
70+
71+
public static synchronized void setFieldPermissionsCache(FieldPermissionsCache cache) {
72+
RoleDescriptor.fieldPermissionsCache = Objects.requireNonNull(cache);
73+
}
74+
6275
public RoleDescriptor(
6376
String name,
6477
@Nullable String[] clusterPrivileges,
@@ -678,7 +691,7 @@ private static RoleDescriptor.IndicesPrivileges parseIndex(String roleName, XCon
678691

679692
private static void checkIfExceptFieldsIsSubsetOfGrantedFields(String roleName, String[] grantedFields, String[] deniedFields) {
680693
try {
681-
FieldPermissions.buildPermittedFieldsAutomaton(grantedFields, deniedFields);
694+
fieldPermissionsCache.getFieldPermissions(new FieldPermissionsDefinition(grantedFields, deniedFields));
682695
} catch (ElasticsearchSecurityException e) {
683696
throw new ElasticsearchParseException("failed to parse indices privileges for role [{}] - {}", e, roleName, e.getMessage());
684697
}

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/FieldPermissionsCache.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
import org.apache.lucene.util.automaton.Automaton;
1010
import org.elasticsearch.ElasticsearchException;
11+
import org.elasticsearch.ElasticsearchSecurityException;
1112
import org.elasticsearch.common.cache.Cache;
1213
import org.elasticsearch.common.cache.CacheBuilder;
1314
import org.elasticsearch.common.settings.Setting;
@@ -48,6 +49,10 @@ public FieldPermissionsCache(Settings settings) {
4849
.build();
4950
}
5051

52+
public Cache.CacheStats getCacheStats() {
53+
return cache.stats();
54+
}
55+
5156
/**
5257
* Gets a {@link FieldPermissions} instance that corresponds to the granted and denied parameters. The instance may come from the cache
5358
* or if it gets created, the instance will be cached
@@ -67,7 +72,11 @@ public FieldPermissions getFieldPermissions(FieldPermissionsDefinition fieldPerm
6772
(key) -> new FieldPermissions(key, FieldPermissions.initializePermittedFieldsAutomaton(key))
6873
);
6974
} catch (ExecutionException e) {
70-
throw new ElasticsearchException("unable to compute field permissions", e);
75+
if (e.getCause() instanceof ElasticsearchException) {
76+
throw (ElasticsearchException) e.getCause();
77+
} else {
78+
throw new ElasticsearchSecurityException("unable to compute field permissions", e);
79+
}
7180
}
7281
}
7382

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@
145145
import org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken;
146146
import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine;
147147
import org.elasticsearch.xpack.core.security.authz.AuthorizationServiceField;
148+
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
148149
import org.elasticsearch.xpack.core.security.authz.accesscontrol.DocumentSubsetBitsetCache;
149150
import org.elasticsearch.xpack.core.security.authz.accesscontrol.IndicesAccessControl;
150151
import org.elasticsearch.xpack.core.security.authz.accesscontrol.SecurityIndexReaderWrapper;
@@ -709,6 +710,7 @@ Collection<Object> createComponents(
709710
);
710711
final NativeRolesStore nativeRolesStore = new NativeRolesStore(settings, client, getLicenseState(), securityIndex.get());
711712
final ReservedRolesStore reservedRolesStore = new ReservedRolesStore();
713+
RoleDescriptor.setFieldPermissionsCache(fieldPermissionsCache);
712714

713715
final Map<String, List<BiConsumer<Set<String>, ActionListener<RoleRetrievalResult>>>> customRoleProviders = new LinkedHashMap<>();
714716
for (SecurityExtension extension : securityExtensions) {
@@ -835,6 +837,7 @@ Collection<Object> createComponents(
835837
final AuthorizationService authzService = new AuthorizationService(
836838
settings,
837839
allRolesStore,
840+
fieldPermissionsCache,
838841
clusterService,
839842
auditTrailService,
840843
failureHandler,

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
import org.elasticsearch.xpack.core.security.authz.AuthorizationServiceField;
6464
import org.elasticsearch.xpack.core.security.authz.ResolvedIndices;
6565
import org.elasticsearch.xpack.core.security.authz.accesscontrol.IndicesAccessControl;
66+
import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissionsCache;
6667
import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeDescriptor;
6768
import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilegeResolver;
6869
import org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege;
@@ -138,6 +139,7 @@ public class AuthorizationService {
138139
public AuthorizationService(
139140
Settings settings,
140141
CompositeRolesStore rolesStore,
142+
FieldPermissionsCache fieldPermissionsCache,
141143
ClusterService clusterService,
142144
AuditTrailService auditTrailService,
143145
AuthenticationFailureHandler authcFailureHandler,
@@ -160,6 +162,7 @@ public AuthorizationService(
160162
this.rbacEngine = new RBACEngine(
161163
settings,
162164
rolesStore,
165+
fieldPermissionsCache,
163166
new LoadAuthorizedIndicesTimeChecker.Factory(logger, settings, clusterService.getClusterSettings())
164167
);
165168
this.authorizationEngine = authorizationEngine == null ? this.rbacEngine : authorizationEngine;

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,10 +123,11 @@ public class RBACEngine implements AuthorizationEngine {
123123
public RBACEngine(
124124
Settings settings,
125125
CompositeRolesStore rolesStore,
126+
FieldPermissionsCache fieldPermissionsCache,
126127
LoadAuthorizedIndicesTimeChecker.Factory authzIndicesTimerFactory
127128
) {
128129
this.rolesStore = rolesStore;
129-
this.fieldPermissionsCache = new FieldPermissionsCache(settings);
130+
this.fieldPermissionsCache = fieldPermissionsCache;
130131
this.authzIndicesTimerFactory = authzIndicesTimerFactory;
131132
}
132133

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,13 +227,15 @@ public class AuthorizationServiceTests extends ESTestCase {
227227
private ThreadPool threadPool;
228228
private Map<String, RoleDescriptor> roleMap = new HashMap<>();
229229
private CompositeRolesStore rolesStore;
230+
private FieldPermissionsCache fieldPermissionsCache;
230231
private OperatorPrivileges.OperatorPrivilegesService operatorPrivilegesService;
231232
private boolean shouldFailOperatorPrivilegesCheck = false;
232233
private boolean setFakeOriginatingAction = true;
233234

234235
@SuppressWarnings("unchecked")
235236
@Before
236237
public void setup() {
238+
fieldPermissionsCache = new FieldPermissionsCache(Settings.EMPTY);
237239
rolesStore = mock(CompositeRolesStore.class);
238240
clusterService = mock(ClusterService.class);
239241
final Settings settings = Settings.builder().put("cluster.remote.other_cluster.seeds", "localhost:9999").build();
@@ -275,6 +277,7 @@ public void setup() {
275277
authorizationService = new AuthorizationService(
276278
settings,
277279
rolesStore,
280+
fieldPermissionsCache,
278281
clusterService,
279282
auditTrailService,
280283
new DefaultAuthenticationFailureHandler(Collections.emptyMap()),
@@ -1511,6 +1514,7 @@ public void testDenialForAnonymousUser() throws IOException {
15111514
authorizationService = new AuthorizationService(
15121515
settings,
15131516
rolesStore,
1517+
fieldPermissionsCache,
15141518
clusterService,
15151519
auditTrailService,
15161520
new DefaultAuthenticationFailureHandler(Collections.emptyMap()),
@@ -1557,6 +1561,7 @@ public void testDenialForAnonymousUserAuthorizationExceptionDisabled() throws IO
15571561
authorizationService = new AuthorizationService(
15581562
settings,
15591563
rolesStore,
1564+
fieldPermissionsCache,
15601565
clusterService,
15611566
auditTrailService,
15621567
new DefaultAuthenticationFailureHandler(Collections.emptyMap()),
@@ -2680,6 +2685,7 @@ public void getUserPrivileges(
26802685
authorizationService = new AuthorizationService(
26812686
Settings.EMPTY,
26822687
rolesStore,
2688+
fieldPermissionsCache,
26832689
clusterService,
26842690
auditTrailService,
26852691
new DefaultAuthenticationFailureHandler(Collections.emptyMap()),

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine.AuthorizationInfo;
5555
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
5656
import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissions;
57+
import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissionsCache;
5758
import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissionsDefinition;
5859
import org.elasticsearch.xpack.core.security.authz.permission.ResourcePrivileges;
5960
import org.elasticsearch.xpack.core.security.authz.permission.Role;
@@ -118,7 +119,7 @@ public class RBACEngineTests extends ESTestCase {
118119
public void createEngine() {
119120
final LoadAuthorizedIndicesTimeChecker.Factory timerFactory = mock(LoadAuthorizedIndicesTimeChecker.Factory.class);
120121
when(timerFactory.newTimer(any())).thenReturn(LoadAuthorizedIndicesTimeChecker.NO_OP_CONSUMER);
121-
engine = new RBACEngine(Settings.EMPTY, mock(CompositeRolesStore.class), timerFactory);
122+
engine = new RBACEngine(Settings.EMPTY, mock(CompositeRolesStore.class), new FieldPermissionsCache(Settings.EMPTY), timerFactory);
122123
}
123124

124125
public void testSameUserPermission() {

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RoleDescriptorTests.java

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import org.elasticsearch.common.Strings;
1212
import org.elasticsearch.common.bytes.BytesArray;
1313
import org.elasticsearch.common.bytes.BytesReference;
14+
import org.elasticsearch.common.cache.Cache;
1415
import org.elasticsearch.common.io.stream.ByteBufferStreamInput;
1516
import org.elasticsearch.common.io.stream.BytesStreamOutput;
1617
import org.elasticsearch.common.io.stream.NamedWriteableAwareStreamInput;
@@ -27,6 +28,7 @@
2728
import org.elasticsearch.xpack.core.XPackClientPlugin;
2829
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
2930
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor.ApplicationResourcePrivileges;
31+
import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissionsCache;
3032
import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilegeResolver;
3133
import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivilege;
3234
import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivileges;
@@ -228,6 +230,35 @@ public void testParse() throws Exception {
228230
assertThat(ex.getMessage(), containsString("not_supported"));
229231
}
230232

233+
public void testParsingFieldPermissionsUsesCache() throws IOException {
234+
FieldPermissionsCache fieldPermissionsCache = new FieldPermissionsCache(Settings.EMPTY);
235+
RoleDescriptor.setFieldPermissionsCache(fieldPermissionsCache);
236+
237+
final Cache.CacheStats beforeStats = fieldPermissionsCache.getCacheStats();
238+
239+
final String json = "{ \"index\": [ "
240+
+ "{ \"names\": \"index-001\", \"privileges\": [ \"read\" ],"
241+
+ " \"field_security\": { \"grant\": [ \"field-001\", \"field-002\" ] } },"
242+
+ "{ \"names\": \"index-001\", \"privileges\": [ \"read\" ], "
243+
+ " \"field_security\": { \"grant\": [ \"*\" ], \"except\": [ \"field-003\" ] } }"
244+
+ "] }";
245+
RoleDescriptor.parse("test", new BytesArray(json), false, XContentType.JSON);
246+
247+
final int numberOfFieldSecurityBlocks = 2;
248+
final Cache.CacheStats betweenStats = fieldPermissionsCache.getCacheStats();
249+
assertThat(betweenStats.getMisses(), equalTo(beforeStats.getMisses() + numberOfFieldSecurityBlocks));
250+
assertThat(betweenStats.getHits(), equalTo(beforeStats.getHits()));
251+
252+
final int iterations = randomIntBetween(1, 5);
253+
for (int i = 0; i < iterations; i++) {
254+
RoleDescriptor.parse("test", new BytesArray(json), false, XContentType.JSON);
255+
}
256+
257+
final Cache.CacheStats afterStats = fieldPermissionsCache.getCacheStats();
258+
assertThat(afterStats.getMisses(), equalTo(betweenStats.getMisses()));
259+
assertThat(afterStats.getHits(), equalTo(beforeStats.getHits() + numberOfFieldSecurityBlocks * iterations));
260+
}
261+
231262
public void testSerialization() throws Exception {
232263
final Version version = VersionUtils.randomVersionBetween(random(), Version.V_6_4_0, null);
233264
logger.info("Testing serialization with version {}", version);

0 commit comments

Comments
 (0)