You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by ke...@apache.org on 2021/08/18 08:43:05 UTC
[skywalking] 01/01: Harden the security of Groovy-based DSL,
MAL and LAL
This is an automated email from the ASF dual-hosted git repository.
kezhenxu94 pushed a commit to branch groovy-security
in repository https://gitbox.apache.org/repos/asf/skywalking.git
commit 9ffcefeebc1ca01e372ae1273df8c213d32ddaac
Author: kezhenxu94 <ke...@apache.org>
AuthorDate: Wed Aug 18 16:42:26 2021 +0800
Harden the security of Groovy-based DSL, MAL and LAL
---
CHANGES.md | 1 +
.../skywalking/oap/log/analyzer/dsl/DSL.java | 25 ++++
.../oap/log/analyzer/dsl/DSLSecurityTest.java | 140 +++++++++++++++++++++
.../skywalking/oap/meter/analyzer/dsl/DSL.java | 29 +++++
4 files changed, 195 insertions(+)
diff --git a/CHANGES.md b/CHANGES.md
index 8da38e0..09f1b63 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -42,6 +42,7 @@ Release Notes.
* Add `rpcStatusCode` for `rpc.status_code` tag. The `responseCode` field is marked as deprecated and replaced by `httpResponseStatusCode` field.
* Remove the duplicated tags to reduce the storage payload.
* Add a new API to test log analysis language.
+* Harden the security of Groovy-based DSL, MAL and LAL.
#### UI
diff --git a/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/dsl/DSL.java b/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/dsl/DSL.java
index c7db405..7b54de6 100644
--- a/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/dsl/DSL.java
+++ b/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/dsl/DSL.java
@@ -18,9 +18,13 @@
package org.apache.skywalking.oap.log.analyzer.dsl;
+import com.google.common.collect.ImmutableList;
import groovy.lang.GroovyShell;
import groovy.transform.CompileStatic;
import groovy.util.DelegatingScript;
+import java.lang.reflect.Array;
+import java.util.List;
+import java.util.Map;
import lombok.AccessLevel;
import lombok.RequiredArgsConstructor;
import org.apache.skywalking.oap.log.analyzer.dsl.spec.LALDelegatingScript;
@@ -28,8 +32,13 @@ import org.apache.skywalking.oap.log.analyzer.dsl.spec.filter.FilterSpec;
import org.apache.skywalking.oap.log.analyzer.provider.LogAnalyzerModuleConfig;
import org.apache.skywalking.oap.server.library.module.ModuleManager;
import org.apache.skywalking.oap.server.library.module.ModuleStartException;
+import org.codehaus.groovy.ast.stmt.DoWhileStatement;
+import org.codehaus.groovy.ast.stmt.ForStatement;
+import org.codehaus.groovy.ast.stmt.Statement;
+import org.codehaus.groovy.ast.stmt.WhileStatement;
import org.codehaus.groovy.control.CompilerConfiguration;
import org.codehaus.groovy.control.customizers.ASTTransformationCustomizer;
+import org.codehaus.groovy.control.customizers.SecureASTCustomizer;
import static java.util.Collections.singletonList;
import static java.util.Collections.singletonMap;
@@ -53,6 +62,22 @@ public class DSL {
CompileStatic.class
);
cc.addCompilationCustomizers(customizer);
+ final SecureASTCustomizer secureASTCustomizer = new SecureASTCustomizer();
+ secureASTCustomizer.setDisallowedStatements(
+ ImmutableList.<Class<? extends Statement>>builder()
+ .add(WhileStatement.class)
+ .add(DoWhileStatement.class)
+ .add(ForStatement.class)
+ .build());
+ // noinspection rawtypes
+ secureASTCustomizer.setAllowedReceiversClasses(
+ ImmutableList.<Class>builder()
+ .add(Object.class)
+ .add(Map.class)
+ .add(List.class)
+ .add(Array.class)
+ .build());
+ cc.addCompilationCustomizers(secureASTCustomizer);
cc.setScriptBaseClass(LALDelegatingScript.class.getName());
final GroovyShell sh = new GroovyShell(cc);
diff --git a/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/dsl/DSLSecurityTest.java b/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/dsl/DSLSecurityTest.java
new file mode 100644
index 0000000..2f0cb26
--- /dev/null
+++ b/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/dsl/DSLSecurityTest.java
@@ -0,0 +1,140 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.oap.log.analyzer.dsl;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import org.apache.skywalking.apm.network.logging.v3.LogData;
+import org.apache.skywalking.oap.log.analyzer.provider.LogAnalyzerModuleConfig;
+import org.apache.skywalking.oap.server.core.CoreModule;
+import org.apache.skywalking.oap.server.core.config.ConfigService;
+import org.apache.skywalking.oap.server.core.source.SourceReceiver;
+import org.apache.skywalking.oap.server.library.module.ModuleManager;
+import org.apache.skywalking.oap.server.library.module.ModuleProviderHolder;
+import org.apache.skywalking.oap.server.library.module.ModuleServiceHolder;
+import org.apache.skywalking.oap.server.library.module.ModuleStartException;
+import org.codehaus.groovy.control.MultipleCompilationErrorsException;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.powermock.reflect.Whitebox;
+
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+@RunWith(Parameterized.class)
+public class DSLSecurityTest {
+ @Parameterized.Parameters(name = "{index}: {0}")
+ public static Collection<Object[]> data() {
+ return Arrays.asList(
+ new String[] {
+ "DDOS",
+ "" +
+ "filter {\n" +
+ " System.exit(0)\n" +
+ " sink {}\n" +
+ "}",
+ },
+ new String[] {
+ "DDOS",
+ "" +
+ "filter {\n" +
+ " for (;;) {}\n" +
+ " sink {}\n" +
+ "}",
+ },
+ new String[] {
+ "DDOS",
+ "" +
+ "filter {\n" +
+ " while (true) {}\n" +
+ " sink {}\n" +
+ "}",
+ },
+ new String[] {
+ "DDOS",
+ "" +
+ "filter {\n" +
+ " do {} while (true)\n" +
+ " sink {}\n" +
+ "}",
+ },
+ new String[] {
+ "Steal or delete files on server",
+ "" +
+ "filter {\n" +
+ " Files.delete(\"/etc/pwd\");\n" +
+ " sink {}\n" +
+ "}",
+ },
+ new String[] {
+ "Evaluate malicious codes in GroovyShell from inside DSL to get rid of outer DSL restriction",
+ "" +
+ "filter {\n" +
+ " new GroovyShell().evaluate('malicious codes or URL')\n" +
+ " sink {}\n" +
+ "}",
+ },
+ new String[] {
+ "disallowed methods",
+ "filter {\n" +
+ "java.util.Collections.singleton(1)" +
+ "}",
+ }
+ );
+ }
+
+ @Parameterized.Parameter()
+ public String name;
+
+ @Parameterized.Parameter(1)
+ public String script;
+
+ final ModuleManager manager = mock(ModuleManager.class);
+
+ @Before
+ public void setup() {
+ Whitebox.setInternalState(manager, "isInPrepareStage", false);
+ when(manager.find(anyString())).thenReturn(mock(ModuleProviderHolder.class));
+ when(manager.find(CoreModule.NAME).provider()).thenReturn(mock(ModuleServiceHolder.class));
+ when(manager.find(CoreModule.NAME).provider().getService(SourceReceiver.class))
+ .thenReturn(mock(SourceReceiver.class));
+ when(manager.find(CoreModule.NAME).provider().getService(ConfigService.class))
+ .thenReturn(mock(ConfigService.class));
+ when(manager.find(CoreModule.NAME)
+ .provider()
+ .getService(ConfigService.class)
+ .getSearchableLogsTags())
+ .thenReturn("");
+ }
+
+ @Test(expected = MultipleCompilationErrorsException.class)
+ public void testSecurity() throws ModuleStartException {
+ final DSL dsl = DSL.of(manager, new LogAnalyzerModuleConfig(), script);
+ Whitebox.setInternalState(
+ Whitebox.getInternalState(dsl, "filterSpec"), "factories", Collections.emptyList()
+ );
+
+ dsl.bind(new Binding().log(LogData.newBuilder()));
+ dsl.evaluate();
+ }
+}
diff --git a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/DSL.java b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/DSL.java
index bb8c737..51cbc80 100644
--- a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/DSL.java
+++ b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/DSL.java
@@ -18,13 +18,22 @@
package org.apache.skywalking.oap.meter.analyzer.dsl;
+import com.google.common.collect.ImmutableList;
import groovy.lang.Binding;
import groovy.lang.GroovyShell;
import groovy.util.DelegatingScript;
+import java.lang.reflect.Array;
+import java.util.List;
+import java.util.Map;
import org.apache.skywalking.oap.meter.analyzer.dsl.tagOpt.K8sRetagType;
import org.apache.skywalking.oap.server.core.source.DetectPoint;
+import org.codehaus.groovy.ast.stmt.DoWhileStatement;
+import org.codehaus.groovy.ast.stmt.ForStatement;
+import org.codehaus.groovy.ast.stmt.Statement;
+import org.codehaus.groovy.ast.stmt.WhileStatement;
import org.codehaus.groovy.control.CompilerConfiguration;
import org.codehaus.groovy.control.customizers.ImportCustomizer;
+import org.codehaus.groovy.control.customizers.SecureASTCustomizer;
/**
* DSL combines methods to parse groovy based DSL expression.
@@ -44,6 +53,26 @@ public final class DSL {
icz.addImport("K8sRetagType", K8sRetagType.class.getName());
icz.addImport("DetectPoint", DetectPoint.class.getName());
cc.addCompilationCustomizers(icz);
+
+ final SecureASTCustomizer secureASTCustomizer = new SecureASTCustomizer();
+ secureASTCustomizer.setDisallowedStatements(
+ ImmutableList.<Class<? extends Statement>>builder()
+ .add(WhileStatement.class)
+ .add(DoWhileStatement.class)
+ .add(ForStatement.class)
+ .build());
+ // noinspection rawtypes
+ secureASTCustomizer.setAllowedReceiversClasses(
+ ImmutableList.<Class>builder()
+ .add(Object.class)
+ .add(Map.class)
+ .add(List.class)
+ .add(Array.class)
+ .add(K8sRetagType.class)
+ .add(DetectPoint.class)
+ .build());
+ cc.addCompilationCustomizers(secureASTCustomizer);
+
GroovyShell sh = new GroovyShell(new Binding(), cc);
DelegatingScript script = (DelegatingScript) sh.parse(expression);
return new Expression(expression, script);