Uploaded image for project: 'Hadoop Common'
  1. Hadoop Common
  2. HADOOP-2543

No-permission-checking mode for smooth transition to 0.16's permissions features.

    Details

    • Type: New Feature
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.15.1
    • Fix Version/s: 0.16.0
    • Component/s: None
    • Labels:
      None

      Description

      In moving to 0.16, which will support permissions, a mode of no-permission checking has been proposed to allow smooth transition to using the new permissions feature.
      The idea is that at first 0.16 will be used for a period of time with permission checking off.
      Later after the admin has changed ownership and permissions of various files, the permission checking can be turned off.

      This Jira defines what the semantics are of the no-permission-checking mode.

      1. permissionRelated.patch
        8 kB
        Hairong Kuang
      2. permissionRelated1.patch
        8 kB
        Hairong Kuang

        Activity

        Hide
        sanjay.radia Sanjay Radia added a comment -

        Proposal:
        When no-permission-checking mode is set:
        a) No checking of permissions when files are created, accessed or deleted.

        • this allows for backward compatibility when there were no permissions
          b) The newly added operations, setOwner and setPermission (lets you do the equivalent of chmod, chown and chgroup operations)
          will check that the the user who is changing these properties is indeed the owner (or is super user); the system will preserve the permissions and ownership that
          are set
        • these are new operations and so there is no compatibility issue.
        • allows the admin as a super user to change the owner and permissions to the actual owner user and not worry about some one unauthorized
          re-changing the ownership. The same applies to any ownership and mode set at upgrade time.

        c) Newly created files and directories will be owned by the user that creates those files.

        • this has the advantage that files created on project directories (i.e outside of the home directories) will have the creator's user name recorded and hence this
          will assist the admin to determine ownership of shared project files during the transition phase.
          d) stat (and ls -l) will show the actual recorded owner and permission of the file (even though all permission checks are ignored)

        In addition, the admin can turn-on permission-checking and later turn-off permission checking again if problems are observed.
        Again ownership and permissions set during the time mode was turned on are preserved.

        Show
        sanjay.radia Sanjay Radia added a comment - Proposal: When no-permission-checking mode is set: a) No checking of permissions when files are created, accessed or deleted. this allows for backward compatibility when there were no permissions b) The newly added operations, setOwner and setPermission (lets you do the equivalent of chmod, chown and chgroup operations) will check that the the user who is changing these properties is indeed the owner (or is super user); the system will preserve the permissions and ownership that are set these are new operations and so there is no compatibility issue. allows the admin as a super user to change the owner and permissions to the actual owner user and not worry about some one unauthorized re-changing the ownership. The same applies to any ownership and mode set at upgrade time. c) Newly created files and directories will be owned by the user that creates those files. this has the advantage that files created on project directories (i.e outside of the home directories) will have the creator's user name recorded and hence this will assist the admin to determine ownership of shared project files during the transition phase. d) stat (and ls -l) will show the actual recorded owner and permission of the file (even though all permission checks are ignored) In addition, the admin can turn-on permission-checking and later turn-off permission checking again if problems are observed. Again ownership and permissions set during the time mode was turned on are preserved.
        Hide
        cutting Doug Cutting added a comment -

        How does this differ from the way that dfs.permissions=false works already? Is this a documentation issue, or are there functional changes required?

        Show
        cutting Doug Cutting added a comment - How does this differ from the way that dfs.permissions=false works already? Is this a documentation issue, or are there functional changes required?
        Hide
        hairong Hairong Kuang added a comment -

        a) and c) have already been implemented in HADOOP-1298. I will work on b). In addition, I'd like to make some change to the upgrade.
        1) all files and directories will be owned by the super user and super group;
        2) the permission of the files is set to be 0600 and the permission of the directories is set to be 0700.
        These changes make sure that files/directories won't be accessed by illegal users even when an administrator forget to change their permissions when permission checking is turned on.

        Show
        hairong Hairong Kuang added a comment - a) and c) have already been implemented in HADOOP-1298 . I will work on b). In addition, I'd like to make some change to the upgrade. 1) all files and directories will be owned by the super user and super group; 2) the permission of the files is set to be 0600 and the permission of the directories is set to be 0700. These changes make sure that files/directories won't be accessed by illegal users even when an administrator forget to change their permissions when permission checking is turned on.
        Hide
        hairong Hairong Kuang added a comment -

        > How does this differ from the way that dfs.permissions=false works already? Is this a documentation issue, or are there functional changes required?
        Currently when dfs.permisssion=false, setOwner and setPermission does not check permission either. The current proposal is that even when dfs.permission is equal to false, setOwner and setPermission should be permission checked. This assumes the following use case for 0.16.0.
        1, First dfs is upgraded to 0.16.0. Permission checking is turned off;
        2. The admin changes file permissions and ownership while users run their jobs the same as in 0.15.
        3. The admin shuts down the cluster, turns on the permission checking, and brings up the cluster again.
        4. Now users' access to some data may be denied.

        The proposal allows that only the admin and file owners make changes to file ownership and permissions when permission checking is turned off.

        Show
        hairong Hairong Kuang added a comment - > How does this differ from the way that dfs.permissions=false works already? Is this a documentation issue, or are there functional changes required? Currently when dfs.permisssion=false, setOwner and setPermission does not check permission either. The current proposal is that even when dfs.permission is equal to false, setOwner and setPermission should be permission checked. This assumes the following use case for 0.16.0. 1, First dfs is upgraded to 0.16.0. Permission checking is turned off; 2. The admin changes file permissions and ownership while users run their jobs the same as in 0.15. 3. The admin shuts down the cluster, turns on the permission checking, and brings up the cluster again. 4. Now users' access to some data may be denied. The proposal allows that only the admin and file owners make changes to file ownership and permissions when permission checking is turned off.
        Hide
        cutting Doug Cutting added a comment -

        > 1) all files and directories will be owned by the super user and super group;

        That seems fine.

        > 2) the permission of the files is set to be 0600 and the permission of the directories is set to be 0700.

        The use of dfs.permissions=false should be optional, no? Folks should be able to upgrade and use the filesystem as before, but this would break that. The default protection after upgrade should continue to be 777 and that folks should need to explicitly tighten permissions rather than explicitly loosen them.

        Show
        cutting Doug Cutting added a comment - > 1) all files and directories will be owned by the super user and super group; That seems fine. > 2) the permission of the files is set to be 0600 and the permission of the directories is set to be 0700. The use of dfs.permissions=false should be optional, no? Folks should be able to upgrade and use the filesystem as before, but this would break that. The default protection after upgrade should continue to be 777 and that folks should need to explicitly tighten permissions rather than explicitly loosen them.
        Hide
        hairong Hairong Kuang added a comment -

        > folks should need to explicitly tighten permissions rather than explicitly loosen them.

        Explicitly tightening them is more backwards compatible, but from the security point of view, explicitly loossening them is safer.

        Show
        hairong Hairong Kuang added a comment - > folks should need to explicitly tighten permissions rather than explicitly loosen them. Explicitly tightening them is more backwards compatible, but from the security point of view, explicitly loossening them is safer.
        Hide
        cutting Doug Cutting added a comment -

        > Explicitly tightening them is more backwards compatible, but from the security point of view, explicitly loosening them is safer.

        Yes, and for this upgrade, back-compatibility is more important than immediately increasing security. We don't decrease security any, and folks can easily increase security after the upgrade by tightening permissions. But we don't want things to be broken as soon as they upgrade by automatically tightening permissions.

        What I'm proposing is essentially the use-case you describe above for using dfs.permission=false, but without setting that: after the upgrade everything is permitted, and folks can start restricting access, but without having to restart the cluster. I think for most sites this is simpler, less surprising and sufficient.

        Show
        cutting Doug Cutting added a comment - > Explicitly tightening them is more backwards compatible, but from the security point of view, explicitly loosening them is safer. Yes, and for this upgrade, back-compatibility is more important than immediately increasing security. We don't decrease security any, and folks can easily increase security after the upgrade by tightening permissions. But we don't want things to be broken as soon as they upgrade by automatically tightening permissions. What I'm proposing is essentially the use-case you describe above for using dfs.permission=false, but without setting that: after the upgrade everything is permitted, and folks can start restricting access, but without having to restart the cluster. I think for most sites this is simpler, less surprising and sufficient.
        Hide
        aw Allen Wittenauer added a comment -

        I disagree. Permissions are a security feature. It is much better to be safe than backwards compatible when implementing security features.

        From an ops perspective, this is the way in which I see the upgrade happening:

        1. upgrade to 0.16 w/perms off.
        2. use chown and chmod manually set to set proper ownership and perms
        3. turn perms on.

        There is significant risk that files may get missed during step 2. It is MUCH safer from a security perspective to set the default perms to x00 than x77. This is especially important given that there is no way to audit who is accessing what files in Hadoop. It is a good thing if users notice that they lost access to files they previously had access to--it means the permissions are either incorrect, got missed, or the file perms need to be re-evaluated.

        Additionally, setting x77 means that there is a potential window where missed files can be co-opted by someone who shouldn't have them.

        That said: all those requiring backwards compatibility should just keep perms turned off.

        Show
        aw Allen Wittenauer added a comment - I disagree. Permissions are a security feature. It is much better to be safe than backwards compatible when implementing security features. From an ops perspective, this is the way in which I see the upgrade happening: 1. upgrade to 0.16 w/perms off. 2. use chown and chmod manually set to set proper ownership and perms 3. turn perms on. There is significant risk that files may get missed during step 2. It is MUCH safer from a security perspective to set the default perms to x00 than x77. This is especially important given that there is no way to audit who is accessing what files in Hadoop. It is a good thing if users notice that they lost access to files they previously had access to--it means the permissions are either incorrect, got missed, or the file perms need to be re-evaluated. Additionally, setting x77 means that there is a potential window where missed files can be co-opted by someone who shouldn't have them. That said: all those requiring backwards compatibility should just keep perms turned off.
        Hide
        cutting Doug Cutting added a comment -

        > setting x77 means that there is a potential window where missed files can be co-opted by someone who shouldn't have them.

        Like all files are today? I don't follow. We currently have zero security. The security we're adding in this release is easy to subvert and mostly to keep folks from shooting themselves in the foot. Keeping the "window" that's wide open today open a bit longer doesn't significantly compromise anything.

        > all those requiring backwards compatibility should just keep perms turned off.

        We want folks to be able to upgrade, then use new features, without jumping through hoops. Hoops should be optional. If you wish to be able to configure a non-777 permission for after upgrade, that would be a reasonable feature, but 777 should be the default.

        So perhaps we need a dfs.initial.permission parameter, used by the upgrade, whose default value is 777, but that you can override along with setting dfs.permissions=false, to support the upgrade procedure you desire. But I don't think we should force all installations through that procedure in order to get a usable system. We know from experience that most folks just install the new version and expect things to work out of the box. When they don't they file bugs.

        Show
        cutting Doug Cutting added a comment - > setting x77 means that there is a potential window where missed files can be co-opted by someone who shouldn't have them. Like all files are today? I don't follow. We currently have zero security. The security we're adding in this release is easy to subvert and mostly to keep folks from shooting themselves in the foot. Keeping the "window" that's wide open today open a bit longer doesn't significantly compromise anything. > all those requiring backwards compatibility should just keep perms turned off. We want folks to be able to upgrade, then use new features, without jumping through hoops. Hoops should be optional. If you wish to be able to configure a non-777 permission for after upgrade, that would be a reasonable feature, but 777 should be the default. So perhaps we need a dfs.initial.permission parameter, used by the upgrade, whose default value is 777, but that you can override along with setting dfs.permissions=false, to support the upgrade procedure you desire. But I don't think we should force all installations through that procedure in order to get a usable system. We know from experience that most folks just install the new version and expect things to work out of the box. When they don't they file bugs.
        Hide
        rangadi Raghu Angadi added a comment -

        > So perhaps we need a dfs.initial.permission parameter, used by the upgrade, whose default value is 777,
        +1. I think there was a talk of another config for default user/group during the upgrade.

        Show
        rangadi Raghu Angadi added a comment - > So perhaps we need a dfs.initial.permission parameter, used by the upgrade, whose default value is 777, +1. I think there was a talk of another config for default user/group during the upgrade.
        Hide
        hairong Hairong Kuang added a comment -

        The patch added a "secret" configuration parameter "dfs.upgrade.permission" with a default value of 0777. If an administrator wants to use a different value for the initial permission, he/she can set it in hadoop-site.xml.

        Show
        hairong Hairong Kuang added a comment - The patch added a "secret" configuration parameter "dfs.upgrade.permission" with a default value of 0777. If an administrator wants to use a different value for the initial permission, he/she can set it in hadoop-site.xml.
        Hide
        szetszwo Tsz Wo Nicholas Sze added a comment -

        Codes look good. I only have a comment that FSNamesystem.defaultPermission should better be private or final.

        Show
        szetszwo Tsz Wo Nicholas Sze added a comment - Codes look good. I only have a comment that FSNamesystem.defaultPermission should better be private or final.
        Hide
        hairong Hairong Kuang added a comment -

        The patch made the defaultPermission to be private.

        Show
        hairong Hairong Kuang added a comment - The patch made the defaultPermission to be private.
        Hide
        szetszwo Tsz Wo Nicholas Sze added a comment -

        +1

        Show
        szetszwo Tsz Wo Nicholas Sze added a comment - +1
        Hide
        hadoopqa Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12373320/permissionRelated1.patch
        against trunk revision r612913.

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

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

        javac +1. The applied patch does not generate any new compiler warnings.

        findbugs +1. The patch does not introduce any new Findbugs warnings.

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

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

        Test results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1623/testReport/
        Findbugs warnings: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1623/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1623/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1623/console

        This message is automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12373320/permissionRelated1.patch against trunk revision r612913. @author +1. The patch does not contain any @author tags. javadoc +1. The javadoc tool did not generate any warning messages. javac +1. The applied patch does not generate any new compiler warnings. findbugs +1. The patch does not introduce any new Findbugs warnings. core tests +1. The patch passed core unit tests. contrib tests +1. The patch passed contrib unit tests. Test results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1623/testReport/ Findbugs warnings: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1623/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1623/artifact/trunk/build/test/checkstyle-errors.html Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1623/console This message is automatically generated.
        Hide
        dhruba dhruba borthakur added a comment -

        I just committed this. Thanks Hairong!

        Show
        dhruba dhruba borthakur added a comment - I just committed this. Thanks Hairong!

          People

          • Assignee:
            hairong Hairong Kuang
            Reporter:
            sanjay.radia Sanjay Radia
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development