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

disable javascript execution by default #3818

Merged
merged 1 commit into from
Feb 13, 2017
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
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public void verify(List<String> usedCols)
@Override
public Parser<String, Object> makeParser()
{
if (config.isDisabled()) {
if (!config.isEnabled()) {
throw new ISE("JavaScript is disabled");
}

Expand Down
39 changes: 21 additions & 18 deletions api/src/main/java/io/druid/js/JavaScriptConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,31 +19,31 @@

package io.druid.js;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;

import java.util.Objects;

public class JavaScriptConfig
{
public static final int DEFAULT_OPTIMIZATION_LEVEL = 9;

private static final JavaScriptConfig DEFAULT = new JavaScriptConfig(false);
private static final JavaScriptConfig ENABLED_INSTANCE = new JavaScriptConfig(true);

@JsonProperty
private boolean disabled = false;

public JavaScriptConfig()
{
}
private boolean enabled = false;

public JavaScriptConfig(boolean disabled)
@JsonCreator
public JavaScriptConfig(
@JsonProperty("enabled") Boolean enabled
)
{
this.disabled = disabled;
if (enabled != null) {
this.enabled = enabled.booleanValue();
}
}

public boolean isDisabled()
public boolean isEnabled()
{
return disabled;
return enabled;
}

@Override
Expand All @@ -55,26 +55,29 @@ public boolean equals(Object o)
if (o == null || getClass() != o.getClass()) {
return false;
}
JavaScriptConfig config = (JavaScriptConfig) o;
return disabled == config.disabled;

JavaScriptConfig that = (JavaScriptConfig) o;

return enabled == that.enabled;

}

@Override
public int hashCode()
{
return Objects.hash(disabled);
return (enabled ? 1 : 0);
}

@Override
public String toString()
{
return "JavaScriptConfig{" +
"disabled=" + disabled +
"enabled=" + enabled +
'}';
}

public static JavaScriptConfig getDefault()
public static JavaScriptConfig getEnabledInstance()
{
return DEFAULT;
return ENABLED_INSTANCE;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,14 @@ public void testSerde() throws IOException
jsonMapper.setInjectableValues(
new InjectableValues.Std().addValue(
JavaScriptConfig.class,
JavaScriptConfig.getDefault()
JavaScriptConfig.getEnabledInstance()
)
);
JavaScriptParseSpec spec = new JavaScriptParseSpec(
new TimestampSpec("abc", "iso", null),
new DimensionsSpec(DimensionsSpec.getDefaultSchemas(Arrays.asList("abc")), null, null),
"abc",
JavaScriptConfig.getDefault()
JavaScriptConfig.getEnabledInstance()
);
final JavaScriptParseSpec serde = jsonMapper.readValue(
jsonMapper.writeValueAsString(spec),
Expand All @@ -73,7 +73,7 @@ public void testSerde() throws IOException
@Test
public void testMakeParser()
{
final JavaScriptConfig config = JavaScriptConfig.getDefault();
final JavaScriptConfig config = JavaScriptConfig.getEnabledInstance();
JavaScriptParseSpec spec = new JavaScriptParseSpec(
new TimestampSpec("abc", "iso", null),
new DimensionsSpec(DimensionsSpec.getDefaultSchemas(Arrays.asList("abc")), null, null),
Expand All @@ -89,7 +89,7 @@ public void testMakeParser()
@Test
public void testMakeParserNotAllowed()
{
final JavaScriptConfig config = new JavaScriptConfig(true);
final JavaScriptConfig config = new JavaScriptConfig(false);
JavaScriptParseSpec spec = new JavaScriptParseSpec(
new TimestampSpec("abc", "iso", null),
new DimensionsSpec(DimensionsSpec.getDefaultSchemas(Arrays.asList("abc")), null, null),
Expand Down
36 changes: 31 additions & 5 deletions api/src/test/java/io/druid/js/JavaScriptConfigTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,39 @@

public class JavaScriptConfigTest
{
private static ObjectMapper mapper = new ObjectMapper();

@Test
public void testSerde() throws Exception
{
final JavaScriptConfig config = new JavaScriptConfig(true);
final ObjectMapper mapper = new ObjectMapper();
final JavaScriptConfig config2 = mapper.readValue(mapper.writeValueAsBytes(config), JavaScriptConfig.class);
Assert.assertTrue(config2.isDisabled());
Assert.assertEquals(config, config2);
String json = "{\"enabled\":true}";

JavaScriptConfig config = mapper.readValue(
mapper.writeValueAsString(
mapper.readValue(
json,
JavaScriptConfig.class
)
), JavaScriptConfig.class
);

Assert.assertTrue(config.isEnabled());
}

@Test
public void testSerdeWithDefaults() throws Exception
{
String json = "{}";

JavaScriptConfig config = mapper.readValue(
mapper.writeValueAsString(
mapper.readValue(
json,
JavaScriptConfig.class
)
), JavaScriptConfig.class
);

Assert.assertFalse(config.isEnabled());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ public class FilterPartitionBenchmark
private BenchmarkSchemaInfo schemaInfo;

private static String JS_FN = "function(str) { return 'super-' + str; }";
private static ExtractionFn JS_EXTRACTION_FN = new JavaScriptExtractionFn(JS_FN, false, JavaScriptConfig.getDefault());
private static ExtractionFn JS_EXTRACTION_FN = new JavaScriptExtractionFn(JS_FN, false, JavaScriptConfig.getEnabledInstance());

static {
JSON_MAPPER = new DefaultObjectMapper();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ public class FilteredAggregatorBenchmark
private TimeseriesQuery query;

private static String JS_FN = "function(str) { return 'super-' + str; }";
private static ExtractionFn JS_EXTRACTION_FN = new JavaScriptExtractionFn(JS_FN, false, JavaScriptConfig.getDefault());
private static ExtractionFn JS_EXTRACTION_FN = new JavaScriptExtractionFn(JS_FN, false, JavaScriptConfig.getEnabledInstance());

static {
JSON_MAPPER = new DefaultObjectMapper();
Expand Down Expand Up @@ -167,7 +167,7 @@ public void setup() throws IOException
filter = new OrDimFilter(
Arrays.asList(
new BoundDimFilter("dimSequential", "-1", "-1", true, true, null, null, StringComparators.ALPHANUMERIC),
new JavaScriptDimFilter("dimSequential", "function(x) { return false }", null, JavaScriptConfig.getDefault()),
new JavaScriptDimFilter("dimSequential", "function(x) { return false }", null, JavaScriptConfig.getEnabledInstance()),
new RegexDimFilter("dimSequential", "X", null),
new SearchQueryDimFilter("dimSequential", new ContainsSearchQuerySpec("X", false), null),
new InDimFilter("dimSequential", Arrays.asList("X"), null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ public void readWithFilters(Blackhole blackhole) throws Exception
DimFilter filter = new OrDimFilter(
Arrays.asList(
new BoundDimFilter("dimSequential", "-1", "-1", true, true, null, null, StringComparators.ALPHANUMERIC),
new JavaScriptDimFilter("dimSequential", "function(x) { return false }", null, JavaScriptConfig.getDefault()),
new JavaScriptDimFilter("dimSequential", "function(x) { return false }", null, JavaScriptConfig.getEnabledInstance()),
new RegexDimFilter("dimSequential", "X", null),
new SearchQueryDimFilter("dimSequential", new ContainsSearchQuerySpec("X", false), null),
new InDimFilter("dimSequential", Arrays.asList("X"), null)
Expand Down
5 changes: 2 additions & 3 deletions docs/content/configuration/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -384,9 +384,8 @@ the following properties.

|Property|Description|Default|
|--------|-----------|-------|
|`druid.javascript.disabled`|Set to "true" to disable JavaScript functionality. This affects the JavaScript parser, filter, extractionFn, aggregator, post-aggregator, router strategy, and worker selection strategy.|false|
|`druid.javascript.enabled`|Set to "true" to enable JavaScript functionality. This affects the JavaScript parser, filter, extractionFn, aggregator, post-aggregator, router strategy, and worker selection strategy.|false|

<div class="note info">
Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines
about using Druid's JavaScript functionality.
JavaScript-based functionality is disabled by default. Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines about using Druid's JavaScript functionality, including instructions on how to enable it.
</div>
3 changes: 1 addition & 2 deletions docs/content/configuration/indexing-service.md
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,7 @@ Example: a function that sends batch_index_task to workers 10.0.0.1 and 10.0.0.2
```

<div class="note info">
Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines
about using Druid's JavaScript functionality.
JavaScript-based functionality is disabled by default. Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines about using Druid's JavaScript functionality, including instructions on how to enable it.
</div>

#### Autoscaler
Expand Down
15 changes: 8 additions & 7 deletions docs/content/development/javascript.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,14 @@ without needing to write and deploy Druid extensions.

Druid uses the Mozilla Rhino engine at optimization level 9 to compile and execute JavaScript.

## Security

Druid does not execute JavaScript functions in a sandbox, so they have full access to the machine. So Javascript
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"arbitrary"

functions allow users to execute arbutrary code inside druid process. So, by default, Javascript is disabled.
However, on dev/staging environments or secured production environments you can enable those by setting
the [configuration property](../configuration/index.html)
`druid.javascript.enabled = true`.

## Global variables

Avoid using global variables. Druid may share the global scope between multiple threads, which can lead to
Expand All @@ -36,13 +44,6 @@ You may need to pay special attention to garbage collection when making heavy us
garbage collection of the compiled classes themselves. Be sure to use a garbage collector configuration that supports
timely collection of unused classes (this is generally easier on JDK8 with the Metaspace than it is on JDK7).

## Security

Druid does not execute JavaScript functions in a sandbox, so they have full access to the machine. If you are running
a cluster where users that can submit queries should not be allowed to execute arbitrary code, we recommend disabling
JavaScript functionality by setting the [configuration property](../configuration/index.html)
`druid.javascript.disabled = true`.

## JavaScript vs. Native Extensions

Generally we recommend using JavaScript when security is not an issue, and when speed of development is more important
Expand Down
3 changes: 1 addition & 2 deletions docs/content/development/router.md
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,7 @@ Allows defining arbitrary routing rules using a JavaScript function. The functio
```

<div class="note info">
Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines
about using Druid's JavaScript functionality.
JavaScript-based functionality is disabled by default. Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines about using Druid's JavaScript functionality, including instructions on how to enable it.
</div>

HTTP Endpoints
Expand Down
3 changes: 1 addition & 2 deletions docs/content/ingestion/data-formats.md
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,7 @@ Note with the JavaScript parser that data must be fully parsed and returned as a
This means any flattening or parsing multi-dimensional values must be done here.

<div class="note info">
Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines
about using Druid's JavaScript functionality.
JavaScript-based functionality is disabled by default. Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines about using Druid's JavaScript functionality, including instructions on how to enable it.
</div>

### Multi-value dimensions
Expand Down
3 changes: 1 addition & 2 deletions docs/content/querying/aggregations.md
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,7 @@ JavaScript functions are expected to return floating-point values.
```

<div class="note info">
Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines
about using Druid's JavaScript functionality.
JavaScript-based functionality is disabled by default. Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines about using Druid's JavaScript functionality, including instructions on how to enable it.
</div>

## Approximate Aggregations
Expand Down
3 changes: 1 addition & 2 deletions docs/content/querying/dimensionspecs.md
Original file line number Diff line number Diff line change
Expand Up @@ -308,8 +308,7 @@ Example for the `__time` dimension:
```

<div class="note info">
Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines
about using Druid's JavaScript functionality.
JavaScript-based functionality is disabled by default. Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines about using Druid's JavaScript functionality, including instructions on how to enable it.
</div>

### Lookup extraction function
Expand Down
5 changes: 2 additions & 3 deletions docs/content/querying/filters.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,7 @@ The following matches any dimension values for the dimension `name` between `'ba
The JavaScript filter supports the use of extraction functions, see [Filtering with Extraction Functions](#filtering-with-extraction-functions) for details.

<div class="note info">
Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines
about using Druid's JavaScript functionality.
JavaScript-based functionality is disabled by default. Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines about using Druid's JavaScript functionality, including instructions on how to enable it.
</div>

### Extraction filter
Expand Down Expand Up @@ -440,4 +439,4 @@ Filtering on a set of ISO 8601 intervals:
"2014-11-15T00:00:00.000Z/2014-11-16T00:00:00.000Z"
]
}
```
```
3 changes: 1 addition & 2 deletions docs/content/querying/post-aggregations.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,7 @@ Example JavaScript aggregator:
```

<div class="note info">
Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines
about using Druid's JavaScript functionality.
JavaScript-based functionality is disabled by default. Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines about using Druid's JavaScript functionality, including instructions on how to enable it.
</div>

### HyperUnique Cardinality post-aggregator
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public JavaScriptWorkerSelectStrategy(
{
Preconditions.checkNotNull(fn, "function must not be null");

if (config.isDisabled()) {
if (!config.isEnabled()) {
throw new ISE("JavaScript is disabled");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public class JavaScriptWorkerSelectStrategyTest
+ "}\n"
+ "return null;\n"
+ "}",
JavaScriptConfig.getDefault()
JavaScriptConfig.getEnabledInstance()
);

@Test
Expand All @@ -79,7 +79,7 @@ public void testSerde() throws Exception
mapper.setInjectableValues(
new InjectableValues.Std().addValue(
JavaScriptConfig.class,
JavaScriptConfig.getDefault()
JavaScriptConfig.getEnabledInstance()
)
);

Expand All @@ -99,7 +99,7 @@ public void testDisabled() throws Exception
mapper.setInjectableValues(
new InjectableValues.Std().addValue(
JavaScriptConfig.class,
new JavaScriptConfig(true)
new JavaScriptConfig(false)
)
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,10 @@ public JavaScriptAggregatorFactory(
this.fnCombine = fnCombine;
this.config = config;

if (config.isDisabled()) {
this.compiledScript = null;
} else {
if (config.isEnabled()) {
this.compiledScript = compileScript(fnAggregate, fnReset, fnCombine);
} else {
this.compiledScript = null;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public JavaScriptPostAggregator(
Preconditions.checkNotNull(name, "Must have a valid, non-null post-aggregator name");
Preconditions.checkNotNull(fieldNames, "Must have a valid, non-null fieldNames");
Preconditions.checkNotNull(function, "Must have a valid, non-null function");
Preconditions.checkState(!config.isDisabled(), "JavaScript is disabled");
Preconditions.checkState(config.isEnabled(), "JavaScript is disabled.");

this.name = name;
this.fieldNames = fieldNames;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,10 @@ public JavaScriptExtractionFn(
this.function = function;
this.injective = injective;

if (config.isDisabled()) {
this.fn = null;
} else {
if (config.isEnabled()) {
this.fn = compile(function);
} else {
this.fn = null;
}
}

Expand Down
Loading