[Openvpn-devel,v2] Persist management-query-remote and proxy prompts

Message ID 1582254028-7763-1-git-send-email-selva.nair@gmail.com
State Accepted
Headers show
Series [Openvpn-devel,v2] Persist management-query-remote and proxy prompts | expand

Commit Message

Selva Nair Feb. 20, 2020, 4 p.m. UTC
From: Selva Nair <selva.nair@gmail.com>

Currently this prompt is only output once, not re-written to the
management interface when the management client connects. It is thus
not seen by a client that connects after the prompt is output or one that
disconnects and reconnects. This leads to a deadlock: the daemon waiting
for the "remote" command from the client, the latter not aware of it.

Resolve by adding the ">REMOTE" and ">PROXY" prompt to
man.persist.special_state_msg as done for other persisted prompts such
as ">PASSWORD"

Signed-off-by: Selva Nair <selva.nair@gmail.com>
---
v2: bump and rebase to master

 src/openvpn/init.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Gert Doering May 13, 2020, 3:57 a.m. UTC | #1
Acked-by: Gert Doering <gert@greenie.muc.de>

I won't claim to understand management, but the patch does what it
says, and it does so in the same way as querying for >PASSWORD is
done.  Selva understands management way better than I do, so if he
says this is needed, and the code is in line with what we currently
have (manage.c, management_query_user_pass(), management_hold(),
management_query_multiline()...) and does not look like "mem leak" 
or "overflow" - which it doesn't - good enough for me.

The code *looks* as if a reference to "out" is going out of scope,
but that one is allocated in &gc, so it's fine.  Just hard to read.

These functions could do with a bit of C99 modernizing, getting 
rid of extra nesting levels that are just there to enable local
variables... but that's for a different round of refactoring.


Your patch has been applied to the master branch.

commit 93ba6ccddafcc87f336f50dadde144ea4f6178ad
Author: Selva Nair
Date:   Thu Feb 20 22:00:28 2020 -0500

     Persist management-query-remote and proxy prompts

     Signed-off-by: Selva Nair <selva.nair@gmail.com>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <1582254028-7763-1-git-send-email-selva.nair@gmail.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19497.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
Gert Doering May 13, 2020, 4:24 a.m. UTC | #2
Hi,

On Wed, May 13, 2020 at 03:57:54PM +0200, Gert Doering wrote:
> Your patch has been applied to the master branch.
> 
> commit 93ba6ccddafcc87f336f50dadde144ea4f6178ad
> Author: Selva Nair
> Date:   Thu Feb 20 22:00:28 2020 -0500


*and* to release/2.4, because "bugfix".  Forgot about that comment when
I was done with review and testing...

commit 38b46e6bf65489c2c5d75da1c02a3a1c33e6da88 (HEAD -> release/2.4)
Author: Selva Nair <selva.nair@gmail.com>
Date:   Thu Feb 20 22:00:28 2020 -0500

    Persist management-query-remote and proxy prompts


applies fine, tests fine.

gert

Patch

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 1cfffbb..b4781a2 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -271,6 +271,7 @@  ce_management_query_proxy(struct context *c)
             buf_printf(&out, ">PROXY:%u,%s,%s", (l ? l->current : 0) + 1,
                        (proto_is_udp(ce->proto) ? "UDP" : "TCP"), np(ce->remote));
             management_notify_generic(management, BSTR(&out));
+            management->persist.special_state_msg = BSTR(&out);
         }
         ce->flags |= CE_MAN_QUERY_PROXY;
         while (ce->flags & CE_MAN_QUERY_PROXY)
@@ -282,6 +283,7 @@  ce_management_query_proxy(struct context *c)
                 break;
             }
         }
+        management->persist.special_state_msg = NULL;
         gc_free(&gc);
     }
 
@@ -351,6 +353,7 @@  ce_management_query_remote(struct context *c)
         buf_printf(&out, ">REMOTE:%s,%s,%s", np(ce->remote), ce->remote_port,
                    proto2ascii(ce->proto, ce->af, false));
         management_notify_generic(management, BSTR(&out));
+        management->persist.special_state_msg = BSTR(&out);
 
         ce->flags &= ~(CE_MAN_QUERY_REMOTE_MASK << CE_MAN_QUERY_REMOTE_SHIFT);
         ce->flags |= (CE_MAN_QUERY_REMOTE_QUERY << CE_MAN_QUERY_REMOTE_SHIFT);
@@ -364,6 +367,7 @@  ce_management_query_remote(struct context *c)
                 break;
             }
         }
+        management->persist.special_state_msg = NULL;
     }
     gc_free(&gc);