hadoop-common-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dian Fu (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HADOOP-11335) KMS ACL in meta data or database
Date Sat, 17 Jan 2015 14:00:37 GMT

    [ https://issues.apache.org/jira/browse/HADOOP-11335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14281352#comment-14281352
] 

Dian Fu commented on HADOOP-11335:
----------------------------------

Hi [~asuresh], thanks a lot for your review and comments.
{quote}
1) JavaKeyStoreProvider:
   * createKey() : I think it might be a bit odd that we have metadata with version == 0 
{quote}
OK, have updated the patch and version will no longer be equal to 0.
{quote}
* deleteKey() : Am sorry, I might be missing something but, I did not quite understand why
we can't just delete the metadata from the cache when the key is deleted
{quote}
As we may store ACL in metadata and ACL shouldn't be deleted when the key is deleted.
{quote}
2) KeyProvider:
   * The Metadata Class now has a dependency on {{KeyOpType}} which is defined in {{KeyProviderAuthorizationExtension}}.
The Extension classes were meant to add functionality to a KeyProvider. It seems a bit weird
that the KeyProvider class should have a dependency on an Extension.
{quote}
Agree. Have moved {{KeyOpType}} to {{KeyProvider}}.
{quote}
* Not very confortable adding setters to Metadata. I Guess the original implementaion conciously
made a choice to not allow modification of metadata once it is created (except for the version)
{quote}
Agree. Have removed the setters of Metadata.
{quote}
3) KeyProviderExtension:
   * Do we really need the read and write locks ? The undelying KeyProvider should take care
of the synchronization. (for eg. {{JavaKeyStoreProvider}}) does infact use write and read
locks for createKey etc.. this would probably lead to unnecessary double locking. 
4) KeyProviderAuthorizationExtension:
   * Same as above.. do we really need the read and write locks ? I feel the Extension class
should handle its own concurrency semantics
{quote}
The lock in {{JavaKeyStoreProvider}} is only used for key related operations, such as {{createKey}},
{{deleteKey}}, etc. As the new added methods such as {{createKeyAcl}}, {{deleteKeyAcl}} also
need to read/write keystore and these methods only exist in {{KeyProviderAuthorizationExtension}},
not in {{KeyProvider}}. So we add lock in {{KeyProviderExtension}} and the lock in {{KeyProviderAuthorizationExtension}}
is inherited from {{KeyProviderExtension}}.
{quote}
5) MetadataKeyAuthorizer
   * Remove commented code
{quote}
Removed the commented code in latest patch.
{quote}
Looking at the commented code in {{MetadataKeyAuthorizer}}, I see that you had initially toyed
with having an extended {{MetadataWithACL}} class. Any reason why you did not pursue that
design ? It seems to me like that could have been a way to probably avoid having to modify
the {{JavaKeyStoreProvider}} and {{KeyProvider}}. One suggestion would have been to templatized
{{KeyProvider}} like so :
{noformat}
  public class KeyProvider<M extends Metadata>
  ...
{noformat}
and have different implemenetation of a {{KeyProvider}} like :
{noformat}
  public classs KeyProviderWithACls extends KeyProvider<MetadataWithACLs>
  ...  
{noformat} 
{quote}
The method you suggested is a good method. I have tried to use this method but I found there
are some problems. For example, currently we create {{KeyProviderAuthorizationExtension}}
by wrapping a {{KeyProvider}} in it and this {{KeyProvider}} should be created first. Then
when we create {{KeyProvider}}, we should know the type for {{Metadata}}, for example whether
it's {{Metadata}} or {{MetadataWithAcl}}. This is a little weird as whether ACL is stored
in Metadata should be controlled in {{KeyProviderAuthorizationExtension}}.
I choose to solve this issue by modifying {{Metadata}} and it currently has two elements {{MetadataForKey}}
and {{MetadataForAcl}}. You can refer to the latest patch (revision 003) for detailed implementation.

> KMS ACL in meta data or database
> --------------------------------
>
>                 Key: HADOOP-11335
>                 URL: https://issues.apache.org/jira/browse/HADOOP-11335
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: kms
>    Affects Versions: 2.6.0
>            Reporter: Jerry Chen
>            Assignee: Dian Fu
>              Labels: Security
>         Attachments: HADOOP-11335.001.patch, HADOOP-11335.002.patch, HADOOP-11335.003.patch,
KMS ACL in metadata or database.pdf
>
>   Original Estimate: 504h
>  Remaining Estimate: 504h
>
> Currently Hadoop KMS has implemented ACL for keys and the per key ACL are stored in the
configuration file kms-acls.xml.
> The management of ACL in configuration file would not be easy in enterprise usage and
it is put difficulties for backup and recovery.
> It is ideal to store the ACL for keys in the key meta data similar to what file system
ACL does.  In this way, the backup and recovery that works on keys should work for ACL for
keys too.
> On the other hand, with the ACL in meta data, the ACL of each key can be easily manipulate
with API or command line tool and take effect instantly.  This is very important for enterprise
level access control management.  This feature can be addressed by separate JIRA. While with
the configuration file, these would be hard to provide.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message