Skip to content

Commit

Permalink
[2.x] Backport SAML permissions changes in DynamicConfigModelV7 (#3853)
Browse files Browse the repository at this point in the history 
### Description

Resolves an issue with permissions in the SAML auth flow after merge of
#3671

This backports code from
#2987 that wraps the
call to `instantiateAAA` in `AccessController.doPrivileged`. Expand the
section below to see the stack trace with permissions error.

<details>
<summary>Permission issue stack trace</summary>
[2023-12-15T23:45:14,828][WARN ][o.o.s.s.ReflectionHelper ]
[6c7e67b914eb] Unable to enable
'com.amazon.dlic.auth.http.saml.HTTPSamlAuthenticator' due to
java.lang.reflect.InvocationTargetException
[2023-12-15T23:45:14,832][ERROR][o.o.s.s.DynamicConfigModelV7]
[6c7e67b914eb] Unable to initialize auth domain
saml_auth_domain=AuthcDomain [http_enabled=true,
transport_enabled=false, order=5, http_authenticator=HttpAuthenticator
[challenge=true, type=saml,
config={idp={metadata_url=http://localhost:7000/metadata,
entity_id=urn:example:idp}, sp={entity_id=https://localhost:9200/},
kibana_url=http://localhost:5601/,
exchange_key=6aff3042-1327-4f3d-82f0-40a157ac4464}],
authentication_backend=AuthcBackend [type=noop, config={}],
description=null] due to
OpenSearchException[java.lang.reflect.InvocationTargetException];
nested: InvocationTargetException; nested:
RuntimeException[java.security.AccessControlException: access denied
("java.util.PropertyPermission" "*" "read,write")]; nested:
AccessControlException[access denied ("java.util.PropertyPermission" "*"
"read,write")];
org.opensearch.OpenSearchException:
java.lang.reflect.InvocationTargetException
at
org.opensearch.security.support.ReflectionHelper.instantiateAAA(ReflectionHelper.java:73)
~[opensearch-security-2.12.0.0-SNAPSHOT.jar:2.12.0.0-SNAPSHOT]
at
org.opensearch.security.securityconf.DynamicConfigModelV7.newInstance(DynamicConfigModelV7.java:443)
~[opensearch-security-2.12.0.0-SNAPSHOT.jar:2.12.0.0-SNAPSHOT]
at
org.opensearch.security.securityconf.DynamicConfigModelV7.buildAAA(DynamicConfigModelV7.java:330)
[opensearch-security-2.12.0.0-SNAPSHOT.jar:2.12.0.0-SNAPSHOT]
at
org.opensearch.security.securityconf.DynamicConfigModelV7.<init>(DynamicConfigModelV7.java:100)
[opensearch-security-2.12.0.0-SNAPSHOT.jar:2.12.0.0-SNAPSHOT]
at
org.opensearch.security.securityconf.DynamicConfigFactory.onChange(DynamicConfigFactory.java:285)
[opensearch-security-2.12.0.0-SNAPSHOT.jar:2.12.0.0-SNAPSHOT]
at
org.opensearch.security.configuration.ConfigurationRepository.notifyAboutChanges(ConfigurationRepository.java:406)
[opensearch-security-2.12.0.0-SNAPSHOT.jar:2.12.0.0-SNAPSHOT]
at
org.opensearch.security.configuration.ConfigurationRepository.reloadConfiguration0(ConfigurationRepository.java:395)
[opensearch-security-2.12.0.0-SNAPSHOT.jar:2.12.0.0-SNAPSHOT]
at
org.opensearch.security.configuration.ConfigurationRepository.reloadConfiguration(ConfigurationRepository.java:379)
[opensearch-security-2.12.0.0-SNAPSHOT.jar:2.12.0.0-SNAPSHOT]
at
org.opensearch.security.action.configupdate.TransportConfigUpdateAction.nodeOperation(TransportConfigUpdateAction.java:128)
[opensearch-security-2.12.0.0-SNAPSHOT.jar:2.12.0.0-SNAPSHOT]
at
org.opensearch.security.action.configupdate.TransportConfigUpdateAction.nodeOperation(TransportConfigUpdateAction.java:52)
[opensearch-security-2.12.0.0-SNAPSHOT.jar:2.12.0.0-SNAPSHOT]
at
org.opensearch.action.support.nodes.TransportNodesAction.nodeOperation(TransportNodesAction.java:200)
[opensearch-2.12.0-SNAPSHOT.jar:2.12.0-SNAPSHOT]
at
org.opensearch.action.support.nodes.TransportNodesAction$NodeTransportHandler.messageReceived(TransportNodesAction.java:328)
[opensearch-2.12.0-SNAPSHOT.jar:2.12.0-SNAPSHOT]
at
org.opensearch.action.support.nodes.TransportNodesAction$NodeTransportHandler.messageReceived(TransportNodesAction.java:324)
[opensearch-2.12.0-SNAPSHOT.jar:2.12.0-SNAPSHOT]
at
org.opensearch.security.ssl.transport.SecuritySSLRequestHandler.messageReceivedDecorate(SecuritySSLRequestHandler.java:224)
[opensearch-security-2.12.0.0-SNAPSHOT.jar:2.12.0.0-SNAPSHOT]
at
org.opensearch.security.transport.SecurityRequestHandler.messageReceivedDecorate(SecurityRequestHandler.java:211)
[opensearch-security-2.12.0.0-SNAPSHOT.jar:2.12.0.0-SNAPSHOT]
at
org.opensearch.security.ssl.transport.SecuritySSLRequestHandler.messageReceived(SecuritySSLRequestHandler.java:109)
[opensearch-security-2.12.0.0-SNAPSHOT.jar:2.12.0.0-SNAPSHOT]
at
org.opensearch.security.OpenSearchSecurityPlugin$6$1.messageReceived(OpenSearchSecurityPlugin.java:794)
[opensearch-security-2.12.0.0-SNAPSHOT.jar:2.12.0.0-SNAPSHOT]
at
org.opensearch.transport.RequestHandlerRegistry.processMessageReceived(RequestHandlerRegistry.java:106)
[opensearch-2.12.0-SNAPSHOT.jar:2.12.0-SNAPSHOT]
at
org.opensearch.transport.TransportService$7.doRun(TransportService.java:1067)
[opensearch-2.12.0-SNAPSHOT.jar:2.12.0-SNAPSHOT]
at
org.opensearch.common.util.concurrent.ThreadContext$ContextPreservingAbstractRunnable.doRun(ThreadContext.java:911)
[opensearch-2.12.0-SNAPSHOT.jar:2.12.0-SNAPSHOT]
at
org.opensearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:52)
[opensearch-2.12.0-SNAPSHOT.jar:2.12.0-SNAPSHOT]
at
java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
[?:?]
at
java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
[?:?]
	at java.base/java.lang.Thread.run(Thread.java:829) [?:?]
Caused by: java.lang.reflect.InvocationTargetException
at
java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native
Method) ~[?:?]
at
java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
~[?:?]
at
java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
~[?:?]
at
java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:490)
~[?:?]
at
org.opensearch.security.support.ReflectionHelper.instantiateAAA(ReflectionHelper.java:62)
~[opensearch-security-2.12.0.0-SNAPSHOT.jar:2.12.0.0-SNAPSHOT]
	... 23 more
Caused by: java.lang.RuntimeException:
java.security.AccessControlException: access denied
("java.util.PropertyPermission" "*" "read,write")
at
com.amazon.dlic.auth.http.saml.HTTPSamlAuthenticator.<init>(HTTPSamlAuthenticator.java:154)
~[opensearch-security-2.12.0.0-SNAPSHOT.jar:2.12.0.0-SNAPSHOT]
at
java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native
Method) ~[?:?]
at
java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
~[?:?]
at
java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
~[?:?]
at
java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:490)
~[?:?]
at
org.opensearch.security.support.ReflectionHelper.instantiateAAA(ReflectionHelper.java:62)
~[opensearch-security-2.12.0.0-SNAPSHOT.jar:2.12.0.0-SNAPSHOT]
	... 23 more
Caused by: java.security.AccessControlException: access denied
("java.util.PropertyPermission" "*" "read,write")
at
java.base/java.security.AccessControlContext.checkPermission(AccessControlContext.java:472)
~[?:?]
at
java.base/java.security.AccessController.checkPermission(AccessController.java:897)
~[?:?]
at
java.base/java.lang.SecurityManager.checkPermission(SecurityManager.java:322)
~[?:?]
at
java.base/java.lang.SecurityManager.checkPropertiesAccess(SecurityManager.java:1034)
~[?:?]
	at java.base/java.lang.System.getProperties(System.java:731) ~[?:?]
at
org.opensaml.core.config.provider.SystemPropertyConfigurationPropertiesSource.getProperties(SystemPropertyConfigurationPropertiesSource.java:31)
~[opensaml-core-4.3.0.jar:?]
at
org.opensaml.core.config.ConfigurationService.getConfigurationProperties(ConfigurationService.java:148)
~[opensaml-core-4.3.0.jar:?]
at
org.opensaml.core.config.ConfigurationService.getPartitionName(ConfigurationService.java:192)
~[opensaml-core-4.3.0.jar:?]
at
org.opensaml.core.config.ConfigurationService.get(ConfigurationService.java:90)
~[opensaml-core-4.3.0.jar:?]
at
org.opensaml.core.xml.config.XMLObjectProviderRegistrySupport.getUnmarshallerFactory(XMLObjectProviderRegistrySupport.java:126)
~[opensaml-core-4.3.0.jar:?]
at
org.opensaml.saml.metadata.resolver.impl.AbstractMetadataResolver.<init>(AbstractMetadataResolver.java:118)
~[opensaml-saml-impl-4.3.0.jar:?]
at
org.opensaml.saml.metadata.resolver.impl.AbstractBatchMetadataResolver.<init>(AbstractBatchMetadataResolver.java:75)
~[opensaml-saml-impl-4.3.0.jar:?]
at
org.opensaml.saml.metadata.resolver.impl.AbstractReloadingMetadataResolver.<init>(AbstractReloadingMetadataResolver.java:128)
~[opensaml-saml-impl-4.3.0.jar:?]
at
org.opensaml.saml.metadata.resolver.impl.HTTPMetadataResolver.<init>(HTTPMetadataResolver.java:103)
~[opensaml-saml-impl-4.3.0.jar:?]
at
org.opensaml.saml.metadata.resolver.impl.HTTPMetadataResolver.<init>(HTTPMetadataResolver.java:89)
~[opensaml-saml-impl-4.3.0.jar:?]
at
com.amazon.dlic.auth.http.saml.SamlHTTPMetadataResolver.<init>(SamlHTTPMetadataResolver.java:34)
~[opensearch-security-2.12.0.0-SNAPSHOT.jar:2.12.0.0-SNAPSHOT]
at
com.amazon.dlic.auth.http.saml.HTTPSamlAuthenticator.createMetadataResolver(HTTPSamlAuthenticator.java:336)
~[opensearch-security-2.12.0.0-SNAPSHOT.jar:2.12.0.0-SNAPSHOT]
at
com.amazon.dlic.auth.http.saml.HTTPSamlAuthenticator.<init>(HTTPSamlAuthenticator.java:133)
~[opensearch-security-2.12.0.0-SNAPSHOT.jar:2.12.0.0-SNAPSHOT]
at
java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native
Method) ~[?:?]
at
java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
~[?:?]
at
java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
~[?:?]
at
java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:490)
~[?:?]
at
org.opensearch.security.support.ReflectionHelper.instantiateAAA(ReflectionHelper.java:62)
~[opensearch-security-2.12.0.0-SNAPSHOT.jar:2.12.0.0-SNAPSHOT]
	... 23 more
</details>

* Category (Enhancement, New feature, Bug fix, Test fix, Refactoring,
Maintenance, Documentation)

Bug fix

### Issues Resolved

Fixes CI issues seen in:

-
opensearch-project/security-dashboards-plugin#1691
-
opensearch-project/security-dashboards-plugin#1686

### Testing

Tested by creating a local distribution and running the
security-dashboards-plugin integration tests before and after this
change to show that it resolves the issue.

### Check List
- [ ] New functionality includes testing
- [ ] New functionality has been documented
- [ ] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and
signing off your commits, please check
[here](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin).

Signed-off-by: Craig Perkins <[email protected]>
  • Loading branch information
@cwperks
cwperks authored Dec 18, 2023
1 parent 5a161e4 commit af2b595
Showing 1 changed file with 8 additions and 8 deletions.
16 changes: 8 additions & 8 deletions src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@

import java.net.InetAddress;
import java.nio.file.Path;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
Expand All @@ -44,6 +46,7 @@
import com.google.common.collect.Multimap;
import com.google.common.collect.Multimaps;

import org.opensearch.SpecialPermission;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.xcontent.XContentType;
import org.opensearch.security.auth.AuthDomain;
Expand Down Expand Up @@ -433,14 +436,11 @@ private void destroyDestroyables(List<Destroyable> destroyableComponents) {
}

private <T> T newInstance(final String clazzOrShortcut, String type, final Settings settings, final Path configPath) {

String clazz = clazzOrShortcut;

if (authImplMap.containsKey(clazz + "_" + type)) {
clazz = authImplMap.get(clazz + "_" + type);
}

return ReflectionHelper.instantiateAAA(clazz, settings, configPath);
final String clazz = authImplMap.computeIfAbsent(clazzOrShortcut + "_" + type, k -> clazzOrShortcut);
return AccessController.doPrivileged((PrivilegedAction<T>) () -> {
SpecialPermission.check();
return ReflectionHelper.instantiateAAA(clazz, settings, configPath);
});
}

private String translateShortcutToClassName(final String clazzOrShortcut, final String type) {
Expand Down

0 comments on commit af2b595

Please sign in to comment.