RANGER-472: Fixes for KMS UI and best practices (Gautam Borad via Velmurugan Periasamy)
Signed-off-by: sneethiraj <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/incubator-ranger/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-ranger/commit/0336e2b2 Tree: http://git-wip-us.apache.org/repos/asf/incubator-ranger/tree/0336e2b2 Diff: http://git-wip-us.apache.org/repos/asf/incubator-ranger/diff/0336e2b2 Branch: refs/heads/tag-policy Commit: 0336e2b2c7a0cadd99b2dcc2a1afa7fb3439f503 Parents: 64582f0 Author: Velmurugan Periasamy <[email protected]> Authored: Sun May 17 13:59:14 2015 -0400 Committer: sneethiraj <[email protected]> Committed: Sun May 17 14:11:18 2015 -0400 ---------------------------------------------------------------------- .../hadoop/crypto/key/RangerKeyStore.java | 2 +- .../crypto/key/RangerKeyStoreProvider.java | 98 +++++++++++--------- .../java/org/apache/ranger/biz/KmsKeyMgr.java | 7 +- .../webapp/scripts/controllers/Controller.js | 2 +- .../webapp/scripts/views/kms/KMSTableLayout.js | 20 ++-- 5 files changed, 72 insertions(+), 57 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/0336e2b2/kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStore.java ---------------------------------------------------------------------- diff --git a/kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStore.java b/kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStore.java index 93109e2..f9e134f 100644 --- a/kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStore.java +++ b/kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStore.java @@ -417,7 +417,7 @@ public class RangerKeyStore extends KeyStoreSpi { * at the end. If this check fails, the store has been tampered * with */ - if (password != null) { + if (computed != null) { int counter = 0; for (int i = computed.length-1; i >= 0; i--) { if (computed[i] != data[data.length-(1+counter)]) { http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/0336e2b2/kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStoreProvider.java ---------------------------------------------------------------------- diff --git a/kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStoreProvider.java b/kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStoreProvider.java index 7b8e977..541f369 100755 --- a/kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStoreProvider.java +++ b/kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStoreProvider.java @@ -37,7 +37,7 @@ import java.util.List; import java.util.Map; import javax.crypto.spec.SecretKeySpec; - +import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.crypto.key.KeyProvider; import org.apache.hadoop.crypto.key.KeyProviderFactory; @@ -46,7 +46,11 @@ import org.apache.hadoop.fs.Path; import org.apache.ranger.credentialapi.CredentialReader; import org.apache.ranger.kms.dao.DaoManager; import org.apache.log4j.Logger; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; [email protected] public class RangerKeyStoreProvider extends KeyProvider{ static final Logger logger = Logger.getLogger(RangerKeyStoreProvider.class); @@ -66,6 +70,8 @@ public class RangerKeyStoreProvider extends KeyProvider{ private boolean changed = false; private final Map<String, Metadata> cache = new HashMap<String, Metadata>(); private DaoManager daoManager; + + private Lock readLock; public RangerKeyStoreProvider(Configuration conf) throws Throwable { super(conf); @@ -90,12 +96,12 @@ public class RangerKeyStoreProvider extends KeyProvider{ try { loadKeys(masterKey); } catch (NoSuchAlgorithmException e) { - e.printStackTrace(); throw new IOException("Can't load Keys"); }catch(CertificateException e){ - e.printStackTrace(); throw new IOException("Can't load Keys"); } + ReadWriteLock lock = new ReentrantReadWriteLock(true); + readLock = lock.readLock(); } public static Configuration getDBKSConf() { @@ -159,7 +165,6 @@ public class RangerKeyStoreProvider extends KeyProvider{ String attribute = om.writeValueAsString(attributes); dbStore.engineSetKeyEntry(versionName, new SecretKeySpec(material, cipher), masterKey, cipher, bitLength, description, version, attribute); } catch (KeyStoreException e) { - e.printStackTrace(); throw new IOException("Can't store key " + versionName,e); } changed = true; @@ -187,7 +192,6 @@ public class RangerKeyStoreProvider extends KeyProvider{ dbStore.engineDeleteEntry(name); } } catch (KeyStoreException e) { - e.printStackTrace(); throw new IOException("Problem removing " + name + " from " + this, e); } cache.remove(name); @@ -208,46 +212,45 @@ public class RangerKeyStoreProvider extends KeyProvider{ String attributes = om.writeValueAsString(metadata.getAttributes()); dbStore.engineSetKeyEntry(entry.getKey(), new KeyMetadata(metadata), masterKey, metadata.getAlgorithm(), metadata.getBitLength(), metadata.getDescription(), metadata.getVersions(), attributes); } catch (KeyStoreException e) { - e.printStackTrace(); throw new IOException("Can't set metadata key " + entry.getKey(),e ); } } try { dbStore.engineStore(null, masterKey); } catch (NoSuchAlgorithmException e) { - e.printStackTrace(); throw new IOException("No such algorithm storing key", e); } catch (CertificateException e) { - e.printStackTrace(); throw new IOException("Certificate exception storing key", e); } changed = false; }catch (IOException ioe) { - ioe.printStackTrace(); throw ioe; } } @Override public KeyVersion getKeyVersion(String versionName) throws IOException { - SecretKeySpec key = null; - try { - if (!dbStore.engineContainsAlias(versionName)) { - return null; - } - key = (SecretKeySpec) dbStore.engineGetKey(versionName, masterKey); - } catch (NoSuchAlgorithmException e) { - e.printStackTrace(); - throw new IOException("Can't get algorithm for key " + key, e); - } catch (UnrecoverableKeyException e) { - e.printStackTrace(); - throw new IOException("Can't recover key " + key, e); - } - if (key == null) { - return null; - } else { - return new KeyVersion(getBaseName(versionName), versionName, key.getEncoded()); - } + readLock.lock(); + try { + SecretKeySpec key = null; + try { + if (!dbStore.engineContainsAlias(versionName)) { + return null; + } + key = (SecretKeySpec) dbStore.engineGetKey(versionName, masterKey); + } catch (NoSuchAlgorithmException e) { + throw new IOException("Can't get algorithm for key " + key, e); + } catch (UnrecoverableKeyException e) { + throw new IOException("Can't recover key " + key, e); + } + if (key == null) { + return null; + } else { + return new KeyVersion(getBaseName(versionName), versionName, key.getEncoded()); + } + } finally { + readLock.unlock(); + } } @Override @@ -286,23 +289,30 @@ public class RangerKeyStoreProvider extends KeyProvider{ @Override public Metadata getMetadata(String name) throws IOException { - if (cache.containsKey(name)) { - return cache.get(name); - } - try { - if (!dbStore.engineContainsAlias(name)) { - return null; - } - Metadata meta = ((KeyMetadata) dbStore.engineGetKey(name, masterKey)).metadata; - cache.put(name, meta); - return meta; - } catch (NoSuchAlgorithmException e) { - e.printStackTrace(); - throw new IOException("Can't get algorithm for " + name, e); - } catch (UnrecoverableKeyException e) { - e.printStackTrace(); - throw new IOException("Can't recover key for " + name, e); - } + readLock.lock(); + try { + if (cache.containsKey(name)) { + return cache.get(name); + } + try { + if (!dbStore.engineContainsAlias(name)) { + return null; + } + Key key = dbStore.engineGetKey(name, masterKey); + if(key != null){ + Metadata meta = ((KeyMetadata) key).metadata; + cache.put(name, meta); + return meta; + } + } catch (NoSuchAlgorithmException e) { + throw new IOException("Can't get algorithm for " + name, e); + } catch (UnrecoverableKeyException e) { + throw new IOException("Can't recover key for " + name, e); + } + return null; + } finally { + readLock.unlock(); + } } @Override http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/0336e2b2/security-admin/src/main/java/org/apache/ranger/biz/KmsKeyMgr.java ---------------------------------------------------------------------- diff --git a/security-admin/src/main/java/org/apache/ranger/biz/KmsKeyMgr.java b/security-admin/src/main/java/org/apache/ranger/biz/KmsKeyMgr.java index ebc05b8..5f2d4af 100755 --- a/security-admin/src/main/java/org/apache/ranger/biz/KmsKeyMgr.java +++ b/security-admin/src/main/java/org/apache/ranger/biz/KmsKeyMgr.java @@ -141,11 +141,8 @@ public class KmsKeyMgr { deleteRest = deleteRest.replaceAll(Pattern.quote("${userName}"), currentUserLoginId); String uri = provider + (provider.endsWith("/") ? deleteRest : ("/" + deleteRest)); WebResource r = c.resource(uri) ; - ClientResponse response = r.delete(ClientResponse.class) ; - logger.debug("delete RESPONSE: [" + response.toString() + "]") ; - if (response.getStatus() == 200) { - logger.debug("Alias "+name+" deleted successfully"); - } + String response = r.delete(String.class) ; + logger.debug("delete RESPONSE: [" + response + "]") ; } public VXKmsKey createKey(String provider, VXKmsKey vXKey){ http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/0336e2b2/security-admin/src/main/webapp/scripts/controllers/Controller.js ---------------------------------------------------------------------- diff --git a/security-admin/src/main/webapp/scripts/controllers/Controller.js b/security-admin/src/main/webapp/scripts/controllers/Controller.js index 96a458d..60f8976 100755 --- a/security-admin/src/main/webapp/scripts/controllers/Controller.js +++ b/security-admin/src/main/webapp/scripts/controllers/Controller.js @@ -408,7 +408,7 @@ define(function(require) { var KmsKey = require('models/VXKmsKey'); App.rContent.show(new view({ - model : new KmsKey({'length' : 128, 'cipher' : 'AES' }), + model : new KmsKey({'length' : 128, 'cipher' : 'AES/CTR/NoPadding' }), kmsServiceName : kmsServiceName })); }, http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/0336e2b2/security-admin/src/main/webapp/scripts/views/kms/KMSTableLayout.js ---------------------------------------------------------------------- diff --git a/security-admin/src/main/webapp/scripts/views/kms/KMSTableLayout.js b/security-admin/src/main/webapp/scripts/views/kms/KMSTableLayout.js index 467a318..52990dd 100755 --- a/security-admin/src/main/webapp/scripts/views/kms/KMSTableLayout.js +++ b/security-admin/src/main/webapp/scripts/views/kms/KMSTableLayout.js @@ -325,10 +325,14 @@ define(function(require){ XAUtil.notifySuccess('Success', localization.tt('msg.keyDeleteMsg')); that.renderKeyTab(); that.collection.fetch(); - }, - 'error': function (model, response, options) { + }, + 'error' : function(model,resp){ + var errorMsg = 'Error deleting key!'; XAUtil.blockUI('unblock'); - XAUtil.notifyError('Error', 'Error deleting key!'); + if(!_.isUndefined(resp) && !_.isUndefined(resp.responseJSON) && !_.isUndefined(resp.responseJSON.msgDesc)){ + errorMsg = resp.responseJSON.msgDesc; + } + XAUtil.notifyError('Error', errorMsg); } }); } @@ -355,9 +359,13 @@ define(function(require){ that.renderKeyTab(); that.collection.fetch(); }, - 'error': function (model, response, options) { - XAUtil.blockUI('unblock'); - XAUtil.notifyError('Error', 'Error rollovering key!'); + 'error' : function(model,resp){ + var errorMsg = 'Error rollovering key!'; + XAUtil.blockUI('unblock'); + if(!_.isUndefined(resp) && !_.isUndefined(resp.responseJSON) && !_.isUndefined(resp.responseJSON.msgDesc)){ + errorMsg = resp.responseJSON.msgDesc; + } + XAUtil.notifyError('Error', errorMsg); } }); }
