Skip to content
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

Revert "Replace opensearch class names with opendistro class names during serialization and restore them back during deserialization (#1278)" #1691

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion plugin-security.policy
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ grant {
//Java 9+
permission java.lang.RuntimePermission "accessClassInPackage.com.sun.jndi.*";

permission java.io.SerializablePermission "enableSubstitution";
//Enable this permission to debug unauthorized de-serialization attempt
//permission java.io.SerializablePermission "enableSubstitution";
};

grant codeBase "${codebase.netty-common}" {
Expand Down
85 changes: 0 additions & 85 deletions src/main/java/org/opensearch/security/support/Base64Helper.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,6 @@
package org.opensearch.security.support;

import com.amazon.dlic.auth.ldap.LdapUser;
import org.apache.commons.lang3.SerializationUtils;
import org.slf4j.LoggerFactory;
import org.slf4j.Logger;
import org.ldaptive.AbstractLdapBean;
import org.ldaptive.LdapAttribute;
import org.ldaptive.LdapEntry;
Expand All @@ -49,7 +46,6 @@
import java.io.ObjectStreamClass;
import java.io.OutputStream;
import java.io.Serializable;
import java.lang.reflect.Field;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.SocketAddress;
Expand All @@ -60,8 +56,6 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.regex.Pattern;

import org.opensearch.OpenSearchException;
Expand All @@ -76,10 +70,6 @@
import com.google.common.io.BaseEncoding;

public class Base64Helper {
private static final Logger logger = LoggerFactory.getLogger(Base64Helper.class);

private static final String ODFE_PACKAGE = "com.amazon.opendistroforelasticsearch";
private static final String OS_PACKAGE = "org.opensearch";

private static final Set<Class<?>> SAFE_CLASSES = ImmutableSet.of(
String.class,
Expand Down Expand Up @@ -114,69 +104,10 @@ private static boolean isSafeClass(Class<?> cls) {
SAFE_ASSIGNABLE_FROM_CLASSES.stream().anyMatch(c -> c.isAssignableFrom(cls));
}

private static class DescriptorNameSetter {
private static final Field NAME = getField();

private DescriptorNameSetter() {
}

private static Field getFieldPrivileged() {
try {
final Field field = ObjectStreamClass.class.getDeclaredField("name");
field.setAccessible(true);
return field;
} catch (NoSuchFieldException | SecurityException e) {
logger.error("Failed to get ObjectStreamClass declared field", e);
if (e instanceof RuntimeException) {
throw (RuntimeException) e;
} else {
throw new RuntimeException(e);
}
}
}

private static Field getField() {
SpecialPermission.check();
return AccessController.doPrivileged((PrivilegedAction<Field>) () -> getFieldPrivileged());
}

public static void setName(ObjectStreamClass desc, String name) {
try {
logger.debug("replacing descriptor name from [{}] to [{}]", desc.getName(), name);
NAME.set(desc, name);
} catch (IllegalAccessException e) {
logger.error("Failed to replace descriptor name from {} to {}", desc.getName(), name, e);
throw new OpenSearchException(e);
}
}
}

private static class DescriptorReplacer {
private final ConcurrentMap<String, ObjectStreamClass> nameToDescriptor = new ConcurrentHashMap<>();

public ObjectStreamClass replace(final ObjectStreamClass desc) {
final String name = desc.getName();
if (name.startsWith(OS_PACKAGE)) {
return nameToDescriptor.computeIfAbsent(name, s -> {
SpecialPermission.check();
// we can't modify original descriptor as it is cached by ObjectStreamClass, create clone
final ObjectStreamClass clone = AccessController.doPrivileged(
(PrivilegedAction<ObjectStreamClass>)() -> SerializationUtils.clone(desc)
);
DescriptorNameSetter.setName(clone, s.replace(OS_PACKAGE, ODFE_PACKAGE));
return clone;
});
}
return desc;
}
}

private final static class SafeObjectOutputStream extends ObjectOutputStream {

private static final boolean useSafeObjectOutputStream = checkSubstitutionPermission();

private final DescriptorReplacer descriptorReplacer = new DescriptorReplacer();

private static boolean checkSubstitutionPermission() {
SecurityManager sm = System.getSecurityManager();
if (sm != null) {
Expand Down Expand Up @@ -218,11 +149,6 @@ private SafeObjectOutputStream(OutputStream out) throws IOException {
);
}

@Override
protected void writeClassDescriptor(final ObjectStreamClass desc) throws IOException {
super.writeClassDescriptor(descriptorReplacer.replace(desc));
}

@Override
protected Object replaceObject(Object obj) throws IOException {
Class<?> clazz = obj.getClass();
Expand Down Expand Up @@ -276,16 +202,5 @@ protected Class<?> resolveClass(ObjectStreamClass desc) throws IOException, Clas

throw new InvalidClassException("Unauthorized deserialization attempt ", clazz.getName());
}

@Override
protected ObjectStreamClass readClassDescriptor() throws IOException, ClassNotFoundException {
ObjectStreamClass desc = super.readClassDescriptor();
final String name = desc.getName();
if (name.startsWith(ODFE_PACKAGE)) {
desc = ObjectStreamClass.lookup(Class.forName(name.replace(ODFE_PACKAGE, OS_PACKAGE)));
logger.debug("replaced descriptor name from [{}] to [{}]", name, desc.getName());
}
return desc;
}
}
}
12 changes: 6 additions & 6 deletions src/main/java/org/opensearch/security/user/User.java
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public class User implements Serializable, Writeable, CustomAttributesAware {
* roles == backend_roles
*/
private final Set<String> roles = new HashSet<String>();
private final Set<String> openDistroSecurityRoles = new HashSet<String>();
private final Set<String> securityRoles = new HashSet<String>();
private String requestedTenant;
private Map<String, String> attributes = new HashMap<>();
private boolean isInjected = false;
Expand All @@ -78,7 +78,7 @@ public User(final StreamInput in) throws IOException {
roles.addAll(in.readList(StreamInput::readString));
requestedTenant = in.readString();
attributes = in.readMap(StreamInput::readString, StreamInput::readString);
openDistroSecurityRoles.addAll(in.readList(StreamInput::readString));
securityRoles.addAll(in.readList(StreamInput::readString));
}

/**
Expand Down Expand Up @@ -244,7 +244,7 @@ public void writeTo(StreamOutput out) throws IOException {
out.writeStringCollection(new ArrayList<String>(roles));
out.writeString(requestedTenant);
out.writeMap(attributes, StreamOutput::writeString, StreamOutput::writeString);
out.writeStringCollection(openDistroSecurityRoles ==null?Collections.emptyList():new ArrayList<String>(openDistroSecurityRoles));
out.writeStringCollection(securityRoles ==null?Collections.emptyList():new ArrayList<String>(securityRoles));
}

/**
Expand All @@ -260,12 +260,12 @@ public synchronized final Map<String, String> getCustomAttributesMap() {
}

public final void addSecurityRoles(final Collection<String> securityRoles) {
if(securityRoles != null && this.openDistroSecurityRoles != null) {
this.openDistroSecurityRoles.addAll(securityRoles);
if(securityRoles != null && this.securityRoles != null) {
this.securityRoles.addAll(securityRoles);
}
}

public final Set<String> getSecurityRoles() {
return this.openDistroSecurityRoles == null ? Collections.emptySet() : Collections.unmodifiableSet(this.openDistroSecurityRoles);
return this.securityRoles == null ? Collections.emptySet() : Collections.unmodifiableSet(this.securityRoles);
}
}