From 78580f1e4ea86eda0f8f737fa679f0b730075d54 Mon Sep 17 00:00:00 2001 From: Jaikiran Pai Date: Thu, 9 Apr 2026 04:56:30 +0000 Subject: [PATCH] 8378291: Test vector in test/jdk/java/util/jar/JarEntry/GetMethodsReturnClones.java is effectively unsigned Reviewed-by: eirbjo, lancea --- .../jar/JarEntry/GetMethodsReturnClones.java | 151 ++++++++++++++---- test/jdk/java/util/jar/JarEntry/test.jar | Bin 3022 -> 0 bytes 2 files changed, 121 insertions(+), 30 deletions(-) delete mode 100644 test/jdk/java/util/jar/JarEntry/test.jar diff --git a/test/jdk/java/util/jar/JarEntry/GetMethodsReturnClones.java b/test/jdk/java/util/jar/JarEntry/GetMethodsReturnClones.java index a9b35220de3..d2ab41cb831 100644 --- a/test/jdk/java/util/jar/JarEntry/GetMethodsReturnClones.java +++ b/test/jdk/java/util/jar/JarEntry/GetMethodsReturnClones.java @@ -21,72 +21,163 @@ * questions. */ +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.security.CodeSigner; +import java.security.KeyStore; +import java.security.cert.Certificate; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Enumeration; +import java.util.List; +import java.util.concurrent.TimeUnit; +import java.util.jar.JarEntry; +import java.util.jar.JarFile; +import java.util.jar.JarOutputStream; +import java.util.zip.ZipFile; + +import jdk.security.jarsigner.JarSigner; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import sun.security.tools.keytool.CertAndKeyGen; +import sun.security.x509.X500Name; +import static java.nio.charset.StandardCharsets.US_ASCII; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; + /* * @test * @bug 6337925 * @summary Ensure that callers cannot modify the internal JarEntry cert and * codesigner arrays. + * @modules java.base/sun.security.tools.keytool + * java.base/sun.security.x509 * @run junit GetMethodsReturnClones */ -import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Test; +class GetMethodsReturnClones { -import java.io.IOException; -import java.io.InputStream; -import java.security.CodeSigner; -import java.security.cert.Certificate; -import java.util.*; -import java.util.jar.*; -import static org.junit.jupiter.api.Assertions.assertNotNull; + private static final String ENTRY_NAME = "foobar.txt"; + private static final String SIGNER_NAME = "DUMMY"; + private static final String DIGEST_ALGORITHM = "SHA-256"; + private static final String SIGNATURE_ALGORITHM = "SHA256withRSA"; + private static final String KEY_TYPE = "RSA"; -public class GetMethodsReturnClones { - - private static final String BASE = System.getProperty("test.src", ".") + - System.getProperty("file.separator"); private static List jarEntries; + /* + * Creates a signed JAR file and initializes the "jarEntries" + */ @BeforeAll() - static void setupEntries() throws IOException { + static void setupJarEntries() throws Exception { + Path unsigned = createJar(); + Path signed = signJar(unsigned); + System.err.println("created signed JAR file at " + signed.toAbsolutePath()); List entries = new ArrayList<>(); - try (JarFile jf = new JarFile(BASE + "test.jar", true)) { - byte[] buffer = new byte[8192]; + try (JarFile jf = new JarFile(signed.toFile(), true)) { Enumeration e = jf.entries(); while (e.hasMoreElements()) { JarEntry je = e.nextElement(); entries.add(je); try (InputStream is = jf.getInputStream(je)) { - while (is.read(buffer, 0, buffer.length) != -1) { - // we just read. this will throw a SecurityException - // if a signature/digest check fails. - } + // we just read. this will throw a SecurityException + // if a signature/digest check fails. + var _ = is.readAllBytes(); } } } jarEntries = entries; } + /* + * For entries in the signed JAR file, this test verifies that if a non-null + * array is returned by the JarEntry.getCertificates() method, then any subsequent + * updates to that returned array do not propagate back to the original array. + */ @Test - void certsTest() { + void testCertificatesArray() { for (JarEntry je : jarEntries) { Certificate[] certs = je.getCertificates(); - if (certs != null) { - certs[0] = null; - certs = je.getCertificates(); - assertNotNull(certs[0], "Modified internal certs array"); + System.err.println("Certificates for " + je.getName() + " " + Arrays.toString(certs)); + if (isSignatureRelated(je)) { + // we don't expect this entry to be signed + assertNull(certs, "JarEntry.getCertificates() returned non-null for " + je.getName()); + continue; } + assertNotNull(certs, "JarEntry.getCertificates() returned null for " + je.getName()); + assertNotNull(certs[0], "Certificate is null"); + + certs[0] = null; // intentionally update the returned array + certs = je.getCertificates(); // now get the certs again + assertNotNull(certs, "JarEntry.getCertificates() returned null for " + je.getName()); + // verify that the newly returned array doesn't have the overwritten value + assertNotNull(certs[0], "Internal certificates array was modified"); } } + /* + * For entries in the signed JAR file, this test verifies that if a non-null + * array is returned by the JarEntry.getCodeSigners() method, then any subsequent + * updates to that returned array do not propagate back to the original array. + */ @Test - void signersTest() { + void testCodeSignersArray() { for (JarEntry je : jarEntries) { CodeSigner[] signers = je.getCodeSigners(); - if (signers != null) { - signers[0] = null; - signers = je.getCodeSigners(); - assertNotNull(signers[0], "Modified internal codesigners array"); + System.err.println("CodeSigners for " + je.getName() + " " + Arrays.toString(signers)); + if (isSignatureRelated(je)) { + // we don't expect this entry to be signed + assertNull(signers, "JarEntry.getCodeSigners() returned non-null for " + je.getName()); + continue; } + assertNotNull(signers, "JarEntry.getCodeSigners() returned null for " + je.getName()); + assertNotNull(signers[0], "CodeSigner is null"); + + signers[0] = null; // intentionally update the array + signers = je.getCodeSigners(); // now get the codesigners again + assertNotNull(signers, "JarEntry.getCodeSigners() returned null for " + je.getName()); + // verify that the newly returned array doesn't have the overwritten value + assertNotNull(signers[0], "CodeSigner is null"); } } + + private static Path createJar() throws IOException { + final Path unsigned = Path.of("unsigned.jar"); + try (JarOutputStream out = new JarOutputStream(Files.newOutputStream(unsigned))) { + out.putNextEntry(new JarEntry(ENTRY_NAME)); + out.write("hello world".getBytes(US_ASCII)); + } + return unsigned; + } + + private static Path signJar(final Path unsigned) throws Exception { + final Path signed = Path.of("signed.jar"); + final JarSigner signer = new JarSigner.Builder(privateKeyEntry()) + .signerName(SIGNER_NAME) + .digestAlgorithm(DIGEST_ALGORITHM) + .signatureAlgorithm(SIGNATURE_ALGORITHM) + .build(); + try (ZipFile zip = new ZipFile(unsigned.toFile()); + OutputStream out = Files.newOutputStream(signed)) { + signer.sign(zip, out); + } + return signed; + } + + private static KeyStore.PrivateKeyEntry privateKeyEntry() throws Exception { + final CertAndKeyGen gen = new CertAndKeyGen(KEY_TYPE, SIGNATURE_ALGORITHM); + gen.generate(4096); + final long oneDayInSecs = TimeUnit.SECONDS.convert(1, TimeUnit.DAYS); + Certificate cert = gen.getSelfCertificate(new X500Name("cn=duke"), oneDayInSecs); + return new KeyStore.PrivateKeyEntry(gen.getPrivateKey(), new Certificate[] {cert}); + } + + private static boolean isSignatureRelated(final JarEntry entry) { + final String entryName = entry.getName(); + return entryName.equals("META-INF/" + SIGNER_NAME + ".SF") + || entryName.equals("META-INF/" + SIGNER_NAME + ".RSA"); + } } diff --git a/test/jdk/java/util/jar/JarEntry/test.jar b/test/jdk/java/util/jar/JarEntry/test.jar deleted file mode 100644 index fb9e4c1734966f2a72279b65426359002e48c81a..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 3022 zcmaKuXH-+!0)<2Gf&!s~g5c1J2!ervcdc{Rx%=F+?~m_pV+aK$8-SKHMv`=70H?%8nj2`E zgC%thwWJNehPqmsX68}`TI~IG^c^xRfMMk{7_LJ7PBpD*UdXiBfV zlWjHuqtfJ`O0!n_NYq<^TZj6?x)yvgSR03sGiC^{U&c7kM#YOW*qFpc6V+$hga(x$R=H`bD}y#2pv z&;D0hQ!}uvr?afb+?e;4=iy)%gpL4Pe%}bgF)Rl)k=1Sa0|^*Luf$nR>trSI!`wzGGra;_}2gnsWpN}^K-g-ABemkK7d!RL5U2=9A>rU?dv=$7BV7wL#$Ih zp14O>*s)6z+@^o4G+UQ_>YHf4JrOgo%mg&Gxy)n!4CzVbGcE9Gxus<4gF~I?aruCL zKZ+5B6lUyX)R1Ua z2)pu`!75`{KC=pPbeJiR@kGDet1Wg~Z?Y>*@_CN`c;u^C|2cNj@<<_Mcl(21jhf<@ z5j}Hq*!$;^lCONvv(V_$WQKTsnDZ8~50x?BG zLXewVkdS;n6SKT@MLj)=AY#tKMrjih!*NVOoxUp8_S$N1XF=1N5o^7!A}bFxVB$`w z(;E&c`w0TCM|w*vboTo|q9RPJO)r`d^KENEdo77SLL7kzJ<5a~&OFLk5uPrDL-=EV z$!WIh4oH1NXOgS5xO0n7)}9w?Drd4beyiq-Gok=*Y4|#{7NJrdG3!>H;zbo85>91x zAhxSE@-QV1IKJIYdxpl^iKPI0n@I-*y6mWXH^u0RI?_#LTWZ)!bY&mQf1Y;@JJ6zp zGr#H#*Gg~`CkWExJSFAOxc# zSZ%`ER=X%X%G|6<`d?V>JY1=Y*(;iq8+S-d(fyj+wpI|hb%@@S<}-2_R1&dou*I1> zFOoe_5n$Ho)A2AEnFPT$U$p7J;Mld`!#oMy$|sI7@1`b6MV6Y_zw;lO>A*KM1rO{K zIDp|-@qIh?pSCrZg-F<>q0LT^aNwo{0F(S@RzY4NZdzWxZsu;mAyO{BPQk&+7JgzPoW!#B+RIwR@tpu5_N=vQ>0 z1|llAS227I{b2F>;{5y??0Apif*t(vY*EsX#iSU<)Wq1t{FZGO1^X8Ae@!>?~D~uPOIdN9nb4D0pX`l6u9*)f4^vW1! z>9UAyy4ZE672o4BRDb7d(*wN-Nr_~DJSnEL_54XLlV(FC@u zp}dZ^J$ctxoiuwy{E5wpyx329UI?bPp!0Y>s4SnPgaJBY z2aA`iVpUXeM`2lOQl{9XJl3w9C7;D!fg)qd7wotsP9M@od zc;|_*n0&LrgJuY7DeM#1lPG9Wy9_V@uFPTT?z#;vW-7w%vn^FrH^ombbKOy*)NK|dq{LtFwGqjE zf{yJ@ceaV#2+91!9d4R9opdN@Coz_N#*1OsPLj zl4<$sbu!*n8S-3xmxC2v>9wGVtYCxfFeQ8g?m3~Mao+1@gz&?I*V}Dn6=1+O*{mkf zNu9-AjoUX3Y+BWZvm|ag31n<^?GWBXQYX&x(Tpohq?I%j?t3z;8@v>ra&Wqb0p0f# zbtu5O=ePy~R9ie>1Dp;<&cq!JN6}uX&QTrRInO0HEn%q(Wo>16jNA@=d?+k34opUK zFT8@rp=Dkg%4{*drc*CcpJlrnjUa%`dhm2*=ZaYM`7;Hui5JTY(^j;ZCEIEJnn5tj*`8-6KJv4!$d@dh z(p2|RhNIP$>|rTiORadotI}#3^XD)<%;v>o8eBxex`3#-2BnPNn1pGsi^dS8wf^Wa zMnf|1%?aWcH$z|Z=5y3n1}&%f*6%BV5q?>n3>Nhihcr*Nd{UpxF!1OMqh zO~R8R^;3;x|8$dT)oJ^mxcrlaC%F76W>VO{UVZ}4)7BGMp8hH)u>2`~lJ#HEX$+wv SEdT&iq@hGQTrB;`-G2b2MhxQs