diff mbox series

[Openvpn-devel] protocol_dump: support tls-crypt

Message ID 8237adde-2523-9e48-5cd4-070463887dc1@gmail.com
State Accepted
Headers
Series [Openvpn-devel] protocol_dump: support tls-crypt |

Commit Message

Reynir Oct. 30, 2023, 9:58 a.m. UTC 
  Dear list,

Please find attached a patch to add support for tls-crypt packets in 
protocol_dump. Currently, protocol_dump will print garbage for tls-crypt 
packets.

This patch makes protocol_dump print the clear text parts of the packet 
such as the auth tag and replay packet id. It does not try to print the 
wKc for HARD_RESET_CLIENT_V3 or CONTROL_WKC_V1 packets. A previous 
iteration, not submitted to the list, printed ENCRYPTED placeholders for 
ack list and DATA, but I decided to cut down on the noise instead.

This is my first patch submitted to openvpn so please bear with me.

Best,
Reynir Björnsson

Comments

Arne Schwabe Nov. 20, 2023, 10:22 a.m. UTC | #1 
Am 30.10.23 um 11:58 schrieb Reynir:
> Dear list,
> 
> Please find attached a patch to add support for tls-crypt packets in 
> protocol_dump. Currently, protocol_dump will print garbage for tls-crypt 
> packets.
> 
> This patch makes protocol_dump print the clear text parts of the packet 
> such as the auth tag and replay packet id. It does not try to print the 
> wKc for HARD_RESET_CLIENT_V3 or CONTROL_WKC_V1 packets. A previous 
> iteration, not submitted to the list, printed ENCRYPTED placeholders for 
> ack list and DATA, but I decided to cut down on the noise instead.
> 
> This is my first patch submitted to openvpn so please bear with me.
>

Code looks good and works fine here.

Acked-By: Arne Schwabe <arne@rfc2549.org>
Gert Doering Nov. 20, 2023, 2:37 p.m. UTC | #2 
Your patch has been applied to the master and release/2.6 branch 
("printing garbage" is arguably a bug that needs fixing).

I have taken parts of the "mail mail" into the commit message, and
added the "signed-off-by:" that the OpenVPN project wants.

I haven't actually tested it beyond "does it compile" - it looks good,
and Arne is the current authority on tls-crypt anyway.

commit 227799b8345128dd3adf2029323457804209fe93 (master)
commit 0a39d1c1e2582330db09052bcf0e32bbf5bafde2 (release/2.6)
Author: Reynir Björnsson
Date:   Thu Oct 26 16:55:32 2023 +0200

     protocol_dump: tls-crypt support

     Signed-off-by: Reynir Björnsson <reynir@reynir.dk>
     Acked-by: Arne Schwabe <arne@rfc2549.org>
     Message-Id: <8237adde-2523-9e48-5cd4-070463887dc1@gmail.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg27310.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

From 11926a6234b860a09965e5a074460abe4b4f6e71 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Reynir=20Bj=C3=B6rnsson?= <reynir@reynir.dk>
Date: Thu, 26 Oct 2023 16:55:32 +0200
Subject: [PATCH] protocol_dump: tls-crypt support

---
 src/openvpn/openvpn.h |  3 ++-
 src/openvpn/ssl.c     | 26 ++++++++++++++++++++++++++
 src/openvpn/ssl.h     |  1 +
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h
index 077effeb..0816360d 100644
--- a/src/openvpn/openvpn.h
+++ b/src/openvpn/openvpn.h
@@ -544,7 +544,8 @@  struct context
 #define PROTO_DUMP(buf, gc) protocol_dump((buf), \
                                           PROTO_DUMP_FLAGS   \
                                           |(c->c2.tls_multi ? PD_TLS : 0)   \
-                                          |(c->options.tls_auth_file ? md_kt_size(c->c1.ks.key_type.digest) : 0), \
+                                          |(c->options.tls_auth_file ? md_kt_size(c->c1.ks.key_type.digest) : 0) \
+                                          |(c->options.tls_crypt_file || c->options.tls_crypt_v2_file ? PD_TLS_CRYPT : 0), \
                                           gc)
 
 /* this represents "disabled peer-id" */
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 5e6205cc..8bd3cb00 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -4202,6 +4202,32 @@  protocol_dump(struct buffer *buffer, unsigned int flags, struct gc_arena *gc)
         }
         buf_printf(&out, " pid=%s", packet_id_net_print(&pin, (flags & PD_VERBOSE), gc));
     }
+    /*
+     * packet_id + tls-crypt hmac
+     */
+    if (flags & PD_TLS_CRYPT)
+    {
+        struct packet_id_net pin;
+        uint8_t tls_crypt_hmac[TLS_CRYPT_TAG_SIZE];
+
+        if (!packet_id_read(&pin, &buf, true))
+        {
+            goto done;
+        }
+        buf_printf(&out, " pid=%s", packet_id_net_print(&pin, (flags & PD_VERBOSE), gc));
+        if (!buf_read(&buf, tls_crypt_hmac, TLS_CRYPT_TAG_SIZE))
+        {
+            goto done;
+        }
+        if (flags & PD_VERBOSE)
+        {
+            buf_printf(&out, " tls_crypt_hmac=%s", format_hex(tls_crypt_hmac, TLS_CRYPT_TAG_SIZE, 0, gc));
+        }
+        /*
+         * Remainder is encrypted and optional wKc
+         */
+        goto done;
+    }
 
     /*
      * ACK list
diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index 3c40fbed..e8427461 100644
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -525,6 +525,7 @@  tls_set_single_session(struct tls_multi *multi)
 #define PD_SHOW_DATA               (1<<8)
 #define PD_TLS                     (1<<9)
 #define PD_VERBOSE                 (1<<10)
+#define PD_TLS_CRYPT               (1<<11)
 
 const char *protocol_dump(struct buffer *buffer,
                           unsigned int flags,
-- 
2.30.2