[
https://issues.apache.org/jira/browse/HADOOP-1298?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12514827
]
Tsz Wo (Nicholas), SZE commented on HADOOP-1298:
------------------------------------------------
Hi Christophe and others,
I would like to work on this issue. Below are my comments.
*Major problems:*
- I have tried to apply hadoop-dev-20070720-1633.patch.gz to the current trunk
but the program cannot be compiled. Did you update your version with current
trunk? It seems to me that the patch is out dated
- The entire patch does not use any Java security technology. It would be
difficult to provide real security later on. java.security.Permission and
other related classes already have some very good design. Java Authentication
and Authorization Service (JAAS) provides pluggable login modules. We probably
want to use them.
- Using DFSClientContext as a parameter in all method calls for checking
permissions. The idea is not right. We should pass some kind of credential /
ticket / signature, instead of a context.
- Also, it is better to provide a framework to support the general secure
operations. For example, add a method setCredential(...) in ClientProtocol.
Then, the credential be used for the later method calls and we don't have to
change other method headers.
- I think we need authentication server(s) to manage users (i.e. UserDatabase
should be resided in an authentication servers). System components
authenticate itself with the server and obtain credentials. It reduces the
complication and use of resource in Namenode.
- INode has
private String owner;
private String group;
private short mode;
It is better to have a FilePermission class (which extends
java.security.PermissionCollection) to encapsulate them. I think it is valid
to assume that there are many file having the same permissions. Then, we might
be able to save spaces. Also, we will have the flexibility that use permission
model other than posix.
- It seems to me that the patch was not careful written: for example, in class
Path, there is a constant SEPARATOR (= "/") but the codes inside keep using "/".
- HDFS should have its own user management. It is not right to use arbitrary
username (like OS's username), especially there is no real security
implemented. It will bring a lot of problems by user's careless mistakes later.
- All entries (user, host, etc.) should have its own principal. Something like
"nobody" is just a shared account.
*Minor problems:*
- The patch changed a lot of unnecessary code formatting. That's why it has
8000+ lines. Also, the codes have a lot of lines longer than 80 characters.
- FileMode and FileAcccessMode are overlapped. FileAcccessMode using enum is
better.
- It is better to create some packages since there are quite many new classes.
- why change FSDirectory.INode to INode? Again, it makes the patch hard to
follow.
- bad idea to use static variables for non-constants, e.g. in UserDatabase,
private final static UserDatabase db = new UserDatabase();
it will be difficult to manage, e.g. restarting NameNode.
- There are un-used classes Pair and Triplet. Programming exercises?
- missing javadoc for many methods.
*Other comments:*
- For some design issues like "changing FSDirectory.INode to INode", if there
is a good reason behind, we can create a new issue and change it later.
- We should only provide core user/group support in the first patch for making
the patch as small as possible. We can work on features like sticky bit later.
*Please vote:*
For a messy patch like this, how many of you think that it is bug free and does
not hide some back doors?
-1 for me.
I suggest to remove all the unnecessary changes and work on a small core patch
and I am willing to do it. Of course, we should reuse the well written codes
in the patch.
Nicholas
> adding user info to file
> ------------------------
>
> Key: HADOOP-1298
> URL: https://issues.apache.org/jira/browse/HADOOP-1298
> Project: Hadoop
> Issue Type: New Feature
> Components: dfs, fs
> Reporter: Kurtis Heimerl
> Fix For: 0.15.0
>
> Attachments: hadoop-dev-20070720-1633.patch.gz,
> hadoop-user-munncha.patch, hadoop-user-munncha.patch,
> hadoop-user-munncha.patch, hadoop-user-munncha.patch10,
> hadoop-user-munncha.patch11, hadoop-user-munncha.patch12,
> hadoop-user-munncha.patch13, hadoop-user-munncha.patch14,
> hadoop-user-munncha.patch15, hadoop-user-munncha.patch16,
> hadoop-user-munncha.patch17, hadoop-user-munncha.patch4,
> hadoop-user-munncha.patch5, hadoop-user-munncha.patch6,
> hadoop-user-munncha.patch7, hadoop-user-munncha.patch8,
> hadoop-user-munncha.patch9, hdfs-access-control.patch.gz
>
>
> I'm working on adding a permissions model to hadoop's DFS. The first step is
> this change, which associates user info with files. Following this I'll
> assoicate permissions info, then block methods based on that user info, then
> authorization of the user info.
> So, right now i've implemented adding user info to files. I'm looking for
> feedback before I clean this up and make it offical.
> I wasn't sure what release, i'm working off trunk.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
