Skip to content

Commit 7686e87

Browse files
coffeyscoffeys
committed
8250968: Symlinks attributes not preserved when using jarsigner on zip files
Reviewed-by: lancea, weijun, hchao
1 parent 8d6d43c commit 7686e87

File tree

10 files changed

+199
-38
lines changed

10 files changed

+199
-38
lines changed

‎src/java.base/share/classes/java/util/zip/ZipEntry.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ public class ZipEntry implements ZipConstants, Cloneable {
5757
int flag = 0; // general purpose flag
5858
byte[] extra; // optional extra field data for entry
5959
String comment; // optional comment string for entry
60-
int posixPerms = -1;// posix permissions
60+
int extraAttributes = -1; // e.g. POSIX permissions, sym links.
6161
/**
6262
* Compression method for uncompressed entries.
6363
*/
@@ -131,7 +131,7 @@ public ZipEntry(ZipEntry e) {
131131
flag = e.flag;
132132
extra = e.extra;
133133
comment = e.comment;
134-
posixPerms = e.posixPerms;
134+
extraAttributes = e.extraAttributes;
135135
}
136136

137137
/**

‎src/java.base/share/classes/java/util/zip/ZipFile.java‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -658,8 +658,8 @@ private ZipEntry getZipEntry(String name, int pos) {
658658
e.csize = CENSIZ(cen, pos);
659659
e.method = CENHOW(cen, pos);
660660
if (CENVEM_FA(cen, pos) == FILE_ATTRIBUTES_UNIX) {
661-
// 12 bits for setuid, setgid, sticky + perms
662-
e.posixPerms = CENATX_PERMS(cen, pos) & 0xFFF;
661+
// read all bits in this field, including sym link attributes
662+
e.extraAttributes = CENATX_PERMS(cen, pos) & 0xFFFF;
663663
}
664664

665665
if (elen != 0) {
@@ -1096,14 +1096,13 @@ public Stream<JarEntry> stream(ZipFile zip) {
10961096
public Stream<String> entryNameStream(ZipFile zip) {
10971097
return zip.entryNameStream();
10981098
}
1099-
// only set posix perms value via ZipEntry contructor for now
11001099
@Override
1101-
public int getPosixPerms(ZipEntry ze) {
1102-
return ze.posixPerms;
1100+
public int getExtraAttributes(ZipEntry ze) {
1101+
return ze.extraAttributes;
11031102
}
11041103
@Override
1105-
public void setPosixPerms(ZipEntry ze, int perms) {
1106-
ze.posixPerms = perms;
1104+
public void setExtraAttributes(ZipEntry ze, int extraAttrs) {
1105+
ze.extraAttributes = extraAttrs;
11071106
}
11081107

11091108
}

‎src/java.base/share/classes/java/util/zip/ZipOutputStream.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 1996, 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 1996, 2020, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -511,7 +511,7 @@ private void writeEXT(ZipEntry e) throws IOException {
511511
* to a version value.
512512
*/
513513
private int versionMadeBy(ZipEntry e, int version) {
514-
return (e.posixPerms < 0) ? version :
514+
return (e.extraAttributes < 0) ? version :
515515
VERSION_MADE_BY_BASE_UNIX | (version & 0xff);
516516
}
517517

@@ -606,8 +606,8 @@ private void writeCEN(XEntry xentry) throws IOException {
606606
}
607607
writeShort(0); // starting disk number
608608
writeShort(0); // internal file attributes (unused)
609-
// external file attributes, used for storing posix permissions
610-
writeInt(e.posixPerms > 0 ? e.posixPerms << 16 : 0);
609+
// extra file attributes, used for storing posix permissions etc.
610+
writeInt(e.extraAttributes > 0 ? e.extraAttributes << 16 : 0);
611611
writeInt(offset); // relative offset of local header
612612
writeBytes(nameBytes, 0, nameBytes.length);
613613

‎src/java.base/share/classes/jdk/internal/access/JavaUtilZipFileAccess.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public interface JavaUtilZipFileAccess {
4141
public Enumeration<JarEntry> entries(ZipFile zip);
4242
public Stream<JarEntry> stream(ZipFile zip);
4343
public Stream<String> entryNameStream(ZipFile zip);
44-
public void setPosixPerms(ZipEntry ze, int posixPerms);
45-
public int getPosixPerms(ZipEntry ze);
44+
public void setExtraAttributes(ZipEntry ze, int extraAttrs);
45+
public int getExtraAttributes(ZipEntry ze);
4646
}
4747

‎src/java.base/share/classes/sun/security/util/Event.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ private Event() {}
3737

3838
public enum ReporterCategory {
3939
CRLCHECK(),
40-
POSIXPERMS();
40+
ZIPFILEATTRS();
4141

4242
private Reporter reporter;
4343
}

‎src/jdk.jartool/share/classes/jdk/security/jarsigner/JarSigner.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2015, 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2015, 2020, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -505,7 +505,7 @@ public JarSigner build() {
505505
private final boolean externalSF; // leave the .SF out of the PKCS7 block
506506
private final String altSignerPath;
507507
private final String altSigner;
508-
private boolean posixPermsDetected;
508+
private boolean extraAttrsDetected;
509509

510510
private JarSigner(JarSigner.Builder builder) {
511511

@@ -949,12 +949,12 @@ private void writeEntry(ZipFile zf, ZipOutputStream os, ZipEntry ze)
949949
ze2.setTime(ze.getTime());
950950
ze2.setComment(ze.getComment());
951951
ze2.setExtra(ze.getExtra());
952-
int perms = JUZFA.getPosixPerms(ze);
953-
if (!posixPermsDetected && perms != -1) {
954-
posixPermsDetected = true;
955-
Event.report(Event.ReporterCategory.POSIXPERMS, "detected");
952+
int extraAttrs = JUZFA.getExtraAttributes(ze);
953+
if (!extraAttrsDetected && extraAttrs != -1) {
954+
extraAttrsDetected = true;
955+
Event.report(Event.ReporterCategory.ZIPFILEATTRS, "detected");
956956
}
957-
JUZFA.setPosixPerms(ze2, perms);
957+
JUZFA.setExtraAttributes(ze2, extraAttrs);
958958
if (ze.getMethod() == ZipEntry.STORED) {
959959
ze2.setSize(ze.getSize());
960960
ze2.setCrc(ze.getCrc());

‎src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ public class Main {
110110
private static final Set<CryptoPrimitive> SIG_PRIMITIVE_SET = Collections
111111
.unmodifiableSet(EnumSet.of(CryptoPrimitive.SIGNATURE));
112112

113-
private static boolean permsDetected;
113+
private static boolean extraAttrsDetected;
114114

115115
static final String VERSION = "1.0";
116116

@@ -782,8 +782,8 @@ void verifyJar(String jarName)
782782
JarEntry je = e.nextElement();
783783
String name = je.getName();
784784

785-
if (!permsDetected && JUZFA.getPosixPerms(je) != -1) {
786-
permsDetected = true;
785+
if (!extraAttrsDetected && JUZFA.getExtraAttributes(je) != -1) {
786+
extraAttrsDetected = true;
787787
}
788788
hasSignature = hasSignature
789789
|| SignatureFileVerifier.isBlockOrSF(name);
@@ -1247,8 +1247,8 @@ private void displayMessagesAndResult(boolean isSigning) {
12471247
}
12481248
}
12491249

1250-
if (permsDetected) {
1251-
warnings.add(rb.getString("posix.attributes.detected"));
1250+
if (extraAttrsDetected) {
1251+
warnings.add(rb.getString("extra.attributes.detected"));
12521252
}
12531253

12541254
if ((strict) && (!errors.isEmpty())) {
@@ -1777,8 +1777,8 @@ void signJar(String jarName, String alias)
17771777
String failedMessage = null;
17781778

17791779
try {
1780-
Event.setReportListener(Event.ReporterCategory.POSIXPERMS,
1781-
(t, o) -> permsDetected = true);
1780+
Event.setReportListener(Event.ReporterCategory.ZIPFILEATTRS,
1781+
(t, o) -> extraAttrsDetected = true);
17821782
builder.build().sign(zipFile, fos);
17831783
} catch (JarSignerException e) {
17841784
failedCause = e.getCause();
@@ -1813,7 +1813,7 @@ void signJar(String jarName, String alias)
18131813
fos.close();
18141814
}
18151815

1816-
Event.clearReportListener(Event.ReporterCategory.POSIXPERMS);
1816+
Event.clearReportListener(Event.ReporterCategory.ZIPFILEATTRS);
18171817
}
18181818

18191819
if (failedCause != null) {

‎src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Resources.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ public class Resources extends java.util.ListResourceBundle {
170170
{"key.bit.weak", "%d-bit key (weak)"},
171171
{"key.bit.disabled", "%d-bit key (disabled)"},
172172
{"unknown.size", "unknown size"},
173-
{"posix.attributes.detected", "POSIX file permission attributes detected. These attributes are ignored when signing and are not protected by the signature."},
173+
{"extra.attributes.detected", "POSIX file permission and/or symlink attributes detected. These attributes are ignored when signing and are not protected by the signature."},
174174

175175
{"jarsigner.", "jarsigner: "},
176176
{"signature.filename.must.consist.of.the.following.characters.A.Z.0.9.or.",

‎test/jdk/sun/security/tools/jarsigner/PosixPermissionsTest.java‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,9 @@ public class PosixPermissionsTest {
7272
private static int count;
7373
private static Set<PosixFilePermission> permsSet;
7474
private static String expectedJarPerms;
75-
private static final String POSIXWARNING = "POSIX file permission attributes detected. " +
76-
"These attributes are ignored when signing and are not protected by the signature.";
75+
private static final String WARNING_MSG = "POSIX file permission and/or symlink " +
76+
"attributes detected. These attributes are ignored when signing and are not " +
77+
"protected by the signature.";
7778

7879
public static void main(String[] args) throws Exception {
7980
if (!FileSystems.getDefault().supportedFileAttributeViews().contains("posix")) {
@@ -104,7 +105,7 @@ public static void main(String[] args) throws Exception {
104105
"-keypass", "password",
105106
"examplekey")
106107
.shouldHaveExitValue(0)
107-
.shouldContain(POSIXWARNING);
108+
.shouldContain(WARNING_MSG);
108109

109110
// zip file now signed. Recheck file permissions
110111
verifyFilePermissions(ZIPURI, true);
@@ -116,7 +117,7 @@ public static void main(String[] args) throws Exception {
116117
"-keypass", "password",
117118
"examplekey")
118119
.shouldHaveExitValue(0)
119-
.shouldNotContain(POSIXWARNING);
120+
.shouldNotContain(WARNING_MSG);
120121

121122
// default attributes expected
122123
verifyFilePermissions(JARURI, false);
@@ -127,7 +128,7 @@ public static void main(String[] args) throws Exception {
127128
"-verbose",
128129
"-verify", ZIPFILENAME)
129130
.shouldHaveExitValue(0)
130-
.shouldContain(POSIXWARNING);
131+
.shouldContain(WARNING_MSG);
131132

132133
// no warning expected for regular jar file
133134
SecurityTools.jarsigner("-keystore", "examplekeystore",
@@ -136,7 +137,7 @@ public static void main(String[] args) throws Exception {
136137
"-verbose",
137138
"-verify", JARFILENAME)
138139
.shouldHaveExitValue(0)
139-
.shouldNotContain(POSIXWARNING);
140+
.shouldNotContain(WARNING_MSG);
140141
}
141142

142143
private static void createFiles() throws Exception {

‎test/jdk/sun/security/tools/jarsigner/SymLinkTest.java‎

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
/*
2+
* Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
/**
25+
* @test
26+
* @bug 8250968
27+
* @summary Symlinks attributes not preserved when using jarsigner on zip files
28+
* @modules jdk.jartool/sun.security.tools.jarsigner
29+
* java.base/sun.security.tools.keytool
30+
* @library /test/lib
31+
* @run main/othervm SymLinkTest
32+
*/
33+
34+
import java.io.*;
35+
import java.net.URI;
36+
import java.nio.file.*;
37+
import java.util.Formatter;
38+
39+
import jdk.test.lib.SecurityTools;
40+
41+
public class SymLinkTest {
42+
private final static String ZIPFILENAME = "8250968-test.zip";
43+
private static final String WARNING_MSG = "POSIX file permission and/or symlink " +
44+
"attributes detected. These attributes are ignored when signing and are not " +
45+
"protected by the signature.";
46+
47+
public static void main(String[] args) throws Exception {
48+
Files.deleteIfExists(Paths.get(ZIPFILENAME));
49+
try (FileOutputStream fos = new FileOutputStream(ZIPFILENAME)) {
50+
fos.write(ZIPBYTES);
51+
}
52+
53+
// check permissions before signing
54+
verifyExtraAttrs(ZIPFILENAME);
55+
56+
SecurityTools.keytool(
57+
"-genkey",
58+
"-keyalg", "RSA",
59+
"-dname", "CN=Coffey, OU=JPG, O=Oracle, L=Santa Clara, ST=California, C=US",
60+
"-alias", "examplekey",
61+
"-storepass", "password",
62+
"-keypass", "password",
63+
"-keystore", "examplekeystore",
64+
"-validity", "365")
65+
.shouldHaveExitValue(0);
66+
67+
SecurityTools.jarsigner(
68+
"-keystore", "examplekeystore",
69+
"-verbose", ZIPFILENAME,
70+
"-storepass", "password",
71+
"-keypass", "password",
72+
"examplekey")
73+
.shouldHaveExitValue(0)
74+
.shouldContain(WARNING_MSG);
75+
76+
// zip file now signed. Recheck attributes
77+
verifyExtraAttrs(ZIPFILENAME);
78+
79+
SecurityTools.jarsigner("-keystore", "examplekeystore",
80+
"-storepass", "password",
81+
"-keypass", "password",
82+
"-verbose",
83+
"-verify", ZIPFILENAME)
84+
.shouldHaveExitValue(0)
85+
.shouldContain(WARNING_MSG);
86+
}
87+
88+
private static void verifyExtraAttrs(String zipFileName) throws IOException {
89+
// the 16 bit extra attributes value should equal 0xa1ff - look for that pattern.
90+
// Such values can be read from zip file via 'unzip -Z -l -v <zipfile>'
91+
try (FileInputStream fis = new FileInputStream(ZIPFILENAME)) {
92+
byte[] b = fis.readAllBytes();
93+
boolean patternFound;
94+
for (int i = 0; i < b.length -1; i++) {
95+
patternFound = ((b[i] & 0xFF) == 0xFF) && ((b[i + 1] & 0xFF) == 0xA1);
96+
if (patternFound) {
97+
return;
98+
}
99+
}
100+
throw new RuntimeException("extra attribute value not detected");
101+
}
102+
}
103+
104+
/**
105+
* Utility method which takes an byte array and converts to byte array
106+
* declaration. For example:
107+
* <pre>
108+
* {@code
109+
* var fooJar = Files.readAllBytes(Path.of("foo.jar"));
110+
* var result = createByteArray(fooJar, "FOOBYTES");
111+
* }
112+
* </pre>
113+
* @param bytes A byte array used to create a byte array declaration
114+
* @param name Name to be used in the byte array declaration
115+
* @return The formatted byte array declaration
116+
*/
117+
public static String createByteArray(byte[] bytes, String name) {
118+
StringBuilder sb = new StringBuilder(bytes.length * 5);
119+
Formatter fmt = new Formatter(sb);
120+
fmt.format(" public static byte[] %s = {", name);
121+
final int linelen = 8;
122+
for (int i = 0; i < bytes.length; i++) {
123+
if (i % linelen == 0) {
124+
fmt.format("%n ");
125+
}
126+
fmt.format(" (byte) 0x%x,", bytes[i] & 0xff);
127+
}
128+
fmt.format("%n };%n");
129+
return sb.toString();
130+
}
131+
132+
/*
133+
* Created using the createByteArray utility method.
134+
* The zipfile itself was created via this example:
135+
* $ ls -l z
136+
* lrwxrwxrwx 1 test test 4 Aug 27 18:33 z -> ../z
137+
* $ zip -ry test.zip z
138+
*/
139+
public final static byte[] ZIPBYTES = {
140+
(byte) 0x50, (byte) 0x4b, (byte) 0x3, (byte) 0x4, (byte) 0xa, (byte) 0x0, (byte) 0x0, (byte) 0x0,
141+
(byte) 0x0, (byte) 0x0, (byte) 0x2e, (byte) 0x94, (byte) 0x1b, (byte) 0x51, (byte) 0xb4, (byte) 0xcc,
142+
(byte) 0xb6, (byte) 0xf1, (byte) 0x4, (byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x4, (byte) 0x0,
143+
(byte) 0x0, (byte) 0x0, (byte) 0x1, (byte) 0x0, (byte) 0x1c, (byte) 0x0, (byte) 0x7a, (byte) 0x55,
144+
(byte) 0x54, (byte) 0x9, (byte) 0x0, (byte) 0x3, (byte) 0x77, (byte) 0xfc, (byte) 0x47, (byte) 0x5f,
145+
(byte) 0x78, (byte) 0xfc, (byte) 0x47, (byte) 0x5f, (byte) 0x75, (byte) 0x78, (byte) 0xb, (byte) 0x0,
146+
(byte) 0x1, (byte) 0x4, (byte) 0xec, (byte) 0x3, (byte) 0x0, (byte) 0x0, (byte) 0x4, (byte) 0xec,
147+
(byte) 0x3, (byte) 0x0, (byte) 0x0, (byte) 0x2e, (byte) 0x2e, (byte) 0x2f, (byte) 0x7a, (byte) 0x50,
148+
(byte) 0x4b, (byte) 0x1, (byte) 0x2, (byte) 0x1e, (byte) 0x3, (byte) 0xa, (byte) 0x0, (byte) 0x0,
149+
(byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x2e, (byte) 0x94, (byte) 0x1b, (byte) 0x51, (byte) 0xb4,
150+
(byte) 0xcc, (byte) 0xb6, (byte) 0xf1, (byte) 0x4, (byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x4,
151+
(byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x1, (byte) 0x0, (byte) 0x18, (byte) 0x0, (byte) 0x0,
152+
(byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0xff,
153+
(byte) 0xa1, (byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x7a, (byte) 0x55, (byte) 0x54,
154+
(byte) 0x5, (byte) 0x0, (byte) 0x3, (byte) 0x77, (byte) 0xfc, (byte) 0x47, (byte) 0x5f, (byte) 0x75,
155+
(byte) 0x78, (byte) 0xb, (byte) 0x0, (byte) 0x1, (byte) 0x4, (byte) 0xec, (byte) 0x3, (byte) 0x0,
156+
(byte) 0x0, (byte) 0x4, (byte) 0xec, (byte) 0x3, (byte) 0x0, (byte) 0x0, (byte) 0x50, (byte) 0x4b,
157+
(byte) 0x5, (byte) 0x6, (byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x1, (byte) 0x0,
158+
(byte) 0x1, (byte) 0x0, (byte) 0x47, (byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x3f, (byte) 0x0,
159+
(byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x0,
160+
};
161+
}

0 commit comments

Comments
 (0)