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

Not allowed perform sensitive operations via gremlin #176

Merged
merged 17 commits into from
Jul 29, 2019
Merged

Conversation

Linary
Copy link
Contributor

@Linary Linary commented Nov 6, 2018

Implement #145

Change-Id: I9a590fe40d3b5a808b569ed0af8fd83214a2941a

@codecov
Copy link

codecov bot commented Mar 15, 2019

Codecov Report

Merging #176 into master will increase coverage by 0.48%.
The diff coverage is 91.03%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #176      +/-   ##
============================================
+ Coverage     71.42%   71.91%   +0.48%     
- Complexity     3527     3617      +90     
============================================
  Files           216      218       +2     
  Lines         16657    16802     +145     
  Branches       2379     2389      +10     
============================================
+ Hits          11898    12083     +185     
+ Misses         3509     3464      -45     
- Partials       1250     1255       +5
Impacted Files Coverage Δ Complexity Δ
.../java/com/baidu/hugegraph/version/CoreVersion.java 80% <ø> (ø) 2 <0> (ø) ⬇️
...aidu/hugegraph/exception/HugeGremlinException.java 100% <100%> (ø) 3 <3> (?)
.../baidu/hugegraph/security/HugeSecurityManager.java 90.57% <90.57%> (ø) 68 <68> (?)
...a/com/baidu/hugegraph/backend/query/Condition.java 65.07% <0%> (-0.53%) 23% <0%> (ø)
...du/hugegraph/backend/tx/GraphIndexTransaction.java 79.68% <0%> (+0.15%) 145% <0%> (+1%) ⬆️
...va/com/baidu/hugegraph/backend/cache/RamCache.java 72.54% <0%> (+0.39%) 45% <0%> (+1%) ⬆️
...n/java/com/baidu/hugegraph/task/TaskScheduler.java 53.52% <0%> (+1.24%) 27% <0%> (+1%) ⬆️
...ain/java/com/baidu/hugegraph/task/TaskManager.java 66.66% <0%> (+1.44%) 13% <0%> (+2%) ⬆️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3fcfce2...9bc5c20. Read the comment docs.

@javeme
Copy link
Contributor

javeme commented Jun 3, 2019

only check in the gremlin threads, also consider asynchronous gremlin job

@Linary Linary force-pushed the security-manager branch 2 times, most recently from 5778b9a to 08d01f7 Compare June 24, 2019 05:07
@Linary Linary requested a review from javeme June 24, 2019 06:14
@Linary Linary force-pushed the security-manager branch 2 times, most recently from f6fde25 to 1cc250c Compare July 2, 2019 08:17
@Linary Linary force-pushed the security-manager branch 2 times, most recently from b06a5fc to 966726f Compare July 11, 2019 03:31
@Linary Linary force-pushed the security-manager branch 4 times, most recently from 859f530 to a7e1bd4 Compare July 23, 2019 03:05
Linary added 14 commits July 23, 2019 19:54
Implement #145

Change-Id: I9a590fe40d3b5a808b569ed0af8fd83214a2941a
Change-Id: I10faac1d2b5a0eeb925961c3a2d3287d45a7271c
Change-Id: I5d9e4b97aa7da9603c9654f35610c6d15a0dc485
Change-Id: I5af3b2b23b2c4d2b0acd68773e13af96bf349735
Change-Id: I53371c58257d28d2e1f6e567c0e790428c35576b
Change-Id: Ic76795aca51685289546149509492fe9b1eb3736
Change-Id: Ie4ebf0f499434a8d561ff56611cbf1554ba4049a
Change-Id: I9e73542272adb9ee6e3a4fdad1b60ce47747ed5b
Change-Id: I8db5dbb14470db6e357e3f442e79c625343fa9a1
Change-Id: Ia9ace74728757140d2de8596bf3695f4e018493a
Change-Id: I68d699273422d49d599fd674b393c2b19451b4ba
Change-Id: I20b149ca2fbf7695e03c8d19cbd1279be6041e5a
Change-Id: Ie18b2226f2b0f2587860e8d4e233a6f528acb2c5
Change-Id: Id42b8d4fd5cd4ce487fd880ddb32de7a07a9cc36
Change-Id: I96b88d3fe78062b8951a21e3022219f7bdf4f2bc
Change-Id: I3e175559bfc0efb3c5beea794b0a6b7b19d63e50
}

@Override
public void checkPackageAccess(String pkg) {
Copy link
Contributor

Choose a reason for hiding this comment

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

seems redundant. why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

just override all methods explicitly, avoid dangers caused by omissions


@Test
public void testExec() throws IOException {
Runtime.getRuntime().exec("ls");
Copy link
Contributor

Choose a reason for hiding this comment

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

same command is better

Change-Id: Iec95172d2fe97613eb210aacb2a0843145df2f9c
@zhoney zhoney merged commit b01549a into master Jul 29, 2019
@zhoney zhoney deleted the security-manager branch July 29, 2019 06:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants