Merge branch 'pkcs1.and.pem.bc.#6'

* pkcs1.and.pem.bc.#6:
  refactor if-else-cascade to switch and improve method names and error messages
  package and publish dependencies (esp. bouncycastle) using shadow jar
  replace hard-coded key type with constants
  inline method and improve error message for PKCS#8 key generation
  replace properitary `sun.securtiy.*` API with bouncycastle code
  reuse already existing private key generation from bytes for PKSC#8 standard
  incorporate requested review changes from #6
  document the new key probabilities in README.md
  use newly introduced PKCS1 and PKCS8 support for pem files (#2)
  add PKCS1 and PKCS8 support for pem files (#2)
  enhance integration tests to be able to cover all kinds of key types easily (#2)
  cleanup
  add support for PKCS#1 keys

Author: aaschmid

README.md

@@ -111,10 +111,6 @@ Note: Keys can be transformed using `openssl`, e.g. from [PKCS#8]() in [PEM]() f
 openssl pkcs8 -topk8 -nocrypt -in $TASKD_GENERATED_KEY.key.pem -inform PEM -out $KEY_NAME.key.pkcs8.der -outform DER
 ```
 
-**Word of warning**: Current key parsing algorithm for [PKCS#1]() [PEM](() key uses
-`sun.security.util.DerInputStream` and `sun.security.util.DerValue` which are not part of the public interface, see
-https://www.oracle.com/java/technologies/faq-sun-packages.html for further explainations.
-
 [PKCS#1]: https://en.wikipedia.org/wiki/PKCS_1
 [PKCS#8]: https://en.wikipedia.org/wiki/PKCS_8
 [PEM]: https://en.wikipedia.org/wiki/Privacy-Enhanced_Mail

build.gradle.kts

@@ -4,6 +4,7 @@ import java.time.format.DateTimeFormatter
 
 plugins {
     `java-library`
+    id("com.github.johnrengelman.shadow") version "5.2.0"
     jacoco
 
     id("com.github.spotbugs") version "3.0.0"
@@ -29,7 +30,7 @@ java {
     sourceCompatibility = JavaVersion.VERSION_1_8
     targetCompatibility = JavaVersion.VERSION_1_8
 
-    withSourcesJar()
+    withJavadocJar()
     withSourcesJar()
 }
 
@@ -45,6 +46,8 @@ dependencies {
         testCompileOnly(it)
     }
 
+    implementation("org.bouncycastle:bcpkix-jdk15on:1.64")
+
     testImplementation("org.junit.jupiter:junit-jupiter:5.5.2")
     testImplementation("org.assertj:assertj-core:3.14.0")
     testImplementation("org.mockito:mockito-junit-jupiter:3.2.4")
@@ -53,7 +56,7 @@ dependencies {
 tasks {
     withType<JavaCompile> {
         options.encoding = "UTF-8"
-        options.compilerArgs.addAll(listOf("-Xlint:all", "-Werror", "-Xlint:-processing", "-XDenableSunApiLintControl"))
+        options.compilerArgs.addAll(listOf("-Xlint:all", "-Werror", "-Xlint:-processing"))
     }
 
     withType<Jar> {
@@ -64,6 +67,7 @@ tasks {
     }
 
     jar {
+        enabled = false
         manifest {
             val now = LocalDate.now()
 
@@ -92,6 +96,15 @@ tasks {
         }
     }
 
+    shadowJar {
+        archiveClassifier.set("")
+        minimize()
+        relocate("org.bouncycastle", "de.aaschmid.taskwarrior.thirdparty.org.bouncycastle")
+    }
+    assemble {
+        dependsOn(shadowJar)
+    }
+
     test {
         useJUnitPlatform()
     }
@@ -172,7 +185,11 @@ tasks.withType<GenerateModuleMetadata> {
 publishing {
     publications {
         register<MavenPublication>("mavenJava") {
-            from(components["java"])
+            // Not using `from(components["java"])` prevents from adding any shadowed dependency to resulting pom.xml
+            shadow.component(this)
+            artifact(tasks.get("javadocJar"))
+            artifact(tasks.get("sourcesJar"))
+
             pom {
                 packaging = "jar"
 

src/main/java/de/aaschmid/taskwarrior/client/KeyStoreBuilder.java

@@ -1,10 +1,11 @@
 package de.aaschmid.taskwarrior.client;
 
 import java.io.BufferedInputStream;
+import java.io.ByteArrayInputStream;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
-import java.math.BigInteger;
+import java.io.InputStreamReader;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.security.KeyFactory;
@@ -19,25 +20,26 @@
 import java.security.cert.CertificateException;
 import java.security.cert.CertificateFactory;
 import java.security.spec.InvalidKeySpecException;
-import java.security.spec.KeySpec;
 import java.security.spec.PKCS8EncodedKeySpec;
-import java.security.spec.RSAPrivateCrtKeySpec;
 import java.util.ArrayList;
-import java.util.Base64;
 import java.util.List;
 import java.util.concurrent.atomic.AtomicInteger;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
+
+import org.bouncycastle.asn1.pkcs.RSAPrivateKey;
+import org.bouncycastle.crypto.params.RSAPrivateCrtKeyParameters;
+import org.bouncycastle.crypto.util.PrivateKeyInfoFactory;
+import org.bouncycastle.openssl.jcajce.JcaPEMKeyConverter;
+import org.bouncycastle.util.io.pem.PemObject;
+import org.bouncycastle.util.io.pem.PemReader;
 
 import static java.util.Objects.requireNonNull;
 
 class KeyStoreBuilder {
 
-    private static final String TYPE_CERTIFICATE = "X.509";
-    private static final String ALGORITHM_PRIVATE_KEY = "RSA";
-
-    private static final Pattern PATTERN_PKCS1_PEM = Pattern.compile("-----BEGIN RSA PRIVATE KEY-----(.*)-----END RSA PRIVATE KEY-----");
-    private static final Pattern PATTERN_PKCS8_PEM = Pattern.compile("-----BEGIN PRIVATE KEY-----(.*)-----END PRIVATE KEY-----");
+    private static final String CERTIFICATE_TYPE = "X.509";
+    private static final String KEY_ALGORITHM_RSA = "RSA";
+    private static final String PEM_TYPE_PKCS1 = "RSA PRIVATE KEY";
+    private static final String PEM_TYPE_PKCS8 = "PRIVATE KEY";
 
     private ProtectionParameter keyStoreProtection;
     private File caCertFile;
@@ -71,6 +73,10 @@ KeyStoreBuilder withPrivateKeyCertFile(File privateKeyCertFile) {
         return this;
     }
 
+    /**
+     * Provide the non-null private key file to use. Supported are PKCS#1 and PKCS#8 keys in {@code *.PEM} format as well as PKCS#8 keys in
+     * {@code *.DER} format.
+     */
     KeyStoreBuilder withPrivateKeyFile(File privateKeyFile) {
         requireNonNull(privateKeyFile, "'privateKeyFile' must not be null.");
         if (!privateKeyFile.exists()) {
@@ -112,7 +118,7 @@ KeyStore build() {
     private List<Certificate> createCertificatesFor(File certFile) {
         List<Certificate> result = new ArrayList<>();
         try (BufferedInputStream bis = new BufferedInputStream(new FileInputStream(certFile))) {
-            CertificateFactory cf = CertificateFactory.getInstance(TYPE_CERTIFICATE);
+            CertificateFactory cf = CertificateFactory.getInstance(CERTIFICATE_TYPE);
             while (bis.available() > 0) {
                 result.add(cf.generateCertificate(bis));
             }
@@ -128,67 +134,51 @@ private PrivateKey createPrivateKeyFor(File privateKeyFile) {
         try {
             byte[] bytes = Files.readAllBytes(privateKeyFile.toPath());
             if (privateKeyFile.getName().endsWith("pem")) {
-                String content = new String(bytes, StandardCharsets.UTF_8).replaceAll("\\n", "");
-
-                Matcher pkcs1Matcher = PATTERN_PKCS1_PEM.matcher(content);
-                if (pkcs1Matcher.find()) {
-                    return createPrivateKeyFromPemPkcs1(pkcs1Matcher.group(1));
+                PemReader pemReader = new PemReader(new InputStreamReader(new ByteArrayInputStream(bytes), StandardCharsets.UTF_8));
+                PemObject privateKeyObject = pemReader.readPemObject();
+
+                switch (privateKeyObject.getType()) {
+                    case PEM_TYPE_PKCS1:
+                        return createPrivateKeyForPkcs1(privateKeyObject.getContent());
+                    case PEM_TYPE_PKCS8:
+                        return createPrivateKeyForPkcs8(privateKeyObject.getContent());
+                    default:
+                        throw new TaskwarriorKeyStoreException("Unsupported key algorithm '%s'.", privateKeyObject.getType());
                 }
-
-                Matcher pkcs8Matcher = PATTERN_PKCS8_PEM.matcher(content);
-                if (pkcs8Matcher.find()) {
-                    return createPrivateKeyFromPemPkcs8(pkcs8Matcher.group(1));
-                }
-
-                throw new TaskwarriorKeyStoreException("Could not detect key algorithm for '%s'.", privateKeyFile);
             }
-            return createPrivateKeyFromPkcs8Der(bytes);
+            return createPrivateKeyForPkcs8(bytes);
         } catch (IOException e) {
             throw new TaskwarriorKeyStoreException(e, "Could not read private key of '%s' via input stream.", privateKeyFile);
         }
     }
 
-    @SuppressWarnings("sunapi")
-    private PrivateKey createPrivateKeyFromPemPkcs1(String privateKeyContent) throws IOException {
+    private PrivateKey createPrivateKeyForPkcs1(byte[] privateKeyBytes) {
+        RSAPrivateKey rsa = RSAPrivateKey.getInstance(privateKeyBytes);
+        RSAPrivateCrtKeyParameters keyParameters = new RSAPrivateCrtKeyParameters(
+                rsa.getModulus(),
+                rsa.getPublicExponent(),
+                rsa.getPrivateExponent(),
+                rsa.getPrime1(),
+                rsa.getPrime2(),
+                rsa.getExponent1(),
+                rsa.getExponent2(),
+                rsa.getCoefficient());
+
         try {
-            byte[] bytes = Base64.getDecoder().decode(privateKeyContent);
-
-            sun.security.util.DerInputStream derReader = new sun.security.util.DerInputStream(bytes);
-            sun.security.util.DerValue[] seq = derReader.getSequence(0);
-            // skip version seq[0];
-            BigInteger modulus = seq[1].getBigInteger();
-            BigInteger publicExp = seq[2].getBigInteger();
-            BigInteger privateExp = seq[3].getBigInteger();
-            BigInteger prime1 = seq[4].getBigInteger();
-            BigInteger prime2 = seq[5].getBigInteger();
-            BigInteger exp1 = seq[6].getBigInteger();
-            BigInteger exp2 = seq[7].getBigInteger();
-            BigInteger crtCoef = seq[8].getBigInteger();
-
-            RSAPrivateCrtKeySpec keySpec = new RSAPrivateCrtKeySpec(modulus, publicExp, privateExp, prime1, prime2, exp1, exp2, crtCoef);
-            return createPrivateKey(privateKeyFile, keySpec);
-        } catch (Error | Exception e) {
-            throw new TaskwarriorKeyStoreException("Could not use required but proprietary 'sun.security.util' package on this platform.", e);
+            return new JcaPEMKeyConverter().getPrivateKey(PrivateKeyInfoFactory.createPrivateKeyInfo(keyParameters));
+        } catch (IOException e) {
+            throw new TaskwarriorKeyStoreException(e, "Failed to encode PKCS#1 private key of '%s'.", privateKeyFile);
         }
     }
 
-    private PrivateKey createPrivateKeyFromPemPkcs8(String privateKeyContent) {
-        byte[] bytes = Base64.getDecoder().decode(privateKeyContent);
-        return createPrivateKey(privateKeyFile, new PKCS8EncodedKeySpec(bytes));
-    }
-
-    private PrivateKey createPrivateKeyFromPkcs8Der(byte[] privateKeyBytes) {
-        return createPrivateKey(privateKeyFile, new PKCS8EncodedKeySpec(privateKeyBytes));
-    }
-
-    private PrivateKey createPrivateKey(File privateKeyFile, KeySpec keySpec) {
+    private PrivateKey createPrivateKeyForPkcs8(byte[] privateKeyBytes) {
         try {
-            KeyFactory keyFactory = KeyFactory.getInstance(ALGORITHM_PRIVATE_KEY);
-            return keyFactory.generatePrivate(keySpec);
+            KeyFactory keyFactory = KeyFactory.getInstance(KEY_ALGORITHM_RSA);
+            return keyFactory.generatePrivate(new PKCS8EncodedKeySpec(privateKeyBytes));
         } catch (NoSuchAlgorithmException e) {
-            throw new TaskwarriorKeyStoreException(e, "Key factory could not be initialized for algorithm '%s'.", ALGORITHM_PRIVATE_KEY);
+            throw new TaskwarriorKeyStoreException(e, "Key factory could not be initialized for algorithm '%s'.", KEY_ALGORITHM_RSA);
         } catch (InvalidKeySpecException e) {
-            throw new TaskwarriorKeyStoreException(e, "Could not generate private key for '%s'.", privateKeyFile);
+            throw new TaskwarriorKeyStoreException(e, "Invalid key spec for %s private key in '%s'.", KEY_ALGORITHM_RSA, privateKeyFile);
         }
     }
 }

src/test/java/de/aaschmid/taskwarrior/client/KeyStoreBuilderTest.java

@@ -148,7 +148,7 @@ void build_shouldTaskwarriorKeyStoreExceptionIfPrivateKeyIsInvalid() throws IOEx
 
         assertThatThrownBy(builder::build)
                 .isInstanceOf(TaskwarriorKeyStoreException.class)
-                .hasMessage("Could not generate private key for '" + privateKeyFile.toAbsolutePath() + "'.")
+                .hasMessage("Invalid key spec for RSA private key in '" + privateKeyFile.toAbsolutePath() + "'.")
                 .hasCauseInstanceOf(InvalidKeySpecException.class);
     }