You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2020/02/15 12:22:14 UTC
[tomcat] branch master updated: 64141: Allow overriding JVM trust
store
This is an automated email from the ASF dual-hosted git repository.
remm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push:
new d14c5b4 64141: Allow overriding JVM trust store
d14c5b4 is described below
commit d14c5b409d08df88ab437f76334342b8f8094b44
Author: remm <re...@apache.org>
AuthorDate: Sat Feb 15 13:21:53 2020 +0100
64141: Allow overriding JVM trust store
If using a CA certificate, remove a default value for the trust store
file when not using a JSSE configuration.
---
java/org/apache/tomcat/util/net/SSLHostConfig.java | 26 +++++++++++++++++++---
webapps/docs/changelog.xml | 4 ++++
2 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/SSLHostConfig.java b/java/org/apache/tomcat/util/net/SSLHostConfig.java
index 4a8549d..724497f 100644
--- a/java/org/apache/tomcat/util/net/SSLHostConfig.java
+++ b/java/org/apache/tomcat/util/net/SSLHostConfig.java
@@ -151,15 +151,24 @@ public class SSLHostConfig implements Serializable {
}
- void setProperty(String name, Type configType) {
+ /**
+ * Set property which belongs to the specified configuration type.
+ * @param name the property name
+ * @param configType the configuration type
+ * @return true if the property belongs to the current confuguration,
+ * and false otherwise
+ */
+ boolean setProperty(String name, Type configType) {
if (this.configType == null) {
this.configType = configType;
} else {
if (configType != this.configType) {
log.warn(sm.getString("sslHostConfig.mismatch",
name, getHostName(), configType, this.configType));
+ return false;
}
}
+ return true;
}
@@ -662,7 +671,13 @@ public class SSLHostConfig implements Serializable {
public void setCaCertificateFile(String caCertificateFile) {
- setProperty("caCertificateFile", Type.OPENSSL);
+ if (setProperty("caCertificateFile", Type.OPENSSL)) {
+ // Reset default JSSE trust store if not a JSSE configuration
+ if (truststoreFile != null) {
+ System.out.println("RESET !!!!!!!!!!!!!!!!!");
+ truststoreFile = null;
+ }
+ }
this.caCertificateFile = caCertificateFile;
}
@@ -673,7 +688,12 @@ public class SSLHostConfig implements Serializable {
public void setCaCertificatePath(String caCertificatePath) {
- setProperty("caCertificatePath", Type.OPENSSL);
+ if (setProperty("caCertificatePath", Type.OPENSSL)) {
+ // Reset default JSSE trust store if not a JSSE configuration
+ if (truststoreFile != null) {
+ truststoreFile = null;
+ }
+ }
this.caCertificatePath = caCertificatePath;
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 753fb4c..e0673d1 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -51,6 +51,10 @@
Fix support of native jakarta servlet attributes in AJP connector.
(remm)
</fix>
+ <fix>
+ <bug>64141</bug>: If using a CA certificate, remove a default value
+ for the truststore file when not using a JSSE configuration. (remm)
+ </fix>
</changelog>
</subsection>
</section>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch master updated: 64141: Allow overriding JVM trust store
Posted by Martin Grigorov <mg...@apache.org>.
On Sat, Feb 15, 2020 at 2:22 PM <re...@apache.org> wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> remm pushed a commit to branch master
> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>
>
> The following commit(s) were added to refs/heads/master by this push:
> new d14c5b4 64141: Allow overriding JVM trust store
> d14c5b4 is described below
>
> commit d14c5b409d08df88ab437f76334342b8f8094b44
> Author: remm <re...@apache.org>
> AuthorDate: Sat Feb 15 13:21:53 2020 +0100
>
> 64141: Allow overriding JVM trust store
>
> If using a CA certificate, remove a default value for the trust store
> file when not using a JSSE configuration.
> ---
> java/org/apache/tomcat/util/net/SSLHostConfig.java | 26
> +++++++++++++++++++---
> webapps/docs/changelog.xml | 4 ++++
> 2 files changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/java/org/apache/tomcat/util/net/SSLHostConfig.java
> b/java/org/apache/tomcat/util/net/SSLHostConfig.java
> index 4a8549d..724497f 100644
> --- a/java/org/apache/tomcat/util/net/SSLHostConfig.java
> +++ b/java/org/apache/tomcat/util/net/SSLHostConfig.java
> @@ -151,15 +151,24 @@ public class SSLHostConfig implements Serializable {
> }
>
>
> - void setProperty(String name, Type configType) {
> + /**
> + * Set property which belongs to the specified configuration type.
> + * @param name the property name
> + * @param configType the configuration type
> + * @return true if the property belongs to the current confuguration,
> + * and false otherwise
> + */
> + boolean setProperty(String name, Type configType) {
> if (this.configType == null) {
> this.configType = configType;
> } else {
> if (configType != this.configType) {
> log.warn(sm.getString("sslHostConfig.mismatch",
> name, getHostName(), configType,
> this.configType));
> + return false;
> }
> }
> + return true;
> }
>
>
> @@ -662,7 +671,13 @@ public class SSLHostConfig implements Serializable {
>
>
> public void setCaCertificateFile(String caCertificateFile) {
> - setProperty("caCertificateFile", Type.OPENSSL);
> + if (setProperty("caCertificateFile", Type.OPENSSL)) {
> + // Reset default JSSE trust store if not a JSSE configuration
> + if (truststoreFile != null) {
> + System.out.println("RESET !!!!!!!!!!!!!!!!!");
>
Debug leftover.
> + truststoreFile = null;
> + }
> + }
> this.caCertificateFile = caCertificateFile;
> }
>
> @@ -673,7 +688,12 @@ public class SSLHostConfig implements Serializable {
>
>
> public void setCaCertificatePath(String caCertificatePath) {
> - setProperty("caCertificatePath", Type.OPENSSL);
> + if (setProperty("caCertificatePath", Type.OPENSSL)) {
> + // Reset default JSSE trust store if not a JSSE configuration
> + if (truststoreFile != null) {
> + truststoreFile = null;
> + }
> + }
> this.caCertificatePath = caCertificatePath;
> }
>
> diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
> index 753fb4c..e0673d1 100644
> --- a/webapps/docs/changelog.xml
> +++ b/webapps/docs/changelog.xml
> @@ -51,6 +51,10 @@
> Fix support of native jakarta servlet attributes in AJP connector.
> (remm)
> </fix>
> + <fix>
> + <bug>64141</bug>: If using a CA certificate, remove a default
> value
> + for the truststore file when not using a JSSE configuration.
> (remm)
> + </fix>
> </changelog>
> </subsection>
> </section>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
Re: [tomcat] branch master updated: 64141: Allow overriding JVM trust store
Posted by Rémy Maucherat <re...@apache.org>.
On Sun, Feb 16, 2020 at 3:46 PM Christopher Schultz <
chris@christopherschultz.net> wrote:
> > + truststoreFile = null; + } + }
> > this.caCertificateFile = caCertificateFile; }
>
> Also, is this the best way to do this? Instead of defaulting to
> javax.ssl.trustStore for the JSSE TrustStore, should we not apply the
> default when it's needed?
>
> Like when configuring the SSLContext, check for a null TrustStore and
> inherit the JVM default?
>
> I think that would b a little cleaner than blanking-out the default
> value of *another* configuration when a different (but related)
> configuration option is set.
>
> There is also the possibility that truststoreFile != null, but
> truststoreFile is also not set to the JVM default.
>
> What if we throw an exception if both of the configuration options are
> present? We won't know which one to use, anyway.
>
If the value is not set to the system property, then the configuration type
will be JSSE. Mixing configuration types is handled as it was (a warning is
logged).
So I think this works and is the best way (will not cause changes/problems
...).
Rémy
>
> > @@ -673,7 +688,12 @@ public class SSLHostConfig implements
> > Serializable {
> >
> >
> > public void setCaCertificatePath(String caCertificatePath) { -
> > setProperty("caCertificatePath", Type.OPENSSL); + if
> > (setProperty("caCertificatePath", Type.OPENSSL)) { + //
> > Reset default JSSE trust store if not a JSSE configuration +
> > if (truststoreFile != null) { + truststoreFile =
> > null; + } + } this.caCertificatePath =
> > caCertificatePath; }
> >
> > diff --git a/webapps/docs/changelog.xml
> > b/webapps/docs/changelog.xml index 753fb4c..e0673d1 100644 ---
> > a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@
> > -51,6 +51,10 @@ Fix support of native jakarta servlet attributes in
> > AJP connector. (remm) </fix> + <fix> +
> > <bug>64141</bug>: If using a CA certificate, remove a default
> > value + for the truststore file when not using a JSSE
> > configuration. (remm) + </fix> </changelog> </subsection>
> > </section>
> >
> >
>
> - -chris
> -----BEGIN PGP SIGNATURE-----
> Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/
>
> iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAl5JVccACgkQHPApP6U8
> pFhjKRAAo0Nscss8xqlihW7kzjY5tb6JotdVnMzvVRHo1OsYeS93jaCsqNID6+wY
> Q9dEXP58CJy05W72ftWltvB1npRV+90pSB3tOOFoQ1QTjjTuVBw9vDp8nfR8bSnW
> 0G+3ClxQX0f5bWJLZbe4ws0z7fPyvu0XS267NpsMIhHCuWkw7CQE0re3f319FPOV
> 6fHZj82enI9YuRbuLTNCeuv87XSiY5mi3usyZl+lUH0oOqVrQFsk1qRTz/T+5ZQw
> vHmJ5Ei1/4JOOJiHd5HeRKtNh3uUR3wSWmdlDKP44v2FXb4Ozj6ztDDMy4orIDX+
> nRKOXsq5YajGpwd1A4hj8wbXDBlyvVtbjOe5iAeoDmXveI7Z3PqZsryFQXhWnr65
> d/oJGZg8wo/Dh+1G2yEfR83c9Z6pPKd3HNMPqRQCc7nqDiKraKPVUv8ZiDJaD5+i
> hFAo4DQccy9++6o72ZPQp4ylxfoq5AhD5bbvn3mkSUd7b7DGoFgXgaC2NqlmHGmI
> SeNnZrrUpSsVxVyePsTYcPtt6KRY8TNoId0FuB++L8s4Nth0MF1m/cLhir2U7dgU
> paUIyLzHgyn6AQe/Ve+JLKgiqKXffy6K3uImwHNSGO3AYCgWU4q6SBw99xSIyPmk
> QrR51PVNwc2txwhYG5wZ745rAXqw2/J5F5/5q+k/jzlHBotgzM8=
> =pS2Q
> -----END PGP SIGNATURE-----
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
Re: [tomcat] branch master updated: 64141: Allow overriding JVM trust
store
Posted by Christopher Schultz <ch...@christopherschultz.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Rémy,
On 2/15/20 07:22, remm@apache.org wrote:
> This is an automated email from the ASF dual-hosted git
> repository.
>
> remm pushed a commit to branch master in repository
> https://gitbox.apache.org/repos/asf/tomcat.git
>
>
> The following commit(s) were added to refs/heads/master by this
> push: new d14c5b4 64141: Allow overriding JVM trust store d14c5b4
> is described below
>
> commit d14c5b409d08df88ab437f76334342b8f8094b44 Author: remm
> <re...@apache.org> AuthorDate: Sat Feb 15 13:21:53 2020 +0100
>
> 64141: Allow overriding JVM trust store
>
> If using a CA certificate, remove a default value for the trust
> store file when not using a JSSE configuration. ---
> java/org/apache/tomcat/util/net/SSLHostConfig.java | 26
> +++++++++++++++++++--- webapps/docs/changelog.xml
> | 4 ++++ 2 files changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/java/org/apache/tomcat/util/net/SSLHostConfig.java
> b/java/org/apache/tomcat/util/net/SSLHostConfig.java index
> 4a8549d..724497f 100644 ---
> a/java/org/apache/tomcat/util/net/SSLHostConfig.java +++
> b/java/org/apache/tomcat/util/net/SSLHostConfig.java @@ -151,15
> +151,24 @@ public class SSLHostConfig implements Serializable { }
>
>
> - void setProperty(String name, Type configType) { + /** +
> * Set property which belongs to the specified configuration type. +
> * @param name the property name + * @param configType the
> configuration type + * @return true if the property belongs to
> the current confuguration, + * and false otherwise + */ +
> boolean setProperty(String name, Type configType) { if
> (this.configType == null) { this.configType = configType; } else {
> if (configType != this.configType) {
> log.warn(sm.getString("sslHostConfig.mismatch", name,
> getHostName(), configType, this.configType)); +
> return false; } } + return true; }
>
>
> @@ -662,7 +671,13 @@ public class SSLHostConfig implements
> Serializable {
>
>
> public void setCaCertificateFile(String caCertificateFile) { -
> setProperty("caCertificateFile", Type.OPENSSL); + if
> (setProperty("caCertificateFile", Type.OPENSSL)) { + //
> Reset default JSSE trust store if not a JSSE configuration +
> if (truststoreFile != null) { +
> System.out.println("RESET !!!!!!!!!!!!!!!!!");
Probably want this gone. :)
> + truststoreFile = null; + } + }
> this.caCertificateFile = caCertificateFile; }
Also, is this the best way to do this? Instead of defaulting to
javax.ssl.trustStore for the JSSE TrustStore, should we not apply the
default when it's needed?
Like when configuring the SSLContext, check for a null TrustStore and
inherit the JVM default?
I think that would b a little cleaner than blanking-out the default
value of *another* configuration when a different (but related)
configuration option is set.
There is also the possibility that truststoreFile != null, but
truststoreFile is also not set to the JVM default.
What if we throw an exception if both of the configuration options are
present? We won't know which one to use, anyway.
> @@ -673,7 +688,12 @@ public class SSLHostConfig implements
> Serializable {
>
>
> public void setCaCertificatePath(String caCertificatePath) { -
> setProperty("caCertificatePath", Type.OPENSSL); + if
> (setProperty("caCertificatePath", Type.OPENSSL)) { + //
> Reset default JSSE trust store if not a JSSE configuration +
> if (truststoreFile != null) { + truststoreFile =
> null; + } + } this.caCertificatePath =
> caCertificatePath; }
>
> diff --git a/webapps/docs/changelog.xml
> b/webapps/docs/changelog.xml index 753fb4c..e0673d1 100644 ---
> a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@
> -51,6 +51,10 @@ Fix support of native jakarta servlet attributes in
> AJP connector. (remm) </fix> + <fix> +
> <bug>64141</bug>: If using a CA certificate, remove a default
> value + for the truststore file when not using a JSSE
> configuration. (remm) + </fix> </changelog> </subsection>
> </section>
>
>
- -chris
-----BEGIN PGP SIGNATURE-----
Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/
iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAl5JVccACgkQHPApP6U8
pFhjKRAAo0Nscss8xqlihW7kzjY5tb6JotdVnMzvVRHo1OsYeS93jaCsqNID6+wY
Q9dEXP58CJy05W72ftWltvB1npRV+90pSB3tOOFoQ1QTjjTuVBw9vDp8nfR8bSnW
0G+3ClxQX0f5bWJLZbe4ws0z7fPyvu0XS267NpsMIhHCuWkw7CQE0re3f319FPOV
6fHZj82enI9YuRbuLTNCeuv87XSiY5mi3usyZl+lUH0oOqVrQFsk1qRTz/T+5ZQw
vHmJ5Ei1/4JOOJiHd5HeRKtNh3uUR3wSWmdlDKP44v2FXb4Ozj6ztDDMy4orIDX+
nRKOXsq5YajGpwd1A4hj8wbXDBlyvVtbjOe5iAeoDmXveI7Z3PqZsryFQXhWnr65
d/oJGZg8wo/Dh+1G2yEfR83c9Z6pPKd3HNMPqRQCc7nqDiKraKPVUv8ZiDJaD5+i
hFAo4DQccy9++6o72ZPQp4ylxfoq5AhD5bbvn3mkSUd7b7DGoFgXgaC2NqlmHGmI
SeNnZrrUpSsVxVyePsTYcPtt6KRY8TNoId0FuB++L8s4Nth0MF1m/cLhir2U7dgU
paUIyLzHgyn6AQe/Ve+JLKgiqKXffy6K3uImwHNSGO3AYCgWU4q6SBw99xSIyPmk
QrR51PVNwc2txwhYG5wZ745rAXqw2/J5F5/5q+k/jzlHBotgzM8=
=pS2Q
-----END PGP SIGNATURE-----
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org