You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2020/10/06 12:31:28 UTC

[GitHub] [kafka] omkreddy commented on a change in pull request #9345: KAFKA-10338; Support PEM format for SSL key and trust stores (KIP-651)

omkreddy commented on a change in pull request #9345:
URL: https://github.com/apache/kafka/pull/9345#discussion_r500186433



##########
File path: clients/src/test/java/org/apache/kafka/test/TestSslUtils.java
##########
@@ -199,6 +206,156 @@ public static void createKeyStore(String filename,
         return builder.build();
     }
 
+    public static void convertToPem(Map<String, Object> sslProps, boolean writeToFile, boolean encryptPrivateKey) throws Exception {
+        String tsPath = (String) sslProps.get(SslConfigs.SSL_TRUSTSTORE_LOCATION_CONFIG);
+        String tsType = (String) sslProps.get(SslConfigs.SSL_TRUSTSTORE_TYPE_CONFIG);
+        Password tsPassword = (Password) sslProps.remove(SslConfigs.SSL_TRUSTSTORE_PASSWORD_CONFIG);
+        Password trustCerts = (Password) sslProps.remove(SslConfigs.SSL_TRUSTSTORE_CERTIFICATES_CONFIG);
+        if (trustCerts == null && tsPath != null) {
+            trustCerts = exportCertificates(tsPath, tsPassword, tsType);
+        }
+        if (trustCerts != null) {
+            if (tsPath == null) {
+                tsPath = File.createTempFile("truststore", ".pem").getPath();
+                sslProps.put(SslConfigs.SSL_TRUSTSTORE_LOCATION_CONFIG, tsPath);
+            }
+            sslProps.put(SslConfigs.SSL_TRUSTSTORE_TYPE_CONFIG, PEM_TYPE);
+            if (writeToFile)
+                writeToFile(tsPath, trustCerts);
+            else {
+                sslProps.put(SslConfigs.SSL_TRUSTSTORE_CERTIFICATES_CONFIG, trustCerts);
+                sslProps.remove(SslConfigs.SSL_TRUSTSTORE_LOCATION_CONFIG);
+            }
+        }
+
+        String ksPath = (String) sslProps.get(SslConfigs.SSL_KEYSTORE_LOCATION_CONFIG);
+        Password certChain = (Password) sslProps.remove(SslConfigs.SSL_KEYSTORE_CERTIFICATE_CHAIN_CONFIG);
+        Password key = (Password) sslProps.remove(SslConfigs.SSL_KEYSTORE_KEY_CONFIG);
+        if (certChain == null && ksPath != null) {
+            String ksType = (String) sslProps.get(SslConfigs.SSL_KEYSTORE_TYPE_CONFIG);
+            Password ksPassword = (Password) sslProps.remove(SslConfigs.SSL_KEYSTORE_PASSWORD_CONFIG);
+            Password keyPassword = (Password) sslProps.get(SslConfigs.SSL_KEY_PASSWORD_CONFIG);
+            certChain = exportCertificates(ksPath, ksPassword, ksType);
+            Password pemKeyPassword = encryptPrivateKey ? keyPassword : null;
+            key = exportPrivateKey(ksPath, ksPassword, keyPassword, ksType, pemKeyPassword);
+            if (!encryptPrivateKey)
+                sslProps.remove(SslConfigs.SSL_KEY_PASSWORD_CONFIG);
+        } else if (!encryptPrivateKey) {

Review comment:
       Can we remove empty else block?

##########
File path: clients/src/test/java/org/apache/kafka/common/network/CertStores.java
##########
@@ -54,13 +60,30 @@ public CertStores(boolean server, String commonName, InetAddress hostAddress) th
     }
 
     private CertStores(boolean server, String commonName, TestSslUtils.CertificateBuilder certBuilder) throws Exception {
+        this(server, commonName, "RSA", certBuilder, false);
+    }
+
+    private CertStores(boolean server, String commonName, String keyAlgorithm, TestSslUtils.CertificateBuilder certBuilder, boolean usePem) throws Exception {
         String name = server ? "server" : "client";
         Mode mode = server ? Mode.SERVER : Mode.CLIENT;
-        File truststoreFile = File.createTempFile(name + "TS", ".jks");
-        sslConfig = TestSslUtils.createSslConfig(!server, true, mode, truststoreFile, name, commonName, certBuilder);
+        File truststoreFile = usePem ? null : File.createTempFile(name + "TS", ".jks");
+        sslConfig = new SslConfigsBuilder(mode)
+                .useClientCert(!server)
+                .certAlias(name)
+                .cn(commonName)
+                .createNewTrustStore(truststoreFile)
+                .certBuilder(certBuilder)
+                .algorithm(keyAlgorithm)
+                .usePem(usePem)
+                .build();
     }
 
+
     public Map<String, Object> getTrustingConfig(CertStores truststoreConfig) {
+        return getTrustingConfig(truststoreConfig, false);
+    }
+
+    public Map<String, Object> getTrustingConfig(CertStores truststoreConfig, boolean usePemCerts) {

Review comment:
       unused usePemCerts?

##########
File path: core/src/main/scala/kafka/server/KafkaConfig.scala
##########
@@ -923,9 +926,12 @@ object KafkaConfig {
   val SslKeystoreLocationDoc = SslConfigs.SSL_KEYSTORE_LOCATION_DOC
   val SslKeystorePasswordDoc = SslConfigs.SSL_KEYSTORE_PASSWORD_DOC
   val SslKeyPasswordDoc = SslConfigs.SSL_KEY_PASSWORD_DOC
+  val SslKeystoreKeyDoc = SslConfigs.SSL_KEYSTORE_KEY_DOC
+  val SslKeystoreCertificateChainDoc = SslConfigs.SSL_KEYSTORE_CERTIFICATE_CHAIN_CONFIG

Review comment:
       SslConfigs.SSL_TRUSTSTORE_CERTIFICATES_CONFIG => SslConfigs.SSL_TRUSTSTORE_CERTIFICATES_DOC

##########
File path: core/src/main/scala/kafka/server/KafkaConfig.scala
##########
@@ -923,9 +926,12 @@ object KafkaConfig {
   val SslKeystoreLocationDoc = SslConfigs.SSL_KEYSTORE_LOCATION_DOC
   val SslKeystorePasswordDoc = SslConfigs.SSL_KEYSTORE_PASSWORD_DOC
   val SslKeyPasswordDoc = SslConfigs.SSL_KEY_PASSWORD_DOC
+  val SslKeystoreKeyDoc = SslConfigs.SSL_KEYSTORE_KEY_DOC
+  val SslKeystoreCertificateChainDoc = SslConfigs.SSL_KEYSTORE_CERTIFICATE_CHAIN_CONFIG
   val SslTruststoreTypeDoc = SslConfigs.SSL_TRUSTSTORE_TYPE_DOC
   val SslTruststorePasswordDoc = SslConfigs.SSL_TRUSTSTORE_PASSWORD_DOC
   val SslTruststoreLocationDoc = SslConfigs.SSL_TRUSTSTORE_LOCATION_DOC
+  val SslTruststoreCertificatesDoc = SslConfigs.SSL_TRUSTSTORE_CERTIFICATES_CONFIG

Review comment:
       SslConfigs.SSL_TRUSTSTORE_CERTIFICATES_CONFIG => SslConfigs.SSL_TRUSTSTORE_CERTIFICATES_DOC

##########
File path: clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java
##########
@@ -167,17 +181,12 @@ public void testValidEndpointIdentificationSanIp() throws Exception {
     @Test
     public void testValidEndpointIdentificationCN() throws Exception {
         String node = "0";

Review comment:
       unused `node` variable here and other methods.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org