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

2017-05-19 Thread serguei.spit...@oracle.com
On 5/19/17 06:22, Robbin Ehn wrote: Hi, On 05/19/2017 08:27 AM, serguei.spit...@oracle.com wrote: On 5/18/17 04:05, Robbin Ehn wrote: Hi of those: One more alternate solution to suggest is to add new function: jdwpTransportError SetTransportConfiguration(jdwpTransportConfiguration config);

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

2017-05-19 Thread Robbin Ehn
Hi, On 05/19/2017 08:27 AM, serguei.spit...@oracle.com wrote: On 5/18/17 04:05, Robbin Ehn wrote: Hi of those: One more alternate solution to suggest is to add new function: jdwpTransportError SetTransportConfiguration(jdwpTransportConfiguration config); Where: typedef struct { con

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

2017-05-19 Thread serguei.spit...@oracle.com
Dmitry, On 5/18/17 04:33, Dmitry Samersoff wrote: Serguei, *a* 4. The suggested *allow* feature is too destructive for the transport API. It makes the original function StartListening unusable and its entry slot a garbage. It will become a big mess if we add more function clones like StartLi

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

2017-05-18 Thread serguei.spit...@oracle.com
On 5/18/17 04:05, Robbin Ehn wrote: Hi of those: One more alternate solution to suggest is to add new function: jdwpTransportError SetTransportConfiguration(jdwpTransportConfiguration config); Where: typedef struct { const char* allowed_peers; } jdwpTransportConfiguration; Thi

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

2017-05-18 Thread Dmitry Samersoff
Robbin, Please, see my answer to Serguei. -Dmitry On 2017-05-18 14:05, Robbin Ehn wrote: > Hi of those: > >> One more alternate solution to suggest is to add new function: >> jdwpTransportError >> SetTransportConfiguration(jdwpTransportConfiguration config); >> >> Where: >>typedef struct {

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

2017-05-18 Thread Dmitry Samersoff
Serguei, *a* > 4. The suggested *allow* feature is too destructive for the > transport API. It makes the original function StartListening unusable > and its entry slot a garbage. It will become a big mess if we add > more function clones like StartListening11. . . . 6. The suggested > API update

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

2017-05-18 Thread Robbin Ehn
Hi of those: One more alternate solution to suggest is to add new function: jdwpTransportError SetTransportConfiguration(jdwpTransportConfiguration config); Where: typedef struct { const char* allowed_peers; } jdwpTransportConfiguration; This approach allows to extend the jdwpTra

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

2017-05-18 Thread serguei.spit...@oracle.com
Hi Dmitry, Let's get to a consensus for the last item on our plate:   jdwpTransportError AllowPeers(const char* peers);                  .VS.   jdwpTransportError StartListeningWithAllow(const char* address, char** a

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

2017-05-12 Thread serguei.spit...@oracle.com
Hi Dmitry, Thank you for the update with some review comments resolved! I looked at it but will not provide my comments at this point. We will need another update according to the recent design review and items where we still have to reach an agreement. Thanks, Serguei On 5/10/17 08:27, Dmitry

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

2017-05-11 Thread Robbin Ehn
Hi, I find both your approaches acceptable regarding the version and StartListening11 vs AllowPeers. Personally I prefer not using version instead using sizeof as syscall does. E.g. http://man7.org/linux/man-pages/man2/bind.2.html But obviously this also require a new StartListeningXX method.

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

2017-05-10 Thread Dmitry Samersoff
Serguei, Fixed minor issues (comments, netmask etc). Added an error for attempt to use allow with an old transport. http://cr.openjdk.java.net/~dsamersoff/JDK-8061228/webrev.17/ see also below. > I do not understand this reason for adding more complexity. > It seems, there should not be any is

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

2017-05-10 Thread serguei.spit...@oracle.com
Dmitry, Please, see one correction below. On 5/10/17 02:37, serguei.spit...@oracle.com wrote: Dmitry, Thank you a lot for the detailed reply! On 5/10/17 01:10, Dmitry Samersoff wrote: Serguei, Please see my comments in-line. On 2017-05-10 00:42, serguei.spit...@oracle.com wrote: Hi Dm

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

2017-05-10 Thread serguei.spit...@oracle.com
Dmitry, Thank you a lot for the detailed reply! On 5/10/17 01:10, Dmitry Samersoff wrote: Serguei, Please see my comments in-line. On 2017-05-10 00:42, serguei.spit...@oracle.com wrote: Hi Dmitry, I'd like to resolve my questions before the upcoming design discussion on Thu. http://cr.

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

2017-05-10 Thread Dmitry Samersoff
Serguei, Please see my comments in-line. On 2017-05-10 00:42, serguei.spit...@oracle.com wrote: > Hi Dmitry, > > > I'd like to resolve my questions before the upcoming design discussion > on Thu. > > > http://cr.openjdk.java.net/%7Edsamersoff/JDK-8061228/webrev.15/src/jdk.jdwp.agent/share/na

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

2017-05-09 Thread serguei.spit...@oracle.com
(The formatting is broken in my reply. I've changed my Zhunderbird 'Send Options' to avoid this in the future. I tried to fix the formatting below.) Hi Dmitry, I'd like to resolve my questions before the upcoming design discussion on Thu. http://cr.openjdk.java.net/%7Edsamersoff/JDK-8061228/w

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

2017-05-09 Thread serguei.spit...@oracle.com
Hi Dmitry, I'd like to resolve my questions before the upcoming design discussion on Thu. http://cr.openjdk.java.net/%7Edsamersoff/JDK-8061228/webrev.15/src/jdk.jdwp.agent/share/native/libdt_socket/socketTransport.c.udiff.html (0) The design description from the bug report tells: > Than

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

2017-04-19 Thread Robbin Ehn
Hi Dmitry, sorry for the delay. Yes thanks, everything seems to work. Code looks reasonable. /Robbin On 03/29/2017 05:10 PM, Dmitry Samersoff wrote: Robbin, One follow-up issue is that if you start suspended and than connect with an unallowed client the JVM starts and executes the program.

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

2017-04-06 Thread Dmitry Samersoff
Serguei, (resending lost e-mail) Please, see: http://cr.openjdk.java.net/~dsamersoff/JDK-8061228/webrev.15/ Nits fixed. > It is not clear why do you > need the static variable transportVersion and this dance with it. We have to keep transport version in the global variable because void *rese

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

2017-04-06 Thread Dmitry Samersoff
Serguei, 1. New webrev with couple of issues addressed (see Robbin's e-mails): http://cr.openjdk.java.net/~dsamersoff/JDK-8061228/webrev.15 2. We have to keep transport version in global variable because old transport doesn't initialize reserved1 field and we are loosing version information aft

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

2017-03-29 Thread Dmitry Samersoff
Robbin, > One follow-up issue is that if you start suspended > and than connect with > an unallowed client the JVM starts and executes the program. Fixed. see http://cr.openjdk.java.net/~dsamersoff/JDK-8061228/webrev.15 -Dmitry On 2017-03-16 15:59, Robbin Ehn wrote: > Hi Dmitry, thanks for the

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

2017-03-17 Thread serguei.spit...@oracle.com
Dmitry, Some quick comments on the webrev.12. The style of comments must be /* */, not //. http://cr.openjdk.java.net/~dsamersoff/JDK-8061228/webrev.12/src/jdk.jdwp.agent/share/native/libjdwp/transport.c.frames.html 205 // Pass MAXIMAL supported version, expect actual transport version in

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

2017-03-17 Thread serguei.spit...@oracle.com
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.spit...@o

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

2017-03-16 Thread Robbin Ehn
Hi Dmitry, thanks for the update. One follow-up issue is that if you start suspended and than connect with an unallowed client the JVM starts and executes the program. Simple program prints "Hello". [rehn@rehn-ws vanilla-hs]$ java -agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=*

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

2017-03-15 Thread Dmitry Samersoff
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.spit...@oracle.com wrote: > Hi Dmitry, > > We already had a big review thread back in 2014 on this. >

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

2017-03-14 Thread serguei.spit...@oracle.com
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 imp

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

2017-03-13 Thread Dmitry Samersoff
Robbin, Please, see: http://cr.openjdk.java.net/~dsamersoff/JDK-8061228/webrev.11 > 1: > So connecting with an unallowed client terminates the VM. Fixed. > 2: > Starting with an bad allow filter terminates the VM when connecting a > client. Moved allowed parameter (and parser call) to StartLi

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

2017-03-10 Thread Dmitry Samersoff
Robbin, Agree, I'll fix it. -Dmitry On 2017-03-10 15:56, Robbin Ehn wrote: > Hi Dmitry, > > I took a look at this, I have two practical issues: > > 1: > [rehn@rehn-ws dev]$ java > -agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=*:,allow=6.6.6.6 > -cp runs ForEver > Listening

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

2017-03-10 Thread Robbin Ehn
Hi Dmitry, I took a look at this, I have two practical issues: 1: [rehn@rehn-ws dev]$ java -agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=*:,allow=6.6.6.6 -cp runs ForEver Listening for transport dt_socket at address: ERROR: transport error 202: peer not allowed to conn