diff --git a/src/main/java/com/amazon/dlic/auth/http/jwt/AbstractHTTPJwtAuthenticator.java b/src/main/java/com/amazon/dlic/auth/http/jwt/AbstractHTTPJwtAuthenticator.java index 3d795ed3b1..06dd89ef35 100644 --- a/src/main/java/com/amazon/dlic/auth/http/jwt/AbstractHTTPJwtAuthenticator.java +++ b/src/main/java/com/amazon/dlic/auth/http/jwt/AbstractHTTPJwtAuthenticator.java @@ -28,8 +28,6 @@ import org.apache.http.HttpHeaders; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import org.apache.logging.log4j.LogManager; import org.opensearch.OpenSearchSecurityException; import org.opensearch.SpecialPermission; import org.opensearch.common.Strings; diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 9591667f10..c22a950738 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -801,7 +801,7 @@ public Collection 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, xffResolver); final String principalExtractorClass = settings.get(SSLConfigConstants.SECURITY_SSL_TRANSPORT_PRINCIPAL_EXTRACTOR_CLASS, null); diff --git a/src/main/java/org/opensearch/security/auditlog/AuditLog.java b/src/main/java/org/opensearch/security/auditlog/AuditLog.java index 58740de6c0..a7cda20025 100644 --- a/src/main/java/org/opensearch/security/auditlog/AuditLog.java +++ b/src/main/java/org/opensearch/security/auditlog/AuditLog.java @@ -48,9 +48,7 @@ public interface AuditLog extends Closeable { //login - void logFailedLogin(String effectiveUser, boolean securityadmin, String initiatingUser, TransportRequest request, Task task); void logFailedLogin(String effectiveUser, boolean securityadmin, String initiatingUser, RestRequest request); - void logSucceededLogin(String effectiveUser, boolean securityadmin, String initiatingUser, TransportRequest request, String action, Task task); void logSucceededLogin(String effectiveUser, boolean securityadmin, String initiatingUser, RestRequest request); //privs diff --git a/src/main/java/org/opensearch/security/auditlog/NullAuditLog.java b/src/main/java/org/opensearch/security/auditlog/NullAuditLog.java index 20b1faa909..06761f5636 100644 --- a/src/main/java/org/opensearch/security/auditlog/NullAuditLog.java +++ b/src/main/java/org/opensearch/security/auditlog/NullAuditLog.java @@ -52,21 +52,11 @@ public void close() throws IOException { //noop, intentionally left empty } - @Override - public void logFailedLogin(String effectiveUser, boolean securityadmin, String initiatingUser, TransportRequest request, Task task) { - //noop, intentionally left empty - } - @Override public void logFailedLogin(String effectiveUser, boolean securityadmin, String initiatingUser, RestRequest request) { //noop, intentionally left empty } - @Override - public void logSucceededLogin(String effectiveUser, boolean securityadmin, String initiatingUser, TransportRequest request, String action, Task task) { - //noop, intentionally left empty - } - @Override public void logSucceededLogin(String effectiveUser, boolean securityadmin, String initiatingUser, RestRequest request) { //noop, intentionally left empty diff --git a/src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java b/src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java index 04dd285b5c..5613508a8a 100644 --- a/src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java +++ b/src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java @@ -133,23 +133,6 @@ public ComplianceConfig getComplianceConfig() { return this.complianceConfig; } - @Override - public void logFailedLogin(String effectiveUser, boolean securityadmin, String initiatingUser, TransportRequest request, Task task) { - final String action = null; - - if(!checkTransportFilter(AuditCategory.FAILED_LOGIN, action, effectiveUser, request)) { - return; - } - - final TransportAddress remoteAddress = getRemoteAddress(); - final List msgs = RequestResolver.resolve(AuditCategory.FAILED_LOGIN, getOrigin(), action, null, effectiveUser, securityadmin, initiatingUser, remoteAddress, request, getThreadContextHeaders(), task, resolver, clusterService, settings, auditConfigFilter.shouldLogRequestBody(), auditConfigFilter.shouldResolveIndices(), auditConfigFilter.shouldResolveBulkRequests(), securityIndex, auditConfigFilter.shouldExcludeSensitiveHeaders(), null); - - for(AuditMessage msg: msgs) { - save(msg); - } - } - - @Override public void logFailedLogin(String effectiveUser, boolean securityadmin, String initiatingUser, RestRequest request) { @@ -168,21 +151,6 @@ public void logFailedLogin(String effectiveUser, boolean securityadmin, String i save(msg); } - @Override - public void logSucceededLogin(String effectiveUser, boolean securityadmin, String initiatingUser, TransportRequest request, String action, Task task) { - - if(!checkTransportFilter(AuditCategory.AUTHENTICATED, action, effectiveUser, request)) { - return; - } - - final TransportAddress remoteAddress = getRemoteAddress(); - final List msgs = RequestResolver.resolve(AuditCategory.AUTHENTICATED, getOrigin(), action, null, effectiveUser, securityadmin, initiatingUser,remoteAddress, request, getThreadContextHeaders(), task, resolver, clusterService, settings, auditConfigFilter.shouldLogRequestBody(), auditConfigFilter.shouldResolveIndices(), auditConfigFilter.shouldResolveBulkRequests(), securityIndex, auditConfigFilter.shouldExcludeSensitiveHeaders(), null); - - for(AuditMessage msg: msgs) { - save(msg); - } - } - @Override public void logSucceededLogin(String effectiveUser, boolean securityadmin, String initiatingUser, RestRequest request) { diff --git a/src/main/java/org/opensearch/security/auditlog/impl/AuditLogImpl.java b/src/main/java/org/opensearch/security/auditlog/impl/AuditLogImpl.java index 1bb802f291..516ae7a980 100644 --- a/src/main/java/org/opensearch/security/auditlog/impl/AuditLogImpl.java +++ b/src/main/java/org/opensearch/security/auditlog/impl/AuditLogImpl.java @@ -128,13 +128,6 @@ protected void save(final AuditMessage msg) { } } - @Override - public void logFailedLogin(String effectiveUser, boolean securityAdmin, String initiatingUser, TransportRequest request, Task task) { - if (enabled) { - super.logFailedLogin(effectiveUser, securityAdmin, initiatingUser, request, task); - } - } - @Override public void logFailedLogin(String effectiveUser, boolean securityAdmin, String initiatingUser, RestRequest request) { if (enabled) { @@ -142,13 +135,6 @@ public void logFailedLogin(String effectiveUser, boolean securityAdmin, String i } } - @Override - public void logSucceededLogin(String effectiveUser, boolean securityAdmin, String initiatingUser, TransportRequest request, String action, Task task) { - if (enabled) { - super.logSucceededLogin(effectiveUser, securityAdmin, initiatingUser, request, action, task); - } - } - @Override public void logSucceededLogin(String effectiveUser, boolean securityAdmin, String initiatingUser, RestRequest request) { if (enabled) { diff --git a/src/main/java/org/opensearch/security/auth/BackendRegistry.java b/src/main/java/org/opensearch/security/auth/BackendRegistry.java index d1454e353e..2d434d560a 100644 --- a/src/main/java/org/opensearch/security/auth/BackendRegistry.java +++ b/src/main/java/org/opensearch/security/auth/BackendRegistry.java @@ -42,10 +42,6 @@ import java.util.concurrent.Callable; import java.util.concurrent.TimeUnit; -import javax.naming.InvalidNameException; -import javax.naming.ldap.LdapName; -import javax.naming.ldap.Rdn; - import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LogManager; import org.opensearch.OpenSearchSecurityException; @@ -64,12 +60,9 @@ import org.opensearch.security.securityconf.DynamicConfigModel; import org.opensearch.security.ssl.util.Utils; import org.opensearch.security.support.ConfigConstants; -import org.opensearch.security.support.HTTPHelper; import org.opensearch.security.user.AuthCredentials; import org.opensearch.security.user.User; -import org.opensearch.tasks.Task; import org.opensearch.threadpool.ThreadPool; -import org.opensearch.transport.TransportRequest; import org.greenrobot.eventbus.Subscribe; import com.google.common.base.Strings; @@ -84,8 +77,6 @@ public class BackendRegistry { protected final Logger log = LogManager.getLogger(this.getClass()); private SortedSet restAuthDomains; private Set restAuthorizers; - private SortedSet transportAuthDomains; - private Set transportAuthorizers; private List ipAuthFailureListeners; private Multimap authBackendFailureListeners; @@ -105,15 +96,8 @@ public class BackendRegistry { private final int ttlInMin; private Cache userCache; //rest standard private Cache restImpersonationCache; //used for rest impersonation - private Cache userCacheTransport; //transport no creds, possibly impersonated - private Cache authenticatedUserCacheTransport; //transport creds, no impersonation - - private Cache> transportRoleCache; // private Cache> restRoleCache; // - private Cache transportImpersonationCache; //used for transport impersonation - private volatile String transportUsernameAttribute = null; - private void createCaches() { userCache = CacheBuilder.newBuilder().expireAfterWrite(ttlInMin, TimeUnit.MINUTES) .removalListener(new RemovalListener() { @@ -123,22 +107,6 @@ public void onRemoval(RemovalNotification notification) { } }).build(); - userCacheTransport = CacheBuilder.newBuilder().expireAfterWrite(ttlInMin, TimeUnit.MINUTES) - .removalListener(new RemovalListener() { - @Override - public void onRemoval(RemovalNotification notification) { - log.debug("Clear user cache for {} due to {}", notification.getKey(), notification.getCause()); - } - }).build(); - - authenticatedUserCacheTransport = CacheBuilder.newBuilder().expireAfterWrite(ttlInMin, TimeUnit.MINUTES) - .removalListener(new RemovalListener() { - @Override - public void onRemoval(RemovalNotification notification) { - log.debug("Clear user cache for {} due to {}", notification.getKey().getUsername(), notification.getCause()); - } - }).build(); - restImpersonationCache = CacheBuilder.newBuilder().expireAfterWrite(ttlInMin, TimeUnit.MINUTES) .removalListener(new RemovalListener() { @Override @@ -147,14 +115,6 @@ public void onRemoval(RemovalNotification notification) { } }).build(); - transportRoleCache = CacheBuilder.newBuilder().expireAfterWrite(ttlInMin, TimeUnit.MINUTES) - .removalListener(new RemovalListener>() { - @Override - public void onRemoval(RemovalNotification> notification) { - log.debug("Clear user cache for {} due to {}", notification.getKey(), notification.getCause()); - } - }).build(); - restRoleCache = CacheBuilder.newBuilder().expireAfterWrite(ttlInMin, TimeUnit.MINUTES) .removalListener(new RemovalListener>() { @Override @@ -163,14 +123,6 @@ public void onRemoval(RemovalNotification> notification) { } }).build(); - transportImpersonationCache = CacheBuilder.newBuilder().expireAfterWrite(ttlInMin, TimeUnit.MINUTES) - .removalListener(new RemovalListener() { - @Override - public void onRemoval(RemovalNotification notification) { - log.debug("Clear user cache for {} due to {}", notification.getKey(), notification.getCause()); - } - }).build(); - } public BackendRegistry(final Settings settings, final AdminDNs adminDns, @@ -197,26 +149,19 @@ public boolean isInitialized() { public void invalidateCache() { userCache.invalidateAll(); - userCacheTransport.invalidateAll(); - authenticatedUserCacheTransport.invalidateAll(); restImpersonationCache.invalidateAll(); restRoleCache.invalidateAll(); - transportRoleCache.invalidateAll(); - transportImpersonationCache.invalidateAll(); } @Subscribe public void onDynamicConfigModelChanged(DynamicConfigModel dcm) { invalidateCache(); - transportUsernameAttribute = dcm.getTransportUsernameAttribute();// config.dynamic.transport_userrname_attribute; anonymousAuthEnabled = dcm.isAnonymousAuthenticationEnabled()//config.dynamic.http.anonymous_auth_enabled && !opensearchSettings.getAsBoolean(ConfigConstants.SECURITY_COMPLIANCE_DISABLE_ANONYMOUS_AUTHENTICATION, false); restAuthDomains = Collections.unmodifiableSortedSet(dcm.getRestAuthDomains()); - transportAuthDomains = Collections.unmodifiableSortedSet(dcm.getTransportAuthDomains()); restAuthorizers = Collections.unmodifiableSet(dcm.getRestAuthorizers()); - transportAuthorizers = Collections.unmodifiableSet(dcm.getTransportAuthorizers()); ipAuthFailureListeners = dcm.getIpAuthFailureListeners(); authBackendFailureListeners = dcm.getAuthBackendFailureListeners(); @@ -227,123 +172,6 @@ public void onDynamicConfigModelChanged(DynamicConfigModel dcm) { initialized = !restAuthDomains.isEmpty() || anonymousAuthEnabled || injectedUserEnabled; } - public User authenticate(final TransportRequest request, final String sslPrincipal, final Task task, final String action) { - final boolean isDebugEnabled = log.isDebugEnabled(); - if(isDebugEnabled && request.remoteAddress() != null) { - log.debug("Transport authentication request from {}", request.remoteAddress()); - } - - if (request.remoteAddress() != null && isBlocked(request.remoteAddress().address().getAddress())) { - if (isDebugEnabled) { - log.debug("Rejecting transport request because of blocked address: {}", request.remoteAddress()); - } - return null; - } - - User injectedUser = userInjector.getInjectedUser(); - - if(injectedUser != null) { - auditLog.logSucceededLogin(injectedUser.getName(), true, null, request, action, task); - return injectedUser; - } - - if(sslPrincipal == null) { - return null; - } - - User origPKIUser = new User(sslPrincipal); - - if(adminDns.isAdmin(origPKIUser)) { - auditLog.logSucceededLogin(origPKIUser.getName(), true, null, request, action, task); - return origPKIUser; - } - - if (!isInitialized()) { - log.error("Not yet initialized (you may need to run securityadmin)"); - return null; - } - - final String authorizationHeader = threadPool.getThreadContext().getHeader("Authorization"); - //Use either impersonation OR credentials authentication - //if both is supplied credentials authentication win - final AuthCredentials creds = HTTPHelper.extractCredentials(authorizationHeader, log); - - User impersonatedTransportUser = null; - - if(creds != null) { - if (isDebugEnabled) { - log.debug("User {} submitted also basic credentials: {}", origPKIUser.getName(), creds); - } - } - - //loop over all transport auth domains - for (final AuthDomain authDomain: transportAuthDomains) { - - if (isDebugEnabled) { - log.debug("Check transport authdomain {}/{} or {} in total", authDomain.getBackend().getType(), authDomain.getOrder(), transportAuthDomains.size()); - } - - User authenticatedUser = null; - - if(creds == null) { - //no credentials submitted - //impersonation possible - impersonatedTransportUser = impersonate(request, origPKIUser); - origPKIUser = resolveTransportUsernameAttribute(origPKIUser); - authenticatedUser = checkExistsAndAuthz(userCacheTransport, - impersonatedTransportUser == null ? origPKIUser : impersonatedTransportUser, authDomain.getBackend(), transportAuthorizers); - } else { - //auth credentials submitted - //impersonation not possible, if requested it will be ignored - authenticatedUser = authcz(authenticatedUserCacheTransport, transportRoleCache, creds, authDomain.getBackend(), transportAuthorizers); - } - - if (authenticatedUser == null) { - for (AuthFailureListener authFailureListener : authBackendFailureListeners.get(authDomain.getBackend().getClass().getName())) { - authFailureListener.onAuthFailure(request.remoteAddress() != null ? request.remoteAddress().address().getAddress() : null, creds, - request); - } - - if (isDebugEnabled) { - log.debug("Cannot authenticate transport user {} (or add roles) with authdomain {}/{} of {}, try next", creds==null?(impersonatedTransportUser==null?origPKIUser.getName():impersonatedTransportUser.getName()):creds.getUsername(), authDomain.getBackend().getType(), authDomain.getOrder(), transportAuthDomains.size()); - } - continue; - } - - if(adminDns.isAdmin(authenticatedUser)) { - log.error("Cannot authenticate transport user because admin user is not permitted to login"); - auditLog.logFailedLogin(authenticatedUser.getName(), true, null, request, task); - return null; - } - - if (isDebugEnabled) { - log.debug("Transport user '{}' is authenticated", authenticatedUser); - } - - auditLog.logSucceededLogin(authenticatedUser.getName(), false, impersonatedTransportUser == null ? null : origPKIUser.getName(), request, - action, task); - - return authenticatedUser; - }//end looping auth domains - - - //auditlog - if(creds == null) { - auditLog.logFailedLogin(impersonatedTransportUser == null ? origPKIUser.getName() : impersonatedTransportUser.getName(), false, - impersonatedTransportUser == null ? null : origPKIUser.getName(), request, task); - } else { - auditLog.logFailedLogin(creds.getUsername(), false, null, request, task); - } - - log.warn("Transport authentication finally failed for {} from {}", - creds == null ? impersonatedTransportUser == null ? origPKIUser.getName() : impersonatedTransportUser.getName() : creds.getUsername(), - request.remoteAddress()); - - notifyIpAuthFailureListeners(request.remoteAddress() != null ? request.remoteAddress().address().getAddress() : null, creds, request); - - return null; - } - /** * * @param request @@ -567,9 +395,6 @@ private void notifyIpAuthFailureListeners(InetAddress remoteAddress, AuthCredent /** * no auditlog, throw no exception, does also authz for all authorizers * - * @param cache - * @param ac - * @param authDomain * @return null if user cannot b authenticated */ private User checkExistsAndAuthz(final Cache cache, final User user, final AuthenticationBackend authenticationBackend, @@ -646,9 +471,6 @@ private void authz(User authenticatedUser, Cache> roleCache, f /** * no auditlog, throw no exception, does also authz for all authorizers * - * @param cache - * @param ac - * @param authDomain * @return null if user cannot b authenticated */ private User authcz(final Cache cache, Cache> roleCache, final AuthCredentials ac, @@ -686,65 +508,6 @@ public User call() throws Exception { } } - private User impersonate(final TransportRequest tr, final User origPKIuser) throws OpenSearchSecurityException { - - final String impersonatedUser = threadPool.getThreadContext().getHeader("opendistro_security_impersonate_as"); - - if(Strings.isNullOrEmpty(impersonatedUser)) { - return null; //nothing to do - } - - if (!isInitialized()) { - throw new OpenSearchSecurityException("Could not check for impersonation because OpenSearch Security is not yet initialized"); - } - - if (origPKIuser == null) { - throw new OpenSearchSecurityException("no original PKI user found"); - } - - User aU = origPKIuser; - - if (adminDns.isAdminDN(impersonatedUser)) { - throw new OpenSearchSecurityException( - "'" + origPKIuser.getName() + "' is not allowed to impersonate as an adminuser '" + impersonatedUser + "'"); - } - - try { - if (impersonatedUser != null && !adminDns.isTransportImpersonationAllowed(new LdapName(origPKIuser.getName()), impersonatedUser)) { - throw new OpenSearchSecurityException( - "'"+origPKIuser.getName() + "' is not allowed to impersonate as '" + impersonatedUser+"'"); - } else if (impersonatedUser != null) { - final boolean isDebugEnabled = log.isDebugEnabled(); - //loop over all transport auth domains - for (final AuthDomain authDomain : transportAuthDomains) { - final AuthenticationBackend authenticationBackend = authDomain.getBackend(); - final User impersonatedUserObject = checkExistsAndAuthz(transportImpersonationCache, new User(impersonatedUser), - authenticationBackend, transportAuthorizers); - - if (impersonatedUserObject == null) { - log.debug( - "Unable to impersonate transport user from '{}' to '{}' because the impersonated user does not exists in {}, try next ...", - origPKIuser.getName(), impersonatedUser, authenticationBackend.getType()); - continue; - } - - if (isDebugEnabled) { - log.debug("Impersonate transport user from '{}' to '{}'", origPKIuser.getName(), impersonatedUser); - } - return impersonatedUserObject; - } - - log.debug("Unable to impersonate transport user from '{}' to '{}' because the impersonated user does not exists", - origPKIuser.getName(), impersonatedUser); - throw new OpenSearchSecurityException("No such transport user: " + impersonatedUser, RestStatus.FORBIDDEN); - } - } catch (final InvalidNameException e1) { - throw new OpenSearchSecurityException("PKI does not have a valid name ('" + origPKIuser.getName() + "'), should never happen", e1); - } - - return aU; - } - private User impersonate(final RestRequest request, final User originalUser) throws OpenSearchSecurityException { final String impersonatedUserHeader = request.header("opendistro_security_impersonate_as"); @@ -794,24 +557,6 @@ private User impersonate(final RestRequest request, final User originalUser) thr } - private User resolveTransportUsernameAttribute(User pkiUser) { - //#547 - if(transportUsernameAttribute != null && !transportUsernameAttribute.isEmpty()) { - try { - final LdapName sslPrincipalAsLdapName = new LdapName(pkiUser.getName()); - for(final Rdn rdn: sslPrincipalAsLdapName.getRdns()) { - if(rdn.getType().equals(transportUsernameAttribute)) { - return new User((String) rdn.getValue()); - } - } - } catch (InvalidNameException e) { - //cannot happen - } - } - - return pkiUser; - } - private boolean isBlocked(InetAddress address) { if (this.ipClientBlockRegistries == null || this.ipClientBlockRegistries.isEmpty()) { return false; diff --git a/src/main/java/org/opensearch/security/auth/RolesInjector.java b/src/main/java/org/opensearch/security/auth/RolesInjector.java index 4e867d0a76..9a11543657 100644 --- a/src/main/java/org/opensearch/security/auth/RolesInjector.java +++ b/src/main/java/org/opensearch/security/auth/RolesInjector.java @@ -82,6 +82,5 @@ private void addUser(final User user, final TransportRequest transportRequest, return; threadContext.putTransient(ConfigConstants.OPENDISTRO_SECURITY_USER, user); - auditLog.logSucceededLogin(user.getName(), false, null, transportRequest, action, task); } } diff --git a/src/main/java/org/opensearch/security/auth/UserInjector.java b/src/main/java/org/opensearch/security/auth/UserInjector.java index 1709f14ab7..d85ab610ea 100644 --- a/src/main/java/org/opensearch/security/auth/UserInjector.java +++ b/src/main/java/org/opensearch/security/auth/UserInjector.java @@ -59,7 +59,7 @@ public class UserInjector { private final XFFResolver xffResolver; private final Boolean injectUserEnabled; - UserInjector(Settings settings, ThreadPool threadPool, AuditLog auditLog, XFFResolver xffResolver) { + public UserInjector(Settings settings, ThreadPool threadPool, AuditLog auditLog, XFFResolver xffResolver) { this.threadPool = threadPool; this.auditLog = auditLog; this.xffResolver = xffResolver; @@ -102,7 +102,7 @@ public void setTransportAddress(String addr) throws UnknownHostException, Illega } } - InjectedUser getInjectedUser() { + public InjectedUser getInjectedUser() { if (!injectUserEnabled) { return null; } diff --git a/src/main/java/org/opensearch/security/configuration/AdminDNs.java b/src/main/java/org/opensearch/security/configuration/AdminDNs.java index 55e2fb1933..f5fd217ed3 100644 --- a/src/main/java/org/opensearch/security/configuration/AdminDNs.java +++ b/src/main/java/org/opensearch/security/configuration/AdminDNs.java @@ -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; } diff --git a/src/main/java/org/opensearch/security/filter/SecurityFilter.java b/src/main/java/org/opensearch/security/filter/SecurityFilter.java index 0bb424a2a7..7fee09b379 100644 --- a/src/main/java/org/opensearch/security/filter/SecurityFilter.java +++ b/src/main/java/org/opensearch/security/filter/SecurityFilter.java @@ -83,12 +83,14 @@ import org.opensearch.index.reindex.UpdateByQueryRequest; import org.opensearch.rest.RestStatus; import org.opensearch.security.action.whoami.WhoAmIAction; +import org.opensearch.security.auth.UserInjector; import org.opensearch.security.auditlog.AuditLog; import org.opensearch.security.auditlog.AuditLog.Origin; import org.opensearch.security.compliance.ComplianceConfig; import org.opensearch.security.configuration.AdminDNs; import org.opensearch.security.configuration.CompatConfig; import org.opensearch.security.configuration.DlsFlsRequestValve; +import org.opensearch.security.http.XFFResolver; import org.opensearch.security.privileges.PrivilegesEvaluator; import org.opensearch.security.privileges.PrivilegesEvaluatorResponse; import org.opensearch.tasks.Task; @@ -114,17 +116,14 @@ public class SecurityFilter implements ActionFilter { private final ClusterService cs; private final CompatConfig compatConfig; private final IndexResolverReplacer indexResolverReplacer; + private final XFFResolver xffResolver; private final WildcardMatcher immutableIndicesMatcher; private final RolesInjector rolesInjector; - private final Client client; - private final BackendRegistry backendRegistry; - private final NamedXContentRegistry namedXContentRegistry; + private final UserInjector userInjector; - 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, final XFFResolver xffResolver) { this.evalp = evalp; this.adminDns = adminDns; this.dlsFlsValve = dlsFlsValve; @@ -133,10 +132,10 @@ public SecurityFilter(final Client client, final Settings settings, final Privil this.cs = cs; this.compatConfig = compatConfig; this.indexResolverReplacer = indexResolverReplacer; + this.xffResolver = xffResolver; this.immutableIndicesMatcher = WildcardMatcher.from(settings.getAsList(ConfigConstants.SECURITY_COMPLIANCE_IMMUTABLE_INDICES, Collections.emptyList())); this.rolesInjector = new RolesInjector(auditLog); - this.backendRegistry = backendRegistry; - this.namedXContentRegistry = namedXContentRegistry; + this.userInjector = new UserInjector(settings, threadPool, auditLog, xffResolver); log.info("{} indices are made immutable.", immutableIndicesMatcher); } @@ -178,11 +177,14 @@ private void ap final Set 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) { + if(user == null && (user = userInjector.getInjectedUser()) != null) { threadContext.putTransient(ConfigConstants.OPENDISTRO_SECURITY_USER, user); + // since there is no support for TransportClient auth/auth in 2.0 anymore, usually we + // can skip any checks on transport in case of trusted requests. + // However, if another plugin injected a user in the ThreadContext, we still need + // to perform privileges checks. enforcePrivilegesEvaluation = true; } - final boolean userIsAdmin = isUserAdmin(user, adminDns); final boolean interClusterRequest = HeaderHelper.isInterClusterRequest(threadContext); final boolean trustedClusterRequest = HeaderHelper.isTrustedClusterRequest(threadContext); diff --git a/src/main/java/org/opensearch/security/securityconf/DynamicConfigModel.java b/src/main/java/org/opensearch/security/securityconf/DynamicConfigModel.java index d6357f5f91..64e0e260c6 100644 --- a/src/main/java/org/opensearch/security/securityconf/DynamicConfigModel.java +++ b/src/main/java/org/opensearch/security/securityconf/DynamicConfigModel.java @@ -61,9 +61,6 @@ public abstract class DynamicConfigModel { protected final Logger log = LogManager.getLogger(this.getClass()); public abstract SortedSet getRestAuthDomains(); public abstract Set getRestAuthorizers(); - public abstract SortedSet getTransportAuthDomains(); - public abstract Set getTransportAuthorizers(); - public abstract String getTransportUsernameAttribute(); public abstract boolean isAnonymousAuthenticationEnabled(); public abstract boolean isXffEnabled(); public abstract String getInternalProxies(); diff --git a/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV6.java b/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV6.java index 2b72aa5212..ec4286f3e9 100644 --- a/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV6.java +++ b/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV6.java @@ -99,18 +99,6 @@ public Set getRestAuthorizers() { return Collections.unmodifiableSet(restAuthorizers); } @Override - public SortedSet getTransportAuthDomains() { - return Collections.unmodifiableSortedSet(transportAuthDomains); - } - @Override - public Set getTransportAuthorizers() { - return Collections.unmodifiableSet(transportAuthorizers); - } - @Override - public String getTransportUsernameAttribute() { - return config.dynamic.transport_userrname_attribute; - } - @Override public boolean isAnonymousAuthenticationEnabled() { return config.dynamic.http.anonymous_auth_enabled; } diff --git a/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java b/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java index aa4950398a..66fcf9c4ec 100644 --- a/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java +++ b/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java @@ -99,18 +99,6 @@ public Set getRestAuthorizers() { return Collections.unmodifiableSet(restAuthorizers); } @Override - public SortedSet getTransportAuthDomains() { - return Collections.unmodifiableSortedSet(transportAuthDomains); - } - @Override - public Set getTransportAuthorizers() { - return Collections.unmodifiableSet(transportAuthorizers); - } - @Override - public String getTransportUsernameAttribute() { - return config.dynamic.transport_userrname_attribute; - } - @Override public boolean isAnonymousAuthenticationEnabled() { return config.dynamic.http.anonymous_auth_enabled; } diff --git a/src/main/java/org/opensearch/security/ssl/util/ExceptionUtils.java b/src/main/java/org/opensearch/security/ssl/util/ExceptionUtils.java index fd014dd7c0..f06c4d9808 100644 --- a/src/main/java/org/opensearch/security/ssl/util/ExceptionUtils.java +++ b/src/main/java/org/opensearch/security/ssl/util/ExceptionUtils.java @@ -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."); + } } diff --git a/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java b/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java index 588e7cc870..e9ac5aea54 100644 --- a/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java +++ b/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java @@ -113,7 +113,7 @@ public SecurityInterceptor(final Settings settings, public SecurityRequestHandler getHandler(String action, TransportRequestHandler actualHandler) { - return new SecurityRequestHandler(action, actualHandler, threadPool, backendRegistry, auditLog, + return new SecurityRequestHandler(action, actualHandler, threadPool, auditLog, principalExtractor, requestEvalProvider, cs, SSLConfig, sslExceptionHandler); } diff --git a/src/main/java/org/opensearch/security/transport/SecurityRequestHandler.java b/src/main/java/org/opensearch/security/transport/SecurityRequestHandler.java index 114f6002d3..2f7888f862 100644 --- a/src/main/java/org/opensearch/security/transport/SecurityRequestHandler.java +++ b/src/main/java/org/opensearch/security/transport/SecurityRequestHandler.java @@ -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; @@ -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; @@ -72,16 +68,13 @@ public class SecurityRequestHandler extends SecuritySSLRequestHandler { - 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 actualHandler, final ThreadPool threadPool, - final BackendRegistry backendRegistry, final AuditLog auditLog, final PrincipalExtractor principalExtractor, final InterClusterRequestEvaluator requestEvalProvider, @@ -89,11 +82,9 @@ public class SecurityRequestHandler extends Security 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 @@ -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()) { diff --git a/src/test/java/org/opensearch/security/auditlog/impl/DisabledCategoriesTest.java b/src/test/java/org/opensearch/security/auditlog/impl/DisabledCategoriesTest.java index 454a6a43c2..07f75501fb 100644 --- a/src/test/java/org/opensearch/security/auditlog/impl/DisabledCategoriesTest.java +++ b/src/test/java/org/opensearch/security/auditlog/impl/DisabledCategoriesTest.java @@ -116,10 +116,8 @@ public void enableAllCategoryTest() throws Exception { Assert.assertTrue(AuditCategory.values()+"#"+result, categoriesPresentInLog(result, filterComplianceCategories(AuditCategory.values()))); - Assert.assertThat(result, containsString("testuser.transport.succeededlogin")); Assert.assertThat(result, containsString("testuser.rest.succeededlogin")); Assert.assertThat(result, containsString("testuser.rest.failedlogin")); - Assert.assertThat(result, containsString("testuser.transport.failedlogin")); Assert.assertThat(result, containsString("privilege.missing")); Assert.assertThat(result, containsString("action.indexattempt")); Assert.assertThat(result, containsString("action.transport.ssl")); @@ -195,7 +193,6 @@ protected boolean categoriesPresentInLog(String result, AuditCategory... categor } protected void logAll(AuditLog auditLog) { - //11 requests logRestFailedLogin(auditLog); logRestBadHeaders(auditLog); logRestSSLException(auditLog); @@ -207,8 +204,6 @@ protected void logAll(AuditLog auditLog) { logTransportSSLException(auditLog); logTransportBadHeaders(auditLog); - logTransportFailedLogin(auditLog); - logTransportSucceededLogin(auditLog); logIndexEvent(auditLog); } @@ -217,19 +212,10 @@ protected void logRestSucceededLogin(AuditLog auditLog) { auditLog.logSucceededLogin("testuser.rest.succeededlogin", false, "testuser.rest.succeededlogin", new MockRestRequest()); } - protected void logTransportSucceededLogin(AuditLog auditLog) { - auditLog.logSucceededLogin("testuser.transport.succeededlogin", false, "testuser.transport.succeededlogin", new TransportRequest.Empty(), "test/action", new Task(0, "x", "ac", "", null, null)); - } - - protected void logRestFailedLogin(AuditLog auditLog) { auditLog.logFailedLogin("testuser.rest.failedlogin", false, "testuser.rest.failedlogin", new MockRestRequest()); } - protected void logTransportFailedLogin(AuditLog auditLog) { - auditLog.logFailedLogin("testuser.transport.failedlogin", false, "testuser.transport.failedlogin", new TransportRequest.Empty(), null); - } - protected void logMissingPrivileges(AuditLog auditLog) { auditLog.logMissingPrivileges("privilege.missing", new TransportRequest.Empty(), null); } diff --git a/src/test/java/org/opensearch/security/ccstest/CrossClusterSearchTests.java b/src/test/java/org/opensearch/security/ccstest/CrossClusterSearchTests.java index fb723094cc..a665f79923 100644 --- a/src/test/java/org/opensearch/security/ccstest/CrossClusterSearchTests.java +++ b/src/test/java/org/opensearch/security/ccstest/CrossClusterSearchTests.java @@ -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 diff --git a/src/test/java/org/opensearch/security/filter/SecurityFilterTest.java b/src/test/java/org/opensearch/security/filter/SecurityFilterTest.java index 3454a6d2e9..3d002cde53 100644 --- a/src/test/java/org/opensearch/security/filter/SecurityFilterTest.java +++ b/src/test/java/org/opensearch/security/filter/SecurityFilterTest.java @@ -17,10 +17,12 @@ import org.opensearch.OpenSearchSecurityException; import org.opensearch.security.auditlog.AuditLog; +import org.opensearch.security.auth.UserInjector; import org.opensearch.security.auth.BackendRegistry; import org.opensearch.security.configuration.AdminDNs; import org.opensearch.security.configuration.CompatConfig; import org.opensearch.security.configuration.DlsFlsRequestValve; +import org.opensearch.security.http.XFFResolver; import org.opensearch.security.privileges.PrivilegesEvaluator; import org.opensearch.security.resolver.IndexResolverReplacer; import org.opensearch.security.support.ConfigConstants; @@ -79,7 +81,6 @@ public static Collection data() { @Test public void testImmutableIndicesWildcardMatcher() { final SecurityFilter filter = new SecurityFilter( - mock(Client.class), settings, mock(PrivilegesEvaluator.class), mock(AdminDNs.class), @@ -89,8 +90,7 @@ public void testImmutableIndicesWildcardMatcher() { mock(ClusterService.class), mock(CompatConfig.class), mock(IndexResolverReplacer.class), - mock(BackendRegistry.class), - mock(NamedXContentRegistry.class) + mock(XFFResolver.class) ); assertEquals(expected, filter.getImmutableIndicesMatcher()); } @@ -104,7 +104,6 @@ public void testUnexepectedCausesAreNotSendToCallers() { final ActionListener listener = mock(ActionListener.class); final SecurityFilter filter = new SecurityFilter( - mock(Client.class), settings, mock(PrivilegesEvaluator.class), mock(AdminDNs.class), @@ -114,8 +113,7 @@ public void testUnexepectedCausesAreNotSendToCallers() { mock(ClusterService.class), mock(CompatConfig.class), mock(IndexResolverReplacer.class), - mock(BackendRegistry.class), - mock(NamedXContentRegistry.class) + mock(XFFResolver.class) ); // Act diff --git a/src/test/java/org/opensearch/security/test/AbstractSecurityUnitTest.java b/src/test/java/org/opensearch/security/test/AbstractSecurityUnitTest.java index e3e0316e5d..6dcd5dc908 100644 --- a/src/test/java/org/opensearch/security/test/AbstractSecurityUnitTest.java +++ b/src/test/java/org/opensearch/security/test/AbstractSecurityUnitTest.java @@ -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;