Hi Max,

This is looking good. I'm about halfway through. Here are some code comments to start with. I think we'll need a couple more rounds of code reviews before we finish so I wanted to get these to you now.

* JarFile

[329-330]: How about adding a SignatureFileVerifier.isBlock method instead? That would be cleaner.

[338]: I believe you can replace the argument to getBytes with the local variable e :

s/getBytes(getJarEntry(names[i])),/getBytes(e),

* PKCS7

Can we avoid duplicating code across SignerInfo and PKCS7Verifier? Can SignerInfo.verify invoke methods of PKCS7Verifier?

[538-540]: fields should be marked final

[542]: Why is a static method necessary here? Since the method always creates a PKCS7Verifier object, it doesn't seem like it is that useful or necessary.

[661-665]: replace this code with MessageDigest.isEqual.

--Sean


On 1/14/11 3:31 AM, Weijun Wang wrote:
Hi Sean

http://cr.openjdk.java.net/~weijun/7012160/webrev.00/

I've made changes to the following classes to enable streaming mode SF file
reading:

- java/util/jar/JarVerifier.java:

1. New verifyBlock method.

2. Change the constructor from JarVerifier(byte[]) to JarVerifier(byte[],
Manifest). In SignatureFileVerifier.processImpl(), if we already confirm the
*-Digest-Manifest header in the SF file matches the whole MANIFEST.MF, there'se
no need to parse the rest of the SF file, since we can be sure that entries in
the SF file are identical to those in MANIFEST.MF. Of course, the content of the
SF file still needs to be fed into PKCS7Verifier to verify the signature.

- java/util/jar/JarFile.java:

Read DSA file in byte[] and SF file in InputStream, and call
JarVerifier.verifyBlock() to verify.

- java/util/jar/Manifest.java:

Adding update(byte[]) to read manifest in streaming mode. This is a new public 
API.

- sun/security/pkcs/PKCS7.java:

New PKCS7Verifier class to verify SignedData in streaming mode. I basically
divide the SignerInfo.verify(PKCS7 block, byte[] data) method into 3 parts and
make them the 3 methods of this class.

- sun/security/util/SignatureFileVerifier.java:

Rewrite the processImpl(*) method to make use of new methods in PKCS7 and 
Manifest.

No new regression tests, use existing ones.

I've tried NetBeans profiler to look at the memory. The program simply calls
JarSigner.main(new String[]{"-verify", "x.jar"}) and the signed jar x.jar has
10000 files inside.

Before After
byte[] 3.6MB 2.8MB
char[] 2.0MB 1.3MB
String 1.1MB 650KB

So it does have some difference.

Thanks
Max


-------- Original Message --------
*Change Request ID*: 7012160
*Synopsis*: read SF file in signed jar in streaming mode


=== *Description* ============================================================
When a signed jar is verified, its SF file is read into a byte array and
verified against the signature. When there are many files in the jar, the SF
file can be very big. It will be better if the file can be read in streaming 
mode.

*** (#1 of 1): 2011-01-13 12:23:25 GMT+00:00 weijun.w...@oracle.com

Reply via email to