Use Pkcs10Csr and related types in CertificateSigner

5 files changed, 64 insertions, 148 deletions

diff --git a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ca/CertificateSerializedPayload.java b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ca/CertificateSerializedPayload.java
index 25733bf0075..cfef2bc0e33 100644
--- a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ca/CertificateSerializedPayload.java
+++ b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ca/CertificateSerializedPayload.java
@@ -7,12 +7,9 @@ import com.fasterxml.jackson.core.JsonGenerator;
 import com.fasterxml.jackson.databind.JsonSerializer;
 import com.fasterxml.jackson.databind.SerializerProvider;
 import com.fasterxml.jackson.databind.annotation.JsonSerialize;
-import org.bouncycastle.openssl.jcajce.JcaPEMWriter;
-import org.bouncycastle.util.io.pem.PemObject;
+import com.yahoo.vespa.athenz.tls.X509CertificateUtils;
 
 import java.io.IOException;
-import java.io.StringWriter;
-import java.security.cert.CertificateEncodingException;
 import java.security.cert.X509Certificate;
 
 /**
@@ -56,13 +53,7 @@ public class CertificateSerializedPayload {
         @Override
         public void serialize(
                 X509Certificate certificate, JsonGenerator gen, SerializerProvider serializers) throws IOException {
-            try (StringWriter stringWriter = new StringWriter(); JcaPEMWriter pemWriter = new JcaPEMWriter(stringWriter)) {
-                pemWriter.writeObject(new PemObject("CERTIFICATE", certificate.getEncoded()));
-                pemWriter.flush();
-                gen.writeString(stringWriter.toString());
-            } catch (CertificateEncodingException e) {
-                throw new RuntimeException("Failed to encode X509Certificate", e);
-            }
+            gen.writeString(X509CertificateUtils.toPem(certificate));
         }
     }
 }
diff --git a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ca/CertificateSigner.java b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ca/CertificateSigner.java
index f6f6bb1dbca..7b4a599d5dd 100644
--- a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ca/CertificateSigner.java
+++ b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ca/CertificateSigner.java
@@ -6,41 +6,23 @@ import com.google.inject.Inject;
 import com.yahoo.cloud.config.ConfigserverConfig;
 import com.yahoo.config.provision.Zone;
 import com.yahoo.log.LogLevel;
+import com.yahoo.vespa.athenz.tls.Extension;
+import com.yahoo.vespa.athenz.tls.Pkcs10Csr;
+import com.yahoo.vespa.athenz.tls.SignatureAlgorithm;
+import com.yahoo.vespa.athenz.tls.X509CertificateBuilder;
+import com.yahoo.vespa.athenz.tls.X509CertificateUtils;
 import com.yahoo.vespa.hosted.athenz.instanceproviderservice.KeyProvider;
 import com.yahoo.vespa.hosted.athenz.instanceproviderservice.config.AthenzProviderServiceConfig;
-import org.bouncycastle.asn1.ASN1ObjectIdentifier;
-import org.bouncycastle.asn1.DERUTF8String;
-import org.bouncycastle.asn1.pkcs.PKCSObjectIdentifiers;
-import org.bouncycastle.asn1.x500.AttributeTypeAndValue;
-import org.bouncycastle.asn1.x500.X500Name;
-import org.bouncycastle.asn1.x500.style.BCStyle;
-import org.bouncycastle.asn1.x509.BasicConstraints;
-import org.bouncycastle.asn1.x509.Extension;
-import org.bouncycastle.asn1.x509.Extensions;
-import org.bouncycastle.cert.X509v3CertificateBuilder;
-import org.bouncycastle.cert.jcajce.JcaX509CertificateConverter;
-import org.bouncycastle.cert.jcajce.JcaX509v3CertificateBuilder;
-import org.bouncycastle.jce.provider.BouncyCastleProvider;
-import org.bouncycastle.operator.ContentSigner;
-import org.bouncycastle.operator.jcajce.JcaContentSignerBuilder;
-import org.bouncycastle.pkcs.PKCS10CertificationRequest;
-import org.bouncycastle.pkcs.jcajce.JcaPKCS10CertificationRequest;
-
-import java.math.BigInteger;
+
+import javax.security.auth.x500.X500Principal;
 import java.security.PrivateKey;
-import java.security.Provider;
-import java.security.PublicKey;
 import java.security.cert.X509Certificate;
 import java.time.Clock;
 import java.time.Duration;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.Date;
-import java.util.Enumeration;
+import java.time.Instant;
 import java.util.List;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
-import java.util.stream.Stream;
 
 import static com.yahoo.vespa.hosted.athenz.instanceproviderservice.impl.Utils.getZoneConfig;
 
@@ -56,16 +38,13 @@ public class CertificateSigner {
 
     private static final Logger log = Logger.getLogger(CertificateSigner.class.getName());
 
-    static final String SIGNER_ALGORITHM = "SHA256withRSA";
+    static final SignatureAlgorithm SIGNER_ALGORITHM = SignatureAlgorithm.SHA256_WITH_RSA;
     static final Duration CERTIFICATE_EXPIRATION = Duration.ofDays(30);
-    private static final List<ASN1ObjectIdentifier> ILLEGAL_EXTENSIONS = ImmutableList.of(
-            Extension.basicConstraints, Extension.subjectAlternativeName);
-
-    private final JcaX509CertificateConverter certificateConverter = new JcaX509CertificateConverter();
-    private final Provider provider = new BouncyCastleProvider();
+    private static final List<Extension> ILLEGAL_EXTENSIONS = ImmutableList.of(
+            Extension.BASIC_CONSTRAINS, Extension.SUBJECT_ALTERNATIVE_NAMES);
 
     private final PrivateKey caPrivateKey;
-    private final X500Name issuer;
+    private final X500Principal issuer;
     private final Clock clock;
 
     @Inject
@@ -78,7 +57,7 @@ public class CertificateSigner {
 
     CertificateSigner(PrivateKey caPrivateKey, String loadBalancerAddress, Clock clock) {
         this.caPrivateKey = caPrivateKey;
-        this.issuer = new X500Name("CN=" + loadBalancerAddress);
+        this.issuer = new X500Principal("CN=" + loadBalancerAddress);
         this.clock = clock;
     }
 
@@ -89,46 +68,28 @@ public class CertificateSigner {
      *  <li>CSR does not contain any any of the extensions in {@code ILLEGAL_EXTENSIONS}</li>
      * </ul>
      */
-    X509Certificate generateX509Certificate(PKCS10CertificationRequest certReq, String remoteHostname) {
-        verifyCertificateCommonName(certReq.getSubject(), remoteHostname);
-        verifyCertificateExtensions(certReq);
-
-        Date notBefore = Date.from(clock.instant());
-        Date notAfter = Date.from(clock.instant().plus(CERTIFICATE_EXPIRATION));
+    X509Certificate generateX509Certificate(Pkcs10Csr csr, String remoteHostname) {
+        verifyCertificateCommonName(csr.getSubject(), remoteHostname);
+        verifyCertificateExtensions(csr);
 
+        Instant now = clock.instant();
         try {
-            PublicKey publicKey = new JcaPKCS10CertificationRequest(certReq).getPublicKey();
-            X509v3CertificateBuilder caBuilder = new JcaX509v3CertificateBuilder(
-                    issuer, BigInteger.valueOf(clock.millis()), notBefore, notAfter, certReq.getSubject(), publicKey)
-
-                    // Set Basic Constraints to false
-                    .addExtension(Extension.basicConstraints, true, new BasicConstraints(false));
-
-            ContentSigner caSigner = new JcaContentSignerBuilder(SIGNER_ALGORITHM)
-                    .setProvider(provider)
-                    .build(caPrivateKey);
-
-            return certificateConverter
-                    .setProvider(provider)
-                    .getCertificate(caBuilder.build(caSigner));
+            return X509CertificateBuilder.fromCsr(csr, issuer, now, now.plus(CERTIFICATE_EXPIRATION), caPrivateKey, SIGNER_ALGORITHM, now.toEpochMilli())
+                    .setBasicConstraints(true, false)
+                    .build();
         } catch (Exception ex) {
             log.log(LogLevel.ERROR, "Failed to generate X509 Certificate", ex);
-            throw new RuntimeException("Failed to generate X509 Certificate");
+            throw new RuntimeException("Failed to generate X509 Certificate", ex);
         }
     }
 
-    static void verifyCertificateCommonName(X500Name subject, String remoteHostname) {
-        List<AttributeTypeAndValue> attributesAndValues = Arrays.stream(subject.getRDNs())
-                .flatMap(rdn -> rdn.isMultiValued() ?
-                        Stream.of(rdn.getTypesAndValues()) : Stream.of(rdn.getFirst()))
-                .filter(attr -> attr.getType() == BCStyle.CN)
-                .collect(Collectors.toList());
-
-        if (attributesAndValues.size() != 1) {
+    static void verifyCertificateCommonName(X500Principal subject, String remoteHostname) {
+        List<String> commonNames = X509CertificateUtils.getCommonNames(subject);
+        if (commonNames.size() != 1) {
             throw new IllegalArgumentException("Only 1 common name should be set");
         }
 
-        String actualCommonName = DERUTF8String.getInstance(attributesAndValues.get(0).getValue()).getString();
+        String actualCommonName = commonNames.get(0);
         if (! actualCommonName.equals(remoteHostname)) {
             throw new IllegalArgumentException("Remote hostname " + remoteHostname +
                     " does not match common name " + actualCommonName);
@@ -136,15 +97,12 @@ public class CertificateSigner {
     }
 
     @SuppressWarnings("unchecked")
-    static void verifyCertificateExtensions(PKCS10CertificationRequest request) {
-        List<String> illegalExt = Arrays
-                .stream(request.getAttributes(PKCSObjectIdentifiers.pkcs_9_at_extensionRequest))
-                .map(attribute -> Extensions.getInstance(attribute.getAttrValues().getObjectAt(0)))
-                .flatMap(ext -> Collections.list((Enumeration<ASN1ObjectIdentifier>) ext.oids()).stream())
-                .filter(ILLEGAL_EXTENSIONS::contains)
-                .map(ASN1ObjectIdentifier::getId)
+    static void verifyCertificateExtensions(Pkcs10Csr csr) {
+        List<String> extensionOIds = csr.getExtensionOIds();
+        List<String> illegalExt = ILLEGAL_EXTENSIONS.stream()
+                .map(Extension::getOId)
+                .filter(extensionOIds::contains)
                 .collect(Collectors.toList());
-
         if (! illegalExt.isEmpty()) {
             throw new IllegalArgumentException("CSR contains illegal extensions: " + String.join(", ", illegalExt));
         }
diff --git a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ca/CertificateSignerResource.java b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ca/CertificateSignerResource.java
index 0c6199efdcb..1dd452866a5 100644
--- a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ca/CertificateSignerResource.java
+++ b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ca/CertificateSignerResource.java
@@ -4,7 +4,7 @@ package com.yahoo.vespa.hosted.athenz.instanceproviderservice.ca;
 import com.google.inject.Inject;
 import com.yahoo.container.jaxrs.annotation.Component;
 import com.yahoo.log.LogLevel;
-import org.bouncycastle.pkcs.PKCS10CertificationRequest;
+import com.yahoo.vespa.athenz.tls.Pkcs10Csr;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.ws.rs.BadRequestException;
@@ -45,7 +45,7 @@ public class CertificateSignerResource {
         try {
             InetAddress addr = InetAddress.getByName(req.getRemoteAddr());
             String remoteHostname = addr.getHostName();
-            PKCS10CertificationRequest csr = csrPayload.csr;
+            Pkcs10Csr csr = csrPayload.csr;
             log.log(LogLevel.DEBUG, "Certification request from " + remoteHostname + ": " + csr);
             X509Certificate certificate = certificateSigner.generateX509Certificate(csr, remoteHostname);
             return new CertificateSerializedPayload(certificate);
diff --git a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ca/CsrSerializedPayload.java b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ca/CsrSerializedPayload.java
index f56214513aa..375a4c3e17d 100644
--- a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ca/CsrSerializedPayload.java
+++ b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ca/CsrSerializedPayload.java
@@ -7,11 +7,10 @@ import com.fasterxml.jackson.core.JsonParser;
 import com.fasterxml.jackson.databind.DeserializationContext;
 import com.fasterxml.jackson.databind.JsonDeserializer;
 import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
-import org.bouncycastle.openssl.PEMParser;
-import org.bouncycastle.pkcs.PKCS10CertificationRequest;
+import com.yahoo.vespa.athenz.tls.Pkcs10Csr;
+import com.yahoo.vespa.athenz.tls.Pkcs10CsrUtils;
 
 import java.io.IOException;
-import java.io.StringReader;
 
 /**
  * Contains PEM formatted Certificate Signing Request (CSR)
@@ -20,11 +19,11 @@ import java.io.StringReader;
  */
 public class CsrSerializedPayload {
 
-    @JsonProperty("csr") public final PKCS10CertificationRequest csr;
+    @JsonProperty("csr") public final Pkcs10Csr csr;
 
     @JsonCreator
     public CsrSerializedPayload(@JsonProperty("csr") @JsonDeserialize(using = CertificateRequestDeserializer.class)
-                                            PKCS10CertificationRequest csr) {
+                                        Pkcs10Csr csr) {
         this.csr = csr;
     }
 
@@ -50,13 +49,11 @@ public class CsrSerializedPayload {
                 '}';
     }
 
-    public static class CertificateRequestDeserializer extends JsonDeserializer<PKCS10CertificationRequest> {
+    public static class CertificateRequestDeserializer extends JsonDeserializer<Pkcs10Csr> {
         @Override
-        public PKCS10CertificationRequest deserialize(
+        public Pkcs10Csr deserialize(
                 JsonParser jsonParser, DeserializationContext deserializationContext) throws IOException {
-            try (PEMParser pemParser = new PEMParser(new StringReader(jsonParser.getValueAsString()))) {
-                return (PKCS10CertificationRequest) pemParser.readObject();
-            }
+            return Pkcs10CsrUtils.fromPem(jsonParser.getValueAsString());
         }
     }
 }
diff --git a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ca/CertificateSignerTest.java b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ca/CertificateSignerTest.java
index 594bbf77fce..6c624eb1da0 100644
--- a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ca/CertificateSignerTest.java
+++ b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ca/CertificateSignerTest.java
@@ -2,23 +2,16 @@
 package com.yahoo.vespa.hosted.athenz.instanceproviderservice.ca;
 
 import com.yahoo.test.ManualClock;
-import org.bouncycastle.asn1.pkcs.PKCSObjectIdentifiers;
-import org.bouncycastle.asn1.x500.X500Name;
-import org.bouncycastle.asn1.x509.Extension;
-import org.bouncycastle.asn1.x509.Extensions;
-import org.bouncycastle.asn1.x509.ExtensionsGenerator;
-import org.bouncycastle.asn1.x509.GeneralName;
-import org.bouncycastle.asn1.x509.GeneralNames;
-import org.bouncycastle.operator.ContentSigner;
-import org.bouncycastle.operator.jcajce.JcaContentSignerBuilder;
-import org.bouncycastle.pkcs.PKCS10CertificationRequest;
-import org.bouncycastle.pkcs.PKCS10CertificationRequestBuilder;
-import org.bouncycastle.pkcs.jcajce.JcaPKCS10CertificationRequestBuilder;
+import com.yahoo.vespa.athenz.tls.Extension;
+import com.yahoo.vespa.athenz.tls.KeyAlgorithm;
+import com.yahoo.vespa.athenz.tls.KeyUtils;
+import com.yahoo.vespa.athenz.tls.Pkcs10Csr;
+import com.yahoo.vespa.athenz.tls.Pkcs10CsrBuilder;
 import org.junit.Test;
 
+import javax.security.auth.x500.X500Principal;
 import java.math.BigInteger;
 import java.security.KeyPair;
-import java.security.KeyPairGenerator;
 import java.security.cert.X509Certificate;
 import java.time.Instant;
 import java.util.Collection;
@@ -35,8 +28,6 @@ import static org.junit.Assert.fail;
  */
 public class CertificateSignerTest {
 
-    private final KeyPair clientKeyPair = getKeyPair();
-
     private final long startTime = 1234567890000L;
     private final KeyPair caKeyPair = getKeyPair();
     private final String cfgServerHostname = "cfg1.us-north-1.vespa.domain.tld";
@@ -47,22 +38,21 @@ public class CertificateSignerTest {
 
     @Test
     public void test_signing() throws Exception {
-        ExtensionsGenerator extGen = new ExtensionsGenerator();
-        String subject = "C=NO,OU=Vespa,CN=" + requestersHostname;
-        PKCS10CertificationRequest request = makeRequest(subject, extGen.generate());
+        String subject = String.format("CN=%s,OU=Vespa,C=NO", requestersHostname);
+        Pkcs10Csr csr = createCsrBuilder(subject).build();
 
-        X509Certificate certificate = signer.generateX509Certificate(request, requestersHostname);
-        assertCertificate(certificate, subject, Collections.singleton(Extension.basicConstraints.getId()));
+        X509Certificate certificate = signer.generateX509Certificate(csr, requestersHostname);
+        assertCertificate(certificate, subject, Collections.singleton(Extension.BASIC_CONSTRAINS.getOId()));
     }
 
     @Test
     public void common_name_test() throws Exception {
         CertificateSigner.verifyCertificateCommonName(
-                new X500Name("CN=" + requestersHostname), requestersHostname);
+                new X500Principal("CN=" + requestersHostname), requestersHostname);
         CertificateSigner.verifyCertificateCommonName(
-                new X500Name("C=NO,OU=Vespa,CN=" + requestersHostname), requestersHostname);
+                new X500Principal("C=NO,OU=Vespa,CN=" + requestersHostname), requestersHostname);
         CertificateSigner.verifyCertificateCommonName(
-                new X500Name("C=NO+OU=org,CN=" + requestersHostname), requestersHostname);
+                new X500Principal("C=NO+OU=org,CN=" + requestersHostname), requestersHostname);
 
         assertCertificateCommonNameException("C=NO", "Only 1 common name should be set");
         assertCertificateCommonNameException("C=US+CN=abc123.domain.tld,C=NO+CN=" + requestersHostname, "Only 1 common name should be set");
@@ -72,26 +62,15 @@ public class CertificateSignerTest {
 
     @Test(expected = IllegalArgumentException.class)
     public void extensions_test_subject_alternative_names() throws Exception {
-        ExtensionsGenerator extGen = new ExtensionsGenerator();
-        extGen.addExtension(Extension.subjectAlternativeName, false, new GeneralNames(new GeneralName[] {
-                new GeneralName(GeneralName.dNSName, "some.other.domain.tld")}));
-        PKCS10CertificationRequest request = makeRequest("OU=Vespa", extGen.generate());
-
-        CertificateSigner.verifyCertificateExtensions(request);
-    }
-
-    @Test
-    public void extensions_allowed() throws Exception {
-        ExtensionsGenerator extGen = new ExtensionsGenerator();
-        extGen.addExtension(Extension.certificateIssuer, true, new byte[0]);
-        PKCS10CertificationRequest request = makeRequest("OU=Vespa", extGen.generate());
-
-        CertificateSigner.verifyCertificateExtensions(request);
+        Pkcs10Csr csr = createCsrBuilder("OU=Vespa")
+                .addSubjectAlternativeName("some.other.domain.tld")
+                .build();
+        CertificateSigner.verifyCertificateExtensions(csr);
     }
 
     private void assertCertificateCommonNameException(String subject, String expectedMessage) {
         try {
-            CertificateSigner.verifyCertificateCommonName(new X500Name(subject), requestersHostname);
+            CertificateSigner.verifyCertificateCommonName(new X500Principal(subject), requestersHostname);
             fail("Expected to fail");
         } catch (IllegalArgumentException e) {
             assertEquals(expectedMessage, e.getMessage());
@@ -103,8 +82,8 @@ public class CertificateSignerTest {
         assertEquals(BigInteger.valueOf(startTime), certificate.getSerialNumber());
         assertEquals(startTime, certificate.getNotBefore().getTime());
         assertEquals(startTime + CertificateSigner.CERTIFICATE_EXPIRATION.toMillis(), certificate.getNotAfter().getTime());
-        assertEquals(CertificateSigner.SIGNER_ALGORITHM, certificate.getSigAlgName());
-        assertEquals(expectedSubjectName, certificate.getSubjectDN().getName());
+        assertEquals(CertificateSigner.SIGNER_ALGORITHM.getAlgorithmName(), certificate.getSigAlgName());
+        assertEquals(new X500Principal(expectedSubjectName), certificate.getSubjectX500Principal());
         assertEquals("CN=" + cfgServerHostname, certificate.getIssuerX500Principal().getName());
 
         Set<String> extensions = Stream.of(certificate.getNonCriticalExtensionOIDs(),
@@ -116,20 +95,11 @@ public class CertificateSignerTest {
         certificate.verify(caKeyPair.getPublic());
     }
 
-    private PKCS10CertificationRequest makeRequest(String subject, Extensions extensions) throws Exception {
-        PKCS10CertificationRequestBuilder builder = new JcaPKCS10CertificationRequestBuilder(
-                new X500Name(subject), clientKeyPair.getPublic());
-        builder.addAttribute(PKCSObjectIdentifiers.pkcs_9_at_extensionRequest, extensions);
-
-        ContentSigner signGen = new JcaContentSignerBuilder(CertificateSigner.SIGNER_ALGORITHM).build(caKeyPair.getPrivate());
-        return builder.build(signGen);
+    private Pkcs10CsrBuilder createCsrBuilder(String subject) {
+        return Pkcs10CsrBuilder.fromKeypair(new X500Principal(subject), caKeyPair, CertificateSigner.SIGNER_ALGORITHM);
     }
 
     private static KeyPair getKeyPair() {
-        try {
-            return KeyPairGenerator.getInstance("RSA").genKeyPair();
-        } catch (Exception e) {
-            throw new RuntimeException(e);
-        }
+        return KeyUtils.generateKeypair(KeyAlgorithm.RSA);
     }
 }