You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2019/07/12 15:35:25 UTC
[tomcat] 02/02: Fix
https://bz.apache.org/bugzilla/show_bug.cgi?id=63524 part 2 of 2
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit f1a93f0945cd4d90ea8e63d700c7cc5d70ae16e1
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Jul 12 16:34:43 2019 +0100
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63524 part 2 of 2
Enable PKCS#1 format private keys to be read into the in-memory key
store.
---
java/org/apache/tomcat/util/buf/Asn1Parser.java | 89 ++++++++++++++++++++++
.../apache/tomcat/util/buf/LocalStrings.properties | 3 +
.../tomcat/util/net/jsse/LocalStrings.properties | 2 +
java/org/apache/tomcat/util/net/jsse/PEMFile.java | 54 +++++++++++--
webapps/docs/changelog.xml | 7 ++
5 files changed, 150 insertions(+), 5 deletions(-)
diff --git a/java/org/apache/tomcat/util/buf/Asn1Parser.java b/java/org/apache/tomcat/util/buf/Asn1Parser.java
new file mode 100644
index 0000000..35fe161
--- /dev/null
+++ b/java/org/apache/tomcat/util/buf/Asn1Parser.java
@@ -0,0 +1,89 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.util.buf;
+
+import java.math.BigInteger;
+
+import org.apache.tomcat.util.res.StringManager;
+
+/**
+ * This is a very basic ASN.1 parser that provides the limited functionality
+ * required by Tomcat. It is a long way from a complete parser.
+ *
+ * TODO: Consider extending this parser and refactoring the SpnegoTokenFixer to
+ * use it.
+ */
+public class Asn1Parser {
+
+ private static final StringManager sm = StringManager.getManager(Asn1Parser.class);
+
+ private final byte[] source;
+
+ private int pos = 0;
+
+
+ public Asn1Parser(byte[] source) {
+ this.source = source;
+ }
+
+
+ public void parseTag(int tag) {
+ int value = next();
+ if (value != tag) {
+ throw new IllegalArgumentException(sm.getString("asn1Parser.tagMismatch",
+ Integer.valueOf(tag), Integer.valueOf(value)));
+ }
+ }
+
+
+ public void parseFullLength() {
+ int len = parseLength();
+ if (len + pos != source.length) {
+ throw new IllegalArgumentException(sm.getString("asn1Parser.lengthInvalid",
+ Integer.valueOf(len), Integer.valueOf(source.length - pos)));
+ }
+ }
+
+
+ public int parseLength() {
+ int len = next();
+ if (len > 127) {
+ int bytes = len - 128;
+ len = 0;
+ for (int i = 0; i < bytes; i++) {
+ len = len << 8;
+ len = len + next();
+ }
+ }
+ return len;
+ }
+
+
+ public BigInteger parseInt() {
+ parseTag(0x02);
+ int len = parseLength();
+ byte[] val = new byte[len];
+ System.arraycopy(source, pos, val, 0, len);
+ pos += len;
+ return new BigInteger(val);
+ }
+
+
+ private int next() {
+ return source[pos++] & 0xFF;
+ }
+}
diff --git a/java/org/apache/tomcat/util/buf/LocalStrings.properties b/java/org/apache/tomcat/util/buf/LocalStrings.properties
index 9c47113..d595e75 100644
--- a/java/org/apache/tomcat/util/buf/LocalStrings.properties
+++ b/java/org/apache/tomcat/util/buf/LocalStrings.properties
@@ -13,6 +13,9 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+asn1Parser.lengthInvalid=Invalid length [{0}] bytes reported when the input data length is [{1}] bytes
+asn1Parser.tagMismatch=Expected to find value [{0}] but found value [{1}]
+
b2cConverter.unknownEncoding=The character encoding [{0}] is not supported
byteBufferUtils.cleaner=Cannot use direct ByteBuffer cleaner, memory leaking may occur
diff --git a/java/org/apache/tomcat/util/net/jsse/LocalStrings.properties b/java/org/apache/tomcat/util/net/jsse/LocalStrings.properties
index 931410a..2bcb836 100644
--- a/java/org/apache/tomcat/util/net/jsse/LocalStrings.properties
+++ b/java/org/apache/tomcat/util/net/jsse/LocalStrings.properties
@@ -35,3 +35,5 @@ jsseUtil.noCrlSupport=The truststoreProvider [{0}] does not support the certific
jsseUtil.noVerificationDepth=The truststoreProvider [{0}] does not support the certificateVerificationDepth configuration option
jsseUtil.trustedCertNotChecked=The validity dates of the trusted certificate with alias [{0}] were not checked as the certificate was of an unknown type
jsseUtil.trustedCertNotValid=The trusted certificate with alias [{0}] and DN [{1}] is not valid due to [{2}]. Certificates signed by this trusted certificate WILL be accepted
+
+pemFile.noMultiPrimes=The PKCS#1 certificate is in multi-prime format and Java does not provide an API for constructing an RSA private key object from that format
\ No newline at end of file
diff --git a/java/org/apache/tomcat/util/net/jsse/PEMFile.java b/java/org/apache/tomcat/util/net/jsse/PEMFile.java
index 9a3541f..47c8787 100644
--- a/java/org/apache/tomcat/util/net/jsse/PEMFile.java
+++ b/java/org/apache/tomcat/util/net/jsse/PEMFile.java
@@ -21,6 +21,7 @@ import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
+import java.math.BigInteger;
import java.nio.charset.StandardCharsets;
import java.security.GeneralSecurityException;
import java.security.InvalidKeyException;
@@ -32,6 +33,7 @@ import java.security.cert.X509Certificate;
import java.security.spec.InvalidKeySpecException;
import java.security.spec.KeySpec;
import java.security.spec.PKCS8EncodedKeySpec;
+import java.security.spec.RSAPrivateCrtKeySpec;
import java.util.ArrayList;
import java.util.List;
@@ -41,6 +43,7 @@ import javax.crypto.SecretKey;
import javax.crypto.SecretKeyFactory;
import javax.crypto.spec.PBEKeySpec;
+import org.apache.tomcat.util.buf.Asn1Parser;
import org.apache.tomcat.util.codec.binary.Base64;
import org.apache.tomcat.util.file.ConfigFileLoader;
import org.apache.tomcat.util.res.StringManager;
@@ -100,10 +103,13 @@ public class PEMFile {
for (Part part : parts) {
switch (part.type) {
case "PRIVATE KEY":
- privateKey = part.toPrivateKey(null, keyAlgorithm);
+ privateKey = part.toPrivateKey(null, keyAlgorithm, Format.PKCS8);
break;
case "ENCRYPTED PRIVATE KEY":
- privateKey = part.toPrivateKey(password, keyAlgorithm);
+ privateKey = part.toPrivateKey(password, keyAlgorithm, Format.PKCS8);
+ break;
+ case "RSA PRIVATE KEY":
+ privateKey = part.toPrivateKey(null, keyAlgorithm, Format.PKCS1);
break;
case "CERTIFICATE":
case "X509 CERTIFICATE":
@@ -129,11 +135,21 @@ public class PEMFile {
return (X509Certificate) factory.generateCertificate(new ByteArrayInputStream(decode()));
}
- public PrivateKey toPrivateKey(String password, String keyAlgorithm) throws GeneralSecurityException, IOException {
- KeySpec keySpec;
+ public PrivateKey toPrivateKey(String password, String keyAlgorithm, Format format)
+ throws GeneralSecurityException, IOException {
+ KeySpec keySpec = null;
if (password == null) {
- keySpec = new PKCS8EncodedKeySpec(decode());
+ switch (format) {
+ case PKCS1: {
+ keySpec = parsePKCS1(decode());
+ break;
+ }
+ case PKCS8: {
+ keySpec = new PKCS8EncodedKeySpec(decode());
+ break;
+ }
+ }
} else {
EncryptedPrivateKeyInfo privateKeyInfo = new EncryptedPrivateKeyInfo(decode());
SecretKeyFactory secretKeyFactory = SecretKeyFactory.getInstance(privateKeyInfo.getAlgName());
@@ -164,5 +180,33 @@ public class PEMFile {
throw exception;
}
+
+
+ private RSAPrivateCrtKeySpec parsePKCS1(byte[] source) {
+ Asn1Parser p = new Asn1Parser(source);
+
+ // https://en.wikipedia.org/wiki/X.690#BER_encoding
+ // https://tools.ietf.org/html/rfc8017#page-55
+
+ // Type
+ p.parseTag(0x30);
+ // Length
+ p.parseFullLength();
+
+ BigInteger version = p.parseInt();
+ if (version.intValue() == 1) {
+ // JRE doesn't provide a suitable constructor for multi-prime
+ // keys
+ throw new IllegalArgumentException(sm.getString("pemFile.noMultiPrimes"));
+ }
+ return new RSAPrivateCrtKeySpec(p.parseInt(), p.parseInt(), p.parseInt(), p.parseInt(),
+ p.parseInt(), p.parseInt(), p.parseInt(), p.parseInt());
+ }
+ }
+
+
+ private enum Format {
+ PKCS1,
+ PKCS8
}
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index e898ca8..d1f80e7 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -65,6 +65,13 @@
Windows operating systems that required multiple smaller pollsets to be
used are no longer supported. (markt)
</scode>
+ <fix>
+ <bug>63524</bug>: Improve the handling of PEM file based keys and
+ certificates that do not include a full certificate chain when
+ configuring the internal, in-memory key store. Improve the handling of
+ PKCS#1 formatted private keys when configuring the internal, in-memory
+ key store. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Cluster">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] 02/02: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63524
part 2 of 2
Posted by Rémy Maucherat <re...@apache.org>.
On Fri, Jul 12, 2019 at 5:35 PM <ma...@apache.org> wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> markt pushed a commit to branch master
> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>
> commit f1a93f0945cd4d90ea8e63d700c7cc5d70ae16e1
> Author: Mark Thomas <ma...@apache.org>
> AuthorDate: Fri Jul 12 16:34:43 2019 +0100
>
> Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63524 part 2 of 2
>
> Enable PKCS#1 format private keys to be read into the in-memory key
> store.
>
Nice, I did not add PKCS1 support along with the cloud support (it was not
exactly super critical to have it, so I passed on it), so this is a good
addition since it also use PEMFile.
Rémy
> ---
> java/org/apache/tomcat/util/buf/Asn1Parser.java | 89
> ++++++++++++++++++++++
> .../apache/tomcat/util/buf/LocalStrings.properties | 3 +
> .../tomcat/util/net/jsse/LocalStrings.properties | 2 +
> java/org/apache/tomcat/util/net/jsse/PEMFile.java | 54 +++++++++++--
> webapps/docs/changelog.xml | 7 ++
> 5 files changed, 150 insertions(+), 5 deletions(-)
>
> diff --git a/java/org/apache/tomcat/util/buf/Asn1Parser.java
> b/java/org/apache/tomcat/util/buf/Asn1Parser.java
> new file mode 100644
> index 0000000..35fe161
> --- /dev/null
> +++ b/java/org/apache/tomcat/util/buf/Asn1Parser.java
> @@ -0,0 +1,89 @@
> +/*
> + * Licensed to the Apache Software Foundation (ASF) under one or more
> + * contributor license agreements. See the NOTICE file distributed with
> + * this work for additional information regarding copyright ownership.
> + * The ASF licenses this file to You under the Apache License, Version 2.0
> + * (the "License"); you may not use this file except in compliance with
> + * the License. You may obtain a copy of the License at
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +package org.apache.tomcat.util.buf;
> +
> +import java.math.BigInteger;
> +
> +import org.apache.tomcat.util.res.StringManager;
> +
> +/**
> + * This is a very basic ASN.1 parser that provides the limited
> functionality
> + * required by Tomcat. It is a long way from a complete parser.
> + *
> + * TODO: Consider extending this parser and refactoring the
> SpnegoTokenFixer to
> + * use it.
> + */
> +public class Asn1Parser {
> +
> + private static final StringManager sm =
> StringManager.getManager(Asn1Parser.class);
> +
> + private final byte[] source;
> +
> + private int pos = 0;
> +
> +
> + public Asn1Parser(byte[] source) {
> + this.source = source;
> + }
> +
> +
> + public void parseTag(int tag) {
> + int value = next();
> + if (value != tag) {
> + throw new
> IllegalArgumentException(sm.getString("asn1Parser.tagMismatch",
> + Integer.valueOf(tag), Integer.valueOf(value)));
> + }
> + }
> +
> +
> + public void parseFullLength() {
> + int len = parseLength();
> + if (len + pos != source.length) {
> + throw new
> IllegalArgumentException(sm.getString("asn1Parser.lengthInvalid",
> + Integer.valueOf(len), Integer.valueOf(source.length -
> pos)));
> + }
> + }
> +
> +
> + public int parseLength() {
> + int len = next();
> + if (len > 127) {
> + int bytes = len - 128;
> + len = 0;
> + for (int i = 0; i < bytes; i++) {
> + len = len << 8;
> + len = len + next();
> + }
> + }
> + return len;
> + }
> +
> +
> + public BigInteger parseInt() {
> + parseTag(0x02);
> + int len = parseLength();
> + byte[] val = new byte[len];
> + System.arraycopy(source, pos, val, 0, len);
> + pos += len;
> + return new BigInteger(val);
> + }
> +
> +
> + private int next() {
> + return source[pos++] & 0xFF;
> + }
> +}
> diff --git a/java/org/apache/tomcat/util/buf/LocalStrings.properties
> b/java/org/apache/tomcat/util/buf/LocalStrings.properties
> index 9c47113..d595e75 100644
> --- a/java/org/apache/tomcat/util/buf/LocalStrings.properties
> +++ b/java/org/apache/tomcat/util/buf/LocalStrings.properties
> @@ -13,6 +13,9 @@
> # See the License for the specific language governing permissions and
> # limitations under the License.
>
> +asn1Parser.lengthInvalid=Invalid length [{0}] bytes reported when the
> input data length is [{1}] bytes
> +asn1Parser.tagMismatch=Expected to find value [{0}] but found value [{1}]
> +
> b2cConverter.unknownEncoding=The character encoding [{0}] is not supported
>
> byteBufferUtils.cleaner=Cannot use direct ByteBuffer cleaner, memory
> leaking may occur
> diff --git a/java/org/apache/tomcat/util/net/jsse/LocalStrings.properties
> b/java/org/apache/tomcat/util/net/jsse/LocalStrings.properties
> index 931410a..2bcb836 100644
> --- a/java/org/apache/tomcat/util/net/jsse/LocalStrings.properties
> +++ b/java/org/apache/tomcat/util/net/jsse/LocalStrings.properties
> @@ -35,3 +35,5 @@ jsseUtil.noCrlSupport=The truststoreProvider [{0}] does
> not support the certific
> jsseUtil.noVerificationDepth=The truststoreProvider [{0}] does not
> support the certificateVerificationDepth configuration option
> jsseUtil.trustedCertNotChecked=The validity dates of the trusted
> certificate with alias [{0}] were not checked as the certificate was of an
> unknown type
> jsseUtil.trustedCertNotValid=The trusted certificate with alias [{0}] and
> DN [{1}] is not valid due to [{2}]. Certificates signed by this trusted
> certificate WILL be accepted
> +
> +pemFile.noMultiPrimes=The PKCS#1 certificate is in multi-prime format and
> Java does not provide an API for constructing an RSA private key object
> from that format
> \ No newline at end of file
> diff --git a/java/org/apache/tomcat/util/net/jsse/PEMFile.java
> b/java/org/apache/tomcat/util/net/jsse/PEMFile.java
> index 9a3541f..47c8787 100644
> --- a/java/org/apache/tomcat/util/net/jsse/PEMFile.java
> +++ b/java/org/apache/tomcat/util/net/jsse/PEMFile.java
> @@ -21,6 +21,7 @@ import java.io.ByteArrayInputStream;
> import java.io.IOException;
> import java.io.InputStream;
> import java.io.InputStreamReader;
> +import java.math.BigInteger;
> import java.nio.charset.StandardCharsets;
> import java.security.GeneralSecurityException;
> import java.security.InvalidKeyException;
> @@ -32,6 +33,7 @@ import java.security.cert.X509Certificate;
> import java.security.spec.InvalidKeySpecException;
> import java.security.spec.KeySpec;
> import java.security.spec.PKCS8EncodedKeySpec;
> +import java.security.spec.RSAPrivateCrtKeySpec;
> import java.util.ArrayList;
> import java.util.List;
>
> @@ -41,6 +43,7 @@ import javax.crypto.SecretKey;
> import javax.crypto.SecretKeyFactory;
> import javax.crypto.spec.PBEKeySpec;
>
> +import org.apache.tomcat.util.buf.Asn1Parser;
> import org.apache.tomcat.util.codec.binary.Base64;
> import org.apache.tomcat.util.file.ConfigFileLoader;
> import org.apache.tomcat.util.res.StringManager;
> @@ -100,10 +103,13 @@ public class PEMFile {
> for (Part part : parts) {
> switch (part.type) {
> case "PRIVATE KEY":
> - privateKey = part.toPrivateKey(null, keyAlgorithm);
> + privateKey = part.toPrivateKey(null, keyAlgorithm,
> Format.PKCS8);
> break;
> case "ENCRYPTED PRIVATE KEY":
> - privateKey = part.toPrivateKey(password,
> keyAlgorithm);
> + privateKey = part.toPrivateKey(password,
> keyAlgorithm, Format.PKCS8);
> + break;
> + case "RSA PRIVATE KEY":
> + privateKey = part.toPrivateKey(null, keyAlgorithm,
> Format.PKCS1);
> break;
> case "CERTIFICATE":
> case "X509 CERTIFICATE":
> @@ -129,11 +135,21 @@ public class PEMFile {
> return (X509Certificate) factory.generateCertificate(new
> ByteArrayInputStream(decode()));
> }
>
> - public PrivateKey toPrivateKey(String password, String
> keyAlgorithm) throws GeneralSecurityException, IOException {
> - KeySpec keySpec;
> + public PrivateKey toPrivateKey(String password, String
> keyAlgorithm, Format format)
> + throws GeneralSecurityException, IOException {
> + KeySpec keySpec = null;
>
> if (password == null) {
> - keySpec = new PKCS8EncodedKeySpec(decode());
> + switch (format) {
> + case PKCS1: {
> + keySpec = parsePKCS1(decode());
> + break;
> + }
> + case PKCS8: {
> + keySpec = new PKCS8EncodedKeySpec(decode());
> + break;
> + }
> + }
> } else {
> EncryptedPrivateKeyInfo privateKeyInfo = new
> EncryptedPrivateKeyInfo(decode());
> SecretKeyFactory secretKeyFactory =
> SecretKeyFactory.getInstance(privateKeyInfo.getAlgName());
> @@ -164,5 +180,33 @@ public class PEMFile {
>
> throw exception;
> }
> +
> +
> + private RSAPrivateCrtKeySpec parsePKCS1(byte[] source) {
> + Asn1Parser p = new Asn1Parser(source);
> +
> + // https://en.wikipedia.org/wiki/X.690#BER_encoding
> + // https://tools.ietf.org/html/rfc8017#page-55
> +
> + // Type
> + p.parseTag(0x30);
> + // Length
> + p.parseFullLength();
> +
> + BigInteger version = p.parseInt();
> + if (version.intValue() == 1) {
> + // JRE doesn't provide a suitable constructor for
> multi-prime
> + // keys
> + throw new
> IllegalArgumentException(sm.getString("pemFile.noMultiPrimes"));
> + }
> + return new RSAPrivateCrtKeySpec(p.parseInt(), p.parseInt(),
> p.parseInt(), p.parseInt(),
> + p.parseInt(), p.parseInt(), p.parseInt(),
> p.parseInt());
> + }
> + }
> +
> +
> + private enum Format {
> + PKCS1,
> + PKCS8
> }
> }
> diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
> index e898ca8..d1f80e7 100644
> --- a/webapps/docs/changelog.xml
> +++ b/webapps/docs/changelog.xml
> @@ -65,6 +65,13 @@
> Windows operating systems that required multiple smaller pollsets
> to be
> used are no longer supported. (markt)
> </scode>
> + <fix>
> + <bug>63524</bug>: Improve the handling of PEM file based keys and
> + certificates that do not include a full certificate chain when
> + configuring the internal, in-memory key store. Improve the
> handling of
> + PKCS#1 formatted private keys when configuring the internal,
> in-memory
> + key store. (markt)
> + </fix>
> </changelog>
> </subsection>
> <subsection name="Cluster">
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>