Exclude ping and control packets from activity

Problem: using --ping and --inactive together partially defeats the
point of using --inactive as periodic ping packets are counted as
activity. Here is the original discussion:

http://article.gmane.org/gmane.network.openvpn.devel/3676

It turns out that "activity" is detected and recorded in two places
in the code, both in forward.c: in process_outgoing_tun() for received
packets, after they've been decrypted and sent to the TUN device; and
in process_outgoing_link(), after they've been encrypted and written
to the network socket.

In the first case we can be sure that packets that get so far are
really due to user activity, whereas in the second case there can be
non-user packets (like OpenVPN's internal ping packets, and TLS control
packets), and those should not be counted as activity as they are not
coming from the user.

So a need arises to detect those control packets and not count them as
activity for the purposes of --inactive. Unfortunately, at that stage
packets are already compressed and encrypted, so it's not possible to
look into them to see what they are. However, there seems to be a
convention in the code that packets whose buffer length in the context_2
structure is 0 should be ignored for certain purposes. TLS control
packets follow that convention already, so this patch makes a small
change in the code that generates the ping packets to set their buffer
length to 0 as well.
Finally, the call to register_activity() in process_outgoing_link() is
made conditional to the buffer length being > 0.

According to my tests, now --inactive behaves correctly according to
the configured parameters (time or time+bytes) even when --ping is
being used.

forward.c:
    Call register_activity() in process_outgoing_link() only if the
    packet is not a ping or TLS control packet.

openvpn.8:
    Updated the description of --inactive to describe the new semantics.

ping.c:
    Set c->c2.buf.len = 0 after the ping packet has been generated and
    encrypted.

Test routine is described here:
<https://community.openvpn.net/openvpn/wiki/PingInactivePatch?version=6>

Signed-off-by: Davide Brini <dave_br@gmx.com>
Acked-by: David Sommerseth <dazo@users.sourceforge.net>
Signed-off-by: David Sommerseth <dazo@users.sourceforge.net>

3 files changed, 20 insertions, 7 deletions

diff --git a/forward.c b/forward.c
index a0d67d0..6e3c5f7 100644
--- a/forward.c
+++ b/forward.c
@@ -1168,8 +1168,9 @@ process_outgoing_link (struct context *c)
 		 size);
 	}
 
-      /* indicate activity regarding --inactive parameter */
-      register_activity (c, size);
+      /* if not a ping/control message, indicate activity regarding --inactive parameter */
+      if (c->c2.buf.len > 0 )
+        register_activity (c, size);
     }
   else
     {
diff --git a/openvpn.8 b/openvpn.8
index 6357b95..f7fe55a 100644
--- a/openvpn.8
+++ b/openvpn.8
@@ -1366,15 +1366,25 @@ to be between 100 bytes/sec and 100 Mbytes/sec.
 .B \-\-inactive n [bytes]
 Causes OpenVPN to exit after
 .B n
-seconds of inactivity on the TUN/TAP device.  The time length
-of inactivity is measured since the last incoming tunnel packet.
+seconds of inactivity on the TUN/TAP device. The time length of
+inactivity is measured since the last incoming or outgoing tunnel
+packet.
 
 If the optional
 .B bytes
 parameter is included,
-exit after n seconds of activity on tun/tap device
-produces a combined in/out byte count that is less than
-.B bytes.
+exit if less than
+.B bytes
+of combined in/out traffic are produced on the tun/tap device
+in
+.B n
+seconds.
+
+In any case, OpenVPN's internal ping packets (which are just
+keepalives) and TLS control packets are not considered
+"activity", nor are they counted as traffic, as they are used
+internally by OpenVPN and are not an indication of actual user
+activity.
 .\"*********************************************************
 .TP
 .B \-\-ping n
diff --git a/ping.c b/ping.c
index b29927d..191ad74 100644
--- a/ping.c
+++ b/ping.c
@@ -86,5 +86,7 @@ check_ping_send_dowork (struct context *c)
    * encrypt, sign, etc.
    */
   encrypt_sign (c, true);
+  /* Set length to 0, so it won't be counted as activity */
+  c->c2.buf.len = 0;
   dmsg (D_PING, "SENT PING");
 }