Tighten API key behaviour with DLS and incompatible license#78378
Conversation
When the cluster license is incompatible with DLS and FLS, instead of failing open, API keys now fail closed by throwing an error if any of the target indices has DLS/FLS protection.
|
Pinging @elastic/es-security (Team:Security) |
|
@ywangd thanks for creating this. |
Yes as long as the index has DLS/FLS protection regardless where the protection comes from (key roles or creator roles). The updated test covers the both scenarios. |
|
@ywangd what's the behavior for api keys that have both dls and non-dls index privileges? For example: {
"name": "my-api-key",
"role_descriptors": {
"my-role": {
"indices": [
{
"names": ["someindex-1"],
"privileges": ["read"]
},
{
"names": ["someindex-*"],
"privileges": ["read"],
"query": { ... }
}
]
}
}
}Will the |
|
After some more thinking, I decided to add a new interceptor instead of relying on existing interceptors. The purpose of existing interceptors is to prevent certain misuse of DLS/FLS (e.g. update with DLS/FLS, caching etc.). So they don't cover the cases where misuse is not applicable. For example, the Get API is not covered and hence previous approach of handling incompatible license does not work correctly for Get API, i.e. you can still The new interceptor should cover all cases. I didn't add new tests for the new changes because I'd like to agree on the approach first. Thanks! |
Yes. You will still be able to search |
| "Checking for whether the request touches any indices that have DLS or FLS configured"); | ||
| final IndicesAccessControl indicesAccessControl = threadContext.getTransient(INDICES_PERMISSIONS_KEY); | ||
| if (indicesAccessControl != null && indicesAccessControl.hasFieldOrDocumentLevelSecurity()) { | ||
| final ElasticsearchSecurityException licenseException = |
There was a problem hiding this comment.
The actual check for DLS/FLS is guarded by several conditions for performance because it can be expensive to iterate through a large list of indices on every request. The assumption here is that a Role with DLS/FLS while license is incompatible should be an exceptional case. Therefore, most normal usages will return quickly.
|
@elasticmachine update branch |
|
@elasticmachine run elasticsearch-ci/part-1-fips |
| public boolean hasFieldOrDocumentLevelSecurity() { | ||
| return indices.hasFieldOrDocumentLevelSecurity(); | ||
| } |
There was a problem hiding this comment.
If the PR gets merged, this new method here can be used in IndicesPermission#authorize to avoid the loop for computing DLS and FLS controls when the Roles does not have it.
Great—thanks @ywangd! |
| import static org.elasticsearch.xpack.core.security.authz.AuthorizationServiceField.AUTHORIZATION_INFO_KEY; | ||
| import static org.elasticsearch.xpack.core.security.authz.AuthorizationServiceField.INDICES_PERMISSIONS_KEY; | ||
|
|
||
| public class LicenseComplianceRequestInterceptor implements RequestInterceptor { |
There was a problem hiding this comment.
I wonder whether this should have a more specific name.
Since it's only installed if DLS/FLS is enabled, it's probably worth naming it to be explicitly about DLS/FLS.
There was a problem hiding this comment.
Thanks, I renamed it to DlsFlsLicenseComplianceRequestInterceptor.
Btw, it wasn't more specifically named because I noticed BulkShardRequestInterceptor is about DLS/FLS and only has a generic name. But on hindsight, I don't think it is a good reference.
💔 Backport failed
You can use sqren/backport to manually backport by running |
…78378) When the cluster license is incompatible with DLS and FLS, instead of failing open, API keys now fail closed by throwing an error if any of the target indices has DLS/FLS protection.
When the cluster license is incompatible with DLS and FLS, instead of
failing open, API keys now fail closed by throwing an error if any of
the target indices has DLS/FLS protection.