hadoop-yetus commented on a change in pull request #664: [HADOOP-14951] Make 
the KMSACLs implementation customizable, with an …
URL: https://github.com/apache/hadoop/pull/664#discussion_r280745681
 
 

 ##########
 File path: 
hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSACLs.java
 ##########
 @@ -65,269 +66,105 @@ public String getBlacklistConfigKey() {
     }
   }
 
-  public static final String ACL_DEFAULT = 
AccessControlList.WILDCARD_ACL_VALUE;
-
-  public static final int RELOADER_SLEEP_MILLIS = 1000;
-
-  private volatile Map<Type, AccessControlList> acls;
-  private volatile Map<Type, AccessControlList> blacklistedAcls;
-  @VisibleForTesting
-  volatile Map<String, HashMap<KeyOpType, AccessControlList>> keyAcls;
-  @VisibleForTesting
-  volatile Map<KeyOpType, AccessControlList> defaultKeyAcls = new HashMap<>();
-  @VisibleForTesting
-  volatile Map<KeyOpType, AccessControlList> whitelistKeyAcls = new 
HashMap<>();
-  private ScheduledExecutorService executorService;
-  private long lastReload;
-
-  KMSACLs(Configuration conf) {
-    if (conf == null) {
-      conf = loadACLs();
-    }
-    setKMSACLs(conf);
-    setKeyACLs(conf);
-  }
-
-  public KMSACLs() {
-    this(null);
-  }
-
-  private void setKMSACLs(Configuration conf) {
-    Map<Type, AccessControlList> tempAcls = new HashMap<Type, 
AccessControlList>();
-    Map<Type, AccessControlList> tempBlacklist = new HashMap<Type, 
AccessControlList>();
-    for (Type aclType : Type.values()) {
-      String aclStr = conf.get(aclType.getAclConfigKey(), ACL_DEFAULT);
-      tempAcls.put(aclType, new AccessControlList(aclStr));
-      String blacklistStr = conf.get(aclType.getBlacklistConfigKey());
-      if (blacklistStr != null) {
-        // Only add if blacklist is present
-        tempBlacklist.put(aclType, new AccessControlList(blacklistStr));
-        LOG.info("'{}' Blacklist '{}'", aclType, blacklistStr);
-      }
-      LOG.info("'{}' ACL '{}'", aclType, aclStr);
-    }
-    acls = tempAcls;
-    blacklistedAcls = tempBlacklist;
-  }
-
-  @VisibleForTesting
-  void setKeyACLs(Configuration conf) {
-    Map<String, HashMap<KeyOpType, AccessControlList>> tempKeyAcls =
-        new HashMap<String, HashMap<KeyOpType,AccessControlList>>();
-    Map<String, String> allKeyACLS =
-        conf.getValByRegex(KMSConfiguration.KEY_ACL_PREFIX_REGEX);
-    for (Map.Entry<String, String> keyAcl : allKeyACLS.entrySet()) {
-      String k = keyAcl.getKey();
-      // this should be of type "key.acl.<KEY_NAME>.<OP_TYPE>"
-      int keyNameStarts = KMSConfiguration.KEY_ACL_PREFIX.length();
-      int keyNameEnds = k.lastIndexOf(".");
-      if (keyNameStarts >= keyNameEnds) {
-        LOG.warn("Invalid key name '{}'", k);
-      } else {
-        String aclStr = keyAcl.getValue();
-        String keyName = k.substring(keyNameStarts, keyNameEnds);
-        String keyOp = k.substring(keyNameEnds + 1);
-        KeyOpType aclType = null;
-        try {
-          aclType = KeyOpType.valueOf(keyOp);
-        } catch (IllegalArgumentException e) {
-          LOG.warn("Invalid key Operation '{}'", keyOp);
-        }
-        if (aclType != null) {
-          // On the assumption this will be single threaded.. else we need to
-          // ConcurrentHashMap
-          HashMap<KeyOpType,AccessControlList> aclMap =
-              tempKeyAcls.get(keyName);
-          if (aclMap == null) {
-            aclMap = new HashMap<KeyOpType, AccessControlList>();
-            tempKeyAcls.put(keyName, aclMap);
-          }
-          aclMap.put(aclType, new AccessControlList(aclStr));
-          LOG.info("KEY_NAME '{}' KEY_OP '{}' ACL '{}'",
-              keyName, aclType, aclStr);
-        }
-      }
-    }
-    keyAcls = tempKeyAcls;
-
-    final Map<KeyOpType, AccessControlList> tempDefaults = new HashMap<>();
-    final Map<KeyOpType, AccessControlList> tempWhitelists = new HashMap<>();
-    for (KeyOpType keyOp : KeyOpType.values()) {
-      parseAclsWithPrefix(conf, KMSConfiguration.DEFAULT_KEY_ACL_PREFIX,
-          keyOp, tempDefaults);
-      parseAclsWithPrefix(conf, KMSConfiguration.WHITELIST_KEY_ACL_PREFIX,
-          keyOp, tempWhitelists);
-    }
-    defaultKeyAcls = tempDefaults;
-    whitelistKeyAcls = tempWhitelists;
-  }
+  /**
+   * First Check if user is in ACL for the KMS operation, if yes, then return
+   * true if user is not present in any configured blacklist for the operation.
+   * 
+   * @param keyOperationType
+   *          KMS Operation
+   * @param ugi
+   *          UserGroupInformation of user
+   * @param key
+   *          the key name
+   * @return true is user has access
+   */
+  public abstract boolean hasAccess(Type keyOperationType,
+      UserGroupInformation ugi, String key);
 
+  
   /**
-   * Parse the acls from configuration with the specified prefix. Currently
-   * only 2 possible prefixes: whitelist and default.
-   *
-   * @param conf The configuration.
-   * @param prefix The prefix.
-   * @param keyOp The key operation.
-   * @param results The collection of results to add to.
+   * This is called by the KeyProvider to check if the given user is
+   * authorized to perform the specified operation on the given acl name.
+   * @param aclName name of the key ACL
+   * @param ugi User's UserGroupInformation
+   * @param opType Operation Type 
+   * @return true if user has access to the aclName and opType else false
    */
-  private void parseAclsWithPrefix(final Configuration conf,
-      final String prefix, final KeyOpType keyOp,
-      Map<KeyOpType, AccessControlList> results) {
-    String confKey = prefix + keyOp;
-    String aclStr = conf.get(confKey);
-    if (aclStr != null) {
-      if (keyOp == KeyOpType.ALL) {
-        // Ignore All operation for default key and whitelist key acls
-        LOG.warn("Invalid KEY_OP '{}' for {}, ignoring", keyOp, prefix);
-      } else {
-        if (aclStr.equals("*")) {
-          LOG.info("{} for KEY_OP '{}' is set to '*'", prefix, keyOp);
-        }
-        results.put(keyOp, new AccessControlList(aclStr));
-      }
-    }
-  }
+  @Override
+  public abstract boolean hasAccessToKey(String aclName,
+      UserGroupInformation ugi, KeyOpType opType);
 
+  /**
+   * 
 
 Review comment:
   whitespace:end of line
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

Reply via email to