Re: RFR 8168075: Custom system class loader + security manager + malformed policy file = recursive initialization

2017-01-24 Thread Sean Mullan
On 1/24/17 12:38 PM, Adam Petcher wrote: Thanks Sean and Mandy for the feedback. I believe I incorporated all feedback into the latest patch, with one exception (Sean see below). http://cr.openjdk.java.net/~apetcher/8168075/webrev.02/ In PolicyParser.ParsingException, there is a comment stat

Re: Code Review Request, JDK-8172869 4096 is not supported yet for the DH Parameter Generator

2017-01-24 Thread Sean Mullan
Looks good. --Sean On 1/24/17 2:24 PM, Xuelei Fan wrote: Hi, Please review this spec documentation update: http://cr.openjdk.java.net/~xuelei/8172869/webrev.00/ In the java.security.AlgorithmParameterGenerator specification, 4096 bits DH parameter generator is specified as a required feat

Re: Review Request JDK-8172808: Handle sun.security.util.Resources bundle in ResourcesMgr in the same way as AuthResources

2017-01-25 Thread Sean Mullan
Looks good. --Sean On 1/25/17 11:37 AM, Mandy Chung wrote: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8172808/webrev.01/ This includes a simple patch from Adam to fixup the copyright headers in the fix for JDK-8168075. Mandy On Jan 24, 2017, at 2:25 PM, Mandy Chung wrote: Adam, Sean

Re: [9] RFR: 8168423: Test Task: Custom system class loader + security manager + malformed policy file = recursive initialization

2017-01-26 Thread Sean Mullan
e comments. Seems fine otherwise. --Sean On 1/11/17 10:33 AM, Sibabrata Sahoo wrote: Hi Adam/Sean, This patch is waiting for your review. Thanks, Siba *From:*Sibabrata Sahoo *Sent:* Friday, December 02, 2016 6:56 PM *To:* Sean Mullan; security-dev@openjdk.java.net *Subject:* [9] RFR

Re: Review: release note for JDK-8015081

2017-01-26 Thread Sean Mullan
In the first sentence I would give the fully qualified name (javax.security.auth.Subject) instead of just Subject to provide a bit more context. Otherwise, looks good. --Sean On 1/19/17 12:14 PM, Jamil Nimeh wrote: Hello all, Please review this one release note that documents a change in beh

Re: RFR: 8160655 Fix denyAfter and usage types for security properties

2017-01-26 Thread Sean Mullan
Looks good, mostly minor stuff so far, just have one other file I need more time to review: * java.security Update description of new constraints to match CCC. * PKIXExtendedParameters.java Update class description (it is out-of-date). * CertConstraintParameters.java 2 * Copyright (c) 2016

Re: RFR: 8173581 : performance regression in com/sun/crypto/provider/OutputFeedback.java

2017-01-30 Thread Sean Mullan
The fix looks ok to me, but I would also like Valerie to review it since she is more familiar with this code. As far as JDK 9 goes, we are in Rampdown Phase 1. According to the rules [1], since it is a P3 and is new in JDK 9 we should try to fix this issue if we can. Were you offering to pus

Re: RFR(S): 8173763: Two security tests fail with message: "java.security.NoSuchAlgorithmException: EC KeyFactory not available"

2017-02-01 Thread Sean Mullan
Looks fine to me. --Sean On 2/1/17 10:24 AM, Sergei Kovalev wrote: Hi team, Please review a small fix for tests. Bug ID: https://bugs.openjdk.java.net/browse/JDK-8173763 WebReview: http://cr.openjdk.java.net/~skovalev/8173763/webrev.00/ Issue: Two tests failing during execution with module l

Re: RFR: 8145337: [JVMCI] JVMCI initialization with SecurityManager installed fails: java.security.AccessControlException: access denied

2017-02-01 Thread Sean Mullan
Couple of comments: - jdk.vm.ci is already loaded by the boot loader so it is implicitly granted AllPermission and does not need an entry in default.policy. - all internal APIs in the jdk.vm.compiler module will now be restricted by default by SecurityManager::checkPackageAccess(), so if you

Re: RFR: 8145337: [JVMCI] JVMCI initialization with SecurityManager installed fails: java.security.AccessControlException: access denied

2017-02-01 Thread Sean Mullan
On 2/1/17 4:27 PM, Doug Simon wrote: Can I now consider this change reviewed and integrate it? Yes. --Sean

RFR: 8173827: Remove forRemoval=true from several deprecated security APIs

2017-02-02 Thread Sean Mullan
Please review this change to undo, or remove the forRemoval=true element from the Deprecated annotation of a number of security APIs. Since marking these APIs for removal in a future version of SE, it has since been reported that some external applications/code are still using these APIs, and t

Re: RFR: 8173827: Remove forRemoval=true from several deprecated security APIs

2017-02-03 Thread Sean Mullan
emoval to some later release rather than drop the intent to remove like this. /Claes Sean Mullan skrev: (2 februari 2017 19:35:03 CET) Please review this change to undo, or remove the forRemoval=true element from the Deprecated annotation of a number of security APIs. Since marking th

Re: RFR: 8160655 Fix denyAfter and usage types for security properties

2017-02-06 Thread Sean Mullan
Hi Tony, Here are my comments on the latest webrev: * SignerInfo.java 355 try { 356 JAR_DISABLED_CHECK.permits(digestAlgname, cparams); 357 } catch (CertPathValidatorException e) { 358 throw new SignatureException(e.g

Re: [9] RFR: 8168423: Test Task: Custom system class loader + security manager + malformed policy file = recursive initialization

2017-02-08 Thread Sean Mullan
bdirectories contained in that directory. --Sean 2) In ClassLoaderTest.java, @bug renamed from 8168423 to 8168075. 3) In ClassLoaderTest.java, the code comments has been removed from @summary section. But it retains the same at line: 91-102. Thanks, Siba -Original Message- From:

Re: RFR: 8160655 Fix denyAfter and usage types for security properties

2017-02-08 Thread Sean Mullan
01 at that time. Tony On 02/06/2017 12:17 PM, Sean Mullan wrote: Hi Tony, Here are my comments on the latest webrev: * SignerInfo.java 355 try { 356 JAR_DISABLED_CHECK.permits(digestAlgname, cparams); 357 } catch (CertPathValidatorE

Re: RFR 8174157: Backout 8151116

2017-02-08 Thread Sean Mullan
Looks fine. --Sean On 2/8/17 6:33 AM, Anthony Scarpino wrote: Please review this simple backout. I pushed using the wrong bug, so I need to back this out, so I can push under the correct bug (8173410). http://cr.openjdk.java.net/~ascarpino/8174157/webrev/ thanks Tony

Re: [9] RFR: 8168423: Test Task: Custom system class loader + security manager + malformed policy file = recursive initialization

2017-02-09 Thread Sean Mullan
Original Message----- From: Sean Mullan Sent: Wednesday, February 08, 2017 10:00 PM To: Sibabrata Sahoo; Adam Petcher; security-dev@openjdk.java.net Subject: Re: [9] RFR: 8168423: Test Task: Custom system class loader + security manager + malformed policy file = recursive initialization On 2/7/17 4:26

Re: RFR release notes for multiple enhancements: krb5, SASL, JAAS, policytool

2017-02-13 Thread Sean Mullan
On 1/19/17 9:20 AM, Weijun Wang wrote: On 01/19/2017 10:45 AM, Weijun Wang wrote: One more: https://bugs.openjdk.java.net/browse/JDK-8173018 (https://bugs.openjdk.java.net/browse/JDK-8076535) Deprecate the com.sun.jarsigner package The `com.sun.jarsigner` package is being deprecated. This

Re: RFR release notes for multiple enhancements: krb5, SASL, JAAS, policytool

2017-02-13 Thread Sean Mullan
On 1/19/17 9:20 AM, Weijun Wang wrote: Another one: https://bugs.openjdk.java.net/browse/JDK-8173035 (https://bugs.openjdk.java.net/browse/JDK-8029904) Remove com.sun.security.auth.callback.DialogCallbackHandler `com.sun.security.auth.callback.DialogCallbackHandler` has been removed in JDK

RFR: 8174837: Add "since=9" to deprecated ContentSigner and ContentSignerParameters classes

2017-02-13 Thread Sean Mullan
Could I get a quick code review for this simple fix for https://bugs.openjdk.java.net/browse/JDK-8174837?: diff --git a/src/jdk.jartool/share/classes/com/sun/jarsigner/ContentSigner.java b/src/jdk.jartool/share/classes/com/sun/jarsigner/ContentSigner.java --- a/src/jdk.jartool/share/classes/c

Re: [RFR] 8174849: Change SHA1 certpath restrictions

2017-02-13 Thread Sean Mullan
Looks fine. You'll need to add a noreg label to the bug though. --Sean On 2/13/17 4:47 PM, Anthony Scarpino wrote: Hi, I need a quick review on a simple certpath config change. http://cr.openjdk.java.net/~ascarpino/8174849/webrev/ thanks Tony

Re: RFR 8174909: Doc error in SecureRandom

2017-02-14 Thread Sean Mullan
Looks good. --Sean On 2/14/17 5:55 AM, Wang Weijun wrote: Please review this doc bug diff --git a/src/java.base/share/classes/java/security/DrbgParameters.java b/src/java.base/share/classes/java/security/DrbgParameters.java --- a/src/java.base/share/classes/java/security/DrbgParameters.java ++

Re: [RFR] 8174849: Change SHA1 certpath restrictions

2017-02-14 Thread Sean Mullan
On 2/14/17 2:33 AM, Bernd Eckenfels wrote: Hello, The bug does not explain why. I would understand to completely deny SHA1 (I.e. Unconditionally), but allowing it seems strange, especially without a justification. The initial disabling of SHA-1 certificates in JDK 9 is too broad and affects a

Re: RFR 8168410: Multiple JCK tests are failing due to SecurityException is not thrown.

2017-02-14 Thread Sean Mullan
Hi Max, I agree this change is necessary so that we can resolve this tck-red issue before ZBB. However, since the TCK Policy provider implementation is not a "typical" implementation in the sense that it is denying permissions instead of granting permissions, I think we should continue to exp

Re: RFR 8168410: Multiple JCK tests are failing due to SecurityException is not thrown.

2017-02-15 Thread Sean Mullan
Looks good. --Sean On 2/14/17 8:04 PM, Weijun Wang wrote: Webrev updated at http://cr.openjdk.java.net/~weijun/8168410/webrev.01/ Change since webrev.00: http://cr.openjdk.java.net/~weijun/8168410/webrev.01/interdiff.patch.html Thanks Max On 02/14/2017 11:55 PM, Sean Mullan wrote

Re: RFR 8168410: Multiple JCK tests are failing due to SecurityException is not thrown.

2017-02-15 Thread Sean Mullan
10/webrev.01/ Change since webrev.00: http://cr.openjdk.java.net/~weijun/8168410/webrev.01/interdiff.patch.html Thanks Max On 02/14/2017 11:55 PM, Sean Mullan wrote: Hi Max, I agree this change is necessary so that we can resolve this tck-red issue before ZBB. However, since the TCK Policy provider implementa

Re: [JDK-8146293] - Proposal to fix RSASSA-PSS AlgorithmChecker constraints for TLS 1.2

2017-02-17 Thread Sean Mullan
Hi Chris, Comments inline ... On 2/10/17 4:41 PM, Christopher Fox wrote: We have been looking into supporting RSASSA-PSS signature algorithms within the chain of an end-entity certificate used for TLS 1.2. The EE certificate itself is not signed with RSASSA-PSS. As mentioned in JDK-8146293

Re: [JDK-8146293] - Proposal to fix RSASSA-PSS AlgorithmChecker constraints for TLS 1.2

2017-02-17 Thread Sean Mullan
s Fox *From:* Sean Mullan *Sent:* Friday, February 17, 2017 10:57:33 AM *To:* Christopher Fox; security-dev@openjdk.java.net *Cc:* Glen Beasley; Timothy Jackson *Subject:* Re: [JDK-8146293] - Proposal to fix RSASSA-PSS AlgorithmChecker constraint

Re: RFR 8175250: Manifest checking throws exception with no entry

2017-02-22 Thread Sean Mullan
Looks ok. I think another way to fix this is to set weakAlgs to false initially, and then only set it to true if permittedCheck returns false. You need a reason for the noreg label (i.e. noreg-{something}). --Sean On 2/22/17 2:29 PM, Anthony Scarpino wrote: Hi, I need a review of this small

Re: RFR 8175250: Manifest checking throws exception with no entry

2017-02-22 Thread Sean Mullan
On 2/22/17 3:21 PM, Sean Mullan wrote: Looks ok. I think another way to fix this is to set weakAlgs to false initially, and then only set it to true if permittedCheck returns false. Never mind the comment above, this would not work because as long as you have one strong alg, it is ok

Re: RFR 8175846: Provide javadoc descriptions for jdk.policytool and jdk.crypto.* modules

2017-03-07 Thread Sean Mullan
On 3/7/17 9:44 AM, Weijun Wang wrote: Ping again. Sorry I missed this before you pushed the changeset, but the @Deprecated annotation for the jdk.policytool module should have "forRemoval=true" on it, since it scheduled for removal in JDK 10: https://bugs.openjdk.java.net/browse/JDK-8147400

Re: RFR 8176350: Usage constraints don't take effect when using PKIX

2017-03-08 Thread Sean Mullan
- DisabledAlgorithmConstraints.java You can just call Thread.dumpStack (which dumps to stderr) here if debug is set. OTOH, I am concerned this will quickly fill up the log file with stack traces (which can be long). Do we really need this? --Sean On 3/8/17 4:15 PM, Anthony Scarpino wrote: H

Re: RFR 8176350: Usage constraints don't take effect when using PKIX

2017-03-09 Thread Sean Mullan
On 3/8/17 5:52 PM, Anthony Scarpino wrote: Since Sean and you are asking similar questions, I'll answer it here. It was to be consistent with by using the Debug class. If a change is made to Debug to print elsewhere, what I have now is compatible. Sending it to stderr is basically saying that De

Re: RFR 8176350: Usage constraints don't take effect when using PKIX

2017-03-09 Thread Sean Mullan
Looks good. Minor comment in PKIXCertPathValidator.java you can combine lines 193-194. --Sean On 3/9/17 1:29 PM, Anthony Scarpino wrote: On 03/08/2017 01:15 PM, Anthony Scarpino wrote: Hi, I need a code review of this small change.. The PKIX path for usage checking didn't pass the variant to

Re: RFR 8176542: Missing @Deprecated arguments for jdk.policytool

2017-03-13 Thread Sean Mullan
Looks fine. --Sean On 3/11/17 5:28 AM, Weijun Wang wrote: Please take a review on the patch below. Notes: 1. The current javadoc output expands the arguments into sentences like "Deprecated, for removal: This API element is subject to removal in a future version." which means there is no need

RFR: JDK-8176503: Disable SHA-1 TLS Server Certificates

2017-03-13 Thread Sean Mullan
Please review this configuration change to disable SHA-1 TLS server certificates by default in JDK 9. In order to be disabled, the certificates must chain back to trusted root certificate in the cacerts keystore that has a " [jdk]" attribute appended to their alias name. --Sean diff --git a/s

Re: RFR 8176715: sun/security/krb5/auto/HttpNegotiateServer.java does not compile

2017-03-14 Thread Sean Mullan
Looks good. --Sean On 3/13/17 11:38 PM, Weijun Wang wrote: Please review the patch below: diff --git a/test/sun/security/krb5/auto/HttpNegotiateServer.java b/test/sun/security/krb5/auto/HttpNegotiateServer.java --- a/test/sun/security/krb5/auto/HttpNegotiateServer.java +++ b/test/sun/security/

Re: [9] RFR 8175251: Failed to load RSA private key from pkcs12

2017-03-14 Thread Sean Mullan
The fix looks good. --Sean On 3/13/17 9:25 PM, Valerie Peng wrote: Re-send with the correct release version. This fix is intended for JDK-9. Thanks, Valerie On 3/13/2017 6:24 PM, Valerie Peng wrote: Hi Sean, Can you please help reviewing the fix for JDK-8175251 "Failed to load RSA private

Re: [9] RFR 8175251: Failed to load RSA private key from pkcs12

2017-03-15 Thread Sean Mullan
Looks good. --Sean On 3/15/17 3:35 PM, Valerie Peng wrote: Sean, I added a regression test and fixed a minor issue in existing regression test. Can you please take a look? http://cr.openjdk.java.net/~valeriep/8175251/webrev.01/ Thanks, Valerie On 3/14/2017 12:54 PM, Sean Mullan wrote: The

Re: RFR 8176536: Backport weak algorithms checking

2017-03-17 Thread Sean Mullan
Looks good to me. Please also include the recent fix to disable SHA-1 TLS Server certificates in this backport: https://bugs.openjdk.java.net/browse/JDK-8176503 --Sean On 3/16/17 1:04 AM, Anthony Scarpino wrote: Hi, I need a review of this large backport of the weak algorithm checking code t

Re: Review: Release note for JDK-8042967

2017-03-20 Thread Sean Mullan
Can you also add NONEwithDSAinP1363Format and NONEwithECDSAinP1363Format? Looks good otherwise. --Sean On 3/19/17 1:28 AM, Jamil Nimeh wrote: --Resending on the open alias Hi guys, Can someone give me a review of this release note for an RFE that Jason put into JDK 9? Release note: https://

Re: RFR 8176536: Backport weak algorithms checking

2017-03-21 Thread Sean Mullan
rpino wrote: Oh yeah. I had forgot to resync to get your change. Thanks for the reminder. Tony On Mar 17, 2017, at 1:06 PM, Sean Mullan wrote: Looks good to me. Please also include the recent fix to disable SHA-1 TLS Server certificates in this backport: https://bugs.openjdk.java.net/brows

Re: RFR: 3 security-libs release notes on keytool/krb5/etc

2017-03-24 Thread Sean Mullan
On 3/24/17 7:01 AM, Weijun Wang wrote: I'll use "short key". I prefer the term "weak" which implies it is a risk. We already use that term in jarsigner so I think we should keep it consistent. You also print the size of the key so that describes what is wrong with it. --Sean Thanks Max

Re: JDK-8016345 (DNSName does not accept names with leading numbers) will-not-fix? Why?

2017-03-27 Thread Sean Mullan
Good catch, I actually closed it as a duplicate but marked it as a duplicate of the wrong bug, it should have been https://bugs.openjdk.java.net/browse/JDK-8054380 I fixed it so 8054380 is now the duplicate. --Sean On 3/27/17 9:26 AM, Thomas Stüfe wrote: Hi all, just a question, I hope this

Re: JDK-8016345 (DNSName does not accept names with leading numbers) will-not-fix? Why?

2017-03-27 Thread Sean Mullan
certificates with keytool and not when validating certificates. You could try using another tool to create certificates. --Sean ..Thomas On Mon, Mar 27, 2017 at 4:42 PM, Sean Mullan mailto:sean.mul...@oracle.com>> wrote: Good catch, I actually closed it as a duplicate but marked i

Re: JDK-8016345 (DNSName does not accept names with leading numbers) will-not-fix? Why?

2017-03-27 Thread Sean Mullan
On 3/27/17 11:41 AM, Thomas Stüfe wrote: thanks for the info. So, basically, there is no particular reason I'm too blind to see not to fix the parser to accept hostnames with leading numbers, e.g., just lack of time/resources? I am asking because then we could fix it in our product downstream an

Re: RFR: 3 security-libs release notes on keytool/krb5/etc

2017-03-29 Thread Sean Mullan
The last two release notes look fine to me. I'll review the first one later. Thanks, Sean On 3/23/17 9:12 PM, Weijun Wang wrote: Hi All Please take a review on 3 release notes. The content itself is pasted as quotation below. https://bugs.openjdk.java.net/browse/JDK-8176087 keytool now prints

Re: [9] RFR 8177569: keytool should not warn if signature algorithm used in cacerts is weak

2017-03-29 Thread Sean Mullan
On 3/29/17 12:13 PM, Xuelei Fan wrote: I see the point that a trust anchor should be trusted. In application level, we don't actually check weakness of trust anchor because the user has made the decision to trust the cert. However, in keytool level, I think it might be nice to warning weakness

Re: [9] RFR 8177569: keytool should not warn if signature algorithm used in cacerts is weak

2017-03-29 Thread Sean Mullan
The updated fix looks good to me. --Sean On 3/29/17 4:38 AM, Weijun Wang wrote: Webrev updated at http://cr.openjdk.java.net/~weijun/8177569/webrev.01 Changes since last version: - Trusted cert entries in the current keystore are also trusted. See the new isTrusted() method. - A cert is t

[10] RFR: 8175029: StackOverflowError in X509CRL and X509Certificate.verify(PublicKey, Provider)

2017-03-31 Thread Sean Mullan
Please review this fix for a StackOverflowError caused by the default implementation of verify(PublicKey, Provider). bug: https://bugs.openjdk.java.net/browse/JDK-8175029 webrev: http://cr.openjdk.java.net/~mullan/webrevs/8175029/webrev.00/ Thanks, Sean

Re: [10] RFR: 8175029: StackOverflowError in X509CRL and X509Certificate.verify(PublicKey, Provider)

2017-03-31 Thread Sean Mullan
06 PM, Sean Mullan wrote: Please review this fix for a StackOverflowError caused by the default implementation of verify(PublicKey, Provider). bug: https://bugs.openjdk.java.net/browse/JDK-8175029 webrev: http://cr.openjdk.java.net/~mullan/webrevs/8175029/webrev.00/ Thanks, Sean

Re: [10] RFR: 8175029: StackOverflowError in X509CRL and X509Certificate.verify(PublicKey, Provider)

2017-03-31 Thread Sean Mullan
New webrev uploaded to address your comments: http://cr.openjdk.java.net/~mullan/webrevs/8175029/webrev.01/ --Sean On 3/31/17 9:11 AM, Sean Mullan wrote: On 3/31/17 9:08 AM, Weijun Wang wrote: Can we just move the code you just added into the public API directly? Looks like it does not

Re: RFR 8177291: [doc] weak algorithms and crypto policy in JGSS docs

2017-04-03 Thread Sean Mullan
Hi Max, Just a few comments: "The default jurisdiction policy files bundled in Java SE is now unlimited, which means the AES-256 encryption type is available by default." s/is/are/ "The DES based encryption types (including des-cbc-md5 and des-cbc-crc) are disabled by default." Was this i

Re: RFR: 3 security-libs release notes on keytool/krb5/etc

2017-04-03 Thread Sean Mullan
On 3/29/17 10:33 AM, Sean Mullan wrote: https://bugs.openjdk.java.net/browse/JDK-8176087 keytool now prints warnings when reading or generating cert/cert req using weak algorithms In all keytool functions, if the certificate/certificate request/CRL that is working on (whether it be the input

Re: RFR 8177291: [doc] weak algorithms and crypto policy in JGSS docs

2017-04-04 Thread Sean Mullan
On 4/3/17 10:44 AM, Weijun Wang wrote: "First, you need to update to use the KDC that supports the required Kerberos encryption types, such as latest Solaris or the MIT Kerberos from MIT distribution. If you are using Active Directory on a Windows platform, the latest version also supports RC4-HM

[10] RFR: 8161973: PKIXRevocationChecker.getSoftFailExceptions() not working

2017-04-06 Thread Sean Mullan
Please review this fix to the getSoftFailExceptions method of PKIXRevocationChecker so that it does not always return an empty List. The problem was that the clone method of the PKIXRevocationChecker implementation was creating a new List each time it was called, and the Exceptions were not bei

Re: RFR 8177784 Use CounterMode intrinsic for AES/GCM

2017-04-10 Thread Sean Mullan
On 4/6/17 4:39 PM, Anthony Scarpino wrote: I'd like to get a review for this performance change to use the existing CounterMode parallelized intrinsic instead of GCTR's own version. The two classes were nearly identical except for the doFinal() method which doesn't belong in CounterMode.java. I

Re: RFR 8177784 Use CounterMode intrinsic for AES/GCM

2017-04-10 Thread Sean Mullan
On 4/7/17 3:47 PM, Anthony Scarpino wrote: On 04/07/2017 06:58 AM, Chris Hegarty wrote: On 06/04/17 21:39, Anthony Scarpino wrote: I'd like to get a review for this performance change to use the existing CounterMode parallelized intrinsic instead of GCTR's own version. The two classes were nea

Re: RFR 8177784 Use CounterMode intrinsic for AES/GCM

2017-04-11 Thread Sean Mullan
http://cr.openjdk.java.net/~ascarpino/8177784/webrev.02/ Looks good. --Sean

Re: RFR: Remove map synchronization from SignatureAndHashAlgorithm

2017-04-25 Thread Sean Mullan
Hi Steven, Thanks, I filed an issue on your behalf for tracking purposes: https://bugs.openjdk.java.net/browse/JDK-8179275 Also, have you signed the OCA? See http://www.oracle.com/technetwork/community/oca-486395.html for more information. --Sean On 4/15/17 3:25 PM, Steven Davidovitz wrot

Re: RFR: Remove map synchronization from SignatureAndHashAlgorithm

2017-04-26 Thread Sean Mullan
2017 at 7:05 AM, Sean Mullan mailto:sean.mul...@oracle.com>> wrote: Hi Steven, Thanks, I filed an issue on your behalf for tracking purposes: https://bugs.openjdk.java.net/browse/JDK-8179275 <https://bugs.openjdk.java.net/browse/JDK-8179275> Also, have you s

Re: RFR 8179369: src/java.security.jgss/share/classes/org/ietf/jgss/package.html should be HTML5-friendly

2017-04-27 Thread Sean Mullan
Update the copyright date. Otherwise looks fine. --Sean On 4/27/17 10:37 AM, Weijun Wang wrote: Please review this small doc change: diff --git a/src/java.security.jgss/share/classes/org/ietf/jgss/package.html b/src/java.security.jgss/share/classes/org/ietf/jgss/package.html --- a/src/java.sec

Re: RFR: 8178014: CryptoPolicyParser's API comment contains < and > characters

2017-05-01 Thread Sean Mullan
Looks good to me. --Sean On 5/1/17 4:19 PM, Brad R. Wetmore wrote: Kumar (+ anyone else), Simple review for fixing two errors when building javadocs with -private (non-default option). Also, the 'name' attribute is no longer supported in HTML5. So doing a trivial name->id rename to HTML5 cl

Re: RFR: JDK-8178278 Move Standard Algorithm Names document to specs directory

2017-05-05 Thread Sean Mullan
General: change the text of all of the links from "Java Cryptography Architecture Standard Algorithm Name Documentation" (the old name) to "Java Security Standard Algorithm Names Specification". Looks good otherwise. Thanks, Sean On 5/5/17 9:17 AM, Magnus Ihse Bursie wrote: The Security Stan

Re: [10] RFR 8179389: X509Certificate generateCRLs is extremely slow using a PEM crl list

2017-05-10 Thread Sean Mullan
On 4/28/17 8:44 PM, Weijun Wang wrote: Please take a review at http://cr.openjdk.java.net/~weijun/8179389/webrev.00/ The bug report [1] is correct that the frequent reallocation of data is the problem. This fix above delegate this task to ByteArrayOutputStream. There is no significant pe

Re: FW: SecurityManager.checkPackageAccess for qualified exports

2017-05-12 Thread Sean Mullan
On 5/12/17 3:26 AM, Langer, Christoph wrote: Adding security-dev… Any comments? - Hi all, while playing with the security manager (using -Djava.security.manager) in Java 9 and testing platform modules that we have add

Re: FW: SecurityManager.checkPackageAccess for qualified exports

2017-05-12 Thread Sean Mullan
On 5/12/17 9:14 AM, Langer, Christoph wrote: Hi Sean, thanks for your response. *Implementation Note:*** This implementation also restricts all non-exported packages of modules loaded bythe platform class loader

[9] RFR: 8180307: Add new JDK 9 Required Algorithms to Cipher class

2017-05-12 Thread Sean Mullan
A couple of new required algorithms for JDK 9 were accidentally omitted for the Cipher class as part of JDK-8015388. Docs only change. diff --git a/src/java.base/share/classes/javax/crypto/Cipher.java b/src/java.base/share/classes/javax/crypto/Cipher.java --- a/src/java.base/share/classes/java

Re: [9] RFR: 8180307: Add new JDK 9 Required Algorithms to Cipher class

2017-05-15 Thread Sean Mullan
it was already (accidentally) approved as part of JDK-8015388. --Sean Brad Valerie On 5/12/2017 10:59 AM, Sean Mullan wrote: A couple of new required algorithms for JDK 9 were accidentally omitted for the Cipher class as part of JDK-8015388. Docs only change. diff --git a/src/java.base

Re: [10] RFR 8166222: Don't treat signed jars with invalid timestamps as unsigned

2017-05-16 Thread Sean Mullan
* Resources.java [278] s/includes/include/ * SignatureFileVerifier [728] setting ts to null in the catch block seems a bit ugly. Maybe it would be better to refactor that code into a static method that returns a valid Timestamp or null if not. * Main.java [271-2] Perhaps you should use a d

Re: JPMS Access Checks, Verification and the Security Manager

2017-05-23 Thread Sean Mullan
On 5/23/17 10:44 AM, Volker Simonis wrote: The other question is if '--add-exports' should implicitly grant the corresponding security permissions? I understand that package/class visibility and security checks are somewhat orthogonal concepts so I tend to agree with the current functionality (an

Re: RFR 8178794: krb5 client should ignore sname in incoming tickets

2017-05-25 Thread Sean Mullan
Looks fine to me. --Sean On 5/10/17 7:35 PM, Weijun Wang wrote: Ping again. On 04/14/2017 03:18 PM, Weijun Wang wrote: Hi Valerie Please take a review of http://cr.openjdk.java.net/~weijun/8178794/webrev.00/ The sname field in the ticket is meant to be read by a server and the client sh

Re: [9] RFR 8180635: (doc) Clarify the compatibility and interoperability issue when using provider default values

2017-05-26 Thread Sean Mullan
Just a couple of comments: - the same text is in KeyPairGeneratorSpi and AlgorithmParameterGeneratorSpi so we should add the warning in those classes too - we should add the same warning to javax.crypto.KeyGenerator and KeyGeneratorSpi - Please change "Oracle Providers" to "JDK Providers",

Re: [9] RFR 8180635: (doc) Clarify the compatibility and interoperability issue when using provider default values

2017-05-30 Thread Sean Mullan
viders. --Sean On 5/26/17 6:54 PM, Valerie Peng wrote: Sean, Thanks for the wording suggestions. Webrev updated at: http://cr.openjdk.java.net/~valeriep/8180635/webrev.01/ Have a wonderful long weekend, Valerie On 5/26/2017 5:48 AM, Sean Mullan wrote: Just a couple of comments: - the same

Re: [9] RFR 8180635: (doc) Clarify the compatibility and interoperability issue when using provider default values

2017-05-31 Thread Sean Mullan
Valerie On 5/30/2017 10:50 AM, Sean Mullan wrote: Just a couple more comments, in this text in each class: * Java™ * Cryptography Architecture JDK Providers Documentation * document for information on the AlgorithmParameterGenerator defaults * used by JDK providers. Don't use bold () on the

Re: RFR JDK-8179614: Test for jarsigner on verifying jars that are signed and timestamped by other JDK releases

2017-06-06 Thread Sean Mullan
Hi John, This looks like a very useful test. I have not gone through all of the code, but here are a few comments for now until I have more time: - add tests for EC keys - add tests for SHA-512 variants of the signature algorithms - add tests for larger key sizes (ex: 2048 for DSA/RSA) - you c

Re: RFR JDK-8179614: Test for jarsigner on verifying jars that are signed and timestamped by other JDK releases

2017-06-07 Thread Sean Mullan
On 6/6/17 9:14 PM, sha.ji...@oracle.com wrote: Hi Sean, On 07/06/2017 04:27, Sean Mullan wrote: Hi John, This looks like a very useful test. I have not gone through all of the code, but here are a few comments for now until I have more time: - add tests for EC keys - add tests for SHA-512

Re: Stricter Public Key checking corrupts JKS

2017-06-12 Thread Sean Mullan
Hi Bernd, This issue should be fixed in 8u131. Can you try that and let us know? --Sean On 6/9/17 10:18 PM, Bernd wrote: I noticed there is a bug (8177657,etc) about stricter DER checking on JDK Certificate code. I have an JKS Keystore which no longer can be opened because of that. I unders

Re: [9] RFR 8181978: Keystore probing mechanism fails for large PKCS12 keystores

2017-06-13 Thread Sean Mullan
Looks fine to me. --Sean On 6/13/17 7:31 AM, Vincent Ryan wrote: Martin has reported a serious regression involving PKCS12 keystores in JDK 9. It affects large PKCS12 keystores loaded using the new KeyStore.getInstance(File, xxx) method. The error is due to a typo in the masks used by the keys

RFR [9]: 8181295: Document that SecurityManager::checkPackageAccess may be called by the VM

2017-06-16 Thread Sean Mullan
Please review this clarification to the SecurityManager::checkPackageAccess method to note that the method may be called by the Virtual Machine when loading classes: http://cr.openjdk.java.net/~mullan/webrevs/8181295/webrev.00/ A small correction was also made to the checkPackageDefinition met

Re: RFR [9]: 8181295: Document that SecurityManager::checkPackageAccess may be called by the VM

2017-06-16 Thread Sean Mullan
On 6/16/17 11:13 AM, Mandy Chung wrote: On Jun 16, 2017, at 8:00 AM, Sean Mullan wrote: Please review this clarification to the SecurityManager::checkPackageAccess method to note that the method may be called by the Virtual Machine when loading classes: http://cr.openjdk.java.net/~mullan

Re: RFR 8182118: Package summary is missing in jdk.security.auth module

2017-06-19 Thread Sean Mullan
On 6/19/17 12:33 AM, Mandy Chung wrote: On Jun 18, 2017, at 8:33 PM, Weijun Wang wrote: Hi All Please take a review at http://cr.openjdk.java.net/~weijun/8182118/webrev.00/ Basically, a description line is added into package-info.java of each of these packages: - com/sun/security/auth

Re: RFR 8182118: Package summary is missing in jdk.security.auth module

2017-06-19 Thread Sean Mullan
On 6/19/17 8:17 AM, Weijun Wang wrote: There is more than one, so this should be "implementations". In fact, I originally used "implementations" (without "the") and "an implementation", but then I saw the module-info.java for the module saying "Contains the implementation of the javax.securit

Re: RFR 8182118: Package summary is missing in jdk.security.auth module

2017-06-19 Thread Sean Mullan
: Updated at http://cr.openjdk.java.net/~weijun/8182118/webrev.02/. --Max On 06/19/2017 08:23 PM, Sean Mullan wrote: On 6/19/17 8:17 AM, Weijun Wang wrote: There is more than one, so this should be "implementations". In fact, I originally used "implementations" (wi

Re: How do I know which granted permission is not needed?

2017-06-21 Thread Sean Mullan
On 6/21/17 3:05 AM, Weijun Wang wrote: Suppose I have a Java program running with a security manager and a policy file. There are quite a lot of permissions granted in the policy file but maybe not all of them are necessary. Is there a way I can find out which one is not needed? I don't know

Re: How do I know which granted permission is not needed?

2017-06-21 Thread Sean Mullan
he same permission. By reading the -Djava.security.debug=access output I cannot find out if one is actually not needed. Daniel suggests I can write my own Policy implementation. https://docs.oracle.com/javase/8/docs/technotes/guides/security/troubleshooting-security.html Regards, Sean. On 21/06

Re: AccessController.doPrivileged and default methods

2017-07-06 Thread Sean Mullan
On 7/5/17 4:01 PM, Adam Petcher wrote: Very interesting. The exception originates in the VM, so I added the hotspot-dev list to the discussion. It looks like the VM either can't find the method, or it decides that it's not the right kind of method. Right. The check that is causing this error

Re: [10] RFR 8183509: keytool should not allow multiple commands

2017-07-07 Thread Sean Mullan
I would use "specified" instead of "provided" since that word is used more commonly in the keytool warnings. Thanks, Sean On 7/7/17 3:58 AM, Weijun Wang wrote: Please take a look at http://cr.openjdk.java.net/~weijun/8183509/webrev.00/ Multiple commands on the same keytool command line is no

Re: AccessController.doPrivileged and default methods

2017-07-07 Thread Sean Mullan
On 7/6/17 10:33 AM, Sean Mullan wrote: On 7/5/17 4:01 PM, Adam Petcher wrote: Very interesting. The exception originates in the VM, so I added the hotspot-dev list to the discussion. It looks like the VM either can't find the method, or it decides that it's not the right kind

Re: [10] RFR 8183509: keytool should not allow multiple commands

2017-07-07 Thread Sean Mullan
Looks good. --Sean On 7/7/17 10:55 AM, Weijun Wang wrote: Updated at http://cr.openjdk.java.net/~weijun/8183509/webrev.01/ Changes: http://cr.openjdk.java.net/~weijun/8183509/webrev.01/interdiff.patch.html Thanks Max On Jul 7, 2017, at 8:00 PM, Sean Mullan wrote: I would use

Re: [RFR] 8174849: Change SHA1 certpath restrictions - issue with 3rd party JCE provider

2017-07-11 Thread Sean Mullan
Langer, Christoph Sent: Sonntag, 9. Juli 2017 07:57 To: 'Anthony Scarpino' ; 'Sean Mullan' Cc: OpenJDK Security ; 'Dieter Bratko' Subject: RE: [RFR] 8174849: Change SHA1 certpath restrictions - issue with 3rd party JCE provider Hi Tony et. al., I'm wondering why

Re: [RFR] 8174849: Change SHA1 certpath restrictions - issue with 3rd party JCE provider

2017-07-12 Thread Sean Mullan
On 7/11/17 3:10 PM, Langer, Christoph wrote: Well, probably you are right that it is not a bug - at least when you look at the documentation of Java9 (the link that you have cited). However, if we look at the documentation of X509Certificate, it's not that clear, resp. it wasn't for pre JDK9 r

Re: [10] RFR 8166222: Don't treat signed jars with invalid timestamps as unsigned

2017-07-14 Thread Sean Mullan
Finally getting back to reviewing this update. A few comments: SignatureFileVerifier.java: 729 debug.println("getTimeStamp caught: "+e); Can you add a more descriptive message here, like: "Exception processing timestamp, code will be treated as signed, but not timestamped:

Re: JDK-8182879: Add warnings to keytool when using JKS and JCEKS

2017-07-14 Thread Sean Mullan
On 7/14/17 11:12 AM, Weijun Wang wrote: On Jul 14, 2017, at 7:00 PM, Sean Mullan wrote: I think we should add a Release Note to 8182879 indicating that keytool now emits a warning for JKS/JCEKS keystores. https://bugs.openjdk.java.net/browse/JDK-8184671 filed. Please take a review

Re: RFR 10 (XS): 8184673: Fix compatibility issue in AlgorithmChecker for 3rd party JCE providers

2017-07-14 Thread Sean Mullan
It would be nice to write a regression test for this, but I suspect it is quite a bit of work or not practical. Please consider it, or add an appropriate noreg label to the bug. --Sean On 7/14/17 12:56 PM, Anthony Scarpino wrote: On 07/14/2017 08:37 AM, Langer, Christoph wrote: Hi, after th

Re: 8184916: DisabledAlgorithmConstraints loading should be delayed until needed

2017-07-19 Thread Sean Mullan
No objection. Looks fine to me. --Sean On 7/19/17 7:44 AM, Alan Bateman wrote: I'm looking at the performance of an app that initializes a lot more security classes than I expected. One part to this is the initial opening of a JAR file which ends up loading a lot of machinery that should only

Re: RFR 8183591: Incorrect behavior when reading DER value with Integer.MAX_VALUE length

2017-07-20 Thread Sean Mullan
Looks good to me. --Sean On 7/20/17 9:49 AM, Adam Petcher wrote: Oops. Better to throw an IOException when a negative length is given to readFully. Webrev: http://cr.openjdk.java.net/~apetcher/8183591/webrev.02/ On 7/18/2017 1:55 PM, Adam Petcher wrote: Some additional investigation reveale

Re: Code Review Request, JDK-8184316, Typo in javax.net.ssl.SSLServerSocket class documentation

2017-07-25 Thread Sean Mullan
SSLSocket has the same typo in its first sentence ("extends Sockets"), and should also say "provides secure sockets" (note the plural sockets). Can you fix that as part of this too? One wording improvement below: On 7/25/17 12:02 PM, Xuelei Fan wrote: Hi, Please review the document typo upda

Re: Code Review Request, JDK-8184316, Typo in javax.net.ssl.SSLServerSocket class documentation

2017-07-25 Thread Sean Mullan
On 7/25/17 12:26 PM, Xuelei Fan wrote: Good catch! More update: javax/net/ssl/SSLServerSocket.java -- /** - * This class extends ServerSockets and + * This class extends ServerSocket and * provides secure server sockets using protocols such as the Secure

Re: Code Review Request, JDK-8184316, Typo in javax.net.ssl.SSLServerSocket class documentation

2017-07-25 Thread Sean Mullan
rs of this API. * - * SSLSockets are created by SSLSocketFactorys, + * SSLSocket is created by SSLSocketFactory, * or by accepting a connection from a * SSLServerSocket. Thanks, Xuelei On 7/25/2017 9:14 AM, Sean Mullan wrote: SSLSocket has the same typo in its first sentence ("extends Sockets

<    12   13   14   15   16   17   18   19   20   21   >