diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/util/PasswordUtils.java b/agents-common/src/main/java/org/apache/ranger/plugin/util/PasswordUtils.java index 1343b7c413..5d777ebd4f 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/util/PasswordUtils.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/util/PasswordUtils.java @@ -32,6 +32,7 @@ import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; import java.util.ArrayList; +import java.util.Arrays; import java.util.Base64; import java.util.List; import java.util.Map; @@ -47,6 +48,14 @@ public class PasswordUtils { public static final int DEFAULT_ITERATION_COUNT = 17; public static final byte[] DEFAULT_INITIAL_VECTOR = new byte[16]; private static final String LEN_SEPARATOR_STR = ":"; + private static final String MSG_ENCRYPT_KEY_STILL_DEFAULT = "ranger.password.encryption.key is not set — it is still the default, publicly-known " + + "value that ships with every Ranger install (in source control and every tarball), so encrypting with it would protect nothing. Set " + + "ranger.password.encryption.key to a unique, secret value in ranger-admin-site.xml — the exact same value on every Admin node in this " + + "cluster — before service config passwords can be encrypted."; + + private static final String V2_FORMAT_PREFIX = "v2,"; + private static final String LEGACY_ENV_KEY_OVERRIDE_NAME = "lEncryptKey"; + private static final String LEGACY_ENV_SALT_OVERRIDE_NAME = "ENCRYPT_SALT"; private final RangerSupportedCryptoAlgo cryptAlgo; private final int iterationCount; @@ -112,6 +121,15 @@ public class PasswordUtils { } } + private PasswordUtils(RangerSupportedCryptoAlgo cryptAlgo, char[] key, byte[] salt, int iterationCount, byte[] iv, String password) { + this.cryptAlgo = cryptAlgo; + this.encryptKey = key; + this.salt = salt; + this.iterationCount = iterationCount; + this.iv = iv; + this.password = password; + } + public static String encryptPassword(String aPassword) throws IOException { return build(aPassword).encrypt(); } @@ -124,6 +142,126 @@ public static String decryptPassword(String aPassword) throws IOException { return build(aPassword).decrypt(); } + public static boolean isV2Format(String storedValue) { + if (storedValue == null || !storedValue.startsWith(V2_FORMAT_PREFIX)) { + return false; + } + String[] fields = storedValue.substring(V2_FORMAT_PREFIX.length()).split(",", 2); + if (fields.length == 0 || StringUtils.isEmpty(fields[0])) { + return false; + } + try { + RangerSupportedCryptoAlgo.getValueOf(fields[0]); + return true; + } catch (Exception e) { + return false; + } + } + + public static String getCryptAlgoV2(String storedValue) { + if (!isV2Format(storedValue)) { + throw new IllegalArgumentException("value is not in v2 format (missing '" + V2_FORMAT_PREFIX + "' prefix)"); + } + return storedValue.substring(V2_FORMAT_PREFIX.length()).split(",", 3)[0]; + } + + public static boolean isLegacyFormat(String storedValue) { + if (StringUtils.isEmpty(storedValue) || isV2Format(storedValue) || !storedValue.contains(",")) { + return false; + } + String[] fields = storedValue.split(",", -1); + if (fields.length <= 4) { + return false; + } + try { + RangerSupportedCryptoAlgo.getValueOf(fields[0]); + return true; + } catch (Exception e) { + return false; + } + } + + public static void validateEncryptionKeyConfigured(char[] key) { + if (key == null || key.length == 0) { + throw new IllegalArgumentException("ranger.password.encryption.key is not set."); + } + if (Arrays.equals(key, DEFAULT_ENCRYPT_KEY.toCharArray())) { + throw new IllegalStateException(MSG_ENCRYPT_KEY_STILL_DEFAULT); + } + } + + public static void checkNoLegacyEnvKeyOverride() { + Map env = System.getenv(); + if (env.get(LEGACY_ENV_KEY_OVERRIDE_NAME) != null || env.get(LEGACY_ENV_SALT_OVERRIDE_NAME) != null) { + throw new IllegalStateException("This node has the legacy '" + LEGACY_ENV_KEY_OVERRIDE_NAME + "' and/or '" + LEGACY_ENV_SALT_OVERRIDE_NAME + + "' environment variable set, which silently overrides the key/salt used by legacy (v1) password encryption. The v2 format and this " + + "migration intentionally do not honor this override — they use only ranger.password.encryption.key — so migrating now would decrypt " + + "existing data under the env-var key but re-encrypt it under the configured property, silently changing which secret protects it. " + + "Confirm whether this environment variable is actually relied on before proceeding: if it is, reconcile ranger.password.encryption.key " + + "to match it first; if it is unused/vestigial, unset it, then re-run this migration."); + } + } + + public static String encryptPasswordV2(String plainText, RangerSupportedCryptoAlgo cryptAlgo, char[] key, byte[] salt, int iterationCount) throws IOException { + if (key == null || key.length == 0) { + throw new IllegalArgumentException("encryptPasswordV2() requires a non-empty key"); + } + try { + String ivStr = generateIvIfNeeded(cryptAlgo.getAlgoName()); + byte[] ivBytes = ivStr != null ? Base64.getDecoder().decode(ivStr) : DEFAULT_INITIAL_VECTOR; + String password = new PasswordUtils(cryptAlgo, key, salt, iterationCount, ivBytes, plainText).encrypt(); + List fields = new ArrayList<>(); + fields.add(cryptAlgo.getAlgoName()); + fields.add(Base64.getEncoder().encodeToString(salt)); + fields.add(String.valueOf(iterationCount)); + if (ivStr != null) { + fields.add(ivStr); + } + fields.add(password); + return V2_FORMAT_PREFIX + String.join(",", fields); + } catch (NoSuchAlgorithmException e) { + throw new IOException("Unable to generate IV for v2 password encryption", e); + } + } + + public static String decryptPasswordV2(String storedValue, char[] key) throws IOException { + if (key == null || key.length == 0) { + throw new IllegalArgumentException("decryptPasswordV2() requires a non-empty key — refusing to fall back to a default"); + } + if (!isV2Format(storedValue)) { + throw new IllegalArgumentException("value is not in v2 format (missing '" + V2_FORMAT_PREFIX + "' prefix)"); + } + String[] fields = Lists.newArrayList(Splitter.on(",").split(storedValue.substring(V2_FORMAT_PREFIX.length()))).toArray(new String[0]); + if (fields.length < 4) { + throw new IOException("Malformed v2 password value (expected at least 4 fields, found " + fields.length + ")"); + } + + int index = 0; + RangerSupportedCryptoAlgo cryptAlgo = RangerSupportedCryptoAlgo.getValueOf(fields[index++]); + byte[] salt = Base64.getDecoder().decode(fields[index++]); + int iterationCount = Integer.parseInt(fields[index++]); + byte[] iv; + if (needsIv(cryptAlgo.getAlgoName())) { + if (fields.length < 5) { + throw new IOException("Malformed v2 password value (algorithm " + cryptAlgo.getAlgoName() + " requires an IV; expected at least 5 fields, found " + fields.length + ")"); + } + iv = Base64.getDecoder().decode(fields[index++]); + } else { + iv = DEFAULT_INITIAL_VECTOR; + } + StringBuilder cipherText = new StringBuilder(fields[index++]); + // defensive: a stray comma inside the ciphertext (shouldn't happen — Base64's alphabet + // has none) would otherwise silently truncate the value instead of failing loudly. + for (int i = index; i < fields.length; i++) { + cipherText.append(",").append(fields[i]); + } + String result = new PasswordUtils(cryptAlgo, key, salt, iterationCount, iv, cipherText.toString()).decrypt(); + if (result == null) { + throw new IOException("decryptPasswordV2() failed — wrong key or corrupted value (decrypted output was not in the expected format)"); + } + return result; + } + public static boolean needsIv(String cryptoAlgo) { if (StringUtils.isEmpty(cryptoAlgo)) { return false; diff --git a/agents-common/src/test/java/org/apache/ranger/plugin/util/PasswordUtilsTest.java b/agents-common/src/test/java/org/apache/ranger/plugin/util/PasswordUtilsTest.java index dfff13b5b4..701d56c64d 100644 --- a/agents-common/src/test/java/org/apache/ranger/plugin/util/PasswordUtilsTest.java +++ b/agents-common/src/test/java/org/apache/ranger/plugin/util/PasswordUtilsTest.java @@ -23,9 +23,14 @@ import java.io.IOException; import java.security.NoSuchAlgorithmException; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; public class PasswordUtilsTest { @Test @@ -206,6 +211,190 @@ public void testDecryptEmptyResultInNull() throws Throwable { assertNull(string0); } + @Test + public void testEncryptDecryptV2RoundTrip() throws Exception { + char[] key = "operator-configured-key-2026".toCharArray(); + String storedValue = PasswordUtils.encryptPasswordV2("secretPasswordNoOneWillEverKnow", RangerSupportedCryptoAlgo.PBEWITHHMACSHA512ANDAES_128, + key, "f77aLYLo".getBytes(), 1000); + assertNotNull(storedValue); + assertEquals("secretPasswordNoOneWillEverKnow", PasswordUtils.decryptPasswordV2(storedValue, key)); + } + + @Test + public void testEncryptV2NeverEmbedsKeyInStoredValue() throws Exception { + char[] key = "operator-configured-key-2026".toCharArray(); + String storedValue = PasswordUtils.encryptPasswordV2("secretPasswordNoOneWillEverKnow", RangerSupportedCryptoAlgo.PBEWITHHMACSHA512ANDAES_128, + key, "f77aLYLo".getBytes(), 1000); + assertFalse(storedValue.contains(new String(key)), "the encryption key must never appear in the stored value"); + } + + @Test + public void testIsV2FormatDetection() throws Exception { + char[] key = "operator-configured-key-2026".toCharArray(); + String v2Value = PasswordUtils.encryptPasswordV2("secretPasswordNoOneWillEverKnow", RangerSupportedCryptoAlgo.PBEWITHHMACSHA512ANDAES_128, + key, "f77aLYLo".getBytes(), 1000); + String v1Value = PasswordUtils.encryptPassword("secretPasswordNoOneWillEverKnow"); + assertTrue(PasswordUtils.isV2Format(v2Value)); + assertFalse(PasswordUtils.isV2Format(v1Value)); + assertFalse(PasswordUtils.isV2Format("secretPasswordNoOneWillEverKnow")); // plaintext, no format at all + assertFalse(PasswordUtils.isV2Format("")); + assertFalse(PasswordUtils.isV2Format(null)); + } + + @Test + public void testDecryptV2WithWrongKeyThrows() throws Exception { + String storedValue = PasswordUtils.encryptPasswordV2("secretPasswordNoOneWillEverKnow", RangerSupportedCryptoAlgo.PBEWITHHMACSHA512ANDAES_128, + "operator-configured-key-2026".toCharArray(), "f77aLYLo".getBytes(), 1000); + assertThrows(Exception.class, () -> PasswordUtils.decryptPasswordV2(storedValue, "a-completely-different-key".toCharArray()), + "decrypting a v2 value under the wrong key must throw, never silently return garbage or the original ciphertext"); + } + + @Test + public void testDecryptV2RejectsLegacyV1Value() throws Exception { + String v1Value = PasswordUtils.encryptPassword(join("PBEWITHHMACSHA512ANDAES_128", "tzL1AKl5uc4NKYaoQ4P3WLGIBFPXWPWdu1fRm9004jtQiV", "f77aLYLo", "1000", + PasswordUtils.generateIvIfNeeded("PBEWITHHMACSHA512ANDAES_128"), "secretPasswordNoOneWillEverKnow")); + String v1StoredValue = join("PBEWITHHMACSHA512ANDAES_128", "tzL1AKl5uc4NKYaoQ4P3WLGIBFPXWPWdu1fRm9004jtQiV", "f77aLYLo", "1000", + PasswordUtils.generateIvIfNeeded("PBEWITHHMACSHA512ANDAES_128"), v1Value); + assertFalse(PasswordUtils.isV2Format(v1StoredValue)); + assertThrows(Exception.class, () -> PasswordUtils.decryptPasswordV2(v1StoredValue, "operator-configured-key-2026".toCharArray())); + } + + @Test + public void testEncryptV2CommaInKeyDoesNotCorruptFormat() throws Exception { + char[] keyWithComma = "abc,def-key-with-a-comma".toCharArray(); + String storedValue = PasswordUtils.encryptPasswordV2("secretPasswordNoOneWillEverKnow", RangerSupportedCryptoAlgo.PBEWITHHMACSHA512ANDAES_128, + keyWithComma, "f77aLYLo".getBytes(), 1000); + assertTrue(PasswordUtils.isV2Format(storedValue)); + assertEquals("secretPasswordNoOneWillEverKnow", PasswordUtils.decryptPasswordV2(storedValue, keyWithComma)); + } + + @Test + public void testMigrationScenarioLegacyDecryptThenV2Encrypt() throws Exception { + // Mirrors PatchServicePasswordV2Migration_J10067.migrateValue(): decrypt an existing + // legacy-format row (self-contained, no external key needed), then re-encrypt it as v2 + // under the operator's configured key, and confirm the round trip reproduces the original. + char[] newKey = "operator-configured-key-2026".toCharArray(); + + // generateIvIfNeeded() returns a fresh/random IV on every call, so it must be generated + // once and reused for both the inner encrypted value and the outer metadata string below + // — a real legacy row always has the two agree (that's how it was originally written). + String iv = PasswordUtils.generateIvIfNeeded("PBEWITHHMACSHA512ANDAES_128"); + String legacyValue = PasswordUtils.encryptPassword(join("PBEWITHHMACSHA512ANDAES_128", "tzL1AKl5uc4NKYaoQ4P3WLGIBFPXWPWdu1fRm9004jtQiV", "f77aLYLo", "1000", + iv, "existingServicePassword")); + String legacyStoredValue = join("PBEWITHHMACSHA512ANDAES_128", "tzL1AKl5uc4NKYaoQ4P3WLGIBFPXWPWdu1fRm9004jtQiV", "f77aLYLo", "1000", + iv, legacyValue); + String decryptedPwd = PasswordUtils.decryptPassword(legacyStoredValue); + assertEquals("existingServicePassword", decryptedPwd); + String migratedValue = PasswordUtils.encryptPasswordV2(decryptedPwd, RangerSupportedCryptoAlgo.PBEWITHHMACSHA512ANDAES_128, newKey, "f77aLYLo".getBytes(), 1000); + assertTrue(PasswordUtils.isV2Format(migratedValue)); + assertNotEquals(legacyStoredValue, migratedValue); + assertEquals("existingServicePassword", PasswordUtils.decryptPasswordV2(migratedValue, newKey)); + } + + @Test + public void testDecryptV2RejectsEmptyOrNullKey() { + String storedValue = "v2,PBEWITHHMACSHA512ANDAES_128,c29tZXNhbHQ=,1000,someciphertext"; + assertThrows(IllegalArgumentException.class, () -> PasswordUtils.decryptPasswordV2(storedValue, null), "a null key must never be silently accepted for a v2 decrypt"); + assertThrows(IllegalArgumentException.class, () -> PasswordUtils.decryptPasswordV2(storedValue, new char[0]), "an empty key must never be silently accepted for a v2 decrypt"); + } + + @Test + public void testEncryptV2RejectsEmptyOrNullKey() { + assertThrows(IllegalArgumentException.class, () -> PasswordUtils.encryptPasswordV2("secretPasswordNoOneWillEverKnow", + RangerSupportedCryptoAlgo.PBEWITHHMACSHA512ANDAES_128, null, "f77aLYLo".getBytes(), 1000), + "a null key must never be silently accepted for a v2 encrypt"); + assertThrows(IllegalArgumentException.class, () -> PasswordUtils.encryptPasswordV2("secretPasswordNoOneWillEverKnow", + RangerSupportedCryptoAlgo.PBEWITHHMACSHA512ANDAES_128, new char[0], "f77aLYLo".getBytes(), 1000), + "an empty key must never be silently accepted for a v2 encrypt"); + } + + @Test + public void testValidateEncryptionKeyConfiguredRejectsDefaultKey() { + IllegalStateException ex = assertThrows(IllegalStateException.class, + () -> PasswordUtils.validateEncryptionKeyConfigured(PasswordUtils.DEFAULT_ENCRYPT_KEY.toCharArray()), + "the well-known default key must never be accepted as 'configured'"); + assertTrue(ex.getMessage().contains("ranger.password.encryption.key"), "the error should name the property an operator needs to set"); + } + + @Test + public void testValidateEncryptionKeyConfiguredRejectsNullOrEmptyKey() { + assertThrows(IllegalArgumentException.class, () -> PasswordUtils.validateEncryptionKeyConfigured(null)); + assertThrows(IllegalArgumentException.class, () -> PasswordUtils.validateEncryptionKeyConfigured(new char[0])); + } + + @Test + public void testValidateEncryptionKeyConfiguredAcceptsRealKey() { + assertDoesNotThrow(() -> PasswordUtils.validateEncryptionKeyConfigured("a-real-operator-configured-key".toCharArray())); + } + + @Test + public void testIsLegacyFormatDetectsRealLegacyValue() throws Exception { + String iv = PasswordUtils.generateIvIfNeeded("PBEWITHHMACSHA512ANDAES_128"); + String legacyMetadata = join("PBEWITHHMACSHA512ANDAES_128", "tzL1AKl5uc4NKYaoQ4P3WLGIBFPXWPWdu1fRm9004jtQiV", "f77aLYLo", "1000", iv); + String encryptedPassword = PasswordUtils.encryptPassword(join(legacyMetadata, "secretPasswordNoOneWillEverKnow")); + String legacyStoredValue = join(legacyMetadata, encryptedPassword); + assertTrue(PasswordUtils.isLegacyFormat(legacyStoredValue)); + } + + @Test + public void testIsLegacyFormatRejectsPlaintextWithComma() { + assertFalse(PasswordUtils.isLegacyFormat("my,password,with,commas")); + assertFalse(PasswordUtils.isLegacyFormat("plaintext-no-comma-at-all")); + assertFalse(PasswordUtils.isLegacyFormat("")); + assertFalse(PasswordUtils.isLegacyFormat(null)); + } + + @Test + public void testIsLegacyFormatRejectsV2Value() throws Exception { + String v2Value = PasswordUtils.encryptPasswordV2("secretPasswordNoOneWillEverKnow", RangerSupportedCryptoAlgo.PBEWITHHMACSHA512ANDAES_128, + "operator-configured-key-2026".toCharArray(), "f77aLYLo".getBytes(), 1000); + assertFalse(PasswordUtils.isLegacyFormat(v2Value), "a v2-format value is not legacy-format, even though it also contains commas"); + } + + @Test + public void testIsLegacyFormatRejectsTooFewFields() { + // Fewer than 5 comma-separated fields can't be a real "ALGO,KEY,SALT,ITER,CIPHERTEXT" value. + assertFalse(PasswordUtils.isLegacyFormat("PBEWithMD5AndDES,key,salt,4")); + } + + @Test + public void testIsV2FormatRejectsBarePrefixWithoutRealAlgorithm() { + assertFalse(PasswordUtils.isV2Format("v2,"), "prefix alone, no fields at all, must not count as v2"); + assertFalse(PasswordUtils.isV2Format("v2,notARealAlgorithm,rest,of,value"), "a made-up algorithm name must not count as v2"); + assertFalse(PasswordUtils.isV2Format("v2,thisIsJustAPlaintextPasswordThatHappensToStartWithTheV2Prefix")); + } + + @Test + public void testIsV2FormatAcceptsRealAlgorithmAfterPrefix() throws Exception { + String storedValue = PasswordUtils.encryptPasswordV2("secretPasswordNoOneWillEverKnow", RangerSupportedCryptoAlgo.PBEWITHMD5ANDDES, + "operator-configured-key-2026".toCharArray(), "f77aLYLo".getBytes(), 1000); + assertTrue(PasswordUtils.isV2Format(storedValue)); + } + + @Test + public void testDecryptV2WithIvAlgorithmAndTooFewFieldsThrowsIOException() { + // PBEWITHHMACSHA512ANDAES_128 needs an IV (5 fields: algo, salt, iter, iv, cipherText) — + // this value only has 4, mimicking corruption/truncation that dropped the IV field. + String malformedValue = "v2,PBEWITHHMACSHA512ANDAES_128,c29tZXNhbHQ=,1000,someciphertext"; + IOException ex = assertThrows(IOException.class, () -> PasswordUtils.decryptPasswordV2(malformedValue, "operator-configured-key-2026".toCharArray()), + "a truncated IV-requiring value must fail with IOException, never an unhandled ArrayIndexOutOfBoundsException"); + assertTrue(ex.getMessage().contains("IV"), "the error should point at the missing IV field"); + } + + @Test + public void testDecryptV2WithIvAlgorithmAndFullFieldsRoundTrips() throws Exception { + char[] key = "operator-configured-key-2026".toCharArray(); + String storedValue = PasswordUtils.encryptPasswordV2("secretPasswordNoOneWillEverKnow", RangerSupportedCryptoAlgo.PBEWITHHMACSHA512ANDAES_128, + key, "f77aLYLo".getBytes(), 1000); + assertEquals("secretPasswordNoOneWillEverKnow", PasswordUtils.decryptPasswordV2(storedValue, key)); + } + + @Test + public void testCheckNoLegacyEnvKeyOverrideDoesNotThrowWhenUnset() { + assertDoesNotThrow(PasswordUtils::checkNoLegacyEnvKeyOverride, + "in a normal environment (neither lEncryptKey nor ENCRYPT_SALT set), this must not throw"); + } + private String join(String... strings) { return Joiner.on(",").skipNulls().join(strings); } diff --git a/dev-support/checkstyle-suppressions.xml b/dev-support/checkstyle-suppressions.xml index f20f609e94..4154c5464e 100644 --- a/dev-support/checkstyle-suppressions.xml +++ b/dev-support/checkstyle-suppressions.xml @@ -93,4 +93,5 @@ + diff --git a/security-admin/db/mysql/optimized/current/ranger_core_db_mysql.sql b/security-admin/db/mysql/optimized/current/ranger_core_db_mysql.sql index cfc8fd645c..f59449e715 100644 --- a/security-admin/db/mysql/optimized/current/ranger_core_db_mysql.sql +++ b/security-admin/db/mysql/optimized/current/ranger_core_db_mysql.sql @@ -2007,4 +2007,5 @@ INSERT INTO x_db_version_h (version,inst_at,inst_by,updated_at,updated_by,active INSERT INTO x_db_version_h (version,inst_at,inst_by,updated_at,updated_by,active) VALUES ('J10064',UTC_TIMESTAMP(),'Ranger 3.0.0',UTC_TIMESTAMP(),'localhost','Y'); INSERT INTO x_db_version_h (version,inst_at,inst_by,updated_at,updated_by,active) VALUES ('J10065',UTC_TIMESTAMP(),'Ranger 3.0.0',UTC_TIMESTAMP(),'localhost','Y'); INSERT INTO x_db_version_h (version,inst_at,inst_by,updated_at,updated_by,active) VALUES ('J10066',UTC_TIMESTAMP(),'Ranger 3.0.0',UTC_TIMESTAMP(),'localhost','Y'); +INSERT INTO x_db_version_h (version,inst_at,inst_by,updated_at,updated_by,active) VALUES ('J10070',UTC_TIMESTAMP(),'Ranger 3.0.0',UTC_TIMESTAMP(),'localhost','Y'); INSERT INTO x_db_version_h (version,inst_at,inst_by,updated_at,updated_by,active) VALUES ('JAVA_PATCHES',UTC_TIMESTAMP(),'Ranger 1.0.0',UTC_TIMESTAMP(),'localhost','Y'); diff --git a/security-admin/db/oracle/optimized/current/ranger_core_db_oracle.sql b/security-admin/db/oracle/optimized/current/ranger_core_db_oracle.sql index 24f12a461e..c743dad254 100644 --- a/security-admin/db/oracle/optimized/current/ranger_core_db_oracle.sql +++ b/security-admin/db/oracle/optimized/current/ranger_core_db_oracle.sql @@ -2230,5 +2230,6 @@ INSERT INTO x_db_version_h (id,version,inst_at,inst_by,updated_at,updated_by,act INSERT INTO x_db_version_h (id,version,inst_at,inst_by,updated_at,updated_by,active) VALUES (X_DB_VERSION_H_SEQ.nextval,'J10064',sys_extract_utc(systimestamp),'Ranger 3.0.0',sys_extract_utc(systimestamp),'localhost','Y'); INSERT INTO x_db_version_h (id,version,inst_at,inst_by,updated_at,updated_by,active) VALUES (X_DB_VERSION_H_SEQ.nextval,'J10065',sys_extract_utc(systimestamp),'Ranger 3.0.0',sys_extract_utc(systimestamp),'localhost','Y'); INSERT INTO x_db_version_h (id,version,inst_at,inst_by,updated_at,updated_by,active) VALUES (X_DB_VERSION_H_SEQ.nextval,'J10066',sys_extract_utc(systimestamp),'Ranger 3.0.0',sys_extract_utc(systimestamp),'localhost','Y'); +INSERT INTO x_db_version_h (id,version,inst_at,inst_by,updated_at,updated_by,active) VALUES (X_DB_VERSION_H_SEQ.nextval,'J10070',sys_extract_utc(systimestamp),'Ranger 3.0.0',sys_extract_utc(systimestamp),'localhost','Y'); INSERT INTO x_db_version_h (id,version,inst_at,inst_by,updated_at,updated_by,active) VALUES (X_DB_VERSION_H_SEQ.nextval,'JAVA_PATCHES',sys_extract_utc(systimestamp),'Ranger 1.0.0',sys_extract_utc(systimestamp),'localhost','Y'); commit; diff --git a/security-admin/db/postgres/optimized/current/ranger_core_db_postgres.sql b/security-admin/db/postgres/optimized/current/ranger_core_db_postgres.sql index 8ef5bbca49..33c3c3dada 100644 --- a/security-admin/db/postgres/optimized/current/ranger_core_db_postgres.sql +++ b/security-admin/db/postgres/optimized/current/ranger_core_db_postgres.sql @@ -2163,6 +2163,7 @@ INSERT INTO x_db_version_h (version,inst_at,inst_by,updated_at,updated_by,active INSERT INTO x_db_version_h (version,inst_at,inst_by,updated_at,updated_by,active) VALUES ('J10064',current_timestamp,'Ranger 3.0.0',current_timestamp,'localhost','Y'); INSERT INTO x_db_version_h (version,inst_at,inst_by,updated_at,updated_by,active) VALUES ('J10065',current_timestamp,'Ranger 3.0.0',current_timestamp,'localhost','Y'); INSERT INTO x_db_version_h (version,inst_at,inst_by,updated_at,updated_by,active) VALUES ('J10066',current_timestamp,'Ranger 3.0.0',current_timestamp,'localhost','Y'); +INSERT INTO x_db_version_h (version,inst_at,inst_by,updated_at,updated_by,active) VALUES ('J10070',current_timestamp,'Ranger 3.0.0',current_timestamp,'localhost','Y'); INSERT INTO x_db_version_h (version,inst_at,inst_by,updated_at,updated_by,active) VALUES ('JAVA_PATCHES',current_timestamp,'Ranger 1.0.0',current_timestamp,'localhost','Y'); DROP VIEW IF EXISTS vx_principal; diff --git a/security-admin/db/sqlanywhere/optimized/current/ranger_core_db_sqlanywhere.sql b/security-admin/db/sqlanywhere/optimized/current/ranger_core_db_sqlanywhere.sql index e558454ffc..19582912d6 100644 --- a/security-admin/db/sqlanywhere/optimized/current/ranger_core_db_sqlanywhere.sql +++ b/security-admin/db/sqlanywhere/optimized/current/ranger_core_db_sqlanywhere.sql @@ -2394,6 +2394,8 @@ INSERT INTO x_db_version_h (version,inst_at,inst_by,updated_at,updated_by,active GO INSERT INTO x_db_version_h (version,inst_at,inst_by,updated_at,updated_by,active) VALUES ('J10066',CURRENT_TIMESTAMP,'Ranger 3.0.0',CURRENT_TIMESTAMP,'localhost','Y'); GO +INSERT INTO x_db_version_h (version,inst_at,inst_by,updated_at,updated_by,active) VALUES ('J10070',CURRENT_TIMESTAMP,'Ranger 3.0.0',CURRENT_TIMESTAMP,'localhost','Y'); +GO INSERT INTO x_db_version_h (version,inst_at,inst_by,updated_at,updated_by,active) VALUES ('JAVA_PATCHES',CURRENT_TIMESTAMP,'Ranger 1.0.0',CURRENT_TIMESTAMP,'localhost','Y'); GO exit diff --git a/security-admin/db/sqlserver/optimized/current/ranger_core_db_sqlserver.sql b/security-admin/db/sqlserver/optimized/current/ranger_core_db_sqlserver.sql index 4b83351513..1d4bd6a104 100644 --- a/security-admin/db/sqlserver/optimized/current/ranger_core_db_sqlserver.sql +++ b/security-admin/db/sqlserver/optimized/current/ranger_core_db_sqlserver.sql @@ -4596,5 +4596,6 @@ INSERT INTO x_db_version_h (version,inst_at,inst_by,updated_at,updated_by,active INSERT INTO x_db_version_h (version,inst_at,inst_by,updated_at,updated_by,active) VALUES ('J10064',CURRENT_TIMESTAMP,'Ranger 3.0.0',CURRENT_TIMESTAMP,'localhost','Y'); INSERT INTO x_db_version_h (version,inst_at,inst_by,updated_at,updated_by,active) VALUES ('J10065',CURRENT_TIMESTAMP,'Ranger 3.0.0',CURRENT_TIMESTAMP,'localhost','Y'); INSERT INTO x_db_version_h (version,inst_at,inst_by,updated_at,updated_by,active) VALUES ('J10066',CURRENT_TIMESTAMP,'Ranger 3.0.0',CURRENT_TIMESTAMP,'localhost','Y'); +INSERT INTO x_db_version_h (version,inst_at,inst_by,updated_at,updated_by,active) VALUES ('J10070',CURRENT_TIMESTAMP,'Ranger 3.0.0',CURRENT_TIMESTAMP,'localhost','Y'); INSERT INTO x_db_version_h (version,inst_at,inst_by,updated_at,updated_by,active) VALUES ('JAVA_PATCHES',CURRENT_TIMESTAMP,'Ranger 1.0.0',CURRENT_TIMESTAMP,'localhost','Y'); GO 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 ee16980d72..dc92b932bc 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 @@ -694,9 +694,8 @@ private Subject getSubjectForKerberos(String provider) throws Exception { private String getKMSPassword(String srvName) throws Exception { XXService rangerService = rangerDaoManagerBase.getXXService().findByName(srvName); XXServiceConfigMap xxConfigMap = rangerDaoManagerBase.getXXServiceConfigMap().findByServiceAndConfigKey(rangerService.getId(), KMS_PASSWORD); - String encryptedPwd = xxConfigMap.getConfigvalue(); - - return PasswordUtils.decryptPassword(encryptedPwd); + String storedValue = xxConfigMap.getConfigvalue(); + return PasswordUtils.isV2Format(storedValue) ? PasswordUtils.decryptPasswordV2(storedValue, ServiceDBStore.ENCRYPT_KEY.toCharArray()) : PasswordUtils.decryptPassword(storedValue); } private String getKMSUserName(String srvName) throws Exception { diff --git a/security-admin/src/main/java/org/apache/ranger/biz/PasswordEncryptionKeyConsistencyChecker.java b/security-admin/src/main/java/org/apache/ranger/biz/PasswordEncryptionKeyConsistencyChecker.java new file mode 100644 index 0000000000..fad14d053c --- /dev/null +++ b/security-admin/src/main/java/org/apache/ranger/biz/PasswordEncryptionKeyConsistencyChecker.java @@ -0,0 +1,136 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.ranger.biz; + +import org.apache.ranger.db.RangerDaoManager; +import org.apache.ranger.entity.XXServiceConfigMap; +import org.apache.ranger.plugin.util.PasswordUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Lazy; +import org.springframework.stereotype.Component; + +import javax.annotation.PostConstruct; + +import java.util.List; + +/** + * RANGER-5773 (v1->v2 service-config password format): a v2-format value is decrypted using + * whichever key THIS process has configured for {@code ranger.password.encryption.key} - unlike + * legacy v1, the key is never stored with the value, so it is no longer self-describing. Before + * this fix, a mismatched key across Admin nodes in an HA deployment was silently tolerated (each + * node read the key back out of the row it was decrypting); after this fix, every node MUST agree + * on the same key, or a node with a different key cannot decrypt service passwords that another + * node in the same cluster wrote. Nothing enforced that agreement before this class - operators + * would only discover a mismatch when a "test connection" or resource lookup unexpectedly failed + * on one specific node, which is a hard failure mode to diagnose after the fact. (That failure + * mode is no longer silent either - see RangerServiceService.getConfigsWithDecryptedPassword(), + * which now throws an actionable error, with this same remediation text, at the point of use + * instead of quietly leaving the "*****" mask in place.) + *

+ * This check is deliberately opportunistic rather than authoritative: it does not introduce any + * new stored state (no new table, no persisted fingerprint) - it simply tries to decrypt one + * already-migrated v2 row, using whatever this node's configured key is, at startup. If some other + * node already wrote v2 data under a different key - including a key that was rotated on some + * nodes but not re-encrypted everywhere - this node will fail to decrypt it and log a loud, + * actionable warning. It cannot catch every case (e.g. the very first node to write a v2 value + * under a since-changed key, before any other node has restarted to notice) - it catches the + * realistic, steady-state case: a node (re)starts and can't decrypt what the cluster already has. + *

+ * Design decision flagged for reviewer sign-off, not silently assumed: this WARNS and lets startup + * continue, rather than failing startup outright. A hard failure would guarantee the mismatch is + * never missed, but risks turning a legitimate transient state (e.g. a deliberate key-rotation + * window someone is midway through) into an outage. Kept as fail-open/warn-only to match this + * fix's overall risk posture (see PatchServicePasswordV2Migration_J10070's per-row fail-soft + * choice for the same reasoning) - worth a second opinion before this ships, not a unilateral call. + *

+ * Looks up one v2-format row via a targeted, indexed-friendly {@code configvalue LIKE 'v2,%'} + * query (XXServiceConfigMapDao.findByConfigValuePrefix(), LIMIT 1) rather than loading the entire + * x_service_config_map table - unlike the migration patch, which genuinely needs every row (it + * migrates all of them), this check only ever needs one, so there is no reason to pay for a full + * scan on every Admin process startup. + * + * @Lazy(false) is required, not decorative: this context's applicationContext.xml sets + * default-lazy-init="true" for every bean, and nothing else in the codebase ever autowires or + * otherwise references this class - a startup-only checker is by design never looked up by + * anything else. Under the context default, that combination means Spring would never construct + * this bean at all, so @PostConstruct would silently never fire and this check would never run + * in a real deployment. Confirmed by testing against a live docker environment: without this + * annotation, checkKeyConsistency() produced zero log output at any level - not even the + * fail-open outer catch - because the bean itself was never instantiated. + */ +@Lazy(value = false) +@Component +public class PasswordEncryptionKeyConsistencyChecker { + private static final Logger LOG = LoggerFactory.getLogger(PasswordEncryptionKeyConsistencyChecker.class); + + @Autowired + RangerDaoManager daoMgr; + + @PostConstruct + public void checkKeyConsistency() { + checkEncryptionKeyNotDefault(); + try { + List v2ConfigMaps = daoMgr.getXXServiceConfigMap().findByConfigValuePrefix("v2,", 1); + if (v2ConfigMaps.isEmpty()) { + LOG.debug("Password encryption key consistency check skipped: no v2-format service config data found yet."); + return; + } + XXServiceConfigMap configMap = v2ConfigMaps.get(0); + try { + PasswordUtils.decryptPasswordV2(configMap.getConfigvalue(), ServiceDBStore.ENCRYPT_KEY.toCharArray()); + LOG.debug("Password encryption key consistency check passed: this node can decrypt existing v2-format service config data."); + } catch (Exception e) { + // Deliberately do not log configMap.getConfigkey()'s value, the stored value, or + // any part of the key - only enough to point an operator at the right service and + // the right node. + LOG.warn("This Admin node ({}, config path ranger-admin-site.xml) failed to decrypt an existing v2-format service config value " + + "(serviceId=[{}]) using its configured ranger.password.encryption.key. {}", + ServiceDBStore.LOCAL_HOSTNAME, configMap.getServiceId(), ServiceDBStore.ENCRYPT_KEY_MISMATCH_REMEDIATION, e); + } + } catch (Exception e) { + // This check must never block Admin startup on its own account, even if it can't run at all. + LOG.warn("Password encryption key consistency check could not run at startup - continuing without it.", e); + } + } + + /** + * Loudly WARNs, once per Admin startup, if this node would encrypt (or already has encrypted) + * service config passwords under the default, publicly-known key that ships with every Ranger + * install - i.e. {@code ranger.password.encryption.key} was never actually set. Deliberately + * WARN rather than a hard failure on every service create/update: the write path itself + * (ServiceDBStore.createService()/updateService()) does not refuse a default key today, so + * making startup itself fail here would not stop those writes anyway, only make the + * misconfiguration harder to fix (an Admin that won't start can't have its config corrected + * through its own UI/API). PatchServicePasswordV2Migration_J10070 is the one place in this fix + * that DOES hard-refuse on a default/unset key (see PasswordUtils.validateEncryptionKeyConfigured()) + * - that migration is a deliberate, one-time, operator-triggered action with no legacy fallback + * to preserve, unlike an interactive service create/update call. + */ + private void checkEncryptionKeyNotDefault() { + try { + PasswordUtils.validateEncryptionKeyConfigured(ServiceDBStore.ENCRYPT_KEY.toCharArray()); + } catch (Exception e) { + LOG.warn("This Admin node ({}, config path ranger-admin-site.xml) does not have ranger.password.encryption.key set to a real value - service " + + "config passwords are being encrypted (or already were) under the default, publicly-known key that ships with every Ranger " + + "install, which protects nothing. Set ranger.password.encryption.key to a unique, secret value as soon as possible - the same " + + "value on every Admin node in this cluster.", + ServiceDBStore.LOCAL_HOSTNAME); + } + } +} diff --git a/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java b/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java index 0d3f5bc1bc..7f2de80e09 100644 --- a/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java +++ b/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java @@ -24,7 +24,6 @@ import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.SerializationUtils; import org.apache.commons.lang3.StringUtils; -import org.apache.hadoop.thirdparty.com.google.common.base.Joiner; import org.apache.poi.hssf.usermodel.HSSFWorkbook; import org.apache.poi.ss.usermodel.Cell; import org.apache.poi.ss.usermodel.CellStyle; @@ -141,6 +140,7 @@ import org.apache.ranger.plugin.util.RangerCommonConstants; import org.apache.ranger.plugin.util.RangerPolicyDeltaUtil; import org.apache.ranger.plugin.util.RangerPurgeResult; +import org.apache.ranger.plugin.util.RangerSupportedCryptoAlgo; import org.apache.ranger.plugin.util.SearchFilter; import org.apache.ranger.plugin.util.ServiceDefUtil; import org.apache.ranger.plugin.util.ServicePolicies; @@ -195,6 +195,7 @@ import java.io.InputStream; import java.io.OutputStream; import java.net.UnknownHostException; +import java.nio.charset.StandardCharsets; import java.text.DateFormat; import java.text.SimpleDateFormat; import java.util.ArrayList; @@ -232,6 +233,9 @@ public class ServiceDBStore extends AbstractServiceStore { public static final String RANGER_PLUGIN_AUDIT_FILTERS = "ranger.plugin.audit.filters"; public static final String RANGER_PLUGINS_CONFIG_CONF_PREFIX = "ranger.plugins.conf."; public static final String HIDDEN_PASSWORD_STR = "*****"; + public static final String ENCRYPT_KEY_MISMATCH_REMEDIATION = "In a multi-node deployment, ALL Admin nodes must share the exact same value for ranger.password.encryption.key." + + "If they don't — including if the key was rotated on some nodes without re-encrypting existing data — service passwords written or migrated under a different key will " + + "not work on this node. Check that ranger.password.encryption.key is identical across every Admin node in this cluster."; public static final String CONFIG_KEY_PASSWORD = "password"; public static final String CONFIG_TYPE_PASSWORD = "password"; public static final String ACCESS_TYPE_DECRYPT_EEK = "decrypteek"; @@ -269,7 +273,7 @@ public class ServiceDBStore extends AbstractServiceStore { public static Integer TRANSACTION_RECORDS_RETENTION_PERIOD_IN_DAYS; public static boolean SUPPORTS_PURGE_POLICY_EXPORT_LOGS; public static Integer POLICY_EXPORT_LOGS_RETENTION_PERIOD_IN_DAYS; - private static String LOCAL_HOSTNAME; + public static String LOCAL_HOSTNAME; private static boolean isRolesDownloadedByService; private static volatile boolean legacyServiceDefsInitDone; private final String optionUgsyncConfigChange = "ugsyncConfigChange"; @@ -1008,16 +1012,14 @@ public RangerService createService(RangerService service) throws Exception { } if (isPasswordConfigKey(passwordConfigKeys, configKey)) { - Joiner joiner = Joiner.on(",").skipNulls(); - String iv = PasswordUtils.generateIvIfNeeded(CRYPT_ALGO); - String cryptConfigString = joiner.join(CRYPT_ALGO, ENCRYPT_KEY, SALT, ITERATION_COUNT, iv, configValue); - String encryptedPwd = PasswordUtils.encryptPassword(cryptConfigString); - String paddedEncryptedPwd = joiner.join(CRYPT_ALGO, ENCRYPT_KEY, SALT, ITERATION_COUNT, iv, encryptedPwd); - String decryptedPwd = PasswordUtils.decryptPassword(paddedEncryptedPwd); + String storedValue = PasswordUtils.encryptPasswordV2(configValue, RangerSupportedCryptoAlgo.getValueOf(CRYPT_ALGO), ENCRYPT_KEY.toCharArray(), SALT.getBytes(StandardCharsets.UTF_8), ITERATION_COUNT); + String decryptedPwd = PasswordUtils.decryptPasswordV2(storedValue, ENCRYPT_KEY.toCharArray()); - if (StringUtils.equals(decryptedPwd, configValue)) { - configValue = paddedEncryptedPwd; + if (!StringUtils.equals(decryptedPwd, configValue)) { + throw new IllegalStateException("Failed to verify v2-encrypted value for password config key [" + configKey + "] on service [" + service.getName() + "] — refusing to store it"); } + + configValue = storedValue; } XXServiceConfigMap xConfMap = new XXServiceConfigMap(); @@ -1201,17 +1203,19 @@ public RangerService updateService(RangerService service, Map op if (StringUtils.equalsIgnoreCase(configValue, HIDDEN_PASSWORD_STR)) { if (oldPassword != null && oldPassword.contains(",")) { - PasswordUtils util = PasswordUtils.build(oldPassword); + boolean oldIsV2 = PasswordUtils.isV2Format(oldPassword); + String oldCryptAlgo = oldIsV2 ? PasswordUtils.getCryptAlgoV2(oldPassword) : PasswordUtils.build(oldPassword).getCryptAlgo(); - if (!util.getCryptAlgo().equalsIgnoreCase(CRYPT_ALGO)) { - String decryptedPwd = PasswordUtils.decryptPassword(oldPassword); - String paddingString = Joiner.on(",").skipNulls().join(CRYPT_ALGO, new String(util.getEncryptKey()), new String(util.getSalt()), util.getIterationCount(), PasswordUtils.generateIvIfNeeded(CRYPT_ALGO)); - String encryptedPwd = PasswordUtils.encryptPassword(paddingString + "," + decryptedPwd); - String newDecryptedPwd = PasswordUtils.decryptPassword(paddingString + "," + encryptedPwd); + if (!oldCryptAlgo.equalsIgnoreCase(CRYPT_ALGO)) { + String decryptedPwd = oldIsV2 ? PasswordUtils.decryptPasswordV2(oldPassword, ENCRYPT_KEY.toCharArray()) : PasswordUtils.decryptPassword(oldPassword); + String newStoredValue = PasswordUtils.encryptPasswordV2(decryptedPwd, RangerSupportedCryptoAlgo.getValueOf(CRYPT_ALGO), ENCRYPT_KEY.toCharArray(), SALT.getBytes(StandardCharsets.UTF_8), ITERATION_COUNT); + String newDecryptedPwd = PasswordUtils.decryptPasswordV2(newStoredValue, ENCRYPT_KEY.toCharArray()); - if (StringUtils.equals(newDecryptedPwd, decryptedPwd)) { - configValue = paddingString + "," + encryptedPwd; + if (!StringUtils.equals(newDecryptedPwd, decryptedPwd)) { + throw new IllegalStateException("Failed to verify re-encrypted value for password config key [" + configKey + "] on service [" + service.getName() + "] — refusing to store it"); } + + configValue = newStoredValue; } else { configValue = oldPassword; } @@ -1219,13 +1223,14 @@ public RangerService updateService(RangerService service, Map op configValue = oldPassword; } } else { - String paddingString = Joiner.on(",").skipNulls().join(CRYPT_ALGO, ENCRYPT_KEY, SALT, ITERATION_COUNT, PasswordUtils.generateIvIfNeeded(CRYPT_ALGO)); - String encryptedPwd = PasswordUtils.encryptPassword(paddingString + "," + configValue); - String decryptedPwd = PasswordUtils.decryptPassword(paddingString + "," + encryptedPwd); + String storedValue = PasswordUtils.encryptPasswordV2(configValue, RangerSupportedCryptoAlgo.getValueOf(CRYPT_ALGO), ENCRYPT_KEY.toCharArray(), SALT.getBytes(StandardCharsets.UTF_8), ITERATION_COUNT); + String decryptedPwd = PasswordUtils.decryptPasswordV2(storedValue, ENCRYPT_KEY.toCharArray()); - if (StringUtils.equals(decryptedPwd, configValue)) { - configValue = paddingString + "," + encryptedPwd; + if (!StringUtils.equals(decryptedPwd, configValue)) { + throw new IllegalStateException("Failed to verify v2-encrypted value for password config key [" + configKey + "] on service [" + service.getName() + "] — refusing to store it"); } + + configValue = storedValue; } } diff --git a/security-admin/src/main/java/org/apache/ranger/db/XXServiceConfigMapDao.java b/security-admin/src/main/java/org/apache/ranger/db/XXServiceConfigMapDao.java index ffc99eb007..b94fd536da 100644 --- a/security-admin/src/main/java/org/apache/ranger/db/XXServiceConfigMapDao.java +++ b/security-admin/src/main/java/org/apache/ranger/db/XXServiceConfigMapDao.java @@ -17,6 +17,7 @@ package org.apache.ranger.db; +import org.apache.commons.lang3.StringUtils; import org.apache.ranger.common.db.BaseDao; import org.apache.ranger.entity.XXServiceConfigMap; import org.apache.ranger.services.tag.RangerServiceTag; @@ -95,6 +96,28 @@ public List findServiceIdsByClusterName(String clusterName) { return findServiceIdsByConfigKeyAndConfigValueFilterByServiceType(SERVICE_CLUSTER_NAME_CONF_KEY, clusterName, RangerServiceTag.TAG_RESOURCE_NAME); } + /** + * Returns up to {@code maxResults} rows whose configValue starts with {@code prefix} — an + * indexed-friendly {@code LIKE 'prefix%'} query, not a full-table scan. Used by + * PasswordEncryptionKeyConsistencyChecker to find one v2-format ("v2,") row cheaply at every + * Admin startup, instead of loading the entire x_service_config_map table just to find the + * first row matching a prefix. + */ + public List findByConfigValuePrefix(String prefix, int maxResults) { + if (StringUtils.isEmpty(prefix)) { + return Collections.emptyList(); + } + try { + return getEntityManager() + .createNamedQuery("XXServiceConfigMap.findByConfigValueLike", tClass) + .setParameter("configValuePrefix", prefix + "%") + .setMaxResults(maxResults) + .getResultList(); + } catch (NoResultException e) { + return Collections.emptyList(); + } + } + public List findByConfigKey(String configKey) { if (configKey == null) { return Collections.emptyList(); diff --git a/security-admin/src/main/java/org/apache/ranger/patch/PatchServicePasswordV2Migration_J10070.java b/security-admin/src/main/java/org/apache/ranger/patch/PatchServicePasswordV2Migration_J10070.java new file mode 100644 index 0000000000..1b4ca62115 --- /dev/null +++ b/security-admin/src/main/java/org/apache/ranger/patch/PatchServicePasswordV2Migration_J10070.java @@ -0,0 +1,237 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.ranger.patch; + +import org.apache.commons.lang3.StringUtils; +import org.apache.ranger.biz.ServiceDBStore; +import org.apache.ranger.db.RangerDaoManager; +import org.apache.ranger.entity.XXService; +import org.apache.ranger.entity.XXServiceConfigMap; +import org.apache.ranger.plugin.util.PasswordUtils; +import org.apache.ranger.plugin.util.RangerSupportedCryptoAlgo; +import org.apache.ranger.util.CLIUtil; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Component; + +import java.nio.charset.StandardCharsets; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; + +/** + * One-time upgrade migration for RANGER-5773 (service-config credential encryption key was + * stored alongside its own ciphertext). The code-level fix (see PasswordUtils.encryptPasswordV2/ + * decryptPasswordV2 and their callers in ServiceDBStore/RangerServiceService) only changes how + * *new* writes are stored — every password already in x_service_config_map before this patch + * runs still carries the vulnerable v1 format, with the key embedded in the same row as its + * ciphertext. This patch is what actually closes that exposure for existing data: it decrypts + * every password-type config value still in the legacy format and re-writes it in the v2 format + * (key sourced from configuration, never stored). + *

+ * Modeled on {@link PatchPasswordEncryption_J10001}, which did the equivalent migration for the + * legacy x_asset config column when password encryption was first introduced. + *

+ * Design notes worth a reviewer's attention rather than being silently assumed: + * - Single-pass, single-transaction, like PatchPasswordEncryption_J10001 (batchSize is not set, + * so BaseLoader runs execLoad() exactly once and commits at the end). For a very large + * x_service_config_map this is a real limitation (lock/timeout risk) — matches existing + * precedent, but is worth a second opinion on very large deployments before this ships, + * rather than assuming precedent alone settles it. + * - Per-row failures are caught and logged, NOT allowed to abort the whole patch — this covers + * the full per-row body (this row's XXService/service-def lookups included, not just the + * decrypt/re-encrypt/write step) precisely so a transient DAO issue on one row can't do it + * either. This is a deliberate improvement over the precedent (which has no per-row error + * handling at all, so a single bad row would abort the entire migration via BaseLoader's + * top-level catch). A row that fails to migrate is simply left in v1 format — still fully + * functional, just not yet protected — rather than blocking every other row from being + * migrated. + * - Idempotent / safe to re-run: rows already in v2 format (isV2Format()) are skipped, so this + * patch can be re-run (e.g. after fixing a config issue that caused failures) without + * re-encrypting already-migrated rows. + * - Config (key + algorithm) is validated ONCE up front, before any row is touched — see + * {@link #validateMigrationConfig()} — rather than letting a bad config burn every single row + * as its own per-row failure with the same root cause. + * - A row is only treated as "legacy-format, needs migrating" when {@code + * PasswordUtils.isLegacyFormat()} recognizes it (first field names a real crypto algorithm), + * not merely because its stored value contains a comma. The old, looser + * {@code storedValue.contains(",")} heuristic could misclassify a plaintext password that + * happens to contain a comma as "already encrypted" and attempt to decrypt/re-encrypt it — + * rows that don't match either recognized format are counted as notEncrypted and left alone. + */ +@Component +public class PatchServicePasswordV2Migration_J10070 extends BaseLoader { + private static final Logger logger = LoggerFactory.getLogger(PatchServicePasswordV2Migration_J10070.class); + + @Autowired + RangerDaoManager daoMgr; + + int lineCount; + int migratedCount; + int alreadyV2Count; + int notPasswordCount; + int notEncryptedCount; + int orphanedCount; + int failedCount; + + public PatchServicePasswordV2Migration_J10070() { + } + + public static void main(String[] args) { + logger.info("main()"); + + try { + PatchServicePasswordV2Migration_J10070 loader = (PatchServicePasswordV2Migration_J10070) CLIUtil.getBean(PatchServicePasswordV2Migration_J10070.class); + loader.init(); + while (loader.isMoreToProcess()) { + loader.load(); + } + logger.info("Load complete. Exiting!!!"); + System.exit(0); + } catch (Exception e) { + logger.error("Error loading", e); + System.exit(1); + } + } + + @Override + public void printStats() { + logger.info("Time taken so far:{}, moreToProcess={}", timeTakenSoFar(lineCount), isMoreToProcess()); + print(lineCount, "Processed config rows"); + } + + @Override + public void execLoad() { + migrateServicePasswordsToV2(); + } + + void migrateServicePasswordsToV2() { + validateMigrationConfig(); // fail fast, once, with one clear message — see javadoc below + List allConfigMaps = daoMgr.getXXServiceConfigMap().getAll(); + Map> passwordConfigKeysByServiceDefId = new HashMap<>(); + + for (XXServiceConfigMap configMap : allConfigMaps) { + lineCount++; + + // Everything below is per-row and deliberately inside one try/catch, not just the + // crypto/write step at the bottom - a row-specific DAO lookup failure here (e.g. a + // transient issue resolving this row's XXService or its service-def's password config + // keys) must not be allowed to abort the whole patch and roll back every row already + // migrated in this run, any more than a decrypt/encrypt failure should. Only + // daoMgr.getXXServiceConfigMap().getAll() above stays outside - that one operation + // isn't "per-row", there's no partial-row granularity to preserve if it fails. + XXService xService = null; + + try { + xService = daoMgr.getXXService().getById(configMap.getServiceId()); + + if (xService == null) { + logger.warn("Skipping config row [{}] — no service found for serviceId [{}] (orphaned row)", configMap.getId(), configMap.getServiceId()); + orphanedCount++; + continue; + } + + Set passwordConfigKeys = passwordConfigKeysByServiceDefId.computeIfAbsent(xService.getType(), + serviceDefId -> ServiceDBStore.getPasswordConfigKeys(daoMgr.getXXServiceConfigDef().findConfigNamesByServiceDefIdAndType(serviceDefId, ServiceDBStore.CONFIG_TYPE_PASSWORD))); + + if (!ServiceDBStore.isPasswordConfigKey(passwordConfigKeys, configMap.getConfigkey())) { + notPasswordCount++; + continue; + } + + String configValue = configMap.getConfigvalue(); + boolean isV2 = PasswordUtils.isV2Format(configValue); + + if (!isV2 && !PasswordUtils.isLegacyFormat(configValue)) { + logger.warn("Password-type config key [{}] on service [{}] (id={}) does not look encrypted (neither legacy nor v2 format) — it may be " + + "stored in plaintext. Left unchanged by this migration; investigate and re-save the service config to encrypt it.", + configMap.getConfigkey(), xService.getName(), xService.getId()); + notEncryptedCount++; + continue; + } + + if (isV2) { + alreadyV2Count++; + continue; // already migrated — safe to re-run this patch + } + + String newStoredValue = migrateValue(configValue, RangerSupportedCryptoAlgo.getValueOf(ServiceDBStore.CRYPT_ALGO), + ServiceDBStore.ENCRYPT_KEY.toCharArray(), ServiceDBStore.SALT.getBytes(StandardCharsets.UTF_8), ServiceDBStore.ITERATION_COUNT); + + configMap.setConfigvalue(newStoredValue); + + daoMgr.getXXServiceConfigMap().update(configMap); + + migratedCount++; + } catch (Exception e) { + // Leave this row in v1 format — it keeps working via the untouched legacy + // decrypt path; it just isn't protected by this fix until it's retried. Applies + // equally whether the failure was the crypto/write step or an earlier lookup for + // this row - either way, only this one row is affected. + String serviceLabel = xService != null ? xService.getName() : ("serviceId=" + configMap.getServiceId()); + + logger.error("Failed to migrate password to v2 for service [{}], configKey [{}] — row left in legacy format", serviceLabel, configMap.getConfigkey(), e); + + failedCount++; + } + } + + setMoreToProcess(false); + + logger.info("Password v1->v2 migration complete: migrated={}, alreadyV2={}, notPasswordConfig={}, notEncryptedOrPlaintext={}, orphaned={}, failed={} (total rows seen={})", + migratedCount, alreadyV2Count, notPasswordCount, notEncryptedCount, orphanedCount, failedCount, lineCount); + } + + /** + * Fails the whole patch run immediately, before touching a single row, if this node's + * password-encryption configuration can't actually produce valid v2 output — an unset/default + * {@code ranger.password.encryption.key}, or a {@code ranger.password.encryption.algorithm} + * value that doesn't name a supported algorithm. Without this up-front check, a bad config + * doesn't fail cleanly: every single candidate row fails individually inside the per-row + * try/catch in {@link #migrateServicePasswordsToV2()}, each logged as its own "Failed to + * migrate password..." error with the same root cause, and failedCount ends up misleadingly + * large. A configuration problem should stop the whole patch with ONE clear, actionable + * message instead of N confusing per-row ones. + */ + private static void validateMigrationConfig() { + PasswordUtils.validateEncryptionKeyConfigured(ServiceDBStore.ENCRYPT_KEY.toCharArray()); + PasswordUtils.checkNoLegacyEnvKeyOverride(); + + RangerSupportedCryptoAlgo.getValueOf(ServiceDBStore.CRYPT_ALGO); // throws if not a supported algorithm name + } + + /** + * Pure migration logic for one stored value, split out from the DAO/Spring plumbing above so + * it can be exercised directly in a test: decrypt via the legacy (self-contained) path, + * re-encrypt via v2 with the supplied key, and verify the round-trip before trusting it. + * Throws on any failure (wrong/missing key, corrupt data, decrypt/encrypt error) — callers + * must treat that as "leave the row unmigrated," never as a signal to fall back silently. + */ + static String migrateValue(String legacyStoredValue, RangerSupportedCryptoAlgo cryptAlgo, char[] key, byte[] salt, int iterationCount) throws Exception { + String decryptedPwd = PasswordUtils.decryptPassword(legacyStoredValue); + String newStoredValue = PasswordUtils.encryptPasswordV2(decryptedPwd, cryptAlgo, key, salt, iterationCount); + String verifyDecrypted = PasswordUtils.decryptPasswordV2(newStoredValue, key); + + if (!StringUtils.equals(decryptedPwd, verifyDecrypted)) { + throw new IllegalStateException("v2 round-trip verification did not reproduce the original decrypted value"); + } + + return newStoredValue; + } +} diff --git a/security-admin/src/main/java/org/apache/ranger/service/RangerServiceService.java b/security-admin/src/main/java/org/apache/ranger/service/RangerServiceService.java index ce704ba880..0dd3301e9a 100644 --- a/security-admin/src/main/java/org/apache/ranger/service/RangerServiceService.java +++ b/security-admin/src/main/java/org/apache/ranger/service/RangerServiceService.java @@ -33,6 +33,7 @@ import org.springframework.context.annotation.Scope; import org.springframework.stereotype.Service; +import java.io.IOException; import java.util.ArrayList; import java.util.Date; import java.util.HashMap; @@ -94,19 +95,31 @@ public Map getConfigsWithDecryptedPassword(RangerService service if (pwdConfig != null) { String encryptedPwd = pwdConfig.getConfigvalue(); - if (encryptedPwd.contains(",")) { + + if (PasswordUtils.isV2Format(encryptedPwd)) { + String decryptedPwd; + try { + decryptedPwd = PasswordUtils.decryptPasswordV2(encryptedPwd, ServiceDBStore.ENCRYPT_KEY.toCharArray()); + } catch (IllegalArgumentException e) { + throw new Exception("Stored password value for config key [" + configKey + "] on service [" + service.getName() + + "] is malformed or corrupted — this is a data integrity problem, not a key mismatch.", e); + } catch (IOException e) { + throw new Exception("Failed to decrypt password for config key [" + configKey + "] on service [" + service.getName() + + "] using this node's configured ranger.password.encryption.key. " + ServiceDBStore.ENCRYPT_KEY_MISMATCH_REMEDIATION, e); + } + + configs.put(configKey, decryptedPwd); + } else if (encryptedPwd.contains(",")) { PasswordUtils util = PasswordUtils.build(encryptedPwd); String freeTextPasswordMetaData = Joiner.on(",").skipNulls().join(util.getCryptAlgo(), new String(util.getEncryptKey()), new String(util.getSalt()), util.getIterationCount(), PasswordUtils.needsIv(util.getCryptAlgo()) ? util.getIvAsString() : null); String decryptedPwd = PasswordUtils.decryptPassword(encryptedPwd); if (StringUtils.equalsIgnoreCase(freeTextPasswordMetaData + "," + PasswordUtils.encryptPassword(freeTextPasswordMetaData + "," + decryptedPwd), encryptedPwd)) { - configs.put(configKey, encryptedPwd); - // XXX: method name is getConfigsWithDecryptedPassword, then why do we store the encryptedPwd? + configs.put(configKey, decryptedPwd); } } else { String decryptedPwd = PasswordUtils.decryptPassword(encryptedPwd); if (StringUtils.equalsIgnoreCase(PasswordUtils.encryptPassword(decryptedPwd), encryptedPwd)) { - configs.put(configKey, encryptedPwd); - // XXX: method name is getConfigsWithDecryptedPassword, then why do we store the encryptedPwd? + configs.put(configKey, decryptedPwd); } } } diff --git a/security-admin/src/main/resources/META-INF/jpa_named_queries.xml b/security-admin/src/main/resources/META-INF/jpa_named_queries.xml index 1dcffe9aee..d797572b3c 100755 --- a/security-admin/src/main/resources/META-INF/jpa_named_queries.xml +++ b/security-admin/src/main/resources/META-INF/jpa_named_queries.xml @@ -736,6 +736,10 @@ select obj from XXServiceConfigMap obj where obj.configKey = :configKey + + select obj from XXServiceConfigMap obj where obj.configValue LIKE :configValuePrefix + + select obj from XXServiceConfigMap obj where obj.serviceId = :serviceId and obj.configKey = :configKey diff --git a/security-admin/src/test/java/org/apache/ranger/biz/TestKmsKeyMgr.java b/security-admin/src/test/java/org/apache/ranger/biz/TestKmsKeyMgr.java index 9b9a38dfd1..9f9c725738 100644 --- a/security-admin/src/test/java/org/apache/ranger/biz/TestKmsKeyMgr.java +++ b/security-admin/src/test/java/org/apache/ranger/biz/TestKmsKeyMgr.java @@ -61,6 +61,7 @@ import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; +import java.io.IOException; import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; @@ -881,6 +882,62 @@ public void testGetKMSPassword() throws Exception { } } + @Test + public void testGetKMSPassword_V2Format() throws Exception { + // getKMSPassword() reads x_service_config_map directly, bypassing + // RangerServiceService.getConfigsWithDecryptedPassword() - it needs its own + // isV2Format(...) ? decryptPasswordV2(...) : decryptPassword(...) dispatch, exercised here. + XXService xxService = new XXService(); + xxService.setId(1L); + XXServiceConfigMap xxServiceConfigMap = new XXServiceConfigMap(); + xxServiceConfigMap.setConfigvalue("v2,PBEWithMD5AndTripleDES,c29tZXNhbHQ=,17,ZW5jcnlwdGVk"); + + try (MockedStatic passwordUtilsMock = Mockito.mockStatic(PasswordUtils.class)) { + passwordUtilsMock.when(() -> PasswordUtils.isV2Format("v2,PBEWithMD5AndTripleDES,c29tZXNhbHQ=,17,ZW5jcnlwdGVk")).thenReturn(true); + passwordUtilsMock.when(() -> PasswordUtils.decryptPasswordV2(Mockito.eq("v2,PBEWithMD5AndTripleDES,c29tZXNhbHQ=,17,ZW5jcnlwdGVk"), Mockito.any(char[].class))).thenReturn(TEST_PASSWORD); + + Mockito.when(rangerDaoManagerBase.getXXService()).thenReturn(xxServiceDao); + Mockito.when(xxServiceDao.findByName(TEST_REPO_NAME)).thenReturn(xxService); + Mockito.when(rangerDaoManagerBase.getXXServiceConfigMap()).thenReturn(xxServiceConfigMapDao); + Mockito.when(xxServiceConfigMapDao.findByServiceAndConfigKey(1L, "password")).thenReturn(xxServiceConfigMap); + + Method getKMSPasswordMethod = KmsKeyMgr.class.getDeclaredMethod("getKMSPassword", String.class); + getKMSPasswordMethod.setAccessible(true); + + String result = (String) getKMSPasswordMethod.invoke(kmsKeyMgr, TEST_REPO_NAME); + + Assertions.assertEquals(TEST_PASSWORD, result); + passwordUtilsMock.verify(() -> PasswordUtils.decryptPasswordV2(Mockito.eq("v2,PBEWithMD5AndTripleDES,c29tZXNhbHQ=,17,ZW5jcnlwdGVk"), Mockito.any(char[].class))); + passwordUtilsMock.verify(() -> PasswordUtils.decryptPassword(Mockito.anyString()), Mockito.never()); + } + } + + @Test + public void testGetKMSPassword_V2Format_KeyMismatchFailsClosed() throws Exception { + // Unlike RangerServiceService's read path, getKMSPassword() has no try/catch around the + // v2 decrypt call - a key mismatch must propagate, not silently fall back to a bad value. + XXService xxService = new XXService(); + xxService.setId(1L); + XXServiceConfigMap xxServiceConfigMap = new XXServiceConfigMap(); + xxServiceConfigMap.setConfigvalue("v2,PBEWithMD5AndTripleDES,c29tZXNhbHQ=,17,ZW5jcnlwdGVk"); + + try (MockedStatic passwordUtilsMock = Mockito.mockStatic(PasswordUtils.class)) { + passwordUtilsMock.when(() -> PasswordUtils.isV2Format("v2,PBEWithMD5AndTripleDES,c29tZXNhbHQ=,17,ZW5jcnlwdGVk")).thenReturn(true); + passwordUtilsMock.when(() -> PasswordUtils.decryptPasswordV2(Mockito.anyString(), Mockito.any(char[].class))) + .thenThrow(new IOException("decryptPasswordV2() failed — wrong key or corrupted value (decrypted output was not in the expected format)")); + + Mockito.when(rangerDaoManagerBase.getXXService()).thenReturn(xxServiceDao); + Mockito.when(xxServiceDao.findByName(TEST_REPO_NAME)).thenReturn(xxService); + Mockito.when(rangerDaoManagerBase.getXXServiceConfigMap()).thenReturn(xxServiceConfigMapDao); + Mockito.when(xxServiceConfigMapDao.findByServiceAndConfigKey(1L, "password")).thenReturn(xxServiceConfigMap); + + Method getKMSPasswordMethod = KmsKeyMgr.class.getDeclaredMethod("getKMSPassword", String.class); + getKMSPasswordMethod.setAccessible(true); + + Assertions.assertThrows(InvocationTargetException.class, () -> getKMSPasswordMethod.invoke(kmsKeyMgr, TEST_REPO_NAME)); + } + } + @Test public void testGetKMSUserName() throws Exception { RangerService rangerService = createMockRangerService(); diff --git a/security-admin/src/test/java/org/apache/ranger/patch/TestPatchServicePasswordV2Migration_J10070.java b/security-admin/src/test/java/org/apache/ranger/patch/TestPatchServicePasswordV2Migration_J10070.java new file mode 100644 index 0000000000..a1e79c1c81 --- /dev/null +++ b/security-admin/src/test/java/org/apache/ranger/patch/TestPatchServicePasswordV2Migration_J10070.java @@ -0,0 +1,209 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.ranger.patch; + +import org.apache.ranger.biz.ServiceDBStore; +import org.apache.ranger.db.RangerDaoManager; +import org.apache.ranger.db.XXServiceConfigDefDao; +import org.apache.ranger.db.XXServiceConfigMapDao; +import org.apache.ranger.db.XXServiceDao; +import org.apache.ranger.entity.XXService; +import org.apache.ranger.entity.XXServiceConfigMap; +import org.apache.ranger.plugin.util.PasswordUtils; +import org.apache.ranger.plugin.util.RangerSupportedCryptoAlgo; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.util.Collections; +import java.util.List; + +/** + * @description Unit tests for PatchServicePasswordV2Migration_J10067, covering the gaps flagged in + * review: the up-front config validation (see {@link PatchServicePasswordV2Migration_J10070#validateMigrationConfig()}) + * and the "already-v2 row is a no-op" idempotency guarantee. + */ +@ExtendWith(MockitoExtension.class) +public class TestPatchServicePasswordV2Migration_J10070 { + @Test + public void testMigrateServicePasswordsToV2_DefaultKeyConfig_FailsFastBeforeTouchingAnyRow() { + // No test in this suite (or anywhere else in this module) overrides + // ranger.password.encryption.key, so ServiceDBStore.ENCRYPT_KEY is - here, exactly like in + // a freshly-installed, never-configured real deployment - still PasswordUtils.DEFAULT_ENCRYPT_KEY. + // validateMigrationConfig() must refuse to proceed in that state, and it must do so BEFORE + // even reading x_service_config_map - not burn every row as an individual per-row failure. + Assertions.assertEquals(PasswordUtils.DEFAULT_ENCRYPT_KEY, ServiceDBStore.ENCRYPT_KEY, + "test precondition: this suite never configures a real encryption key"); + + PatchServicePasswordV2Migration_J10070 patch = new PatchServicePasswordV2Migration_J10070(); + RangerDaoManager daoMgr = Mockito.mock(RangerDaoManager.class); + + patch.daoMgr = daoMgr; + + Assertions.assertThrows(IllegalStateException.class, patch::migrateServicePasswordsToV2, + "migration must refuse to run against an unset/default ranger.password.encryption.key"); + + Mockito.verifyNoInteractions(daoMgr); + } + + @Test + public void testMigrateServicePasswordsToV2_AlreadyV2Row_IsNoOp() throws Exception { + // Bypass only the key-configuration guard (already covered directly by + // PasswordUtilsTest#testValidateEncryptionKeyConfiguredRejectsDefaultKey and by the + // fail-fast test above) so the real per-row dispatch logic below it can be exercised + // end-to-end against a v2-format row, the same way a re-run of this patch after a partial + // prior run would encounter one. + try (MockedStatic pwdUtilsMock = Mockito.mockStatic(PasswordUtils.class, Mockito.CALLS_REAL_METHODS)) { + pwdUtilsMock.when(() -> PasswordUtils.validateEncryptionKeyConfigured(Mockito.any())).thenAnswer(invocation -> null); + + PatchServicePasswordV2Migration_J10070 patch = new PatchServicePasswordV2Migration_J10070(); + + RangerDaoManager daoMgr = Mockito.mock(RangerDaoManager.class); + XXServiceConfigMapDao xServiceConfigMapDao = Mockito.mock(XXServiceConfigMapDao.class); + XXServiceDao xServiceDao = Mockito.mock(XXServiceDao.class); + XXServiceConfigDefDao xServiceConfigDefDao = Mockito.mock(XXServiceConfigDefDao.class); + + patch.daoMgr = daoMgr; + + String v2StoredValue = PasswordUtils.encryptPasswordV2("existingServicePassword", RangerSupportedCryptoAlgo.PBEWITHHMACSHA512ANDAES_128, + "some-operator-key".toCharArray(), "f77aLYLo".getBytes(), 1000); + + XXServiceConfigMap configMap = new XXServiceConfigMap(); + configMap.setId(1L); + configMap.setServiceId(100L); + configMap.setConfigkey("password"); + configMap.setConfigvalue(v2StoredValue); + + XXService xService = new XXService(); + xService.setId(100L); + xService.setName("svc1"); + xService.setType(10L); + + Mockito.when(daoMgr.getXXServiceConfigMap()).thenReturn(xServiceConfigMapDao); + Mockito.when(xServiceConfigMapDao.getAll()).thenReturn(Collections.singletonList(configMap)); + + Mockito.when(daoMgr.getXXService()).thenReturn(xServiceDao); + Mockito.when(xServiceDao.getById(100L)).thenReturn(xService); + + Mockito.when(daoMgr.getXXServiceConfigDef()).thenReturn(xServiceConfigDefDao); + Mockito.when(xServiceConfigDefDao.findConfigNamesByServiceDefIdAndType(10L, ServiceDBStore.CONFIG_TYPE_PASSWORD)) + .thenReturn(Collections.emptyList()); + + patch.migrateServicePasswordsToV2(); + + Mockito.verify(xServiceConfigMapDao, Mockito.never()).update(Mockito.any(XXServiceConfigMap.class)); + Assertions.assertEquals(v2StoredValue, configMap.getConfigvalue(), "an already-v2 row must be left byte-for-byte unchanged"); + } + } + + @Test + public void testMigrateServicePasswordsToV2_PlaintextWithCommaIsNotMisclassifiedAsLegacy() throws Exception { + // Guards the fix for the old ".contains(\",\")" heuristic: a plaintext password that + // merely contains a comma must be left alone (counted as "not encrypted"), never run + // through decrypt/re-encrypt as if it were a genuine legacy-format value. + try (MockedStatic pwdUtilsMock = Mockito.mockStatic(PasswordUtils.class, Mockito.CALLS_REAL_METHODS)) { + pwdUtilsMock.when(() -> PasswordUtils.validateEncryptionKeyConfigured(Mockito.any())).thenAnswer(invocation -> null); + + PatchServicePasswordV2Migration_J10070 patch = new PatchServicePasswordV2Migration_J10070(); + + RangerDaoManager daoMgr = Mockito.mock(RangerDaoManager.class); + XXServiceConfigMapDao xServiceConfigMapDao = Mockito.mock(XXServiceConfigMapDao.class); + XXServiceDao xServiceDao = Mockito.mock(XXServiceDao.class); + XXServiceConfigDefDao xServiceConfigDefDao = Mockito.mock(XXServiceConfigDefDao.class); + + patch.daoMgr = daoMgr; + + XXServiceConfigMap configMap = new XXServiceConfigMap(); + configMap.setId(2L); + configMap.setServiceId(101L); + configMap.setConfigkey("password"); + configMap.setConfigvalue("plaintext,password,with,commas"); // first field is not a real algo name + + List allConfigMaps = Collections.singletonList(configMap); + + XXService xService = new XXService(); + xService.setId(101L); + xService.setName("svc2"); + xService.setType(10L); + + Mockito.when(daoMgr.getXXServiceConfigMap()).thenReturn(xServiceConfigMapDao); + Mockito.when(xServiceConfigMapDao.getAll()).thenReturn(allConfigMaps); + + Mockito.when(daoMgr.getXXService()).thenReturn(xServiceDao); + Mockito.when(xServiceDao.getById(101L)).thenReturn(xService); + + Mockito.when(daoMgr.getXXServiceConfigDef()).thenReturn(xServiceConfigDefDao); + Mockito.when(xServiceConfigDefDao.findConfigNamesByServiceDefIdAndType(10L, ServiceDBStore.CONFIG_TYPE_PASSWORD)) + .thenReturn(Collections.singletonList("password")); + + patch.migrateServicePasswordsToV2(); + + // The row IS looked up now (that's the point of the reordering fix) but must never be + // decrypted/re-encrypted/written - it doesn't look like either recognized format. + Mockito.verify(xServiceConfigMapDao, Mockito.never()).update(Mockito.any(XXServiceConfigMap.class)); + Assertions.assertEquals("plaintext,password,with,commas", configMap.getConfigvalue()); + Assertions.assertEquals(1, patch.notEncryptedCount, "the plaintext-with-comma row must be counted as not-encrypted, not silently dropped"); + } + } + + @Test + public void testMigrateServicePasswordsToV2_GenuinelyPlaintextPasswordField_WarnedNotSkippedSilently() throws Exception { + try (MockedStatic pwdUtilsMock = Mockito.mockStatic(PasswordUtils.class, Mockito.CALLS_REAL_METHODS)) { + pwdUtilsMock.when(() -> PasswordUtils.validateEncryptionKeyConfigured(Mockito.any())).thenAnswer(invocation -> null); + + PatchServicePasswordV2Migration_J10070 patch = new PatchServicePasswordV2Migration_J10070(); + + RangerDaoManager daoMgr = Mockito.mock(RangerDaoManager.class); + XXServiceConfigMapDao xServiceConfigMapDao = Mockito.mock(XXServiceConfigMapDao.class); + XXServiceDao xServiceDao = Mockito.mock(XXServiceDao.class); + XXServiceConfigDefDao xServiceConfigDefDao = Mockito.mock(XXServiceConfigDefDao.class); + + patch.daoMgr = daoMgr; + + XXServiceConfigMap configMap = new XXServiceConfigMap(); + configMap.setId(3L); + configMap.setServiceId(102L); + configMap.setConfigkey("password"); + configMap.setConfigvalue("aPlaintextPasswordWithNoCommaAtAll"); + + XXService xService = new XXService(); + xService.setId(102L); + xService.setName("svc3"); + xService.setType(11L); + + Mockito.when(daoMgr.getXXServiceConfigMap()).thenReturn(xServiceConfigMapDao); + Mockito.when(xServiceConfigMapDao.getAll()).thenReturn(Collections.singletonList(configMap)); + + Mockito.when(daoMgr.getXXService()).thenReturn(xServiceDao); + Mockito.when(xServiceDao.getById(102L)).thenReturn(xService); + + Mockito.when(daoMgr.getXXServiceConfigDef()).thenReturn(xServiceConfigDefDao); + Mockito.when(xServiceConfigDefDao.findConfigNamesByServiceDefIdAndType(11L, ServiceDBStore.CONFIG_TYPE_PASSWORD)) + .thenReturn(Collections.singletonList("password")); + + patch.migrateServicePasswordsToV2(); + + Mockito.verify(xServiceConfigMapDao, Mockito.never()).update(Mockito.any(XXServiceConfigMap.class)); + Assertions.assertEquals("aPlaintextPasswordWithNoCommaAtAll", configMap.getConfigvalue()); + Assertions.assertEquals(1, patch.notEncryptedCount); + Assertions.assertEquals(0, patch.migratedCount); + } + } +} diff --git a/security-admin/src/test/java/org/apache/ranger/service/TestRangerServiceService.java b/security-admin/src/test/java/org/apache/ranger/service/TestRangerServiceService.java index 8e38db26bb..e3950471df 100644 --- a/security-admin/src/test/java/org/apache/ranger/service/TestRangerServiceService.java +++ b/security-admin/src/test/java/org/apache/ranger/service/TestRangerServiceService.java @@ -37,6 +37,7 @@ import org.apache.ranger.entity.XXServiceVersionInfo; import org.apache.ranger.plugin.model.RangerService; import org.apache.ranger.plugin.util.PasswordUtils; +import org.apache.ranger.plugin.util.RangerSupportedCryptoAlgo; import org.apache.ranger.security.context.RangerContextHolder; import org.apache.ranger.security.context.RangerSecurityContext; import org.junit.jupiter.api.Assertions; @@ -368,8 +369,82 @@ public void test4bGetConfigsWithDecryptedPasswordResolvesMixedCaseKeys() throws Map result = serviceService.getConfigsWithDecryptedPassword(service); - Assertions.assertEquals(storedValue, result.get(configKey), - "mixed-case password-typed key must be resolved via case-insensitive lookup, not silently skipped"); + Assertions.assertEquals(plainValue, result.get(configKey), + "mixed-case password-typed key must be resolved via case-insensitive lookup, not silently skipped, and must return the actual decrypted value"); + } + + @Test + public void test4cGetConfigsWithDecryptedPasswordReturnsPlaintextForV2Format() throws Exception { + final String configKey = "password"; + final String plainValue = "s3cr3t-v2-password"; + + String storedValue = PasswordUtils.encryptPasswordV2(plainValue, RangerSupportedCryptoAlgo.getValueOf(ServiceDBStore.CRYPT_ALGO), + ServiceDBStore.ENCRYPT_KEY.toCharArray(), ServiceDBStore.SALT.getBytes(), ServiceDBStore.ITERATION_COUNT); + + Assertions.assertTrue(PasswordUtils.isV2Format(storedValue), "test fixture must actually be in v2 format"); + + XXServiceConfigMapDao xServiceConfigMapDao = Mockito.mock(XXServiceConfigMapDao.class); + XXServiceConfigDefDao xServiceConfigDefDao = Mockito.mock(XXServiceConfigDefDao.class); + + Map configs = new HashMap<>(); + configs.put(configKey, ServiceDBStore.HIDDEN_PASSWORD_STR); // UI didn't change this field -> sentinel value + + RangerService service = new RangerService(); + service.setId(userId); + service.setType("hdfs"); + service.setConfigs(configs); + + XXServiceConfigMap storedConfigMap = new XXServiceConfigMap(); + storedConfigMap.setConfigkey(configKey); + storedConfigMap.setConfigvalue(storedValue); + + Mockito.when(daoManager.getXXServiceConfigDef()).thenReturn(xServiceConfigDefDao); + Mockito.when(xServiceConfigDefDao.findConfigNamesByServiceDefNameAndType("hdfs", ServiceDBStore.CONFIG_TYPE_PASSWORD)).thenReturn(Collections.singletonList(configKey)); + Mockito.when(daoManager.getXXServiceConfigMap()).thenReturn(xServiceConfigMapDao); + Mockito.when(xServiceConfigMapDao.findByServiceAndConfigKey(userId, configKey)).thenReturn(storedConfigMap); + + Mockito.when(stringUtil.isEmpty(Mockito.anyString())).thenReturn(false); + + Map result = serviceService.getConfigsWithDecryptedPassword(service); + + Assertions.assertEquals(plainValue, result.get(configKey), + "v2-format password must be resolved to its real decrypted plaintext, not left as the stored v2 string"); + } + + @Test + public void test4dGetConfigsWithDecryptedPasswordFailsClosedOnV2DecryptFailure() throws Exception { + final String configKey = "password"; + + String storedValue = PasswordUtils.encryptPasswordV2("whatever", RangerSupportedCryptoAlgo.getValueOf(ServiceDBStore.CRYPT_ALGO), + "a-completely-different-key".toCharArray(), ServiceDBStore.SALT.getBytes(), ServiceDBStore.ITERATION_COUNT); + + XXServiceConfigMapDao xServiceConfigMapDao = Mockito.mock(XXServiceConfigMapDao.class); + XXServiceConfigDefDao xServiceConfigDefDao = Mockito.mock(XXServiceConfigDefDao.class); + + Map configs = new HashMap<>(); + configs.put(configKey, ServiceDBStore.HIDDEN_PASSWORD_STR); + + RangerService service = new RangerService(); + service.setId(userId); + service.setType("hdfs"); + service.setConfigs(configs); + + XXServiceConfigMap storedConfigMap = new XXServiceConfigMap(); + storedConfigMap.setConfigkey(configKey); + storedConfigMap.setConfigvalue(storedValue); + + Mockito.when(daoManager.getXXServiceConfigDef()).thenReturn(xServiceConfigDefDao); + Mockito.when(xServiceConfigDefDao.findConfigNamesByServiceDefNameAndType("hdfs", ServiceDBStore.CONFIG_TYPE_PASSWORD)).thenReturn(Collections.singletonList(configKey)); + Mockito.when(daoManager.getXXServiceConfigMap()).thenReturn(xServiceConfigMapDao); + Mockito.when(xServiceConfigMapDao.findByServiceAndConfigKey(userId, configKey)).thenReturn(storedConfigMap); + + Mockito.when(stringUtil.isEmpty(Mockito.anyString())).thenReturn(false); + + Exception ex = Assertions.assertThrows(Exception.class, () -> serviceService.getConfigsWithDecryptedPassword(service), + "a v2-format value that fails to decrypt under the configured key must fail closed, not silently return the masked sentinel"); + + Assertions.assertTrue(ex.getMessage().contains(configKey), "the thrown error should name the config key that failed to decrypt"); + Assertions.assertTrue(ex.getMessage().contains("ranger.password.encryption.key"), "the thrown error should point the operator at the property to check"); } private XXServiceConfigMap configMap(String key, String value) {