Re: 8241336: Some java.net tests failed with NoRouteToHostException on MacOS with special network configuration

2020-03-26 Thread Daniel Fuchs

Hi Alan.

On 26/03/2020 17:29, Alan Bateman wrote:
NetworkConfiguration.isTestable - is the hardcoded check for "awdl" 
needed? I don't think I've seen it with non-link local address so I 
can't tell if it is needed. I'm


It might no longer be needed, but we know that awdl is always
the wrong choice, so excluding it right away won't harm.
That was part of Chris's fix for 7155591 [1].

I've applied your other comments:

http://cr.openjdk.java.net/~dfuchs/webrev_8241336/webrev.02/

best regards,

-- daniel

[1] 
http://cr.openjdk.java.net/~msheppar/7155591/webrev/test/java/net/MulticastSocket/SetOutgoingIf.java.frames.html 



Promiscuous - can "addr" be renamed to toSocketAddress so the usages are 
a bit clearer? >
SetOutgoingIf - the initializer at L38 may not be obvious to some 
readers that aren't used to that syntax, can it use a regular 
constructor instead?


AdaptorMulticasting - I don't mind if it included here or separated into 
its own issue as the changes aren't needed for JDK-8241336. If you are 
including it then the patch should be cleaned up a bit so that it is 
consistent with the existing code. It was an oversight (by me) when I 
added this test. testSendNoReceive should have just ignored datagrams 
that are sent by an unexpected sender or with unexpected bytes. I think 
it complicates things to try to decode the bytes of a random datagram 
and prefer to see that part removed. It's okay to lot a message that an 
expected message has been received but its content it not interesting 
for the test. One thing that we should do some time is change some of 
the multicasting tests to use other multicast groups. I suspect some of 
the tests have copied the multicast address from other tests so we end 
up with several tests using the same group.







Re: 8241336: Some java.net tests failed with NoRouteToHostException on MacOS with special network configuration

2020-03-26 Thread Alan Bateman

On 26/03/2020 15:36, Daniel Fuchs wrote:

Hi,

Please find below a fix for:
8241336: Some java.net tests failed with NoRouteToHostException on
 MacOS with special network configuration
https://bugs.openjdk.java.net/browse/JDK-8241336

webrev:
http://cr.openjdk.java.net/~dfuchs/webrev_8241336/webrev.01/index.html
NetworkConfiguration.isTestable - is the hardcoded check for "awdl" 
needed? I don't think I've seen it with non-link local address so I 
can't tell if it is needed. I'm


Promiscuous - can "addr" be renamed to toSocketAddress so the usages are 
a bit clearer?


SetOutgoingIf - the initializer at L38 may not be obvious to some 
readers that aren't used to that syntax, can it use a regular 
constructor instead?


AdaptorMulticasting - I don't mind if it included here or separated into 
its own issue as the changes aren't needed for JDK-8241336. If you are 
including it then the patch should be cleaned up a bit so that it is 
consistent with the existing code. It was an oversight (by me) when I 
added this test. testSendNoReceive should have just ignored datagrams 
that are sent by an unexpected sender or with unexpected bytes. I think 
it complicates things to try to decode the bytes of a random datagram 
and prefer to see that part removed. It's okay to lot a message that an 
expected message has been received but its content it not interesting 
for the test. One thing that we should do some time is change some of 
the multicasting tests to use other multicast groups. I suspect some of 
the tests have copied the multicast address from other tests so we end 
up with several tests using the same group.


-Alan.


Re: RFR JDK-8239595/JDK-8239594 : ssl context version is not respected/jdk.tls.client.protocols is not respected

2020-03-26 Thread Xuelei Fan
With this update, the logic looks like: if TLSv1.3 is not enabled in the 
SSLContext, use TLSv1.2 instead;  Otherwise, use TLSv1.3 and TLSv1.2.


There may be a couple of issues:
1. TLSv1.2 may be not enabled, although TLSv1.3 is enabled.
For example:
   System.setProperty("jdk.tls.client.protocols", "TLSv1.3")
   System.setProperty("jdk.tls.client.protocols", "TLSv1.1, TLSv1.0")

2. TLSv1.2 may be not supported in the SSLContext.
For example:
   SSLContext context = SSLContext.getInstance("DTLS");
   HttpClient.newBuilder().sslContext(context)...

3. The application may not want to use TLS 1.2.
For example:
   System.setProperty("jdk.tls.client.protocols", "TLSv1.1, TLSv1.0")

The System property may be shared by code other than httpclient.  So the 
setting may not consider the impact on httpclient.


I may use enabled protocols only. If no TLSv1.2/TLSv1.3, I may use an 
empty protocol array, and test to see what happens in the httpclient 
implementation stack.


Xuelei

On 3/26/2020 9:28 AM, Sean Mullan wrote:

Cross-posting to security-dev as this involves TLS/SSL configuration.

--Sean

On 3/26/20 10:02 AM, rahul.r.ya...@oracle.com wrote:

Hello,

Request to have my fix reviewed for issues:

 JDK-8239595 : ssl context version is not respected
 JDK-8239594 : jdk.tls.client.protocols is not respected

The fix updates 
jdk.internal.net.http.HttpClientImpl.getDefaultParams(SSLContext ctx)
to use ctx.getDefaultSSLParameters()instead of 
ctx.getSupportedSSLParameters(),

as the latter does not respect the context parameters set by the user.

Issue: https://bugs.openjdk.java.net/browse/JDK-8239595
Issue: https://bugs.openjdk.java.net/browse/JDK-8239594

Webrev: 
http://cr.openjdk.java.net/~jboes/rayayada/webrevs/8239595/webrev.00/


-- Rahul


Re: RFR JDK-8239595/JDK-8239594 : ssl context version is not respected/jdk.tls.client.protocols is not respected

2020-03-26 Thread Sean Mullan
I think you should mark one of the two bugs a duplicate. Typically I 
mark the more recent one as a duplicate, unless there is a good reason 
to do otherwise.


--Sean

On 3/26/20 12:28 PM, Sean Mullan wrote:

Cross-posting to security-dev as this involves TLS/SSL configuration.

--Sean

On 3/26/20 10:02 AM, rahul.r.ya...@oracle.com wrote:

Hello,

Request to have my fix reviewed for issues:

 JDK-8239595 : ssl context version is not respected
 JDK-8239594 : jdk.tls.client.protocols is not respected

The fix updates 
jdk.internal.net.http.HttpClientImpl.getDefaultParams(SSLContext ctx)
to use ctx.getDefaultSSLParameters()instead of 
ctx.getSupportedSSLParameters(),

as the latter does not respect the context parameters set by the user.

Issue: https://bugs.openjdk.java.net/browse/JDK-8239595
Issue: https://bugs.openjdk.java.net/browse/JDK-8239594

Webrev: 
http://cr.openjdk.java.net/~jboes/rayayada/webrevs/8239595/webrev.00/


-- Rahul


Re: RFR JDK-8239595/JDK-8239594 : ssl context version is not respected/jdk.tls.client.protocols is not respected

2020-03-26 Thread Sean Mullan

Cross-posting to security-dev as this involves TLS/SSL configuration.

--Sean

On 3/26/20 10:02 AM, rahul.r.ya...@oracle.com wrote:

Hello,

Request to have my fix reviewed for issues:

     JDK-8239595 : ssl context version is not respected
     JDK-8239594 : jdk.tls.client.protocols is not respected

The fix updates 
jdk.internal.net.http.HttpClientImpl.getDefaultParams(SSLContext ctx)
to use ctx.getDefaultSSLParameters()instead of 
ctx.getSupportedSSLParameters(),

as the latter does not respect the context parameters set by the user.

Issue: https://bugs.openjdk.java.net/browse/JDK-8239595
Issue: https://bugs.openjdk.java.net/browse/JDK-8239594

Webrev: 
http://cr.openjdk.java.net/~jboes/rayayada/webrevs/8239595/webrev.00/


-- Rahul


8241336: Some java.net tests failed with NoRouteToHostException on MacOS with special network configuration

2020-03-26 Thread Daniel Fuchs

Hi,

Please find below a fix for:
8241336: Some java.net tests failed with NoRouteToHostException on
 MacOS with special network configuration
https://bugs.openjdk.java.net/browse/JDK-8241336

webrev:
http://cr.openjdk.java.net/~dfuchs/webrev_8241336/webrev.01/index.html

The crux of the issue is that on some macOS hardware/OS versions
we get network interfaces that look like regular ethernet interfaces
though they are not.
Picking them up while testing make some tests fail.

Most of these tests were already relying on the NetworkConfiguration
test library class - which has now been updated to skip these
problematic interfaces. I have heuristically observed that these
interfaces seemed to only have one link-local IPv6 address, while the
other regular interfaces had at least one IPv4, or one non link-local
IPv6. That seems to be the only observable difference.

The fix also contains some code for AdaptorMulticasting.java aim at
better diagnosing (and possibly fixing?) JDK-8238516 [1] - I might close
JDK-8238516 as a duplicate of JDK-8241336 if the issue doesn't
reproduce.

best regards,

-- daniel
[1] https://bugs.openjdk.java.net/browse/JDK-8238516


Re: 8241674 (XS): Fix incorrect jtreg option in FilePublisherPermsTest

2020-03-26 Thread Chris Hegarty
LGTM.

-Chris

> On 26 Mar 2020, at 14:32, Julia Boes  wrote:
> 
> Hi,
> 
> Could someone please review this small fix of a recently pushed test? The 
> option to extend the policy file has to be 'java.security.policy' instead of 
> 'policy' -  this was discussed on this mailing list but then miraculously 
> ignored by myself...
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8241674
> 
> Webrev: http://cr.openjdk.java.net/~jboes/webrevs/8241674/webrev.00/
> 
> Regards,
> 
> Julia
> 
> 



Re: 8241674 (XS): Fix incorrect jtreg option in FilePublisherPermsTest

2020-03-26 Thread Daniel Fuchs

Thanks for jumping on this Julia!

Reviewed. Since this is making some noise in the CI I suggest
to push it asap.

best regards,

-- daniel

On 26/03/2020 14:32, Julia Boes wrote:

Hi,

Could someone please review this small fix of a recently pushed test? 
The option to extend the policy file has to be 'java.security.policy' 
instead of 'policy' -  this was discussed on this mailing list but then 
miraculously ignored by myself...


Bug: https://bugs.openjdk.java.net/browse/JDK-8241674

Webrev: http://cr.openjdk.java.net/~jboes/webrevs/8241674/webrev.00/

Regards,

Julia






8241674 (XS): Fix incorrect jtreg option in FilePublisherPermsTest

2020-03-26 Thread Julia Boes

Hi,

Could someone please review this small fix of a recently pushed test? 
The option to extend the policy file has to be 'java.security.policy' 
instead of 'policy' -  this was discussed on this mailing list but then 
miraculously ignored by myself...


Bug: https://bugs.openjdk.java.net/browse/JDK-8241674

Webrev: http://cr.openjdk.java.net/~jboes/webrevs/8241674/webrev.00/

Regards,

Julia




RFR JDK-8239595/JDK-8239594 : ssl context version is not respected/jdk.tls.client.protocols is not respected

2020-03-26 Thread rahul . r . yadav

Hello,

Request to have my fix reviewed for issues:

    JDK-8239595 : ssl context version is not respected
    JDK-8239594 : jdk.tls.client.protocols is not respected

The fix updates 
jdk.internal.net.http.HttpClientImpl.getDefaultParams(SSLContext ctx)
to use ctx.getDefaultSSLParameters()instead of 
ctx.getSupportedSSLParameters(),

as the latter does not respect the context parameters set by the user.

Issue: https://bugs.openjdk.java.net/browse/JDK-8239595
Issue: https://bugs.openjdk.java.net/browse/JDK-8239594

Webrev: 
http://cr.openjdk.java.net/~jboes/rayayada/webrevs/8239595/webrev.00/


-- Rahul