Skip to content

Commit

Permalink
Improve loading of jar entry certificates
Browse files Browse the repository at this point in the history
Co-Authored-By: Phillip Webb <phil.webb@broadcom.com>
  • Loading branch information
wilkinsona and philwebb committed Aug 22, 2024
1 parent 112cfc8 commit 0b24ee8
Show file tree
Hide file tree
Showing 10 changed files with 340 additions and 45 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
/*
* Copyright 2012-2024 the original author or authors.
*
* Licensed 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
*
* https://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.springframework.boot.loader.jar;

import java.io.Closeable;
import java.io.DataInputStream;
import java.io.EOFException;
import java.io.IOException;
import java.io.InputStream;
import java.util.Arrays;
import java.util.jar.JarEntry;
import java.util.jar.JarInputStream;
import java.util.zip.Inflater;
import java.util.zip.ZipEntry;

/**
* Helper class to iterate entries in a jar file and check that content matches a related
* entry.
*
* @author Phillip Webb
* @author Andy Wilkinson
*/
class JarEntriesStream implements Closeable {

private static final int BUFFER_SIZE = 4 * 1024;

private final JarInputStream in;

private final byte[] inBuffer = new byte[BUFFER_SIZE];

private final byte[] compareBuffer = new byte[BUFFER_SIZE];

private final Inflater inflater = new Inflater(true);

private JarEntry entry;

JarEntriesStream(InputStream in) throws IOException {
this.in = new JarInputStream(in);
}

JarEntry getNextEntry() throws IOException {
this.entry = this.in.getNextJarEntry();
if (this.entry != null) {
this.entry.getSize();
}
this.inflater.reset();
return this.entry;
}

boolean matches(boolean directory, int size, int compressionMethod, InputStreamSupplier streamSupplier)
throws IOException {
if (this.entry.isDirectory() != directory) {
fail("directory");
}
if (this.entry.getMethod() != compressionMethod) {
fail("compression method");
}
if (this.entry.isDirectory()) {

This comment has been minimized.

Copy link
@maloewe-ona

maloewe-ona Dec 2, 2024

To be safe, should this also verify that entry actually has size 0 / no readable bytes?

Because JarEntry#isDirectory() just checks if the entry name ends with /, but ignores whether the entry has content or not.

This comment has been minimized.

Copy link
@philwebb

philwebb Dec 2, 2024

Author Member

I don't think we need to because in practice a directory entry doesn't have content.

This comment has been minimized.

Copy link
@maloewe-ona

maloewe-ona Dec 3, 2024

My concern was that a malicious JAR could contain a 'directory' entry with entry size > 0 and circumvent the checks this way, being considered matching despite unexpectedly having content.

And combined with ZipFile#getEntry(String) adding a trailing / during lookup (e.g. looking up entry can yield entry/), maybe this is a problem?

But I am not that very familiar with this validation logic here, so maybe this is not actually a problem for this use case. Sorry for the confusion in that case.

this.in.closeEntry();
return true;
}
try (DataInputStream expected = new DataInputStream(getInputStream(size, streamSupplier))) {
assertSameContent(expected);
}
return true;
}

private InputStream getInputStream(int size, InputStreamSupplier streamSupplier) throws IOException {
InputStream inputStream = streamSupplier.get();
return (this.entry.getMethod() != ZipEntry.DEFLATED) ? inputStream
: new ZipInflaterInputStream(inputStream, this.inflater, size);
}

private void assertSameContent(DataInputStream expected) throws IOException {
int len;
while ((len = this.in.read(this.inBuffer)) > 0) {
try {
expected.readFully(this.compareBuffer, 0, len);
if (Arrays.equals(this.inBuffer, 0, len, this.compareBuffer, 0, len)) {
continue;
}
}
catch (EOFException ex) {
// Continue and throw exception due to mismatched content length.
}
fail("content");
}
if (expected.read() != -1) {
fail("content");
}
}

private void fail(String check) {
throw new IllegalStateException("Content mismatch when reading security info for entry '%s' (%s check)"
.formatted(this.entry.getName(), check));
}

@Override
public void close() throws IOException {
this.inflater.end();
this.in.close();
}

@FunctionalInterface
interface InputStreamSupplier {

InputStream get() throws IOException;

}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2023 the original author or authors.
* Copyright 2012-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -26,7 +26,6 @@
import java.util.NoSuchElementException;
import java.util.jar.Attributes;
import java.util.jar.Attributes.Name;
import java.util.jar.JarInputStream;
import java.util.jar.Manifest;
import java.util.zip.ZipEntry;

Expand Down Expand Up @@ -334,37 +333,30 @@ private AsciiBytes applyFilter(AsciiBytes name) {
JarEntryCertification getCertification(JarEntry entry) throws IOException {
JarEntryCertification[] certifications = this.certifications;
if (certifications == null) {
certifications = new JarEntryCertification[this.size];
// We fall back to use JarInputStream to obtain the certs. This isn't that
// fast, but hopefully doesn't happen too often.
try (JarInputStream certifiedJarStream = new JarInputStream(this.jarFile.getData().getInputStream())) {
java.util.jar.JarEntry certifiedEntry;
while ((certifiedEntry = certifiedJarStream.getNextJarEntry()) != null) {
// Entry must be closed to trigger a read and set entry certificates
certifiedJarStream.closeEntry();
int index = getEntryIndex(certifiedEntry.getName());
if (index != -1) {
certifications[index] = JarEntryCertification.from(certifiedEntry);
}
}
}
certifications = getCertifications();
this.certifications = certifications;
}
JarEntryCertification certification = certifications[entry.getIndex()];
return (certification != null) ? certification : JarEntryCertification.NONE;
}

private int getEntryIndex(CharSequence name) {
int hashCode = AsciiBytes.hashCode(name);
int index = getFirstIndex(hashCode);
while (index >= 0 && index < this.size && this.hashCodes[index] == hashCode) {
FileHeader candidate = getEntry(index, FileHeader.class, false, null);
if (candidate.hasName(name, NO_SUFFIX)) {
return index;
private JarEntryCertification[] getCertifications() throws IOException {
JarEntryCertification[] certifications = new JarEntryCertification[this.size];
try (JarEntriesStream entries = new JarEntriesStream(this.jarFile.getData().getInputStream())) {
java.util.jar.JarEntry entry = entries.getNextEntry();
while (entry != null) {
JarEntry relatedEntry = this.doGetEntry(entry.getName(), JarEntry.class, false, null);
if (relatedEntry != null && entries.matches(relatedEntry.isDirectory(), (int) relatedEntry.getSize(),
relatedEntry.getMethod(), () -> getEntryData(relatedEntry).getInputStream())) {
int index = relatedEntry.getIndex();
if (index != -1) {
certifications[index] = JarEntryCertification.from(entry);
}
}
entry = entries.getNextEntry();
}
index++;
}
return -1;
return certifications;
}

private static void swap(int[] array, int i, int j) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2023 the original author or authors.
* Copyright 2012-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -30,12 +30,23 @@
*/
class ZipInflaterInputStream extends InflaterInputStream {

private final boolean ownsInflator;

private int available;

private boolean extraBytesWritten;

ZipInflaterInputStream(InputStream inputStream, int size) {
super(inputStream, new Inflater(true), getInflaterBufferSize(size));
this(inputStream, new Inflater(true), size, true);
}

ZipInflaterInputStream(InputStream inputStream, Inflater inflater, int size) {
this(inputStream, inflater, size, false);
}

private ZipInflaterInputStream(InputStream inputStream, Inflater inflater, int size, boolean ownsInflator) {
super(inputStream, inflater, getInflaterBufferSize(size));
this.ownsInflator = ownsInflator;
this.available = size;
}

Expand All @@ -59,7 +70,9 @@ public int read(byte[] b, int off, int len) throws IOException {
@Override
public void close() throws IOException {
super.close();
this.inf.end();
if (this.ownsInflator) {
this.inf.end();
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -666,6 +666,26 @@ void jarFileEntryWithEpochTimeOfZeroShouldNotFail() throws Exception {
}
}

@Test
void mismatchedStreamEntriesThrowsException() throws IOException {
File mismatchJar = new File("src/test/resources/jars/mismatch.jar");
IllegalStateException failure = null;
try (JarFile jarFile = new JarFile(mismatchJar)) {
JarFile nestedJarFile = jarFile.getNestedJarFile(jarFile.getJarEntry("inner.jar"));
Enumeration<JarEntry> entries = nestedJarFile.entries();
while (entries.hasMoreElements()) {
try {
entries.nextElement().getCodeSigners();
}
catch (IllegalStateException ex) {
failure = (failure != null) ? failure : ex;
}
}
}
assertThat(failure)
.hasMessage("Content mismatch when reading security info for entry 'content' (content check)");
}

private File createJarFileWithEpochTimeOfZero() throws Exception {
File jarFile = new File(this.tempDir, "temp.jar");
FileOutputStream fileOutputStream = new FileOutputStream(jarFile);
Expand Down
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
/*
* Copyright 2012-2024 the original author or authors.
*
* Licensed 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
*
* https://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.springframework.boot.loader.jar;

import java.io.Closeable;
import java.io.DataInputStream;
import java.io.EOFException;
import java.io.IOException;
import java.io.InputStream;
import java.util.Arrays;
import java.util.jar.JarEntry;
import java.util.jar.JarInputStream;
import java.util.zip.Inflater;
import java.util.zip.ZipEntry;

/**
* Helper class to iterate entries in a jar file and check that content matches a related
* entry.
*
* @author Phillip Webb
* @author Andy Wilkinson
*/
class JarEntriesStream implements Closeable {

private static final int BUFFER_SIZE = 4 * 1024;

private final JarInputStream in;

private final byte[] inBuffer = new byte[BUFFER_SIZE];

private final byte[] compareBuffer = new byte[BUFFER_SIZE];

private final Inflater inflater = new Inflater(true);

private JarEntry entry;

JarEntriesStream(InputStream in) throws IOException {
this.in = new JarInputStream(in);
}

JarEntry getNextEntry() throws IOException {
this.entry = this.in.getNextJarEntry();
if (this.entry != null) {
this.entry.getSize();
}
this.inflater.reset();
return this.entry;
}

boolean matches(boolean directory, int size, int compressionMethod, InputStreamSupplier streamSupplier)
throws IOException {
if (this.entry.isDirectory() != directory) {
fail("directory");
}
if (this.entry.getMethod() != compressionMethod) {
fail("compression method");
}
if (this.entry.isDirectory()) {
this.in.closeEntry();
return true;
}
try (DataInputStream expected = new DataInputStream(getInputStream(size, streamSupplier))) {
assertSameContent(expected);
}
return true;
}

private InputStream getInputStream(int size, InputStreamSupplier streamSupplier) throws IOException {
InputStream inputStream = streamSupplier.get();
return (this.entry.getMethod() != ZipEntry.DEFLATED) ? inputStream
: new ZipInflaterInputStream(inputStream, this.inflater, size);
}

private void assertSameContent(DataInputStream expected) throws IOException {
int len;
while ((len = this.in.read(this.inBuffer)) > 0) {
try {
expected.readFully(this.compareBuffer, 0, len);
if (Arrays.equals(this.inBuffer, 0, len, this.compareBuffer, 0, len)) {
continue;
}
}
catch (EOFException ex) {
// Continue and throw exception due to mismatched content length.
}
fail("content");
}
if (expected.read() != -1) {
fail("content");
}
}

private void fail(String check) {
throw new IllegalStateException("Content mismatch when reading security info for entry '%s' (%s check)"
.formatted(this.entry.getName(), check));
}

@Override
public void close() throws IOException {
this.inflater.end();
this.in.close();
}

@FunctionalInterface
interface InputStreamSupplier {

InputStream get() throws IOException;

}

}
Loading

0 comments on commit 0b24ee8

Please sign in to comment.