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

Jianchun Xu commented on RANGER-4506:
-------------------------------------

[~vikkumar] Thanks for looking. With your info and re-checking now I realized 
the issue was fixed in 
[https://github.com/apache/ranger/commit/6e7cca01a041dc68e961fe13ef699ee799f8a890]
 (Jan 16 2023). I had discovered the issue on an earlier version. Prior to 
above commit, "dbStore.engineLoad" implementation does not operate on a temp 
map; instead it directly operates on ["synchronized (keyEntries)", 
"keyEntries.clear()", 
...|https://github.com/apache/ranger/commit/6e7cca01a041dc68e961fe13ef699ee799f8a890#diff-285d977276d43d86adf274f6817e023b704ada82bd505b0aed83edba133d2d45].
 (before-change RangerKeyStore.java, engineLoad). After above commit it should 
be ok as in your analysis.

> Illegal read lock usage in getMetadata/getKeyVersion
> ----------------------------------------------------
>
>                 Key: RANGER-4506
>                 URL: https://issues.apache.org/jira/browse/RANGER-4506
>             Project: Ranger
>          Issue Type: Bug
>          Components: kms
>            Reporter: Jianchun Xu
>            Assignee: Vikas Kumar
>            Priority: Major
>
> RangerKeyStoreProvider illegally writes to key store under Read lock. This 
> happens in both getMetadata and getKeyVersion.
> E.g. in following getKeyVersion, under Read lock, the code calls 
> `dbStore.engineLoad(null, masterKey)` which reloads all the keys. Since 
> multiple threads can hold the Read lock, multiple threads can enter and 
> reload all the keys. Thus the 2nd `dbStore.engineContainsAlias(versionName)` 
> test and following `dbStore.engineGetDecryptedZoneKeyByte(versionName)` can 
> both get wrong result if another thread is reloading keys.
> [https://github.com/apache/ranger/blob/master/kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStoreProvider.java#L331]
> {code:java}
>     @Override
>     public KeyVersion getKeyVersion(String versionName) throws IOException {
>         if (logger.isDebugEnabled()) {
>             logger.debug("==> getKeyVersion({})", versionName);
>         }
>         KeyVersion ret = null;
>         try (AutoClosableReadLock ignored = new AutoClosableReadLock(lock)) {
>             if (keyVaultEnabled) {
>                 try {
>                     boolean versionNameExists = 
> dbStore.engineContainsAlias(versionName);
>                     if (!versionNameExists) {
>                         dbStore.engineLoad(null, masterKey);
>                         versionNameExists = 
> dbStore.engineContainsAlias(versionName);
>                     }
>                     if (versionNameExists) {
>                         byte[] decryptKeyByte;
>                         try {
>                             decryptKeyByte = 
> dbStore.engineGetDecryptedZoneKeyByte(versionName);
> {code}



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to