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:54:50 UTC

[tomcat] branch 8.5.x updated (83636c9 -> 495bff8)

This is an automated email from the ASF dual-hosted git repository.

markt pushed a change to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git.


    from 83636c9  Add BZ reference to changelog entry
     new 6279283  Back-port option to configure key algorithm
     new d55ed7e  Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63524 part 1 of 2
     new 495bff8  Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63524 part 2 of 2

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 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  | 71 +++++++++++++++--
 .../util/net/openssl/LocalStrings.properties       |  1 +
 .../tomcat/util/net/openssl/OpenSSLUtil.java       | 22 ++++--
 webapps/docs/changelog.xml                         | 11 +++
 7 files changed, 186 insertions(+), 13 deletions(-)
 create mode 100644 java/org/apache/tomcat/util/buf/Asn1Parser.java


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 02/03: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63524 part 1 of 2

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit d55ed7ebbc9f79ee834cab41b0cc98a888447643
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Jul 12 16:18:17 2019 +0100

    Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63524 part 1 of 2
    
    Adding a key to the in-memory key store is required for private keys.
    Improve the handling of the case when it is not present.
---
 .../util/net/openssl/LocalStrings.properties       |  1 +
 .../tomcat/util/net/openssl/OpenSSLUtil.java       | 22 ++++++++++++++++------
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/java/org/apache/tomcat/util/net/openssl/LocalStrings.properties b/java/org/apache/tomcat/util/net/openssl/LocalStrings.properties
index 1dca2b5..eb037b9 100644
--- a/java/org/apache/tomcat/util/net/openssl/LocalStrings.properties
+++ b/java/org/apache/tomcat/util/net/openssl/LocalStrings.properties
@@ -51,6 +51,7 @@ openssl.errorSSLCtxInit=Error initializing SSL context
 openssl.keyManagerMissing=No key manager found
 openssl.makeConf=Creating OpenSSLConf context
 openssl.nonJsseCertficate=The certificate [{0}] or its private key [{1}] could not be processed using a JSSE key manager and will be given directly to OpenSSL
+openssl.nonJsseChain=The certificate chain [{0}] was not specified or was not valid and JSSE requires a valid certificate chain so attempting to use OpenSSL directly
 openssl.trustManagerMissing=No trust manager found
 
 opensslconf.applyCommand=OpenSSLConf applying command (name [{0}], value [{1}])
diff --git a/java/org/apache/tomcat/util/net/openssl/OpenSSLUtil.java b/java/org/apache/tomcat/util/net/openssl/OpenSSLUtil.java
index e8f0b9b..fada2ca 100644
--- a/java/org/apache/tomcat/util/net/openssl/OpenSSLUtil.java
+++ b/java/org/apache/tomcat/util/net/openssl/OpenSSLUtil.java
@@ -96,16 +96,26 @@ public class OpenSSLUtil extends SSLUtilBase {
     public KeyManager[] getKeyManagers() throws Exception {
         try {
             return super.getKeyManagers();
+        } catch (IllegalArgumentException e) {
+            // No (or invalid?) certificate chain was provided for the cert
+            String msg = sm.getString("openssl.nonJsseChain", certificate.getCertificateChainFile());
+            if (log.isDebugEnabled()) {
+                log.info(msg, e);
+            } else {
+                log.info(msg);
+            }
+            return null;
         } catch (KeyStoreException | IOException e) {
-            // Depending on what is presented, JSSE may throw either of the
-            // above exceptions if it doesn't understand the provided file.
+            // Depending on what is presented, JSSE may also throw
+            // KeyStoreException or IOException if it doesn't understand the
+            // provided file.
             if (certificate.getCertificateFile() != null) {
+                String msg = sm.getString("openssl.nonJsseCertficate",
+                        certificate.getCertificateFile(), certificate.getCertificateKeyFile());
                 if (log.isDebugEnabled()) {
-                    log.info(sm.getString("openssl.nonJsseCertficate",
-                            certificate.getCertificateFile(), certificate.getCertificateKeyFile()), e);
+                    log.info(msg, e);
                 } else {
-                    log.info(sm.getString("openssl.nonJsseCertficate",
-                            certificate.getCertificateFile(), certificate.getCertificateKeyFile()));
+                    log.info(msg);
                 }
                 // Assume JSSE processing of the certificate failed, try again with OpenSSL
                 // without a key manager


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 03/03: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63524 part 2 of 2

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 495bff88dd39cd8c2d654de6bb8914313a7bc6d1
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                         | 11 +++
 5 files changed, 154 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 d204f12..9f2c9fa 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 f373939..add1be5 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 f784486..32774ff 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -58,6 +58,17 @@
       </fix>
      </changelog>
   </subsection>
+  <subsection name="Coyote">
+    <changelog>
+      <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="Other">
     <changelog>
       <update>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 01/03: Back-port option to configure key algorithm

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 6279283acd41d0f9a38636e6f8614b47f3a0f5aa
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Jul 12 16:51:56 2019 +0100

    Back-port option to configure key algorithm
    
    No currently required in 8.5.x but it reduces the diff to 9.0.x, making
    other back-ports cleaner / easier.
---
 java/org/apache/tomcat/util/net/jsse/PEMFile.java | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/java/org/apache/tomcat/util/net/jsse/PEMFile.java b/java/org/apache/tomcat/util/net/jsse/PEMFile.java
index ee09cb2..f373939 100644
--- a/java/org/apache/tomcat/util/net/jsse/PEMFile.java
+++ b/java/org/apache/tomcat/util/net/jsse/PEMFile.java
@@ -71,6 +71,11 @@ public class PEMFile {
     }
 
     public PEMFile(String filename, String password) throws IOException, GeneralSecurityException {
+        this(filename, password, null);
+    }
+
+    public PEMFile(String filename, String password, String keyAlgorithm)
+            throws IOException, GeneralSecurityException {
         this.filename = filename;
 
         List<Part> parts = new ArrayList<>();
@@ -95,10 +100,10 @@ public class PEMFile {
         for (Part part : parts) {
             switch (part.type) {
                 case "PRIVATE KEY":
-                    privateKey = part.toPrivateKey(null);
+                    privateKey = part.toPrivateKey(null, keyAlgorithm);
                     break;
                 case "ENCRYPTED PRIVATE KEY":
-                    privateKey = part.toPrivateKey(password);
+                    privateKey = part.toPrivateKey(password, keyAlgorithm);
                     break;
                 case "CERTIFICATE":
                 case "X509 CERTIFICATE":
@@ -124,7 +129,7 @@ public class PEMFile {
             return (X509Certificate) factory.generateCertificate(new ByteArrayInputStream(decode()));
         }
 
-        public PrivateKey toPrivateKey(String password) throws GeneralSecurityException, IOException {
+        public PrivateKey toPrivateKey(String password, String keyAlgorithm) throws GeneralSecurityException, IOException {
             KeySpec keySpec;
 
             if (password == null) {
@@ -141,9 +146,17 @@ public class PEMFile {
             }
 
             InvalidKeyException exception = new InvalidKeyException(sm.getString("jsse.pemParseError", filename));
-            for (String algorithm : new String[] {"RSA", "DSA", "EC"}) {
+            if (keyAlgorithm == null) {
+                for (String algorithm : new String[] {"RSA", "DSA", "EC"}) {
+                    try {
+                        return KeyFactory.getInstance(algorithm).generatePrivate(keySpec);
+                    } catch (InvalidKeySpecException e) {
+                        exception.addSuppressed(e);
+                    }
+                }
+            } else {
                 try {
-                    return KeyFactory.getInstance(algorithm).generatePrivate(keySpec);
+                    return KeyFactory.getInstance(keyAlgorithm).generatePrivate(keySpec);
                 } catch (InvalidKeySpecException e) {
                     exception.addSuppressed(e);
                 }


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org