Re: RFR: 8169229: RSAClientKeyExchange debug info is incorrect

[Xuelei Fan]

> http://cr.openjdk.java.net/~wetmore/8169229/webrev.01
Looks fine to me.

Xuelei

On 11/9/2016 7:09 AM, Bradford Wetmore wrote:

Xuelei/I inadvertently left off security-dev in a later discussion, so
cc'ing here on some main points.

On 11/4/2016 4:33 PM, Xuelei Fan wrote:

As there is a preMaster, may not need the debug-only
debugProtocolVersion class field.  It can be extracted from preMaster in
the print() implementation.


On 11/6/2016 12:51 AM, Bradford Wetmore wrote:

What if it's not extractible?


Xuelei wrote:


We know the version for the ClientKeyExchange message generation
(client side).  For the receiving/server side, no idea about how to
get the version.  Maybe, we can just dump "version is not
extractable"?


For the client, the clientKeyExchange protocol version field for the
message is actually set in the KeyGenerator, so
RSAClientKeyExchange.protocolVersion may or may not be what is sent over
the wire.  That is: RSAClientKeyExchange.protocolVersion is only a
guess, and may not be accurate and will confuse any debug analysis.

For the server side, I would expect the same: if it's not extractable we
could output some currentVersion, but again it's only a guess and would
confuse things.

So IMHO, we should not look at this.protocolVersion for debug if the
preMaster is not extractable:

void print(PrintStream s) throws IOException {
+   String version = "protocol version not available";
+
+   byte[] ba = preMaster.getEncoded();
+   if (ba != null && ba.length >= 2) {
+   version = ProtocolVersion.valueOf(ba[0], ba[1]).name;
+   }
+
s.println("*** ClientKeyExchange, RSA PreMasterSecret, " +
- protocolVersion);
+ version);

Final update:

https://bugs.openjdk.java.net/browse/JDK-8169229
http://cr.openjdk.java.net/~wetmore/8169229/webrev.01

I'll run it through JPRT, but I'll mark as noreg-trivial.

Brad



On 11/5/2016 6:17 AM, Bradford Wetmore wrote:


https://bugs.openjdk.java.net/browse/JDK-8169229
http://cr.openjdk.java.net/~wetmore/8169229/webrev.00/

Please review this minor bug fix.  Our RSAClientKeyExchange isn't
properly outputing the RSA PreMasterSecret field.

Thanks,

Brad

Re: RFR 7004967: SecureRandom should be more explicit about threading

[Wang Weijun]

> On Nov 9, 2016, at 5:06 AM, Sean Mullan  wrote:
> 
>> In fact, can we
>> deprecate the getSeed() method? It's not unsafe so we don't need to give
>> it a forRemoval value.
> 
> Sounds reasonable. I would file a bug, but I don't think it is critical for 9 
> - we can do it in 10.

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

--Max

Re: RFR: 8169229: RSAClientKeyExchange debug info is incorrect

[Bradford Wetmore]

Xuelei/I inadvertently left off security-dev in a later discussion, so 
cc'ing here on some main points.


On 11/4/2016 4:33 PM, Xuelei Fan wrote:

As there is a preMaster, may not need the debug-only
debugProtocolVersion class field.  It can be extracted from preMaster in
the print() implementation.


On 11/6/2016 12:51 AM, Bradford Wetmore wrote:
> What if it's not extractible?

Xuelei wrote:

> We know the version for the ClientKeyExchange message generation
> (client side).  For the receiving/server side, no idea about how to
> get the version.  Maybe, we can just dump "version is not
> extractable"?

For the client, the clientKeyExchange protocol version field for the 
message is actually set in the KeyGenerator, so 
RSAClientKeyExchange.protocolVersion may or may not be what is sent over 
the wire.  That is: RSAClientKeyExchange.protocolVersion is only a 
guess, and may not be accurate and will confuse any debug analysis.


For the server side, I would expect the same: if it's not extractable we 
could output some currentVersion, but again it's only a guess and would 
confuse things.


So IMHO, we should not look at this.protocolVersion for debug if the 
preMaster is not extractable:


void print(PrintStream s) throws IOException {
+   String version = "protocol version not available";
+
+   byte[] ba = preMaster.getEncoded();
+   if (ba != null && ba.length >= 2) {
+   version = ProtocolVersion.valueOf(ba[0], ba[1]).name;
+   }
+
s.println("*** ClientKeyExchange, RSA PreMasterSecret, " +
- protocolVersion);
+ version);

Final update:

https://bugs.openjdk.java.net/browse/JDK-8169229
http://cr.openjdk.java.net/~wetmore/8169229/webrev.01

I'll run it through JPRT, but I'll mark as noreg-trivial.

Brad


> On 11/5/2016 6:17 AM, Bradford Wetmore wrote:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8169229
>> http://cr.openjdk.java.net/~wetmore/8169229/webrev.00/
>>
>> Please review this minor bug fix.  Our RSAClientKeyExchange isn't
>> properly outputing the RSA PreMasterSecret field.
>>
>> Thanks,
>>
>> Brad

Re: [9] RFR: 8168882: keytool doesn't print certificate info if disabled algorithm was used for signing a jar

[Sean Mullan]

Looks fine to me.

--Sean

On 11/7/16 10:10 PM, Wang Weijun wrote:

Everything looks fine now.

--Max


On Nov 8, 2016, at 11:09 AM, Artem Smotrakov  wrote:

Here is final version (I hope)

http://cr.openjdk.java.net/~asmotrak/8168882/webrev.04/

Artem

Re: RFR 7004967: SecureRandom should be more explicit about threading

[Sean Mullan]

On 11/7/16 4:59 AM, Wang Weijun wrote:

Accepted.

Please review http://ccc.us.oracle.com/8169312.


Reviewed.


In fact, can we
deprecate the getSeed() method? It's not unsafe so we don't need to give
it a forRemoval value.


Sounds reasonable. I would file a bug, but I don't think it is critical 
for 9 - we can do it in 10.


--Sean



Thanks
Max

On 11/4/2016 10:54 PM, Sean Mullan wrote:

* SecureRandom

 131  * If this attribute is not set or is "false", this class will
instead
 132  * synchronize access to each of the methods of the {@code
SecureRandomSpi}
 133  * implementation.

Not all of the methods are synchronized - engineGetParameters is not,
for example. I think to avoid ambiguity, you should list the names of
the methods that are synchronized.

810  * @throws IllegalArgumentException if {@code numBytes} is
negative

Since this is a new @throws, you really need to file a new CCC (or
withdraw and amend the current one with this @throws).

Please also file a docs bug with a description of the new attribute.

* SecureRandomSpi

lines 63-83, I think the wording could be improved/simplified, how about:

See {@link SecureRandom} for additional details on thread safety. By
default, a SecureRandomSpi implementation is considered to be not safe
for use by multiple concurrent threads and SecureRandom will synchronize
access to each of the applicable engine methods (see SecureRandom for
the list of methods). However, if a SecureRandomSpi implementation is
thread-safe, the service

provider attribute "ThreadSafe" should be set to "true" during its
registration, as follows:

  put("SecureRandom.AlgName ThreadSafe", "true");

  or

  putService(new Service(this, "SecureRandom", "AlgName", className,
 null, Map.of("ThreadSafe", "true")));

{@code SecureRandom} will then call the applicable engine methods
without any synchronization.

--Sean

On 11/2/16 3:27 AM, Wang Weijun wrote:

Ping again.

There is an updated version at
http://cr.openjdk.java.net/~weijun/7004967/webrev.01/ with doc-only
changes.

Thanks
Max


On Aug 25, 2016, at 10:00 AM, Weijun Wang 
wrote:

Please review the enhancement at

 http://cr.openjdk.java.net/~weijun/7004967/webrev.00/

Basically, we want SecureRandom to be more efficient by removing all
synchronized keywords from its public methods and let an
implementation to take care of thread-safety (We already did some in
JDK-8098581). On the other hand, we need to make sure that existing
implementations that have not synchronized correctly to behave just
as good as before.

Therefore a new Service Attribute "ThreadSafe" is introduced. If you
think your implementation is already thread-safe, set it to "true"
and SecureRandom will be happy. Otherwise, don't set it and
SecureRandom will continuously call your SecureRandomSpi engine
methods in synchronized blocks.

Thanks
Max

Re: Updates to documentation for JEP 287

[Jurrian Fahner]

Thanks for the update.

Kind regards,

Jurrian

Op ma 7 nov. 2016 23:45 schreef Sean Mullan :

> There's a bug open to update the Standard Names doc to include SHA-3:
> https://bugs.openjdk.java.net/browse/JDK-8004078
>
> The security guides typically get updated a bit later. I don't have an
> estimate but it will be done before 9 is released.
>
> Thanks,
> Sean
>
>
> On 10/29/16 11:08 AM, Jurrian Fahner wrote:
> > Hi all,
> >
> > I discovered that for Java 9, the following page is not updated for the
> > new hashing algorithms:
> >
> http://download.java.net/java/jdk9/docs/technotes/guides/security/StandardNames.html
> >
> > I don't know whether it is on your to do list, if it is already an item
> > then just ignore this email.
> > If I can be of any assistance, please let me know.
> >
> > Kind regards,
> >
> > Jurrian Fahner
>