----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/75248/#review227264 -----------------------------------------------------------
Fix it, then Ship it! Ship It! kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStore.java Lines 1101 (patched) <https://reviews.apache.org/r/75248/#comment315504> Is it required at info level or can we move to debug kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStoreProvider.java Lines 103 (patched) <https://reviews.apache.org/r/75248/#comment315503> Please remove kms/src/main/java/org/apache/hadoop/crypto/key/RangerMasterKey.java Line 228 (original), 313 (patched) <https://reviews.apache.org/r/75248/#comment315502> Same here. Please remove kms/src/main/java/org/apache/hadoop/crypto/key/RangerMasterKey.java Line 301 (original), 386 (patched) <https://reviews.apache.org/r/75248/#comment315501> Please remove unused/commented out code - Sailaja Polavarapu On Oct. 29, 2024, 4:41 p.m., Vikas Kumar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/75248/ > ----------------------------------------------------------- > > (Updated Oct. 29, 2024, 4:41 p.m.) > > > Review request for ranger, bhavik patel, Dhaval Shah, Madhan Neethiraj, and > Sailaja Polavarapu. > > > Repository: ranger > > > Description > ------- > > RANGER-3174: Weak Cryptographic Algorithm and hash function used for PBE > encryption > > Approach: > If FIPS flag enabled and latest algo configured inside dbks-site.xml > > Then, programatically adding BouncyCastleProvider as security/crypto > provider. Dependencies for this jar is already present, so no new dependecies > introduced. > > Then, it decrypts the Masterkey using older algo and re-encrypts it using the > latest one. This way, MK key material remains same but it gets encrypted with > the new algo. MK format in DB contains the algo name, so it becomes easy to > decide which algo to use to decrypt the material. > > Second, For zone keys, One new key attribute is introduced that keep the name > of algo used to encrypt the key material. If not re-encrypted, it remains > empty. > > If FIPS flag is not enabled, there will not be any impact and it should work > as older. > > Please note that I have not added BouncyCastleFipsProvider ( from bc-fips > jar) as it was contradicting with bc-prov jar (already present in KMS) that > is being used for some HSM client. But we are using the same algo from > bc-prov jar. Anyways, bc-prov is the superset of bc-fips. > > > Diffs > ----- > > kms/src/main/java/org/apache/hadoop/crypto/key/RangerKMSMKI.java b09cd5bad > kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStore.java > 39b5d65d1 > kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStoreProvider.java > 957d2ca0e > kms/src/main/java/org/apache/hadoop/crypto/key/RangerMasterKey.java > db3750ecc > kms/src/main/java/org/apache/hadoop/crypto/key/SupportedPBECryptoAlgo.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/75248/diff/1/ > > > Testing > ------- > > Testing: > > Strategy: > - Generate one EDEK, > - decrypt it to get DEK and save the DEK material. > - After patch, decrypt the same EDEK and verify the DEK material. > - Expectation is that it must remain same and if DEK content is same, then > the files encrypted with older key will remain accessible after key > re-encryption. > > Test executions: > > Old test cluster (without this change) > > Step1: Created one EDEK > > Reqest: curl -ivk -H "Content-Type: application/json" -H -X POST > 'http://apacheprtest.site:9292/kms/v1/key/pbealgotest/_eek?eek_op=generate&num_keys=1&user.name=keyadmin' > > Response: > { > "encryptedKeyVersion" : { > "material" : "G90ZtTKOWIICXG_wpqx0tA", > "name" : "pbealgotest", > "versionName" : "EEK" > }, > "versionName" : "pbealgotest@0", > "iv" : "6-ZA5dd7a-TWuee3coFzWg" > } > > Step2: Decrypted the EDEK > > Request: curl -ivk -H "Content-Type: application/json" -H -X POST --data > '{"name":"pbealgotest","iv":"6-ZA5dd7a-TWuee3coFzWg","material":"G90ZtTKOWIICXG_wpqx0tA"}' > > 'http://apacheprtest.site:9292/kms/v1/keyversion/pbealgotest@0/_eek?eek_op=decrypt&user.name=keyadmin' > > Response: > > { > "material" : "7l07elmiCGbeCx2OgCH2Rg", > "name" : "pbealgotest", > "versionName" : "EK" > } > > > Step3: > Patch the cluster with current changes > Update the value of > "ranger.kms.service.masterkey.password.encryption.algorithm" property inside > dbks-site.xml from PBEWithMD5AndDES to PBKDF2WithHmacSHA256 > Manually mark the fips flag enabled ( in code for testing) > Restart the cluster > > Step4: Here expectation was as follows: > > - MasterKey should get re-encrypted with latest algo, that is, > PBKDF2WithHmacSHA256 > - All zone keys should get re-encrypted with PBKDF2WithHmacSHA256 > > Manually checked the DB changes that shows following for the MasterKey: > > > AES,256,8,PBEWithMD5AndDES,SHA,1000,abcdefghijklmnopqrstuvwxyz01234567890,x/H7cBXW6s+nHrh8IcoQ018wwCc2xXmWbTxHGqxZnNOlfGc1Y8KxJg== > > And similarly, zone keys starts containing one attribute that indicates ( new > attribute introduced)the algo used to encrypt the key material. It will > remain empty if not re-encrypted. Something like following: > > {"key.acl.name":"pbealgotest","abc":"123","keyEncrAlgoName":"PBKDF2WithHmacSHA256"} > > Step5: To further verify the if existing keys are getting decrypted > correctly. I hit the decrypt API again and got the same response. I mean, > same DEK material. > Response: > > { > "material" : "7l07elmiCGbeCx2OgCH2Rg", > "name" : "pbealgotest", > "versionName" : "EK" > } > > I also verified other APIs like getMetadata, getkey, deletekey etc. > > > Thanks, > > Vikas Kumar > >
