diff mbox series

[Openvpn-devel,v2] Allow management to kill client instances by CN wildcard

Message ID 20200814171204.22289-1-themiron@yandex-team.ru
State Changes Requested
Headers
Series [Openvpn-devel,v2] Allow management to kill client instances by CN wildcard |

Commit Message

Vladislav Grishenko Aug. 14, 2020, 7:12 a.m. UTC 
  In case of some permanent part of common name (ex. domain) and/or
long complex common name consisting of multiple x509 fields, it's
handly to kill client instances via management interface with just
prefix of common name, not by exact match only.

Patch allows to use asterisk as wildcard placeholder in the last
trailing symbol of kill command parameter.
Single asterisk - empty prefix would be too greedy and can be too
harmful, therefore not allowed. Wildcards in the middle of
parameter string are not supported to keep the the things simple at the moment.

v2: fine tune comments

Signed-off-by: Vladislav Grishenko <themiron@yandex-team.ru>
---
 doc/management-notes.txt |  2 ++
 src/openvpn/multi.c      | 15 ++++++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

Comments

Arne Schwabe Aug. 14, 2020, 7:35 a.m. UTC | #1 
Am 14.08.20 um 19:12 schrieb Vladislav Grishenko:
> In case of some permanent part of common name (ex. domain) and/or
> long complex common name consisting of multiple x509 fields, it's
> handly to kill client instances via management interface with just
> prefix of common name, not by exact match only.
> 
> Patch allows to use asterisk as wildcard placeholder in the last
> trailing symbol of kill command parameter.
> Single asterisk - empty prefix would be too greedy and can be too
> harmful, therefore not allowed. Wildcards in the middle of
> parameter string are not supported to keep the the things simple at the moment.
> 
> v2: fine tune comments
>

Thanks for v2,

Acked-By; Arne Schwabe <arne@rfc2549.org>
Selva Nair Aug. 14, 2020, 8:22 a.m. UTC | #2 
Hi

On Fri, Aug 14, 2020 at 1:36 PM Arne Schwabe <arne@rfc2549.org> wrote:
>
> Am 14.08.20 um 19:12 schrieb Vladislav Grishenko:
> > In case of some permanent part of common name (ex. domain) and/or
> > long complex common name consisting of multiple x509 fields, it's
> > handly to kill client instances via management interface with just
> > prefix of common name, not by exact match only.
> >
> > Patch allows to use asterisk as wildcard placeholder in the last
> > trailing symbol of kill command parameter.
> > Single asterisk - empty prefix would be too greedy and can be too
> > harmful, therefore not allowed. Wildcards in the middle of
> > parameter string are not supported to keep the the things simple at the moment.
> >
> > v2: fine tune comments
> >
>
> Thanks for v2,
>
> Acked-By; Arne Schwabe <arne@rfc2549.org>

'*' is an allowed character in x509 common name unless we explicitly
forbid it. So killing a client with name ending in * would get tricky
if not impossible without side effects.

Selva
Vladislav Grishenko Aug. 14, 2020, 9:06 a.m. UTC | #3 
Hi,

Yes, killing a client with cn ending in * will also lead to killing all the
clients whose cn starts with that prefix.
Use other char would no-intuitive (ex. +).
What about optional "prefix" mode word for explicit mode (can be also
enhanced one day with suffix/regexp/etc).

	kill cn [mode]: Kill the client instance(s) having common name cn.

--
Best Regards, Vladislav Grishenko

-----Original Message-----
From: Selva Nair <selva.nair@gmail.com> 
Sent: Friday, August 14, 2020 11:22 PM
To: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Subject: Re: [Openvpn-devel] [PATCH v2] Allow management to kill client
instances by CN wildcard

Hi

On Fri, Aug 14, 2020 at 1:36 PM Arne Schwabe <arne@rfc2549.org> wrote:
>
> Am 14.08.20 um 19:12 schrieb Vladislav Grishenko:
> > In case of some permanent part of common name (ex. domain) and/or 
> > long complex common name consisting of multiple x509 fields, it's 
> > handly to kill client instances via management interface with just 
> > prefix of common name, not by exact match only.
> >
> > Patch allows to use asterisk as wildcard placeholder in the last 
> > trailing symbol of kill command parameter.
> > Single asterisk - empty prefix would be too greedy and can be too 
> > harmful, therefore not allowed. Wildcards in the middle of parameter 
> > string are not supported to keep the the things simple at the moment.
> >
> > v2: fine tune comments
> >
>
> Thanks for v2,
>
> Acked-By; Arne Schwabe <arne@rfc2549.org>

'*' is an allowed character in x509 common name unless we explicitly forbid
it. So killing a client with name ending in * would get tricky if not
impossible without side effects.

Selva
Selva Nair Aug. 14, 2020, 9:36 a.m. UTC | #4 
Hi

On Fri, Aug 14, 2020 at 3:06 PM Vladislav Grishenko
<themiron@yandex-team.ru> wrote:
>
> Hi,
>
> Yes, killing a client with cn ending in * will also lead to killing all the
> clients whose cn starts with that prefix.
> Use other char would no-intuitive (ex. +).
> What about optional "prefix" mode word for explicit mode (can be also
> enhanced one day with suffix/regexp/etc).
>
>         kill cn [mode]: Kill the client instance(s) having common name cn.

That sounds good to me -- avoids the use of any special character.

Also, updating the "help" command of management interface was missed
in the previous version of the patch.

Selva

>
> --
> Best Regards, Vladislav Grishenko
>
> -----Original Message-----
> From: Selva Nair <selva.nair@gmail.com>
> Sent: Friday, August 14, 2020 11:22 PM
> To: openvpn-devel <openvpn-devel@lists.sourceforge.net>
> Subject: Re: [Openvpn-devel] [PATCH v2] Allow management to kill client
> instances by CN wildcard
>
> Hi
>
> On Fri, Aug 14, 2020 at 1:36 PM Arne Schwabe <arne@rfc2549.org> wrote:
> >
> > Am 14.08.20 um 19:12 schrieb Vladislav Grishenko:
> > > In case of some permanent part of common name (ex. domain) and/or
> > > long complex common name consisting of multiple x509 fields, it's
> > > handly to kill client instances via management interface with just
> > > prefix of common name, not by exact match only.
> > >
> > > Patch allows to use asterisk as wildcard placeholder in the last
> > > trailing symbol of kill command parameter.
> > > Single asterisk - empty prefix would be too greedy and can be too
> > > harmful, therefore not allowed. Wildcards in the middle of parameter
> > > string are not supported to keep the the things simple at the moment.
> > >
> > > v2: fine tune comments
> > >
> >
> > Thanks for v2,
> >
> > Acked-By; Arne Schwabe <arne@rfc2549.org>
>
> '*' is an allowed character in x509 common name unless we explicitly forbid
> it. So killing a client with name ending in * would get tricky if not
> impossible without side effects.
>
> Selva
>
>
> _______________________________________________
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>
diff mbox series

Patch

diff --git a/doc/management-notes.txt b/doc/management-notes.txt
index 61daaf07..91073693 100644
--- a/doc/management-notes.txt
+++ b/doc/management-notes.txt
@@ -195,6 +195,8 @@  Command examples:
 
   kill Test-Client -- kill the client instance having a
                       common name of "Test-Client".
+  kill Test-Cli*   -- kill the client instances having a
+                      common name starting with "Test-Cli".
   kill 1.2.3.4:4000 -- kill the client instance having a
                        source address and port of 1.2.3.4:4000
 
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 13738180..36be5de2 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -3820,6 +3820,19 @@  management_callback_kill_by_cn(void *arg, const char *del_cn)
     struct hash_element *he;
     int count = 0;
 
+    /* Check passed string for non-empty prefix with trailing asterisk */
+    size_t len = strlen(del_cn);
+    if (len > 1 && del_cn[len - 1] == '*')
+    {
+        /* Exclude trailing asterisk from string comparison */
+        len--;
+    }
+    else
+    {
+        /* Include terminating NUL char to perform exact string comparison */
+        len++;
+    }
+
     hash_iterator_init(m->iter, &hi);
     while ((he = hash_iterator_next(&hi)))
     {
@@ -3827,7 +3840,7 @@  management_callback_kill_by_cn(void *arg, const char *del_cn)
         if (!mi->halt)
         {
             const char *cn = tls_common_name(mi->context.c2.tls_multi, false);
-            if (cn && !strcmp(cn, del_cn))
+            if (cn && !strncmp(cn, del_cn, len))
             {
                 multi_signal_instance(m, mi, SIGTERM);
                 ++count;