You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by GitBox <gi...@apache.org> on 2021/08/29 10:02:19 UTC
[GitHub] [james-project] chibenwa opened a new pull request #626: JAMES-3638 Allow use PEM keys for SSL
chibenwa opened a new pull request #626:
URL: https://github.com/apache/james-project/pull/626
In this tread we discuss enhancements to the IMAP/POP3/SMTP cryptography: https://www.mail-archive.com/server-dev@james.apache.org/msg70772.html
The need of supporting non-keystore PEM files makes James SSL configuration
more in line with the non-java world.
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] fiery06 commented on pull request #626: JAMES-3639 Allow use PEM keys for SSL
Posted by GitBox <gi...@apache.org>.
fiery06 commented on pull request #626:
URL: https://github.com/apache/james-project/pull/626#issuecomment-919006587
Tested with LE certs in Kubernetes.
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] Hakky54 commented on a change in pull request #626: JAMES-3639 Allow use PEM keys for SSL
Posted by GitBox <gi...@apache.org>.
Hakky54 commented on a change in pull request #626:
URL: https://github.com/apache/james-project/pull/626#discussion_r698311756
##########
File path: server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/crypto/SecurityKeyLoader.java
##########
@@ -54,15 +55,25 @@
public AsymmetricKeys load() throws Exception {
Preconditions.checkState(jmapDraftConfiguration.isEnabled(), "JMAP is not enabled");
- KeyStore keystore = KeyStore.getInstance(JKS);
+ if (jmapDraftConfiguration.getKeystore().isPresent()) {
+ return loadFromKeystore();
+ }
+ return loadFromPEM();
+ }
+
+ private AsymmetricKeys loadFromKeystore() throws Exception {
+ Preconditions.checkState(jmapDraftConfiguration.getKeystore().isPresent());
+ Preconditions.checkState(jmapDraftConfiguration.getSecret().isPresent());
+
+ KeyStore keystore = KeyStore.getInstance(jmapDraftConfiguration.getKeystoreType());
char[] secret;
- try (InputStream fis = fileSystem.getResource(jmapDraftConfiguration.getKeystore())) {
- secret = jmapDraftConfiguration.getSecret().toCharArray();
+ try (InputStream fis = fileSystem.getResource(jmapDraftConfiguration.getKeystore().get())) {
+ secret = jmapDraftConfiguration.getSecret().get().toCharArray();
keystore.load(fis, secret);
Review comment:
By the way, I am not sure if it is worth but you might be able to even refactor line number 68 till 72 into a single line with the KeyStoreUtils:
```
KeyStore keyStore = KeyStoreUtils.loadKeyStore(fileSystem.getResource(jmapDraftConfiguration.getKeystore()), jmapDraftConfiguration.getSecret().get().toCharArray())
```
It will also take care of opening/closing the resource.
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #626: JAMES-3639 Allow use PEM keys for SSL
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #626:
URL: https://github.com/apache/james-project/pull/626#issuecomment-907985467
It's a POC :-P
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa merged pull request #626: JAMES-3639 Allow use PEM keys for SSL
Posted by GitBox <gi...@apache.org>.
chibenwa merged pull request #626:
URL: https://github.com/apache/james-project/pull/626
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] Hakky54 commented on a change in pull request #626: JAMES-3639 Allow use PEM keys for SSL
Posted by GitBox <gi...@apache.org>.
Hakky54 commented on a change in pull request #626:
URL: https://github.com/apache/james-project/pull/626#discussion_r698409961
##########
File path: server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/crypto/SecurityKeyLoader.java
##########
@@ -54,15 +55,25 @@
public AsymmetricKeys load() throws Exception {
Preconditions.checkState(jmapDraftConfiguration.isEnabled(), "JMAP is not enabled");
- KeyStore keystore = KeyStore.getInstance(JKS);
+ if (jmapDraftConfiguration.getKeystore().isPresent()) {
+ return loadFromKeystore();
+ }
+ return loadFromPEM();
+ }
+
+ private AsymmetricKeys loadFromKeystore() throws Exception {
+ Preconditions.checkState(jmapDraftConfiguration.getKeystore().isPresent());
+ Preconditions.checkState(jmapDraftConfiguration.getSecret().isPresent());
+
+ KeyStore keystore = KeyStore.getInstance(jmapDraftConfiguration.getKeystoreType());
char[] secret;
- try (InputStream fis = fileSystem.getResource(jmapDraftConfiguration.getKeystore())) {
- secret = jmapDraftConfiguration.getSecret().toCharArray();
+ try (InputStream fis = fileSystem.getResource(jmapDraftConfiguration.getKeystore().get())) {
+ secret = jmapDraftConfiguration.getSecret().get().toCharArray();
keystore.load(fis, secret);
Review comment:
Yes, that was the reason why I wasn't sure to propose it and if it is worth it. Up to you guys to decide, I just wanted to suggest an alternative.
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a change in pull request #626: JAMES-3639 Allow use PEM keys for SSL
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #626:
URL: https://github.com/apache/james-project/pull/626#discussion_r698383171
##########
File path: server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/crypto/SecurityKeyLoader.java
##########
@@ -54,15 +55,25 @@
public AsymmetricKeys load() throws Exception {
Preconditions.checkState(jmapDraftConfiguration.isEnabled(), "JMAP is not enabled");
- KeyStore keystore = KeyStore.getInstance(JKS);
+ if (jmapDraftConfiguration.getKeystore().isPresent()) {
+ return loadFromKeystore();
+ }
+ return loadFromPEM();
+ }
+
+ private AsymmetricKeys loadFromKeystore() throws Exception {
+ Preconditions.checkState(jmapDraftConfiguration.getKeystore().isPresent());
+ Preconditions.checkState(jmapDraftConfiguration.getSecret().isPresent());
+
+ KeyStore keystore = KeyStore.getInstance(jmapDraftConfiguration.getKeystoreType());
char[] secret;
- try (InputStream fis = fileSystem.getResource(jmapDraftConfiguration.getKeystore())) {
- secret = jmapDraftConfiguration.getSecret().toCharArray();
+ try (InputStream fis = fileSystem.getResource(jmapDraftConfiguration.getKeystore().get())) {
+ secret = jmapDraftConfiguration.getSecret().get().toCharArray();
keystore.load(fis, secret);
Review comment:
Mmm propagating a dependency to gain one line...
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #626: JAMES-3639 Allow use PEM keys for SSL
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #626:
URL: https://github.com/apache/james-project/pull/626#issuecomment-915727739
Objections towards merging this?
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #626: JAMES-3639 Allow use PEM keys for SSL
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #626:
URL: https://github.com/apache/james-project/pull/626#issuecomment-908005563
I added those tests.
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] rouazana commented on a change in pull request #626: JAMES-3639 Allow use PEM keys for SSL
Posted by GitBox <gi...@apache.org>.
rouazana commented on a change in pull request #626:
URL: https://github.com/apache/james-project/pull/626#discussion_r700276431
##########
File path: docs/modules/servers/pages/distributed/configure/ssl.adoc
##########
@@ -21,13 +21,33 @@ for TLS 1.0 which, as a result, is sometimes referred to as SSL 3.1.
You need to add a block in the corresponding configuration file (smtpserver.xml, pop3server.xml, imapserver.xml,..)
....
-<tls socketTLS="false" startTLS="false">
+<tls socketTLS="false" startTLS="true">
<keystore>file://conf/keystore</keystore>
+ <keystoreType>PKCS12</keystoreType>
<secret>yoursecret</secret>
<provider>org.bouncycastle.jce.provider.BouncyCastleProvider</provider>
</tls>
....
+Alternatively TLS keys can be supplied via PEM files:
+
+....
+<tls socketTLS="true" startTLS="false">
+ <privateKey>file://conf/private.key</privateKey>
+ <certificates>file://conf/certs.self-signed.csr</certificates>
Review comment:
Often certificates can be provided as a single file and/or as a directory. Would it be possible also to add the complementary directory option?
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] Hakky54 commented on a change in pull request #626: JAMES-3639 Allow use PEM keys for SSL
Posted by GitBox <gi...@apache.org>.
Hakky54 commented on a change in pull request #626:
URL: https://github.com/apache/james-project/pull/626#discussion_r698409961
##########
File path: server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/crypto/SecurityKeyLoader.java
##########
@@ -54,15 +55,25 @@
public AsymmetricKeys load() throws Exception {
Preconditions.checkState(jmapDraftConfiguration.isEnabled(), "JMAP is not enabled");
- KeyStore keystore = KeyStore.getInstance(JKS);
+ if (jmapDraftConfiguration.getKeystore().isPresent()) {
+ return loadFromKeystore();
+ }
+ return loadFromPEM();
+ }
+
+ private AsymmetricKeys loadFromKeystore() throws Exception {
+ Preconditions.checkState(jmapDraftConfiguration.getKeystore().isPresent());
+ Preconditions.checkState(jmapDraftConfiguration.getSecret().isPresent());
+
+ KeyStore keystore = KeyStore.getInstance(jmapDraftConfiguration.getKeystoreType());
char[] secret;
- try (InputStream fis = fileSystem.getResource(jmapDraftConfiguration.getKeystore())) {
- secret = jmapDraftConfiguration.getSecret().toCharArray();
+ try (InputStream fis = fileSystem.getResource(jmapDraftConfiguration.getKeystore().get())) {
+ secret = jmapDraftConfiguration.getSecret().get().toCharArray();
keystore.load(fis, secret);
Review comment:
Yes, that was the reason why I wasn't sure to propose it and if it is worth it
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a change in pull request #626: JAMES-3639 Allow use PEM keys for SSL
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #626:
URL: https://github.com/apache/james-project/pull/626#discussion_r700719111
##########
File path: docs/modules/servers/pages/distributed/configure/ssl.adoc
##########
@@ -21,13 +21,33 @@ for TLS 1.0 which, as a result, is sometimes referred to as SSL 3.1.
You need to add a block in the corresponding configuration file (smtpserver.xml, pop3server.xml, imapserver.xml,..)
....
-<tls socketTLS="false" startTLS="false">
+<tls socketTLS="false" startTLS="true">
<keystore>file://conf/keystore</keystore>
+ <keystoreType>PKCS12</keystoreType>
<secret>yoursecret</secret>
<provider>org.bouncycastle.jce.provider.BouncyCastleProvider</provider>
</tls>
....
+Alternatively TLS keys can be supplied via PEM files:
+
+....
+<tls socketTLS="true" startTLS="false">
+ <privateKey>file://conf/private.key</privateKey>
+ <certificates>file://conf/certs.self-signed.csr</certificates>
Review comment:
IMO could be. Changes would be welcome, feel free to propose an exact format and to open a PR to complete this one .
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a change in pull request #626: JAMES-3639 Allow use PEM keys for SSL
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #626:
URL: https://github.com/apache/james-project/pull/626#discussion_r698417937
##########
File path: server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/crypto/SecurityKeyLoader.java
##########
@@ -54,15 +55,25 @@
public AsymmetricKeys load() throws Exception {
Preconditions.checkState(jmapDraftConfiguration.isEnabled(), "JMAP is not enabled");
- KeyStore keystore = KeyStore.getInstance(JKS);
+ if (jmapDraftConfiguration.getKeystore().isPresent()) {
+ return loadFromKeystore();
+ }
+ return loadFromPEM();
+ }
+
+ private AsymmetricKeys loadFromKeystore() throws Exception {
+ Preconditions.checkState(jmapDraftConfiguration.getKeystore().isPresent());
+ Preconditions.checkState(jmapDraftConfiguration.getSecret().isPresent());
+
+ KeyStore keystore = KeyStore.getInstance(jmapDraftConfiguration.getKeystoreType());
char[] secret;
- try (InputStream fis = fileSystem.getResource(jmapDraftConfiguration.getKeystore())) {
- secret = jmapDraftConfiguration.getSecret().toCharArray();
+ try (InputStream fis = fileSystem.getResource(jmapDraftConfiguration.getKeystore().get())) {
+ secret = jmapDraftConfiguration.getSecret().get().toCharArray();
keystore.load(fis, secret);
Review comment:
The dependency was already there so I applied your suggestion.
Thanks!
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #626: JAMES-3639 Allow use PEM keys for SSL
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #626:
URL: https://github.com/apache/james-project/pull/626#issuecomment-914900356
Rebased to fix conflicts and squashed
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] Hakky54 commented on a change in pull request #626: JAMES-3639 Allow use PEM keys for SSL
Posted by GitBox <gi...@apache.org>.
Hakky54 commented on a change in pull request #626:
URL: https://github.com/apache/james-project/pull/626#discussion_r698006088
##########
File path: server/protocols/protocols-library/src/main/java/org/apache/james/protocols/lib/netty/AbstractConfigurableAsyncServer.java
##########
@@ -390,18 +400,33 @@ protected Encryption getEncryption() {
protected void buildSSLContext() throws Exception {
if (useStartTLS || useSSL) {
FileInputStream fis = null;
+ // Initialize the SSLContext to work with our key managers.
+ SSLContext context = SSLContext.getInstance("TLS");
Review comment:
You might be able to simplify the code logic here by delegating everything to the SSLFactory, see here for the code snippet:
```
SSLFactory.Builder sslFactoryBuilder = SSLFactory.builder()
.withSslContextAlgorithm("TLS");
if (keyStore != null) {
sslFactoryBuilder.withIdentityMaterial(
fileSystem.getFile(keystore).toPath(),
secret.toCharArray(),
Optional.of(secret)
.orElse("")
.toCharArray());
} else {
X509ExtendedKeyManager keyManager = PemUtils.loadIdentityMaterial(
fileSystem.getResource(certificates),
fileSystem.getResource(privateKey),
Optional.ofNullable(secret)
.map(String::toCharArray)
.orElse(null));
sslFactoryBuilder.withIdentityMaterial(keyManager);
}
SSLContext context = sslFactoryBuilder.build()
.getSslContext();
if (useStartTLS) {
encryption = Encryption.createStartTls(context, enabledCipherSuites);
} else {
encryption = Encryption.createTls(context, enabledCipherSuites);
}
```
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] Hakky54 commented on a change in pull request #626: JAMES-3639 Allow use PEM keys for SSL
Posted by GitBox <gi...@apache.org>.
Hakky54 commented on a change in pull request #626:
URL: https://github.com/apache/james-project/pull/626#discussion_r698409961
##########
File path: server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/crypto/SecurityKeyLoader.java
##########
@@ -54,15 +55,25 @@
public AsymmetricKeys load() throws Exception {
Preconditions.checkState(jmapDraftConfiguration.isEnabled(), "JMAP is not enabled");
- KeyStore keystore = KeyStore.getInstance(JKS);
+ if (jmapDraftConfiguration.getKeystore().isPresent()) {
+ return loadFromKeystore();
+ }
+ return loadFromPEM();
+ }
+
+ private AsymmetricKeys loadFromKeystore() throws Exception {
+ Preconditions.checkState(jmapDraftConfiguration.getKeystore().isPresent());
+ Preconditions.checkState(jmapDraftConfiguration.getSecret().isPresent());
+
+ KeyStore keystore = KeyStore.getInstance(jmapDraftConfiguration.getKeystoreType());
char[] secret;
- try (InputStream fis = fileSystem.getResource(jmapDraftConfiguration.getKeystore())) {
- secret = jmapDraftConfiguration.getSecret().toCharArray();
+ try (InputStream fis = fileSystem.getResource(jmapDraftConfiguration.getKeystore().get())) {
+ secret = jmapDraftConfiguration.getSecret().get().toCharArray();
keystore.load(fis, secret);
Review comment:
Yes, that was the reason why I wasn't sure to propose it
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org