Skip to content

Commit fe0ad29

Browse files
ThoSapThoSap
committed
keep the cause of a failed password fingerprint key load in the Role status
The catch block around the HMAC computation also caught every error from the load of the key Secret, such as a 403 from the Kubernetes API, and reported it as "HmacSHA256 not available". The key is now loaded before the try block, and the catch covers only the two checked exceptions of the HMAC API. The new test covers the creation of the key Secret with a 32 byte key, the reuse of an existing key by a second operator process, a new key after the Secret is lost, the error for a Secret without the `key` entry, and the exact HMAC construction that existing `Role` statuses depend on.
1 parent 3092173 commit fe0ad29

2 files changed

Lines changed: 205 additions & 2 deletions

File tree

‎operator/src/main/java/it/aboutbits/postgresql/core/PasswordFingerprintService.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
import javax.crypto.spec.SecretKeySpec;
1616
import java.net.HttpURLConnection;
1717
import java.nio.charset.StandardCharsets;
18+
import java.security.InvalidKeyException;
19+
import java.security.NoSuchAlgorithmException;
1820
import java.security.SecureRandom;
1921
import java.util.Base64;
2022

@@ -58,15 +60,18 @@ public String fingerprint(
5860
var encryption = passwordEncryption.toValue().getBytes(StandardCharsets.UTF_8);
5961
var secret = password.getBytes(StandardCharsets.UTF_8);
6062

63+
// Load the key outside the try block, so that a Kubernetes error keeps its own message
64+
var key = getKey();
65+
6166
try {
6267
var mac = Mac.getInstance(HMAC_SHA_256);
63-
mac.init(new SecretKeySpec(getKey(), HMAC_SHA_256));
68+
mac.init(new SecretKeySpec(key, HMAC_SHA_256));
6469
mac.update(encryption);
6570
mac.update(SEPARATOR);
6671
mac.update(secret);
6772

6873
return Base64.getEncoder().encodeToString(mac.doFinal());
69-
} catch (Exception e) {
74+
} catch (NoSuchAlgorithmException | InvalidKeyException e) {
7075
throw new IllegalStateException("%s not available".formatted(HMAC_SHA_256), e);
7176
}
7277
}

‎operator/src/test/java/it/aboutbits/postgresql/core/PasswordFingerprintServiceTest.java‎

Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
1+
package it.aboutbits.postgresql.core;
2+
3+
import io.fabric8.kubernetes.api.model.SecretBuilder;
4+
import io.fabric8.kubernetes.client.KubernetesClient;
5+
import io.fabric8.kubernetes.client.server.mock.EnableKubernetesMockClient;
6+
import it.aboutbits.postgresql.crd.role.PasswordEncryption;
7+
import org.jspecify.annotations.NullMarked;
8+
import org.junit.jupiter.api.BeforeEach;
9+
import org.junit.jupiter.api.DisplayName;
10+
import org.junit.jupiter.api.Nested;
11+
import org.junit.jupiter.api.Test;
12+
13+
import javax.crypto.Mac;
14+
import javax.crypto.spec.SecretKeySpec;
15+
import java.nio.charset.StandardCharsets;
16+
import java.security.GeneralSecurityException;
17+
import java.util.Base64;
18+
19+
import static org.assertj.core.api.Assertions.assertThat;
20+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
21+
22+
@NullMarked
23+
@EnableKubernetesMockClient(crud = true)
24+
class PasswordFingerprintServiceTest {
25+
private static final String SECRET_NAME = "test-password-fingerprint-key";
26+
27+
@SuppressWarnings("NullAway.Init")
28+
static KubernetesClient client;
29+
30+
@BeforeEach
31+
void clearSecrets() {
32+
client.secrets().inAnyNamespace().delete();
33+
}
34+
35+
/// Each instance has its own key cache, like a fresh operator process.
36+
private static PasswordFingerprintService newService() {
37+
var service = new PasswordFingerprintService(client);
38+
service.secretName = SECRET_NAME;
39+
40+
return service;
41+
}
42+
43+
@Nested
44+
class KeySecret {
45+
@Test
46+
@DisplayName("When the key Secret does not exist, should create it with a random 32 byte key")
47+
void whenSecretMissing_shouldCreateIt() {
48+
// given
49+
var service = newService();
50+
51+
// when
52+
service.fingerprint("password", PasswordEncryption.SCRAM_SHA_256);
53+
54+
// then
55+
var secret = client.secrets()
56+
.inNamespace(client.getNamespace())
57+
.withName(SECRET_NAME)
58+
.get();
59+
60+
assertThat(secret).isNotNull();
61+
assertThat(secret.getType()).isEqualTo("Opaque");
62+
assertThat(secret.getData()).containsOnlyKeys(PasswordFingerprintService.SECRET_DATA_KEY);
63+
64+
var key = Base64.getDecoder().decode(secret.getData().get(PasswordFingerprintService.SECRET_DATA_KEY));
65+
assertThat(key).hasSize(32);
66+
}
67+
68+
@Test
69+
@DisplayName("When the key Secret exists, should reuse its key")
70+
void whenSecretExists_shouldReuseKey() {
71+
// given
72+
var fingerprint = newService().fingerprint("password", PasswordEncryption.SCRAM_SHA_256);
73+
74+
// when: a second operator process starts
75+
var fingerprintOfSecondProcess = newService().fingerprint("password", PasswordEncryption.SCRAM_SHA_256);
76+
77+
// then
78+
assertThat(fingerprintOfSecondProcess).isEqualTo(fingerprint);
79+
}
80+
81+
@Test
82+
@DisplayName("When the key Secret is lost, should create a new key and the fingerprints change")
83+
void whenSecretLost_shouldCreateNewKey() {
84+
// given
85+
var fingerprint = newService().fingerprint("password", PasswordEncryption.SCRAM_SHA_256);
86+
87+
client.secrets()
88+
.inNamespace(client.getNamespace())
89+
.withName(SECRET_NAME)
90+
.delete();
91+
92+
// when
93+
var fingerprintWithNewKey = newService().fingerprint("password", PasswordEncryption.SCRAM_SHA_256);
94+
95+
// then
96+
assertThat(fingerprintWithNewKey).isNotEqualTo(fingerprint);
97+
}
98+
99+
@Test
100+
@DisplayName("When the key Secret has no 'key' entry, should fail with a message that names the Secret")
101+
void whenSecretHasNoKeyEntry_shouldFail() {
102+
// given
103+
client.secrets()
104+
.inNamespace(client.getNamespace())
105+
.resource(new SecretBuilder()
106+
.withNewMetadata()
107+
.withName(SECRET_NAME)
108+
.endMetadata()
109+
.addToData("other", Base64.getEncoder().encodeToString("value".getBytes(StandardCharsets.UTF_8)))
110+
.build()
111+
)
112+
.create();
113+
114+
var service = newService();
115+
116+
// when / then
117+
assertThatThrownBy(() -> service.fingerprint("password", PasswordEncryption.SCRAM_SHA_256))
118+
.isInstanceOf(IllegalStateException.class)
119+
.hasMessageContaining("missing required data 'key'")
120+
.hasMessageContaining(SECRET_NAME);
121+
}
122+
}
123+
124+
@Nested
125+
class Fingerprint {
126+
@Test
127+
@DisplayName("When the same password and encryption are given, should return the same fingerprint")
128+
void whenSameInput_shouldReturnSameFingerprint() {
129+
// given
130+
var service = newService();
131+
132+
// when
133+
var first = service.fingerprint("password", PasswordEncryption.SCRAM_SHA_256);
134+
var second = service.fingerprint("password", PasswordEncryption.SCRAM_SHA_256);
135+
136+
// then
137+
assertThat(second).isEqualTo(first);
138+
}
139+
140+
@Test
141+
@DisplayName("When the password changes, should return a different fingerprint")
142+
void whenPasswordChanges_shouldReturnDifferentFingerprint() {
143+
// given
144+
var service = newService();
145+
146+
// when
147+
var first = service.fingerprint("password", PasswordEncryption.SCRAM_SHA_256);
148+
var second = service.fingerprint("other-password", PasswordEncryption.SCRAM_SHA_256);
149+
150+
// then
151+
assertThat(second).isNotEqualTo(first);
152+
}
153+
154+
@Test
155+
@DisplayName("When the encryption changes, should return a different fingerprint")
156+
void whenEncryptionChanges_shouldReturnDifferentFingerprint() {
157+
// given
158+
var service = newService();
159+
160+
// when
161+
var first = service.fingerprint("password", PasswordEncryption.SCRAM_SHA_256);
162+
var second = service.fingerprint("password", PasswordEncryption.SERVER);
163+
164+
// then
165+
assertThat(second).isNotEqualTo(first);
166+
}
167+
168+
@Test
169+
@DisplayName("Should be the Base64 HMAC-SHA256 of '<encryption> 0x00 <password>' with the key from the Secret")
170+
void shouldMatchDocumentedConstruction() throws GeneralSecurityException {
171+
// Fingerprints in existing Role statuses must stay valid after an upgrade, so the construction is fixed.
172+
173+
// given
174+
var fingerprint = newService().fingerprint("password", PasswordEncryption.SCRAM_SHA_256);
175+
176+
var key = Base64.getDecoder().decode(
177+
client.secrets()
178+
.inNamespace(client.getNamespace())
179+
.withName(SECRET_NAME)
180+
.require()
181+
.getData()
182+
.get(PasswordFingerprintService.SECRET_DATA_KEY)
183+
);
184+
185+
// when
186+
var mac = Mac.getInstance("HmacSHA256");
187+
mac.init(new SecretKeySpec(key, "HmacSHA256"));
188+
mac.update("scram-sha-256".getBytes(StandardCharsets.UTF_8));
189+
mac.update((byte) 0);
190+
mac.update("password".getBytes(StandardCharsets.UTF_8));
191+
192+
var expected = Base64.getEncoder().encodeToString(mac.doFinal());
193+
194+
// then
195+
assertThat(fingerprint).isEqualTo(expected);
196+
}
197+
}
198+
}

0 commit comments

Comments
 (0)