From 4ee54243f3e49a3f78f201989d2f9665363422f2 Mon Sep 17 00:00:00 2001 From: James Brunton Date: Wed, 15 Jul 2026 13:28:29 +0100 Subject: [PATCH] Remove encryption from stored policies JSON (#7035) # Description of Changes The policies stored in the DB are currently encrypted at rest, because one version in the past included S3 keys. These are now stored properly in the credentials system and I've manually removed the only policy that used S3 (it was very recently released). Since there's no S3 (or other) credentials in the policies stored JSON now, we might as well just decrypt them. This PR pairs with #7034 to fix the issues - #7034 makes it resilient to crashing when attempting to load encrypted JSON that's been encrypted with the wrong key, and this makes it so if it does load any encrypted policies, they'll be re-saved decrypted, so we should have a vanishingly small number of encrypted policies over time. --- .../crypto/LegacyDecryptStringConverter.java | 35 +++++++++++++ .../LenientEncryptedStringConverter.java | 31 ----------- .../s3/EmbeddedS3CredentialMigration.java | 4 +- .../policy/source/SourceEntity.java | 8 +-- .../policy/store/PolicyEntity.java | 8 +-- .../LegacyDecryptStringConverterTest.java | 51 +++++++++++++++++++ .../LenientEncryptedStringConverterTest.java | 45 ---------------- 7 files changed, 96 insertions(+), 86 deletions(-) create mode 100644 app/proprietary/src/main/java/stirling/software/proprietary/integration/crypto/LegacyDecryptStringConverter.java delete mode 100644 app/proprietary/src/main/java/stirling/software/proprietary/integration/crypto/LenientEncryptedStringConverter.java create mode 100644 app/proprietary/src/test/java/stirling/software/proprietary/integration/crypto/LegacyDecryptStringConverterTest.java delete mode 100644 app/proprietary/src/test/java/stirling/software/proprietary/integration/crypto/LenientEncryptedStringConverterTest.java diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/integration/crypto/LegacyDecryptStringConverter.java b/app/proprietary/src/main/java/stirling/software/proprietary/integration/crypto/LegacyDecryptStringConverter.java new file mode 100644 index 0000000000..118608b925 --- /dev/null +++ b/app/proprietary/src/main/java/stirling/software/proprietary/integration/crypto/LegacyDecryptStringConverter.java @@ -0,0 +1,35 @@ +package stirling.software.proprietary.integration.crypto; + +import jakarta.persistence.AttributeConverter; +import jakarta.persistence.Converter; + +/** + * For columns that were once whole-blob encrypted but no longer hold secrets. + * + *

Writes plaintext, so the value is readable by any instance regardless of the per-installation + * encryption key. Any value that isn't our ciphertext (already-plaintext JSON, or ciphertext from a + * key we don't hold) is returned as-is; the latter is the caller's to reject. + */ +@Converter +public class LegacyDecryptStringConverter implements AttributeConverter { + + @Override + public String convertToDatabaseColumn(String attribute) { + return attribute; + } + + @Override + public String convertToEntityAttribute(String dbData) { + // Plaintext JSON can never be our Base64 ciphertext ('{' is not in the Base64 alphabet), so + // skip the decrypt attempt for it. + if (dbData == null || dbData.stripLeading().startsWith("{")) { + return dbData; + } + try { + return CredentialEncryption.decrypt(dbData); + } catch (IllegalArgumentException | IllegalStateException e) { + // Legacy ciphertext we can't read (key we don't hold) - the caller's to reject. + return dbData; + } + } +} diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/integration/crypto/LenientEncryptedStringConverter.java b/app/proprietary/src/main/java/stirling/software/proprietary/integration/crypto/LenientEncryptedStringConverter.java deleted file mode 100644 index 7bb6190220..0000000000 --- a/app/proprietary/src/main/java/stirling/software/proprietary/integration/crypto/LenientEncryptedStringConverter.java +++ /dev/null @@ -1,31 +0,0 @@ -package stirling.software.proprietary.integration.crypto; - -import jakarta.persistence.AttributeConverter; -import jakarta.persistence.Converter; - -/** - * {@link EncryptedStringConverter} for columns that held plaintext before encryption shipped: - * writes are always encrypted, but a stored value that is not valid ciphertext is returned as-is, - * so pre-encryption rows keep loading and become encrypted on their next save. The discrimination - * is exact for JSON payloads, which can never be mistaken for ciphertext ('{' is not in the Base64 - * alphabet). The trade-off is that a genuinely corrupted ciphertext surfaces as garbage to the - * caller's parser instead of failing here. - */ -@Converter -public class LenientEncryptedStringConverter implements AttributeConverter { - - @Override - public String convertToDatabaseColumn(String attribute) { - return CredentialEncryption.encrypt(attribute); - } - - @Override - public String convertToEntityAttribute(String dbData) { - try { - return CredentialEncryption.decrypt(dbData); - } catch (IllegalArgumentException | IllegalStateException e) { - // Not ciphertext: legacy plaintext from before encryption shipped. - return dbData; - } - } -} diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/policy/s3/EmbeddedS3CredentialMigration.java b/app/proprietary/src/main/java/stirling/software/proprietary/policy/s3/EmbeddedS3CredentialMigration.java index 5aa82a8e66..398b2bd709 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/policy/s3/EmbeddedS3CredentialMigration.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/policy/s3/EmbeddedS3CredentialMigration.java @@ -32,8 +32,8 @@ import tools.jackson.databind.ObjectMapper; * One-time, idempotent extraction of legacy embedded S3 credentials into stored connections: * sources and policy outputs written before connections shipped carry bucket/credentials in their * own options; this rewrites each to reference a (deduplicated) S3 {@link IntegrationConfig} and - * keeps only per-use options (prefix, mode). MUST be programmatic - the option JSON is encrypted at - * the application layer, so no SQL migration can read it. + * keeps only per-use options (prefix, mode). MUST be programmatic - it parses and rewrites the + * option JSON (and decrypts any legacy ciphertext row on read), which no SQL migration can do. * *

Idempotent by construction: rewritten rows no longer embed credentials, so re-runs find * nothing to do. Connections are deduplicated against both this run's extractions and existing S3 diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/policy/source/SourceEntity.java b/app/proprietary/src/main/java/stirling/software/proprietary/policy/source/SourceEntity.java index c94e4cb079..728612742e 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/policy/source/SourceEntity.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/policy/source/SourceEntity.java @@ -12,7 +12,7 @@ import lombok.Getter; import lombok.NoArgsConstructor; import lombok.Setter; -import stirling.software.proprietary.integration.crypto.LenientEncryptedStringConverter; +import stirling.software.proprietary.integration.crypto.LegacyDecryptStringConverter; /** * JPA row for a {@link Source}. The whole source lives as JSON in {@code sourceJson} (authoritative @@ -48,9 +48,9 @@ public class SourceEntity implements Serializable { @Column(name = "enabled") private boolean enabled; - // Encrypted at rest: source options carry user-supplied credentials (e.g. an S3 secret - // access key). Lenient so rows written before encryption shipped still load. - @Convert(converter = LenientEncryptedStringConverter.class) + // Plaintext at rest: the S3 credentials that used to live here now sit in a referenced + // IntegrationConfig connection (still encrypted). Decrypts legacy ciphertext on read. + @Convert(converter = LegacyDecryptStringConverter.class) @Column(name = "source_json", columnDefinition = "text") private String sourceJson; } diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/policy/store/PolicyEntity.java b/app/proprietary/src/main/java/stirling/software/proprietary/policy/store/PolicyEntity.java index e393e47833..c871267bc4 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/policy/store/PolicyEntity.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/policy/store/PolicyEntity.java @@ -12,7 +12,7 @@ import lombok.Getter; import lombok.NoArgsConstructor; import lombok.Setter; -import stirling.software.proprietary.integration.crypto.LenientEncryptedStringConverter; +import stirling.software.proprietary.integration.crypto.LegacyDecryptStringConverter; /** * JPA row for a {@link stirling.software.proprietary.policy.model.Policy}. The whole policy lives @@ -58,9 +58,9 @@ public class PolicyEntity implements Serializable { @Column(name = "sort_order") private Integer sortOrder; - // Encrypted at rest: output options carry user-supplied credentials (e.g. an S3 secret - // access key). Lenient so rows written before encryption shipped still load. - @Convert(converter = LenientEncryptedStringConverter.class) + // Plaintext at rest: the S3 credentials that used to live here now sit in a referenced + // IntegrationConfig connection (still encrypted). Decrypts legacy ciphertext on read. + @Convert(converter = LegacyDecryptStringConverter.class) @Column(name = "policy_json", columnDefinition = "text") private String policyJson; } diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/integration/crypto/LegacyDecryptStringConverterTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/integration/crypto/LegacyDecryptStringConverterTest.java new file mode 100644 index 0000000000..11b913e9d7 --- /dev/null +++ b/app/proprietary/src/test/java/stirling/software/proprietary/integration/crypto/LegacyDecryptStringConverterTest.java @@ -0,0 +1,51 @@ +package stirling.software.proprietary.integration.crypto; + +import static org.assertj.core.api.Assertions.assertThat; + +import javax.crypto.KeyGenerator; +import javax.crypto.SecretKey; + +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +class LegacyDecryptStringConverterTest { + + private final LegacyDecryptStringConverter converter = new LegacyDecryptStringConverter(); + + @BeforeAll + static void initKey() throws Exception { + KeyGenerator generator = KeyGenerator.getInstance("AES"); + generator.init(256); + SecretKey key = generator.generateKey(); + CredentialEncryption.initialiseForTesting(key); + } + + @Test + void writesPlaintext() { + String json = "{\"bucket\":\"inbox\",\"mode\":\"consume\"}"; + + assertThat(converter.convertToDatabaseColumn(json)).isEqualTo(json); + } + + @Test + void decryptsLegacyCiphertextOnRead() { + String json = "{\"bucket\":\"inbox\"}"; + String legacyCiphertext = CredentialEncryption.encrypt(json); + + assertThat(legacyCiphertext).isNotEqualTo(json); + assertThat(converter.convertToEntityAttribute(legacyCiphertext)).isEqualTo(json); + } + + @Test + void passesPlaintextThroughOnRead() { + String json = "{\"bucket\":\"inbox\",\"mode\":\"consume\"}"; + + assertThat(converter.convertToEntityAttribute(json)).isEqualTo(json); + } + + @Test + void nullsPassThrough() { + assertThat(converter.convertToDatabaseColumn(null)).isNull(); + assertThat(converter.convertToEntityAttribute(null)).isNull(); + } +} diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/integration/crypto/LenientEncryptedStringConverterTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/integration/crypto/LenientEncryptedStringConverterTest.java deleted file mode 100644 index b61ded9708..0000000000 --- a/app/proprietary/src/test/java/stirling/software/proprietary/integration/crypto/LenientEncryptedStringConverterTest.java +++ /dev/null @@ -1,45 +0,0 @@ -package stirling.software.proprietary.integration.crypto; - -import static org.assertj.core.api.Assertions.assertThat; - -import javax.crypto.KeyGenerator; -import javax.crypto.SecretKey; - -import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Test; - -class LenientEncryptedStringConverterTest { - - private final LenientEncryptedStringConverter converter = new LenientEncryptedStringConverter(); - - @BeforeAll - static void initKey() throws Exception { - KeyGenerator generator = KeyGenerator.getInstance("AES"); - generator.init(256); - SecretKey key = generator.generateKey(); - CredentialEncryption.initialiseForTesting(key); - } - - @Test - void roundTripsThroughCiphertext() { - String json = "{\"bucket\":\"inbox\",\"secretAccessKey\":\"shh\"}"; - - String stored = converter.convertToDatabaseColumn(json); - - assertThat(stored).isNotEqualTo(json).doesNotContain("shh"); - assertThat(converter.convertToEntityAttribute(stored)).isEqualTo(json); - } - - @Test - void legacyPlaintextRowsPassThroughOnRead() { - String legacy = "{\"bucket\":\"inbox\",\"mode\":\"consume\"}"; - - assertThat(converter.convertToEntityAttribute(legacy)).isEqualTo(legacy); - } - - @Test - void nullsPassThrough() { - assertThat(converter.convertToDatabaseColumn(null)).isNull(); - assertThat(converter.convertToEntityAttribute(null)).isNull(); - } -}