Bill commented on a change in pull request #6707:
URL: https://github.com/apache/geode/pull/6707#discussion_r673479246



##########
File path: 
geode-junit/src/main/java/org/apache/geode/cache/ssl/CertificateBuilder.java
##########
@@ -79,16 +77,27 @@ public CertificateBuilder(int days, String algorithm) {
   }
 
   private static GeneralName dnsGeneralName(String name) {
-    return new GeneralName(GeneralName.dNSName, name);
+    try {
+      return new GeneralName(new DNSName(name));
+    } catch (IOException ex) {
+      throw new UncheckedIOException(ex);
+    }

Review comment:
       The value of bundling this as into a runtime exception is that this 
method can still be used in contexts (e.g. Stream.map(f)`) where a method 
throwing a checked exception would not be allowed. Ok.

##########
File path: geode-junit/build.gradle
##########
@@ -19,6 +19,10 @@ apply from: 
"${rootDir}/${scriptDir}/standard-subproject-configuration.gradle"
 
 apply from: "${project.projectDir}/../gradle/publish-java.gradle"
 
+compileJava {
+  options.compilerArgs << '-Xlint:-sunapi'
+  options.compilerArgs << '-XDenableSunApiLintControl'

Review comment:
       So we've moved from Bouncy Castle, a high quality open source project, 
to an internal JDK class. These compiler options suppress what would be compile 
errors I suppose (or at least warnings) that we are illegally accessing JDK 
internal classes.
   
   My research shows that in JDK 9 these classes in a `sun` package will move 
to a `javax` one. I suppose when that happens the JDK8 internal classes 
accessed here will have direct replacements available so all we'll have to do 
is change the includes in these files.
   
   

##########
File path: 
geode-junit/src/main/java/org/apache/geode/cache/ssl/CertificateBuilder.java
##########
@@ -146,58 +164,69 @@ public CertificateMaterial generate() {
   }
 
   private X509Certificate generate(PublicKey publicKey, PrivateKey privateKey) 
{
+    Date from = new Date();
+    Date to = new Date(from.getTime() + days * 86_400_000L);
+
+    CertificateValidity interval = new CertificateValidity(from, to);
+    BigInteger sn = new BigInteger(64, new SecureRandom());
+
+    X509CertInfo info = new X509CertInfo();
+
     try {
-      AlgorithmIdentifier sigAlgId =
-          new DefaultSignatureAlgorithmIdentifierFinder().find(algorithm);
-      AlgorithmIdentifier digAlgId = new 
DefaultDigestAlgorithmIdentifierFinder().find(sigAlgId);
-      AsymmetricKeyParameter publicKeyAsymKeyParam =
-          PublicKeyFactory.createKey(publicKey.getEncoded());
-      AsymmetricKeyParameter privateKeyAsymKeyParam =
-          PrivateKeyFactory.createKey(privateKey.getEncoded());
-      SubjectPublicKeyInfo subPubKeyInfo =
-          SubjectPublicKeyInfo.getInstance(publicKey.getEncoded());
-
-      ContentSigner sigGen =
-          new BcRSAContentSignerBuilder(sigAlgId, 
digAlgId).build(privateKeyAsymKeyParam);
-
-      Date from = new Date();
-      Date to = new Date(from.getTime() + days * 86400000L);
-      BigInteger sn = new BigInteger(64, new SecureRandom());
-
-      X509v3CertificateBuilder v3CertGen;
+      info.set(X509CertInfo.VALIDITY, interval);
+      info.set(X509CertInfo.SERIAL_NUMBER, new CertificateSerialNumber(sn));
+      info.set(X509CertInfo.SUBJECT, name);
+      info.set(X509CertInfo.KEY, new CertificateX509Key(publicKey));
+      info.set(X509CertInfo.VERSION, new 
CertificateVersion(CertificateVersion.V3));
+      AlgorithmId algo = new AlgorithmId(AlgorithmId.md5WithRSAEncryption_oid);
+      info.set(X509CertInfo.ALGORITHM_ID, new CertificateAlgorithmId(algo));
+
       if (issuer == null) {
         // This is a self-signed certificate
-        v3CertGen = new X509v3CertificateBuilder(name, sn, from, to, name, 
subPubKeyInfo);
+        info.set(X509CertInfo.ISSUER, name);
       } else {
-        v3CertGen = new X509v3CertificateBuilder(
-            new X500Name(issuer.getCertificate().getIssuerDN().getName()),
-            sn, from, to, name, subPubKeyInfo);
+        info.set(X509CertInfo.ISSUER, issuer.getCertificate().getSubjectDN());
       }
 
-      byte[] subjectAltName = san();
-      if (subjectAltName != null) {
-        v3CertGen.addExtension(Extension.subjectAlternativeName, false, 
subjectAltName);
+      CertificateExtensions extensions = new CertificateExtensions();
+
+      byte[] keyIdBytes = new KeyIdentifier(publicKey).getIdentifier();
+      SubjectKeyIdentifierExtension keyIdentifier = new 
SubjectKeyIdentifierExtension(keyIdBytes);
+      extensions.set(SubjectKeyIdentifierExtension.NAME, keyIdentifier);
+
+      GeneralNames subjectAltNames = san();
+      if (!subjectAltNames.isEmpty()) {
+        SubjectAlternativeNameExtension altNames =
+            new SubjectAlternativeNameExtension(subjectAltNames);
+        extensions.set(SubjectAlternativeNameExtension.NAME, altNames);
       }
 
       if (isCA) {
-        v3CertGen.addExtension(Extension.keyUsage, true, new 
KeyUsage(KeyUsage.keyCertSign));
-        v3CertGen.addExtension(Extension.basicConstraints, true, new 
BasicConstraints(0));
+        KeyUsageExtension usageExtension = new KeyUsageExtension();
+        usageExtension.set(KeyUsageExtension.KEY_CERTSIGN, true);
+        extensions.set(KeyUsageExtension.NAME, usageExtension);
+
+        BasicConstraintsExtension basicConstraints = new 
BasicConstraintsExtension(true, 0);
+        extensions.set(BasicConstraintsExtension.NAME, basicConstraints);
+      }
+
+      if (!extensions.getAllExtensions().isEmpty()) {
+        info.set(X509CertInfo.EXTENSIONS, extensions);
       }
 
-      // Not strictly necessary, but this makes the certs look like those 
generated
-      // by `keytool`.
-      SubjectKeyIdentifier subjectKeyIdentifier =
-          new SubjectKeyIdentifier(
-              
SubjectPublicKeyInfoFactory.createSubjectPublicKeyInfo(publicKeyAsymKeyParam)
-                  .getEncoded());
-      v3CertGen.addExtension(Extension.subjectKeyIdentifier, false, 
subjectKeyIdentifier);
-
-      X509CertificateHolder certificateHolder = v3CertGen.build(sigGen);
-      return new JcaX509CertificateConverter()
-          .setProvider(new BouncyCastleProvider())
-          .getCertificate(certificateHolder);
-    } catch (Exception e) {
-      throw new RuntimeException("Unable to create certificate", e);
+      // Sign the cert to identify the algorithm that's used.
+      X509CertImpl cert = new X509CertImpl(info);
+      cert.sign(privateKey, algorithm);
+
+      // Update the algorithm, and resign.
+      algo = (AlgorithmId) cert.get(X509CertImpl.SIG_ALG);
+      info.set(CertificateAlgorithmId.NAME + "." + 
CertificateAlgorithmId.ALGORITHM, algo);
+      cert = new X509CertImpl(info);
+      cert.sign(privateKey, algorithm);
+
+      return cert;
+    } catch (Exception ex) {
+      throw new RuntimeException("Unable to create certificate", ex);

Review comment:
       I'm counting on our test suite to catch any regressions in this method.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to