Bug#1112645: bookworm-pu: package openvpn/2.6.3-1+deb12u4
Package: release.debian.org
Severity: normal
Tags: bookworm
X-Debbugs-Cc: openvpn@packages.debian.org, berni@debian.org
Control: affects -1 + src:openvpn
User: release.debian.org@packages.debian.org
Usertags: pu
Hi,
[ Reason ]
In 2.6.3-1+deb12u3 we did cherry-pick upstream's fix to CVE-2024-5594
[1], but later a regression was reported in upstream's BTS [2]. The
initial fix to the vulnerability was to restrict characters in control
channel messages including \n and \r, but many scripts add them in these
messages. Suddenly these scripts will fail to connect after the update
to fix the CVE. Although we didn't receive reports initially, there was
reports from people using Arch and Ubuntu with services like watchguard
[2] and Microsoft 2FA [3]. The fix basically allows \n and \r in the
control channel messages.
[ Impact ]
Users using scripts to handle connection or third party services may
be impacted and unable to connect using openvpn.
[ Tests ]
Unit tests are now enabled as part of autopkgtests and they succeed. The
upstream commit also comes with unit tests. Additionaly, the other DEP-8
requiring isolation-machine were run locally in a incus bookworm VM and
passed.
[ Risks ]
The code change is not large or intrusive, it basically encapsulates the
logic handling the buffer read and add a function to remove trailing \r
and \n from the end of message. It was well tested and applied by
upstream in the stable releases of openvpn 2.5 and 2.6.
[ Checklist ]
[x] *all* changes are documented in the d/changelog
[x] I reviewed all changes and I approve them
[x] attach debdiff against the package in (old)stable
[x] the issue is verified as fixed in unstable
[ Changes ]
As explained in "Reason" and "Risks", the upstream commit to fix a
regression was cherry-picked. Additionally unit tests were added as part
of autopkgtests and some changes related to salsa-ci were done for the
pipeline to succed.
[ Other info ]
This bookworm-pu is targeted to 12.13 so we can have more time in
-proposed for testing as indicated by openvpn's maintainer [4].
Cheers,
Charles
[1] https://github.com/OpenVPN/openvpn/commit/90e7a858e5594d9a019ad2b4ac6154124986291a
[2] https://github.com/OpenVPN/openvpn/issues/568
[3] https://github.com/OpenVPN/openvpn/issues/645
[4] https://salsa.debian.org/debian/openvpn/-/merge_requests/16#note_647132
diff -Nru openvpn-2.6.3/debian/changelog openvpn-2.6.3/debian/changelog
--- openvpn-2.6.3/debian/changelog 2025-04-02 12:45:15.000000000 -0300
+++ openvpn-2.6.3/debian/changelog 2025-08-24 22:36:22.000000000 -0300
@@ -1,3 +1,23 @@
+openvpn (2.6.3-1+deb12u4) bookworm; urgency=medium
+
+ * Team upload.
+
+ [ Aquila Macedo ]
+ * Add new autopkgtest for unit tests.
+
+ [ Carlos Henrique Lima Melara ]
+ * debian/patches/CVE-2024-5594-regression-fix.patch: cherry-pick from
+ upstream to fix a regression introduced with CVE-2024-5594's fix. Namely,
+ "Allow trailing \r and \n in control channel message". (Closes: #1112516)
+ * debian/salsa-ci:
+ - Allow lintian job to fail. Sid's version dislikes things from bookworm.
+ - Disable gbp setup-gitattributes.
+ - Disable reprotest on bookworm. It can't run on bookworm, so the build
+ fails because of build dependencies problems.
+ * debian/tests/unit-tests: enable unit-tests in configure and be verbose.
+
+ -- Carlos Henrique Lima Melara <charlesmelara@riseup.net> Sun, 24 Aug 2025 22:36:22 -0300
+
openvpn (2.6.3-1+deb12u3) bookworm; urgency=medium
[ Bernhard Schmidt ]
diff -Nru openvpn-2.6.3/debian/patches/CVE-2024-5594-regression-fix.patch openvpn-2.6.3/debian/patches/CVE-2024-5594-regression-fix.patch
--- openvpn-2.6.3/debian/patches/CVE-2024-5594-regression-fix.patch 1969-12-31 21:00:00.000000000 -0300
+++ openvpn-2.6.3/debian/patches/CVE-2024-5594-regression-fix.patch 2025-08-24 22:36:22.000000000 -0300
@@ -0,0 +1,203 @@
+From: Arne Schwabe <arne@rfc2549.org>
+Date: Wed, 10 Jul 2024 16:06:23 +0200
+Subject: Allow trailing \r and \n in control channel message
+
+Writing a reason from a script will easily end up adding extra \r\n characters
+at the end of the reason. Our current code pushes this to the peer. So be more
+liberal in accepting these message.
+
+Github: closes OpenVPN/openvpn#568
+
+Change-Id: I47c992b6b73b1475cbff8a28f720cf50dc1fbe3e
+Signed-off-by: Arne Schwabe <arne@rfc2549.org>
+Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
+Message-Id: <20240710140623.172829-1-frank@lichtenheld.com>
+URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28910.html
+Signed-off-by: Gert Doering <gert@greenie.muc.de>
+(cherry picked from commit be31325e1dfdffbb152374985c2ae7b6644e3519)
+
+Origin: upstream, https://github.com/OpenVPN/openvpn/commit/343573990135023d855d151fcd9248e5c26d9f8b
+Bug: https://github.com/OpenVPN/openvpn/issues/568
+Last-Update: 2025-08-24
+---
+ src/openvpn/forward.c | 33 +++---------------------------
+ src/openvpn/ssl_pkt.c | 40 +++++++++++++++++++++++++++++++++++++
+ src/openvpn/ssl_pkt.h | 14 +++++++++++++
+ tests/unit_tests/openvpn/test_pkt.c | 35 ++++++++++++++++++++++++++++++++
+ 4 files changed, 92 insertions(+), 30 deletions(-)
+
+diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
+index b565bfa..48c4276 100644
+--- a/src/openvpn/forward.c
++++ b/src/openvpn/forward.c
+@@ -292,41 +292,14 @@ check_incoming_control_channel(struct context *c)
+ struct buffer buf = alloc_buf_gc(len, &gc);
+ if (tls_rec_payload(c->c2.tls_multi, &buf))
+ {
+-
+ while (BLEN(&buf) > 1)
+ {
+- /* commands on the control channel are seperated by 0x00 bytes.
+- * cmdlen does not include the 0 byte of the string */
+- int cmdlen = (int)strnlen(BSTR(&buf), BLEN(&buf));
+-
+- if (cmdlen < BLEN(&buf))
+- {
+- /* include the NUL byte and ensure NUL termination */
+- int cmdlen = (int)strlen(BSTR(&buf)) + 1;
++ struct buffer cmdbuf = extract_command_buffer(&buf, &gc);
+
+- /* Construct a buffer that only holds the current command and
+- * its closing NUL byte */
+- struct buffer cmdbuf = alloc_buf_gc(cmdlen, &gc);
+- buf_write(&cmdbuf, BPTR(&buf), cmdlen);
+-
+- /* check we have only printable characters or null byte in the
+- * command string and no newlines */
+- if (!string_check_buf(&buf, CC_PRINT | CC_NULL, CC_CRLF))
+- {
+- msg(D_PUSH_ERRORS, "WARNING: Received control with invalid characters: %s",
+- format_hex(BPTR(&buf), BLEN(&buf), 256, &gc));
+- }
+- else
+- {
+- parse_incoming_control_channel_command(c, &cmdbuf);
+- }
+- }
+- else
++ if (cmdbuf.len > 0)
+ {
+- msg(D_PUSH_ERRORS, "WARNING: Ignoring control channel "
+- "message command without NUL termination");
++ parse_incoming_control_channel_command(c, &cmdbuf);
+ }
+- buf_advance(&buf, cmdlen);
+ }
+ }
+ else
+diff --git a/src/openvpn/ssl_pkt.c b/src/openvpn/ssl_pkt.c
+index 7229f55..42cb130 100644
+--- a/src/openvpn/ssl_pkt.c
++++ b/src/openvpn/ssl_pkt.c
+@@ -560,3 +560,43 @@ check_session_id_hmac(struct tls_pre_decrypt_state *state,
+ }
+ return false;
+ }
++
++struct buffer
++extract_command_buffer(struct buffer *buf, struct gc_arena *gc)
++{
++ /* commands on the control channel are seperated by 0x00 bytes.
++ * cmdlen does not include the 0 byte of the string */
++ int cmdlen = (int)strnlen(BSTR(buf), BLEN(buf));
++
++ if (cmdlen >= BLEN(buf))
++ {
++ buf_advance(buf, cmdlen);
++ /* Return empty buffer */
++ struct buffer empty = { 0 };
++ return empty;
++ }
++
++ /* include the NUL byte and ensure NUL termination */
++ cmdlen += 1;
++
++ /* Construct a buffer that only holds the current command and
++ * its closing NUL byte */
++ struct buffer cmdbuf = alloc_buf_gc(cmdlen, gc);
++ buf_write(&cmdbuf, BPTR(buf), cmdlen);
++
++ /* Remove \r and \n at the end of the buffer to avoid
++ * problems with scripts and other that add extra \r and \n */
++ buf_chomp(&cmdbuf);
++
++ /* check we have only printable characters or null byte in the
++ * command string and no newlines */
++ if (!string_check_buf(&cmdbuf, CC_PRINT | CC_NULL, CC_CRLF))
++ {
++ msg(D_PUSH_ERRORS, "WARNING: Received control with invalid characters: %s",
++ format_hex(BPTR(&cmdbuf), BLEN(&cmdbuf), 256, gc));
++ cmdbuf.len = 0;
++ }
++
++ buf_advance(buf, cmdlen);
++ return cmdbuf;
++}
+diff --git a/src/openvpn/ssl_pkt.h b/src/openvpn/ssl_pkt.h
+index 43c303f..f92eacc 100644
+--- a/src/openvpn/ssl_pkt.h
++++ b/src/openvpn/ssl_pkt.h
+@@ -238,6 +238,20 @@ tls_reset_standalone(struct tls_wrap_ctx *ctx,
+ uint8_t header,
+ bool request_resend_wkc);
+
++
++/**
++ * Extracts a control channel message from buf and adjusts the size of
++ * buf after the message has been extracted
++ * @param buf The buffer the message should be extracted from
++ * @param gc gc_arena to be used for the returned buffer and displaying
++ * diagnostic messages
++ * @return A buffer with a control channel message or a buffer with
++ * with length 0 if there is no message or the message has
++ * invalid characters.
++ */
++struct buffer
++extract_command_buffer(struct buffer *buf, struct gc_arena *gc);
++
+ static inline const char *
+ packet_opcode_name(int op)
+ {
+diff --git a/tests/unit_tests/openvpn/test_pkt.c b/tests/unit_tests/openvpn/test_pkt.c
+index 736f131..f3dc855 100644
+--- a/tests/unit_tests/openvpn/test_pkt.c
++++ b/tests/unit_tests/openvpn/test_pkt.c
+@@ -636,6 +636,40 @@ test_generate_reset_packet_tls_auth(void **ut_state)
+ free_tas(&tas_server);
+ }
+
++static void
++test_extract_control_message(void **ut_state)
++{
++ struct gc_arena gc = gc_new();
++ struct buffer input_buf = alloc_buf_gc(1024, &gc);
++
++ /* This message will have a \0x00 at the end since it is a C string */
++ const char input[] = "valid control message\r\n\0\0Invalid\r\none\0valid one again";
++
++ buf_write(&input_buf, input, sizeof(input));
++ struct buffer cmd1 = extract_command_buffer(&input_buf, &gc);
++ struct buffer cmd2 = extract_command_buffer(&input_buf, &gc);
++ struct buffer cmd3 = extract_command_buffer(&input_buf, &gc);
++ struct buffer cmd4 = extract_command_buffer(&input_buf, &gc);
++ struct buffer cmd5 = extract_command_buffer(&input_buf, &gc);
++
++ assert_string_equal(BSTR(&cmd1), "valid control message");
++ /* empty message with just a \0x00 */
++ assert_int_equal(cmd2.len, 1);
++ assert_string_equal(BSTR(&cmd2), "");
++ assert_int_equal(cmd3.len, 0);
++ assert_string_equal(BSTR(&cmd4), "valid one again");
++ assert_int_equal(cmd5.len, 0);
++
++ const uint8_t nonull[6] = { 'n', 'o', ' ', 'N', 'U', 'L'};
++ struct buffer nonull_buf = alloc_buf_gc(1024, &gc);
++
++ buf_write(&nonull_buf, nonull, sizeof(nonull));
++ struct buffer nonullcmd = extract_command_buffer(&nonull_buf, &gc);
++ assert_int_equal(nonullcmd.len, 0);
++
++ gc_free(&gc);
++}
++
+ int
+ main(void)
+ {
+@@ -649,6 +683,7 @@ main(void)
+ cmocka_unit_test(test_verify_hmac_tls_auth),
+ cmocka_unit_test(test_generate_reset_packet_plain),
+ cmocka_unit_test(test_generate_reset_packet_tls_auth),
++ cmocka_unit_test(test_extract_control_message)
+ };
+
+ #if defined(ENABLE_CRYPTO_OPENSSL)
diff -Nru openvpn-2.6.3/debian/patches/series openvpn-2.6.3/debian/patches/series
--- openvpn-2.6.3/debian/patches/series 2025-04-02 12:45:15.000000000 -0300
+++ openvpn-2.6.3/debian/patches/series 2025-08-24 22:36:22.000000000 -0300
@@ -1,6 +1,5 @@
move_log_dir.patch
auth-pam_libpam_so_filename.patch
-#debian_nogroup_for_sample_files.patch
openvpn-pkcs11warn.patch
systemd.patch
fix-dangling-pointer-in-pkcs11.patch
@@ -11,3 +10,4 @@
CVE-2024-5594.patch
sample-keys-renew-10-years.patch
CVE-2025-2704.patch
+CVE-2024-5594-regression-fix.patch
diff -Nru openvpn-2.6.3/debian/salsa-ci.yml openvpn-2.6.3/debian/salsa-ci.yml
--- openvpn-2.6.3/debian/salsa-ci.yml 2025-04-02 12:45:15.000000000 -0300
+++ openvpn-2.6.3/debian/salsa-ci.yml 2025-08-24 22:36:22.000000000 -0300
@@ -5,3 +5,12 @@
variables:
RELEASE: 'bookworm'
+ SALSA_CI_DISABLE_GBP_SETUP_GITATTRIBUTES: 1
+ # reprotest can only run on sid and doesn't respect $RELEASE so it's
+ # completely broken for openvpn and we gain nothing by running it
+ SALSA_CI_DISABLE_REPROTEST: 1
+
+# salsa-ci runs on lintian from unstable, it dislikes lots of things from the
+# bookworm era :-)
+lintian:
+ allow_failure: true
diff -Nru openvpn-2.6.3/debian/tests/control openvpn-2.6.3/debian/tests/control
--- openvpn-2.6.3/debian/tests/control 2025-04-02 12:45:15.000000000 -0300
+++ openvpn-2.6.3/debian/tests/control 2025-08-24 22:36:22.000000000 -0300
@@ -4,3 +4,7 @@
Tests: server-setup-with-static-key
Restrictions: needs-root, isolation-machine, allow-stderr
+
+Tests: unit-tests
+Depends: libcmocka-dev, @builddeps@
+Restrictions: allow-stderr
diff -Nru openvpn-2.6.3/debian/tests/unit-tests openvpn-2.6.3/debian/tests/unit-tests
--- openvpn-2.6.3/debian/tests/unit-tests 1969-12-31 21:00:00.000000000 -0300
+++ openvpn-2.6.3/debian/tests/unit-tests 2025-08-24 22:36:22.000000000 -0300
@@ -0,0 +1,13 @@
+#!/bin/sh
+set -ex
+
+# This test sets up and builds the openvpn, then runs the unit tests.
+
+sed -i -e 's/--disable-unit-tests/--enable-unit-tests/' debian/rules
+
+dh_update_autotools_config
+dh_autoreconf
+debian/rules override_dh_auto_configure
+debian/rules override_dh_auto_build
+
+make -C tests/unit_tests check
Reply to: