Re: [9] RFR: 8042967: Add variant of DSA Signature algorithms that do not ASN.1 encode the signature bytes
Hi Sean,
Here is a new webrev with changes addressing all of your comments:
http://cr.openjdk.java.net/~juh/8042967/01/
Thanks,
Jason
On 01/23/2015 10:21 AM, Sean Mullan wrote:
Hi Jason,
Just a few comments:
* DOMSignatureMethod
- If setParameter throws an exception, I think we should fallback and
try the old code. This will allow 3rd party JCE providers to continue to
work until they update their implementations to support the new
ParameterSpec.
* DSA, ECDSASignature, P11Signature
- engineSetParameter should throw InvalidParameterException if the enum
is neither ASN1 nor P1363
* SignatureFormatParameterSpec
- can you add a sentence describing what each format is, ex: for ASN.1:
"An ASN.1 sequence of two INTEGER values: r and s, in that order:
SEQUENCE ::= { r INTEGER, s INTEGER }". For P1363, "an octet-encoding of
r and s, in that order."
* TestECDSA, TestDSA2
- can you add an @bug 8042967 to each of these tests, since you are
extending them to test this new format.
- can you also add a test which uses the
SignatureFormatParameterSpec.ASN1 enum (by passing it to setParameter)?
--Sean
On 01/22/2015 09:46 PM, Jason Uh wrote:
Please review this change, which enables DSA and ECDSA signatures in the
IEEE P1363 format (the concatenation of r and s).
This is accomplished through an implementation of
AlgorithmParameterSpec, SignatureFormatParameterSpec, which can be
passed to the existing Signature.setParameter() method before signing or
verifying a signature to indicate the desired signature format.
webrev: http://cr.openjdk.java.net/~juh/8042967/00/
bug: https://bugs.openjdk.java.net/browse/JDK-8042967
Thanks,
Jason
Re: RFR 8055045: StringIndexOutOfBoundsException while reading krb5.conf
Looks good to me. --Sean On 01/22/2015 09:33 AM, Wang Weijun wrote: Hi All Please review the code change at http://cr.openjdk.java.net/~weijun/8055045/webrev.00/ The old code has an error is the value is a single " or '. Thanks Max
Re: RFR: 8069038: javax/net/ssl/TLS/TLSClientPropertyTest.java needs to be updated for JDK-8061210
Xuelei/Sean looked at this already (below). Brad On 1/23/2015 5:04 AM, Sean Mullan wrote: > On 01/23/2015 02:21 AM, Bradford Wetmore wrote: >> JPRT passed. Off to bed. > > The fix looks fine to me. > > --Sean On 1/22/2015 10:02 PM, Xuelei Fan wrote: > Looks fine to me. > > Thanks, > Xuelei On 1/23/2015 10:28 AM, Bradford Wetmore wrote: This is a P2 test bug impacting continuous integration: http://cr.openjdk.java.net/~wetmore/8069038/webrev.00 A recent change missed updating one of the tests. Pretty straight forward, also corrected one typo. I don't see this test in the JDK 8 workspace, it's apparently only a 9 thing. Brad
RFR: 8069038: javax/net/ssl/TLS/TLSClientPropertyTest.java needs to be updated for JDK-8061210
This is a P2 test bug impacting continuous integration: http://cr.openjdk.java.net/~wetmore/8069038/webrev.00 A recent change missed updating one of the tests. Pretty straight forward, also corrected one typo. I don't see this test in the JDK 8 workspace, it's apparently only a 9 thing. Brad
Re: [9] RFR: 8042967: Add variant of DSA Signature algorithms that do not ASN.1 encode the signature bytes
Hi Jason,
Just a few comments:
* DOMSignatureMethod
- If setParameter throws an exception, I think we should fallback and
try the old code. This will allow 3rd party JCE providers to continue to
work until they update their implementations to support the new
ParameterSpec.
* DSA, ECDSASignature, P11Signature
- engineSetParameter should throw InvalidParameterException if the enum
is neither ASN1 nor P1363
* SignatureFormatParameterSpec
- can you add a sentence describing what each format is, ex: for ASN.1:
"An ASN.1 sequence of two INTEGER values: r and s, in that order:
SEQUENCE ::= { r INTEGER, s INTEGER }". For P1363, "an octet-encoding of
r and s, in that order."
* TestECDSA, TestDSA2
- can you add an @bug 8042967 to each of these tests, since you are
extending them to test this new format.
- can you also add a test which uses the
SignatureFormatParameterSpec.ASN1 enum (by passing it to setParameter)?
--Sean
On 01/22/2015 09:46 PM, Jason Uh wrote:
Please review this change, which enables DSA and ECDSA signatures in the
IEEE P1363 format (the concatenation of r and s).
This is accomplished through an implementation of
AlgorithmParameterSpec, SignatureFormatParameterSpec, which can be
passed to the existing Signature.setParameter() method before signing or
verifying a signature to indicate the desired signature format.
webrev: http://cr.openjdk.java.net/~juh/8042967/00/
bug: https://bugs.openjdk.java.net/browse/JDK-8042967
Thanks,
Jason
Re: RFR: 8065994: HTTP Tunnel connection to NTLM proxy reauthenticates instead of using keep-alive
Looks good to me Sean. Good to see additional logging in this area. -Chris. On 22/01/15 15:21, Seán Coffey wrote: Looking for a review around this issue that came in as a reported performance regression in NTLM proxy authentication. It turned out that HttpsClients were being discarded after Proxy SocketAddress equality tests failed. Lack of caching is expensive in terms for performance for TLS and needless handshakes. The 2nd round of NTLM authentication was passing in a Proxy which had a resolved SocketAddress. The previous Proxy creation for the same connection (via DefaultProxySelector) constructs Proxy using unresolved socketAddress. Proposed fix is to compare like with like and have Proxy construct with unresolved Address. I captured more details in bug report. I'm also using this opportunity to adding some extra logging to the HttpsClient class and to correct a bad null versus NO_PROXY test that existed (line 339) bug report : https://bugs.openjdk.java.net/browse/JDK-8065994 webrev : http://cr.openjdk.java.net/~coffeys/webrev.8065994/webrev/ regards, Sean.
Re: [9] request for review: 8049171: Additional tests for jarsigner's warnings
Hi Max, Please see inline. On 01/23/2015 05:18 AM, Wang Weijun wrote: I have updated the webrev, updateJar() method does the following: >- creates a new jar file (destJarFilename) >- puts files which are specified in files parameter to destJarFilename >- copies files from srcJarFilename to destJarFilename if they are no files with the same names in destJarFilename The process above means the new files are added at the beginning. Yes, "jar -tvf" shows the following for an original signed jar (before updateJar() is called) jar -tvf JTwork/scratch/0/signed.jar 83 Fri Jan 23 10:06:16 MSK 2015 META-INF/MANIFEST.MF 327 Fri Jan 23 10:06:16 MSK 2015 META-INF/ALIAS.SF 1091 Fri Jan 23 10:06:16 MSK 2015 META-INF/ALIAS.RSA 0 Fri Jan 23 10:06:10 MSK 2015 first.txt After pdateJar() is called, "jar -tvf" shows the following for modified jar jar -tvf JTwork/scratch/0/updated_signed.jar 0 Fri Jan 23 10:06:16 MSK 2015 second.txt 83 Fri Jan 23 10:06:16 MSK 2015 META-INF/MANIFEST.MF 327 Fri Jan 23 10:06:16 MSK 2015 META-INF/ALIAS.SF 1091 Fri Jan 23 10:06:16 MSK 2015 META-INF/ALIAS.RSA 0 Fri Jan 23 10:06:10 MSK 2015 first.txt While jarsigner is able to verify such a file (it uses JarFile) the output is actually invalid because the MANIFEST and the signature files must be at the beginning (otherwise a JarInputStream cannot verify it). jarsigner successfully verifies a jar file that was modified by updateJar() method, please see a piece of log for sun/security/tools/jarsigner/warnings/HasUnsignedEntryTest.java test Copy signed.jar to updated_signed.jar, and add second.txt.class, so it contains unsigned entry Updating updated_signed.jar with second.txt Copying META-INF/MANIFEST.MF to updated_signed.jar Copying META-INF/ALIAS.SF to updated_signed.jar Copying META-INF/ALIAS.RSA to updated_signed.jar Copying first.txt to updated_signed.jar Command line: [/home/artem/ws/jdk/jdk9_dev/build/linux-x86_64-normal-server-fastdebug/jdk/bin/jarsigner -verify -verbose -keystore keystore.jks -storepass password -keypass password updated_signed.jar] 0 Fri Jan 23 10:06:16 MSK 2015 second.txt s k 83 Fri Jan 23 10:06:16 MSK 2015 META-INF/MANIFEST.MF 327 Fri Jan 23 10:06:16 MSK 2015 META-INF/ALIAS.SF 1091 Fri Jan 23 10:06:16 MSK 2015 META-INF/ALIAS.RSA smk0 Fri Jan 23 10:06:10 MSK 2015 first.txt s = signature was verified m = entry is listed in manifest k = at least one certificate was found in keystore i = at least one certificate was found in identity scope jar verified. Warning: This jar contains unsigned entries which have not been integrity-checked. This jar contains signatures that does not include a timestamp. Without a timestamp, users may not be able to validate this jar after the signer certificate's expiration date (2016-01-23) or after any future revocation date. Re-run with the -verbose and -certs options for more details. If the MANIFEST and the signature files must be at the beginning, should it be considered as a bug in jarsigner? Should it reject such files? The "jar u" way is to copy each old entry into destination unless the entry name is in the updated list where the new file will be read. Finally the untouched files in the updated list are appended. Since tests were not originally for checking some unusual ways for updating jars, I think they need to be updated to use the "jar u" way for adding unsigned entry. Artem
