Re: 8029886: Change SecurityManager check{TopLevelWindow, SystemClipboardAccessAwtEventQueueAccess} to check AllPermission

2013-12-11 Thread Sean Mullan
On 12/10/2013 08:51 AM, Alan Bateman wrote: In JDK 8 we deprecated the JDK 1.1-era SecurityManager methods checkTopLevelWindow, checkSystemClipboard and checkAccessAwtEventQueueAccess with a warning that they would be changed in a future release to check AllPermission. At the same time we change

Re: JDK 9 RFR: JDK-8030082 Fix raw types lint warnings, etc. in various sun.security libraries

2013-12-13 Thread Sean Mullan
Looks fine to me. --Sean On 12/13/2013 01:02 AM, Joe Darcy wrote: Hello, Please review the below straightforward fix for JDK-8030082 Fix raw types lint warnings, etc. in various sun.security libraries https://bugs.openjdk.java.net/browse/JDK-8030082 Webrev also visible at htt

Re: JDK 9 RFR JDK-8030084 Fix lint warnings in sun.security.tools.policytool

2013-12-13 Thread Sean Mullan
Looks fine to me. I assume you will add the appropriate noreg label before pushing? --Sean On 12/13/2013 11:16 AM, Joe Darcy wrote: Hello, Please review these changes to remove several dozen warnings from the sources to policytool: JDK-8030084 Fix lint warnings in sun.security.tools.pol

Re: Code review request, 7093640, Enable TLS 1.2 for client-side default contexts

2013-12-17 Thread Sean Mullan
I reviewed the source code changes (not the tests) and it looks good to me. --Sean On 12/17/2013 05:08 AM, Xuelei Fan wrote: Hi, This is a request to enabled TLS 1.2 for client-side default contexts. Please review this update. webrev: http://cr.openjdk.java.net/~xuelei/7093640/webrev.00/ We

Re: Code Review request: 8028431: NullPointerException in DerValue.equals(DerValue)

2013-12-20 Thread Sean Mullan
A couple of other comments: 1. Add an @Override annotation to the equals method. While you are in there, could you also add @Override to the toString and hashCode methods. 2. Move the "==" check and make it the first thing you check 3. Nit: don't include space between "!" and "(" @Overri

JDK 8 Review Request for 8030813 : Signed applet fails to load when CRLs are stored in an LDAP directory

2013-12-23 Thread Sean Mullan
Please review the following change which causes signed applets to fail if CRLs are stored in an LDAP directory. This occurs when any of the certificates in the applet's certificate chain contain a CRL Distribution Point extension with an LDAP URL. The fix introduces a new internal system prope

hg: jdk8/tl/jdk: 2 new changesets

2013-12-23 Thread sean . mullan
Changeset: aef6c726810e Author:mullan Date: 2013-12-23 14:03 -0500 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/aef6c726810e 8030813: Signed applet fails to load when CRLs are stored in an LDAP directory Summary: Skip JNDI application resource lookup to avoid recursive JAR valida

Re: JDK 9 RFR of doclint fixes in javax.xml.crypto.dsig

2014-01-02 Thread Sean Mullan
Looks good. --Sean On 12/30/2013 01:26 PM, Joe Darcy wrote: Hello, In the course of working on some further doclint cleanup for JDK 9, I came across a variety of minor issues in javax.xml.crypto.dsig. Please review the patch below for JDK 9. Thanks, -Joe --- old/src/share/classes/javax/xml/

Re: Code Review request: 8028431: NullPointerException in DerValue.equals(DerValue)

2014-01-03 Thread Sean Mullan
://cr.openjdk.java.net/~asmotrak/8028431/webrev.02/ <http://cr.openjdk.java.net/%7Easmotrak/8028431/webrev.02/> Artem On 12/20/2013 06:34 PM, Sean Mullan wrote: A couple of other comments: 1. Add an @Override annotation to the equals method. While you are in there, could you also add @Overr

Re: Code review request, 8030829 Add MD5 to jdk.certpath.disabledAlgorithms security property

2014-01-09 Thread Sean Mullan
The code change looks fine. My main concern is the number of tests that have been converted to run in othervm which will make the tests run slower. Did you explore how much effort it would be to convert some of the test certificates to use stronger algorithms? Also, I noticed many of the tests

Re: Code review request, 8030829 Add MD5 to jdk.certpath.disabledAlgorithms security property

2014-01-10 Thread Sean Mullan
On 01/09/2014 09:28 PM, Xuelei Fan wrote: On 1/10/2014 6:34 AM, Sean Mullan wrote: The code change looks fine. My main concern is the number of tests that have been converted to run in othervm which will make the tests run slower. Did you explore how much effort it would be to convert some of

Re: Code review request, 8031566, regression test failure, SSLEngineBadBufferArrayAccess.java

2014-01-14 Thread Sean Mullan
Looks fine to me. --Sean On 01/14/2014 04:37 AM, Xuelei Fan wrote: Hi, Please review this simple regression test fix. webrev: http://cr.openjdk.java.net/~xuelei/8031566/webrev.00/ I missed something in the fix of JDK-7180038 for the same test. This update makes the test more reliable so that

Re: FYI, JDK 9 RFR of JDK-8031651: Remove unneeded -source and -target flags in jdk repo regression tests

2014-01-14 Thread Sean Mullan
The change looks good, thanks for the FYI. --Sean On 01/14/2014 11:59 AM, Joe Darcy wrote: Hello, FYI, over on core-libs http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-January/024375.html there is a review discussion of removing old -source and -target settings from regression te

JDK 8 Review Request for 8031825: OCSP client can't find responder cert if it uses a different subject key id algorithm than responderID

2014-01-17 Thread Sean Mullan
Please review the following fix for a serious issue found while testing interoperability with an OCSP responder: JBS: https://bugs.openjdk.java.net/browse/JDK-8031825 webrev: http://cr.openjdk.java.net/~mullan/webrevs/8031825/webrev.00/ The regression test depends on proprietary certs, so it wi

hg: jdk8/tl/jdk: 8031825: OCSP client can't find responder cert if it uses a different subject key id algorithm than responderID

2014-01-22 Thread sean . mullan
Changeset: 57c26829deb6 Author:mullan Date: 2014-01-22 19:06 -0500 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/57c26829deb6 8031825: OCSP client can't find responder cert if it uses a different subject key id algorithm than responderID Reviewed-by: vinnie, xuelei ! src/share/c

Re: RFR: 8031572: jarsigner -verify exits with 0 when a jar file is not properly signed

2014-01-24 Thread Sean Mullan
In JarFile, I think you should also upper-case the entry names before passing to SignatureFileVerifier.isBlockOrSF, as all other calls to this method in the JDK do that. The jar specification says that these filenames are case-insensitive, so a file named "*.rsa" should also be treated as a sig

Re: question about restricted packages

2014-01-28 Thread Sean Mullan
On 01/28/2014 06:36 AM, Vicente-Arturo Romero-Zaldivar wrote: Hi Jeff, On 27/01/14 18:56, Jeff Nisewanger wrote: But be aware that this isn't a static list -- applications or middleware can extend it at runtime (generally when they start up). So this means that it's better to use a method to

CFV: New Security Group Member: Jason Uh

2014-01-28 Thread Sean Mullan
cast in the open by replying to this mailing list. For Lazy Consensus voting instructions, see [2]. Sean Mullan [1] http://openjdk.java.net/census#security [2] http://openjdk.java.net/groups/#member-vote

Re: CFV: New Security Group Member: Jason Uh

2014-01-28 Thread Sean Mullan
Vote: yes On 01/28/2014 08:27 AM, Sean Mullan wrote: I hereby nominate Jason Uh to Membership in the Security Group. Jason Uh is a member of the Java security libraries team at Oracle and has been an active contributor to the OpenJDK security group for almost 2 years. Jason was voted in as a

Re: RFR 8029995: accept yes/no for boolean krb5.conf settings

2014-01-28 Thread Sean Mullan
On 01/28/2014 03:53 AM, Wang Weijun wrote: Please review the fix at http://cr.openjdk.java.net/~weijun/8029995/webrev.00/ The supported boolean values in this fix cover what MIT krb5 does and we also added 'f'. The old getBooleanValue() method returns true for “true” and false otherwise but th

Re: RFR(S): 8033154: PPC64: Fix AIX build after integration into jdk9/dev

2014-01-29 Thread Sean Mullan
The java.security-aix file looks fine to me. --Sean On 01/29/2014 12:59 PM, Volker Simonis wrote: Hi, please review the following small change: http://cr.openjdk.java.net/~simonis/webrevs/8033154/ which fixes the AIX build after the integration of ppc-aix-port/stage-9/jdk to jdk9/dev/jdk. I

Re: Security Policy with denying rules

2014-02-04 Thread Sean Mullan
Hi Ondrej, Thanks for sharing the information about your project. If you have a copy of the book "Inside Java 2 Platform Security" [1], there is a section (5.1.5) discussing the rationale for not including support for "negative" permissions in the Java Security model. The section talks about

Re: Code Review request: 8028591: NegativeArraySizeException in sun.security.util.DerInputStream.getUnalignedBitString()

2014-02-05 Thread Sean Mullan
Hi Artem, The specific fix looks fine, but there are many other calls to getLength() in DerInputStream that subsequently initialize an array with the return value, and could also cause the same issue. It seems to me that a better fix would be to pass a flag to the getLength method (or create

Re: RFR: 8031588 warnings from b03 for jdk/src/share/native/sun/security/jgss/wrapper: JNI exception pending

2014-02-06 Thread Sean Mullan
Looks fine to me, Michael. Please add a noreg label to the bug. --Sean On 02/06/2014 04:57 AM, Michael McMahon wrote: Hi, This is a fix adding some checks for pending exceptions in the JGSS native code. All of these cases could only practically happen in case of OutOfMemoryError. There are a c

Result: New Security Group Member: Jason Uh

2014-02-12 Thread Sean Mullan
The vote for Jason Uh [1] is now closed. Yes: 5 Veto: 0 Abstain: 0 According to the Bylaws definition of Lazy Consensus, this is sufficient to approve the nomination. Sean Mullan [1] http://mail.openjdk.java.net/pipermail/security-dev/2014-January/010087.html

Re: [9] Request for Review: 8031025: SQE test CertPath/CertPathBuilderTest/* failed with java.lang.IndexOutOfBoundsException

2014-02-12 Thread Sean Mullan
Looks good. --Sean On 02/12/2014 06:17 PM, Jason Uh wrote: Hi all, Please review this fix for JDK 9, which checks for an empty cert path list in RevocationChecker. webrev: http://cr.openjdk.java.net/~juh/8031025/webrev.01 noreq-sqe, as the fix can be verified with existing SQE tests. Thanks

Review Request for JDK-8025708 : Certificate Path Building problem with AKI serial number

2014-02-13 Thread Sean Mullan
See: http://cr.openjdk.java.net/~mullan/webrevs/8025708/webrev/ This fixes a problem with the PKIX CertPathBuilder where it wasn't able to build a path when the Authority Key Identifier extension of an intermediate CA cert did not contain a serial number field, and the end entity cert did. T

Re: 8034856/8034857: More gcc warnings

2014-02-13 Thread Sean Mullan
Looks fine to me. --Sean On 02/13/2014 08:18 AM, Alan Bateman wrote: The number of native code warnings in the build is annoying so this is another drive-by fix that eliminates a few of them in the serviceability and security areas. The webrev with the changes is here: http://cr.openjdk.java.

Re: Review Request for JDK-8025708 : Certificate Path Building problem with AKI serial number

2014-02-14 Thread Sean Mullan
de the matching rules for these criteria in AdaptableX509CertSelector and never call the superclass methods to set those criteria (which is why they are overridden to throw IllegalArgumentExc). Hope this makes sense. --Sean Thanks! Jason On 2/13/14 5:04 AM, Sean Mullan wrote: See: http://cr.openjdk.java.net/~m

Re: Review Request for JDK-8025708 : Certificate Path Building problem with AKI serial number

2014-02-17 Thread Sean Mullan
e optional. This update allow the absence of subject key ID extension. Yes I was wondering about that, but I am just preserving the previous behavior: see lines 167-170 -- which I assume was done for a good reason. --Sean Otherwise, looks fine to me. Xuelei On 2/13/2014 9:04 PM, Sean M

Re: Dup content in java.security-platform files

2014-02-23 Thread Sean Mullan
On 02/21/2014 01:17 AM, Wang Weijun wrote: Is there a proposal to extract them into a single include file? No solution proposed yet, but there is an RFE for this: https://bugs.openjdk.java.net/browse/JDK-6997010 --Sean

Re: Dup content in java.security-platform files

2014-02-24 Thread Sean Mullan
On 02/23/2014 06:57 PM, Wang Weijun wrote: A somehow related question: is there a proposal to specify a security property on the command line? I agree that would be useful. A workaround: echo "property=foo" > /tmp/props; java -Djava.security.properties=/tmp/foo ... --Sean

Re: Dup content in java.security-platform files

2014-02-24 Thread Sean Mullan
On 02/24/2014 08:57 AM, Sean Mullan wrote: On 02/23/2014 06:57 PM, Wang Weijun wrote: A somehow related question: is there a proposal to specify a security property on the command line? I agree that would be useful. A workaround: echo "property=foo" > /tm

Re: Request for Review: 8035973: NPE in ForwardBuilder

2014-03-03 Thread Sean Mullan
On 02/28/2014 02:56 PM, Jason Uh wrote: Could I please get a review of this change? Looks fine to me, but the priority should be higher if this requires a backport to 7u. Also, the bug should be labeled with "8-na" and "9-na" since this is not an issue in 8 and 9. --Sean Just a simple fi

Re: [9] Request for Review: 8021804: Certpath validation fails if validity period of root cert does not include validity period of intermediate cert

2014-03-10 Thread Sean Mullan
Hi Jason, Sorry for the delay in reviewing this. On 02/28/2014 02:54 PM, Jason Uh wrote: Hi Sean, Could I please get a review of this change? This fix allows a certpath to be validated when a certificate issued by a version 1 trusted cert has a validity period that doesn't fall within the vali

Re: [9] Request for Review: 8021804: Certpath validation fails if validity period of root cert does not include validity period of intermediate cert

2014-03-11 Thread Sean Mullan
10/2014 08:00 PM, Jason Uh wrote: Thanks, Sean. I've simplified my changes to only removing the call to setValidityPeriod. http://cr.openjdk.java.net/~juh/8021804/webrev.01 Jason On 3/10/14 12:00 PM, Sean Mullan wrote: Hi Jason, Sorry for the delay in reviewing this. On 02/28/2014 02:

Re: [9] Request for Review: 8021804: Certpath validation fails if validity period of root cert does not include validity period of intermediate cert

2014-03-12 Thread Sean Mullan
ill passes). I'll push if you're okay with that. If you want to take a look: http://cr.openjdk.java.net/~juh/8021804/webrev.02 Jason On 3/11/14 2:59 PM, Sean Mullan wrote: In the test, you should probably call PKIXParameters.setValidity with a fixed date so that the test won't s

Re: RFR 8037262: typo in error message in KrbAsReq.authenticate()

2014-03-13 Thread Sean Mullan
Looks ok, although I noticed another typo: 409 * AP-REP will need to be generated. Should that also be AP-REQ? --Sean On 03/12/2014 10:18 PM, Wang Weijun wrote: Tiny webrev at http://cr.openjdk.java.net/~weijun/8037262/webrev.00/ Thanks Max

Re: RFR 8037258: AIOB while parsing CRL for revoked certificate

2014-03-24 Thread Sean Mullan
Looks good to me. --Sean On 03/19/2014 11:36 AM, Rajan Halade wrote: Please review this small fix - http://cr.openjdk.java.net/~mullan/webrevs/8037258/webrev.02/ I have also updated the source to remove unused variables and updated javadoc information. Thanks, Rajan

Re: RFR 8029995: accept yes/no for boolean krb5.conf settings

2014-04-04 Thread Sean Mullan
. Some words in javax/security/auth/kerberos/package-info.java 3. getBoolean() renamed to getBooleanObject() because it's quite easy to forget the return value is a Boolean (instead of boolean) and could be null. Thanks Max On Jan 29, 2014, at 5:46, Sean Mullan wrote: On 01/28/2014 03:

Review Request for JDK-8038431: Close InputStream when finished retrieving XML Signature HTTP References

2014-04-08 Thread Sean Mullan
webrev: http://cr.openjdk.java.net/~mullan/webrevs/8038431/webrev.00/ bug: https://bugs.openjdk.java.net/browse/JDK-8038431 Thanks, Sean

Re: Code review request, JDK-8040062 Need to add new methods in BaseSSLSocketImpl

2014-04-14 Thread Sean Mullan
Looks fine to me. Can you add a relates to link to JDK-8036979 and an appropriate noreg label? --Sean On 04/14/2014 06:04 AM, Xuelei Fan wrote: Hi, Please review the fix for JDK-8040062: http://cr.openjdk.java.net/~xuelei/8040062/webrev.00/ In JDK-8036979, there are news methods are ad

Review Request: 8038184: XMLSignature throws StringIndexOutOfBoundsException if ID attribute value is empty String

2014-04-14 Thread Sean Mullan
Please review the following simple fix for JDK-8038184: Bug: https://bugs.openjdk.java.net/browse/JDK-8038184 Webrev: http://cr.openjdk.java.net/~mullan/webrevs/8038184/webrev.01/ Thanks, Sean

Re: RFR 8039853: Provider.Service.newInstance() does not work with current JDK JGSS Mechanisms

2014-04-15 Thread Sean Mullan
Looks fine to me. --Sean On 04/15/2014 04:03 AM, Wang Weijun wrote: Please review the code changes at http://cr.openjdk.java.net/~weijun/8039853/webrev.00/ If you find it confused, I have mistakenly pushed some code changes in http://hg.openjdk.java.net/jdk9/dev/jdk/rev/ba6e2fcdfa15 a

Re: RFR 8029994: Support "include" and "includedir" in krb5.conf

2014-04-17 Thread Sean Mullan
* Config.java - update copyright year [202] can you log the IOException? will finish reviewing later ... --Sean On 04/10/2014 07:40 AM, Weijun Wang wrote: Hi All Please review the code changes at http://cr.openjdk.java.net/~weijun/8029994/webrev.01/ Two major changes made: 1. The incl

Re: RFR 8040068 and 8039951: platform-related JAAS login modules on all platforms

2014-04-18 Thread Sean Mullan
On 04/17/2014 07:38 AM, Wang Weijun wrote: Hi All There are two bugs. The first one is: https://bugs.openjdk.java.net/browse/JDK-8040068 8040068: SolarisSystem should be @Deprecated and @jdk.Exported(false) of which the code change is simply --- a/src/share/classes/com/sun/security/auth/modul

Re: Review request: 8040059 Change default policy for extensions to no permission

2014-04-23 Thread Sean Mullan
On 04/22/2014 06:36 PM, Mandy Chung wrote: Thanks for bringing up this question. I missed to mention the open question to follow up how we want to build the system java.policy. There are platform-specific jar file and also different jar files in Oracle JDK build. I currently list them all in ja

Re: Review request: 8040059 Change default policy for extensions to no permission

2014-04-23 Thread Sean Mullan
On 04/23/2014 12:14 PM, Mandy Chung wrote: Thanks Sean. This patch separates the Oracle-specific content from the OpenJDK java.security files. Is there any plan to handle java.security- differently (I recalled there is a RFE for it and a large part of the content is duplicated)? If this is

Re: Review request: 8040059 Change default policy for extensions to no permission

2014-04-23 Thread Sean Mullan
Just a few comments: 1. When you write a test that uses the jtreg /policy option, the policy file overrides the system policy file. If the test depends on a standard extension, then you may get SecurityExceptions unless additional perms are granted. Thus, there are quite a few tests that defin

Re: Review request: 8040059 Change default policy for extensions to no permission

2014-04-24 Thread Sean Mullan
On 04/23/2014 05:29 PM, Mandy Chung wrote: On 4/23/2014 1:10 PM, Sean Mullan wrote: Just a few comments: 1. When you write a test that uses the jtreg /policy option, the policy file overrides the system policy file. If the test depends on a standard extension, then you may get

JEP Review Request: Improve Security Manager Performance

2014-04-25 Thread Sean Mullan
Please review a draft of a proposed research JEP to improve the performance of the Security Manager: http://cr.openjdk.java.net/~mullan/jeps/Improve-Security-Manager-Performance.00 I am particularly interested in any experience you have measuring or profiling the performance of your code when

Re: JEP Review Request: Improve Security Manager Performance

2014-04-25 Thread Sean Mullan
On 04/25/2014 10:54 AM, David M. Lloyd wrote: On 04/25/2014 09:36 AM, Sean Mullan wrote: Please review a draft of a proposed research JEP to improve the performance of the Security Manager: http://cr.openjdk.java.net/~mullan/jeps/Improve-Security-Manager-Performance.00 I am particularly

Re: Code review request, a simple fix to remove the incorrect comment in RSAClientKeyExchange.java

2014-04-29 Thread Sean Mullan
Looks fine to me. --Sean On 04/29/2014 10:05 AM, Xuelei Fan wrote: Hi, As this is a simple comment fix, I will not generate a webrev as general. In sun/security/ssl/RSAClientKeyExchange.java: 116 // Cannot generate key here, please don't use Cipher.UNWRAP_MODE! 117 cipher.init(Cipher.UNW

JDK 9 Review Request for 8038349: Signing XML with DSA throws Exception when key is larger than 1024 bits

2014-04-29 Thread Sean Mullan
Please review the following change which adds support for 2048-bit DSA keys and the DSA-SHA256 algorithm to the XML Signature implementation: webrev: http://cr.openjdk.java.net/~mullan/webrevs/8038349/webrev.00/ JDK 8 already includes the underlying support for both of these in the Sun provide

Re: JDK 9 Review Request for 8038349: Signing XML with DSA throws Exception when key is larger than 1024 bits

2014-04-30 Thread Sean Mullan
icate the DSA algorithm, for example, convertDsaASN1toXMLDSIG. Yes, sounds like a good suggestion. Need more time to read the update in JavaUtils.java Ok, thanks. --Sean Xuelei On 4/30/2014 4:48 AM, Sean Mullan wrote: Please review the following change which adds support for 2048-bit DSA

Re: JDK 9 Review Request for 8038349: Signing XML with DSA throws Exception when key is larger than 1024 bits

2014-05-01 Thread Sean Mullan
e array, it will throw an exception if the dest array is not big enough. Can you clarify what you mean or show me the corrected code? Thanks, Sean Xuelei On 4/30/2014 4:48 AM, Sean Mullan wrote: Please review the following change which adds support for 2048-bit DSA keys and the DSA-SHA256 algo

Re: webrev request: JDK-6996377

2014-05-08 Thread Sean Mullan
On 05/07/2014 03:12 PM, Jamil Nimeh wrote: Please review the webrev for JDK-6996377 when you get a chance. http://cr.openjdk.java.net/~ascarpino/6996377/webrev.01/ - PKIXValidator[130]: you can use the diamond operator to make the code more concise: new HashMap<>(); - shell script test

Re: webrev request: JDK-6996377

2014-05-08 Thread Sean Mullan
On 05/08/2014 09:55 AM, Jamil Nimeh wrote: Ah, didn't know that we were moving away from the scripts. See https://blogs.oracle.com/jjg/entry/shelling_tests for some more rationale. I had thought about hard-coding certs, but I liked the on-the-fly generation approach because it kept the val

Re: [Update]: webrev request: JDK-6996377

2014-05-09 Thread Sean Mullan
Looks good, very comprehensive test. My only comment is that it would be helpful to add the keytool command you used to create each certificate in a comment above the base64-encoded String. Thanks, Sean On 05/08/2014 09:23 PM, Jamil Nimeh wrote: Hello all, Updated webrev to account for Sean

Re: RFR - 8028627: Unsynchronized code path from javax.crypto.Cipher to the WeakHashMap used by JceSecurity to store codebase mappings

2014-05-16 Thread Sean Mullan
Looks ok to me. While you are in there, can you fix the typo a couple lines above that: s/Retuns/Returns You also need to add an appropriate "noreg" label to the bug. --Sean On 05/16/2014 10:29 AM, Rob McKenna wrote: Hi folks, The synopsis says it all really. There is an unsynchronized code

Re: RFR 8036779: sun.security.krb5.KdcComm interprets kdc_timeout asmsec instead of sec

2014-05-20 Thread Sean Mullan
On 05/19/2014 09:49 AM, Wang Weijun wrote: After some discussion with mit and heimdal lead engineers, I don't want to support ms at the moment. mit does not use kdc_timeout at all and heimdal's internal presentation is of seconds. So this is my plan: support "s" but if unspecified treat it as

Re: RFR 8036709: Java 7 jarsigner displays warning about cert policy tree

2014-05-22 Thread Sean Mullan
Hi Max, Did you consider using a CertPathBuilder instead? This should essentially do the same thing (find a matching trust anchor, and build a validated path). --Sean On 05/21/2014 08:20 PM, Wang Weijun wrote: Hi All Please review the code change at http://cr.openjdk.java.net/~weijun/

Re: RFR 8036709: Java 7 jarsigner displays warning about cert policy tree

2014-05-27 Thread Sean Mullan
On 05/22/2014 07:42 PM, Wang Weijun wrote: On May 23, 2014, at 2:15, Sean Mullan wrote: Hi Max, Did you consider using a CertPathBuilder instead? This should essentially do the same thing (find a matching trust anchor, and build a validated path). I thought about it but anyway the

Re: RFR 8036709: Java 7 jarsigner displays warning about cert policy tree

2014-05-28 Thread Sean Mullan
On 05/28/2014 12:47 AM, Weijun Wang wrote: On 5/28/2014 3:46, Sean Mullan wrote: Ok. The fix looks fine to me, though it looks like the code in the 2 methods is very similar - could it be refactored into a common method? Yes, it's now http://cr.openjdk.java.net/~weijun/8036709/webr

Re: 8044027: Remove unused XML Signature schema and dtd files from source

2014-05-30 Thread Sean Mullan
Looks fine to me. You can remove the schema directory as well since it will be empty after this change. --Sean On 05/30/2014 08:58 AM, Alan Bateman wrote: As part of preparing to restructure the source code as modules, we are finding a number of files in the jdk repository that are not used i

JDK 9 RFR for 8036841 : Reuse no-perms AccessControlContext object when performing isAuthorized check

2014-06-02 Thread Sean Mullan
This is a fix to avoid creating multiple no-perms ACC objects for access control checks. Since this is required in an uncommon scenario, I used the Initialization on demand holder pattern to only create it under those circumstances. webrev: http://cr.openjdk.java.net/~mullan/webrevs/8036841/we

Re: Locking/Singleton in JCAUtil

2014-06-05 Thread Sean Mullan
On 05/22/2014 08:34 PM, Bernd Eckenfels wrote: Hello, by browsing the source code I run across the JCAUtil class. It is (among other stuff) responsible for providing a SecureRandom singleton. The code looks a bit strange. First of all, it defines a LOCK object, but instead of using an unreachab

Re: Webrev request: JDK-8015081

2014-06-05 Thread Sean Mullan
On 06/04/2014 07:29 PM, Jamil Nimeh wrote: Hello all, This is an update to the webrev for JDK-8015081 that takes into account review changes and adds a few more tests. http://cr.openjdk.java.net/~ascarpino/8015081/webrev.03 [1012]

Re: RFR: JDK-8046368

2014-06-10 Thread Sean Mullan
Looks fine to me. Since there is no regression test, please add a "noreg-cleanup" label to the bug. Thanks, Sean On 06/10/2014 02:01 AM, Jamil Nimeh wrote: Hello all, These are some minor clean-up changes in SeedGenerator.java that were found while fixing a different issue. http://cr.openjdk

Re: [9] review request for 8040812: Uninitialised memory in jdk/src/share/native/sun/security/ec/impl/mpi.c

2014-06-10 Thread Sean Mullan
Looks fine to me. Please add a noreg label to the bug before you push. --Sean On 06/10/2014 09:43 AM, Vincent Ryan wrote: Please review this trivial fix to initialize a local variable before use: Bug: https://bugs.openjdk.java.net/browse/JDK-8040812 Webrev: http://cr.openjdk.java.net/~vinnie/8

Re: Webrev request: JDK-8015081

2014-06-10 Thread Sean Mullan
Looks good to me. --Sean On 06/06/2014 06:16 PM, Jamil Nimeh wrote: One more version of this webrev with minor comment changes: http://cr.openjdk.java.net/~ascarpino/8015081/webrev.04 Thanks, --Jamil On 06/04/2014 04:29 PM, Jamil Nimeh wrote: Hello all, This is an update to the webrev for

Re: [9] review request for 8036613: [parfait] JNI exception pending in jdk/src/windows/native/sun/security/provider/WinCAPISeedGenerator.c

2014-06-13 Thread Sean Mullan
Looks ok to me. Bug needs a noreg label. --Sean On 06/10/2014 11:02 AM, Vincent Ryan wrote: Please review this fix to check for a memory allocation failure and clean-up: Bug: https://bugs.openjdk.java.net/browse/JDK-8036613 Webrev: http://cr.openjdk.java.net/~vinnie/8036613/webrev.00/ Thanks.

RFR: JDK-8046044 : Fix raw and unchecked lint warnings in XML Signature Impl

2014-06-16 Thread Sean Mullan
This is a subtask of the task to eliminate all raw and unchecked lint warnings from the JDK. webrev: http://cr.openjdk.java.net/~mullan/webrevs/8046044/webrev.00/ Note that some warnings still require the Suppressed annotation (see DOMXMLSignatureFactory, DOMKeyInfoFactory). These can eventual

Re: [9] RfR 8047085: PKCS11/NSS tests failing intermittently on Windows

2014-06-17 Thread Sean Mullan
The bug needs a noreg label. Also the subcomponent should probably be javax.crypto:pkcs11. Otherwise looks fine to me. --Sean On 06/17/2014 02:41 PM, Vincent Ryan wrote: Please review the following changeset to refresh the version of NSS that is included with the PKCS11 test suite. The old v

Re: RFR 8029994: Support "include" and "includedir" in krb5.conf

2014-06-18 Thread Sean Mullan
Just a few comments on Config.java: 479 if (dups.contains(file)) { 480 throw new IOException("Profile path included more than once"); 481 } else { 482 dups.add(file); 483 } This could be compressed into one call as: if (!dups.add(file)) {

Re: [9] RfR 8047085: PKCS11/NSS tests failing intermittently on Windows

2014-06-18 Thread Sean Mullan
d the subcomponent to also apply to the package or area a test is testing. --Sean On 17 Jun 2014, at 19:55, Sean Mullan wrote: The bug needs a noreg label. Also the subcomponent should probably be javax.crypto:pkcs11. Otherwise looks fine to me. --Sean On 06/17/2014 02:41 PM, Vincent Ryan

Re: RFR 8046046: Test sun/security/pkcs11/Signature/TestDSAKeyLength.java fails intermittently on Solaris 11 in 8u40 nightly

2014-06-19 Thread Sean Mullan
Looks ok to me. --Sean On 06/18/2014 07:04 PM, Valerie Peng wrote: Tony, Can you please help reviewing this trivial fix? Changes are about overriding the SHA1withDSA signature upper limit to 1024 when larger DSA key size is supported. Webrev: http://cr.openjdk.java.net/~valeriep/8046046/webre

Re: RFR 8029994: Support "include" and "includedir" in krb5.conf

2014-06-19 Thread Sean Mullan
On 06/19/2014 01:39 AM, Wang Weijun wrote: >570 public Void run() throws Exception { > >This can be declared to throw IOException, then you can change lines 586-591 to: > >throw pe.getException(); You mean javac will be smart enough to find out that pe's cause can only b

Re: RFR 8029994: Support "include" and "includedir" in krb5.conf

2014-06-19 Thread Sean Mullan
nf.2 not found", the file name will be removed. On Jun 19, 2014, at 21:52, Sean Mullan wrote: On 06/19/2014 01:39 AM, Wang Weijun wrote: 570 public Void run() throws Exception { This can be declared to throw IOException, then you can change lines 586-591 to: throw

Re: RFR 8043406: Change default policy for JCE providers to run with as few privileges,as possible

2014-06-20 Thread Sean Mullan
36 // Needed by Runtime.loadLibrary(String) call 37 permission java.io.FilePermission "<>", "read"; It seems like this is due to a bug in Runtime.loadLibrary, since you have already granted the provider the permission to load the library. I think possibly the call to ClassLo

Re: RFR 8047765: Generate blacklist.certs in build

2014-07-02 Thread Sean Mullan
On 07/02/2014 01:02 AM, Wang Weijun wrote: On Jul 2, 2014, at 12:48, David Holmes wrote: 73 // Output sorted for eye pleasure. ?? "eye pleasure" Well, it's easy for a human to locate one from a sorted output. Or maybe it's because the old one is sorted and I don't want the new on

Re: RFR(S): 8047368: Remove oracle.jrockit.jfr from open package.access list

2014-07-07 Thread Sean Mullan
Looks good. --Sean On 07/06/2014 03:34 PM, Erik Gahlin wrote: Hi, Could I have a review of a small fix that removes references to jfr from the package.access list. Bug: https://bugs.openjdk.java.net/browse/JDK-8047368 Webrev: http://cr.openjdk.java.net/~egahlin/8047368/ Thanks Erik

Re: RFR 8044085: Access ExtendedGSSContext.inquireSecContext() result through SASL

2014-07-07 Thread Sean Mullan
Looks good, just one comment in GssKrb5Base - I would change getNegotiatedProperty to call the superclass method first, and then if that returns null, check for the gss inquiretype properties. This way you don't check for IllegalStateExc twice, and it seems cleaner to me. Also, please add a re

RFR: JDK-8049244: XML Signature performance issue caused by unbuffered signature data

2014-07-08 Thread Sean Mullan
Please review my fix for JDK-8049244: http://cr.openjdk.java.net/~mullan/webrevs/8049244/webrev.00/ This is a direct port of the corresponding fix from Apache Santuario. No regression test since this is a performance related fix. However, the customer that reported this to the Apache Santu

Re: RFR 8049480: Current versions of Java can't verify jars signed and timestamped with Java 9

2014-07-08 Thread Sean Mullan
On 07/08/2014 10:37 AM, Wang Weijun wrote: Please review the jdk7u-only code change at http://cr.openjdk.java.net/~weijun/8049480/webrev.00/ The reason is that the jdk7u version [1] of fix for JDK-8049480 is just a hack and not as powerful as its jdk8 sibling [2] and now I'll have to apply

Re: RFR: 8021804: Certpath validation fails if validity period of root cert does not include validity period of intermediate cert

2014-07-08 Thread Sean Mullan
Looks good to me. --Sean On 07/08/2014 01:22 PM, Seán Coffey wrote: Looking to backport this fix to the JDK 7u code line. The code was refactored in JDK 8 but the change is still easily applied. No issues with JPRT test run. bug ID: https://bugs.openjdk.java.net/browse/JDK-8021804 webrev : htt

Re: RFR 8043071: Expose session key and KRB_CRED through extended GSS-API

2014-07-08 Thread Sean Mullan
Hi Max, Here are my comments: * KerberosKey 37: Did you mean to use @link instead of @see? - several methods are now defined to throw IllegalStateExc, but you (perhaps accidentally) removed the code that does that (ex: getKeyType). * KerberosTicket 352-3: put braces around the if (destroye

Re: RFR 8043406: Change default policy for JCE providers to run with as few privileges,as possible

2014-07-08 Thread Sean Mullan
d at: http://cr.openjdk.java.net/~valeriep/8043406/webrev.01 Sure, I will file a bug after Mandy's confirmation. Thanks, Valerie On 6/20/2014 8:46 AM, Sean Mullan wrote: 36 // Needed by Runtime.loadLibrary(String) call 37 permission java.io.FilePermission "<>", "re

Re: RFR 8043406: Change default policy for JCE providers to run with as few privileges,as possible

2014-07-08 Thread Sean Mullan
ntain the entries for crypto providers. Ok, thanks for the explanation, that sounds good to me. --Sean Thanks, Valerie On 7/8/2014 2:19 PM, Sean Mullan wrote: Looks good, although do you know why you were able to remove the grant AllPermission from so many of the test policy files without gra

Re: RFR 8044085: Access ExtendedGSSContext.inquireSecContext() result through SASL

2014-07-09 Thread Sean Mullan
On 07/09/2014 04:31 AM, Wang Weijun wrote: On Jul 8, 2014, at 2:00, Sean Mullan wrote: Looks good, just one comment in GssKrb5Base - I would change getNegotiatedProperty to call the superclass method first, and then if that returns null, check for the gss inquiretype properties. This way you

Re: RFR 8049480: Current versions of Java can't verify jars signed and timestamped with Java 9

2014-07-09 Thread Sean Mullan
On 07/09/2014 12:31 AM, Wang Weijun wrote: Webrev updated at http://cr.openjdk.java.net/~weijun/8049480/webrev.01/. Looks good to me. --Sean

Re: RFR 8043071: Expose session key and KRB_CRED through extended GSS-API

2014-07-09 Thread Sean Mullan
On 07/09/2014 05:29 AM, Wang Weijun wrote: On Jul 9, 2014, at 3:21, Sean Mullan wrote: - several methods are now defined to throw IllegalStateExc, but you (perhaps accidentally) removed the code that does that (ex: getKeyType). That's because KeyImpl.getKeyType() already throws

Re: RFR 8044085: Access ExtendedGSSContext.inquireSecContext() result through SASL

2014-07-09 Thread Sean Mullan
On 07/09/2014 10:02 AM, Wang Weijun wrote: On Jul 9, 2014, at 20:00, Sean Mullan wrote: Looks good, just one comment in GssKrb5Base - I would change getNegotiatedProperty to call the superclass method first, and then if that returns null, check for the gss inquiretype properties. This way

Re: ThreadLocalRandom clinit troubles

2014-07-14 Thread Sean Mullan
I don't see a pointer to the webrev/patch -- did you forget to include it? --Sean On 07/11/2014 07:33 PM, Martin Buchholz wrote: Thanks to Peter for digging into the secure seed generator classes and coming up with a patch. Openjdk security folks, please review. I confess to getting lost when

Re: RFR [8046343] (smartcardio) CardTerminal.connect('direct') does not work on MacOSX

2014-07-15 Thread Sean Mullan
I would also get Valerie's review before you push this since she is more familiar with this area, but I have a couple of comments: 1. Please add an appropriate noreg label to the bug. 2. The code on lines 65-69 introduces an undesirable dependency on sun.security.action for the JDK 9 modulariz

RFR: JDK-4867890 : Clarify the return value/exception for java.security.SignedObject.verify

2014-07-15 Thread Sean Mullan
Please review this simple fix to clarify the SignatureException thrown by SignedObject.verify: http://cr.openjdk.java.net/~mullan/webrevs/4867890/webrev.00/ Thanks, Sean

Re: RFR: JDK-4867890 : Clarify the return value/exception for java.security.SignedObject.verify

2014-07-16 Thread Sean Mullan
On 07/15/2014 09:16 PM, Wang Weijun wrote: Code change looks fine. I guess this belongs to the category of minor clarification that does not need a CCC? Yes. That is also my opinion. --Sean --Max On Jul 16, 2014, at 4:59, Sean Mullan wrote: Please review this simple fix to clarify the

Re: RFR [8046343] (smartcardio) CardTerminal.connect('direct') does not work on MacOSX

2014-07-16 Thread Sean Mullan
here is no permission needed to get the "os.name" property. The global jre/lib/security/java.policy file also granted all codes to get that. --Max On Jul 15, 2014, at 23:27, Sean Mullan wrote: 2. The code on lines 65-69 introduces an undesirable dependency on sun.security.action for the

Re: RFR [8046343] (smartcardio) CardTerminal.connect('direct') does not work on MacOSX

2014-07-17 Thread Sean Mullan
also added a simple manual test, which should help SQE make sure the bug is gone. Sincerely yours, Ivan On 16.07.2014 16:18, Sean Mullan wrote: On 07/15/2014 10:44 PM, Mandy Chung wrote: But someone could modify the default permissions and take out the PropertyPermission for "os.name"

Re: RFR 8043071: Expose session key and KRB_CRED through extended GSS-API

2014-07-18 Thread Sean Mullan
The updated webrev looks good. --Sean On 07/10/2014 01:59 AM, Wang Weijun wrote: Updated webrev at http://cr.openjdk.java.net/~weijun/8043071/webrev.01/ All your suggestions accepted, plus - New test on new classes added into KerberosHashEqualsTest.java - A duplicate test/sun/security/k

<    1   2   3   4   5   6   7   8   9   10   >