[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 <[email protected]>
logicalclocks · Dec 27, 2023 · 5541baf · 5541baf
1 parent b053bc3
commit 5541baf
Show file tree

Hide file tree

Showing 3 changed files with 79 additions and 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
@@ -140,7 +140,7 @@ public Response revokeCertificate(
 
     final List<X500Name> subjectsToRevoke = new ArrayList<>();
     try {
-      if (exact == null || !exact) {
+      if (!Boolean.TRUE.equals(exact)) {
         X500Name certificateName = pkiUtils.parseCertificateSubjectName(certId, HOST);
         List<String> 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
@@ -839,6 +839,20 @@ public X509Certificate signCertificateSigningRequest(String csrStr, CertificateT
         }
       }
     }
+    Optional<String> 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<String> parseX509CommonName(PKCS10CertificationRequest csr) {
 
   @VisibleForTesting
   Optional<String> 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<X509v3CertificateBuilder, Void> 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
@@ -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,14 +345,22 @@ 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());
     X509CertificateHolder holder = builder.build(signer);
     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<List<?>> sans = cert.getSubjectAlternativeNames();
     Assert.assertEquals(2, sans.size());
     Iterator<List<?>> sansIter = sans.iterator();