RFR(M): JDK-8061228 Allow JDWP socket connector to accept connections from certain ip addresses only
serguei.spitsyn at oracle.com
serguei.spitsyn at oracle.com
Fri Mar 17 07:03:48 UTC 2017
Hi Dmitry,
On 3/15/17 00:56, Dmitry Samersoff wrote:
> Serguei,
>
>> I still see the C .vs. C++ related change in the jdwpTransport.h.
> done.
>
> http://cr.openjdk.java.net/~dsamersoff/JDK-8061228/webrev.12/
Good.
Thank you for the update!
>
> see also inline.
>
> On 2017-03-15 00:40, serguei.spitsyn at oracle.com wrote:
>> Hi Dmitry,
>>
>> We already had a big review thread back in 2014 on this.
>> I think, it is again going in the wrong order.
>>
>> First, I think, it is better to start from a CCC (or its equivalent, CSR).
>> This will allow to focus on and sort out the changes in interface/behavior
>> first before going into the implementation details.
> CCC was filed and approved. The only reason I withdraw it - the fix
> didn't go to jdk9 but CCC tool doesn't have jdk10.
>
> CSR process is also not yet implemented.
The CSR preview message from Joe is attached.
My understanding is that we can continue to use CCC.
At some points the CCC process will be moved to the CSR.
The CCC is out of date now as it does not match the webrev 12.
Also, I do not like the addition of new function StartListening11() next
to StartListening().
Does it mean, that for transport version 1.2 we might have more function
clones with 12 suffix?
Perhaps, we need something smarter here but I'm unsure yet what it has
to be.
>> Second, I'd suggest to separate a couple of other things.
>> I still see the C .vs. C++ related change in the jdwpTransport.h.
>> It is better to leave it along for now as it was suggested in early
>> review rounds.
>> If you still want to fix it then it is better to file a separate bug that
>> should include the JNI as well (as it was discussed with Alan before).
> Do you mean?
>
> 39 #ifdef __cplusplus
> 40 extern "C" {
> 41 #endif
>
> I'll add it back to avoid any confusion.
Yes. I see you added it back in new version of webrev.
>
>> Also, I'm thinking if it is a good idea to separate the transport
>> versioning to an RFE.
>> It would allow to focus on this aspect as necessary.
>> In this case, the 8061228 will depend on the versioning.
>> However, it is much more simple and can be resolved faster.
> It's hard to test versioning code without implementation of
> new, VERSION_1_1 transport.
>
> Network part of 8061228 is simple and clear separated from versioning,
> so I would prefer to keep it together in one CR/one push.
No pressure.
I've got your point above.
Thanks,
Serguei
>
> Restriction turned off by default (I'll file separate CR to enable it
> later), so we have enough time to fix any issues that might come.
>
> -Dmitry
>
>>
>> Thanks,
>> Serguei
>>
>>
>> On 2/28/17 01:41, Dmitry Samersoff wrote:
>>> Everybody,
>>>
>>> Please review:
>>>
>>> http://cr.openjdk.java.net/~dsamersoff/JDK-8061228/webrev.10/
>>>
>>> These changes introduce new parameter[1] of the socket transport -
>>> allow. Users can explicitly specify a list of hosts that allowed to
>>> connect to jdwp server and it's the second part of JDWP hardening[2].
>>>
>>> No restrictions are applied by default now but I'll file a separate CR
>>> to restrict list of allowed peers to localhost by default.
>>>
>>> Also these changes implement versioning for jdwp transport and therefor
>>> simplify feature development of jdwp.
>>>
>>>
>>> 1. Example command line:
>>>
>>> -agentlib:jdwp=transport=dt_socket,server=y,suspend=n,
>>> address=*,allow="127.0.0.0/8;192.168.0.0/24"
>>>
>>> Possible values for allow parameter:
>>> * - accept connections from everywhere.
>>> N.N.N.N - accept connections from this IP address only
>>> N.N.N.N/nn - accept connections from particular ip subnet
>>>
>>>
>>>
>>> 2. JDK-8052136 JDWP hardening
>>>
>>> -Dmitry
>>>
>
-------------- next part --------------
An embedded message was scrubbed...
From: joe darcy <joe.darcy at oracle.com>
Subject: Preview: the ccc is retiring; long live the csr!
Date: Wed, 14 Dec 2016 14:55:42 -0800
Size: 38862
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20170317/d2a33ce3/AttachedMessage-0001.mht>
More information about the serviceability-dev
mailing list