Hi Philipp,
thanks for the reply .. comments inline
On 19/01/20 16:09, Philipp Kunz wrote:
Hi Sean,
I figure that distinguishing zips and jars is ambiguous in a certain
way. After having signed a zip, it contains a manifest and is also a
jar file according to the specs. That would mean we would end up with
two kinds of jar files, those with permissions and those without,
depending on the file name extension of the files. That a tool named
jarsigner is successfully applied to a file is for me convincing enough
to consider that file a jar file or jarsigned would reject it however
it name ends.
Yes, both zip and jar files enjoy the same format. jar files being created
predominantly with the java based API/tools.
Now when signing jar files (namely those with zip file name extension
but depending on the further discussion not necessary limited to those)
with permissions, I wonder if those permissions should also be subject
of the signature. I would consider a change of an executable flag for
example a change of the signed file just the same as a changed byte of
contents. The way jarsigner works now, loosing the permissions, we know
at least that the files contained in signed jars have certain
permission flags (none or defaults) immediately after having signed
them even though later manipulation would not be detected and, hence,
cannot be trusted even with a valid signature.
historically, jar tool never recorded these permissions. As such, it
should make
no difference to how jarsigner operates on such files. The original report
highlights the loss of data on files created with the zip utility. It
think it's reasonable
for jarsigner to preserve attributes in such a case.
Far as I understand the current situation, jars and zips could be told
from one another only before signing depending on the presence or
absence of a manifest. After signing, however, a manifest is always
there. Looking at it that way, the "much more consideration" you
mention would have to take place now, wouldn't it?
I'm not sure if there's much change in this area post fix. The classic
jar tool doesn't add or read the file attributes at hand. The name check
was useful to prevent any unintended behavioural changes in this area.
I also wonder why the "if (filename.endsWith(".zip"))" piece of code
does not occur only in JarSigner. Why is it in ZipFile, too? One such
kind of if statement should have been enough, at first glance. There
may be a good and convincing explanation but I don't see it just like
that. On the other hand, having this if condition duplicated into
ZipFile looks like it could have an undesired side-effect elsewhere. I
also kind of miss another test case to make sure permission flags
continue to be disregarded for "non-zip" jar files.
fair point. I should be able to remove this code. I can also improve the
test case.
And the comment of Michael Osipov is really frightening. Did you know
that i and I are two different characters in turkish both of which have
yet other capital/small counterparts? At that point maybe discussing
the file name extension distinction could easily become lengthier than
a discussion about consequences of supporting permissions for all jar
files.
frightening ?? Yes, I've worked on such issues before. Thanks to Michael
for pointing that out. I should be able to do a comparison using Locale.ROOT
You mention "the case, at hand is unique here" and that made to occur
to me that if there is maybe only one person affected that he or she
could work around it more easily than changing the jdk. Is it really
important for most others? And is it really a bug at all?
Another idea to work around compatibility issues would be to introduce
a flag for jarsigner, for example -p like with tar. Yet another idea
could be to refire the jar file specs and jar and jarsigner tools docs
accordingly.
Let me have a think about this. A new flag in jarsigner may help.
regards,
Sean.
Regards,
Philipp
On Fri, 2020-01-17 at 13:07 +0000, Seán Coffey wrote:
Hi Philipp,
On 17/01/2020 12:40, Philipp Kunz wrote:
Hi Sean,
Nice patch. I wonder why permissions should be preserved only in
zip
files. Jar files also are zip files, according to the jar file
specs,
and hence, shouldn't jar files benefit of preserving permissions,
too?
Thanks for your comments. The jar tool has never been interested in
the
posix
permissions fields for the individual entries. Such a change could
yield
more
interoperability issues. Such a change would also need much more
consideration
The zip tool on the other hand has always populated this field and I
think the case
at hand is unique here (preserving attributes already created by
non-java tools)
The file name extension is most often zip for zip files and jar for
jar
files but is that really a safe assumption? I would not expect it
always. Removing
Yes, I didn't see any easy way to distinguish a zip file from a jar
file
without being
more invasive and scanning file attributes for that file. I could
take
that approach
if it's deemed necessary.
regards,
Sean.
if (zf.getName().toLowerCase().endsWith(".zip")) {
along with similar code in ZipFile would avoid discussing that
question
and the test would not have to check that files with another name
extension than zip don't preserve permissions.
Philipp
On Fri, 2020-01-17 at 10:59 +0000, Seán Coffey wrote:
Hi,
Looking to introduce some JDK private functionality which will
help
preserve internal zip file attribute permissions when jarsigner
is
run
on a zip file. Some of the logic is taken from the recent work
carried
out in this area for zipfs API.
https://bugs.openjdk.java.net/browse/JDK-8218021
http://cr.openjdk.java.net/~coffeys/webrev.8218021/webrev/
regards,
Sean.
