code review request: 7012160: read SF file in signed jar in streaming mode

Sean Mullan sean.mullan at oracle.com
Tue Feb 1 20:42:41 UTC 2011


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.wang at oracle.com
>



More information about the security-dev mailing list