[jira] [Comment Edited] (HADOOP-14845) azure getFileStatus not making any auth checks
[ https://issues.apache.org/jira/browse/HADOOP-14845?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16183766#comment-16183766 ] Sivaguru Sankaridurg edited comment on HADOOP-14845 at 9/28/17 1:04 PM: [~steve_l]. branch2-patch.003 passed yetus automated build and test. I tested the code locally against Azure West US. Could you please review and commit the changes? was (Author: sisan...@microsoft.com): [~steve_l]. branch2-patch.003 passed yetus automated build and test. Could you please review and commit the changes? > 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: Sivaguru Sankaridurg > 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, HADOOP-14845-branch-2-003.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 specand 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: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[jira] [Comment Edited] (HADOOP-14845) azure getFileStatus not making any auth checks
[ https://issues.apache.org/jira/browse/HADOOP-14845?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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: ste...@apache.org): 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 specand 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
[jira] [Comment Edited] (HADOOP-14845) azure getFileStatus not making any auth checks
[ https://issues.apache.org/jira/browse/HADOOP-14845?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16166375#comment-16166375 ] Sivaguru Sankaridurg edited comment on HADOOP-14845 at 9/14/17 2:36 PM: Thanks for confirming [~steve_l]. I suspected HADOOP-14520 tooand I ran tests locally on my box by branching at {{13eda5000304099d1145631f9be13ce8a00b600d}} ... and I dont see any failures. {quote}PS E:\g22\hadoop\hadoop-tools\hadoop-azure> git log commit 13eda5000304099d1145631f9be13ce8a00b600d Author: Steve LoughranDate: Thu Sep 7 18:35:03 2017 +0100 HADOOP-14520. WASB: Block compaction for Azure Block Blobs. Contributed by Georgi Chalakov Running org.apache.hadoop.fs.azure.TestNativeAzureFileSystemMocked Tests run: 46, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 19.965 sec - in org.apache.hadoop.fs.azure.TestNativeAzureFileSystemMocked{quote} was (Author: sisan...@microsoft.com): Thanks for confirming [~steve_l]. I tried suspected HADOOP-14520 tooand I ran tests locally on my box by branching at {{13eda5000304099d1145631f9be13ce8a00b600d}} ... and I dont see any failures. {quote}PS E:\g22\hadoop\hadoop-tools\hadoop-azure> git log commit 13eda5000304099d1145631f9be13ce8a00b600d Author: Steve Loughran Date: Thu Sep 7 18:35:03 2017 +0100 HADOOP-14520. WASB: Block compaction for Azure Block Blobs. Contributed by Georgi Chalakov Running org.apache.hadoop.fs.azure.TestNativeAzureFileSystemMocked Tests run: 46, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 19.965 sec - in org.apache.hadoop.fs.azure.TestNativeAzureFileSystemMocked{quote} > 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 >Reporter: Sivaguru Sankaridurg >Assignee: Sivaguru Sankaridurg > Labels: azure, fs, secure, wasb > Attachments: HADOOP-14845.001.patch, HADOOP-14845.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 specand 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: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[jira] [Comment Edited] (HADOOP-14845) azure getFileStatus not making any auth checks
[ https://issues.apache.org/jira/browse/HADOOP-14845?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16166375#comment-16166375 ] Sivaguru Sankaridurg edited comment on HADOOP-14845 at 9/14/17 2:35 PM: Thanks for confirming [~steve_l]. I tried suspected HADOOP-14520 tooand I ran tests locally on my box by branching at {{13eda5000304099d1145631f9be13ce8a00b600d}} ... and I dont see any failures. {quote}PS E:\g22\hadoop\hadoop-tools\hadoop-azure> git log commit 13eda5000304099d1145631f9be13ce8a00b600d Author: Steve LoughranDate: Thu Sep 7 18:35:03 2017 +0100 HADOOP-14520. WASB: Block compaction for Azure Block Blobs. Contributed by Georgi Chalakov Running org.apache.hadoop.fs.azure.TestNativeAzureFileSystemMocked Tests run: 46, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 19.965 sec - in org.apache.hadoop.fs.azure.TestNativeAzureFileSystemMocked{quote} was (Author: sisan...@microsoft.com): Thanks for confirming [~steve_l]. I tried suspected HADOOP-14520 tooand I ran tests locally on my box by branching at {{13eda5000304099d1145631f9be13ce8a00b600d}} ... and I dont see any failures. {{ PS E:\g22\hadoop\hadoop-tools\hadoop-azure> git log commit 13eda5000304099d1145631f9be13ce8a00b600d Author: Steve Loughran Date: Thu Sep 7 18:35:03 2017 +0100 HADOOP-14520. WASB: Block compaction for Azure Block Blobs. Contributed by Georgi Chalakov Running org.apache.hadoop.fs.azure.TestNativeAzureFileSystemMocked Tests run: 46, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 19.965 sec - in org.apache.hadoop.fs.azure.TestNativeAzureFileSystemMocked}} > 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 >Reporter: Sivaguru Sankaridurg >Assignee: Sivaguru Sankaridurg > Labels: azure, fs, secure, wasb > Attachments: HADOOP-14845.001.patch, HADOOP-14845.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 specand 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: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[jira] [Comment Edited] (HADOOP-14845) azure getFileStatus not making any auth checks
[ https://issues.apache.org/jira/browse/HADOOP-14845?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16166375#comment-16166375 ] Sivaguru Sankaridurg edited comment on HADOOP-14845 at 9/14/17 2:34 PM: Thanks for confirming [~steve_l]. I tried suspected HADOOP-14520 tooand I ran tests locally on my box by branching at {{13eda5000304099d1145631f9be13ce8a00b600d}} ... and I dont see any failures. {{ PS E:\g22\hadoop\hadoop-tools\hadoop-azure> git log commit 13eda5000304099d1145631f9be13ce8a00b600d Author: Steve LoughranDate: Thu Sep 7 18:35:03 2017 +0100 HADOOP-14520. WASB: Block compaction for Azure Block Blobs. Contributed by Georgi Chalakov Running org.apache.hadoop.fs.azure.TestNativeAzureFileSystemMocked Tests run: 46, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 19.965 sec - in org.apache.hadoop.fs.azure.TestNativeAzureFileSystemMocked}} was (Author: sisan...@microsoft.com): Thanks for confirming [~steve_l]. I tried suspected HADOOP-14520 tooand I ran tests locally on my box by branching at {{13eda5000304099d1145631f9be13ce8a00b600d}} ... and I dont see any failures. bq. PS E:\g22\hadoop\hadoop-tools\hadoop-azure> git log bq. commit 13eda5000304099d1145631f9be13ce8a00b600d bq. Author: Steve Loughran bq. Date: Thu Sep 7 18:35:03 2017 +0100 bq. bq. HADOOP-14520. WASB: Block compaction for Azure Block Blobs. bq. Contributed by Georgi Chalakov bq. Running org.apache.hadoop.fs.azure.TestNativeAzureFileSystemMocked bq. Tests run: 46, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 19.965 sec - in org.apache.hadoop.fs.azure.TestNativeAzureFileSystemMocked > 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 >Reporter: Sivaguru Sankaridurg >Assignee: Sivaguru Sankaridurg > Labels: azure, fs, secure, wasb > Attachments: HADOOP-14845.001.patch, HADOOP-14845.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 specand 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: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[jira] [Comment Edited] (HADOOP-14845) azure getFileStatus not making any auth checks
[ https://issues.apache.org/jira/browse/HADOOP-14845?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16166118#comment-16166118 ] Sivaguru Sankaridurg edited comment on HADOOP-14845 at 9/14/17 11:43 AM: - [~steve_l], I am seeing these [unit-test errors | https://builds.apache.org/job/PreCommit-HADOOP-Build/13274/artifact/patchprocess/patch-unit-hadoop-tools_hadoop-azure.txt] even without my changes, when I sync to latest on trunk. It looks like there has been a regression .. can you please confirm ? was (Author: sisan...@microsoft.com): [~steve_l], I am seeing these [unit-test errors | https://builds.apache.org/job/PreCommit-HADOOP-Build/13274/artifact/patchprocess/patch-unit-hadoop-tools_hadoop-azure.txt] even without my changes, when I sync to HEAD on trunk. It looks like there has been a regression .. can you please confirm ? > 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 >Reporter: Sivaguru Sankaridurg >Assignee: Sivaguru Sankaridurg > Labels: azure, fs, secure, wasb > Attachments: HADOOP-14845.001.patch, HADOOP-14845.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 specand 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: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org