Skip to content

Commit 6f2b35b

Browse files
committed
fix(storage): bound the key-registry queries in master-key rotation
Addresses the Aikido code-quality finding on #7155: rotateMasterKey() and verifyMasterKey() loaded every key row and filtered in memory. Both now push the predicate into the query (findByMasterKeyVersionLessThan / countByMasterKeyVersionLessThan), so the rotation check costs a counted query rather than a full table read. The SaaS boot-probe fix that originally shared this commit landed on its own in #7265, so only the rebase leftovers remain here: one assertion proving a storage-disabled node resolves no master key at boot.
1 parent c9c3d16 commit 6f2b35b

4 files changed

Lines changed: 32 additions & 8 deletions

File tree

app/proprietary/src/main/java/stirling/software/proprietary/storage/crypto/FileEncryptionKeyService.java

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -177,10 +177,8 @@ public void invalidate(UUID keyId) {
177177
*/
178178
public int rotateMasterKey() throws StorageEncryptionException {
179179
int rewrapped = 0;
180-
for (FileEncryptionKey row : repository.findAll()) {
181-
if (row.getMasterKeyVersion() >= masterKey.currentVersion()) {
182-
continue;
183-
}
180+
for (FileEncryptionKey row :
181+
repository.findByMasterKeyVersionLessThan(masterKey.currentVersion())) {
184182
byte[] kek = unwrapRow(row);
185183
row.setWrappedKey(
186184
Base64.getEncoder()
@@ -199,10 +197,7 @@ public int rotateMasterKey() throws StorageEncryptionException {
199197
*/
200198
public void verifyMasterKey() {
201199
if (masterKey.hasPreviousKey()) {
202-
long pending =
203-
repository.findAll().stream()
204-
.filter(r -> r.getMasterKeyVersion() < masterKey.currentVersion())
205-
.count();
200+
long pending = repository.countByMasterKeyVersionLessThan(masterKey.currentVersion());
206201
if (pending > 0) {
207202
log.warn(
208203
"{} encryption key row(s) are still wrapped by the previous master key."

app/proprietary/src/main/java/stirling/software/proprietary/storage/repository/FileEncryptionKeyRepository.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package stirling.software.proprietary.storage.repository;
22

3+
import java.util.List;
34
import java.util.Optional;
45
import java.util.UUID;
56

@@ -18,4 +19,12 @@ Optional<FileEncryptionKey> findFirstByScopeTypeAndScopeIdOrderByKeyVersionDesc(
1819
FileEncryptionKey.ScopeType scopeType, long scopeId);
1920

2021
Optional<FileEncryptionKey> findFirstByStatus(FileEncryptionKey.Status status);
22+
23+
/**
24+
* Rows still wrapped by an older master key. Counting and fetching are separate so the startup
25+
* check can ask the database for a number instead of materialising every row.
26+
*/
27+
long countByMasterKeyVersionLessThan(int masterKeyVersion);
28+
29+
List<FileEncryptionKey> findByMasterKeyVersionLessThan(int masterKeyVersion);
2130
}

app/proprietary/src/test/java/stirling/software/proprietary/storage/config/StorageProviderConfigTest.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,8 @@ void storageDisabled_neverQueriesTheKeyRegistry() {
123123
StorageEncryptionState state = newState(cfg);
124124

125125
assertThat(state.isWriteEnabled()).isFalse();
126+
// No probe means no eager init, so no master key is resolved on a node that never stores.
127+
assertThat(state.isMaterialised()).isFalse();
126128
verify(keyRepo.mock, never()).count();
127129
}
128130

app/proprietary/src/test/java/stirling/software/proprietary/storage/crypto/InMemoryKeyRepo.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,24 @@ public InMemoryKeyRepo() {
6363
.findFirst());
6464
when(mock.count()).thenAnswer(inv -> (long) rows.size());
6565
when(mock.findAll()).thenAnswer(inv -> java.util.List.copyOf(rows.values()));
66+
when(mock.countByMasterKeyVersionLessThan(org.mockito.ArgumentMatchers.anyInt()))
67+
.thenAnswer(
68+
inv ->
69+
rows.values().stream()
70+
.filter(
71+
r ->
72+
r.getMasterKeyVersion()
73+
< inv.<Integer>getArgument(0))
74+
.count());
75+
when(mock.findByMasterKeyVersionLessThan(org.mockito.ArgumentMatchers.anyInt()))
76+
.thenAnswer(
77+
inv ->
78+
rows.values().stream()
79+
.filter(
80+
r ->
81+
r.getMasterKeyVersion()
82+
< inv.<Integer>getArgument(0))
83+
.toList());
6684
when(mock.findAll(any(org.springframework.data.domain.Sort.class)))
6785
.thenAnswer(inv -> java.util.List.copyOf(rows.values()));
6886
}

0 commit comments

Comments
 (0)