Re: [Openvpn-devel] OpenVPN protocol extensions update

2015-01-09 Thread James Yonan 

On 09/01/2015 02:41, Lev Stipakov wrote:

Hi James,

A few comments on peer-id part:

   * A disabled peer ID is denoted by 0xFF.
   * Server tells the client to use DATA_V2/peer_id by pushing
  the directive "peer-id ID" where ID is a decimal integer
  in the range [-1, 16777215].  Setting the peer ID to -1
  transmits DATA_V2 packets with the peer ID field set to
  0xFF.  Setting the peer_id to -1 is the same as
  setting it to 16777215 (i.e. 0xFF).

There is no such thing as "disabled peer-id" in current
implementation, server does not send "-1" and client does not send
0xFF. I can surely do it, if needed. Could you maybe explain its
meaning?


I see DATA_V2 as playing multiple roles, e.g. aside from its intended 
use of client float, it also improves packet alignment.


So conceivably, a connection might want to use DATA_V2 for alignment, 
but not need the peer-id/float capability.


Another thing I've observed about protocol changes in the past, is that 
once a protocol change reaches maturity after many years, where almost 
every OpenVPN client/server universally supports it, you can safely 
deprecate the old protocol variant (case in point: 
CONTROL_HARD_RESET_(CLIENT|SERVER)_V1 which has been been completely 
supplanted by V2).  So eventually we may want to deprecate DATA_V1, but 
we can't do that unless DATA_V2 is flexible enough to replace it.  And 
being able to use DATA_V2 without peer-id/float would give it the 
flexibility to be used as a drop-in replacement for DATA_V1 in cases 
where peer-id/float is not wanted or needed.



And just to make sure - if server gets DATA_V2 with peer-id
-1, it should skip peer-id bytes and treat it as data_v1, correct?


Right.


There is no specific check for the max value of peer-id at the place
where peer-id is issued. Logic relies on max_clients option. So in
case if max_clients is set to 2^24 or above, peer-id will wrap. There
is a check, however, that peer-id is between 0 and max_clients in the
code which handles incoming data packets.


It's an improbable corner case, but I would still check for wrap.

My rationale for checking for improbable corner cases is that code is 
always evolving, and sometimes code changes can make previously 
forgotten-about rare corner cases much more probable.



   * Server never pushes a "peer-id" directive unless the client has
indicated its support for DATA_V2 by including "IV_PROTO=2" in the
peer info data.

Server pushes peer-id if IV_PROTO>=2. Is it ok or should it be changed to =2 ?


That's a good question.  I think IV_PROTO>=2 is okay.  If we made a 
future change to the protocol that wasn't backward compatible, then we 
would have to make a new IV symbol for it, but having IV_PROTO be 
versioned does make it easier to introduce changes that _are_ backward 
compatible.




   * ensure that the float doesn't clobber a pre-existing client (i.e.
if the address floated to is already owned by another client) unless
it can be verified that the pre-existing client is a previous instance
of the floating client.

Thanks to this mail, I've checked code again and found a bug - a lame
duck client may float to an address taken by another client. I have
submitted a fix:

http://thread.gmane.org/gmane.network.openvpn.devel/9386

which is waiting for an ACK.

Otherwise implementation is pretty much in line with your spec.


Great, thanks.

James


-Lev

2015-01-07 2:08 GMT+02:00 James Yonan :

I've updated the OpenVPN protocol extension doc with additional details, now
that more of these features have been implemented in OpenVPN 3.

If you are implementing any of these features in OpenVPN 2.x, please review
so we can ensure that OpenVPN 2.x and 3 are on the same page with respect to
protocol extensions.

Changes:

1. Added specific details on DATA_V2/peer-id/float support.

2. For AEAD mode, emphasized that the leading 8 bytes (4 bytes for
DATA_V2/peer-id and 4 for packet ID) is all included in the AD.

3. Added specific details on protocol negotiation where the client indicates
protocol extension availability with IV_x parameters in the peer info
string, and the server responds by pushing directives to the client to
enable the feature.

4. Added "TCP nonlinear mode" section, a new protocol extension that is
needed by multithreaded TCP servers.

James

--
Dive into the World of Parallel Programming! The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] OpenVPN protocol extensions update

2015-01-09 Thread Lev Stipakov 
Hi James,

A few comments on peer-id part:

  * A disabled peer ID is denoted by 0xFF.
  * Server tells the client to use DATA_V2/peer_id by pushing
 the directive "peer-id ID" where ID is a decimal integer
 in the range [-1, 16777215].  Setting the peer ID to -1
 transmits DATA_V2 packets with the peer ID field set to
 0xFF.  Setting the peer_id to -1 is the same as
 setting it to 16777215 (i.e. 0xFF).

There is no such thing as "disabled peer-id" in current
implementation, server does not send "-1" and client does not send
0xFF. I can surely do it, if needed. Could you maybe explain its
meaning? And just to make sure - if server gets DATA_V2 with peer-id
-1, it should skip peer-id bytes and treat it as data_v1, correct?

There is no specific check for the max value of peer-id at the place
where peer-id is issued. Logic relies on max_clients option. So in
case if max_clients is set to 2^24 or above, peer-id will wrap. There
is a check, however, that peer-id is between 0 and max_clients in the
code which handles incoming data packets.

  * Server never pushes a "peer-id" directive unless the client has
indicated its support for DATA_V2 by including "IV_PROTO=2" in the
peer info data.

Server pushes peer-id if IV_PROTO>=2. Is it ok or should it be changed to =2 ?

  * ensure that the float doesn't clobber a pre-existing client (i.e.
if the address floated to is already owned by another client) unless
it can be verified that the pre-existing client is a previous instance
of the floating client.

Thanks to this mail, I've checked code again and found a bug - a lame
duck client may float to an address taken by another client. I have
submitted a fix:

http://thread.gmane.org/gmane.network.openvpn.devel/9386

which is waiting for an ACK.

Otherwise implementation is pretty much in line with your spec.

-Lev

2015-01-07 2:08 GMT+02:00 James Yonan :
> I've updated the OpenVPN protocol extension doc with additional details, now
> that more of these features have been implemented in OpenVPN 3.
>
> If you are implementing any of these features in OpenVPN 2.x, please review
> so we can ensure that OpenVPN 2.x and 3 are on the same page with respect to
> protocol extensions.
>
> Changes:
>
> 1. Added specific details on DATA_V2/peer-id/float support.
>
> 2. For AEAD mode, emphasized that the leading 8 bytes (4 bytes for
> DATA_V2/peer-id and 4 for packet ID) is all included in the AD.
>
> 3. Added specific details on protocol negotiation where the client indicates
> protocol extension availability with IV_x parameters in the peer info
> string, and the server responds by pushing directives to the client to
> enable the feature.
>
> 4. Added "TCP nonlinear mode" section, a new protocol extension that is
> needed by multithreaded TCP servers.
>
> James
>
> --
> Dive into the World of Parallel Programming! The Go Parallel Website,
> sponsored by Intel and developed in partnership with Slashdot Media, is your
> hub for all things parallel software development, from weekly thought
> leadership blogs to news, videos, case studies, tutorials and more. Take a
> look and join the conversation now. http://goparallel.sourceforge.net
> ___
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>



-- 
-Lev