RFR(M): JDK-8061228 Allow JDWP socket connector to accept connections from certain ip addresses only

Dmitry Samersoff dmitry.samersoff at oracle.com
Wed Mar 15 07:56:40 UTC 2017


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/

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.

> 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.

> 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.

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
>>
> 


-- 
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.


More information about the serviceability-dev mailing list