Skip to content

Commit

Permalink
Remove code related to TransportClient auth/auth (opensearch-project#…
Browse files Browse the repository at this point in the history
…1578)

Signed-off-by: Jochen Kressin <jkressin@floragunn.com>
  • Loading branch information
jochenkressin authored and peternied committed Apr 7, 2022
1 parent 2744081 commit 779c09f
Show file tree
Hide file tree
Showing 11 changed files with 17 additions and 352 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -801,7 +801,7 @@ public Collection<Object> createComponents(Client localClient, ClusterService cl
evaluator = new PrivilegesEvaluator(clusterService, threadPool, cr, resolver, auditLog,
settings, privilegesInterceptor, cih, irr, dlsFlsEnabled, namedXContentRegistry);

sf = new SecurityFilter(localClient, settings, evaluator, adminDns, dlsFlsValve, auditLog, threadPool, cs, compatConfig, irr, backendRegistry, namedXContentRegistry);
sf = new SecurityFilter(settings, evaluator, adminDns, dlsFlsValve, auditLog, threadPool, cs, compatConfig, irr);

final String principalExtractorClass = settings.get(SSLConfigConstants.SECURITY_SSL_TRANSPORT_PRINCIPAL_EXTRACTOR_CLASS, null);

Expand Down
255 changes: 0 additions & 255 deletions src/main/java/org/opensearch/security/auth/BackendRegistry.java

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -158,12 +158,6 @@ private boolean isAdminDN(LdapName dn) {
return isAdmin;
}

public boolean isTransportImpersonationAllowed(LdapName dn, String impersonated) {
if(dn == null) return false;

return isAdminDN(dn) || allowedDnsImpersonations.getOrDefault(dn, WildcardMatcher.NONE).test(impersonated);
}

public boolean isRestImpersonationAllowed(final String originalUser, final String impersonated) {
return (originalUser != null) ? allowedRestImpersonations.getOrDefault(originalUser, WildcardMatcher.NONE).test(impersonated) : false;
}
Expand Down
20 changes: 2 additions & 18 deletions src/main/java/org/opensearch/security/filter/SecurityFilter.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import org.opensearch.security.support.WildcardMatcher;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableSet;
import org.opensearch.security.auth.BackendRegistry;
import org.slf4j.LoggerFactory;
import org.slf4j.Logger;

Expand Down Expand Up @@ -72,13 +71,11 @@
import org.opensearch.action.support.ActionFilter;
import org.opensearch.action.support.ActionFilterChain;
import org.opensearch.action.update.UpdateRequest;
import org.opensearch.client.Client;
import org.opensearch.cluster.service.ClusterService;
import org.opensearch.common.logging.LoggerMessageFormat;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.concurrent.ThreadContext;
import org.opensearch.common.util.concurrent.ThreadContext.StoredContext;
import org.opensearch.common.xcontent.NamedXContentRegistry;
import org.opensearch.index.reindex.DeleteByQueryRequest;
import org.opensearch.index.reindex.UpdateByQueryRequest;
import org.opensearch.rest.RestStatus;
Expand Down Expand Up @@ -116,15 +113,10 @@ public class SecurityFilter implements ActionFilter {
private final IndexResolverReplacer indexResolverReplacer;
private final WildcardMatcher immutableIndicesMatcher;
private final RolesInjector rolesInjector;
private final Client client;
private final BackendRegistry backendRegistry;
private final NamedXContentRegistry namedXContentRegistry;

public SecurityFilter(final Client client, final Settings settings, final PrivilegesEvaluator evalp, final AdminDNs adminDns,
public SecurityFilter(final Settings settings, final PrivilegesEvaluator evalp, final AdminDNs adminDns,
DlsFlsRequestValve dlsFlsValve, AuditLog auditLog, ThreadPool threadPool, ClusterService cs,
final CompatConfig compatConfig, final IndexResolverReplacer indexResolverReplacer, BackendRegistry backendRegistry,
NamedXContentRegistry namedXContentRegistry) {
this.client = client;
final CompatConfig compatConfig, final IndexResolverReplacer indexResolverReplacer) {
this.evalp = evalp;
this.adminDns = adminDns;
this.dlsFlsValve = dlsFlsValve;
Expand All @@ -135,8 +127,6 @@ public SecurityFilter(final Client client, final Settings settings, final Privil
this.indexResolverReplacer = indexResolverReplacer;
this.immutableIndicesMatcher = WildcardMatcher.from(settings.getAsList(ConfigConstants.SECURITY_COMPLIANCE_IMMUTABLE_INDICES, Collections.emptyList()));
this.rolesInjector = new RolesInjector(auditLog);
this.backendRegistry = backendRegistry;
this.namedXContentRegistry = namedXContentRegistry;
log.info("{} indices are made immutable.", immutableIndicesMatcher);
}

Expand Down Expand Up @@ -176,12 +166,7 @@ private <Request extends ActionRequest, Response extends ActionResponse> void ap
attachSourceFieldContext(request);
}
final Set<String> injectedRoles = rolesInjector.injectUserAndRoles(request, action, task, threadContext);
boolean enforcePrivilegesEvaluation = false;
User user = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER);
if(user == null && (user = backendRegistry.authenticate(request, null, task, action)) != null) {
threadContext.putTransient(ConfigConstants.OPENDISTRO_SECURITY_USER, user);
enforcePrivilegesEvaluation = true;
}

final boolean userIsAdmin = isUserAdmin(user, adminDns);
final boolean interClusterRequest = HeaderHelper.isInterClusterRequest(threadContext);
Expand Down Expand Up @@ -264,7 +249,6 @@ private <Request extends ActionRequest, Response extends ActionResponse> void ap
if(Origin.LOCAL.toString().equals(threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_ORIGIN))
&& (interClusterRequest || HeaderHelper.isDirectRequest(threadContext))
&& (injectedRoles == null)
&& !enforcePrivilegesEvaluation
) {

chain.proceed(task, action, request, listener);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,6 @@ public abstract class DynamicConfigModel {
protected final Logger log = LoggerFactory.getLogger(this.getClass());
public abstract SortedSet<AuthDomain> getRestAuthDomains();
public abstract Set<AuthorizationBackend> getRestAuthorizers();
public abstract SortedSet<AuthDomain> getTransportAuthDomains();
public abstract Set<AuthorizationBackend> getTransportAuthorizers();
public abstract String getTransportUsernameAttribute();
public abstract boolean isAnonymousAuthenticationEnabled();
public abstract boolean isXffEnabled();
public abstract String getInternalProxies();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,8 @@ public static OpenSearchException createBadHeaderException() {
+ "is spoofing requests. Check your TLS certificate setup as described here: "
+ "See https://opendistro.github.io/for-elasticsearch-docs/docs/troubleshoot/tls/");
}

public static OpenSearchException createTransportClientNoLongerSupportedException() {
return new OpenSearchException("Transport client authentication no longer supported.");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ public SecurityInterceptor(final Settings settings,

public <T extends TransportRequest> SecurityRequestHandler<T> getHandler(String action,
TransportRequestHandler<T> actualHandler) {
return new SecurityRequestHandler<T>(action, actualHandler, threadPool, backendRegistry, auditLog,
return new SecurityRequestHandler<T>(action, actualHandler, threadPool, auditLog,
principalExtractor, requestEvalProvider, cs, SSLConfig, sslExceptionHandler);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,8 @@
import org.opensearch.common.transport.TransportAddress;
import org.opensearch.common.util.concurrent.ThreadContext;
import org.opensearch.search.internal.ShardSearchRequest;
import org.opensearch.security.action.whoami.WhoAmIAction;
import org.opensearch.security.ssl.transport.PrincipalExtractor;
import org.opensearch.security.ssl.util.ExceptionUtils;
import org.opensearch.security.ssl.util.SSLRequestHelper;
import org.opensearch.tasks.Task;
import org.opensearch.threadpool.ThreadPool;
import org.opensearch.transport.TransportChannel;
Expand All @@ -57,13 +55,11 @@

import org.opensearch.security.auditlog.AuditLog;
import org.opensearch.security.auditlog.AuditLog.Origin;
import org.opensearch.security.auth.BackendRegistry;
import org.opensearch.security.ssl.SslExceptionHandler;
import org.opensearch.security.support.Base64Helper;
import org.opensearch.security.support.ConfigConstants;
import org.opensearch.security.support.HeaderHelper;
import org.opensearch.security.user.User;
import org.opensearch.security.ssl.transport.SecuritySSLRequestHandler;
import org.opensearch.security.ssl.transport.SSLConfig;

import com.google.common.base.Strings;
Expand All @@ -72,28 +68,23 @@

public class SecurityRequestHandler<T extends TransportRequest> extends SecuritySSLRequestHandler<T> {

private final BackendRegistry backendRegistry;
private final AuditLog auditLog;
private final InterClusterRequestEvaluator requestEvalProvider;
private final ClusterService cs;
private final SSLConfig SSLConfig;

SecurityRequestHandler(String action,
final TransportRequestHandler<T> actualHandler,
final ThreadPool threadPool,
final BackendRegistry backendRegistry,
final AuditLog auditLog,
final PrincipalExtractor principalExtractor,
final InterClusterRequestEvaluator requestEvalProvider,
final ClusterService cs,
final SSLConfig SSLConfig,
final SslExceptionHandler sslExceptionHandler) {
super(action, actualHandler, threadPool, principalExtractor, SSLConfig, sslExceptionHandler);
this.backendRegistry = backendRegistry;
this.auditLog = auditLog;
this.requestEvalProvider = requestEvalProvider;
this.cs = cs;
this.SSLConfig = SSLConfig;
}

@Override
Expand Down Expand Up @@ -274,56 +265,12 @@ else if(!Strings.isNullOrEmpty(injectedUserHeader)) {
//this is a netty request from a non-server node (maybe also be internal: or a shard request)
//and therefore issued by a transport client

if(SSLRequestHelper.containsBadHeader(getThreadContext(), ConfigConstants.OPENDISTRO_SECURITY_CONFIG_PREFIX)) {
final OpenSearchException exception = ExceptionUtils.createBadHeaderException();
auditLog.logBadHeaders(request, task.getAction(), task);
log.error(exception.toString());
transportChannel.sendResponse(exception);
return;
}

//TODO OpenSearch Security exception handling, introduce authexception
User user;
//try {
if((user = backendRegistry.authenticate(request, principal, task, task.getAction())) == null) {
org.apache.logging.log4j.ThreadContext.remove("user");

if(task.getAction().equals(WhoAmIAction.NAME)) {
super.messageReceivedDecorate(request, handler, transportChannel, task);
return;
}

if(task.getAction().equals("cluster:monitor/nodes/liveness")
|| task.getAction().equals("internal:transport/handshake")) {
super.messageReceivedDecorate(request, handler, transportChannel, task);
return;
}
//since OS 2.0 we do not support this any longer because transport client no longer available


log.error("Cannot authenticate {} for {}", getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER), task.getAction());
transportChannel.sendResponse(new OpenSearchSecurityException("Cannot authenticate "+getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER)));
return;
} else {
// make it possible to filter logs by username
org.apache.logging.log4j.ThreadContext.put("user", user.getName());
}
//} catch (Exception e) {
// log.error("Error authentication transport user "+e, e);
//auditLog.logFailedLogin(principal, false, null, request);
//transportChannel.sendResponse(ExceptionsHelper.convertToOpenSearchException(e));
//return;
//}

getThreadContext().putTransient(ConfigConstants.OPENDISTRO_SECURITY_USER, user);
TransportAddress originalRemoteAddress = request.remoteAddress();

if(originalRemoteAddress != null && (originalRemoteAddress instanceof TransportAddress)) {
getThreadContext().putTransient(ConfigConstants.OPENDISTRO_SECURITY_REMOTE_ADDRESS, originalRemoteAddress);
} else {
log.error("Request has no proper remote address {}", originalRemoteAddress);
transportChannel.sendResponse(new OpenSearchException("Request has no proper remote address"));
return;
}
final OpenSearchException exception = ExceptionUtils.createTransportClientNoLongerSupportedException();
log.error(exception.toString());
transportChannel.sendResponse(exception);
return;
}

if (isActionTraceEnabled()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -945,7 +945,7 @@ public void testCcsWithDiffCertsWithNoNodesDnUpdate() throws Exception {
HttpResponse ccs = rh1.executeGetRequest(uri, encodeBasicHeader("twitter", "nagilum"));
System.out.println(ccs.getBody());
assertThat(ccs.getStatusCode(), equalTo(HttpStatus.SC_INTERNAL_SERVER_ERROR));
assertThat(ccs.getBody(), containsString("no OID or security.nodes_dn incorrect configured"));
assertThat(ccs.getBody(), containsString("Transport client authentication no longer supported"));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ public static Collection<Object[]> data() {
@Test
public void testImmutableIndicesWildcardMatcher() {
final SecurityFilter filter = new SecurityFilter(
mock(Client.class),
settings,
mock(PrivilegesEvaluator.class),
mock(AdminDNs.class),
Expand All @@ -88,9 +87,7 @@ public void testImmutableIndicesWildcardMatcher() {
mock(ThreadPool.class),
mock(ClusterService.class),
mock(CompatConfig.class),
mock(IndexResolverReplacer.class),
mock(BackendRegistry.class),
mock(NamedXContentRegistry.class)
mock(IndexResolverReplacer.class)
);
assertEquals(expected, filter.getImmutableIndicesMatcher());
}
Expand All @@ -104,7 +101,6 @@ public void testUnexepectedCausesAreNotSendToCallers() {
final ActionListener<ActionResponse> listener = mock(ActionListener.class);

final SecurityFilter filter = new SecurityFilter(
mock(Client.class),
settings,
mock(PrivilegesEvaluator.class),
mock(AdminDNs.class),
Expand All @@ -113,9 +109,7 @@ public void testUnexepectedCausesAreNotSendToCallers() {
new ThreadPool(Settings.builder().put("node.name", "mock").build()),
mock(ClusterService.class),
mock(CompatConfig.class),
mock(IndexResolverReplacer.class),
mock(BackendRegistry.class),
mock(NamedXContentRegistry.class)
mock(IndexResolverReplacer.class)
);

// Act
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ protected Settings.Builder minimumSecuritySettingsBuilder(int node, boolean sslO
builder.putList("plugins.security.authcz.admin_dn", "CN=kirk,OU=client,O=client,l=tEst, C=De");
builder.put(ConfigConstants.SECURITY_BACKGROUND_INIT_IF_SECURITYINDEX_NOT_EXIST, false);
}

builder.put("cluster.routing.allocation.disk.threshold_enabled", false);
builder.put(other);

return builder;
Expand Down

0 comments on commit 779c09f

Please sign in to comment.