You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Gregory Chanan <gc...@cloudera.com> on 2016/01/21 21:02:12 UTC
Review Request 42612: SENTRY-989: RealTimeGet with explicit ids can
bypass document level authorization
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42612/
-----------------------------------------------------------
Review request for sentry, Hao Hao and Vamsee Yarlagadda.
Repository: sentry
Description
-------
RealTimeGet just ignores filter queries currently in Solr (see SOLR-8436) which is how document level security is implemented, so if you can guess the document ids, you can access them.
Since we probably don't want to wait for a solr version with SOLR-8436 to be released, this is a "temporary" workaround and some necessary testing.
At a high level this works as follows:
- Run the normal RealTimeGet component
- Filter the responses from the component through the Filter generated from the doc-level component
Most of this is low-level solr/lucene code; most of the meat is in the testing (TestRealTimeGet.java).
Diffs
-----
sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/SecureRealTimeGetHandler.java PRE-CREATION
sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/QueryDocAuthorizationComponent.java 371787df69cfe8ef891eb1760b32f93f0a1110ec
sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SecureRealTimeGetComponent.java PRE-CREATION
sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/AbstractSolrSentryTestBase.java 2495a9eecc00e8de4b297022625b33a98ad7323a
sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/DocLevelGenerator.java PRE-CREATION
sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestDocLevelOperations.java ff508e12898ab0bf9e79f0cc8e1108e4a5ef82ad
sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestRealTimeGet.java PRE-CREATION
sentry-tests/sentry-tests-solr/src/test/resources/solr/collection1/conf/schema.xml 66449ffe59b459352f8a735a208f020e48f0d9b4
sentry-tests/sentry-tests-solr/src/test/resources/solr/collection1/conf/solrconfig-doclevel.xml 4459c0d04c62aa39c096da0faba7ff04fc2bf21b
sentry-tests/sentry-tests-solr/src/test/resources/solr/sentry/test-authz-provider.ini bccc63eeeab503f7e5d3655771eb0c7bef926bba
Diff: https://reviews.apache.org/r/42612/diff/
Testing
-------
Ran the unit tests.
Thanks,
Gregory Chanan
Re: Review Request 42612: SENTRY-989: RealTimeGet with explicit ids
can bypass document level authorization
Posted by Gregory Chanan <gc...@cloudera.com>.
> On Feb. 16, 2016, 8:25 p.m., Hao Hao wrote:
> > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/DocLevelGenerator.java, line 38
> > <https://reviews.apache.org/r/42612/diff/1/?file=1204573#file1204573line38>
> >
> > Could you explain why this is the case?
There's no why, that's just what it does. It's an arbitrary way of generating documents.
> On Feb. 16, 2016, 8:25 p.m., Hao Hao wrote:
> > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SecureRealTimeGetComponent.java, line 117
> > <https://reviews.apache.org/r/42612/diff/1/?file=1204571#file1204571line117>
> >
> > Could you add more comments on this method? It is very long.
added some more comments and restructed a bit. Should be clearer now.
> On Feb. 16, 2016, 8:25 p.m., Hao Hao wrote:
> > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SecureRealTimeGetComponent.java, line 107
> > <https://reviews.apache.org/r/42612/diff/1/?file=1204571#file1204571line107>
> >
> > If user is super user, does not need to add filter? Could you add comments here?
done.
> On Feb. 16, 2016, 8:25 p.m., Hao Hao wrote:
> > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SecureRealTimeGetComponent.java, line 165
> > <https://reviews.apache.org/r/42612/diff/1/?file=1204571#file1204571line165>
> >
> > Spaces.
done.
> On Feb. 16, 2016, 8:25 p.m., Hao Hao wrote:
> > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SecureRealTimeGetComponent.java, line 142
> > <https://reviews.apache.org/r/42612/diff/1/?file=1204571#file1204571line142>
> >
> > Check for null pointer here?
done.
> On Feb. 16, 2016, 8:25 p.m., Hao Hao wrote:
> > sentry-tests/sentry-tests-solr/src/test/resources/solr/collection1/conf/solrconfig-doclevel.xml, line 1364
> > <https://reviews.apache.org/r/42612/diff/1/?file=1204577#file1204577line1364>
> >
> > Remove extra spaces. Same as other places.
cleaned up some; I didn't change this in the commit by the way and these are for the most part, example configs from solr that have the spacing issues.
> On Feb. 16, 2016, 8:25 p.m., Hao Hao wrote:
> > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SecureRealTimeGetComponent.java, line 118
> > <https://reviews.apache.org/r/42612/diff/1/?file=1204571#file1204571line118>
> >
> > How about superusers case in prepare, since there is no adding of AddDocIdReturnFields if are superusers there?
Good point, changed this to a log and added a test.
- Gregory
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42612/#review119348
-----------------------------------------------------------
On Jan. 21, 2016, 8:02 p.m., Gregory Chanan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42612/
> -----------------------------------------------------------
>
> (Updated Jan. 21, 2016, 8:02 p.m.)
>
>
> Review request for sentry, Hao Hao and Vamsee Yarlagadda.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> RealTimeGet just ignores filter queries currently in Solr (see SOLR-8436) which is how document level security is implemented, so if you can guess the document ids, you can access them.
>
> Since we probably don't want to wait for a solr version with SOLR-8436 to be released, this is a "temporary" workaround and some necessary testing.
>
> At a high level this works as follows:
> - Run the normal RealTimeGet component
> - Filter the responses from the component through the Filter generated from the doc-level component
>
> Most of this is low-level solr/lucene code; most of the meat is in the testing (TestRealTimeGet.java).
>
>
> Diffs
> -----
>
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/SecureRealTimeGetHandler.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/QueryDocAuthorizationComponent.java 371787df69cfe8ef891eb1760b32f93f0a1110ec
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SecureRealTimeGetComponent.java PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/AbstractSolrSentryTestBase.java 2495a9eecc00e8de4b297022625b33a98ad7323a
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/DocLevelGenerator.java PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestDocLevelOperations.java ff508e12898ab0bf9e79f0cc8e1108e4a5ef82ad
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestRealTimeGet.java PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/solr/collection1/conf/schema.xml 66449ffe59b459352f8a735a208f020e48f0d9b4
> sentry-tests/sentry-tests-solr/src/test/resources/solr/collection1/conf/solrconfig-doclevel.xml 4459c0d04c62aa39c096da0faba7ff04fc2bf21b
> sentry-tests/sentry-tests-solr/src/test/resources/solr/sentry/test-authz-provider.ini bccc63eeeab503f7e5d3655771eb0c7bef926bba
>
> Diff: https://reviews.apache.org/r/42612/diff/
>
>
> Testing
> -------
>
> Ran the unit tests.
>
>
> Thanks,
>
> Gregory Chanan
>
>
Re: Review Request 42612: SENTRY-989: RealTimeGet with explicit ids
can bypass document level authorization
Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42612/#review119348
-----------------------------------------------------------
sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SecureRealTimeGetComponent.java (line 107)
<https://reviews.apache.org/r/42612/#comment180665>
If user is super user, does not need to add filter? Could you add comments here?
sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SecureRealTimeGetComponent.java (line 117)
<https://reviews.apache.org/r/42612/#comment180661>
Could you add more comments on this method? It is very long.
sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SecureRealTimeGetComponent.java (line 118)
<https://reviews.apache.org/r/42612/#comment180667>
How about superusers case in prepare, since there is no adding of AddDocIdReturnFields if are superusers there?
sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SecureRealTimeGetComponent.java (line 142)
<https://reviews.apache.org/r/42612/#comment180664>
Check for null pointer here?
sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SecureRealTimeGetComponent.java (line 165)
<https://reviews.apache.org/r/42612/#comment180659>
Spaces.
sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/DocLevelGenerator.java (line 38)
<https://reviews.apache.org/r/42612/#comment180663>
Could you explain why this is the case?
sentry-tests/sentry-tests-solr/src/test/resources/solr/collection1/conf/solrconfig-doclevel.xml (line 1364)
<https://reviews.apache.org/r/42612/#comment180660>
Remove extra spaces. Same as other places.
- Hao Hao
On Jan. 21, 2016, 8:02 p.m., Gregory Chanan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42612/
> -----------------------------------------------------------
>
> (Updated Jan. 21, 2016, 8:02 p.m.)
>
>
> Review request for sentry, Hao Hao and Vamsee Yarlagadda.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> RealTimeGet just ignores filter queries currently in Solr (see SOLR-8436) which is how document level security is implemented, so if you can guess the document ids, you can access them.
>
> Since we probably don't want to wait for a solr version with SOLR-8436 to be released, this is a "temporary" workaround and some necessary testing.
>
> At a high level this works as follows:
> - Run the normal RealTimeGet component
> - Filter the responses from the component through the Filter generated from the doc-level component
>
> Most of this is low-level solr/lucene code; most of the meat is in the testing (TestRealTimeGet.java).
>
>
> Diffs
> -----
>
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/SecureRealTimeGetHandler.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/QueryDocAuthorizationComponent.java 371787df69cfe8ef891eb1760b32f93f0a1110ec
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SecureRealTimeGetComponent.java PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/AbstractSolrSentryTestBase.java 2495a9eecc00e8de4b297022625b33a98ad7323a
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/DocLevelGenerator.java PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestDocLevelOperations.java ff508e12898ab0bf9e79f0cc8e1108e4a5ef82ad
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestRealTimeGet.java PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/solr/collection1/conf/schema.xml 66449ffe59b459352f8a735a208f020e48f0d9b4
> sentry-tests/sentry-tests-solr/src/test/resources/solr/collection1/conf/solrconfig-doclevel.xml 4459c0d04c62aa39c096da0faba7ff04fc2bf21b
> sentry-tests/sentry-tests-solr/src/test/resources/solr/sentry/test-authz-provider.ini bccc63eeeab503f7e5d3655771eb0c7bef926bba
>
> Diff: https://reviews.apache.org/r/42612/diff/
>
>
> Testing
> -------
>
> Ran the unit tests.
>
>
> Thanks,
>
> Gregory Chanan
>
>
Re: Review Request 42612: SENTRY-989: RealTimeGet with explicit ids
can bypass document level authorization
Posted by Gregory Chanan <gc...@cloudera.com>.
> On Feb. 19, 2016, 4:07 a.m., Hao Hao wrote:
> > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SecureRealTimeGetComponent.java, line 89
> > <https://reviews.apache.org/r/42612/diff/2/?file=1258921#file1258921line89>
> >
> > So the request will be coming from the server or the end user?
Could be either. Internal requests use "solr", external requests could use "solr", although that's not common.
On Feb. 19, 2016, 4:07 a.m., Gregory Chanan wrote:
> > LGTM, with some minor comments.
will fix.
- Gregory
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42612/#review119799
-----------------------------------------------------------
On Feb. 19, 2016, 2:19 a.m., Gregory Chanan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42612/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2016, 2:19 a.m.)
>
>
> Review request for sentry, Hao Hao and Vamsee Yarlagadda.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> RealTimeGet just ignores filter queries currently in Solr (see SOLR-8436) which is how document level security is implemented, so if you can guess the document ids, you can access them.
>
> Since we probably don't want to wait for a solr version with SOLR-8436 to be released, this is a "temporary" workaround and some necessary testing.
>
> At a high level this works as follows:
> - Run the normal RealTimeGet component
> - Filter the responses from the component through the Filter generated from the doc-level component
>
> Most of this is low-level solr/lucene code; most of the meat is in the testing (TestRealTimeGet.java).
>
>
> Diffs
> -----
>
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/SecureRealTimeGetHandler.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/QueryDocAuthorizationComponent.java 666c0889e07ff329e30773518b28222c8420a510
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SecureRealTimeGetComponent.java PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/AbstractSolrSentryTestBase.java 2495a9eecc00e8de4b297022625b33a98ad7323a
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/DocLevelGenerator.java PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestDocLevelOperations.java ff508e12898ab0bf9e79f0cc8e1108e4a5ef82ad
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestRealTimeGet.java PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/solr/collection1/conf/schema.xml 66449ffe59b459352f8a735a208f020e48f0d9b4
> sentry-tests/sentry-tests-solr/src/test/resources/solr/collection1/conf/solrconfig-doclevel.xml 4459c0d04c62aa39c096da0faba7ff04fc2bf21b
> sentry-tests/sentry-tests-solr/src/test/resources/solr/sentry/test-authz-provider.ini bccc63eeeab503f7e5d3655771eb0c7bef926bba
>
> Diff: https://reviews.apache.org/r/42612/diff/
>
>
> Testing
> -------
>
> Ran the unit tests.
>
>
> Thanks,
>
> Gregory Chanan
>
>
Re: Review Request 42612: SENTRY-989: RealTimeGet with explicit ids
can bypass document level authorization
Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42612/#review119799
-----------------------------------------------------------
Fix it, then Ship it!
sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SecureRealTimeGetComponent.java (line 89)
<https://reviews.apache.org/r/42612/#comment181152>
So the request will be coming from the server or the end user?
sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SecureRealTimeGetComponent.java (line 201)
<https://reviews.apache.org/r/42612/#comment181150>
space after for?
LGTM, with some minor comments.
- Hao Hao
On Feb. 19, 2016, 2:19 a.m., Gregory Chanan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42612/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2016, 2:19 a.m.)
>
>
> Review request for sentry, Hao Hao and Vamsee Yarlagadda.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> RealTimeGet just ignores filter queries currently in Solr (see SOLR-8436) which is how document level security is implemented, so if you can guess the document ids, you can access them.
>
> Since we probably don't want to wait for a solr version with SOLR-8436 to be released, this is a "temporary" workaround and some necessary testing.
>
> At a high level this works as follows:
> - Run the normal RealTimeGet component
> - Filter the responses from the component through the Filter generated from the doc-level component
>
> Most of this is low-level solr/lucene code; most of the meat is in the testing (TestRealTimeGet.java).
>
>
> Diffs
> -----
>
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/SecureRealTimeGetHandler.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/QueryDocAuthorizationComponent.java 666c0889e07ff329e30773518b28222c8420a510
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SecureRealTimeGetComponent.java PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/AbstractSolrSentryTestBase.java 2495a9eecc00e8de4b297022625b33a98ad7323a
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/DocLevelGenerator.java PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestDocLevelOperations.java ff508e12898ab0bf9e79f0cc8e1108e4a5ef82ad
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestRealTimeGet.java PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/solr/collection1/conf/schema.xml 66449ffe59b459352f8a735a208f020e48f0d9b4
> sentry-tests/sentry-tests-solr/src/test/resources/solr/collection1/conf/solrconfig-doclevel.xml 4459c0d04c62aa39c096da0faba7ff04fc2bf21b
> sentry-tests/sentry-tests-solr/src/test/resources/solr/sentry/test-authz-provider.ini bccc63eeeab503f7e5d3655771eb0c7bef926bba
>
> Diff: https://reviews.apache.org/r/42612/diff/
>
>
> Testing
> -------
>
> Ran the unit tests.
>
>
> Thanks,
>
> Gregory Chanan
>
>
Re: Review Request 42612: SENTRY-989: RealTimeGet with explicit ids
can bypass document level authorization
Posted by Gregory Chanan <gc...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42612/
-----------------------------------------------------------
(Updated Feb. 19, 2016, 2:19 a.m.)
Review request for sentry, Hao Hao and Vamsee Yarlagadda.
Changes
-------
Updated for Hao's comments.
Repository: sentry
Description
-------
RealTimeGet just ignores filter queries currently in Solr (see SOLR-8436) which is how document level security is implemented, so if you can guess the document ids, you can access them.
Since we probably don't want to wait for a solr version with SOLR-8436 to be released, this is a "temporary" workaround and some necessary testing.
At a high level this works as follows:
- Run the normal RealTimeGet component
- Filter the responses from the component through the Filter generated from the doc-level component
Most of this is low-level solr/lucene code; most of the meat is in the testing (TestRealTimeGet.java).
Diffs (updated)
-----
sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/SecureRealTimeGetHandler.java PRE-CREATION
sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/QueryDocAuthorizationComponent.java 666c0889e07ff329e30773518b28222c8420a510
sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SecureRealTimeGetComponent.java PRE-CREATION
sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/AbstractSolrSentryTestBase.java 2495a9eecc00e8de4b297022625b33a98ad7323a
sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/DocLevelGenerator.java PRE-CREATION
sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestDocLevelOperations.java ff508e12898ab0bf9e79f0cc8e1108e4a5ef82ad
sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestRealTimeGet.java PRE-CREATION
sentry-tests/sentry-tests-solr/src/test/resources/solr/collection1/conf/schema.xml 66449ffe59b459352f8a735a208f020e48f0d9b4
sentry-tests/sentry-tests-solr/src/test/resources/solr/collection1/conf/solrconfig-doclevel.xml 4459c0d04c62aa39c096da0faba7ff04fc2bf21b
sentry-tests/sentry-tests-solr/src/test/resources/solr/sentry/test-authz-provider.ini bccc63eeeab503f7e5d3655771eb0c7bef926bba
Diff: https://reviews.apache.org/r/42612/diff/
Testing
-------
Ran the unit tests.
Thanks,
Gregory Chanan