Re: Code review request: 7180907: Jarsigner -verify fails if rsa file used sha-256 with authenticated attributes
On 7/6/2012 1:03 PM, Weijun Wang wrote: Hi All I have two fixes for this bug: For 7u6: http://cr.openjdk.java.net/~weijun/7180907/7u/webrev.00/ Looks fine to me, except a very minor copyright date: you may want to use 2012 for SignerInfo.java. This simply makes the name recognizable. It's safe and I don't want anything broken in 7u6. For 8: http://cr.openjdk.java.net/~weijun/7180907/webrev.00/ Looks fine to me. Xuelei This changes the internal name tables of AlgorithmId to match with the Standard Names doc [1]. I've searched thru all codes inside JDK that calls the AlgorithmId.getName() and made some trivial changes. Both using the same regression test. JPRT for jdk8 on the way. Thanks Max [1] http://docs.oracle.com/javase/7/docs/technotes/guides/security/StandardNames.html Original Message === *Description* SHORT SUMMARY: If a signature block (.RSA, a PKCS#7 object) contains authenticated attributes and uses a SHA-256 digest, verification will fail. The digest algorithm is stored in the PKCS7 using the correct OID (2.16.840.1.101.3.4.2.1) but sun.security.x509.AlgorithmId maps this back to an algorithm with name SHA256. This is not a valid MessageDigest name - the correct version is SHA-256. The debug output from: jarsigner -J-Djava.security.debug=all -verbose -verify i3.jar debug.txt and i3.jar available here: ftp://bugftp.us.oracle.com/upload/bug_13/bug13941476 INDICATORS: COUNTER INDICATORS: TRIGGERS: KNOWN WORKAROUND: PRESENT SINCE: N/A HOW TO VERIFY: Run attached test case NOTES FOR SE: None REGRESSION:
Re: Code review request: 7163483 JarSigner -verify -verbose does not format date string according to locale
Hello Max, I's been a long time since my last mail, I did some investigation and had some discussion with i18n developers, but still did not see a nice solution for the alignment problem. There does not seem be an existing API to do this job in JDK scope. So I implemented a simple format function, and use it to format under different locales. http://cr.openjdk.java.net/~luchsh/7163483_3/ The patch is trying to format the code in the same way as java.util.Date.toString() in the format of EEE MMM dd HH:mm:ss zzz , except for using names of month and DOW in localized format. So far, it works good for me under all supported locales. Here's a test case to verify the vertical alignment, which I has been posted to i18n mailing list before, http://cr.openjdk.java.net/~luchsh/VerticalAlignmentTest.java It may still fail under vi_VN locale with this solution due to test case limit, but I do not think it is a real failure since the result fields still get aligned except for multiple words in one field. Could you please take a look at the patch? Many thanks best regards Jonathan On 04/25/2012 07:48 PM, Weijun Wang wrote: Hi Jonathan I'm using English. In your test all the files have a similar modified time so you cannot see the difference. However, in my example, you can see that the widths for date and hour are not zero-padded so the width can be either 1 or 2. French is even worse smk 76 10 nov. 2009 08:57:54 bin/vbin/go smk 1149 8 avr. 2012 16:03:20 bin/vbin/netbeans smk 170 20 nov. 2009 16:47:42 bin/vbin/syncdown smk 671 8 févr. 2012 20:11:22 bin/vbin/ssh.desktop smk 187 20 nov. 2009 16:47:34 bin/vbin/syncsf So here even the width of month abbr can be different. Thanks Max On 04/25/2012 07:09 PM, Jonathan Lu wrote: Hello Max, Terribly sorry for my misunderstanding! On 04/25/2012 05:39 PM, Weijun Wang wrote: On 04/25/2012 05:23 PM, Jonathan Lu wrote: Hi Max, On 04/25/2012 05:12 PM, Weijun Wang wrote: On 04/25/2012 03:28 PM, Jonathan Lu wrote: Hi Weijun, Thanks for your time, I've updated the webrev, could you please take a look? http://cr.openjdk.java.net/~luchsh/7163483_2/ On 04/24/2012 03:06 PM, Weijun Wang wrote: Hi Jonathan Some comments: 1. Can you be sure that the new format always has the same length? jarsigner tries to output in a tabular style and each column should be aligned. I'm not sure of that, so the test case was updated to compare the first several tokens to determine whether there's any differences in the expression of date time. Sorry, I didn't make myself clear last time, I was mainly afraid of unaligned lines that make the output ugly. For example: smk 76 Nov 10, 2009 8:57:54 AM bin/vbin/go smk 1149 Apr 8, 2012 4:03:20 PM bin/vbin/netbeans smk 170 Nov 20, 2009 4:47:42 PM bin/vbin/syncdown smk 671 Feb 8, 2012 8:11:22 PM bin/vbin/ssh.desktop smk 187 Nov 20, 2009 4:47:34 PM bin/vbin/syncsf I think that would not be a problem in the new test case which compares tokenized strings splited by blank spaces instead of String#equals. Does that make sense? I'm not talking about the test. It's the output of jarsigner looking ugly. smk 76 Nov 10, 2009 8:57:54 AM bin/vbin/go smk 1149 Apr 8, 2012 4:03:20 PM bin/vbin/netbeans smk 170 Nov 20, 2009 4:47:42 PM bin/vbin/syncdown smk 671 Feb 8, 2012 8:11:22 PM bin/vbin/ssh.desktop smk 187 Nov 20, 2009 4:47:34 PM bin/vbin/syncsf Compare with the current output: smk 76 Tue Nov 10 08:57:54 CST 2009 bin/vbin/go smk 1149 Sun Apr 08 16:03:20 CST 2012 bin/vbin/netbeans smk 170 Fri Nov 20 16:47:42 CST 2009 bin/vbin/syncdown smk 671 Wed Feb 08 20:11:22 CST 2012 bin/vbin/ssh.desktop smk 187 Fri Nov 20 16:47:34 CST 2009 bin/vbin/syncsf I did not see unaligned format in my testing, did you get these unaligned output after applying the patch? From above lines, I see the starting indices of date string in each line are always the same, which is achieved by jarsigner, but the length of the date strings are not the same, which locale were you testing on? Thanks Max Thanks Max 2. You might need to reformat the modified line to make it fit into 80 characters width. 3. Why not include the test inside the changeset? 2, 3 were done in the new patch Thanks Max On 04/23/2012 05:46 PM, Jonathan Lu wrote: Hello security-dev, Here's a patch for bug 7163483, could anybody please help to take a look? http://cr.openjdk.java.net/~luchsh/7163483/ The problem is that command jarsigner -verify -verbose my.jar does not format date string according to current locale. following simple test case can be used to disclose this problem. /* * Copyright (c) 2012 Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it * under the terms of the GNU General Public License version 2 only, as * published by the Free Software Foundation. * * This code is
Time format in jar and jarsigner output (was Re: Code review request: 7163483 JarSigner -verify -verbose does not format date string according to locale)
Hi Jonathan I have these questions: 1. Why always CAPITAL letters for month and weekday? 2. Why always dd HH:mm:ss zzz for the rest? Some locales uses . instead of : as times delimiters. 3. Why always dd after MMM? Some locales prefer dd before MMM. Well, if you really think the current Fri Jul output is too English, instead of localizing the string, how about we de-localize it totally and choose a neutral format? There are several flavors of ISO date/time format at http://www.w3.org/TR/NOTE-datetime or we can just choose -MM-dd HH:mm:ss zzz BTW, the jar command is using the same format, therefore I'm adding core-libs-dev. Thanks Max On 07/06/2012 05:16 PM, Jonathan Lu wrote: Hello Max, I's been a long time since my last mail, I did some investigation and had some discussion with i18n developers, but still did not see a nice solution for the alignment problem. There does not seem be an existing API to do this job in JDK scope. So I implemented a simple format function, and use it to format under different locales. http://cr.openjdk.java.net/~luchsh/7163483_3/ The patch is trying to format the code in the same way as java.util.Date.toString() in the format of EEE MMM dd HH:mm:ss zzz , except for using names of month and DOW in localized format. So far, it works good for me under all supported locales. Here's a test case to verify the vertical alignment, which I has been posted to i18n mailing list before, http://cr.openjdk.java.net/~luchsh/VerticalAlignmentTest.java It may still fail under vi_VN locale with this solution due to test case limit, but I do not think it is a real failure since the result fields still get aligned except for multiple words in one field. Could you please take a look at the patch? Many thanks best regards Jonathan On 04/25/2012 07:48 PM, Weijun Wang wrote: Hi Jonathan I'm using English. In your test all the files have a similar modified time so you cannot see the difference. However, in my example, you can see that the widths for date and hour are not zero-padded so the width can be either 1 or 2. French is even worse smk 76 10 nov. 2009 08:57:54 bin/vbin/go smk 1149 8 avr. 2012 16:03:20 bin/vbin/netbeans smk 170 20 nov. 2009 16:47:42 bin/vbin/syncdown smk 671 8 févr. 2012 20:11:22 bin/vbin/ssh.desktop smk 187 20 nov. 2009 16:47:34 bin/vbin/syncsf So here even the width of month abbr can be different. Thanks Max On 04/25/2012 07:09 PM, Jonathan Lu wrote: Hello Max, Terribly sorry for my misunderstanding! On 04/25/2012 05:39 PM, Weijun Wang wrote: On 04/25/2012 05:23 PM, Jonathan Lu wrote: Hi Max, On 04/25/2012 05:12 PM, Weijun Wang wrote: On 04/25/2012 03:28 PM, Jonathan Lu wrote: Hi Weijun, Thanks for your time, I've updated the webrev, could you please take a look? http://cr.openjdk.java.net/~luchsh/7163483_2/ On 04/24/2012 03:06 PM, Weijun Wang wrote: Hi Jonathan Some comments: 1. Can you be sure that the new format always has the same length? jarsigner tries to output in a tabular style and each column should be aligned. I'm not sure of that, so the test case was updated to compare the first several tokens to determine whether there's any differences in the expression of date time. Sorry, I didn't make myself clear last time, I was mainly afraid of unaligned lines that make the output ugly. For example: smk 76 Nov 10, 2009 8:57:54 AM bin/vbin/go smk 1149 Apr 8, 2012 4:03:20 PM bin/vbin/netbeans smk 170 Nov 20, 2009 4:47:42 PM bin/vbin/syncdown smk 671 Feb 8, 2012 8:11:22 PM bin/vbin/ssh.desktop smk 187 Nov 20, 2009 4:47:34 PM bin/vbin/syncsf I think that would not be a problem in the new test case which compares tokenized strings splited by blank spaces instead of String#equals. Does that make sense? I'm not talking about the test. It's the output of jarsigner looking ugly. smk 76 Nov 10, 2009 8:57:54 AM bin/vbin/go smk 1149 Apr 8, 2012 4:03:20 PM bin/vbin/netbeans smk 170 Nov 20, 2009 4:47:42 PM bin/vbin/syncdown smk 671 Feb 8, 2012 8:11:22 PM bin/vbin/ssh.desktop smk 187 Nov 20, 2009 4:47:34 PM bin/vbin/syncsf Compare with the current output: smk 76 Tue Nov 10 08:57:54 CST 2009 bin/vbin/go smk 1149 Sun Apr 08 16:03:20 CST 2012 bin/vbin/netbeans smk 170 Fri Nov 20 16:47:42 CST 2009 bin/vbin/syncdown smk 671 Wed Feb 08 20:11:22 CST 2012 bin/vbin/ssh.desktop smk 187 Fri Nov 20 16:47:34 CST 2009 bin/vbin/syncsf I did not see unaligned format in my testing, did you get these unaligned output after applying the patch? From above lines, I see the starting indices of date string in each line are always the same, which is achieved by jarsigner, but the length of the date strings are not the same, which locale were you testing on? Thanks Max Thanks Max 2. You might need to reformat the modified line to make it fit into 80 characters width. 3. Why not include the test inside the changeset? 2, 3 were done in the new patch Thanks Max On 04/23/2012 05:46
Re: Code review request, CR 7180038 regression test failure, SSLEngineBadBufferArrayAccess.java
It was the abbreviated handshaking. I guess that the previous client
has not closed the socket completely, so for *this* handshaking, the
abbreviated handshaking rather than the full handshaking is used.
The issue is not due to closing the previous connections completely.
The two close messages are racing and the messages appeared out of the
normal ordering in this one case when the thread did a simple session
resume.
I tied several different approaches within this test, but failed to
reproduce the abbreviated handshaking. It is not easy to hack the
test without significant changes.
The original testcase was for the bad_record_mac issue, and testing
whether or not ByteBuffers with valid hasArray() was working correctly.
I wasn't intending to test renegotiation/abbreviated handshakes. I
obviously didn't completely step through the code when I added the
second runTest(). :( That explains the race condition on close
ordering, and the issue is still there.
A better fix would be the following so that the test is actually
starting from scratch each time (no session resumes):
diff --git
a/test/sun/security/ssl/com/sun/net/ssl/internal/ssl/SSLEngineImpl/SSLEngineBadBufferArrayAccess.java
b/test/sun/security/ssl/com/sun/net/ssl/internal/ssl/SSLEngineImpl/SSLEngineBadBufferArrayAccess.java
---
a/test/sun/security/ssl/com/sun/net/ssl/internal/ssl/SSLEngineImpl/SSLEngineBadBufferArrayAccess.java
+++
b/test/sun/security/ssl/com/sun/net/ssl/internal/ssl/SSLEngineImpl/SSLEngineBadBufferArrayAccess.java
@@ -146,14 +146,13 @@
SSLv3, TLSv1, TLSv1.1, TLSv1.2 };
for (String protocol : protocols) {
-log(Testing + protocol);
/*
* Run the tests with direct and indirect buffers.
*/
-SSLEngineBadBufferArrayAccess test =
-new SSLEngineBadBufferArrayAccess(protocol);
-test.runTest(true);
-test.runTest(false);
+log(Testing + protocol + :true);
+new SSLEngineBadBufferArrayAccess(protocol).runTest(true);
+log(Testing + protocol + :false);
+new SSLEngineBadBufferArrayAccess(protocol).runTest(false);
}
System.out.println(Test Passed.);
Please consider opening a new bug and changing.
Brad
On 7/2/2012 8:15 PM, Xuelei Fan wrote:
On 7/3/2012 11:09 AM, Weijun Wang wrote:
Your fix looks fine.
Thanks!
IMHO, the remind is not really useful unless you dump more info, say,
the value of serverIn.remaining().
We can get the value from analysis of the log. The remind is only used
for the case that we do not really fix the issue with this update.
Xuelei
QE would report the failure to THE
SECURITY TEAM anyway.
-Max
On 07/03/2012 11:00 AM, Xuelei Fan wrote:
On 7/3/2012 10:40 AM, Weijun Wang wrote:
No new test needed. I only think that you might be able to hack the
current test a little to reproduce this and see if the failure is the
same and if your code change can fix it. There is no need to keep this
hack in your final changeset.
I tied several different approaches within this test, but failed to
reproduce the abbreviated handshaking. ;-) It is not easy to hack the
test without significant changes.
Xuelei
-Max
On 07/03/2012 10:37 AM, Xuelei Fan wrote:
On 7/3/2012 10:02 AM, Weijun Wang wrote:
On 07/03/2012 09:48 AM, Xuelei Fan wrote:
On 7/2/2012 4:35 PM, Weijun Wang wrote:
I take a look at the test output. When the last handshake starts:
server unwrap: OK/NEED_TASK, 230/0 bytes
running delegated task...
new HandshakeStatus: NEED_WRAP
server wrap: OK/NEED_WRAP, 0/86 bytes
Here the first wrap only generates 86 bytes, I guess that's the
ServerHello message? It keeps the state at NEED_WRAP but then never
really generates the Certificate message. What might be the problem?
Good catch!
It was the abbreviated handshaking. I guess that the previous client
has
not closed the socket completely, so for *this* handshaking, the
abbreviated handshaking rather than the full handshaking is used.
For full handshaking, it is the client sending the Finished
message at
first. However, for abbreviated handshaking, the server send the
Finished message at first.
In the current scenarios, it is expected that the client sends its
application data (26 bytes), and then the server sends its
application
data (29 bytes). However, the abbreviated handshaking disorder the
sequence in that it is the sever sends it application data (29 bytes)
before client. In such cases, the following logic does not stand any
more:
if (!closed (serverOut.remaining() == 0)) {
closed = true;
...
if (serverIn.remaining() != clientMsg.length) {
throw new Exception(Client: Data length error);
}
...
}
Because the server has not receive the client message when the server
sends its application data.
