From 5541baf9e2ebe614eb92babdda606d4c06ffbc94 Mon Sep 17 00:00:00 2001 From: Antonis Kouzoupis Date: Wed, 27 Dec 2023 08:51:40 +0100 Subject: [PATCH] [HWORKS-887] Fix revoke Kubernetes certificate API (#1439) [HWORKS-887] Issue Strimzi operator CA certificate * Update hopsworks-ca/src/main/java/io/hops/hopsworks/ca/api/certificates/KubeCertsResource.java * [HWORKS-887] Update check based on suggestion * [HWORKS-887] Fix tests for BasicConstrains --------- Co-authored-by: Ralf --- .../api/certificates/HostCertsResource.java | 2 +- .../io/hops/hopsworks/ca/controllers/PKI.java | 19 +++++- .../controllers/TestCertificateSigning.java | 61 ++++++++++++++++++- 3 files changed, 79 insertions(+), 3 deletions(-) diff --git a/hopsworks-ca/src/main/java/io/hops/hopsworks/ca/api/certificates/HostCertsResource.java b/hopsworks-ca/src/main/java/io/hops/hopsworks/ca/api/certificates/HostCertsResource.java index 35a0b39645..16c0cbfcbf 100644 --- a/hopsworks-ca/src/main/java/io/hops/hopsworks/ca/api/certificates/HostCertsResource.java +++ b/hopsworks-ca/src/main/java/io/hops/hopsworks/ca/api/certificates/HostCertsResource.java @@ -140,7 +140,7 @@ public Response revokeCertificate( final List subjectsToRevoke = new ArrayList<>(); try { - if (exact == null || !exact) { + if (!Boolean.TRUE.equals(exact)) { X500Name certificateName = pkiUtils.parseCertificateSubjectName(certId, HOST); List subjects = pkiUtils.findAllValidSubjectsWithPartialMatch(certificateName.toString()); subjects.forEach(s -> subjectsToRevoke.add(new X500Name(s))); diff --git a/hopsworks-ca/src/main/java/io/hops/hopsworks/ca/controllers/PKI.java b/hopsworks-ca/src/main/java/io/hops/hopsworks/ca/controllers/PKI.java index fa9d937f06..276ba5910b 100644 --- a/hopsworks-ca/src/main/java/io/hops/hopsworks/ca/controllers/PKI.java +++ b/hopsworks-ca/src/main/java/io/hops/hopsworks/ca/controllers/PKI.java @@ -839,6 +839,20 @@ public X509Certificate signCertificateSigningRequest(String csrStr, CertificateT } } } + Optional locality = parseX509Locality(b.certificationRequest); + if (locality.isPresent()) { + if (locality.get().equals("strimzica")) { + try { + // This certificate is for Strimzi operator CA, make it able to sign other certificates + b.certificateBuilder.replaceExtension(Extension.basicConstraints, true, new BasicConstraints(3)); + b.certificateBuilder.replaceExtension(Extension.keyUsage, true, + new KeyUsage(KeyUsage.digitalSignature | KeyUsage.keyEncipherment | KeyUsage.dataEncipherment + | KeyUsage.keyCertSign)); + } catch (CertIOException ex) { + throw new RuntimeException(ex); + } + } + } return null; }; @@ -958,6 +972,9 @@ Optional parseX509CommonName(PKCS10CertificationRequest csr) { @VisibleForTesting Optional parseX509Locality(PKCS10CertificationRequest csr) { + if (csr == null) { + return Optional.empty(); + } return parseX509Rdn(csr, BCStyle.L); } @@ -1311,7 +1328,7 @@ public X509Certificate loadCertificate(String name, PKICertificate.Status status private static final Function INTERMEDIATE_EXTENSIONS = (builder) -> { try { - builder.addExtension(Extension.basicConstraints, true, new BasicConstraints(0)); + builder.addExtension(Extension.basicConstraints, true, new BasicConstraints(5)); builder.addExtension(Extension.keyUsage, true, new KeyUsage(KeyUsage.keyCertSign | KeyUsage.cRLSign | KeyUsage.digitalSignature)); return null; diff --git a/hopsworks-ca/src/test/java/io/hops/hopsworks/ca/controllers/TestCertificateSigning.java b/hopsworks-ca/src/test/java/io/hops/hopsworks/ca/controllers/TestCertificateSigning.java index 29d97a7c19..c646055fcd 100644 --- a/hopsworks-ca/src/test/java/io/hops/hopsworks/ca/controllers/TestCertificateSigning.java +++ b/hopsworks-ca/src/test/java/io/hops/hopsworks/ca/controllers/TestCertificateSigning.java @@ -24,12 +24,16 @@ import io.hops.hopsworks.ca.configuration.UsernamesConfiguration; import io.hops.hopsworks.persistence.entity.pki.CAType; import io.hops.hopsworks.persistence.entity.pki.PKICertificate; +import org.bouncycastle.asn1.ASN1Encodable; import org.bouncycastle.asn1.x500.X500Name; +import org.bouncycastle.asn1.x509.BasicConstraints; import org.bouncycastle.asn1.x509.Extension; import org.bouncycastle.asn1.x509.GeneralName; +import org.bouncycastle.asn1.x509.KeyUsage; import org.bouncycastle.cert.X509CertificateHolder; import org.bouncycastle.cert.X509v3CertificateBuilder; import org.bouncycastle.cert.jcajce.JcaX509CertificateConverter; +import org.bouncycastle.cert.jcajce.JcaX509ExtensionUtils; import org.bouncycastle.cert.jcajce.JcaX509v3CertificateBuilder; import org.bouncycastle.operator.ContentSigner; import org.bouncycastle.operator.jcajce.JcaContentSignerBuilder; @@ -270,6 +274,53 @@ public void testKubernetesCertificateExtensionsBuilderNoSAN() throws Exception { Assert.assertEquals(0, holder.getNonCriticalExtensionOIDs().size()); } + @Test + public void testKubernetesStrimziCA() throws Exception { + CAsConfiguration casConf = new CAsConfiguration(null, null, null); + Gson gson = new Gson(); + String jsonConf = gson.toJson(casConf); + + CAConf caConf = Mockito.mock(CAConf.class); + Mockito.when(caConf.getString(Mockito.any())).thenReturn(jsonConf); + + PKI pki = new PKI(); + pki.setCaConf(caConf); + pki.init(); + + KeyPair keyPair = pki.generateKeyPair(); + X500Name requesterName = new X500Name("CN=strimzi,L=strimzica"); + JcaPKCS10CertificationRequestBuilder csrBuilder = new JcaPKCS10CertificationRequestBuilder(requesterName, + keyPair.getPublic()); + PKCS10CertificationRequest csr = csrBuilder.build( + new JcaContentSignerBuilder("SHA256withRSA").build(keyPair.getPrivate())); + + X509v3CertificateBuilder builder = new X509v3CertificateBuilder( + new X500Name("CN=signer"), + BigInteger.ONE, + Date.from(Instant.now()), + Date.from(Instant.now().plus(10, ChronoUnit.DAYS)), + csr.getSubject(), + csr.getSubjectPublicKeyInfo()); + builder + .addExtension(Extension.basicConstraints, true, new BasicConstraints(false)) + .addExtension(Extension.keyUsage, true, + new KeyUsage(KeyUsage.digitalSignature | KeyUsage.keyEncipherment | KeyUsage.dataEncipherment)); + pki.KUBE_CERTIFICATE_EXTENSIONS_BUILDER.apply(PKI.ExtensionsBuilderParameter.of(builder, csr, + CertificateType.KUBE, "")); + ContentSigner signer = new JcaContentSignerBuilder(PKI.SIGNATURE_ALGORITHM) + .build(keyPair.getPrivate()); + X509CertificateHolder holder = builder.build(signer); + JcaX509CertificateConverter converter = new JcaX509CertificateConverter(); + X509Certificate cert = converter.getCertificate(holder); + Assert.assertNotNull(cert); + byte[] bc = cert.getExtensionValue(Extension.basicConstraints.getId()); + ASN1Encodable e = JcaX509ExtensionUtils.parseExtensionValue(bc); + BasicConstraints basicConstraints = BasicConstraints.getInstance(e); + // Certificate issued for StrimziCA should have the CA flag on + Assert.assertTrue(basicConstraints.isCA()); + Assert.assertEquals(BigInteger.valueOf(3), basicConstraints.getPathLenConstraint()); + } + @Test public void testKubernetesCertificateExtensionsBuilderDNSSAN() throws Exception { Gson gson = new Gson(); @@ -294,7 +345,8 @@ public void testKubernetesCertificateExtensionsBuilderDNSSAN() throws Exception new X500Name("CN=hello"), keyPair.getPublic() ); - + builder + .addExtension(Extension.basicConstraints, true, new BasicConstraints(false)); pki.KUBE_CERTIFICATE_EXTENSIONS_BUILDER.apply(PKI.ExtensionsBuilderParameter.of(builder)); ContentSigner signer = new JcaContentSignerBuilder(PKI.SIGNATURE_ALGORITHM) .build(keyPair.getPrivate()); @@ -302,6 +354,13 @@ public void testKubernetesCertificateExtensionsBuilderDNSSAN() throws Exception Assert.assertEquals(1, holder.getNonCriticalExtensionOIDs().size()); JcaX509CertificateConverter converter = new JcaX509CertificateConverter(); X509Certificate cert = converter.getCertificate(holder); + + // Certificates other than StrimziCA should not be CA + byte[] bc = cert.getExtensionValue(Extension.basicConstraints.getId()); + ASN1Encodable e = JcaX509ExtensionUtils.parseExtensionValue(bc); + BasicConstraints basicConstraints = BasicConstraints.getInstance(e); + Assert.assertFalse(basicConstraints.isCA()); + Collection> sans = cert.getSubjectAlternativeNames(); Assert.assertEquals(2, sans.size()); Iterator> sansIter = sans.iterator();