-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Add a monitor_stats privilege and allow that privilege for remote cluster privileges #114964
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 35 commits
f95eef8
89aa208
32017a7
10dda91
a720c82
1f7c189
5e3f7a7
8ce0e2b
d827849
0e0c5db
fa10f80
4095cca
aeb8ff1
86c0367
c259d9d
3ccc03b
25ee328
49a4d1e
7e4152f
c380112
a34b620
d711d15
e2d4c16
b4cce77
531fe9c
2cde2f8
633aebe
abe85e0
cb31734
2a5f79e
8264279
892ac08
b4b85c8
7e9cb22
d35dee8
1c749bc
0d6acd1
d19a074
1ee1d43
cdb3791
4b1dd9d
c734bbb
23279e5
3182bcd
7a66dcb
d1711c4
fd2503e
b816117
d1f4029
31096b7
ba6b70e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
pr: 114964 | ||
summary: Add a `monitor_stats` privilege and allow that privilege for remote cluster | ||
privileges | ||
area: Authorization | ||
type: enhancement | ||
issues: [] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,6 +36,7 @@ | |
import org.elasticsearch.xpack.core.security.authc.service.ServiceAccountSettings; | ||
import org.elasticsearch.xpack.core.security.authc.support.AuthenticationContextSerializer; | ||
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; | ||
import org.elasticsearch.xpack.core.security.authz.permission.RemoteClusterPermissions; | ||
import org.elasticsearch.xpack.core.security.user.AnonymousUser; | ||
import org.elasticsearch.xpack.core.security.user.InternalUser; | ||
import org.elasticsearch.xpack.core.security.user.InternalUsers; | ||
|
@@ -76,6 +77,7 @@ | |
import static org.elasticsearch.xpack.core.security.authc.AuthenticationField.FALLBACK_REALM_NAME; | ||
import static org.elasticsearch.xpack.core.security.authc.AuthenticationField.FALLBACK_REALM_TYPE; | ||
import static org.elasticsearch.xpack.core.security.authc.RealmDomain.REALM_DOMAIN_PARSER; | ||
import static org.elasticsearch.xpack.core.security.authz.RoleDescriptor.Fields.REMOTE_CLUSTER; | ||
import static org.elasticsearch.xpack.core.security.authz.permission.RemoteClusterPermissions.ROLE_REMOTE_CLUSTER_PRIVS; | ||
|
||
/** | ||
|
@@ -233,8 +235,8 @@ public Authentication maybeRewriteForOlderVersion(TransportVersion olderVersion) | |
+ "]" | ||
); | ||
} | ||
|
||
final Map<String, Object> newMetadata = maybeRewriteMetadata(olderVersion, this); | ||
|
||
final Authentication newAuthentication; | ||
if (isRunAs()) { | ||
// The lookup user for run-as currently doesn't have authentication metadata associated with them because | ||
|
@@ -272,12 +274,23 @@ public Authentication maybeRewriteForOlderVersion(TransportVersion olderVersion) | |
} | ||
|
||
private static Map<String, Object> maybeRewriteMetadata(TransportVersion olderVersion, Authentication authentication) { | ||
if (authentication.isAuthenticatedAsApiKey()) { | ||
return maybeRewriteMetadataForApiKeyRoleDescriptors(olderVersion, authentication); | ||
} else if (authentication.isCrossClusterAccess()) { | ||
return maybeRewriteMetadataForCrossClusterAccessAuthentication(olderVersion, authentication); | ||
} else { | ||
return authentication.getAuthenticatingSubject().getMetadata(); | ||
try { | ||
if (authentication.isAuthenticatedAsApiKey()) { | ||
return maybeRewriteMetadataForApiKeyRoleDescriptors(olderVersion, authentication); | ||
} else if (authentication.isCrossClusterAccess()) { | ||
return maybeRewriteMetadataForCrossClusterAccessAuthentication(olderVersion, authentication); | ||
} else { | ||
return authentication.getAuthenticatingSubject().getMetadata(); | ||
} | ||
} catch (Exception e) { | ||
// CCS workflows may swallow the exception message making this difficult to troubleshoot, so we explicitly log and re-throw | ||
// here. It may result in duplicate logs, so we only log the message at warn level. | ||
if (logger.isDebugEnabled()) { | ||
logger.debug("Un-expected exception thrown while rewriting metadata. This is likely a bug.", e); | ||
} else { | ||
logger.warn("Un-expected exception thrown while rewriting metadata. This is likely a bug [" + e.getMessage() + "]"); | ||
} | ||
throw e; | ||
jakelandis marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} | ||
|
||
|
@@ -1323,6 +1336,7 @@ private static Map<String, Object> maybeRewriteMetadataForApiKeyRoleDescriptors( | |
|
||
if (authentication.getEffectiveSubject().getTransportVersion().onOrAfter(ROLE_REMOTE_CLUSTER_PRIVS) | ||
&& streamVersion.before(ROLE_REMOTE_CLUSTER_PRIVS)) { | ||
// if the remote does not understand the remote_cluster field remove it | ||
metadata = new HashMap<>(metadata); | ||
metadata.put( | ||
AuthenticationField.API_KEY_ROLE_DESCRIPTORS_KEY, | ||
|
@@ -1336,7 +1350,27 @@ private static Map<String, Object> maybeRewriteMetadataForApiKeyRoleDescriptors( | |
(BytesReference) metadata.get(AuthenticationField.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY) | ||
) | ||
); | ||
} | ||
} else if (authentication.getEffectiveSubject() | ||
.getTransportVersion() | ||
.onOrAfter(TRANSPORT_VERSION_ADVANCED_REMOTE_CLUSTER_SECURITY) | ||
|
||
&& streamVersion.onOrAfter(ROLE_REMOTE_CLUSTER_PRIVS)) { | ||
// the remote does understand the remote_cluster field, so check each permission and remove permission as needed | ||
metadata = new HashMap<>(metadata); | ||
metadata.put( | ||
AuthenticationField.API_KEY_ROLE_DESCRIPTORS_KEY, | ||
maybeRemoveRemoteClusterPrivilegesFromRoleDescriptors( | ||
(BytesReference) metadata.get(AuthenticationField.API_KEY_ROLE_DESCRIPTORS_KEY), | ||
streamVersion | ||
) | ||
); | ||
metadata.put( | ||
AuthenticationField.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY, | ||
maybeRemoveRemoteClusterPrivilegesFromRoleDescriptors( | ||
(BytesReference) metadata.get(AuthenticationField.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY), | ||
streamVersion | ||
) | ||
); | ||
} | ||
|
||
if (authentication.getEffectiveSubject().getTransportVersion().onOrAfter(VERSION_API_KEY_ROLES_AS_BYTES) | ||
&& streamVersion.before(VERSION_API_KEY_ROLES_AS_BYTES)) { | ||
|
@@ -1417,7 +1451,7 @@ private static BytesReference convertRoleDescriptorsMapToBytes(Map<String, Objec | |
} | ||
|
||
static BytesReference maybeRemoveRemoteClusterFromRoleDescriptors(BytesReference roleDescriptorsBytes) { | ||
return maybeRemoveTopLevelFromRoleDescriptors(roleDescriptorsBytes, RoleDescriptor.Fields.REMOTE_CLUSTER.getPreferredName()); | ||
return maybeRemoveTopLevelFromRoleDescriptors(roleDescriptorsBytes, REMOTE_CLUSTER.getPreferredName()); | ||
} | ||
|
||
static BytesReference maybeRemoveRemoteIndicesFromRoleDescriptors(BytesReference roleDescriptorsBytes) { | ||
|
@@ -1450,6 +1484,66 @@ static BytesReference maybeRemoveTopLevelFromRoleDescriptors(BytesReference role | |
} | ||
} | ||
|
||
/** | ||
* Before we send the role descriptors to the remote cluster, we need to remove the remote cluster privileges that the other cluster | ||
* will not understand. If all privileges are removed, then the entire "remote_cluster" is removed to avoid sending empty privileges. | ||
* @param roleDescriptorsBytes The role descriptors to be sent to the remote cluster, represented as bytes. | ||
* @return The role descriptors with the privileges that unsupported by version removed, represented as bytes. | ||
*/ | ||
@SuppressWarnings("unchecked") | ||
static BytesReference maybeRemoveRemoteClusterPrivilegesFromRoleDescriptors( | ||
n1v0lg marked this conversation as resolved.
Show resolved
Hide resolved
|
||
BytesReference roleDescriptorsBytes, | ||
TransportVersion outboundVersion | ||
) { | ||
if (roleDescriptorsBytes == null || roleDescriptorsBytes.length() == 0) { | ||
return roleDescriptorsBytes; | ||
} | ||
final Map<String, Object> roleDescriptorsMap = convertRoleDescriptorsBytesToMap(roleDescriptorsBytes); | ||
final Map<String, Object> roleDescriptorsMapMutated = new HashMap<>(roleDescriptorsMap); | ||
final AtomicBoolean modified = new AtomicBoolean(false); | ||
roleDescriptorsMap.forEach((key, value) -> { | ||
if (value instanceof Map) { | ||
Map<String, Object> roleDescriptor = (Map<String, Object>) value; | ||
roleDescriptor.forEach((innerKey, innerValue) -> { | ||
// example: remote_cluster=[{privileges=[monitor_enrich, monitor_stats] | ||
if (REMOTE_CLUSTER.getPreferredName().equals(innerKey)) { | ||
assert innerValue instanceof List; | ||
RemoteClusterPermissions discoveredRemoteClusterPermission = new RemoteClusterPermissions( | ||
(List<Map<String, List<String>>>) innerValue | ||
); | ||
RemoteClusterPermissions mutated = discoveredRemoteClusterPermission.removeUnsupportedPrivileges(outboundVersion); | ||
if (mutated.equals(discoveredRemoteClusterPermission) == false) { | ||
// swap out the old value with the new value | ||
modified.set(true); | ||
Map<String, Object> remoteClusterMap = new HashMap<>((Map<String, Object>) roleDescriptorsMapMutated.get(key)); | ||
if (mutated.hasPrivileges()) { | ||
// has at least one group with privileges | ||
remoteClusterMap.put(innerKey, mutated.toMap()); | ||
} else { | ||
// has no groups with privileges | ||
remoteClusterMap.remove(innerKey); | ||
} | ||
roleDescriptorsMapMutated.put(key, remoteClusterMap); | ||
} | ||
} | ||
}); | ||
} | ||
}); | ||
if (modified.get()) { | ||
logger.debug( | ||
"mutated role descriptors. Changed from {} to {} for outbound version {}", | ||
roleDescriptorsMap, | ||
roleDescriptorsMapMutated, | ||
outboundVersion | ||
); | ||
return convertRoleDescriptorsMapToBytes(roleDescriptorsMapMutated); | ||
} else { | ||
// No need to serialize if we did not change anything. | ||
logger.trace("no change to role descriptors {} for outbound version {}", roleDescriptorsMap, outboundVersion); | ||
return roleDescriptorsBytes; | ||
} | ||
} | ||
|
||
static boolean equivalentRealms(String name1, String type1, String name2, String type2) { | ||
if (false == type1.equals(type2)) { | ||
return false; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,8 @@ | |
*/ | ||
package org.elasticsearch.xpack.core.security.authz; | ||
|
||
import org.apache.logging.log4j.LogManager; | ||
import org.apache.logging.log4j.Logger; | ||
import org.elasticsearch.ElasticsearchParseException; | ||
import org.elasticsearch.ElasticsearchSecurityException; | ||
import org.elasticsearch.TransportVersion; | ||
|
@@ -62,6 +64,7 @@ public class RoleDescriptor implements ToXContentObject, Writeable { | |
public static final TransportVersion SECURITY_ROLE_DESCRIPTION = TransportVersions.V_8_15_0; | ||
|
||
public static final String ROLE_TYPE = "role"; | ||
private static final Logger logger = LogManager.getLogger(RoleDescriptor.class); | ||
|
||
private final String name; | ||
private final String[] clusterPrivileges; | ||
|
@@ -830,25 +833,32 @@ private static RemoteClusterPermissions parseRemoteCluster(final String roleName | |
currentFieldName = parser.currentName(); | ||
} else if (Fields.PRIVILEGES.match(currentFieldName, parser.getDeprecationHandler())) { | ||
privileges = readStringArray(roleName, parser, false); | ||
if (privileges.length != 1 | ||
|| RemoteClusterPermissions.getSupportedRemoteClusterPermissions() | ||
.contains(privileges[0].trim().toLowerCase(Locale.ROOT)) == false) { | ||
throw new ElasticsearchParseException( | ||
"failed to parse remote_cluster for role [{}]. " | ||
+ RemoteClusterPermissions.getSupportedRemoteClusterPermissions() | ||
+ " is the only value allowed for [{}] within [remote_cluster]", | ||
if (Arrays.stream(privileges) | ||
.map(s -> s.toLowerCase(Locale.ROOT)) | ||
|
||
.allMatch(RemoteClusterPermissions.getSupportedRemoteClusterPermissions()::contains) == false) { | ||
final String message = String.format( | ||
Locale.ROOT, | ||
"failed to parse remote_cluster for role [%s]. " | ||
+ "%s are the only values allowed for [%s] within [remote_cluster]. Found %s", | ||
roleName, | ||
currentFieldName | ||
RemoteClusterPermissions.getSupportedRemoteClusterPermissions(), | ||
currentFieldName, | ||
Arrays.toString(privileges) | ||
); | ||
logger.warn(message); | ||
throw new ElasticsearchParseException(message); | ||
} | ||
} else if (Fields.CLUSTERS.match(currentFieldName, parser.getDeprecationHandler())) { | ||
clusters = readStringArray(roleName, parser, false); | ||
} else { | ||
throw new ElasticsearchParseException( | ||
"failed to parse remote_cluster for role [{}]. unexpected field [{}]", | ||
final String message = String.format( | ||
Locale.ROOT, | ||
"failed to parse remote_cluster for role [%s]. unexpected field [%s]", | ||
roleName, | ||
currentFieldName | ||
); | ||
logger.warn(message); | ||
|
||
throw new ElasticsearchParseException(message); | ||
} | ||
} | ||
if (privileges != null && clusters == null) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I've stared at this for a bit but I don't fully get it -- why will that test have the wrong value for num privileges? Is it because the test spec is somehow sourced from an older ES version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, exactly. The compat tests are sourced from the last minor of the last major (8.x). I also realized that this will break as soon as I back port this code to 8.x, so I will skip it for now. There probably isn't a need to ever re-enable it either, since the number of privileges is not relevant to compatibility.