You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Hao Hao <ha...@cloudera.com> on 2016/02/16 21:25:46 UTC
Re: 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/#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. 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
>
>