Skip to content

Commit af8049c

Browse files
authored
Validate the user supplied CA cert chain (#12409)
Signed-off-by: Kate Stanley <11195226+katheris@users.noreply.github.com>
1 parent b55fcdb commit af8049c

File tree

8 files changed

+581
-34
lines changed

8 files changed

+581
-34
lines changed

cluster-operator/src/test/java/io/strimzi/operator/cluster/model/ClusterCaTest.java

Lines changed: 54 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -112,12 +112,62 @@ public void testRemoveOldCertificate() {
112112

113113
@Test
114114
public void testNotRemoveOldCertificateWithCustomCa() {
115+
// Dummy cert valid until 2126 for testing purposes
116+
String newDummyCert = """
117+
-----BEGIN CERTIFICATE-----
118+
MIIDlzCCAn+gAwIBAgIUemVt2M8YnfYLntb16p2/oSFQVHEwDQYJKoZIhvcNAQEL
119+
BQAwWjELMAkGA1UEBhMCQVUxEzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoM
120+
GEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZDETMBEGA1UEAwwKY2x1c3Rlci1jYTAg
121+
Fw0yNjAyMTYxNTI1MzhaGA8yMTI2MDEyMzE1MjUzOFowWjELMAkGA1UEBhMCQVUx
122+
EzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoMGEludGVybmV0IFdpZGdpdHMg
123+
UHR5IEx0ZDETMBEGA1UEAwwKY2x1c3Rlci1jYTCCASIwDQYJKoZIhvcNAQEBBQAD
124+
ggEPADCCAQoCggEBALulA0Z4vXwcQw9BD1ZehrAJPVg6o9ok7WxM6vbSEHc8ptV1
125+
97dXy0EoIcaJKnwzbwzqBL0KDVa/ZXpiHnWN/o7eq3ZBXxTv/1SGcgwx4vDN99ui
126+
qwyQ+eGBjdiuJ4NbRdD5rM59SOxTvL890RELrRAEW6Cx3v0A0kJkxxKLsxpwfgOY
127+
KpY3B46Y+LNx41upA6sLKzWxgATAeXJK5TPtr5Es8wYPYuQR0JGYJJCv2pPABHTr
128+
aWGQHkdDjf+YV+VISVtD3yK5uZopKZzy3qv6mhKSP5ZOv9zVFJffHrAyMjSQMiTT
129+
iuGZ1bJtz/f+BPxU0JlC18ZORXN7iJLHhErM+hkCAwEAAaNTMFEwHQYDVR0OBBYE
130+
FOCa9ssHAazYsg4eCeXJwpVaEWbcMB8GA1UdIwQYMBaAFOCa9ssHAazYsg4eCeXJ
131+
wpVaEWbcMA8GA1UdEwEB/wQFMAMBAf8wDQYJKoZIhvcNAQELBQADggEBAI7tgXgb
132+
PKrT/0mLDK1qeIC/kCXoeAfv0WvQ8LKlGeaHG8IwXvbZS4ZVkiMIb3dN/A+taY4M
133+
3GvYn5O1z0v6BJhDZKVLKQXI0zYW+8w4wjoquELyz7EHR8+8tUNAN6bWoUpo6npO
134+
vuP+weaGjg1FHiyxmuMmqC0WqBOl71uyzUxsV4d1iOPNosdBahONQ36BafgUT4CU
135+
Vl47oh7M7k/idihozfse+qBWyXR8yqdhunrn6arV0aqtJ0htfXaBNFGfESvl6qhd
136+
UEkhEUgk/J/NUrZfq4hmnU0MwPxodua8b8gvl58n9O3f9lbmNc6k9xbt44coZEOx
137+
gs5WXXBDclPQMjg=
138+
-----END CERTIFICATE-----
139+
""";
140+
// Dummy cert valid until 2126 for testing purposes
141+
String dummyCert = """
142+
-----BEGIN CERTIFICATE-----
143+
MIIDlzCCAn+gAwIBAgIULOreW0R5KZmFBnzDzzU+9Yemb2MwDQYJKoZIhvcNAQEL
144+
BQAwWjELMAkGA1UEBhMCQVUxEzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoM
145+
GEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZDETMBEGA1UEAwwKY2x1c3Rlci1jYTAg
146+
Fw0yNjAyMTYxNTI4MzhaGA8yMTI2MDEyMzE1MjgzOFowWjELMAkGA1UEBhMCQVUx
147+
EzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoMGEludGVybmV0IFdpZGdpdHMg
148+
UHR5IEx0ZDETMBEGA1UEAwwKY2x1c3Rlci1jYTCCASIwDQYJKoZIhvcNAQEBBQAD
149+
ggEPADCCAQoCggEBAL5mmWQZhofW12eZIMULMIkDg3vmRnUgR90w2fYER4tbFzPI
150+
pd0ZFkqrs/TkViV5aJcl8heXjBh3rmo18dkC03E0bfCIRSB9v9hllBpcSiX9zI6S
151+
lqQhlXMnbcoPCK2OPZpLlajuiRnc58GLL9tv7xpgBdUeh4sZ+KsAlgFWo7SbO+hR
152+
eyT0Ut/XsNDTVIVpUPuqsDLYzyVLvbupe4Esf+OUGp9WYgpbzPlQtADB7FLWAO7U
153+
4Ag++PR4aeqwxHwXkHR2sLyfsvFQQ6H0SNRZ7EVFVWmcmSpu0OyUtCzSViEJOwTC
154+
wkLL3yuazj3ItntijdxEvXoi1qW33iak4DPXeb8CAwEAAaNTMFEwHQYDVR0OBBYE
155+
FBGpiw0mw5DlDFWNCDymbMKN/M29MB8GA1UdIwQYMBaAFBGpiw0mw5DlDFWNCDym
156+
bMKN/M29MA8GA1UdEwEB/wQFMAMBAf8wDQYJKoZIhvcNAQELBQADggEBADUeHezN
157+
4A/aBNYL23XE9wJntdV02L+NfmlvbPQewKKWxupTkLhvo6Iam25SQ7YpCvN8qWau
158+
u/SvI51sHmUS0uGY2SfVVVADMjGkyJLtYCqYU9QSDxlWjT4pWVKz/t+L42l6Zx+A
159+
OfMbtJxcZnOIk52wGerTXBOYWn/7RFU5oe1Ok/y+w+CdkXVCdOc9AgO82R1KsurH
160+
sKNCWwP0TQxR8J7o6ycO4yQPNs36HrWDq4O05MwArSI10sY04iTNJiwVmV1Yn4t6
161+
jZQE7PgOGbAPOLdqzyH+VfX8N7y5HUcyrVdtCozGHwJmFUIpY3fPAuMokZjGyOm3
162+
b5jRgwN6wcq20no=
163+
-----END CERTIFICATE-----
164+
""";
115165
Map<String, String> clusterCaCertData = new HashMap<>();
116-
clusterCaCertData.put(Ca.CA_CRT, Base64.getEncoder().encodeToString("new-dummy-crt".getBytes()));
166+
clusterCaCertData.put(Ca.CA_CRT, Base64.getEncoder().encodeToString(newDummyCert.getBytes()));
117167
clusterCaCertData.put(Ca.CA_STORE, Base64.getEncoder().encodeToString("dummy-p12".getBytes()));
118168
clusterCaCertData.put(Ca.CA_STORE_PASSWORD, Base64.getEncoder().encodeToString("dummy-password".getBytes()));
119169
// simulate old cert still present
120-
clusterCaCertData.put("ca-2023-03-23T09-00-00Z.crt", Base64.getEncoder().encodeToString("dummy-crt".getBytes()));
170+
clusterCaCertData.put("ca-2023-03-23T09-00-00Z.crt", Base64.getEncoder().encodeToString(dummyCert.getBytes()));
121171

122172
Secret clusterCaCert = new SecretBuilder()
123173
.withNewMetadata()
@@ -143,10 +193,10 @@ public void testNotRemoveOldCertificateWithCustomCa() {
143193
// checking that the cluster CA related Secret was not touched by the operator
144194
Map<String, String> clusterCaCertDataInSecret = clusterCa.caCertData();
145195
assertThat(clusterCaCertDataInSecret.size(), is(4));
146-
assertThat(Util.decodeFromBase64(clusterCaCertDataInSecret.get(Ca.CA_CRT)).equals("new-dummy-crt"), is(true));
196+
assertThat(Util.decodeFromBase64(clusterCaCertDataInSecret.get(Ca.CA_CRT)).equals(newDummyCert), is(true));
147197
assertThat(Util.decodeFromBase64(clusterCaCertDataInSecret.get(Ca.CA_STORE)).equals("dummy-p12"), is(true));
148198
assertThat(Util.decodeFromBase64(clusterCaCertDataInSecret.get(Ca.CA_STORE_PASSWORD)).equals("dummy-password"), is(true));
149-
assertThat(Util.decodeFromBase64(clusterCaCertDataInSecret.get("ca-2023-03-23T09-00-00Z.crt")).equals("dummy-crt"), is(true));
199+
assertThat(Util.decodeFromBase64(clusterCaCertDataInSecret.get("ca-2023-03-23T09-00-00Z.crt")).equals(dummyCert), is(true));
150200
}
151201

152202
@Test

cluster-operator/src/test/java/io/strimzi/operator/cluster/operator/assembly/CaReconcilerReconcileCasTest.java

Lines changed: 124 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@
4646
import org.mockito.ArgumentCaptor;
4747

4848
import java.io.ByteArrayInputStream;
49+
import java.io.File;
50+
import java.io.FileInputStream;
4951
import java.io.IOException;
5052
import java.nio.charset.StandardCharsets;
5153
import java.nio.file.Files;
@@ -58,6 +60,8 @@
5860
import java.security.cert.X509Certificate;
5961
import java.time.Clock;
6062
import java.time.Instant;
63+
import java.time.ZonedDateTime;
64+
import java.time.temporal.ChronoUnit;
6165
import java.util.ArrayList;
6266
import java.util.Base64;
6367
import java.util.HashMap;
@@ -1086,21 +1090,60 @@ public void testReconcileCasWhenUserManagedCertsAreMissingThrows(Vertx vertx, Ve
10861090
}
10871091

10881092
@Test
1089-
public void testUserManagedCertsNotReconciled(Vertx vertx, VertxTestContext context)
1090-
throws IOException, CertificateException, KeyStoreException, NoSuchAlgorithmException {
1093+
public void testUserManagedCertsNotReconciled(Vertx vertx, VertxTestContext context) throws IOException {
10911094
CertificateAuthority certificateAuthority = new CertificateAuthorityBuilder()
1092-
.withValidityDays(2)
1093-
.withRenewalDays(3)
1095+
.withValidityDays(100)
1096+
.withRenewalDays(10)
10941097
.withGenerateCertificateAuthority(false)
10951098
.build();
10961099

1097-
List<Secret> clusterCaSecrets = initialClusterCaSecrets(certificateAuthority);
1098-
Secret initialClusterCaKeySecret = clusterCaSecrets.get(0);
1099-
Secret initialClusterCaCertSecret = clusterCaSecrets.get(1);
1100-
1101-
List<Secret> clientsCaSecrets = initialClientsCaSecrets(certificateAuthority);
1102-
Secret initialClientsCaKeySecret = clientsCaSecrets.get(0);
1103-
Secret initialClientsCaCertSecret = clientsCaSecrets.get(1);
1100+
Subject sbj = new Subject.Builder()
1101+
.withOrganizationName("io.strimzi")
1102+
.withCommonName("ca").build();
1103+
1104+
File rootKey = Files.createTempFile("key-", ".key").toFile();
1105+
rootKey.deleteOnExit();
1106+
File rootCert = Files.createTempFile("crt-", ".crt").toFile();
1107+
rootCert.deleteOnExit();
1108+
File intermediateKey1 = Files.createTempFile("key-", ".key").toFile();
1109+
intermediateKey1.deleteOnExit();
1110+
File intermediateCert1 = Files.createTempFile("crt-", ".crt").toFile();
1111+
intermediateCert1.deleteOnExit();
1112+
File intermediateKey2 = Files.createTempFile("key-", ".key").toFile();
1113+
intermediateKey2.deleteOnExit();
1114+
File intermediateCert2 = Files.createTempFile("crt-", ".crt").toFile();
1115+
1116+
Instant now = Instant.now();
1117+
ZonedDateTime notBefore = now.truncatedTo(ChronoUnit.SECONDS).atZone(Clock.systemUTC().getZone());
1118+
ZonedDateTime notAfter = now.plus(2, ChronoUnit.HOURS).truncatedTo(ChronoUnit.SECONDS).atZone(Clock.systemUTC().getZone());
1119+
CERT_MANAGER.generateRootCaCert(sbj, rootKey, rootCert, notBefore, notAfter, 1);
1120+
1121+
// Generate an intermediate cert
1122+
Subject intermediateSubject1 = new Subject.Builder().withCommonName("IntermediateCn1").withOrganizationName("MyOrganization").build();
1123+
CERT_MANAGER.generateIntermediateCaCert(rootKey, rootCert, intermediateSubject1, intermediateKey1, intermediateCert1, notBefore, notAfter, 1);
1124+
1125+
// Generate an additional intermediate cert
1126+
Subject intermediateSubject2 = new Subject.Builder().withCommonName("IntermediateCn2").withOrganizationName("MyOrganization").build();
1127+
CERT_MANAGER.generateIntermediateCaCert(intermediateKey1, intermediateCert1, intermediateSubject2, intermediateKey2, intermediateCert2, notBefore, notAfter, 1);
1128+
1129+
String caKey = Base64.getEncoder().encodeToString(Files.readAllBytes(rootKey.toPath()));
1130+
1131+
// Generate combined Pem files with CA chain in correct order
1132+
String validCombinedPem;
1133+
try (FileInputStream int2CertFis = new FileInputStream(intermediateCert2);
1134+
FileInputStream int1CertFis = new FileInputStream(intermediateCert1);
1135+
FileInputStream rootCertFis = new FileInputStream(rootCert)) {
1136+
String combinedPem = String.join("\n",
1137+
new String(int2CertFis.readAllBytes(), StandardCharsets.US_ASCII),
1138+
new String(int1CertFis.readAllBytes(), StandardCharsets.US_ASCII),
1139+
new String(rootCertFis.readAllBytes(), StandardCharsets.US_ASCII));
1140+
validCombinedPem = Base64.getEncoder().encodeToString(combinedPem.getBytes(StandardCharsets.US_ASCII));
1141+
}
1142+
1143+
Secret initialClusterCaKeySecret = ResourceUtils.createInitialCaKeySecret(NAMESPACE, NAME, AbstractModel.clusterCaKeySecretName(NAME), caKey);
1144+
Secret initialClusterCaCertSecret = ResourceUtils.createInitialCaCertSecret(NAMESPACE, NAME, AbstractModel.clusterCaCertSecretName(NAME), validCombinedPem, null, null);
1145+
Secret initialClientsCaKeySecret = ResourceUtils.createInitialCaKeySecret(NAMESPACE, NAME, KafkaResources.clientsCaKeySecretName(NAME), caKey);
1146+
Secret initialClientsCaCertSecret = ResourceUtils.createInitialCaCertSecret(NAMESPACE, NAME, KafkaResources.clientsCaCertificateSecretName(NAME), validCombinedPem, null, null);
11041147

11051148
secrets.add(initialClusterCaCertSecret);
11061149
secrets.add(initialClusterCaKeySecret);
@@ -1117,4 +1160,74 @@ public void testUserManagedCertsNotReconciled(Vertx vertx, VertxTestContext cont
11171160
async.flag();
11181161
})));
11191162
}
1163+
1164+
@Test
1165+
public void testUserManagedCasWithInvalidCa(Vertx vertx, VertxTestContext context) throws IOException {
1166+
CertificateAuthority certificateAuthority = new CertificateAuthorityBuilder()
1167+
.withValidityDays(100)
1168+
.withRenewalDays(10)
1169+
.withGenerateCertificateAuthority(false)
1170+
.build();
1171+
1172+
Subject sbj = new Subject.Builder()
1173+
.withOrganizationName("io.strimzi")
1174+
.withCommonName("ca").build();
1175+
1176+
File rootKey = Files.createTempFile("key-", ".key").toFile();
1177+
rootKey.deleteOnExit();
1178+
File rootCert = Files.createTempFile("crt-", ".crt").toFile();
1179+
rootCert.deleteOnExit();
1180+
File intermediateKey1 = Files.createTempFile("key-", ".key").toFile();
1181+
intermediateKey1.deleteOnExit();
1182+
File intermediateCert1 = Files.createTempFile("crt-", ".crt").toFile();
1183+
intermediateCert1.deleteOnExit();
1184+
File intermediateKey2 = Files.createTempFile("key-", ".key").toFile();
1185+
intermediateKey2.deleteOnExit();
1186+
File intermediateCert2 = Files.createTempFile("crt-", ".crt").toFile();
1187+
1188+
Instant now = Instant.now();
1189+
ZonedDateTime notBefore = now.truncatedTo(ChronoUnit.SECONDS).atZone(Clock.systemUTC().getZone());
1190+
ZonedDateTime notAfter = now.plus(2, ChronoUnit.HOURS).truncatedTo(ChronoUnit.SECONDS).atZone(Clock.systemUTC().getZone());
1191+
CERT_MANAGER.generateRootCaCert(sbj, rootKey, rootCert, notBefore, notAfter, 1);
1192+
1193+
// Generate an intermediate cert
1194+
Subject intermediateSubject1 = new Subject.Builder().withCommonName("IntermediateCn1").withOrganizationName("MyOrganization").build();
1195+
CERT_MANAGER.generateIntermediateCaCert(rootKey, rootCert, intermediateSubject1, intermediateKey1, intermediateCert1, notBefore, notAfter, 1);
1196+
1197+
// Generate an additional intermediate cert
1198+
Subject intermediateSubject2 = new Subject.Builder().withCommonName("IntermediateCn2").withOrganizationName("MyOrganization").build();
1199+
CERT_MANAGER.generateIntermediateCaCert(intermediateKey1, intermediateCert1, intermediateSubject2, intermediateKey2, intermediateCert2, notBefore, notAfter, 1);
1200+
1201+
String caKey = Base64.getEncoder().encodeToString(Files.readAllBytes(rootKey.toPath()));
1202+
1203+
// Generate combined Pem files with CA chain in wrong order
1204+
String invalidCombinedPem;
1205+
try (FileInputStream int2CertFis = new FileInputStream(intermediateCert2);
1206+
FileInputStream int1CertFis = new FileInputStream(intermediateCert1);
1207+
FileInputStream rootCertFis = new FileInputStream(rootCert)) {
1208+
String combinedPem = String.join("\n",
1209+
new String(rootCertFis.readAllBytes(), StandardCharsets.US_ASCII),
1210+
new String(int1CertFis.readAllBytes(), StandardCharsets.US_ASCII),
1211+
new String(int2CertFis.readAllBytes(), StandardCharsets.US_ASCII));
1212+
invalidCombinedPem = Base64.getEncoder().encodeToString(combinedPem.getBytes(StandardCharsets.US_ASCII));
1213+
}
1214+
1215+
Secret initialClusterCaKeySecret = ResourceUtils.createInitialCaKeySecret(NAMESPACE, NAME, AbstractModel.clusterCaKeySecretName(NAME), caKey);
1216+
Secret initialClusterCaCertSecret = ResourceUtils.createInitialCaCertSecret(NAMESPACE, NAME, AbstractModel.clusterCaCertSecretName(NAME), invalidCombinedPem, null, null);
1217+
Secret initialClientsCaKeySecret = ResourceUtils.createInitialCaKeySecret(NAMESPACE, NAME, KafkaResources.clientsCaKeySecretName(NAME), caKey);
1218+
Secret initialClientsCaCertSecret = ResourceUtils.createInitialCaCertSecret(NAMESPACE, NAME, KafkaResources.clientsCaCertificateSecretName(NAME), invalidCombinedPem, null, null);
1219+
1220+
secrets.add(initialClusterCaCertSecret);
1221+
secrets.add(initialClusterCaKeySecret);
1222+
secrets.add(initialClientsCaCertSecret);
1223+
secrets.add(initialClientsCaKeySecret);
1224+
1225+
Checkpoint async = context.checkpoint();
1226+
reconcileCas(vertx, certificateAuthority, certificateAuthority)
1227+
.onComplete(context.failing(e -> context.verify(() -> {
1228+
assertThat(e, instanceOf(RuntimeException.class));
1229+
assertThat(e.getMessage(), is("User supplied Cluster CA cert chain ca.crt is not valid. Certificates must be provided in the correct order."));
1230+
async.flag();
1231+
})));
1232+
}
11201233
}

0 commit comments

Comments
 (0)