Skip to content

Commit

Permalink
Merge pull request #4 from sophos/bugfix/NCL-2203-SCC-Windows-Fix-vul…
Browse files Browse the repository at this point in the history
…nerability-in-OpenVPN

Bugfix/ncl 2203 scc windows fix vulnerability in open vpn
  • Loading branch information
rishipal-Sophos authored Jan 20, 2025
2 parents 2eb81bf + 595075a commit 1009b98
Show file tree
Hide file tree
Showing 46 changed files with 796 additions and 277 deletions.
16 changes: 13 additions & 3 deletions .github/workflows/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,16 @@ jobs:
matrix:
ssllib: [ openssl11, openssl3, libressl]
build: [ normal, asan ]
os: [macos-11, macos-12]
os: [macos-12, macos-13, macos-14]
include:
# macos14 and newer runners use ARM CPUs and homebrew uses /opt/homebrew/
# on ARM instead of /usr/local/
- os: macos-12
homebrew: /usr/local/opt
- os: macos-13
homebrew: /usr/local/opt
- os: macos-14
homebrew: /opt/homebrew/opt
- build: asan
cflags: "-fsanitize=address -fno-optimize-sibling-calls -fsanitize-address-use-after-scope -fno-omit-frame-pointer -g -O1"
ldflags: -fsanitize=address
Expand All @@ -220,8 +228,10 @@ jobs:
env:
CFLAGS: ${{ matrix.cflags }}
LDFLAGS: ${{ matrix.ldflags }}
OPENSSL_CFLAGS: "-I/usr/local/opt/${{matrix.libdir}}/include"
OPENSSL_LIBS: "-L/usr/local/opt/${{matrix.libdir}}/lib -lcrypto -lssl"
OPENSSL_CFLAGS: "-I${{matrix.homebrew}}/${{matrix.libdir}}/include"
OPENSSL_LIBS: "-L${{matrix.homebrew}}/${{matrix.libdir}}/lib -lcrypto -lssl"
LZO_CFLAGS: "-I${{matrix.homebrew}}/lzo/include"
LZO_LIBS: "-L${{matrix.homebrew}}/lzo/lib -llzo2"
UBSAN_OPTIONS: print_stacktrace=1
steps:
- name: Install dependencies
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/coverity-scan.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ on:

jobs:
latest:
# Running coverity requires the secrets.COVERITY_SCAN_TOKEN token
# which is only available on the main repository
if: github.repository_owner == 'OpenVPN'
runs-on: ubuntu-latest
steps:
- name: Check submission cache
Expand Down
47 changes: 47 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
@@ -1,6 +1,53 @@
OpenVPN ChangeLog
Copyright (C) 2002-2024 OpenVPN Inc <[email protected]>

2024.07.17 -- Version 2.6.12

Arne Schwabe (1):
Allow trailing \r and \n in control channel message

Frank Lichtenheld (1):
configure: Try to detect LZO with pkg-config

Gianmarco De Gregori (1):
Http-proxy: fix bug preventing proxy credentials caching


2024.06.20 -- Version 2.6.11

5andr0 (1):
Implement server_poll_timeout for socks

Arne Schwabe (6):
Use snprintf instead of sprintf for get_ssl_library_version
Add bracket in fingerprint message and do not warn about missing verification
Replace macos11 with macos14 in github runners
Only run coverity scan in OpenVPN/OpenVPN repository
Workaround issue in LibreSSL crashing when enumerating digests/ciphers
Properly handle null bytes and invalid characters in control messages

Franco Fichtner (1):
Allow to set ifmode for existing DCO interfaces in FreeBSD

Frank Lichtenheld (6):
samples: Update sample configurations
documentation: make section levels consistent
phase2_tcp_server: fix Coverity issue 'Dereference after null check'
script-options.rst: Update ifconfig_* variables
LZO: do not use lzoutils.h macros
Remove "experimental" denotation for --fast-io

Heiko Wundram (1):
Implement Windows CA template match for Crypto-API selector

Lev Stipakov (2):
misc.c: remove unused code
interactive.c: Improve access control for gui<->service pipe

Reynir Björnsson (1):
Only schedule_exit() once


2024.03.20 -- Version 2.6.10

Christoph Schug (1):
Expand Down
86 changes: 86 additions & 0 deletions Changes.rst
Original file line number Diff line number Diff line change
@@ -1,3 +1,89 @@
Overview of changes in 2.6.12
=============================
Bug fixes
---------
- the fix for CVE-2024-5594 (refuse control channel messages with
nonprintable characters) was too strict, breaking user configurations
with AUTH_FAIL messages having trailing CR/NL characters. This often
happens if the AUTH_FAIL reason is set by a script. Strip those before
testing the command buffer (Github: #568). Also, add unit test.

- Http-proxy: fix bug preventing proxy credentials caching (Trac: #1187)

Code maintenance
----------------
- try to detect LZO installation with pkg-config (= on many systems
manually setting LZO_CFLAGS/LZO_LIBS should no longer be necessary)

Overview of changes in 2.6.11
=============================
Security fixes
--------------
- CVE-2024-4877: Windows: harden interactive service pipe.
Security scope: a malicious process with "some" elevated privileges
(SeImpersonatePrivilege) could open the pipe a second time, tricking
openvn GUI into providing user credentials (tokens), getting full
access to the account openvpn-gui.exe runs as.
(Zeze with TeamT5)

- CVE-2024-5594: control channel: refuse control channel messages with
nonprintable characters in them. Security scope: a malicious openvpn
peer can send garbage to openvpn log, or cause high CPU load.
(Reynir Björnsson)

- CVE-2024-28882: only call schedule_exit() once (on a given peer).
Security scope: an authenticated client can make the server "keep the
session" even when the server has been told to disconnect this client
(Reynir Björnsson)

New features
------------
- Windows Crypto-API: Implement Windows CA template match for searching
certificates in windows crypto store.

- support pre-created DCO interface on FreeBSD (OpenVPN would fail to
set ifmode p2p/subnet otherwise)

Bugfixes
--------
- fix connect timeout when using SOCKS proxies (trac #328, github #267)

- work around LibreSSL crashing on OpenBSD 7.5 when enumerating ciphers
(LibreSSL bug, already fixed upstream, but not backported to OpenBSD 7.5,
see also https://github.com/libressl/openbsd/issues/150)

- Add bracket in fingerprint message and do not warn about missing
verification (github #516)

Documentation
-------------
- remove "experimental" denotation for --fast-io

- correctly document ifconfig_* variables passed to scripts (script-options.rst)

- documentation: make section levels consistent

- samples: Update sample configurations
remove compression & old cipher settings, add more informative comments

Code maintenance
----------------
- remove usage of <lzoutils.h> header & macro, discouraged by upstream

- only run coverity scans in OpenVPN/OpenVPN repository (= do not spam
owners of cloned repos with "cannot run this" messages)

- replace macOS 11 github runners with macOS 14

- remove some unused code in misc.c (leftover from commit 3a4fb1)

- phase2_tcp_server: fix Coverity issue 'Dereference after null check'
- the code itself was correct, just doing needless checks

- Use snprintf instead of sprintf for get_ssl_library_version
- the code itself was correct, but macOS clang dislikes sprintf()


Overview of changes in 2.6.10
=============================
Security fixes
Expand Down
6 changes: 0 additions & 6 deletions config.h.cmake.in
Original file line number Diff line number Diff line change
Expand Up @@ -175,15 +175,9 @@ don't. */
/* Define to 1 if you have the <linux/types.h> header file. */
#cmakedefine HAVE_LINUX_TYPES_H

/* Define to 1 if you have the <lzoconf.h> header file. */
#define HAVE_LZO_CONF_H

/* Define to 1 if you have the <lzo1x.h> header file. */
#define HAVE_LZO1X_H 1

/* Define to 1 if you have the <lzoutil.h> header file. */
#define HAVE_LZOUTIL_H 1

/* Define to 1 if you have the `mlockall' function. */
#cmakedefine HAVE_MLOCKALL

Expand Down
33 changes: 20 additions & 13 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -1152,8 +1152,17 @@ fi

AC_ARG_VAR([LZO_CFLAGS], [C compiler flags for lzo])
AC_ARG_VAR([LZO_LIBS], [linker flags for lzo])
have_lzo="yes"
if test -z "${LZO_LIBS}"; then
if test -z "${LZO_CFLAGS}" -a -z "${LZO_LIBS}"; then
# if the user did not explicitly specify flags, try to autodetect
PKG_CHECK_MODULES([LZO],
[lzo2],
[have_lzo="yes"],
[]
)

if test "${have_lzo}" != "yes"; then
# try to detect without pkg-config
have_lzo="yes"
AC_CHECK_LIB(
[lzo2],
[lzo1x_1_15_compress],
Expand All @@ -1165,27 +1174,25 @@ if test -z "${LZO_LIBS}"; then
[have_lzo="no"]
)]
)
fi
else
# assume the user configured it correctly
have_lzo="yes"
fi
if test "${have_lzo}" = "yes"; then
saved_CFLAGS="${CFLAGS}"
CFLAGS="${CFLAGS} ${LZO_CFLAGS}"
AC_CHECK_HEADERS(
[lzo/lzoutil.h],
,
[AC_CHECK_HEADERS(
[lzoutil.h],
,
[AC_MSG_ERROR([lzoutil.h is missing])]
)]
)
AC_CHECK_HEADERS(
[lzo/lzo1x.h],
,
[AC_CHECK_HEADERS(
[lzo1x.h],
,
[AC_MSG_ERROR([lzo1x.h is missing])]
)]
[AC_MSG_ERROR([lzo1x.h is missing])],
[#include <limits.h>
#include <lzodefs.h>
#include <lzoconf.h>]
)],
)
CFLAGS="${saved_CFLAGS}"
fi
Expand Down
16 changes: 8 additions & 8 deletions doc/man-sections/cipher-negotiation.rst
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
Data channel cipher negotiation
===============================
-------------------------------

OpenVPN 2.4 and higher have the capability to negotiate the data cipher that
is used to encrypt data packets. This section describes the mechanism in more detail and the
different backwards compatibility mechanism with older server and clients.

OpenVPN 2.5 and later behaviour
--------------------------------
```````````````````````````````
When both client and server are at least running OpenVPN 2.5, that the order of
the ciphers of the server's ``--data-ciphers`` is used to pick the data cipher.
That means that the first cipher in that list that is also in the client's
Expand All @@ -25,7 +25,7 @@ For backwards compatibility OpenVPN 2.6 and later with ``--compat-mode 2.4.x``
``--cipher`` option to this list.

OpenVPN 2.4 clients
-------------------
```````````````````
The negotiation support in OpenVPN 2.4 was the first iteration of the implementation
and still had some quirks. Its main goal was "upgrade to AES-256-GCM when possible".
An OpenVPN 2.4 client that is built against a crypto library that supports AES in GCM
Expand All @@ -40,7 +40,7 @@ always have the `AES-256-GCM` and `AES-128-GCM` ciphers to the ``--ncp-ciphers``
options to avoid this behaviour.

OpenVPN 3 clients
-----------------
`````````````````
Clients based on the OpenVPN 3.x library (https://github.com/openvpn/openvpn3/)
do not have a configurable ``--ncp-ciphers`` or ``--data-ciphers`` option. Newer
versions by default disable legacy AES-CBC, BF-CBC, and DES-CBC ciphers.
Expand All @@ -52,7 +52,7 @@ included in the server's ``--data-ciphers`` option.


OpenVPN 2.3 and older clients (and clients with ``--ncp-disable``)
------------------------------------------------------------------
``````````````````````````````````````````````````````````````````
When a client without cipher negotiation support connects to a server the
cipher specified with the ``--cipher`` option in the client configuration
must be included in the ``--data-ciphers`` option of the server to allow
Expand All @@ -65,7 +65,7 @@ If the client is 2.3 or older and has been configured with the
cipher used by the client is necessary.

OpenVPN 2.4 server
------------------
``````````````````
When a client indicates support for `AES-128-GCM` and `AES-256-GCM`
(with ``IV_NCP=2``) an OpenVPN 2.4 server will send the first
cipher of the ``--ncp-ciphers`` to the OpenVPN client regardless of what
Expand All @@ -76,7 +76,7 @@ option is required. OpenVPN 2.5+ will only announce the ``IV_NCP=2`` flag if
those ciphers are present.

OpenVPN 2.3 and older servers (and servers with ``--ncp-disable``)
------------------------------------------------------------------
``````````````````````````````````````````````````````````````````
The cipher used by the server must be included in ``--data-ciphers`` to
allow the client connecting to a server without cipher negotiation
support.
Expand All @@ -89,7 +89,7 @@ If the server is 2.3 or older and has been configured with the
cipher used by the server is necessary.

Blowfish in CBC mode (BF-CBC) deprecation
------------------------------------------
`````````````````````````````````````````
The ``--cipher`` option defaulted to `BF-CBC` in OpenVPN 2.4 and older
version. The default was never changed to ensure backwards compatibility.
In OpenVPN 2.5 this behaviour has now been changed so that if the ``--cipher``
Expand Down
6 changes: 3 additions & 3 deletions doc/man-sections/encryption-options.rst
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
Encryption Options
==================
------------------

SSL Library information
-----------------------
```````````````````````

--show-ciphers
(Standalone) Show all cipher algorithms to use with the ``--cipher``
Expand Down Expand Up @@ -32,7 +32,7 @@ SSL Library information
``--ecdh-curve`` and ``tls-groups`` options.

Generating key material
-----------------------
```````````````````````

--genkey args
(Standalone) Generate a key to be used of the type keytype. if keyfile
Expand Down
5 changes: 1 addition & 4 deletions doc/man-sections/generic-options.rst
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@ which mode OpenVPN is configured as.
When using ``--auth-nocache`` in combination with a user/password file
and ``--chroot`` or ``--daemon``, make sure to use an absolute path.

This directive does not affect the ``--http-proxy`` username/password.
It is always cached.

--cd dir
Change directory to ``dir`` prior to reading any files such as
configuration files, key files, scripts, etc. ``dir`` should be an
Expand Down Expand Up @@ -215,7 +212,7 @@ which mode OpenVPN is configured as.
are supported by OpenSSL.

--fast-io
(Experimental) Optimize TUN/TAP/UDP I/O writes by avoiding a call to
Optimize TUN/TAP/UDP I/O writes by avoiding a call to
poll/epoll/select prior to the write operation. The purpose of such a
call would normally be to block until the device or socket is ready to
accept the write. Such blocking is unnecessary on some platforms which
Expand Down
2 changes: 1 addition & 1 deletion doc/man-sections/pkcs11-options.rst
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
PKCS#11 / SmartCard options
---------------------------
```````````````````````````

--pkcs11-cert-private args
Set if access to certificate object should be performed after login.
Expand Down
2 changes: 1 addition & 1 deletion doc/man-sections/renegotiation.rst
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Data Channel Renegotiation
--------------------------
``````````````````````````

When running OpenVPN in client/server mode, the data channel will use a
separate ephemeral encryption key which is rotated at regular intervals.
Expand Down
Loading

0 comments on commit 1009b98

Please sign in to comment.