disable javascript execution by default #3818

himanshug · 2017-01-04T22:50:15Z

most users don't use javascript but still have it enabled because it happens to be default behavior and are surprised that it is enabled by default even with severe security threat.

with 0.10.0, we can disable javascript by default.

gianm · 2017-01-05T00:44:08Z

Pretty bold @himanshug :)

There should be a doc change for this too, at least to javascript.md and probably a few other files. Maybe also add a property druid.javascript.enable so the config to enable javascript doesn't look so goofy (druid.javascript.disable=false is a double negative and looks weird). If you do that then there should be something verifying that the two properties match up if they're both defined.

That all being said, I'm not sure how I feel about making this change. JavaScript being off by default appeals to me because it's generally not recommended except for prototyping, and is not sandboxed at all (which, even though documented, is likely surprising to users). But I am also reticent to go buck wild with incompatible changes. I'll go along with whatever the consensus is though.

pjain1 · 2017-01-05T16:24:43Z

I am 👍 with the changes. I agree with Gian, I think we can just have an enable or enabled property and we can keep getDefault() method in addition to getEnabled() which would have enable set to false.

gianm · 2017-01-05T16:27:29Z

api/src/main/java/io/druid/js/JavaScriptConfig.java

@@ -73,8 +73,8 @@ public String toString()
           '}';
  }

-  public static JavaScriptConfig getDefault()
+  public static JavaScriptConfig getEnabled()


I did a double-take on this, thinking it was a method getting whether or not something was enabled. getEnabledInstance() or enabledInstance() would probably be clearer.

himanshug · 2017-01-05T16:34:55Z

@gianm I can make all the other doc updates etc , if we conclude that it makes sense to disable javascript by default. yes, it is backward incompatible and that is why 0.10.0 would be the right time to do it.
m adding the discuss tag on it.

jon-wei · 2017-01-06T01:31:09Z

I think having javascript disabled is a good default, and IMO it doesn't impose a huge incompatibility transition cost on users (just add the enable property to the config)

Also agree on flipping the setting name from druid.javascript.disabled to druid.javascript.enabled

himanshug · 2017-01-06T20:45:06Z

@gianm @jon-wei @pjain1 switched config name from "disabled" to "enabled" and updated javascript.md docs

pjain1 · 2017-01-09T17:53:11Z

👍

fjy · 2017-01-12T22:13:48Z

👍

jon-wei · 2017-01-12T22:18:48Z

docs/content/development/javascript.md

+
+Druid does not execute JavaScript functions in a sandbox, so they have full access to the machine. So Javascript
+functions allow users to execute arbutrary code inside druid process. So, by default, Javascript is disabled.
+However, on dev/staging environments or a secured production environments you can enable those by setting


a secured production environments -> remove the a

@jon-wei updated

jon-wei · 2017-01-12T22:19:56Z

Had a minor comment on doc wording, otherwise 👍

gianm

Looks good, but can I request we keep this open until next weeks dev sync?

gianm · 2017-01-12T22:38:19Z

docs/content/development/javascript.md

@@ -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


"arbitrary"

vogievetsky · 2017-01-14T01:08:43Z

👎 booo JavaScript makes the world go round. There are a number of cool things in Plywood that would stop working if JavaScript was disabled.

vogievetsky · 2017-01-14T01:44:57Z

The specific things that JS is used for as a gap filler are:

Aggregations over dimensions (Allow aggregators to work on dimensions #3004)
Applying a boolean predicate (filter) as an extractionFn i.e. GROUP BY x == "lol"
Applying cool functions like ABS and POWER
Doing expression transformation within aggregates like SUM(x * y)
Certain time manipulations (Add timePart extractionFn (Or make timeFormat more useful) #1877) that are currently done like so:

$("time").timePart('SECOND_OF_DAY', 'Etc/UTC')

=>

{
  "dimension": "__time",
  "extractionFn": {
    "function": "function(s){try{\nvar d = new org.joda.time.DateTime(s);\nd = d.getSecondOfDay();\nreturn d;\n}catch(e){return null;}}",
    "type": "javascript"
  },
  "outputName": "Time",
  "type": "extraction"
}

himanshug · 2017-01-17T19:27:21Z

@vogievetsky those are compelling reasons to have js enabled on cluster ( and by default specially for Pivot users)
couldn't discuss this one today, will conclude it in next sync-up.

himanshug · 2017-01-24T19:02:09Z

so, we discussed both options in the dev sync and concluded that it should be secure by default and by making users explicitly enable it, they will know the risks versus current default where users have it enabled inadvertently and not even using the JS features.

himanshug · 2017-02-07T18:33:49Z

this was discussed again today and still concluded to go ahead and disable javascript by default. @fjy @gianm

gianm · 2017-02-07T19:57:28Z

Ok, that sounds fair @himanshug. One request I have is to edit the docs for the "javascript" things to point out that it's disabled by default, otherwise users might get confused when the things they read about in the docs don't work. One way to do this is to change the info note that is next to all the javascript things to something like:

<div class="note info">
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>

gianm · 2017-02-07T19:58:18Z

If you search in the docs for "JavaScript programming guide" you should then find all the boxes.

himanshug · 2017-02-10T17:57:44Z

@gianm docs for "JavaScript programming guide" are updated.

gianm

👍 LGTM

@himanshug I am OK with this patch as-is after fixing conflicts

himanshug added Incompatible Release Notes labels Jan 4, 2017

himanshug added this to the 0.10.0 milestone Jan 4, 2017

gianm reviewed Jan 5, 2017

View reviewed changes

himanshug added the Discuss label Jan 5, 2017

himanshug force-pushed the disable_javascript branch 2 times, most recently from efe1136 to 27de087 Compare January 6, 2017 20:43

himanshug removed the Discuss label Jan 6, 2017

himanshug force-pushed the disable_javascript branch 5 times, most recently from 3a5b4bf to e73a915 Compare January 9, 2017 16:01

himanshug assigned gianm and jon-wei Jan 9, 2017

jon-wei reviewed Jan 12, 2017

View reviewed changes

himanshug force-pushed the disable_javascript branch from e73a915 to 8603162 Compare January 12, 2017 22:30

gianm reviewed Jan 12, 2017

View reviewed changes

himanshug force-pushed the disable_javascript branch 2 times, most recently from b672c5d to 266351f Compare February 9, 2017 19:03

gianm approved these changes Feb 13, 2017

View reviewed changes

disable javascript execution by default

563e0b0

himanshug force-pushed the disable_javascript branch from 266351f to 563e0b0 Compare February 13, 2017 22:28

fjy merged commit 9dfcf07 into apache:master Feb 13, 2017

gianm mentioned this pull request Feb 28, 2017

Druid 0.10.0 release notes #3944

Closed

himanshug deleted the disable_javascript branch December 29, 2017 17:30

clambertus unassigned gianm and jon-wei Jul 6, 2018

This was referenced Sep 5, 2021

[Snyk] Security upgrade axios from 0.18.0 to 0.21.3 ajesse11x/incubator-druid#229

Open

[Snyk] Security upgrade axios from 0.21.1 to 0.21.3 terrorizer1980/druid#11

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

disable javascript execution by default #3818

disable javascript execution by default #3818

himanshug commented Jan 4, 2017

gianm commented Jan 5, 2017

pjain1 commented Jan 5, 2017 •

edited

Loading

gianm Jan 5, 2017

himanshug Jan 6, 2017

himanshug commented Jan 5, 2017

jon-wei commented Jan 6, 2017

himanshug commented Jan 6, 2017

pjain1 commented Jan 9, 2017

fjy commented Jan 12, 2017

jon-wei Jan 12, 2017

himanshug Jan 12, 2017

jon-wei commented Jan 12, 2017

gianm left a comment

gianm Jan 12, 2017

vogievetsky commented Jan 14, 2017

vogievetsky commented Jan 14, 2017

himanshug commented Jan 17, 2017

himanshug commented Jan 24, 2017

himanshug commented Feb 7, 2017

gianm commented Feb 7, 2017 •

edited

Loading

gianm commented Feb 7, 2017

himanshug commented Feb 10, 2017

gianm left a comment

disable javascript execution by default #3818

disable javascript execution by default #3818

Conversation

himanshug commented Jan 4, 2017

gianm commented Jan 5, 2017

pjain1 commented Jan 5, 2017 • edited Loading

gianm Jan 5, 2017

Choose a reason for hiding this comment

himanshug Jan 6, 2017

Choose a reason for hiding this comment

himanshug commented Jan 5, 2017

jon-wei commented Jan 6, 2017

himanshug commented Jan 6, 2017

pjain1 commented Jan 9, 2017

fjy commented Jan 12, 2017

jon-wei Jan 12, 2017

Choose a reason for hiding this comment

himanshug Jan 12, 2017

Choose a reason for hiding this comment

jon-wei commented Jan 12, 2017

gianm left a comment

Choose a reason for hiding this comment

gianm Jan 12, 2017

Choose a reason for hiding this comment

vogievetsky commented Jan 14, 2017

vogievetsky commented Jan 14, 2017

himanshug commented Jan 17, 2017

himanshug commented Jan 24, 2017

himanshug commented Feb 7, 2017

gianm commented Feb 7, 2017 • edited Loading

gianm commented Feb 7, 2017

himanshug commented Feb 10, 2017

gianm left a comment

Choose a reason for hiding this comment

pjain1 commented Jan 5, 2017 •

edited

Loading

gianm commented Feb 7, 2017 •

edited

Loading