Re: [PATCH] 8165751: NPE in Signature with java.security.debug=provider

2016-12-13 Thread Adam Petcher

Thanks for the review.

After thinking about this some more, I don't like the idea of using 
Optional for a member variable due to the limitations of this class and 
the lack of support for this usage of it. I'll send out a new diff that 
uses a standard null check.


See below for other comments.


On 12/12/2016 9:20 PM, Wang Weijun wrote:

Hi Adam

The only behavior change is with the debug output, right?

Yes. This will be more obvious in my next diff.


Is this a new pattern that internal optional fields should be defined as an 
Optional?

And, when there is no provider the string form "from: " looks strange, I would rather make it "from 
nowhere". I would also move the space before "from" to where the method is called, say, " 
verification algorithm " + getProvidedByString().
It doesn't print the "from: " when there is no provider. So the string 
will look like "Signature.DSA verification algorithm" in this case. I 
don't have a strong opinion on whether we should print "from: no 
provider" (for example), but Sean expressed a preference for leaving 
this entire part blank when there is no provider (as it is in my last 
diff).


Thanks
Max


On Dec 13, 2016, at 4:05 AM, Adam Petcher  wrote:

Okay. I changed getProvider() to return this.provider.orElse(null), which will 
allow this method to return null. For consistency, I allow all other providers 
(in Instance and Service) to be null using Optional.ofNullable(). Hopefully, I 
found and fixed all the whitespace issues, too. Here is the corrected diff:

diff --git a/src/java.base/share/classes/java/security/Signature.java 
b/src/java.base/share/classes/java/security/Signature.java
--- a/src/java.base/share/classes/java/security/Signature.java
+++ b/src/java.base/share/classes/java/security/Signature.java
@@ -134,7 +134,7 @@
 private String algorithm;

 // The provider
-Provider provider;
+Optional provider = Optional.empty();

 /**
  * Possible {@link #state} value, signifying that
@@ -266,7 +266,7 @@
 SignatureSpi spi = (SignatureSpi)instance.impl;
 sig = new Delegate(spi, algorithm);
 }
-sig.provider = instance.provider;
+sig.provider = Optional.ofNullable(instance.provider);
 return sig;
 }

@@ -449,13 +449,17 @@
  */
 public final Provider getProvider() {
 chooseFirstProvider();
-return this.provider;
+return this.provider.orElse(null);
 }

 void chooseFirstProvider() {
 // empty, overridden in Delegate
 }

+private String getProvidedByString() {
+return provider.map(x -> " from: " + x.getName()).orElse("");
+}
+
 /**
  * Initializes this object for verification. If this method is called
  * again with a different argument, it negates the effect
@@ -473,7 +477,7 @@

 if (!skipDebug && pdebug != null) {
 pdebug.println("Signature." + algorithm +
-" verification algorithm from: " + this.provider.getName());
+" verification algorithm" + getProvidedByString());
 }
 }

@@ -522,7 +526,7 @@

 if (!skipDebug && pdebug != null) {
 pdebug.println("Signature." + algorithm +
-" verification algorithm from: " + this.provider.getName());
+" verification algorithm" + getProvidedByString());
 }
 }

@@ -543,7 +547,7 @@

 if (!skipDebug && pdebug != null) {
 pdebug.println("Signature." + algorithm +
-" signing algorithm from: " + this.provider.getName());
+" signing algorithm" + getProvidedByString());
 }
 }

@@ -566,7 +570,7 @@

 if (!skipDebug && pdebug != null) {
 pdebug.println("Signature." + algorithm +
-" signing algorithm from: " + this.provider.getName());
+" signing algorithm" + getProvidedByString());
 }
 }

@@ -1087,7 +1091,7 @@
 }
 try {
 sigSpi = newInstance(s);
-provider = s.getProvider();
+provider = Optional.ofNullable(s.getProvider());
 // not needed any more
 firstService = null;
 serviceIterator = null;
@@ -1132,7 +1136,7 @@
 try {
 SignatureSpi spi = newInstance(s);
 init(spi, type, key, random);
-provider = s.getProvider();
+provider = Optional.ofNullable(s.getProvider());
 sigSpi = spi;
 firstService = null;
 serviceIterator = null;
diff --git a/test/java/security/Signature/NoProvider.java 
b/test/java/security/Signature/NoProvider.java
new file mode 100644
--- /dev/null
+++ b/test/java/security/Signature/NoProvider.java
@@ -0,0 +1,99 @@
+/*
+ * Copyright (c) 2016, Or

Re: [PATCH] 8165751: NPE in Signature with java.security.debug=provider

2016-12-13 Thread Sean Mullan

On 12/13/16 11:30 AM, Adam Petcher wrote:

Thanks for the review.

After thinking about this some more, I don't like the idea of using
Optional for a member variable due to the limitations of this class and
the lack of support for this usage of it. I'll send out a new diff that
uses a standard null check.

See below for other comments.


On 12/12/2016 9:20 PM, Wang Weijun wrote:

Hi Adam

The only behavior change is with the debug output, right?

Yes. This will be more obvious in my next diff.


Is this a new pattern that internal optional fields should be defined
as an Optional?

And, when there is no provider the string form "from: " looks strange,
I would rather make it "from nowhere". I would also move the space
before "from" to where the method is called, say, " verification
algorithm " + getProvidedByString().

It doesn't print the "from: " when there is no provider. So the string
will look like "Signature.DSA verification algorithm" in this case. I
don't have a strong opinion on whether we should print "from: no
provider" (for example), but Sean expressed a preference for leaving
this entire part blank when there is no provider (as it is in my last
diff).


Maybe "from: (no provider)" would be better. I think a previous version 
had "from: none", but thinking about it more, "from: (no provider)" is 
probably better than not printing anything for a null provider.


--Sean



Thanks
Max


On Dec 13, 2016, at 4:05 AM, Adam Petcher 
wrote:

Okay. I changed getProvider() to return this.provider.orElse(null),
which will allow this method to return null. For consistency, I allow
all other providers (in Instance and Service) to be null using
Optional.ofNullable(). Hopefully, I found and fixed all the
whitespace issues, too. Here is the corrected diff:

diff --git a/src/java.base/share/classes/java/security/Signature.java
b/src/java.base/share/classes/java/security/Signature.java
--- a/src/java.base/share/classes/java/security/Signature.java
+++ b/src/java.base/share/classes/java/security/Signature.java
@@ -134,7 +134,7 @@
 private String algorithm;

 // The provider
-Provider provider;
+Optional provider = Optional.empty();

 /**
  * Possible {@link #state} value, signifying that
@@ -266,7 +266,7 @@
 SignatureSpi spi = (SignatureSpi)instance.impl;
 sig = new Delegate(spi, algorithm);
 }
-sig.provider = instance.provider;
+sig.provider = Optional.ofNullable(instance.provider);
 return sig;
 }

@@ -449,13 +449,17 @@
  */
 public final Provider getProvider() {
 chooseFirstProvider();
-return this.provider;
+return this.provider.orElse(null);
 }

 void chooseFirstProvider() {
 // empty, overridden in Delegate
 }

+private String getProvidedByString() {
+return provider.map(x -> " from: " + x.getName()).orElse("");
+}
+
 /**
  * Initializes this object for verification. If this method is
called
  * again with a different argument, it negates the effect
@@ -473,7 +477,7 @@

 if (!skipDebug && pdebug != null) {
 pdebug.println("Signature." + algorithm +
-" verification algorithm from: " +
this.provider.getName());
+" verification algorithm" + getProvidedByString());
 }
 }

@@ -522,7 +526,7 @@

 if (!skipDebug && pdebug != null) {
 pdebug.println("Signature." + algorithm +
-" verification algorithm from: " +
this.provider.getName());
+" verification algorithm" + getProvidedByString());
 }
 }

@@ -543,7 +547,7 @@

 if (!skipDebug && pdebug != null) {
 pdebug.println("Signature." + algorithm +
-" signing algorithm from: " + this.provider.getName());
+" signing algorithm" + getProvidedByString());
 }
 }

@@ -566,7 +570,7 @@

 if (!skipDebug && pdebug != null) {
 pdebug.println("Signature." + algorithm +
-" signing algorithm from: " + this.provider.getName());
+" signing algorithm" + getProvidedByString());
 }
 }

@@ -1087,7 +1091,7 @@
 }
 try {
 sigSpi = newInstance(s);
-provider = s.getProvider();
+provider =
Optional.ofNullable(s.getProvider());
 // not needed any more
 firstService = null;
 serviceIterator = null;
@@ -1132,7 +1136,7 @@
 try {
 SignatureSpi spi = newInstance(s);
 init(spi, type, key, random);
-provider = s.getProvider();
+provider =
Optional.ofNullable(s.getProvider());
 sigSpi = spi;
 firstService = null;
 serviceIterato

Re: [PATCH] 8165751: NPE in Signature with java.security.debug=provider

2016-12-13 Thread Adam Petcher
Here is the latest diff that simply adds a null check to the existing 
code. When the provider is null, the debug output will be "...algorithm 
from: (no provider)". The test code is the same as the last diff.



diff --git a/src/java.base/share/classes/java/security/Signature.java 
b/src/java.base/share/classes/java/security/Signature.java

--- a/src/java.base/share/classes/java/security/Signature.java
+++ b/src/java.base/share/classes/java/security/Signature.java
@@ -452,6 +452,10 @@
 return this.provider;
 }

+private String getProviderName() {
+return (provider == null)  ? "(no provider)" : provider.getName();
+}
+
 void chooseFirstProvider() {
 // empty, overridden in Delegate
 }
@@ -473,7 +477,7 @@

 if (!skipDebug && pdebug != null) {
 pdebug.println("Signature." + algorithm +
-" verification algorithm from: " + 
this.provider.getName());

+" verification algorithm from: " + getProviderName());
 }
 }

@@ -522,7 +526,7 @@

 if (!skipDebug && pdebug != null) {
 pdebug.println("Signature." + algorithm +
-" verification algorithm from: " + 
this.provider.getName());

+" verification algorithm from: " + getProviderName());
 }
 }

@@ -543,7 +547,7 @@

 if (!skipDebug && pdebug != null) {
 pdebug.println("Signature." + algorithm +
-" signing algorithm from: " + this.provider.getName());
+" signing algorithm from: " + getProviderName());
 }
 }

@@ -566,7 +570,7 @@

 if (!skipDebug && pdebug != null) {
 pdebug.println("Signature." + algorithm +
-" signing algorithm from: " + this.provider.getName());
+" signing algorithm from: " + getProviderName());
 }
 }

diff --git a/test/java/security/Signature/NoProvider.java 
b/test/java/security/Signature/NoProvider.java

new file mode 100644
--- /dev/null
+++ b/test/java/security/Signature/NoProvider.java
@@ -0,0 +1,99 @@
+/*
+ * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License 
version

+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8165751
+ * @summary Verify that that a subclass of Signature that does not 
contain a

+ * provider can be used verify.
+ * @run main/othervm -Djava.security.debug=provider NoProvider
+ */
+
+import java.security.*;
+
+public class NoProvider {
+
+private static class NoProviderPublicKey implements PublicKey {
+
+public String getAlgorithm() {
+return "NoProvider";
+}
+public String getFormat() {
+return "none";
+}
+public byte[] getEncoded() {
+return new byte[1];
+}
+}
+
+private static class NoProviderSignature extends Signature {
+
+public NoProviderSignature() {
+super("NoProvider");
+}
+
+protected void engineInitVerify(PublicKey publicKey)
+throws InvalidKeyException {
+// do nothing
+}
+
+protected void engineInitSign(PrivateKey privateKey)
+throws InvalidKeyException {
+// do nothing
+}
+
+protected void engineUpdate(byte b) throws SignatureException {
+// do nothing
+}
+
+protected void engineUpdate(byte[] b, int off, int len)
+throws SignatureException {
+// do nothing
+}
+
+protected byte[] engineSign() throws SignatureException {
+return new byte[1];
+}
+
+protected boolean engineVerify(byte[] sigBytes)
+throws SignatureException {
+return false;
+}
+
+@Deprecated
+protected void engineSetParameter(String param, Object value)
+throws InvalidParameterException {
+// do nothing
+}
+
+@Deprecated
+protected Object engineGetParameter(String param)
+throws

Re: [PATCH] 8165751: NPE in Signature with java.security.debug=provider

2016-12-13 Thread Sean Mullan

On 12/13/16 1:43 PM, Adam Petcher wrote:

Here is the latest diff that simply adds a null check to the existing
code. When the provider is null, the debug output will be "...algorithm
from: (no provider)". The test code is the same as the last diff.


Looks good to me.

--Sean




diff --git a/src/java.base/share/classes/java/security/Signature.java
b/src/java.base/share/classes/java/security/Signature.java
--- a/src/java.base/share/classes/java/security/Signature.java
+++ b/src/java.base/share/classes/java/security/Signature.java
@@ -452,6 +452,10 @@
 return this.provider;
 }

+private String getProviderName() {
+return (provider == null)  ? "(no provider)" : provider.getName();
+}
+
 void chooseFirstProvider() {
 // empty, overridden in Delegate
 }
@@ -473,7 +477,7 @@

 if (!skipDebug && pdebug != null) {
 pdebug.println("Signature." + algorithm +
-" verification algorithm from: " +
this.provider.getName());
+" verification algorithm from: " + getProviderName());
 }
 }

@@ -522,7 +526,7 @@

 if (!skipDebug && pdebug != null) {
 pdebug.println("Signature." + algorithm +
-" verification algorithm from: " +
this.provider.getName());
+" verification algorithm from: " + getProviderName());
 }
 }

@@ -543,7 +547,7 @@

 if (!skipDebug && pdebug != null) {
 pdebug.println("Signature." + algorithm +
-" signing algorithm from: " + this.provider.getName());
+" signing algorithm from: " + getProviderName());
 }
 }

@@ -566,7 +570,7 @@

 if (!skipDebug && pdebug != null) {
 pdebug.println("Signature." + algorithm +
-" signing algorithm from: " + this.provider.getName());
+" signing algorithm from: " + getProviderName());
 }
 }

diff --git a/test/java/security/Signature/NoProvider.java
b/test/java/security/Signature/NoProvider.java
new file mode 100644
--- /dev/null
+++ b/test/java/security/Signature/NoProvider.java
@@ -0,0 +1,99 @@
+/*
+ * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but
WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License
version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8165751
+ * @summary Verify that that a subclass of Signature that does not
contain a
+ * provider can be used verify.
+ * @run main/othervm -Djava.security.debug=provider NoProvider
+ */
+
+import java.security.*;
+
+public class NoProvider {
+
+private static class NoProviderPublicKey implements PublicKey {
+
+public String getAlgorithm() {
+return "NoProvider";
+}
+public String getFormat() {
+return "none";
+}
+public byte[] getEncoded() {
+return new byte[1];
+}
+}
+
+private static class NoProviderSignature extends Signature {
+
+public NoProviderSignature() {
+super("NoProvider");
+}
+
+protected void engineInitVerify(PublicKey publicKey)
+throws InvalidKeyException {
+// do nothing
+}
+
+protected void engineInitSign(PrivateKey privateKey)
+throws InvalidKeyException {
+// do nothing
+}
+
+protected void engineUpdate(byte b) throws SignatureException {
+// do nothing
+}
+
+protected void engineUpdate(byte[] b, int off, int len)
+throws SignatureException {
+// do nothing
+}
+
+protected byte[] engineSign() throws SignatureException {
+return new byte[1];
+}
+
+protected boolean engineVerify(byte[] sigBytes)
+throws SignatureException {
+return false;
+}
+
+@Deprecated
+protected void engineSetParameter(String param, Object value)
+throws InvalidParameterException {
+// do nothing
+}
+
+@Deprecated
+protected O

RandomCookie problem ?

2016-12-13 Thread Thomas Lußnig

Hi,

even if the case is with the current time not active. Is it an good idea 
to define an fixed value

for random generator under special conditions that are time depending ?

Gruß Thomas

---

package sun.security.ssl;

RandomCookie(final SecureRandom sr) {
final long ts0 = System.currentTimeMillis() / 1000L;
int ts1;
if(ts0 < Integer.MAX_VALUE) { ts1 = (int)ts0; }
else *{ ts1 = Integer.MAX_VALUE; }*
this.random_bytes = new byte[32];
sr.nextBytes(this.random_bytes);
this.random_bytes[0] = (byte)(ts1 >> 24);
this.random_bytes[1] = (byte)(ts1 >> 16);
this.random_bytes[2] = (byte)(ts1 >> 8);
this.random_bytes[3] = (byte) ts1;
}


Re: RFR 8157389: Release Note: New default -sigalg for jarsigner and keytool

2016-12-13 Thread Sean Mullan

Hi Max,

Could you include more information that shows what signature algorithm 
is used based on the key size range and algorithm?


--Sean

On 12/11/16 9:53 PM, Wang Weijun wrote:

Please take a review at the release note at

   https://bugs.openjdk.java.net/browse/JDK-8157389

Thanks
Max



Re: On 8171135: Include javadoc on JarSigner API in security doc

2016-12-13 Thread Sean Mullan
I would check with Jon Gibbons to see if there may be a more "official" 
place for exported, non-SE javadocs; otherwise it seems ok to me.


--Sean

On 12/12/16 9:50 PM, Wang Weijun wrote:

Hi All

I've create a new bug to include the javadoc of the non-Java SE JarSigner API 
into security doc:

   https://bugs.openjdk.java.net/browse/JDK-8171135

If you think this is OK, I'll move the bug to doc.

Thanks
Max



Re: RandomCookie problem ?

2016-12-13 Thread Xuelei Fan

On 12/13/2016 8:46 AM, Thomas Lußnig wrote:

Hi,

even if the case is with the current time not active. Is it an good idea
to define an fixed value
for random generator under special conditions that are time depending ?


The issue was fixed in JDK 9:

   https://bugs.openjdk.java.net/browse/JDK-8046294

Thanks,
Xuelei


Gruß Thomas

---

package sun.security.ssl;

RandomCookie(final SecureRandom sr) {
final long ts0 = System.currentTimeMillis() / 1000L;
int ts1;
if(ts0 < Integer.MAX_VALUE) { ts1 = (int)ts0; }
else   *{ ts1 = Integer.MAX_VALUE; }*
this.random_bytes = new byte[32];
sr.nextBytes(this.random_bytes);
this.random_bytes[0] = (byte)(ts1 >> 24);
this.random_bytes[1] = (byte)(ts1 >> 16);
this.random_bytes[2] = (byte)(ts1 >> 8);
this.random_bytes[3] = (byte) ts1;
}


Re: On 8171135: Include javadoc on JarSigner API in security doc

2016-12-13 Thread Wang Weijun
Jon

Will there be a dedicated page for all non-SE javadocs? We already had some in 
jdk.security.auth, jdk.security.jgss on the security guide home page [1].

Also, I am not sure if this belongs to the docs or build category if the 
javadoc is meant to be generated automatically. Therefore I let it stay in the 
security-libs at the moment.

Thanks
Max

[1] http://download.java.net/java/jdk9/docs/technotes/guides/security/index.html

> On Dec 14, 2016, at 5:32 AM, Sean Mullan  wrote:
> 
> I would check with Jon Gibbons to see if there may be a more "official" place 
> for exported, non-SE javadocs; otherwise it seems ok to me.
> 
> --Sean
> 
> On 12/12/16 9:50 PM, Wang Weijun wrote:
>> Hi All
>> 
>> I've create a new bug to include the javadoc of the non-Java SE JarSigner 
>> API into security doc:
>> 
>>   https://bugs.openjdk.java.net/browse/JDK-8171135
>> 
>> If you think this is OK, I'll move the bug to doc.
>> 
>> Thanks
>> Max
>> 



Re: RFR 8168979: @implNote for invalid FilePermission

2016-12-13 Thread Wang Weijun
Webrev updated at

   http://cr.openjdk.java.net/~weijun/8168979/webrev.01

One comment is moved to its correct position and a typo fixed.

Thanks Daniel for the comment. Can someone with a reviewer hat take a look?

Thanks
Max

> On Dec 12, 2016, at 6:03 PM, Daniel Fuchs  wrote:
> 
> Hi Max,
> 
> Don't count me as reviewer - but I see a mismatched comment
> in the file:
> 
> 209 /**
> 210  * Creates FilePermission objects with special internals.
> 211  * See {@link FilePermCompat#newPermPlusAltPath(Permission)} and
> 212  * {@link FilePermCompat#newPermUsingAltPath(Permission)}.
> 213  */
> 214
> 215 private static final Path here = builtInFS.getPath(
> 216 GetPropertyAction.privilegedGetProperty("user.dir"));
> 
> I guess the comment is a left over from some merge or previous fix?
> 
> 
> Also I noticed the following later on:
> 
> 541  * invalid {@code FilePermission}. Even if two {@code FilePermission}
> 542  * are created with the same invalid path, one does imply the other.
> 
> should this be:
> 
>Even if two {@code FilePermission} are created with the same
>invalid path, one does *not* imply the other.
> 
> best regards,
> 
> -- daniel
> 
> On 12/12/16 09:01, Wang Weijun wrote:
>> Please take a review at
>> 
>>   http://cr.openjdk.java.net/~weijun/8168979/webrev.00/
>> 
>> This further clarifies what an invalid FilePermission behaves. A major 
>> behavior change is that <> now implies an invalid permission, I 
>> hope this is good to minimize incompatibility.
>> 
>> Thanks
>> Max
>> 
> 



Re: RFR 8157389: Release Note: New default -sigalg for jarsigner and keytool

2016-12-13 Thread Wang Weijun
I copied the exact @implNote from JarSigner into the release note.

BTW, I noticed NIST 800-57 Part 1 has a new revision 4, which has the same 
tables as in revision 3. A new bug 
https://bugs.openjdk.java.net/browse/JDK-8171190 created and I'll create a 
webrev soon.

Thanks
Max

> On Dec 14, 2016, at 5:09 AM, Sean Mullan  wrote:
> 
> Hi Max,
> 
> Could you include more information that shows what signature algorithm is 
> used based on the key size range and algorithm?
> 
> --Sean
> 
> On 12/11/16 9:53 PM, Wang Weijun wrote:
>> Please take a review at the release note at
>> 
>>   https://bugs.openjdk.java.net/browse/JDK-8157389
>> 
>> Thanks
>> Max
>> 



RFR 8171190: Bump reference of NIST 800-57 Part 1 Rev 3 to Rev 4 in JarSigner API spec

2016-12-13 Thread Wang Weijun
NIST 800-57 Part 1 has a new revision. The lines below are newly introduced in 
jdk9.

diff --git a/src/java.base/share/classes/sun/security/x509/AlgorithmId.java 
b/src/java.base/share/classes/sun/security/x509/AlgorithmId.java
--- a/src/java.base/share/classes/sun/security/x509/AlgorithmId.java
+++ b/src/java.base/share/classes/sun/security/x509/AlgorithmId.java
@@ -1024,7 +1024,7 @@
 }
 }

-// Values from SP800-57 part 1 rev 3 tables 2 and three
+// Values from SP800-57 part 1 rev 4 tables 2 and 3
 private static String ecStrength (int bitLength) {
 if (bitLength >= 512) { // 256 bits of strength
 return "SHA512";
@@ -1035,7 +1035,7 @@
 }
 }

-// same values for RSA and DSA
+// Same values for RSA and DSA
 private static String ifcFfcStrength (int bitLength) {
 if (bitLength > 7680) { // 256 bits
 return "SHA512";
diff --git 
a/src/jdk.jartool/share/classes/jdk/security/jarsigner/JarSigner.java 
b/src/jdk.jartool/share/classes/jdk/security/jarsigner/JarSigner.java
--- a/src/jdk.jartool/share/classes/jdk/security/jarsigner/JarSigner.java
+++ b/src/jdk.jartool/share/classes/jdk/security/jarsigner/JarSigner.java
@@ -430,7 +430,7 @@
  * SHA384withECDSA for a 384-bit EC key.
  *
  * @implNote This implementation makes use of comparable strengths
- * as defined in Tables 2 and 3 of NIST SP 800-57 Part 1-Rev.3.
+ * as defined in Tables 2 and 3 of NIST SP 800-57 Part 1-Rev.4.
  * Specifically, if a DSA or RSA key with a key size greater than 7680
  * bits, or an EC key with a key size greater than or equal to 512 
bits,
  * SHA-512 will be used as the hash function for the signature.

Thanks
Max



Re: RFR 8168979: @implNote for invalid FilePermission

2016-12-13 Thread Xuelei Fan

On 12/13/2016 5:45 PM, Wang Weijun wrote:

A major behavior change is that <> now implies an invalid 
permission, I hope this is good to minimize incompatibility.
Looks like two sides of the same coin.  If there is an invalid > (not existing in practice, I think), it now implies all;  if no 
change on this point, <> does not imply invalid one.  The 
existing behavior looks more safe to me.  What's you concern to make 
this behavior change?


Otherwise, looks fine to me.

Xuelei


Re: RFR 8168979: @implNote for invalid FilePermission

2016-12-13 Thread Wang Weijun

> On Dec 14, 2016, at 10:11 AM, Xuelei Fan  wrote:
> 
> On 12/13/2016 5:45 PM, Wang Weijun wrote:
>> A major behavior change is that <> now implies an invalid 
>> permission, I hope this is good to minimize incompatibility.
> Looks like two sides of the same coin.  If there is an invalid <> 
> (not existing in practice, I think),

Not existing in theory either. As the change says, invalid happens when the 
conversion of string to NIO Path fails. For <>, there will be no 
such conversion.

> it now implies all;  if no change on this point, <> does not imply 
> invalid one.  The existing behavior looks more safe to me.  What's you 
> concern to make this behavior change?

Just safer. Sometimes an app can recover from a FileNotExistException but not a 
SecurityException.

Thanks
Max

> 
> Otherwise, looks fine to me.
> 
> Xuelei



Re: RFR 8171190: Bump reference of NIST 800-57 Part 1 Rev 3 to Rev 4 in JarSigner API spec

2016-12-13 Thread Xuelei Fan

Looks fine to me.

Xuelei

On 12/13/2016 6:09 PM, Wang Weijun wrote:

NIST 800-57 Part 1 has a new revision. The lines below are newly introduced in 
jdk9.

diff --git a/src/java.base/share/classes/sun/security/x509/AlgorithmId.java 
b/src/java.base/share/classes/sun/security/x509/AlgorithmId.java
--- a/src/java.base/share/classes/sun/security/x509/AlgorithmId.java
+++ b/src/java.base/share/classes/sun/security/x509/AlgorithmId.java
@@ -1024,7 +1024,7 @@
 }
 }

-// Values from SP800-57 part 1 rev 3 tables 2 and three
+// Values from SP800-57 part 1 rev 4 tables 2 and 3
 private static String ecStrength (int bitLength) {
 if (bitLength >= 512) { // 256 bits of strength
 return "SHA512";
@@ -1035,7 +1035,7 @@
 }
 }

-// same values for RSA and DSA
+// Same values for RSA and DSA
 private static String ifcFfcStrength (int bitLength) {
 if (bitLength > 7680) { // 256 bits
 return "SHA512";
diff --git 
a/src/jdk.jartool/share/classes/jdk/security/jarsigner/JarSigner.java 
b/src/jdk.jartool/share/classes/jdk/security/jarsigner/JarSigner.java
--- a/src/jdk.jartool/share/classes/jdk/security/jarsigner/JarSigner.java
+++ b/src/jdk.jartool/share/classes/jdk/security/jarsigner/JarSigner.java
@@ -430,7 +430,7 @@
  * SHA384withECDSA for a 384-bit EC key.
  *
  * @implNote This implementation makes use of comparable strengths
- * as defined in Tables 2 and 3 of NIST SP 800-57 Part 1-Rev.3.
+ * as defined in Tables 2 and 3 of NIST SP 800-57 Part 1-Rev.4.
  * Specifically, if a DSA or RSA key with a key size greater than 7680
  * bits, or an EC key with a key size greater than or equal to 512 
bits,
  * SHA-512 will be used as the hash function for the signature.

Thanks
Max



RFR 8170900: Issue with FilePermission::implies for wildcard flag(-)

2016-12-13 Thread Wang Weijun
An clarification is added to FilePermission::implies:

  * @implNote

  * a simple {@code npath} is recursively inside a wildcard {@code npath}
  * if and only if {@code simple_npath.relativize(wildcard_npath)}
- * is a series of one or more "..". An invalid {@code FilePermission} does
+ * is a series of one or more "..". Note that this means "/-" does not
+ * imply "foo". An invalid {@code FilePermission} does
  * not imply any object except for itself.

The newly added sentence is

  Note that this means "/-" does not imply "foo".

JCK has agreed to update their test.

Since this is just a clarification inside an @implNote and no spec is updated, 
I suppose no CCC is needed. Please confirm.

Thanks
Max