[
https://issues.apache.org/jira/browse/HADOOP-14845?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16171869#comment-16171869
]
Steve Loughran edited comment on HADOOP-14845 at 9/19/17 3:15 PM:
------------------------------------------------------------------
I'm afraid the HADOOP-14553 test changes have broken this going into trunk
right now. Sorry: you'll appreciate the changes once you get used to them;
testing_azure.md covers the changes. For now, how about we get the patch into
branch-2 and then move that forward into trunk?
Overall *roduction code & test changes looks good; just some tuning and it'll
be ready
h3. {{NativeAzureFileSystem}}
* add some minimal javadocs to {{existsInternal}}, mentions calls
getFileStatusInternal so has same costs *And skips {{the auth checks}}
* {{createInternal}} needs some javadocs too
* If these two methods aren't to be overridden/used directly by any (mock)
subclass, mark them as private for now
h3.
{{TestNativeAzureFileSystemAuthorization.testRenamePendingAuthorizationCalls}}
* move the {{catch}} on L947 and {{finally}} on L955 to the same lines as the
preceeding }
* L958 {{ContractTestUtils.assertTrue}} is just {{Assert.assertTrue}}, so can
be used directly. However here, as its an exception which is being examined,
use {{GenericTestUtils.assertExceptionContains(srcPath.toString, fnfe)}}: it'll
rethrow the exception if there's no match. And don't worry about the rest of
the text as that makes for a more brittle comparision.
* L956 typo "does not exits"
was (Author: [email protected]):
I'm afraid the HADOOP-14553 test changes have broken this going into trunk
right now. Sorry: you'll appreciate the changes once you get used to them;
testing_azure.md covers the changes. For now, how about we get the patch into
branch-2 and then move that forward into trunk?
Overall *roduction code & test changes looks good; just some tuning and it'll
be ready
h3. {{NativeAzureFileSystem}}
* add some minimal javadocs to {{existsInternal}}, mentions calls
getFileStatusInternal so has same costs *And skips {{the auth checks}}
* {{createInternal}} needs some javadocs too
* If these two methods aren't to be overridden/used directly by any (mock)
subclass, mark them as private for now
h3.
{{TestNativeAzureFileSystemAuthorization.testRenamePendingAuthorizationCalls}}
* move the {{catch}} on L947 and {{finally}} on L955 to the same lines as the
preceeding }
* L958 {{ContractTestUtils.assertTrue}} is just {{Assert.assertTrue}}, so can
be used directly. However here, as its an exception which is being examined,
use {{GenericTestUtils.assertExceptionContains(srcPath.toString, fnfe)}}: it'll
rethrow the exception if there's no match. And don't worry about the rest of
the text as
that makes for a more brittle comparision.
* L956 typo "does not exits"
> azure getFileStatus not making any auth checks
> ----------------------------------------------
>
> Key: HADOOP-14845
> URL: https://issues.apache.org/jira/browse/HADOOP-14845
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs/azure, security
> Affects Versions: 2.8.0, 2.7.4
> Reporter: Sivaguru Sankaridurg
> Assignee: Steve Loughran
> Labels: azure, fs, secure, wasb
> Attachments: HADOOP-14845.001.patch, HADOOP-14845.002.patch,
> HADOOP-14845.003.patch, HADOOP-14845-branch-2-001.patch.txt,
> HADOOP-14845-branch-2-002.patch
>
>
> The HDFS spec requires only traverse checks for any file accessed via
> getFileStatus ... and since WASB does not support traverse checks, removing
> this call effectively removed all protections for the getFileStatus call. The
> reasoning at that time was that doing a performAuthCheck was the wrong thing
> to do, since it was going against the spec....and that the correct fix to the
> getFileStatus issue was to implement traverse checks rather than go against
> the spec by calling performAuthCheck. The side-effects of such a change were
> not fully clear at that time, but the thinking was that it was safer to
> remain true to the spec, as far as possible.
> The reasoning remains correct even today. But in view of the security hole
> introduced by this change (that anyone can load up any other user's data in
> hive), and keeping in mind that WASB does not intend to implement traverse
> checks, we propose a compromise.
> We propose (re)introducing a read-access check to getFileStatus(), that would
> check the existing ancestor for read-access whenever invoked. Although not
> perfect (in that it is a departure from the spec), we believe that it is a
> good compromise between having no checks at all; and implementing full-blown
> traverse checks.
> For scenarios that deal with intermediate folders like mkdirs, the call would
> check for read access against an existing ancestor (when invoked from shell)
> for intermediate non-existent folders – {{ mkdirs /foo/bar, where only "/"
> exists, would result in read-checks against "/" for "/","/foo" and "/foo/bar"
> }}. This can be thought of, as being a close-enough substitute for the
> traverse checks that hdfs does.
> For other scenarios that don't deal with non-existent intermediate folders –
> like read, delete etc, the check will happen against the parent. Once again,
> we can think of the read-check against the parent as a substitute for the
> traverse check, which can be customized for various users with ranger
> policies.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
