Fix for PemTrustConfigTests.testTrustConfigReloadsFileContents failure (#43539) (#43613)

The test `PemTrustConfigTests.testTrustConfigReloadsFileContents` failed
intermittently with `ArrayIndexOutOfBoundsException` while parsing
the randomly generated bytes array representing DER encoded stream.
This seems to be a bug in JDK (once confirmed we can raise the bug
in JDK bugs system).

The problem arises when the `X509Factory#parseX509orPKCS7()` tries to
[create `PKCS7` block](19fb8f93c5/src/java.base/share/classes/sun/security/provider/X509Factory.java (L460)) from der encoded stream. While constructing PKCS7
block it tries to create `ContentInfo` type but fails to do so for the
stream where the length after the DER SEQUENCE is 0.
`DerInputStream#getSequence` [may return empty array of `DerValue`](19fb8f93c5/src/java.base/share/classes/sun/security/util/DerInputStream.java (L409..L412)) but
[the code in `ContentInfo`](19fb8f93c5/src/java.base/share/classes/sun/security/pkcs/ContentInfo.java (L135)) does not check for the empty thereby throwing
`ArrayIndexOutOfBoundsException`.

Closes #42509
This commit is contained in:
Yogesh Gaikwad 2019-06-26 13:32:01 +10:00 committed by GitHub
parent 05e1f55a88
commit ca43cdf755
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 24 additions and 3 deletions

View File

@ -22,7 +22,6 @@ package org.elasticsearch.common.ssl;
import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.ESTestCase;
import org.hamcrest.Matchers; import org.hamcrest.Matchers;
import javax.net.ssl.X509ExtendedTrustManager;
import java.nio.file.Files; import java.nio.file.Files;
import java.nio.file.NoSuchFileException; import java.nio.file.NoSuchFileException;
import java.nio.file.Path; import java.nio.file.Path;
@ -37,6 +36,8 @@ import java.util.Set;
import java.util.stream.Collectors; import java.util.stream.Collectors;
import java.util.stream.Stream; import java.util.stream.Stream;
import javax.net.ssl.X509ExtendedTrustManager;
public class PemTrustConfigTests extends ESTestCase { public class PemTrustConfigTests extends ESTestCase {
public void testBuildTrustConfigFromSinglePemFile() throws Exception { public void testBuildTrustConfigFromSinglePemFile() throws Exception {
@ -57,7 +58,7 @@ public class PemTrustConfigTests extends ESTestCase {
public void testBadFileFormatFails() throws Exception { public void testBadFileFormatFails() throws Exception {
final Path ca = createTempFile("ca", ".crt"); final Path ca = createTempFile("ca", ".crt");
Files.write(ca, randomByteArrayOfLength(128), StandardOpenOption.APPEND); Files.write(ca, generateRandomByteArrayOfLength(128), StandardOpenOption.APPEND);
final PemTrustConfig trustConfig = new PemTrustConfig(Collections.singletonList(ca)); final PemTrustConfig trustConfig = new PemTrustConfig(Collections.singletonList(ca));
assertThat(trustConfig.getDependentFiles(), Matchers.containsInAnyOrder(ca)); assertThat(trustConfig.getDependentFiles(), Matchers.containsInAnyOrder(ca));
assertInvalidFileFormat(trustConfig, ca); assertInvalidFileFormat(trustConfig, ca);
@ -107,7 +108,7 @@ public class PemTrustConfigTests extends ESTestCase {
Files.delete(ca1); Files.delete(ca1);
assertFileNotFound(trustConfig, ca1); assertFileNotFound(trustConfig, ca1);
Files.write(ca1, randomByteArrayOfLength(128), StandardOpenOption.CREATE); Files.write(ca1, generateRandomByteArrayOfLength(128), StandardOpenOption.CREATE);
assertInvalidFileFormat(trustConfig, ca1); assertInvalidFileFormat(trustConfig, ca1);
} }
@ -149,4 +150,24 @@ public class PemTrustConfigTests extends ESTestCase {
assertThat(exception.getMessage(), Matchers.containsString(file.toAbsolutePath().toString())); assertThat(exception.getMessage(), Matchers.containsString(file.toAbsolutePath().toString()));
assertThat(exception.getCause(), Matchers.instanceOf(NoSuchFileException.class)); assertThat(exception.getCause(), Matchers.instanceOf(NoSuchFileException.class));
} }
private byte[] generateRandomByteArrayOfLength(int length) {
byte[] bytes = randomByteArrayOfLength(length);
/*
* If the bytes represent DER encoded value indicating ASN.1 SEQUENCE followed by length byte if it is zero then while trying to
* parse PKCS7 block from the encoded stream, it failed parsing the content type. The DerInputStream.getSequence() method in this
* case returns an empty DerValue array but ContentType does not check the length of array before accessing the array resulting in a
* ArrayIndexOutOfBoundsException. This check ensures that when we create random stream of bytes we do not create ASN.1 SEQUENCE
* followed by zero length which fails the test intermittently.
*/
while(checkRandomGeneratedBytesRepresentZeroLengthDerSequenceCausingArrayIndexOutOfBound(bytes)) {
bytes = randomByteArrayOfLength(length);
}
return bytes;
}
private static boolean checkRandomGeneratedBytesRepresentZeroLengthDerSequenceCausingArrayIndexOutOfBound(byte[] bytes) {
// Tag value indicating an ASN.1 "SEQUENCE". Reference: sun.security.util.DerValue.tag_Sequence = 0x30
return bytes[0] == 0x30 && bytes[1] == 0x00;
}
} }