-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding TLS support on the deployed Kafka cluster #487
Merged
Merged
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
59d1729
Added first draft of common component for handling certificates and r…
ppatierno e9c2d6a
Added support for subject alternative names
ppatierno 1ba07aa
Added cluster secrets for getting Kafka cluster description
ppatierno b80ab0b
Added generation of the global CA certificate for internal communication
ppatierno 0fbac35
Applied changes to KafkaAssemblyOperator to avoid state
ppatierno 9d34feb
Ignored defaultMode on secret volumes for diff
ppatierno 04cece7
Added secrets deletion
ppatierno 4a8c42b
Added Kafka brokers configuration with TLS/SSL support
ppatierno f17a2ba
Factored out class for decoding certs from Secrets
ppatierno 4835927
Refactored way to generate certificates for handling edge cases better
ppatierno 0d4a2e7
Added a new CLIENT_ENC port 9093 for encrypted traffic (from clients)
ppatierno f18fc66
Added SecretOperator unit test
ppatierno d0ae530
Some renaming on TLS ports, password and expiration date
ppatierno 7288fa7
Added documentation about TLS support
ppatierno 97236fe
Some fixes and refactoring around certs generation and openssl log
ppatierno File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<FindBugsFilter> | ||
<Match> | ||
<Bug pattern="EI_EXPOSE_REP,EI_EXPOSE_REP2"/> | ||
<Package name="io.strimzi.certs"/> | ||
</Match> | ||
</FindBugsFilter> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<project xmlns="http://maven.apache.org/POM/4.0.0" | ||
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd"> | ||
<parent> | ||
<artifactId>strimzi</artifactId> | ||
<groupId>io.strimzi</groupId> | ||
<version>0.5.0-SNAPSHOT</version> | ||
</parent> | ||
<modelVersion>4.0.0</modelVersion> | ||
<artifactId>certificate-manager</artifactId> | ||
<dependencies> | ||
<dependency> | ||
<groupId>io.fabric8</groupId> | ||
<artifactId>kubernetes-model</artifactId> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.apache.logging.log4j</groupId> | ||
<artifactId>log4j-api</artifactId> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.apache.logging.log4j</groupId> | ||
<artifactId>log4j-core</artifactId> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.apache.logging.log4j</groupId> | ||
<artifactId>log4j-slf4j-impl</artifactId> | ||
</dependency> | ||
<dependency> | ||
<groupId>junit</groupId> | ||
<artifactId>junit</artifactId> | ||
</dependency> | ||
</dependencies> | ||
|
||
</project> |
24 changes: 24 additions & 0 deletions
24
certificate-manager/src/main/java/io/strimzi/certs/CertAndKey.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
/* | ||
* Copyright 2017-2018, Strimzi authors. | ||
* License: Apache License 2.0 (see the file LICENSE or http://apache.org/licenses/LICENSE-2.0.html). | ||
*/ | ||
package io.strimzi.certs; | ||
|
||
public class CertAndKey { | ||
|
||
private final byte[] key; | ||
private final byte[] cert; | ||
|
||
public CertAndKey(byte[] key, byte[] cert) { | ||
this.key = key; | ||
this.cert = cert; | ||
} | ||
|
||
public byte[] key() { | ||
return key; | ||
} | ||
|
||
public byte[] cert() { | ||
return cert; | ||
} | ||
} |
65 changes: 65 additions & 0 deletions
65
certificate-manager/src/main/java/io/strimzi/certs/CertManager.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
/* | ||
* Copyright 2017-2018, Strimzi authors. | ||
* License: Apache License 2.0 (see the file LICENSE or http://apache.org/licenses/LICENSE-2.0.html). | ||
*/ | ||
package io.strimzi.certs; | ||
|
||
import java.io.File; | ||
import java.io.IOException; | ||
|
||
public interface CertManager { | ||
|
||
/** | ||
* Generate a self-signed certificate | ||
* | ||
* @param keyFile path to the file which will contain the private key | ||
* @param certFile path to the file which will contain the self signed certificate | ||
* @param sbj subject information | ||
* @param days certificate duration | ||
* @throws IOException | ||
*/ | ||
void generateSelfSignedCert(File keyFile, File certFile, Subject sbj, int days) throws IOException; | ||
|
||
/** | ||
* Generate a self-signed certificate | ||
* | ||
* @param keyFile path to the file which will contain the private key | ||
* @param certFile path to the file which will contain the self signed certificate | ||
* @param days certificate duration | ||
* @throws IOException | ||
*/ | ||
void generateSelfSignedCert(File keyFile, File certFile, int days) throws IOException; | ||
|
||
/** | ||
* Generate a certificate sign request | ||
* | ||
* @param keyFile path to the file which will contain the private key | ||
* @param csrFile path to the file which will contain the certificate sign request | ||
* @param sbj subject information | ||
*/ | ||
void generateCsr(File keyFile, File csrFile, Subject sbj) throws IOException; | ||
|
||
/** | ||
* Generate a certificate signed by a Certificate Authority | ||
* | ||
* @param csrFile path to the file containing the certificate sign request | ||
* @param caKey path to the file containing the CA private key | ||
* @param caCert path to the file containing the CA certificate | ||
* @param crtFile path to the file which will contain the signed certificate | ||
* @param days certificate duration | ||
* @throws IOException | ||
*/ | ||
void generateCert(File csrFile, File caKey, File caCert, File crtFile, int days) throws IOException; | ||
|
||
/** | ||
* Generate a certificate signed by a Certificate Authority | ||
* | ||
* @param csrFile path to the file containing the certificate sign request | ||
* @param caKey CA private key bytes | ||
* @param caCert CA certificate bytes | ||
* @param crtFile path to the file which will contain the signed certificate | ||
* @param days certificate duration | ||
* @throws IOException | ||
*/ | ||
void generateCert(File csrFile, byte[] caKey, byte[] caCert, File crtFile, int days) throws IOException; | ||
} |
210 changes: 210 additions & 0 deletions
210
certificate-manager/src/main/java/io/strimzi/certs/OpenSslCertManager.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,210 @@ | ||
/* | ||
* Copyright 2017-2018, Strimzi authors. | ||
* License: Apache License 2.0 (see the file LICENSE or http://apache.org/licenses/LICENSE-2.0.html). | ||
*/ | ||
package io.strimzi.certs; | ||
|
||
import org.apache.logging.log4j.LogManager; | ||
import org.apache.logging.log4j.Logger; | ||
|
||
import java.io.BufferedWriter; | ||
import java.io.File; | ||
import java.io.FileOutputStream; | ||
import java.io.IOException; | ||
import java.io.InputStream; | ||
import java.io.OutputStream; | ||
import java.io.OutputStreamWriter; | ||
import java.nio.charset.Charset; | ||
import java.nio.file.Files; | ||
import java.nio.file.StandardCopyOption; | ||
import java.util.ArrayList; | ||
import java.util.List; | ||
import java.util.Map; | ||
|
||
import static java.util.Arrays.asList; | ||
|
||
/** | ||
* An OpenSSL based certificates manager | ||
*/ | ||
public class OpenSslCertManager implements CertManager { | ||
|
||
private static final Logger log = LogManager.getLogger(OpenSslCertManager.class); | ||
|
||
@Override | ||
public void generateSelfSignedCert(File keyFile, File certFile, int days) throws IOException { | ||
generateSelfSignedCert(keyFile, certFile, null, days); | ||
} | ||
|
||
@Override | ||
public void generateSelfSignedCert(File keyFile, File certFile, Subject sbj, int days) throws IOException { | ||
|
||
List<String> cmd = new ArrayList<>(asList("openssl", "req", "-x509", "-new", "-days", String.valueOf(days), "-batch", "-nodes", | ||
"-out", certFile.getAbsolutePath(), "-keyout", keyFile.getAbsolutePath())); | ||
|
||
File sna = null; | ||
File openSslConf = null; | ||
if (sbj != null) { | ||
|
||
if (sbj.subjectAltNames() != null && sbj.subjectAltNames().size() > 0) { | ||
|
||
// subject alt names need to be in an openssl configuration file | ||
InputStream is = getClass().getClassLoader().getResourceAsStream("openssl.conf"); | ||
openSslConf = File.createTempFile("openssl", "conf"); | ||
Files.copy(is, openSslConf.toPath(), StandardCopyOption.REPLACE_EXISTING); | ||
|
||
sna = addSubjectAltNames(openSslConf, sbj); | ||
cmd.addAll(asList("-config", sna.toPath().toString(), "-extensions", "v3_req")); | ||
} | ||
|
||
cmd.addAll(asList("-subj", sbj.toString())); | ||
} | ||
|
||
exec(cmd); | ||
|
||
if (sna != null) { | ||
if (!sna.delete()) { | ||
log.warn("{} cannot be deleted", sna.getName()); | ||
} | ||
} | ||
if (openSslConf != null) { | ||
if (!openSslConf.delete()) { | ||
log.warn("{} cannot be deleted", openSslConf.getName()); | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* Add subject alt names section to the provided openssl configuration file | ||
* | ||
* @param opensslConf openssl configuration file | ||
* @param sbj subject information | ||
* @return openssl configuration file with subject alt names added | ||
* @throws IOException | ||
*/ | ||
private File addSubjectAltNames(File opensslConf, Subject sbj) throws IOException { | ||
|
||
File sna = File.createTempFile("sna", "sna"); | ||
Files.copy(opensslConf.toPath(), sna.toPath(), StandardCopyOption.REPLACE_EXISTING); | ||
|
||
BufferedWriter out = null; | ||
try { | ||
out = new BufferedWriter(new OutputStreamWriter(new FileOutputStream(sna, true), "UTF8")); | ||
boolean newline = false; | ||
for (Map.Entry<String, String> entry : sbj.subjectAltNames().entrySet()) { | ||
if (newline) { | ||
out.append("\n"); | ||
} | ||
out.append(entry.getKey()).append(" = ").append(entry.getValue()); | ||
newline = true; | ||
} | ||
} finally { | ||
if (out != null) { | ||
out.close(); | ||
} | ||
} | ||
|
||
return sna; | ||
} | ||
|
||
@Override | ||
public void generateCsr(File keyFile, File csrFile, Subject sbj) throws IOException { | ||
|
||
List<String> cmd = new ArrayList<>(asList("openssl", "req", "-new", "-batch", "-nodes", | ||
"-keyout", keyFile.getAbsolutePath(), "-out", csrFile.getAbsolutePath())); | ||
|
||
File sna = null; | ||
File openSslConf = null; | ||
if (sbj != null) { | ||
|
||
if (sbj.subjectAltNames() != null && sbj.subjectAltNames().size() > 0) { | ||
|
||
// subject alt names need to be in an openssl configuration file | ||
InputStream is = getClass().getClassLoader().getResourceAsStream("openssl.conf"); | ||
openSslConf = File.createTempFile("openssl", "conf"); | ||
Files.copy(is, openSslConf.toPath(), StandardCopyOption.REPLACE_EXISTING); | ||
|
||
sna = addSubjectAltNames(openSslConf, sbj); | ||
cmd.addAll(asList("-config", sna.toPath().toString(), "-extensions", "v3_req")); | ||
} | ||
|
||
cmd.addAll(asList("-subj", sbj.toString())); | ||
} | ||
|
||
exec(cmd); | ||
|
||
if (sna != null) { | ||
if (!sna.delete()) { | ||
log.warn("{} cannot be deleted", sna.getName()); | ||
} | ||
} | ||
if (openSslConf != null) { | ||
if (!openSslConf.delete()) { | ||
log.warn("{} cannot be deleted", openSslConf.getName()); | ||
} | ||
} | ||
} | ||
|
||
@Override | ||
public void generateCert(File csrFile, File caKey, File caCert, File crtFile, int days) throws IOException { | ||
exec("openssl", "x509", "-req", "-days", String.valueOf(days), "-in", csrFile.getAbsolutePath(), | ||
"-CA", caCert.getAbsolutePath(), "-CAkey", caKey.getAbsolutePath(), "-CAcreateserial", | ||
"-out", crtFile.getAbsolutePath()); | ||
} | ||
|
||
@Override | ||
public void generateCert(File csrFile, byte[] caKey, byte[] caCert, File crtFile, int days) throws IOException { | ||
|
||
File caKeyFile = File.createTempFile("tls", "ca"); | ||
Files.write(caKeyFile.toPath(), caKey); | ||
|
||
File caCertFile = File.createTempFile("tls", "ca"); | ||
Files.write(caCertFile.toPath(), caCert); | ||
|
||
generateCert(csrFile, caKeyFile, caCertFile, crtFile, days); | ||
|
||
if (!caKeyFile.delete()) { | ||
log.warn("{} cannot be deleted", caKeyFile.getName()); | ||
} | ||
if (!caCertFile.delete()) { | ||
log.warn("{} cannot be deleted", caCertFile.getName()); | ||
} | ||
} | ||
|
||
private void exec(String... cmd) throws IOException { | ||
exec(asList(cmd)); | ||
} | ||
|
||
private void exec(List<String> cmd) throws IOException { | ||
File out = null; | ||
|
||
try { | ||
|
||
out = File.createTempFile("openssl", Integer.toString(cmd.hashCode())); | ||
|
||
ProcessBuilder processBuilder = new ProcessBuilder(cmd) | ||
.redirectOutput(out) | ||
.redirectErrorStream(true); | ||
log.info("Running command {}", processBuilder.command()); | ||
|
||
Process proc = processBuilder.start(); | ||
|
||
OutputStream outputStream = proc.getOutputStream(); | ||
// close subprocess' stdin | ||
outputStream.close(); | ||
|
||
int result = proc.waitFor(); | ||
String stdout = new String(Files.readAllBytes(out.toPath()), Charset.defaultCharset()); | ||
|
||
log.debug(stdout); | ||
log.info("result {}", result); | ||
|
||
} catch (InterruptedException ignored) { | ||
} finally { | ||
if (out != null) { | ||
if (!out.delete()) { | ||
log.warn("{} cannot be deleted", out.getName()); | ||
} | ||
} | ||
} | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really a question for this PR but more for the community. Why the hell do we have these rules if every second PR increases them to pass? Should we either follow them or remove them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a good question and maybe we should discuss about it with the entire team. Sometimes I don't see any reason to spend more time to be compliant with the current rules (so I just increase them).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wiling to bet this was increased because of
AbstractModel
. I know that because I've also had to deal withAbstractModel
getting increasingly cumbersome, and having to increase these parameters. So in that sense checkstyle has fulfilled its purpose of highlighting to us that there's a problem withAbstractModel
. I'm hoping we'll be able to refactorAbstractModel
soon and revert these parameters.Really it's incumbent on whoever increases these to open (or ensure there is already open) a PR to address the complexity in a future sprint.