[Openvpn-devel,2/2] Add negotiated cipher to status file format 2 and 3

Message ID 20171111161122.30087-2-gert@gertvandijk.net
State Accepted
Headers show
Series
  • [Openvpn-devel,1/2] manpage: improve description of --status and --status-version
Related show

Commit Message

Gert van Dijk Nov. 11, 2017, 4:11 p.m.
With NCP turned off, this will still display the cipher used.

Trac: #814

Signed-off-by: Gert van Dijk <gert@gertvandijk.net>
---
 doc/openvpn.8       |  2 +-
 src/openvpn/multi.c | 11 +++++++----
 2 files changed, 8 insertions(+), 5 deletions(-)

Comments

Arne Schwabe Nov. 12, 2017, 8:09 p.m. | #1
Am 11.11.17 um 17:11 schrieb Gert van Dijk:
> With NCP turned off, this will still display the cipher used.
> 
> Trac: #814
> 
> Signed-off-by: Gert van Dijk <gert@gertvandijk.net>
> ---
>  doc/openvpn.8       |  2 +-
>  src/openvpn/multi.c | 11 +++++++----
>  2 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/doc/openvpn.8 b/doc/openvpn.8
> index 267497fd..00dbff6f 100644
> --- a/doc/openvpn.8
> +++ b/doc/openvpn.8
> @@ -2477,7 +2477,7 @@ Connected Since.
>  .B 2
>  \-\- a more reliable format for external processing. Compared to version 1, the
>  client list contains some additional fields: Virtual Address, Virtual IPv6
> -Address, Username, Client ID, Peer ID.
> +Address, Username, Client ID, Peer ID, Data Channel Cipher.
>  Future versions may extend the number of fields.
>  .br
>  .B 3


This part of the patch is okay but does not apply cleanly for me. Might
need some on the fly fixing on apply.

> @@ -940,8 +942,8 @@ multi_print_status(struct multi_context *m, struct status_output *so, const int
>               */
>              status_printf(so, "TITLE%c%s", sep, title_string);
>              status_printf(so, "TIME%c%s%c%u", sep, time_string(now, 0, false, &gc_top), sep, (unsigned int)now);
> -            status_printf(so, "HEADER%cCLIENT_LIST%cCommon Name%cReal Address%cVirtual Address%cVirtual IPv6 Address%cBytes Received%cBytes Sent%cConnected Since%cConnected Since (time_t)%cUsername%cClient ID%cPeer ID",
> -                          sep, sep, sep, sep, sep, sep, sep, sep, sep, sep, sep, sep);
> +            status_printf(so, "HEADER%cCLIENT_LIST%cCommon Name%cReal Address%cVirtual Address%cVirtual IPv6 Address%cBytes Received%cBytes Sent%cConnected Since%cConnected Since (time_t)%cUsername%cClient ID%cPeer ID%cData Channel Cipher",



Extremely long line, but not your fault, so I will let it pass.


Works for me and code is good so:

Acked-by: Arne Schwabe <arne@rfc2549.org>
Tested-by: Arne Schwabe <arne@rfc2549.org>


Arne

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Gert van Dijk Nov. 12, 2017, 8:18 p.m. | #2
Hi Arne,

Thanks for reviewing this :)

On 12-11-17 21:09, Arne Schwabe wrote:
> Am 11.11.17 um 17:11 schrieb Gert van Dijk:
>> With NCP turned off, this will still display the cipher used.
>>
>> Trac: #814
>>
>> Signed-off-by: Gert van Dijk <gert@gertvandijk.net>
>> ---
>>  doc/openvpn.8       |  2 +-
>>  src/openvpn/multi.c | 11 +++++++----
>>  2 files changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/doc/openvpn.8 b/doc/openvpn.8
>> index 267497fd..00dbff6f 100644
>> --- a/doc/openvpn.8
>> +++ b/doc/openvpn.8
>> @@ -2477,7 +2477,7 @@ Connected Since.
>>  .B 2
>>  \-\- a more reliable format for external processing. Compared to version 1, the
>>  client list contains some additional fields: Virtual Address, Virtual IPv6
>> -Address, Username, Client ID, Peer ID.
>> +Address, Username, Client ID, Peer ID, Data Channel Cipher.
>>  Future versions may extend the number of fields.
>>  .br
>>  .B 3
> 
> 
> This part of the patch is okay but does not apply cleanly for me. Might
> need some on the fly fixing on apply.
> 

Did you apply the first part ([PATCH 1/2] manpage: improve description
of --status and --status-version)?

>> @@ -940,8 +942,8 @@ multi_print_status(struct multi_context *m, struct status_output *so, const int
>>               */
>>              status_printf(so, "TITLE%c%s", sep, title_string);
>>              status_printf(so, "TIME%c%s%c%u", sep, time_string(now, 0, false, &gc_top), sep, (unsigned int)now);
>> -            status_printf(so, "HEADER%cCLIENT_LIST%cCommon Name%cReal Address%cVirtual Address%cVirtual IPv6 Address%cBytes Received%cBytes Sent%cConnected Since%cConnected Since (time_t)%cUsername%cClient ID%cPeer ID",
>> -                          sep, sep, sep, sep, sep, sep, sep, sep, sep, sep, sep, sep);
>> +            status_printf(so, "HEADER%cCLIENT_LIST%cCommon Name%cReal Address%cVirtual Address%cVirtual IPv6 Address%cBytes Received%cBytes Sent%cConnected Since%cConnected Since (time_t)%cUsername%cClient ID%cPeer ID%cData Channel Cipher",
> 
> 
> 
> Extremely long line, but not your fault, so I will let it pass.
> 
> 
> Works for me and code is good so:
> 
> Acked-by: Arne Schwabe <arne@rfc2549.org>
> Tested-by: Arne Schwabe <arne@rfc2549.org>>
> 
> Arne
> 

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Gert Doering March 24, 2018, 6:35 p.m. | #3
Your patch has been applied to the master branch.

(Only to master, because "new feature" - while harmless enough, we want
to lure people into running 2.5 eventually, so: immediate benefit... :-) 
wrt the side discussion about "applies cleanly" - it does, with 1/2 in
place)

commit 8acc40b6a64451d9a17cf4fa12fac2450ca26095
Author: Gert van Dijk
Date:   Sat Nov 11 17:11:22 2017 +0100

     Add negotiated cipher to status file format 2 and 3

     Signed-off-by: Gert van Dijk <gert@gertvandijk.net>
     Acked-by: Arne Schwabe <arne@rfc2549.org>
     Message-Id: <20171111161122.30087-2-gert@gertvandijk.net>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15817.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Patch

diff --git a/doc/openvpn.8 b/doc/openvpn.8
index 267497fd..00dbff6f 100644
--- a/doc/openvpn.8
+++ b/doc/openvpn.8
@@ -2477,7 +2477,7 @@  Connected Since.
 .B 2
 \-\- a more reliable format for external processing. Compared to version 1, the
 client list contains some additional fields: Virtual Address, Virtual IPv6
-Address, Username, Client ID, Peer ID.
+Address, Username, Client ID, Peer ID, Data Channel Cipher.
 Future versions may extend the number of fields.
 .br
 .B 3
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 82a0b9d9..dfad582f 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -50,6 +50,8 @@ 
 #include "forward-inline.h"
 #include "pf-inline.h"
 
+#include "crypto_backend.h"
+
 /*#define MULTI_DEBUG_EVENT_LOOP*/
 
 #ifdef MULTI_DEBUG_EVENT_LOOP
@@ -940,8 +942,8 @@  multi_print_status(struct multi_context *m, struct status_output *so, const int
              */
             status_printf(so, "TITLE%c%s", sep, title_string);
             status_printf(so, "TIME%c%s%c%u", sep, time_string(now, 0, false, &gc_top), sep, (unsigned int)now);
-            status_printf(so, "HEADER%cCLIENT_LIST%cCommon Name%cReal Address%cVirtual Address%cVirtual IPv6 Address%cBytes Received%cBytes Sent%cConnected Since%cConnected Since (time_t)%cUsername%cClient ID%cPeer ID",
-                          sep, sep, sep, sep, sep, sep, sep, sep, sep, sep, sep, sep);
+            status_printf(so, "HEADER%cCLIENT_LIST%cCommon Name%cReal Address%cVirtual Address%cVirtual IPv6 Address%cBytes Received%cBytes Sent%cConnected Since%cConnected Since (time_t)%cUsername%cClient ID%cPeer ID%cData Channel Cipher",
+                          sep, sep, sep, sep, sep, sep, sep, sep, sep, sep, sep, sep, sep);
             hash_iterator_init(m->hash, &hi);
             while ((he = hash_iterator_next(&hi)))
             {
@@ -956,7 +958,7 @@  multi_print_status(struct multi_context *m, struct status_output *so, const int
 #else
                                   ""
 #endif
-                                  "%c%" PRIu32,
+                                  "%c%" PRIu32 "%c%s",
                                   sep, tls_common_name(mi->context.c2.tls_multi, false),
                                   sep, mroute_addr_print(&mi->real, &gc),
                                   sep, print_in_addr_t(mi->reporting_addr, IA_EMPTY_IF_UNDEF, &gc),
@@ -971,7 +973,8 @@  multi_print_status(struct multi_context *m, struct status_output *so, const int
 #else
                                   sep,
 #endif
-                                  sep, mi->context.c2.tls_multi ? mi->context.c2.tls_multi->peer_id : UINT32_MAX);
+                                  sep, mi->context.c2.tls_multi ? mi->context.c2.tls_multi->peer_id : UINT32_MAX,
+                                  sep, translate_cipher_name_to_openvpn(mi->context.options.ciphername));
                 }
                 gc_free(&gc);
             }