Refuse PUSH_REQUEST as client/refactor process_incoming_push_request

When a server sends a client a push request, the client will reply
with a push reply. The reply is bogus and almost empty since almost
all the options that are normally set (remote ip etc) are unset.

I checked 2.4 and master and this does not have any security implications
or other bugs but it is still better to refuse this.

This code also refactors the method to get rid of the ret variable to
make the code flow easier to understand.

Signed-off-by: Arne Schwabe <[email protected]>

Author: schwabe

src/openvpn/push.c

@@ -966,15 +966,20 @@ push_remove_option(struct options *o, const char *p)
 int
 process_incoming_push_request(struct context *c)
 {
-    int ret = PUSH_MSG_ERROR;
+    /* if we receive a message as a client we do not want to reply to it. */
+    if (c->options.pull)
+    {
+        return PUSH_MSG_ERROR;
+    }
+
 
 
     if (tls_authentication_status(c->c2.tls_multi) == TLS_AUTHENTICATION_FAILED
         || c->c2.tls_multi->multi_state == CAS_FAILED)
     {
         const char *client_reason = tls_client_reason(c->c2.tls_multi);
         send_auth_failed(c, client_reason);
-        ret = PUSH_MSG_AUTH_FAILURE;
+        return PUSH_MSG_AUTH_FAILURE;
     }
     else if (tls_authentication_status(c->c2.tls_multi) == TLS_AUTHENTICATION_SUCCEEDED
              && c->c2.tls_multi->multi_state >= CAS_CONNECT_DONE)
@@ -984,29 +989,27 @@ process_incoming_push_request(struct context *c)
         openvpn_time(&now);
         if (c->c2.sent_push_reply_expiry > now)
         {
-            ret = PUSH_MSG_ALREADY_REPLIED;
+            return PUSH_MSG_ALREADY_REPLIED;
         }
-        else
-        {
-            /* per-client push options - peer-id, cipher, ifconfig, ipv6-ifconfig */
-            struct push_list push_list = { 0 };
-            struct gc_arena gc = gc_new();
 
-            if (prepare_push_reply(c, &gc, &push_list)
-                && send_push_reply(c, &push_list))
-            {
-                ret = PUSH_MSG_REQUEST;
-                c->c2.sent_push_reply_expiry = now + 30;
-            }
-            gc_free(&gc);
+        int ret = PUSH_MSG_ERROR;
+        /* per-client push options - peer-id, cipher, ifconfig, ipv6-ifconfig */
+        struct push_list push_list = { 0 };
+        struct gc_arena gc = gc_new();
+
+        if (prepare_push_reply(c, &gc, &push_list)
+            && send_push_reply(c, &push_list))
+        {
+            ret = PUSH_MSG_REQUEST;
+            c->c2.sent_push_reply_expiry = now + 30;
         }
+        gc_free(&gc);
+        return ret;
     }
     else
     {
-        ret = PUSH_MSG_REQUEST_DEFERRED;
+        return PUSH_MSG_REQUEST_DEFERRED;
     }
-
-    return ret;
 }
 
 static void