This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 2945a01d1851c293b95e650203e1ebfef49c7fcd Author: opencode <[email protected]> AuthorDate: Thu Oct 15 15:26:45 2026 +0200 Report malformed structure in PEM files as an error The scanner for the parts of a PEM file silently accepted files with a malformed structure. A part that was never closed by an end boundary, either because the file ended or because the next start boundary appeared, was dropped without any error and the part was simply missing from the result. An end boundary whose type did not match the type of the start boundary that opened the part was accepted. An end boundary that did not follow a start boundary put a null part into the list of parts, leading to a null pointer exception later on. Boundary lines and DEK-Info lines were parsed without any checks, leading to an index out of bounds exception for truncated boundary lines and to a silently ignored or incorrectly parsed DEK-Info line. For example, a DEK-Info line with white space after the comma was parsed with the white space kept as part of the initialisation vector, which only became visible as a failure to decrypt the key. Reject all of these with an IOException that describes the problem. This lets the connector fail at startup with a usable configuration error instead of silently loading an incomplete file or failing later with an unrelated exception. White space at the end of a boundary line remains tolerated, since RFC 7468 allows it there. Tests: 7 new tests in TestPEMFile for the malformed structures listed above and for white space at the end of the boundary lines. All 23 PEM files in the existing test resources parse exactly as before. --- .../tomcat/util/net/jsse/LocalStrings.properties | 5 ++ java/org/apache/tomcat/util/net/jsse/PEMFile.java | 64 +++++++++++++++++--- .../apache/tomcat/util/net/jsse/TestPEMFile.java | 68 ++++++++++++++++++++++ 3 files changed, 128 insertions(+), 9 deletions(-) diff --git a/java/org/apache/tomcat/util/net/jsse/LocalStrings.properties b/java/org/apache/tomcat/util/net/jsse/LocalStrings.properties index 706b236345..f8012507a6 100644 --- a/java/org/apache/tomcat/util/net/jsse/LocalStrings.properties +++ b/java/org/apache/tomcat/util/net/jsse/LocalStrings.properties @@ -21,14 +21,19 @@ jsseUtil.noDefaultProtocols=Unable to determine a default for sslEnabledProtocol jsseUtil.opensslconf.present=A connector is configured to use a JSSE TLS implementation with OpenSSL specific OpenSSLConf configuration elements. The OpenSSLConf configuration elements will be ignored. jsseUtil.psk.present=A connector is configured to use a JSSE TLS implementation with OpenSSL specific pre-shared key configuration elements. The pre-shared key configuration elements will be ignored. +pemFile.boundaryTypeMismatch=The end boundary [{2}] of the PEM file [{0}] does not match the type [{1}] of the start boundary that opened that part pemFile.encryption.broken=The PEM file [{0}] is using [{1}] which is considered broken because a brute force attack is trivial with current hardware pemFile.encryption.insecure=The PEM file [{0}] is using [{1}] which is considered insecure as it is vulnerable to a brute-force attack +pemFile.endWithoutStart=The PEM file [{0}] contains the end boundary [{1}] which does not follow a start boundary +pemFile.invalidBoundary=The PEM file [{0}] contains the line [{1}] which starts with a PEM boundary but does not end with one +pemFile.invalidDekInfo=The PEM file [{0}] contains the DEK-Info line [{1}] which does not provide an algorithm and an IV separated by a comma 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 pemFile.noPassword=A password is required to decrypt the private key pemFile.notPbkdf2=The OID [{0}] is not the correct OID for PBKDF2 which is the only permitted KDF for PBES2 pemFile.notValidRFC5915=The provided key file does not conform to RFC 5915 pemFile.parseError=Unable to parse the key from [{0}] pemFile.parseError.algorithm=Unable to parse the key using algorithm [{0}] +pemFile.startWithoutEnd=The PEM file [{0}] contains the part of type [{1}] which is not closed by an end boundary pemFile.unknownEncryptedFormat=The format [{0}] is not a recognised encrypted PEM file format pemFile.unknownEncryptionAlgorithm=The encryption algorithm with DER encoded OID of [{0}] was not recognised pemFile.unknownPkcs8Algorithm=The PKCS#8 encryption algorithm with DER encoded OID of [{0}] was not recognised diff --git a/java/org/apache/tomcat/util/net/jsse/PEMFile.java b/java/org/apache/tomcat/util/net/jsse/PEMFile.java index f9023bd885..5e413b4d31 100644 --- a/java/org/apache/tomcat/util/net/jsse/PEMFile.java +++ b/java/org/apache/tomcat/util/net/jsse/PEMFile.java @@ -82,6 +82,8 @@ public class PEMFile { // Default defined in RFC 8018 private static final String DEFAULT_PRF = "HmacSHA1"; + private static final String DEK_INFO = "DEK-Info: "; + private static final Map<String,String> OID_TO_PRF = new HashMap<>(); static { // 1.2.840.113549.2.7 @@ -249,27 +251,46 @@ public class PEMFile { String line; while ((line = reader.readLine()) != null) { if (line.startsWith(Part.BEGIN_BOUNDARY)) { + if (part != null) { + throw new IOException(sm.getString("pemFile.startWithoutEnd", filename, part.type)); + } part = new Part(); - part.type = - line.substring(Part.BEGIN_BOUNDARY.length(), line.length() - Part.FINISH_BOUNDARY.length()) - .trim(); + part.type = getBoundaryType(filename, line, Part.BEGIN_BOUNDARY); } else if (line.startsWith(Part.END_BOUNDARY)) { + if (part == null) { + throw new IOException(sm.getString("pemFile.endWithoutStart", filename, line)); + } + String endType = getBoundaryType(filename, line, Part.END_BOUNDARY); + if (!part.type.equals(endType)) { + throw new IOException(sm.getString("pemFile.boundaryTypeMismatch", filename, part.type, + endType)); + } parts.add(part); part = null; } else if (part != null && !line.contains(":") && !line.startsWith(" ")) { part.content += line; } else if (part != null && line.contains(":") && !line.startsWith(" ")) { /* Something like DEK-Info: DES-EDE3-CBC,B5A53CB8B7E50064 */ - if (line.startsWith("DEK-Info: ")) { - String[] pieces = line.split(" "); - pieces = pieces[1].split(","); - if (pieces.length == 2) { - part.algorithm = pieces[0]; - part.ivHex = pieces[1]; + if (line.startsWith(DEK_INFO)) { + String[] pieces = line.substring(DEK_INFO.length()).trim().split(","); + if (pieces.length != 2) { + throw new IOException(sm.getString("pemFile.invalidDekInfo", filename, line)); } + // White space between the comma and the IV is not part + // of the IV + String algorithm = pieces[0].trim(); + String ivHex = pieces[1].trim(); + if (algorithm.isEmpty() || ivHex.isEmpty()) { + throw new IOException(sm.getString("pemFile.invalidDekInfo", filename, line)); + } + part.algorithm = algorithm; + part.ivHex = ivHex; } } } + if (part != null) { + throw new IOException(sm.getString("pemFile.startWithoutEnd", filename, part.type)); + } } String passwordToUse; @@ -310,6 +331,31 @@ public class PEMFile { } } + /** + * Extract the type from a PEM boundary line, checking that the line has the + * structure of a boundary. + * + * @param filename the filename to mention in the error message + * @param line the line to extract the type from + * @param startPrefix the start of the line, either the begin or the end + * boundary prefix + * + * @return the type of the boundary, the text between the prefix and the + * finish boundary, with surrounding white space removed. White + * space at the end of the line is tolerated as RFC 7468 allows it. + * + * @throws IOException if the line does not have the structure of a boundary + */ + private static String getBoundaryType(String filename, String line, String startPrefix) throws IOException { + String trimmedLine = line.trim(); + if (trimmedLine.length() < startPrefix.length() + Part.FINISH_BOUNDARY.length() + || !trimmedLine.endsWith(Part.FINISH_BOUNDARY)) { + throw new IOException(sm.getString("pemFile.invalidBoundary", filename, line)); + } + return trimmedLine.substring(startPrefix.length(), trimmedLine.length() - Part.FINISH_BOUNDARY.length()) + .trim(); + } + private static class Part { public static final String BEGIN_BOUNDARY = "-----BEGIN "; public static final String END_BOUNDARY = "-----END "; diff --git a/test/org/apache/tomcat/util/net/jsse/TestPEMFile.java b/test/org/apache/tomcat/util/net/jsse/TestPEMFile.java index f7b0665abc..da286a8b36 100644 --- a/test/org/apache/tomcat/util/net/jsse/TestPEMFile.java +++ b/test/org/apache/tomcat/util/net/jsse/TestPEMFile.java @@ -20,6 +20,7 @@ import java.io.ByteArrayInputStream; import java.io.File; import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; import java.security.PrivateKey; import java.security.cert.X509Certificate; import java.util.ArrayList; @@ -166,6 +167,73 @@ public class TestPEMFile { } + @Test + public void testEndBoundaryWithoutStartBoundary() throws Exception { + assertInvalidPem("-----END CERTIFICATE-----\n"); + } + + + @Test + public void testStartBoundaryWithoutEndBoundary() throws Exception { + assertInvalidPem("-----BEGIN CERTIFICATE-----\n" + "QUJD\n"); + } + + + @Test + public void testStartBoundaryInsidePart() throws Exception { + assertInvalidPem("-----BEGIN CERTIFICATE-----\n" + "QUJD\n" + "-----BEGIN CERTIFICATE-----\n" + "RUZH\n"); + } + + + @Test + public void testMismatchedBoundaryTypes() throws Exception { + assertInvalidPem("-----BEGIN CERTIFICATE-----\n" + "QUJD\n" + "-----END RSA PRIVATE KEY-----\n"); + } + + + @Test + public void testBoundaryLineWithoutFinishBoundary() throws Exception { + assertInvalidPem("-----BEGIN CERTIFICATE\n"); + } + + + @Test + public void testDekInfoWithoutAlgorithmAndIv() throws Exception { + assertInvalidPem("-----BEGIN RSA PRIVATE KEY-----\n" + "DEK-Info: DES-EDE3-CBC\n" + "QUJD\n" + + "-----END RSA PRIVATE KEY-----\n"); + } + + + @Test + public void testBoundaryLinesWithTrailingWhiteSpace() throws Exception { + // RFC 7468 allows white space between the finish boundary and the end + // of the line + byte[] fileBytes = Files.readAllBytes(new File(getPath(CERTIFICATE)).toPath()); + String pem = new String(fileBytes, StandardCharsets.US_ASCII); + String modified = pem.replace("-----BEGIN CERTIFICATE-----", "-----BEGIN CERTIFICATE-----\t") + .replace("-----END CERTIFICATE-----", "-----END CERTIFICATE----- "); + PEMFile pemFile = new PEMFile("trailing", + new ByteArrayInputStream(modified.getBytes(StandardCharsets.US_ASCII)), null, null); + Assert.assertEquals(1, pemFile.getCertificates().size()); + } + + + /* + * The structure of the file is checked while it is scanned, before any of + * the content is decoded, so these malformed structures must be rejected + * even though the content is not valid PEM encoded data. + */ + private void assertInvalidPem(String content) throws Exception { + ByteArrayInputStream stream = new ByteArrayInputStream(content.getBytes(StandardCharsets.US_ASCII)); + try { + new PEMFile("invalid", stream, null, null); + Assert.fail("Expected the malformed PEM to be rejected"); + } catch (IOException ioe) { + // Expected + } + } + + private void testKeyEncrypted(String file) throws Exception { testKey(file, password, passwordFile); } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
