Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-4765

Permission check of symlink deletion incorrectly throws UnresolvedLinkException

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.0.0, 2.0.3-alpha
    • Fix Version/s: 2.1.0-beta, 0.23.9
    • Component/s: namenode
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      With permissions enabled, the permission check in FSNamesystem#delete will incorrectly throw an UnresolvedLinkException if the path contains a symlink. This leads to FileContext resolving the symlink and instead deleting the link target.

      The correct check is to see if the user has write permissions on the parent directory of the symlink, e.g.

      -> % ls -ld symtest
      drwxr-xr-x 2 root root 4096 Apr 26 14:12 symtest
      -> % ls -l symtest
      total 12
      lrwxrwxrwx 1 root root 6 Apr 26 14:12 link -> target
      -rw-r--r-- 1 root root 0 Apr 26 14:11 target
      -> % rm -f symtest/link
      rm: cannot remove `symtest/link': Permission denied
      -> % sudo chown andrew symtest
      -> % rm -f symtest/link       
      -> % 
      
      1. hdfs-4765-branch-2-1.patch
        11 kB
        Andrew Wang
      2. hdfs-4765-branch-0.23.patch
        10 kB
        Jason Lowe
      3. hdfs-4765-2.patch
        11 kB
        Andrew Wang
      4. hdfs-4765-1.patch
        9 kB
        Andrew Wang

        Issue Links

          Activity

          Hide
          Andrew Wang added a comment -

          Patch attached. In the case of delete, we call the permission check without resolving a symlink if it's the last component.

          Verified fail before/pass after behavior with the included unit test.

          Show
          Andrew Wang added a comment - Patch attached. In the case of delete, we call the permission check without resolving a symlink if it's the last component. Verified fail before/pass after behavior with the included unit test.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12581095/hdfs-4765-1.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 1 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/4341//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/4341//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12581095/hdfs-4765-1.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/4341//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/4341//console This message is automatically generated.
          Hide
          Aaron T. Myers added a comment -

          Patch looks pretty good to me, Andrew, though it no longer applies to trunk after the snapshot merge so it will need to be rebased somewhat. Two little suggestions:

          1. Recommend you use the two argument versions of assertTrue and assertFalse which take error messages to display in the event the assertion fails.
          2. Though not introduced by this patch, I notice that the indentation in FSPermissionChecker#checkPermission is off. Mind fixing that while you're in there?

          +1 once these are addressed.

          Show
          Aaron T. Myers added a comment - Patch looks pretty good to me, Andrew, though it no longer applies to trunk after the snapshot merge so it will need to be rebased somewhat. Two little suggestions: Recommend you use the two argument versions of assertTrue and assertFalse which take error messages to display in the event the assertion fails. Though not introduced by this patch, I notice that the indentation in FSPermissionChecker#checkPermission is off. Mind fixing that while you're in there? +1 once these are addressed.
          Hide
          Andrew Wang added a comment -

          Thanks for the review ATM. I addressed your comments, and did a couple other small cleanups too.

          Show
          Andrew Wang added a comment - Thanks for the review ATM. I addressed your comments, and did a couple other small cleanups too.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12582823/hdfs-4765-2.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 1 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/4385//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/4385//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12582823/hdfs-4765-2.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/4385//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/4385//console This message is automatically generated.
          Hide
          Aaron T. Myers added a comment -

          +1, the patch looks good to me. I'm going to commit this momentarily.

          Show
          Aaron T. Myers added a comment - +1, the patch looks good to me. I'm going to commit this momentarily.
          Hide
          Aaron T. Myers added a comment -

          I've committed this to trunk, but, perhaps obviously, it doesn't apply cleanly to branch-2 since snapshots has not yet been merged there. Mind posting a separate branch-2 patch, Andrew?

          Show
          Aaron T. Myers added a comment - I've committed this to trunk, but, perhaps obviously, it doesn't apply cleanly to branch-2 since snapshots has not yet been merged there. Mind posting a separate branch-2 patch, Andrew?
          Hide
          Hudson added a comment -

          Integrated in Hadoop-trunk-Commit #3746 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3746/)
          HDFS-4765. Permission check of symlink deletion incorrectly throws UnresolvedLinkException. Contributed by Andrew Wang. (Revision 1481976)

          Result = SUCCESS
          atm : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1481976
          Files :

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/security/TestPermission.java
          Show
          Hudson added a comment - Integrated in Hadoop-trunk-Commit #3746 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3746/ ) HDFS-4765 . Permission check of symlink deletion incorrectly throws UnresolvedLinkException. Contributed by Andrew Wang. (Revision 1481976) Result = SUCCESS atm : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1481976 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/security/TestPermission.java
          Hide
          Andrew Wang added a comment -

          Thanks ATM. Here's a branch-2 patch. Tested via running TestPermission.

          Show
          Andrew Wang added a comment - Thanks ATM. Here's a branch-2 patch. Tested via running TestPermission .
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12582959/hdfs-4765-branch-2-1.patch
          against trunk revision .

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/4387//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12582959/hdfs-4765-branch-2-1.patch against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/4387//console This message is automatically generated.
          Hide
          Aaron T. Myers added a comment -

          Thanks a lot, Andrew. I've just committed this trunk and branch-2.

          Show
          Aaron T. Myers added a comment - Thanks a lot, Andrew. I've just committed this trunk and branch-2.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Yarn-trunk #209 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/209/)
          HDFS-4765. Permission check of symlink deletion incorrectly throws UnresolvedLinkException. Contributed by Andrew Wang. (Revision 1481976)

          Result = SUCCESS
          atm : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1481976
          Files :

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/security/TestPermission.java
          Show
          Hudson added a comment - Integrated in Hadoop-Yarn-trunk #209 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/209/ ) HDFS-4765 . Permission check of symlink deletion incorrectly throws UnresolvedLinkException. Contributed by Andrew Wang. (Revision 1481976) Result = SUCCESS atm : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1481976 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/security/TestPermission.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #1398 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1398/)
          HDFS-4765. Permission check of symlink deletion incorrectly throws UnresolvedLinkException. Contributed by Andrew Wang. (Revision 1481976)

          Result = FAILURE
          atm : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1481976
          Files :

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/security/TestPermission.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1398 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1398/ ) HDFS-4765 . Permission check of symlink deletion incorrectly throws UnresolvedLinkException. Contributed by Andrew Wang. (Revision 1481976) Result = FAILURE atm : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1481976 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/security/TestPermission.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #1425 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1425/)
          HDFS-4765. Permission check of symlink deletion incorrectly throws UnresolvedLinkException. Contributed by Andrew Wang. (Revision 1481976)

          Result = SUCCESS
          atm : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1481976
          Files :

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/security/TestPermission.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1425 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1425/ ) HDFS-4765 . Permission check of symlink deletion incorrectly throws UnresolvedLinkException. Contributed by Andrew Wang. (Revision 1481976) Result = SUCCESS atm : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1481976 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/security/TestPermission.java
          Hide
          Jason Lowe added a comment -

          Patch for branch-0.23

          Show
          Jason Lowe added a comment - Patch for branch-0.23
          Hide
          Daryn Sharp added a comment -

          +1 to 23 patch. looks like a straightforward back-port.

          Show
          Daryn Sharp added a comment - +1 to 23 patch. looks like a straightforward back-port.
          Hide
          Jason Lowe added a comment -

          Thanks for the review, Daryn, and thanks for the original code, Andrew! I committed this to branch-0.23.

          Show
          Jason Lowe added a comment - Thanks for the review, Daryn, and thanks for the original code, Andrew! I committed this to branch-0.23.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-0.23-Build #646 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/646/)
          HDFS-4765. Permission check of symlink deletion incorrectly throws UnresolvedLinkException. Contributed by Andrew Wang (Revision 1495603)

          Result = SUCCESS
          jlowe : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1495603
          Files :

          • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
          • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java
          • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/security/TestPermission.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-0.23-Build #646 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/646/ ) HDFS-4765 . Permission check of symlink deletion incorrectly throws UnresolvedLinkException. Contributed by Andrew Wang (Revision 1495603) Result = SUCCESS jlowe : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1495603 Files : /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/security/TestPermission.java

            People

            • Assignee:
              Andrew Wang
              Reporter:
              Andrew Wang
            • Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development