[GitHub] accumulo pull request #222: ACCUMULO-2806: changed permissions of /accumulo ...

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #222: ACCUMULO-2806: changed permissions of /accumulo ...

ctubbsii
GitHub user milleruntime opened a pull request:

    https://github.com/apache/accumulo/pull/222

    ACCUMULO-2806: changed permissions of /accumulo to 700

    Simple fix for ACCUMULO-2806.  I had overcomplicated the issue by attempting to set the permissions independently for each directory under /accumulo but I think just securing permissions at the top will work fine.
   
    My question for the community, will this change break/complicate 3rd party tools?

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/milleruntime/accumulo ACCUMULO-2806

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/accumulo/pull/222.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #222
   
----
commit e89c95f54d3e70b541c800c982e0a6fa5220b554
Author: Mike Miller <[hidden email]>
Date:   2017-02-24T20:32:36Z

    ACCUMULO-2806: changed permissions of /accumulo to 700

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo issue #222: ACCUMULO-2806: changed permissions of /accumulo to 700

ctubbsii
Github user joshelser commented on the issue:

    https://github.com/apache/accumulo/pull/222
 
    > My question for the community, will this change break/complicate 3rd party tools?
   
    I am A-OK with this "breaking" any 3rd party tools. There should be no need for applications to read the "raw" data stored under `/accumulo`. If someone was expecting this, they can do the `-chown` as a reminder that they're allowing this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo issue #222: ACCUMULO-2806: changed permissions of /accumulo to 700

ctubbsii
In reply to this post by ctubbsii
Github user joshelser commented on the issue:

    https://github.com/apache/accumulo/pull/222
 
    Also, @milleruntime can you scope out our tests to see if we have these cases covered with HDFS permissions:
   
    * Bulk load of files into Accumulo
    * ExportTable
   
    We should be able to set up a secure-enough MiniDfsCluster that will enforce fs permissions. These are the two cases that come to mind in which "things" reach into `/accumulo`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo issue #222: ACCUMULO-2806: changed permissions of /accumulo to 700

ctubbsii
In reply to this post by ctubbsii
Github user busbey commented on the issue:

    https://github.com/apache/accumulo/pull/222
 
    Does the shell work after this change? (do we have a test for the shell working?)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo issue #222: ACCUMULO-2806: changed permissions of /accumulo to 700

ctubbsii
In reply to this post by ctubbsii
Github user milleruntime commented on the issue:

    https://github.com/apache/accumulo/pull/222
 
    > I am A-OK with this "breaking" any 3rd party tools. There should be no need for applications to read the "raw" data stored under /accumulo. If someone was expecting this, they can do the -chown as a reminder that they're allowing this.
   
    I agree. This change enforces proper behavior while also securing internal files.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo issue #222: ACCUMULO-2806: changed permissions of /accumulo to 700

ctubbsii
In reply to this post by ctubbsii
Github user milleruntime commented on the issue:

    https://github.com/apache/accumulo/pull/222
 
    > Does the shell work after this change? (do we have a test for the shell working?)
   
    @busbey Yes and yes.  I tested basic shell functionality (create table, insert, scan, create user, grant) manually using Uno.  It also passed ShellServerIT and ShellConfigIT


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo issue #222: ACCUMULO-2806: changed permissions of /accumulo to 700

ctubbsii
In reply to this post by ctubbsii
Github user milleruntime commented on the issue:

    https://github.com/apache/accumulo/pull/222
 
    Discovered these babies (which use the Hadoop FileSystem object in some way) and they all pass:
    - FastBulkImportIT
    - BulkFileIT
    - ImportExportIT
    I will run sunny ITs before I leave for the day.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo issue #222: ACCUMULO-2806: changed permissions of /accumulo to 700

ctubbsii
In reply to this post by ctubbsii
Github user joshelser commented on the issue:

    https://github.com/apache/accumulo/pull/222
 
    > Discovered these babies (which use the Hadoop FileSystem object in some way) and they all pass:
   
    I wouldn't trust this... We very likely don't configure HDFS to actually enforce filesystem permissions (no, this is not the default).
   
    https://hadoop.apache.org/docs/r2.7.2/hadoop-project-dist/hadoop-hdfs/HdfsPermissionsGuide.html#Configuration_Parameters


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo issue #222: ACCUMULO-2806: changed permissions of /accumulo to 700

ctubbsii
In reply to this post by ctubbsii
Github user milleruntime commented on the issue:

    https://github.com/apache/accumulo/pull/222
 
    > We very likely don't configure HDFS to actually enforce filesystem permissions (no, this is not the default).
   
    According to [Hadoop](https://hadoop.apache.org/docs/r2.7.2/hadoop-project-dist/hadoop-hdfs/hdfs-default.xml), the default setting for dfs.permissions.enabled is true.
   
    So I guess the question is whether our current tests are enough or not.
   



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo issue #222: ACCUMULO-2806: changed permissions of /accumulo to 700

ctubbsii
In reply to this post by ctubbsii
Github user joshelser commented on the issue:

    https://github.com/apache/accumulo/pull/222
 
    >According to Hadoop, the default setting for dfs.permissions.enabled is true.
    > So I guess the question is whether our current tests are enough or not.
   
    +1 to both points. Thanks for investigating for me :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #222: ACCUMULO-2806: changed permissions of /accumulo ...

ctubbsii
In reply to this post by ctubbsii
Github user asfgit closed the pull request at:

    https://github.com/apache/accumulo/pull/222


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---