diff --git a/api/src/org/labkey/api/security/Encryption.java b/api/src/org/labkey/api/security/Encryption.java index 484db473869..36d65a069b1 100644 --- a/api/src/org/labkey/api/security/Encryption.java +++ b/api/src/org/labkey/api/security/Encryption.java @@ -50,6 +50,7 @@ import javax.crypto.BadPaddingException; import javax.crypto.Cipher; +import javax.crypto.IllegalBlockSizeException; import javax.crypto.SecretKey; import javax.crypto.SecretKeyFactory; import javax.crypto.spec.GCMParameterSpec; @@ -473,14 +474,32 @@ public String decrypt(byte @NotNull[] cipherText) cipher.init(Cipher.DECRYPT_MODE, _keySpec, _config.createIvSpec(iv)); return new String(cipher.doFinal(encrypted), StringUtilsLabKey.DEFAULT_CHARSET); } - catch (BadPaddingException e) + catch (BadPaddingException | IllegalBlockSizeException e) { - // For now, assume that BadPaddingException means the key has been changed and all other - // exceptions are coding issues. That might change in the future... + // Decryption failure - likely a bad key or old algorithm. - // Track all decryption exceptions that aren't caused by TestCase (below) + // Track all decryption exceptions that aren't caused by TestCase. + // Only the production AES instance (ENCRYPTION_KEY_CHANGED keySource) attempts the fallback; + // migration-temporary instances bypass this block entirely. if (ENCRYPTION_KEY_CHANGED.equals(_keySource)) + { + // During migration, not-yet-migrated values are in the old format. If a fallback algorithm + // is registered (set by prepareMigrationFallback() when migration is known to be incomplete), + // try it before giving up. + Algorithm fallback = _migrationFallback; + if (fallback != null) + { + try + { + return fallback.decrypt(cipherText); + } + catch (RuntimeException ignored) + { + // Both algorithms failed; fall through to increment and rethrow + } + } DECRYPTION_EXCEPTIONS.incrementAndGet(); + } throw new DecryptionException("Could not decrypt this content using the " + _keySource, e); } @@ -493,6 +512,9 @@ public String decrypt(byte @NotNull[] cipherText) private static final String ENCRYPTION_KEY_CHANGED = "currently configured EncryptionKey; has the key changed in " + AppProps.getInstance().getWebappConfigurationFilename() + "?"; private static final AtomicInteger DECRYPTION_EXCEPTIONS = new AtomicInteger(0); + // Set by prepareMigrationFallback() when migration is known to be incomplete; cleared after migration completes. + // Allows HTTP requests to decrypt not-yet-migrated values without failing. + private static volatile Algorithm _migrationFallback = null; public static class DecryptionException extends ConfigurationException { @@ -539,6 +561,39 @@ static void registerHandler(EncryptionMigrationHandler handler) void migrateEncryptedContent(String oldPassPhrase, String keySource, AESConfig oldConfig); } + /** + * Examines the database to determine whether algorithm or key migration is pending, and if so installs a + * fallback algorithm. This allows HTTP requests to transparently decrypt not-yet-migrated values during the + * migration window instead of failing and incrementing DECRYPTION_EXCEPTIONS. + * Must be called after the database and PropertyManager are available (e.g., from CoreModule.afterUpdate()). + * The fallback is cleared automatically once checkMigration() confirms completion. + */ + public static void prepareMigrationFallback() + { + if (!isEncryptionPassPhraseSpecified()) + return; + + String oldPassPhrase = getOldEncryptionPassPhrase(); + + String cipher = PropertyManager.getNormalStore() + .getProperties(ENCRYPTION_CIPHER_CATEGORY) + .get(CIPHER_PROPERTY); + + if (oldPassPhrase != null) + { + // Key-change migration not yet complete; use old key. If cipher is also null, old content used the + // legacy cipher (matching what checkMigration() will use: old key + AESConfig.legacy); otherwise + // old content used the current cipher. + AESConfig fallbackConfig = cipher == null ? AESConfig.legacy : AESConfig.current; + _migrationFallback = new AES(oldPassPhrase, 128, "legacy key migration fallback", fallbackConfig); + } + else if (cipher == null) + { + // Cipher migration not yet complete; fall back to legacy cipher with current key + _migrationFallback = new AES(getEncryptionPassPhrase(), 128, "legacy cipher migration fallback", AESConfig.legacy); + } + } + public static void checkMigration() { String oldPassPhrase = getOldEncryptionPassPhrase(); @@ -547,6 +602,7 @@ public static void checkMigration() if (isEncryptionPassPhraseSpecified() && ModuleLoader.getInstance().shouldInsertData()) { boolean migrationNeeded = false; + boolean migrationSucceeded = false; String keySource = null; if (null != oldPassPhrase) @@ -591,11 +647,16 @@ else if (!cipher.equals(AESConfig.current.getCipherName())) CacheManager.clearAllKnownCaches(); } - // Test to validate conversion and create a validation value if needed + // Test to validate conversion and create a validation value if needed. + // Capture the counter before the test so the save decision is based solely on whether + // this specific test passes, not on concurrent HTTP request decryption failures that may + // have incremented the counter during the (potentially long) migration of auth configurations. + int exceptionsBeforeFinalTest = DECRYPTION_EXCEPTIONS.get(); testEncryptionKey(); + migrationSucceeded = DECRYPTION_EXCEPTIONS.get() == exceptionsBeforeFinalTest; } - if (DECRYPTION_EXCEPTIONS.get() == 0) + if (migrationSucceeded) { if (oldPassPhrase != null) { @@ -610,6 +671,8 @@ else if (!cipher.equals(AESConfig.current.getCipherName())) } } } + + _migrationFallback = null; } @@ -663,6 +726,55 @@ public void testBadKeyException() } } + @Test + public void testMigrationFallback() + { + String text = "test plaintext"; + AES oldAlgorithm = new AES("old pass phrase", 128, "old algorithm"); + byte[] oldEncrypted = oldAlgorithm.encrypt(text); + + // Primary (production) instance: different pass phrase, keySource == ENCRYPTION_KEY_CHANGED + AES primary = new AES("primary pass phrase", 128, ENCRYPTION_KEY_CHANGED); + + // Case 1: no fallback — primary fails and counter increments + int counterBefore = DECRYPTION_EXCEPTIONS.get(); + try + { + primary.decrypt(oldEncrypted); + fail("Expected DecryptionException"); + } + catch (DecryptionException ignored) {} + assertEquals(counterBefore + 1, DECRYPTION_EXCEPTIONS.get()); + + // Case 2: correct fallback — transparent success, counter unchanged + _migrationFallback = oldAlgorithm; + try + { + int counterBeforeFallback = DECRYPTION_EXCEPTIONS.get(); + assertEquals(text, primary.decrypt(oldEncrypted)); + assertEquals("Counter must not increment when fallback succeeds", counterBeforeFallback, DECRYPTION_EXCEPTIONS.get()); + } + finally + { + _migrationFallback = null; + } + + // Case 3: wrong fallback — both algorithms fail, counter increments + _migrationFallback = new AES("wrong pass phrase", 128, "wrong fallback"); + int counterBeforeWrongFallback = DECRYPTION_EXCEPTIONS.get(); + try + { + primary.decrypt(oldEncrypted); + fail("Expected DecryptionException"); + } + catch (DecryptionException ignored) {} + finally + { + _migrationFallback = null; + } + assertEquals(counterBeforeWrongFallback + 1, DECRYPTION_EXCEPTIONS.get()); + } + private void test(Algorithm algorithm) { test(algorithm, algorithm); diff --git a/core/src/org/labkey/core/CoreModule.java b/core/src/org/labkey/core/CoreModule.java index fd2fd568258..23bb96e6f97 100644 --- a/core/src/org/labkey/core/CoreModule.java +++ b/core/src/org/labkey/core/CoreModule.java @@ -869,6 +869,11 @@ public void afterUpdate(ModuleContext moduleContext) ContainerManager.getHomeContainer(); } }); + + // Install a fallback decryption algorithm if AES migration is pending. This prevents concurrent HTTP requests + // from failing to decrypt not-yet-migrated values during the migration window. Called here (afterUpdate) rather + // than in startupAfterSpringConfig so the fallback is active before any long-running upgrade steps run. + Encryption.prepareMigrationFallback(); } private void bootstrap()