Re: [9] review request for 6977937: The SunJCE PBKDF2KeyImpl is requiring the MAC instance also be from SunJCE.
With the current and proposed code, you are effectively requiring the MAC come from JCE, as all the algorithms exist in SunJCE. IIRC, when we discussed the previous change in this area, the idea was that the MAC would follow the standard JCA provider priority ordering. Brad On 4/4/2014 8:45 AM, Vincent Ryan wrote: Hello, Please review the following fix to remove the requirement for the Mac algorithm used by a PBKDF2 algorithm to be supplied by the SunJCE provider. The SunJCE provider is still preferred (for compatibility with previous releases and for performance reasons) but it is no longer required. The com.sun.crypto.provider.PBKDF2KeyImpl class first searches SunJCE for the required Mac algorithm but fails over to searching the other installed JCE providers too. Bug: https://bugs.openjdk.java.net/browse/JDK-6977937 Webrev: http://cr.openjdk.java.net/~vinnie/6977937/webrev.00/ Thanks.
Review request for JDK 9 Enhancement Proposal: Transitioning to a new default keystore type
Hello, Please review this JDK 9 Enhancement Proposal to transition the default keystore type for the Java platform from Java keystone (JKS) to PKCS12, for improved security. The draft JEP is available at: http://cr.openjdk.java.net/~vinnie/jks-jep.txt Comments are welcome. Thanks.
[9] review request for 6977937: The SunJCE PBKDF2KeyImpl is requiring the MAC instance also be from SunJCE.
Hello, Please review the following fix to remove the requirement for the Mac algorithm used by a PBKDF2 algorithm to be supplied by the SunJCE provider. The SunJCE provider is still preferred (for compatibility with previous releases and for performance reasons) but it is no longer required. The com.sun.crypto.provider.PBKDF2KeyImpl class first searches SunJCE for the required Mac algorithm but fails over to searching the other installed JCE providers too. Bug: https://bugs.openjdk.java.net/browse/JDK-6977937 Webrev: http://cr.openjdk.java.net/~vinnie/6977937/webrev.00/ Thanks.
Re: RFR 8029995: accept yes/no for boolean krb5.conf settings
Looks fine, just one nit. On line 241 of Config.java can you add braces to the if-clause? Thanks, Sean On 04/04/2014 07:00 AM, Wang Weijun wrote: Updated webrev at http://cr.openjdk.java.net/~weijun/8029995/webrev.01 Several differences: 1. Only true/false/yes/no are supported now. 2. 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: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 the new method returns null if the value is not supported. I’ve carefully changed how the method is called to ensure maximum compatibility, but there is still one left: We support DNS lookup for realm name by default, which means we do it if dns_lookup_realm is not set to false, or when unset, if dns_fallback is not set to false. Before this change, when dns_lookup_realm is set to “unknown”, it means false so DNS lookup is not performed. After this change, it’s equivalent to dns_lookup_realm unset, and dns_fallback is used. I think the current behavior is better than the old one. I agree, but since it is a behavior change, it should be specified in the CCC and release notes. Fix looks fine otherwise. --Sean
Re: RFR 8029995: accept yes/no for boolean krb5.conf settings
Updated webrev at http://cr.openjdk.java.net/~weijun/8029995/webrev.01 Several differences: 1. Only true/false/yes/no are supported now. 2. 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: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 the new method returns null if the value is not >> supported. I’ve carefully changed how the method is called to ensure >> maximum compatibility, but there is still one left: >> >> We support DNS lookup for realm name by default, which means we do it >> if dns_lookup_realm is not set to false, or when unset, if >> dns_fallback is not set to false. Before this change, when >> dns_lookup_realm is set to “unknown”, it means false so DNS lookup is >> not performed. After this change, it’s equivalent to dns_lookup_realm >> unset, and dns_fallback is used. I think the current behavior is >> better than the old one. > > I agree, but since it is a behavior change, it should be specified in the CCC > and release notes. > > Fix looks fine otherwise. > > --Sean >
