Re: [Openvpn-devel] [PATCH] dco-win: use proper calling convention on x86

2023-01-31 Thread Timo Rothenpieler 

On 31/01/2023 13:25, Lev Stipakov wrote:

From: Lev Stipakov 

WinAPI uses __stdcall calling convention on x86. Wrong
calling convention causes UB, which in this case breaks
dco-win functionality.

Signed-off-by: Lev Stipakov 
---
  src/openvpn/dco_win.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c
index 7594024c..da1e1fbc 100644
--- a/src/openvpn/dco_win.c
+++ b/src/openvpn/dco_win.c
@@ -110,7 +110,7 @@ dco_connect_wait(HANDLE handle, OVERLAPPED *ov, int 
timeout, struct signal_info
  {
  volatile int *signal_received = _info->signal_received;
  /* GetOverlappedResultEx is available starting from Windows 8 */
-typedef BOOL (*get_overlapped_result_ex_t) (HANDLE, LPOVERLAPPED, LPDWORD, 
DWORD, BOOL);
+typedef BOOL (__stdcall *get_overlapped_result_ex_t) (HANDLE, 
LPOVERLAPPED, LPDWORD, DWORD, BOOL);


Wouldn't it be better to use the WINAPI Macro here, so it's the right 
calling convention no matter the arch?


Not sure what header it's defined in, but a quick search suggests it 
comes from minwindef.h.



  get_overlapped_result_ex_t get_overlapped_result_ex =
  
(get_overlapped_result_ex_t)GetProcAddress(GetModuleHandle("Kernel32.dll"),
 "GetOverlappedResultEx");



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] Don't clear capability bounding set on capng_change_id

2023-01-18 Thread Timo Rothenpieler 
The bounding set being empty will overpower the likes of su/sudo
and will make it impossible for any child processes to ever gain
additional privileges again.

This fixes https://github.com/OpenVPN/openvpn/issues/220

Signed-off-by: Timo Rothenpieler 
---
 src/openvpn/platform.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/openvpn/platform.c b/src/openvpn/platform.c
index 1b5fa9ad..580c4cb8 100644
--- a/src/openvpn/platform.c
+++ b/src/openvpn/platform.c
@@ -246,7 +246,7 @@ platform_user_group_set(const struct platform_state_user 
*user_state,
 /* Change to new UID/GID.
  * capng_change_id() internally calls capng_apply() to apply prepared 
capabilities.
  */
-res = capng_change_id(new_uid, new_gid, CAPNG_DROP_SUPP_GRP | 
CAPNG_CLEAR_BOUNDING);
+res = capng_change_id(new_uid, new_gid, CAPNG_DROP_SUPP_GRP);
 if (res == -4 || res == -6)
 {
 /* -4 and -6 mean failure of setuid/gid respectively.
-- 
2.34.1



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v2] GitHub Actions: Use Docker Images for Ubuntu test runs

2022-08-23 Thread Timo Rothenpieler 

On 23/08/2022 15:42, Timo Rothenpieler wrote:

+run: docker exec ovpn -e DEBIAN_FRONTEND=noninteractive sh -c "apt update 
&& apt install -y liblzo2-dev libpam0g-dev liblz4-dev libcap-ng-dev linux-libc-dev 
man2html libcmocka-dev python3-docutils build-essential pkgconf libtool automake autoconf 
iproute2 git ${SSLPKG} ${NLPKG}"


Still not 100% right unfortunately... the -e DEB... part needs to go 
before the image name, and after exec.



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v2] GitHub Actions: Use Docker Images for Ubuntu test runs

2022-08-23 Thread Timo Rothenpieler 
The ubuntu-18.04 base image is deprecated and in the process of being phased 
out:
https://github.blog/changelog/2022-08-09-github-actions-the-ubuntu-18-04-actions-runner-image-is-being-deprecated-and-will-be-removed-by-12-1-22/

It is already causing build failures during the scheduled periods mentioned in 
the blog post.

This is the best alternative I found which does not drop any tests or test 
environments.
Sadly GHAs build-in container-capabilities can't be used, since they don't 
allow specifying --network host.
Without that, IPv6 related tests fail, since the Docker deployment on GHA does 
not support IPv6, even in 2022:
https://github.com/actions/runner-images/issues/668

So instead this manually invokes docker to run the tests in the respective 
Ubuntu Docker-Image.
Some dependencies had to be added since the Docker-Images are more barebones 
than the Github-Runner images.
Otherwise, nothing about the test setup changed.
---
Was missing DEBIAN_FRONTEND=noninteractive


 .github/workflows/build.yaml | 43 
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml
index b0f67a78..53662120 100644
--- a/.github/workflows/build.yaml
+++ b/.github/workflows/build.yaml
@@ -162,79 +162,84 @@ jobs:
 strategy:
   fail-fast: false
   matrix:
-os: [ubuntu-18.04, ubuntu-20.04, ubuntu-22.04]
+img: ["ubuntu:18.04", "ubuntu:20.04", "ubuntu:22.04"]
 sslpkg: [libmbedtls-dev]
 ssllib: [mbedtls]
 libname: [mbed TLS]
 
 include:
-  - os: ubuntu-18.04
+  - img: "ubuntu:18.04"
 sslpkg: "libssl1.0-dev"
 ssllib: openssl
 libname: OpenSSL 1.0.2
-  - os: ubuntu-18.04
+  - img: "ubuntu:18.04"
 sslpkg: "libssl-dev"
 libname: OpenSSL 1.1.1
 ssllib: openssl
-  - os: ubuntu-20.04
+  - img: "ubuntu:20.04"
 sslpkg: "libssl-dev"
 libname: OpenSSL 1.1.1
 ssllib: openssl
-  - os: ubuntu-22.04
+  - img: "ubuntu:22.04"
 sslpkg: "libssl-dev"
 libname: OpenSSL 3.0.2
 ssllib: openssl
-  - os: ubuntu-20.04
+  - img: "ubuntu:20.04"
 sslpkg: "libssl-dev"
 libname: OpenSSL 1.1.1
 ssllib: openssl
 extraconf: "--enable-iproute2"
-  - os: ubuntu-20.04
+  - img: "ubuntu:20.04"
 sslpkg: "libssl-dev"
 libname: OpenSSL 1.1.1
 ssllib: openssl
 extraconf: "--enable-async-push"
-  - os: ubuntu-20.04
+  - img: "ubuntu:20.04"
 sslpkg: "libssl-dev"
 libname: OpenSSL 1.1.1
 ssllib: openssl
 extraconf: "--disable-management"
-  - os: ubuntu-20.04
+  - img: "ubuntu:20.04"
 sslpkg: "libssl-dev"
 libname: OpenSSL 1.1.1
 ssllib: openssl
 extraconf: "--enable-small"
-  - os: ubuntu-20.04
+  - img: "ubuntu:20.04"
 sslpkg: "libssl-dev"
 libname: OpenSSL 1.1.1
 ssllib: openssl
 extraconf: "--disable-lzo --disable-lz4"
-  - os: ubuntu-20.04
+  - img: "ubuntu:20.04"
 sslpkg: "libssl-dev"
 libname: OpenSSL 1.1.1
 ssllib: openssl
 extraconf: "--enable-dco"
 nlpkg: "libnl-genl-3-dev"
 
-name: "gcc - ${{matrix.os}} - ${{matrix.libname}} ${{matrix.extraconf}}"
+name: "gcc - ${{matrix.img}} - ${{matrix.libname}} ${{matrix.extraconf}}"
 env:
   SSLPKG: "${{matrix.sslpkg}}"
   NLPKG: "${{matrix.nlpkg}}"
 
-runs-on: ${{matrix.os}}
+runs-on: ubuntu-latest
 steps:
-  - name: Install dependencies
-run: sudo apt update && sudo apt install -y liblzo2-dev libpam0g-dev 
liblz4-dev libcap-ng-dev linux-libc-dev man2html libcmocka-dev python3-docutils 
libtool automake autoconf ${SSLPKG} ${NLPKG}
   - name: Checkout OpenVPN
 uses: actions/checkout@v3
+  - name: Setup Container
+run: docker run --name ovpn --detach --rm --cap-add NET_ADMIN 
--network host -v "$PWD:/wd" --workdir=/wd "${{matrix.img}}" sleep 3600
+  - name: Install dependencies
+run: docker exec ovpn -e DEBIAN_FRONTEND=noninteractive sh -c "apt 
update && apt install -y liblzo2-dev libpam0g-dev liblz4-dev libcap-ng-dev 
linux-libc-dev man2html libcmocka-dev python3-docutils build-essential pkgconf 
libtool automake autoconf iproute2 git ${SSLPKG} ${NLPKG}"
   - name: autoconf
-run: autoreconf -fvi
+run: docker exec ovpn autoreconf -fvi
   - name: configure
-run: ./configure --with-crypto-library=${{matrix.ssllib}} 
${{matrix.extraconf}} --enable-werror
+run: docker exec ovpn ./configure 
--with-crypto-library=${{matrix.ssllib}} ${{matrix.extraconf}}

[Openvpn-devel] [PATCH] GitHub Actions: Use Docker Images for Ubuntu test runs

2022-08-23 Thread Timo Rothenpieler 
The ubuntu-18.04 base image is deprecated and in the process of being phased 
out:
https://github.blog/changelog/2022-08-09-github-actions-the-ubuntu-18-04-actions-runner-image-is-being-deprecated-and-will-be-removed-by-12-1-22/

It is already causing build failures during the scheduled periods mentioned in 
the blog post.

This is the best alternative I found which does not drop any tests or test 
environments.
Sadly GHAs build-in container-capabilities can't be used, since they don't 
allow specifying --network host.
Without that, IPv6 related tests fail, since the Docker deployment on GHA does 
not support IPv6, even in 2022:
https://github.com/actions/runner-images/issues/668

So instead this manually invokes docker to run the tests in the respective 
Ubuntu Docker-Image.
Some dependencies had to be added since the Docker-Images are more barebones 
than the Github-Runner images.
Otherwise, nothing about the test setup changed.
---
 .github/workflows/build.yaml | 43 
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml
index b0f67a78..b6b04ff2 100644
--- a/.github/workflows/build.yaml
+++ b/.github/workflows/build.yaml
@@ -162,79 +162,84 @@ jobs:
 strategy:
   fail-fast: false
   matrix:
-os: [ubuntu-18.04, ubuntu-20.04, ubuntu-22.04]
+img: ["ubuntu:18.04", "ubuntu:20.04", "ubuntu:22.04"]
 sslpkg: [libmbedtls-dev]
 ssllib: [mbedtls]
 libname: [mbed TLS]
 
 include:
-  - os: ubuntu-18.04
+  - img: "ubuntu:18.04"
 sslpkg: "libssl1.0-dev"
 ssllib: openssl
 libname: OpenSSL 1.0.2
-  - os: ubuntu-18.04
+  - img: "ubuntu:18.04"
 sslpkg: "libssl-dev"
 libname: OpenSSL 1.1.1
 ssllib: openssl
-  - os: ubuntu-20.04
+  - img: "ubuntu:20.04"
 sslpkg: "libssl-dev"
 libname: OpenSSL 1.1.1
 ssllib: openssl
-  - os: ubuntu-22.04
+  - img: "ubuntu:22.04"
 sslpkg: "libssl-dev"
 libname: OpenSSL 3.0.2
 ssllib: openssl
-  - os: ubuntu-20.04
+  - img: "ubuntu:20.04"
 sslpkg: "libssl-dev"
 libname: OpenSSL 1.1.1
 ssllib: openssl
 extraconf: "--enable-iproute2"
-  - os: ubuntu-20.04
+  - img: "ubuntu:20.04"
 sslpkg: "libssl-dev"
 libname: OpenSSL 1.1.1
 ssllib: openssl
 extraconf: "--enable-async-push"
-  - os: ubuntu-20.04
+  - img: "ubuntu:20.04"
 sslpkg: "libssl-dev"
 libname: OpenSSL 1.1.1
 ssllib: openssl
 extraconf: "--disable-management"
-  - os: ubuntu-20.04
+  - img: "ubuntu:20.04"
 sslpkg: "libssl-dev"
 libname: OpenSSL 1.1.1
 ssllib: openssl
 extraconf: "--enable-small"
-  - os: ubuntu-20.04
+  - img: "ubuntu:20.04"
 sslpkg: "libssl-dev"
 libname: OpenSSL 1.1.1
 ssllib: openssl
 extraconf: "--disable-lzo --disable-lz4"
-  - os: ubuntu-20.04
+  - img: "ubuntu:20.04"
 sslpkg: "libssl-dev"
 libname: OpenSSL 1.1.1
 ssllib: openssl
 extraconf: "--enable-dco"
 nlpkg: "libnl-genl-3-dev"
 
-name: "gcc - ${{matrix.os}} - ${{matrix.libname}} ${{matrix.extraconf}}"
+name: "gcc - ${{matrix.img}} - ${{matrix.libname}} ${{matrix.extraconf}}"
 env:
   SSLPKG: "${{matrix.sslpkg}}"
   NLPKG: "${{matrix.nlpkg}}"
 
-runs-on: ${{matrix.os}}
+runs-on: ubuntu-latest
 steps:
-  - name: Install dependencies
-run: sudo apt update && sudo apt install -y liblzo2-dev libpam0g-dev 
liblz4-dev libcap-ng-dev linux-libc-dev man2html libcmocka-dev python3-docutils 
libtool automake autoconf ${SSLPKG} ${NLPKG}
   - name: Checkout OpenVPN
 uses: actions/checkout@v3
+  - name: Setup Container
+run: docker run --name ovpn --detach --rm --cap-add NET_ADMIN 
--network host -v "$PWD:/wd" --workdir=/wd "${{matrix.img}}" sleep 3600
+  - name: Install dependencies
+run: docker exec ovpn sh -c "apt update && apt install -y liblzo2-dev 
libpam0g-dev liblz4-dev libcap-ng-dev linux-libc-dev man2html libcmocka-dev 
python3-docutils build-essential pkgconf libtool automake autoconf iproute2 git 
${SSLPKG} ${NLPKG}"
   - name: autoconf
-run: autoreconf -fvi
+run: docker exec ovpn autoreconf -fvi
   - name: configure
-run: ./configure --with-crypto-library=${{matrix.ssllib}} 
${{matrix.extraconf}} --enable-werror
+run: docker exec ovpn ./configure 
--with-crypto-library=${{matrix.ssllib}} ${{matrix.extraconf}} --enable-werror
   - name: make all
-run: make -j3
+run:

[Openvpn-devel] [PATCH] dco: turn platform config checks into separate function

2022-08-17 Thread Timo Rothenpieler 
All the checks in there are only relevant during startup, and
specifically the capability check might cause issues when checking a CCD
config later at runtime.

So move them to their own function and call it only during startup.
---
 src/openvpn/dco.c |  9 ++---
 src/openvpn/dco.h | 18 ++
 src/openvpn/options.c |  3 ++-
 3 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index f21997de..9eb2685c 100644
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -222,8 +222,8 @@ dco_update_keys(dco_context_t *dco, struct tls_multi *multi)
 }
 }
 
-static bool
-dco_check_option_conflict_platform(int msglevel, const struct options *o)
+bool
+dco_check_startup_option_conflict(int msglevel, const struct options *o)
 {
 #if defined(TARGET_LINUX)
 /* if the device name is fixed, we need to check if an interface with this
@@ -327,11 +327,6 @@ dco_check_option_conflict(int msglevel, const struct 
options *o)
 return false;
 }
 
-if (!dco_check_option_conflict_platform(msglevel, o))
-{
-return false;
-}
-
 if (dev_type_enum(o->dev, o->dev_type) != DEV_TYPE_TUN)
 {
 msg(msglevel, "Note: dev-type not tun, disabling data channel 
offload.");
diff --git a/src/openvpn/dco.h b/src/openvpn/dco.h
index 6b5c016a..e296cf27 100644
--- a/src/openvpn/dco.h
+++ b/src/openvpn/dco.h
@@ -69,6 +69,18 @@ bool dco_available(int msglevel);
  */
 bool dco_check_option_conflict(int msglevel, const struct options *o);
 
+/**
+ * Check whether the options struct has any further option that is not 
supported
+ * by our current dco implementation during early startup.
+ * If so print a warning at warning level for the first conflicting option
+ * found and return false.
+ *
+ * @param msglevel  the msg level to use to print the warnings
+ * @param o the options struct that hold the options
+ * @return  true if no conflict was detected, false otherwise
+ */
+bool dco_check_startup_option_conflict(int msglevel, const struct options *o);
+
 /**
  * Check whether any of the options pushed by the server is not supported by
  * our current dco implementation. If so print a warning at warning level
@@ -236,6 +248,12 @@ dco_check_option_conflict(int msglevel, const struct 
options *o)
 return false;
 }
 
+static inline bool
+dco_check_startup_option_conflict(int msglevel, const struct options *o)
+{
+return false;
+}
+
 static inline bool
 dco_check_pull_options(int msglevel, const struct options *o)
 {
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index bd6db826..2415c1a8 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -3671,7 +3671,8 @@ options_postprocess_mutate(struct options *o, struct 
env_set *es)
 
 /* check if any option should force disabling DCO */
 #if defined(TARGET_LINUX) || defined(TARGET_FREEBSD)
-o->tuntap_options.disable_dco = !dco_check_option_conflict(D_DCO, o);
+o->tuntap_options.disable_dco = !dco_check_option_conflict(D_DCO, o)
+|| 
!dco_check_startup_option_conflict(D_DCO, o);
 #endif
 
 if (dco_enabled(o) && o->dev_node)
-- 
2.34.1



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v2] Github Actions: update used actions

2022-08-17 Thread Timo Rothenpieler 
In the case of the Microsoft action, this fixes security relevant issues 
according to their release notes:
https://github.com/microsoft/setup-msbuild/releases

Unfortunately they don't appear to be following the usual scheme of v1 
referring to all v1.x.x, but instead v1 just points to v1.0.0.

The primary change with all the Github-Provided actions is the switch to a more 
up-to-date NodeJS version (16). Not all that relevant when you just use the 
action as is, but on top of that, the old versions are in low-maintenance mode, 
and basically are considered obsolete.
Github is actively migrating people to the latest ones via dependabot wherever 
they can.
---
 .github/workflows/build.yaml | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml
index c89d3c8c..49b7d6d1 100644
--- a/.github/workflows/build.yaml
+++ b/.github/workflows/build.yaml
@@ -13,7 +13,7 @@ jobs:
   - name: Install dependencies
 run: sudo apt update && sudo apt install -y uncrustify
   - name: Checkout OpenVPN
-uses: actions/checkout@v2
+uses: actions/checkout@v3
 with:
   path: openvpn
   - name: Show uncrustify version
@@ -27,7 +27,7 @@ jobs:
   - name: Show changes on standard output
 run: git diff
 working-directory: openvpn
-  - uses: actions/upload-artifact@v2
+  - uses: actions/upload-artifact@v3
 with:
   name: uncrustify-changes.patch
   path: 'openvpn/uncrustify-changes.patch'
@@ -60,12 +60,12 @@ jobs:
   - name: Install dependencies
 run: sudo apt update && sudo apt install -y mingw-w64 libtool automake 
autoconf man2html unzip
   - name: Checkout ovpn-dco-win
-uses: actions/checkout@v2
+uses: actions/checkout@v3
 with:
   repository: OpenVPN/ovpn-dco-win
   path: ovpn-dco-win
   - name: Checkout OpenVPN
-uses: actions/checkout@v2
+uses: actions/checkout@v3
 with:
   path: openvpn
 
@@ -75,7 +75,7 @@ jobs:
 
   - name: Cache dependencies
 id: cache
-uses: actions/cache@v2
+uses: actions/cache@v3
 with:
   path: '~/mingw/'
   key: ${{ matrix.target }}-mingw-${{ env.OPENSSL_VERSION }}-${{ 
env.LZO_VERSION }}-${{ env.PKCS11_HELPER_VERSION }}-${{ env.TAP_WINDOWS_VERSION 
}}
@@ -226,7 +226,7 @@ jobs:
   - name: Install dependencies
 run: sudo apt update && sudo apt install -y liblzo2-dev libpam0g-dev 
liblz4-dev libcap-ng-dev linux-libc-dev man2html libcmocka-dev python3-docutils 
libtool automake autoconf ${SSLPKG} ${NLPKG}
   - name: Checkout OpenVPN
-uses: actions/checkout@v2
+uses: actions/checkout@v3
   - name: autoconf
 run: autoreconf -fvi
   - name: configure
@@ -250,7 +250,7 @@ jobs:
   - name: Install dependencies
 run: sudo apt update && sudo apt install -y liblzo2-dev libpam0g-dev 
liblz4-dev libcap-ng-dev linux-libc-dev man2html clang libcmocka-dev 
python3-docutils libtool automake autoconf libmbedtls-dev
   - name: Checkout OpenVPN
-uses: actions/checkout@v2
+uses: actions/checkout@v3
   - name: autoconf
 run: autoreconf -fvi
   - name: configure
@@ -288,7 +288,7 @@ jobs:
   - name: Install dependencies
 run: brew install openssl@1.1 openssl@3 lzo lz4 man2html cmocka 
libtool automake autoconf
   - name: Checkout OpenVPN
-uses: actions/checkout@v2
+uses: actions/checkout@v3
   - name: autoconf
 run: autoreconf -fvi
   - name: configure
@@ -319,13 +319,13 @@ jobs:
 
   runs-on: windows-latest
   steps:
-  - uses: actions/checkout@v2
+  - uses: actions/checkout@v3
 
   - name: Add MSBuild to PATH
-uses: microsoft/setup-msbuild@v1
+uses: microsoft/setup-msbuild@v1.1
 
   - name: Set up Python
-uses: actions/setup-python@v2
+uses: actions/setup-python@v4
 with:
   python-version: '3.x'
 
@@ -345,7 +345,7 @@ jobs:
 msbuild /m /p:Configuration=${{env.BUILD_CONFIGURATION}} 
/p:Platform="${{ matrix.plat }}" .
 
   - name: Archive artifacts
-uses: actions/upload-artifact@v2
+uses: actions/upload-artifact@v3
 with:
   name: artifacts-${{ matrix.plat }}
   path: |
-- 
2.34.1



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Github Actions: update used actions

2022-08-17 Thread Timo Rothenpieler 

On 17/08/2022 15:12, Arne Schwabe wrote:

Hey,

newest and greatest is always nice but could you give a bit more 
rationale why we should update? The commit message is unfortunately not 
giving any details.


In the case of the Microsoft action, it actually fixes security relevant 
issues according to their release notes:


https://github.com/microsoft/setup-msbuild/releases

Unfortunately they don't appear to be following the usual scheme of v1 
referring to all v1.x.x, but instead v1 just points to v1.0.0.



The primary change with all the Github-Provided actions is the switch to 
a more up-to-date NodeJS version (16). Not all that relevant when you 
just use the action as is, but on top of that, the old versions are in 
low-maintenance mode, and basically are considered obsolete.
Github is actively migrating people to the latest ones via dependabot 
wherever they can.





___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] dco: disable DCO if --user specified but unable to retain capabilities

2022-08-17 Thread Timo Rothenpieler 
---
 src/openvpn/dco.c | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index caa4ce32..b7db23f4 100644
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -44,6 +44,10 @@
 #include "ssl_ncp.h"
 #include "tun.h"
 
+#ifdef HAVE_LIBCAPNG
+#include 
+#endif
+
 static int
 dco_install_key(struct tls_multi *multi, struct key_state *ks,
 const uint8_t *encrypt_key, const uint8_t *encrypt_iv,
@@ -247,6 +251,28 @@ dco_check_option_conflict_platform(int msglevel, const 
struct options *o)
 }
 }
 #endif /* if defined(TARGET_LINUX) */
+
+#if defined(HAVE_LIBCAPNG)
+/* DCO can't operate without CAP_NET_ADMIN. To retain it when switching 
user
+ * we need CAP_SETPCAP. CAP_NET_ADMIN also needs to be part of the 
permitted set
+ * of capabilities in order to retain it.
+ */
+if (o->username)
+{
+if (!capng_have_capability(CAPNG_EFFECTIVE, CAP_SETPCAP))
+{
+msg(msglevel, "--user specified but lacking CAP_SETPCAP. "
+"Cannot retain CAP_NET_ADMIN. Disabling data channel offload");
+return false;
+}
+if (!capng_have_capability(CAPNG_PERMITTED, CAP_NET_ADMIN))
+{
+msg(msglevel, "--user specified but not permitted to retain 
CAP_NET_ADMIN. "
+"Disabling data channel offload");
+return false;
+}
+}
+#endif /* if defined(HAVE_LIBCAPNG) */
 return true;
 }
 
-- 
2.34.1



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] Github Actions: update used actions

2022-08-17 Thread Timo Rothenpieler 
---
 .github/workflows/build.yaml | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml
index c89d3c8c..49b7d6d1 100644
--- a/.github/workflows/build.yaml
+++ b/.github/workflows/build.yaml
@@ -13,7 +13,7 @@ jobs:
   - name: Install dependencies
 run: sudo apt update && sudo apt install -y uncrustify
   - name: Checkout OpenVPN
-uses: actions/checkout@v2
+uses: actions/checkout@v3
 with:
   path: openvpn
   - name: Show uncrustify version
@@ -27,7 +27,7 @@ jobs:
   - name: Show changes on standard output
 run: git diff
 working-directory: openvpn
-  - uses: actions/upload-artifact@v2
+  - uses: actions/upload-artifact@v3
 with:
   name: uncrustify-changes.patch
   path: 'openvpn/uncrustify-changes.patch'
@@ -60,12 +60,12 @@ jobs:
   - name: Install dependencies
 run: sudo apt update && sudo apt install -y mingw-w64 libtool automake 
autoconf man2html unzip
   - name: Checkout ovpn-dco-win
-uses: actions/checkout@v2
+uses: actions/checkout@v3
 with:
   repository: OpenVPN/ovpn-dco-win
   path: ovpn-dco-win
   - name: Checkout OpenVPN
-uses: actions/checkout@v2
+uses: actions/checkout@v3
 with:
   path: openvpn
 
@@ -75,7 +75,7 @@ jobs:
 
   - name: Cache dependencies
 id: cache
-uses: actions/cache@v2
+uses: actions/cache@v3
 with:
   path: '~/mingw/'
   key: ${{ matrix.target }}-mingw-${{ env.OPENSSL_VERSION }}-${{ 
env.LZO_VERSION }}-${{ env.PKCS11_HELPER_VERSION }}-${{ env.TAP_WINDOWS_VERSION 
}}
@@ -226,7 +226,7 @@ jobs:
   - name: Install dependencies
 run: sudo apt update && sudo apt install -y liblzo2-dev libpam0g-dev 
liblz4-dev libcap-ng-dev linux-libc-dev man2html libcmocka-dev python3-docutils 
libtool automake autoconf ${SSLPKG} ${NLPKG}
   - name: Checkout OpenVPN
-uses: actions/checkout@v2
+uses: actions/checkout@v3
   - name: autoconf
 run: autoreconf -fvi
   - name: configure
@@ -250,7 +250,7 @@ jobs:
   - name: Install dependencies
 run: sudo apt update && sudo apt install -y liblzo2-dev libpam0g-dev 
liblz4-dev libcap-ng-dev linux-libc-dev man2html clang libcmocka-dev 
python3-docutils libtool automake autoconf libmbedtls-dev
   - name: Checkout OpenVPN
-uses: actions/checkout@v2
+uses: actions/checkout@v3
   - name: autoconf
 run: autoreconf -fvi
   - name: configure
@@ -288,7 +288,7 @@ jobs:
   - name: Install dependencies
 run: brew install openssl@1.1 openssl@3 lzo lz4 man2html cmocka 
libtool automake autoconf
   - name: Checkout OpenVPN
-uses: actions/checkout@v2
+uses: actions/checkout@v3
   - name: autoconf
 run: autoreconf -fvi
   - name: configure
@@ -319,13 +319,13 @@ jobs:
 
   runs-on: windows-latest
   steps:
-  - uses: actions/checkout@v2
+  - uses: actions/checkout@v3
 
   - name: Add MSBuild to PATH
-uses: microsoft/setup-msbuild@v1
+uses: microsoft/setup-msbuild@v1.1
 
   - name: Set up Python
-uses: actions/setup-python@v2
+uses: actions/setup-python@v4
 with:
   python-version: '3.x'
 
@@ -345,7 +345,7 @@ jobs:
 msbuild /m /p:Configuration=${{env.BUILD_CONFIGURATION}} 
/p:Platform="${{ matrix.plat }}" .
 
   - name: Archive artifacts
-uses: actions/upload-artifact@v2
+uses: actions/upload-artifact@v3
 with:
   name: artifacts-${{ matrix.plat }}
   path: |
-- 
2.34.1



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH applied] Re: platform: Retain CAP_NET_ADMIN when dropping privileges

2022-08-15 Thread Timo Rothenpieler 




On 15/08/2022 12:29, Gert Doering wrote:

Hi,

On Mon, Aug 15, 2022 at 12:14:23PM +0200, Timo Rothenpieler wrote:

Unfortunately, it seems that our approach to "if SITNL is used, we hard
require that setting CAP_NET_ADMIN succeeds" is too strong for the twisted
ways that people use openvpn.


That's not how the patch operates.
It only hard-requires the capability retention is dco_enabled() returns
true.
In all other cases, it will try to retain capabilities, but continue
with a warning if it fails.


Yes, but we do have DCO here, setting CAP_NET_ADMIN fails, and we abort,
instead of having a working client connect.


Making the dco_enabled() case a "try but continue" would be a matter of
changing a 1 to a -1. But given that DCO can't really work then, I'm not
sure if that's desirable.


*DCO* can work fine, from the looks of that bug report (because all
the DCO open happens before capability drop on a --client config).

"Calling ifconfig and installing routes" cannot, but this is exactly what
we are not doing if --ifconfig-noexec + --route-noexec are set.

Please look closely at the log in the bug report, and keep in mind
that NM will do all the "SITNL" stuff for the user in that scenaro,
not OpenVPN itself.

gert


There's basically two options then:

Remove the dco_enabled() check entirely, and purely check for SITNL, but 
always only warn if it fails and carry on, hoping it works fine.


Add checks for ifconfig-noexec + route-noexec being set, and either only 
warn in that case, or don't even try to retain capabilities, since 
they're not needed either way. I'd prefer the later, since fewer 
capabilities is generally better.


The later of the two options seems nicer, will have a look at 
implementing them. Shouldn't be that hard, given the config is already 
there.

I don't have a NM setup to test that on though.


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH applied] Re: platform: Retain CAP_NET_ADMIN when dropping privileges

2022-08-15 Thread Timo Rothenpieler 

On 15/08/2022 11:54, Gert Doering wrote:

HI,

On Thu, Aug 11, 2022 at 12:03:45PM +0200, Gert Doering wrote:

I have not tested this myself, but if I had, the test setup would have
been very similar to what Frank did (so, big thanks) - run a DCO
environment with "owner nobody", and see if things still work.

I will add this to my DCO server test environment - run one of the
iroute-using instances with "nobody", so it is continuously tested.

[..]

commit 2e359a088226ab1e5ee41fbab27d38d8a8d192ac
Author: Timo Rothenpieler
Date:   Sat May 14 12:37:17 2022 +0200

  platform: Retain CAP_NET_ADMIN when dropping privileges


Unfortunately, it seems that our approach to "if SITNL is used, we hard
require that setting CAP_NET_ADMIN succeeds" is too strong for the twisted
ways that people use openvpn.


That's not how the patch operates.
It only hard-requires the capability retention is dco_enabled() returns 
true.
In all other cases, it will try to retain capabilities, but continue 
with a warning if it fails.


Making the dco_enabled() case a "try but continue" would be a matter of 
changing a 1 to a -1. But given that DCO can't really work then, I'm not 
sure if that's desirable.



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] GitHub Actions: Add new libcap-ng-dev dependency

2022-08-11 Thread Timo Rothenpieler 
Linux builds need this now in order to retain capabilities when dropping root 
privileges.
---
 .github/workflows/build.yaml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml
index b905c0d2..c89d3c8c 100644
--- a/.github/workflows/build.yaml
+++ b/.github/workflows/build.yaml
@@ -224,7 +224,7 @@ jobs:
 runs-on: ${{matrix.os}}
 steps:
   - name: Install dependencies
-run: sudo apt update && sudo apt install -y liblzo2-dev libpam0g-dev 
liblz4-dev linux-libc-dev man2html libcmocka-dev python3-docutils libtool 
automake autoconf ${SSLPKG} ${NLPKG}
+run: sudo apt update && sudo apt install -y liblzo2-dev libpam0g-dev 
liblz4-dev libcap-ng-dev linux-libc-dev man2html libcmocka-dev python3-docutils 
libtool automake autoconf ${SSLPKG} ${NLPKG}
   - name: Checkout OpenVPN
 uses: actions/checkout@v2
   - name: autoconf
@@ -248,7 +248,7 @@ jobs:
 runs-on: ${{matrix.os}}
 steps:
   - name: Install dependencies
-run: sudo apt update && sudo apt install -y liblzo2-dev libpam0g-dev 
liblz4-dev linux-libc-dev man2html clang libcmocka-dev python3-docutils libtool 
automake autoconf libmbedtls-dev
+run: sudo apt update && sudo apt install -y liblzo2-dev libpam0g-dev 
liblz4-dev libcap-ng-dev linux-libc-dev man2html clang libcmocka-dev 
python3-docutils libtool automake autoconf libmbedtls-dev
   - name: Checkout OpenVPN
 uses: actions/checkout@v2
   - name: autoconf
-- 
2.34.1



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v5] platform: Retain CAP_NET_ADMIN when dropping privileges

2022-08-10 Thread Timo Rothenpieler 

On 14/05/2022 12:37, Timo Rothenpieler wrote:

On Linux, when dropping privileges, interaction with
the network configuration, such as tearing down routes
or ovpn-dco interfaces will fail when --user/--group are
used.

This patch sets the CAP_NET_ADMIN capability, which grants
the needed privileges during the lifetime of the OpenVPN
process when dropping root privileges.

Signed-off-by: Timo Rothenpieler 
Reviewed-By: David Sommerseth 


With Linux DCO now fully merged, this patch is ready to land.
It still applies cleanly for all I can tell.


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v5] platform: Retain CAP_NET_ADMIN when dropping privileges

2022-05-14 Thread Timo Rothenpieler 
On Linux, when dropping privileges, interaction with
the network configuration, such as tearing down routes
or ovpn-dco interfaces will fail when --user/--group are
used.

This patch sets the CAP_NET_ADMIN capability, which grants
the needed privileges during the lifetime of the OpenVPN
process when dropping root privileges.

Signed-off-by: Timo Rothenpieler 
Reviewed-By: David Sommerseth 
---
 configure.ac  |  19 +++
 distro/systemd/openvpn-cli...@.service.in |   2 +-
 distro/systemd/openvpn-ser...@.service.in |   2 +-
 src/openvpn/init.c|   5 +-
 src/openvpn/platform.c| 146 +-
 src/openvpn/platform.h|  10 +-
 6 files changed, 175 insertions(+), 9 deletions(-)

diff --git a/configure.ac b/configure.ac
index 85921ddb..d2eb3426 100644
--- a/configure.ac
+++ b/configure.ac
@@ -794,6 +794,25 @@ dnl
esac
 fi
 
+dnl
+dnl Depend on libcap-ng on Linux
+dnl
+case "$host" in
+   *-*-linux*)
+   PKG_CHECK_MODULES([LIBCAPNG],
+ [libcap-ng],
+ [],
+ [AC_MSG_ERROR([libcap-ng package not found. 
Is the development package and pkg-config installed?])]
+   )
+   AC_CHECK_HEADER([sys/prctl.h],,[AC_MSG_ERROR([sys/prctl.h not 
found!])])
+
+   CFLAGS="${CFLAGS} ${LIBCAPNG_CFALGS}"
+   LIBS="${LIBS} ${LIBCAPNG_LIBS}"
+   AC_DEFINE(HAVE_LIBCAPNG, 1, [Enable libcap-ng support])
+   ;;
+esac
+
+
 if test "${with_crypto_library}" = "openssl"; then
AC_ARG_VAR([OPENSSL_CFLAGS], [C compiler flags for OpenSSL])
AC_ARG_VAR([OPENSSL_LIBS], [linker flags for OpenSSL])
diff --git a/distro/systemd/openvpn-cli...@.service.in 
b/distro/systemd/openvpn-cli...@.service.in
index cbcef653..159fb4dc 100644
--- a/distro/systemd/openvpn-cli...@.service.in
+++ b/distro/systemd/openvpn-cli...@.service.in
@@ -11,7 +11,7 @@ Type=notify
 PrivateTmp=true
 WorkingDirectory=/etc/openvpn/client
 ExecStart=@sbindir@/openvpn --suppress-timestamps --nobind --config %i.conf
-CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_RAW CAP_SETGID 
CAP_SETUID CAP_SYS_CHROOT CAP_DAC_OVERRIDE
+CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_RAW CAP_SETGID 
CAP_SETUID CAP_SETPCAP CAP_SYS_CHROOT CAP_DAC_OVERRIDE
 LimitNPROC=10
 DeviceAllow=/dev/null rw
 DeviceAllow=/dev/net/tun rw
diff --git a/distro/systemd/openvpn-ser...@.service.in 
b/distro/systemd/openvpn-ser...@.service.in
index d1cc72cb..6e8e7d94 100644
--- a/distro/systemd/openvpn-ser...@.service.in
+++ b/distro/systemd/openvpn-ser...@.service.in
@@ -11,7 +11,7 @@ Type=notify
 PrivateTmp=true
 WorkingDirectory=/etc/openvpn/server
 ExecStart=@sbindir@/openvpn --status %t/openvpn-server/status-%i.log 
--status-version 2 --suppress-timestamps --config %i.conf
-CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_BIND_SERVICE 
CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SYS_CHROOT CAP_DAC_OVERRIDE 
CAP_AUDIT_WRITE
+CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_BIND_SERVICE 
CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SETPCAP CAP_SYS_CHROOT CAP_DAC_OVERRIDE 
CAP_AUDIT_WRITE
 LimitNPROC=10
 DeviceAllow=/dev/null rw
 DeviceAllow=/dev/net/tun rw
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index a6c93038..39b8f77a 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1163,8 +1163,9 @@ do_uid_gid_chroot(struct context *c, bool no_delay)
 {
 if (no_delay)
 {
-platform_group_set(>platform_state_group);
-platform_user_set(>platform_state_user);
+platform_user_group_set(>platform_state_user,
+>platform_state_group,
+c);
 }
 else if (c->first_time)
 {
diff --git a/src/openvpn/platform.c b/src/openvpn/platform.c
index 61afee83..56dca6e6 100644
--- a/src/openvpn/platform.c
+++ b/src/openvpn/platform.c
@@ -29,6 +29,9 @@
 
 #include "syshead.h"
 
+#include "openvpn.h"
+#include "options.h"
+
 #include "buffer.h"
 #include "crypto.h"
 #include "error.h"
@@ -43,6 +46,11 @@
 #include 
 #endif
 
+#ifdef HAVE_LIBCAPNG
+#include 
+#include 
+#endif
+
 /* Redefine the top level directory of the filesystem
  * to restrict access to files for security */
 void
@@ -91,7 +99,7 @@ platform_user_get(const char *username, struct 
platform_state_user *state)
 return ret;
 }
 
-void
+static void
 platform_user_set(const struct platform_state_user *state)
 {
 #if defined(HAVE_GETPWNAM) && defined(HAVE_SETUID)
@@ -130,7 +138,7 @@ platform_group_get(const char *groupname, struct 
platform_state_group *state)
 return ret;
 }
 
-void
+static void
 platform_group_set(const stru

[Openvpn-devel] [PATCH] platform: Retain CAP_NET_ADMIN when dropping privileges

2022-04-18 Thread Timo Rothenpieler 
On Linux, when dropping privileges, interaction with
the network configuration, such as tearing down routes
or ovpn-dco interfaces will fail when --user/--group are
used.

This patch sets the CAP_NET_ADMIN capability, which grants
the needed privileges during the lifetime of the OpenVPN
process when dropping root privileges.

Signed-off-by: Timo Rothenpieler 
Reviewed-By: David Sommerseth 
---
 configure.ac  |  19 
 distro/systemd/openvpn-cli...@.service.in |   2 +-
 distro/systemd/openvpn-ser...@.service.in |   2 +-
 src/openvpn/init.c|  30 +-
 src/openvpn/platform.c| 107 +-
 src/openvpn/platform.h|   7 +-
 6 files changed, 158 insertions(+), 9 deletions(-)

diff --git a/configure.ac b/configure.ac
index 85921ddb..d2eb3426 100644
--- a/configure.ac
+++ b/configure.ac
@@ -794,6 +794,25 @@ dnl
esac
 fi
 
+dnl
+dnl Depend on libcap-ng on Linux
+dnl
+case "$host" in
+   *-*-linux*)
+   PKG_CHECK_MODULES([LIBCAPNG],
+ [libcap-ng],
+ [],
+ [AC_MSG_ERROR([libcap-ng package not found. 
Is the development package and pkg-config installed?])]
+   )
+   AC_CHECK_HEADER([sys/prctl.h],,[AC_MSG_ERROR([sys/prctl.h not 
found!])])
+
+   CFLAGS="${CFLAGS} ${LIBCAPNG_CFALGS}"
+   LIBS="${LIBS} ${LIBCAPNG_LIBS}"
+   AC_DEFINE(HAVE_LIBCAPNG, 1, [Enable libcap-ng support])
+   ;;
+esac
+
+
 if test "${with_crypto_library}" = "openssl"; then
AC_ARG_VAR([OPENSSL_CFLAGS], [C compiler flags for OpenSSL])
AC_ARG_VAR([OPENSSL_LIBS], [linker flags for OpenSSL])
diff --git a/distro/systemd/openvpn-cli...@.service.in 
b/distro/systemd/openvpn-cli...@.service.in
index cbcef653..159fb4dc 100644
--- a/distro/systemd/openvpn-cli...@.service.in
+++ b/distro/systemd/openvpn-cli...@.service.in
@@ -11,7 +11,7 @@ Type=notify
 PrivateTmp=true
 WorkingDirectory=/etc/openvpn/client
 ExecStart=@sbindir@/openvpn --suppress-timestamps --nobind --config %i.conf
-CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_RAW CAP_SETGID 
CAP_SETUID CAP_SYS_CHROOT CAP_DAC_OVERRIDE
+CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_RAW CAP_SETGID 
CAP_SETUID CAP_SETPCAP CAP_SYS_CHROOT CAP_DAC_OVERRIDE
 LimitNPROC=10
 DeviceAllow=/dev/null rw
 DeviceAllow=/dev/net/tun rw
diff --git a/distro/systemd/openvpn-ser...@.service.in 
b/distro/systemd/openvpn-ser...@.service.in
index d1cc72cb..6e8e7d94 100644
--- a/distro/systemd/openvpn-ser...@.service.in
+++ b/distro/systemd/openvpn-ser...@.service.in
@@ -11,7 +11,7 @@ Type=notify
 PrivateTmp=true
 WorkingDirectory=/etc/openvpn/server
 ExecStart=@sbindir@/openvpn --status %t/openvpn-server/status-%i.log 
--status-version 2 --suppress-timestamps --config %i.conf
-CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_BIND_SERVICE 
CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SYS_CHROOT CAP_DAC_OVERRIDE 
CAP_AUDIT_WRITE
+CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_BIND_SERVICE 
CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SETPCAP CAP_SYS_CHROOT CAP_DAC_OVERRIDE 
CAP_AUDIT_WRITE
 LimitNPROC=10
 DeviceAllow=/dev/null rw
 DeviceAllow=/dev/net/tun rw
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 857e52ef..559f7cda 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1138,6 +1138,30 @@ possibly_become_daemon(const struct options *options)
 return ret;
 }
 
+/*
+ * Determine if we need to retain process capabilities. DCO and SITNL need it.
+ * Enforce it for DCO, but only try and soft-fail for SITNL to keep backwards 
compat.
+ *
+ * Returns the tri-state expected by platform_user_group_set.
+ * -1: try to keep caps, but continue if impossible
+ *  0: don't keep caps
+ *  1: keep caps, fail hard if impossible
+ */
+static int
+need_keep_caps(struct context *c)
+{
+if (dco_enabled(>options))
+{
+return 1;
+}
+
+#ifdef ENABLE_SITNL
+return -1;
+#else
+return 0;
+#endif
+}
+
 /*
  * Actually do UID/GID downgrade, chroot and SELinux context switching, if 
requested.
  */
@@ -1167,8 +1191,10 @@ do_uid_gid_chroot(struct context *c, bool no_delay)
 {
 if (no_delay)
 {
-platform_group_set(>platform_state_group);
-platform_user_set(>platform_state_user);
+int keep_caps = need_keep_caps(c);
+platform_user_group_set(>platform_state_user,
+>platform_state_group,
+keep_caps);
 }
 else if (c->first_time)
 {
diff --git a/src/openvpn/platform.c b/src/openvpn/platform.c
index 450f28ba..5a81e8d0 100644
--- a/src/openvpn/platform.c
+++ b/src/openvpn/platform.c
@@ -43,6 +43,11 @@
 #in

Re: [Openvpn-devel] [PATCH v3] platform: Retain CAP_NET_ADMIN when dropping privileges

2022-04-08 Thread Timo Rothenpieler 

On 08/04/2022 11:35, Gert Doering wrote:

Hi,

On Thu, Apr 07, 2022 at 08:40:24PM +0200, Timo Rothenpieler wrote:

+else if (res < 0)
+{
+if (res == -3)
+{
+msg(M_NONFATAL, "Following error likely due to missing capability 
CAP_SETPCAP.");
+}
+msg(err_flags | M_ERRNO, "capng_change_id('%s','%s') failed retaining 
capabilities: %d",
+user_state->username, group_state->groupname, res);
+goto fallback;
+}


Wouldn't that overwrite errno for the "res == -3" case, given that
msg() will do stdio stuff?  Maybe reorder and print the "error likely due..."
message with a preceding "NOTE:" after the capng_change_id() message?

(That would be more typical for our logs - the "NOTE: this could be
because..." tends to come after the error message)


Good point.
I like that idea better as well, will implement.


+if (new_uid >= 0)
+{
+ msg(M_INFO, "UID set to %s", user_state->username);
+}
+if (new_gid >= 0)
+{
+ msg(M_INFO, "GID set to %s", group_state->groupname);
+}
+
+msg(M_INFO, "Capabilities retained: CAP_NET_ADMIN");
+
+return;
+fallback:


My inner whitespace dragon does would prefer to have the blank line
between "return" and "fallback:" (and no blank linke after the M_INFO).


No strong preference there on my side, so sure.


+/* capng_change_id() can leave this flag clobbered on failure
+ * This is working around a bug in libcap-ng, which can leave the flag set
+ * on failure: https://github.com/stevegrubb/libcap-ng/issues/33 */
+if (prctl(PR_GET_KEEPCAPS) && prctl(PR_SET_KEEPCAPS, 0) < 0)
+{
+msg(M_ERR, "Clearing KEEPCAPS flag failed");
+}
+#endif  /* HAVE_LIBCAPNG */


This one does not really look like it should be in "fallback:" - because
that way it always gets called, even if we jump there right at function
entry, if keep_caps == 0.


No, it's intentional. It ensures that it's printed even if we don't 
HAVE_LIBCAPNG but someone called the function requesting to keep 
capabilities, which is not possible then.



+
+if (keep_caps)
+{
+msg(err_flags, "Unable to retain capabilities");
+}
+
+platform_group_set(group_state);
+platform_user_set(user_state);
+}
+


Maybe "fallback:" should be right before platform_group_set()?


(Sorry for being late to the "complain about your code" party...)

gert



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v3] platform: Retain CAP_NET_ADMIN when dropping privileges

2022-04-07 Thread Timo Rothenpieler 
On Linux, when dropping privileges, interaction with
the network configuration, such as tearing down routes
or ovpn-dco interfaces will fail when --user/--group are
used.

This patch sets the CAP_NET_ADMIN capability, which grants
the needed privileges during the lifetime of the OpenVPN
process when dropping root privileges.

Signed-off-by: Timo Rothenpieler 
Reviewed-By: David Sommerseth 
---
 configure.ac  |  19 
 distro/systemd/openvpn-cli...@.service.in |   2 +-
 distro/systemd/openvpn-ser...@.service.in |   2 +-
 src/openvpn/init.c|  30 ++-
 src/openvpn/platform.c| 105 +-
 src/openvpn/platform.h|   7 +-
 6 files changed, 156 insertions(+), 9 deletions(-)

diff --git a/configure.ac b/configure.ac
index 85921ddb..d2eb3426 100644
--- a/configure.ac
+++ b/configure.ac
@@ -794,6 +794,25 @@ dnl
esac
 fi
 
+dnl
+dnl Depend on libcap-ng on Linux
+dnl
+case "$host" in
+   *-*-linux*)
+   PKG_CHECK_MODULES([LIBCAPNG],
+ [libcap-ng],
+ [],
+ [AC_MSG_ERROR([libcap-ng package not found. 
Is the development package and pkg-config installed?])]
+   )
+   AC_CHECK_HEADER([sys/prctl.h],,[AC_MSG_ERROR([sys/prctl.h not 
found!])])
+
+   CFLAGS="${CFLAGS} ${LIBCAPNG_CFALGS}"
+   LIBS="${LIBS} ${LIBCAPNG_LIBS}"
+   AC_DEFINE(HAVE_LIBCAPNG, 1, [Enable libcap-ng support])
+   ;;
+esac
+
+
 if test "${with_crypto_library}" = "openssl"; then
AC_ARG_VAR([OPENSSL_CFLAGS], [C compiler flags for OpenSSL])
AC_ARG_VAR([OPENSSL_LIBS], [linker flags for OpenSSL])
diff --git a/distro/systemd/openvpn-cli...@.service.in 
b/distro/systemd/openvpn-cli...@.service.in
index cbcef653..159fb4dc 100644
--- a/distro/systemd/openvpn-cli...@.service.in
+++ b/distro/systemd/openvpn-cli...@.service.in
@@ -11,7 +11,7 @@ Type=notify
 PrivateTmp=true
 WorkingDirectory=/etc/openvpn/client
 ExecStart=@sbindir@/openvpn --suppress-timestamps --nobind --config %i.conf
-CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_RAW CAP_SETGID 
CAP_SETUID CAP_SYS_CHROOT CAP_DAC_OVERRIDE
+CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_RAW CAP_SETGID 
CAP_SETUID CAP_SETPCAP CAP_SYS_CHROOT CAP_DAC_OVERRIDE
 LimitNPROC=10
 DeviceAllow=/dev/null rw
 DeviceAllow=/dev/net/tun rw
diff --git a/distro/systemd/openvpn-ser...@.service.in 
b/distro/systemd/openvpn-ser...@.service.in
index d1cc72cb..6e8e7d94 100644
--- a/distro/systemd/openvpn-ser...@.service.in
+++ b/distro/systemd/openvpn-ser...@.service.in
@@ -11,7 +11,7 @@ Type=notify
 PrivateTmp=true
 WorkingDirectory=/etc/openvpn/server
 ExecStart=@sbindir@/openvpn --status %t/openvpn-server/status-%i.log 
--status-version 2 --suppress-timestamps --config %i.conf
-CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_BIND_SERVICE 
CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SYS_CHROOT CAP_DAC_OVERRIDE 
CAP_AUDIT_WRITE
+CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_BIND_SERVICE 
CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SETPCAP CAP_SYS_CHROOT CAP_DAC_OVERRIDE 
CAP_AUDIT_WRITE
 LimitNPROC=10
 DeviceAllow=/dev/null rw
 DeviceAllow=/dev/net/tun rw
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 8818ba6f..0a1b00ca 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1138,6 +1138,30 @@ possibly_become_daemon(const struct options *options)
 return ret;
 }
 
+/*
+ * Determine if we need to retain process capabilities. DCO and SITNL need it.
+ * Enforce it for DCO, but only try and soft-fail for SITNL to keep backwards 
compat.
+ *
+ * Returns the tri-state expected by platform_user_group_set.
+ * -1: try to keep caps, but continue if impossible
+ *  0: don't keep caps
+ *  1: keep caps, fail hard if impossible
+ */
+static int
+need_keep_caps(struct context *c)
+{
+if (dco_enabled(>options))
+{
+return 1;
+}
+
+#ifdef ENABLE_SITNL
+return -1;
+#else
+return 0;
+#endif
+}
+
 /*
  * Actually do UID/GID downgrade, chroot and SELinux context switching, if 
requested.
  */
@@ -1167,8 +1191,10 @@ do_uid_gid_chroot(struct context *c, bool no_delay)
 {
 if (no_delay)
 {
-platform_group_set(>platform_state_group);
-platform_user_set(>platform_state_user);
+int keep_caps = need_keep_caps(c);
+platform_user_group_set(>platform_state_user,
+>platform_state_group,
+keep_caps);
 }
 else if (c->first_time)
 {
diff --git a/src/openvpn/platform.c b/src/openvpn/platform.c
index 450f28ba..1e321369 100644
--- a/src/openvpn/platform.c
+++ b/src/openvpn/platform.c
@@ -43,6 +43,11 @@
 #in

Re: [Openvpn-devel] [PATCH v2] Retain CAP_NET_ADMIN when dropping privileges

2022-04-06 Thread Timo Rothenpieler 

On 06.04.2022 11:52, Antonio Quartulli wrote:

Hi,

On 30/03/2022 22:55, Timo Rothenpieler wrote:

---
Using libcap-ng now


A commit message would be good, but I see that David has already 
proposed one.


The latest rebased version of this patch already has that message.
Just seemed silly to re-send it just because of that:
https://github.com/BtbN/openvpn/tree/dco





  configure.ac  | 19 +
  distro/systemd/openvpn-cli...@.service.in |  2 +-
  distro/systemd/openvpn-ser...@.service.in |  2 +-
  src/openvpn/init.c    | 25 ++-
  src/openvpn/platform.c    | 91 +++
  src/openvpn/platform.h    |  5 ++
  6 files changed, 140 insertions(+), 4 deletions(-)

diff --git a/configure.ac b/configure.ac
index 7199483a..168360d4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -794,6 +794,25 @@ dnl
  esac
  fi
+dnl
+dnl Depend on libcap-ng on Linux
+dnl
+case "$host" in
+    *-*-linux*)
+    PKG_CHECK_MODULES([LIBCAPNG],
+  [libcap-ng],
+  [have_libcapng="yes"],


do we really need have_libcapng? it seems it is not used further in 
configure.ac


I have little to no experience with autotools, and I think this is 
straight up copied from the openvpn3 setup.

Can I just leave the line empty? Or put empty [] there?

+  [AC_MSG_ERROR([libcap-ng package not found. Is the 
development package and pkg-config installed?])]

+    )
+    AC_CHECK_HEADER([sys/prctl.h],,[AC_MSG_ERROR([sys/prctl.h not 
found!])])

+
+    CFLAGS="${CFLAGS} ${LIBCAPNG_CFALGS}"
+    LIBS="${LIBS} ${LIBCAPNG_LIBS}"
+    AC_DEFINE(HAVE_LIBCAPNG, 1, [Enable libcap-ng support])
+    ;;
+esac
+
+
  if test "${with_crypto_library}" = "openssl"; then
  AC_ARG_VAR([OPENSSL_CFLAGS], [C compiler flags for OpenSSL])
  AC_ARG_VAR([OPENSSL_LIBS], [linker flags for OpenSSL])
diff --git a/distro/systemd/openvpn-cli...@.service.in 
b/distro/systemd/openvpn-cli...@.service.in

index cbcef653..159fb4dc 100644
--- a/distro/systemd/openvpn-cli...@.service.in
+++ b/distro/systemd/openvpn-cli...@.service.in
@@ -11,7 +11,7 @@ Type=notify
  PrivateTmp=true
  WorkingDirectory=/etc/openvpn/client
  ExecStart=@sbindir@/openvpn --suppress-timestamps --nobind --config 
%i.conf
-CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_RAW 
CAP_SETGID CAP_SETUID CAP_SYS_CHROOT CAP_DAC_OVERRIDE
+CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_RAW 
CAP_SETGID CAP_SETUID CAP_SETPCAP CAP_SYS_CHROOT CAP_DAC_OVERRIDE

  LimitNPROC=10
  DeviceAllow=/dev/null rw
  DeviceAllow=/dev/net/tun rw
diff --git a/distro/systemd/openvpn-ser...@.service.in 
b/distro/systemd/openvpn-ser...@.service.in

index d1cc72cb..6e8e7d94 100644
--- a/distro/systemd/openvpn-ser...@.service.in
+++ b/distro/systemd/openvpn-ser...@.service.in
@@ -11,7 +11,7 @@ Type=notify
  PrivateTmp=true
  WorkingDirectory=/etc/openvpn/server
  ExecStart=@sbindir@/openvpn --status %t/openvpn-server/status-%i.log 
--status-version 2 --suppress-timestamps --config %i.conf
-CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_BIND_SERVICE 
CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SYS_CHROOT CAP_DAC_OVERRIDE 
CAP_AUDIT_WRITE
+CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_BIND_SERVICE 
CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SETPCAP CAP_SYS_CHROOT 
CAP_DAC_OVERRIDE CAP_AUDIT_WRITE

  LimitNPROC=10
  DeviceAllow=/dev/null rw
  DeviceAllow=/dev/net/tun rw
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 8818ba6f..705eb92e 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1138,6 +1138,25 @@ possibly_become_daemon(const struct options 
*options)

  return ret;
  }
+/*
+ * Determine if we need to retain process capabilities. DCO and SITNL 
need it.
+ * Enforce it for DCO, but only try and soft-fail for SITNL to keep 
backwards compat.

+ */


Since this function is returning a tri-state value, may it be better to 
document what the returned values mean in the comment above?



+static int
+get_need_keep_caps(struct context *c)


I know this function is kinda a getter, but two verbs in the function 
name sounds weird. How about just "need_keep_caps"?



+{
+    if (dco_enabled(>options))
+    {
+    return 1;
+    }
+
+#ifdef ENABLE_SITNL
+    return -1;
+#else
+    return 0;
+#endif
+}


Since the check above is really platform specific, wouldn't it make 
sense to move its definition to platform.c?
At the end of the day this function is just invoked once to get a value 
and immediately pass it to platform_user_group_set().


The reason it ended up here is that it needs access to the context, to 
find out if it's using the dco or not.
Should the context just be passed to the platform function instead? I 
see no other one of them doing that.



+
  /*
   * Actually do UID/GID downgrade, chroot and SELinux context 
switching, if req

Re: [Openvpn-devel] [PATCH v2] Retain CAP_NET_ADMIN when dropping privileges

2022-03-31 Thread Timo Rothenpieler 

On 31.03.2022 13:02, Gert Doering wrote:

Hi,

On Thu, Mar 31, 2022 at 12:06:06PM +0200, David Sommerseth wrote:

There is however another related challenge in OpenVPN 2.x, which became
even clearer than be fore with the sitnl implementation we switched over
to on Linux by default with v2.5.  When using --user/--group without
--persist-tun, a reconnect would tear down the interface but could not
recover again and the connection dies.  Using --persist-tun, it could
work a bit better *unless* it needs to change the IP address of the tun
interface.  I'm not sure how well, OpenVPN 2.x works if new routes are
being pushed (OpenVPN 3 supports that as well).  This challenge is also
resolved by granting the process CAP_NET_ADMIN capabilities.


For most non-trivial stuff, OpenVPN with --user will run into problems,
be it route teardown, installing of new routes at renegotiation time,
...

So most people today just run 2.x as root, not getting any security
benefits.


For now, my opinion is that it is currently acceptable to have
CAP_NET_ADMIN available when running with ovpn-dco; to have a smooth
user experience.  OpenVPN is after all a network related process.


I'd even go for "keep CAP_NET_ADMIN for DCO and sitnl" - because it
means "all the route/interface manipulation *and cleanup* stuff can
be done properly, without having to carry root privileges".


That's exactly what the patch does.
Only difference is that for sitnl, to avoid breaking existing setups, it 
will fall back to the old approach of switching user if the capability 
retaining approach failed.



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v2] Retain CAP_NET_ADMIN when dropping privileges

2022-03-30 Thread Timo Rothenpieler 
---
Using libcap-ng now


 configure.ac  | 19 +
 distro/systemd/openvpn-cli...@.service.in |  2 +-
 distro/systemd/openvpn-ser...@.service.in |  2 +-
 src/openvpn/init.c| 25 ++-
 src/openvpn/platform.c| 91 +++
 src/openvpn/platform.h|  5 ++
 6 files changed, 140 insertions(+), 4 deletions(-)

diff --git a/configure.ac b/configure.ac
index 7199483a..168360d4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -794,6 +794,25 @@ dnl
esac
 fi
 
+dnl
+dnl Depend on libcap-ng on Linux
+dnl
+case "$host" in
+   *-*-linux*)
+   PKG_CHECK_MODULES([LIBCAPNG],
+ [libcap-ng],
+ [have_libcapng="yes"],
+ [AC_MSG_ERROR([libcap-ng package not found. 
Is the development package and pkg-config installed?])]
+   )
+   AC_CHECK_HEADER([sys/prctl.h],,[AC_MSG_ERROR([sys/prctl.h not 
found!])])
+
+   CFLAGS="${CFLAGS} ${LIBCAPNG_CFALGS}"
+   LIBS="${LIBS} ${LIBCAPNG_LIBS}"
+   AC_DEFINE(HAVE_LIBCAPNG, 1, [Enable libcap-ng support])
+   ;;
+esac
+
+
 if test "${with_crypto_library}" = "openssl"; then
AC_ARG_VAR([OPENSSL_CFLAGS], [C compiler flags for OpenSSL])
AC_ARG_VAR([OPENSSL_LIBS], [linker flags for OpenSSL])
diff --git a/distro/systemd/openvpn-cli...@.service.in 
b/distro/systemd/openvpn-cli...@.service.in
index cbcef653..159fb4dc 100644
--- a/distro/systemd/openvpn-cli...@.service.in
+++ b/distro/systemd/openvpn-cli...@.service.in
@@ -11,7 +11,7 @@ Type=notify
 PrivateTmp=true
 WorkingDirectory=/etc/openvpn/client
 ExecStart=@sbindir@/openvpn --suppress-timestamps --nobind --config %i.conf
-CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_RAW CAP_SETGID 
CAP_SETUID CAP_SYS_CHROOT CAP_DAC_OVERRIDE
+CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_RAW CAP_SETGID 
CAP_SETUID CAP_SETPCAP CAP_SYS_CHROOT CAP_DAC_OVERRIDE
 LimitNPROC=10
 DeviceAllow=/dev/null rw
 DeviceAllow=/dev/net/tun rw
diff --git a/distro/systemd/openvpn-ser...@.service.in 
b/distro/systemd/openvpn-ser...@.service.in
index d1cc72cb..6e8e7d94 100644
--- a/distro/systemd/openvpn-ser...@.service.in
+++ b/distro/systemd/openvpn-ser...@.service.in
@@ -11,7 +11,7 @@ Type=notify
 PrivateTmp=true
 WorkingDirectory=/etc/openvpn/server
 ExecStart=@sbindir@/openvpn --status %t/openvpn-server/status-%i.log 
--status-version 2 --suppress-timestamps --config %i.conf
-CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_BIND_SERVICE 
CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SYS_CHROOT CAP_DAC_OVERRIDE 
CAP_AUDIT_WRITE
+CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_BIND_SERVICE 
CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SETPCAP CAP_SYS_CHROOT CAP_DAC_OVERRIDE 
CAP_AUDIT_WRITE
 LimitNPROC=10
 DeviceAllow=/dev/null rw
 DeviceAllow=/dev/net/tun rw
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 8818ba6f..705eb92e 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1138,6 +1138,25 @@ possibly_become_daemon(const struct options *options)
 return ret;
 }
 
+/*
+ * Determine if we need to retain process capabilities. DCO and SITNL need it.
+ * Enforce it for DCO, but only try and soft-fail for SITNL to keep backwards 
compat.
+ */
+static int
+get_need_keep_caps(struct context *c)
+{
+if (dco_enabled(>options))
+{
+return 1;
+}
+
+#ifdef ENABLE_SITNL
+return -1;
+#else
+return 0;
+#endif
+}
+
 /*
  * Actually do UID/GID downgrade, chroot and SELinux context switching, if 
requested.
  */
@@ -1167,8 +1186,10 @@ do_uid_gid_chroot(struct context *c, bool no_delay)
 {
 if (no_delay)
 {
-platform_group_set(>platform_state_group);
-platform_user_set(>platform_state_user);
+int keep_caps = get_need_keep_caps(c);
+platform_user_group_set(>platform_state_user,
+>platform_state_group,
+keep_caps);
 }
 else if (c->first_time)
 {
diff --git a/src/openvpn/platform.c b/src/openvpn/platform.c
index 450f28ba..4fce5a83 100644
--- a/src/openvpn/platform.c
+++ b/src/openvpn/platform.c
@@ -43,6 +43,11 @@
 #include 
 #endif
 
+#ifdef HAVE_LIBCAPNG
+#include 
+#include 
+#endif
+
 /* Redefine the top level directory of the filesystem
  * to restrict access to files for security */
 void
@@ -155,6 +160,92 @@ platform_group_set(const struct platform_state_group 
*state)
 #endif
 }
 
+void platform_user_group_set(const struct platform_state_user *user_state,
+ const struct platform_state_group *group_state,
+ int keep_caps)
+{
+unsigned int err_flags = (keep_caps > 0) ? M_FATAL : M_NONFATAL;
+#ifdef HAVE_LIBCAPNG
+int new_gid = -1, new_uid = -1;
+

[Openvpn-devel] [PATCH] Retain CAP_NET_ADMIN when dropping privileges

2022-03-30 Thread Timo Rothenpieler 
---
 configure.ac  | 18 ++
 distro/systemd/openvpn-cli...@.service.in |  2 +-
 distro/systemd/openvpn-ser...@.service.in |  2 +-
 src/openvpn/init.c| 25 ++-
 src/openvpn/platform.c| 79 +++
 src/openvpn/platform.h|  5 ++
 6 files changed, 127 insertions(+), 4 deletions(-)

diff --git a/configure.ac b/configure.ac
index 7199483a..5832b62f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -794,6 +794,24 @@ dnl
esac
 fi
 
+dnl
+dnl Depend on libcap-ng on Linux
+dnl
+case "$host" in
+   *-*-linux*)
+   PKG_CHECK_MODULES([LIBCAPNG],
+ [libcap-ng],
+ [have_libcapng="yes"],
+ [AC_MSG_ERROR([libcap-ng package not found. 
Is the development package and pkg-config installed?])]
+   )
+
+   CFLAGS="${CFLAGS} ${LIBCAPNG_CFALGS}"
+   LIBS="${LIBS} ${LIBCAPNG_LIBS}"
+   AC_DEFINE(HAVE_LIBCAPNG, 1, [Enable libcap-ng support])
+   ;;
+esac
+
+
 if test "${with_crypto_library}" = "openssl"; then
AC_ARG_VAR([OPENSSL_CFLAGS], [C compiler flags for OpenSSL])
AC_ARG_VAR([OPENSSL_LIBS], [linker flags for OpenSSL])
diff --git a/distro/systemd/openvpn-cli...@.service.in 
b/distro/systemd/openvpn-cli...@.service.in
index cbcef653..159fb4dc 100644
--- a/distro/systemd/openvpn-cli...@.service.in
+++ b/distro/systemd/openvpn-cli...@.service.in
@@ -11,7 +11,7 @@ Type=notify
 PrivateTmp=true
 WorkingDirectory=/etc/openvpn/client
 ExecStart=@sbindir@/openvpn --suppress-timestamps --nobind --config %i.conf
-CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_RAW CAP_SETGID 
CAP_SETUID CAP_SYS_CHROOT CAP_DAC_OVERRIDE
+CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_RAW CAP_SETGID 
CAP_SETUID CAP_SETPCAP CAP_SYS_CHROOT CAP_DAC_OVERRIDE
 LimitNPROC=10
 DeviceAllow=/dev/null rw
 DeviceAllow=/dev/net/tun rw
diff --git a/distro/systemd/openvpn-ser...@.service.in 
b/distro/systemd/openvpn-ser...@.service.in
index d1cc72cb..6e8e7d94 100644
--- a/distro/systemd/openvpn-ser...@.service.in
+++ b/distro/systemd/openvpn-ser...@.service.in
@@ -11,7 +11,7 @@ Type=notify
 PrivateTmp=true
 WorkingDirectory=/etc/openvpn/server
 ExecStart=@sbindir@/openvpn --status %t/openvpn-server/status-%i.log 
--status-version 2 --suppress-timestamps --config %i.conf
-CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_BIND_SERVICE 
CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SYS_CHROOT CAP_DAC_OVERRIDE 
CAP_AUDIT_WRITE
+CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_BIND_SERVICE 
CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SETPCAP CAP_SYS_CHROOT CAP_DAC_OVERRIDE 
CAP_AUDIT_WRITE
 LimitNPROC=10
 DeviceAllow=/dev/null rw
 DeviceAllow=/dev/net/tun rw
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 8818ba6f..705eb92e 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1138,6 +1138,25 @@ possibly_become_daemon(const struct options *options)
 return ret;
 }
 
+/*
+ * Determine if we need to retain process capabilities. DCO and SITNL need it.
+ * Enforce it for DCO, but only try and soft-fail for SITNL to keep backwards 
compat.
+ */
+static int
+get_need_keep_caps(struct context *c)
+{
+if (dco_enabled(>options))
+{
+return 1;
+}
+
+#ifdef ENABLE_SITNL
+return -1;
+#else
+return 0;
+#endif
+}
+
 /*
  * Actually do UID/GID downgrade, chroot and SELinux context switching, if 
requested.
  */
@@ -1167,8 +1186,10 @@ do_uid_gid_chroot(struct context *c, bool no_delay)
 {
 if (no_delay)
 {
-platform_group_set(>platform_state_group);
-platform_user_set(>platform_state_user);
+int keep_caps = get_need_keep_caps(c);
+platform_user_group_set(>platform_state_user,
+>platform_state_group,
+keep_caps);
 }
 else if (c->first_time)
 {
diff --git a/src/openvpn/platform.c b/src/openvpn/platform.c
index 450f28ba..e67844ad 100644
--- a/src/openvpn/platform.c
+++ b/src/openvpn/platform.c
@@ -43,6 +43,10 @@
 #include 
 #endif
 
+#ifdef HAVE_LIBCAPNG
+#include 
+#endif
+
 /* Redefine the top level directory of the filesystem
  * to restrict access to files for security */
 void
@@ -155,6 +159,81 @@ platform_group_set(const struct platform_state_group 
*state)
 #endif
 }
 
+void platform_user_group_set(const struct platform_state_user *user_state,
+ const struct platform_state_group *group_state,
+ int keep_caps)
+{
+unsigned int err_flags = (keep_caps > 0) ? M_FATAL : M_NONFATAL;
+#ifdef HAVE_LIBCAPNG
+int new_gid = -1, new_uid = -1;
+int res;
+
+if (keep_caps == 0)
+{
+goto fallback;
+}
+
+/*
+ * new_uid/new_gid defaults to

Re: [Openvpn-devel] [PATCH] Retain CAP_NET_ADMIN when dropping privileges

2022-03-30 Thread Timo Rothenpieler 

On 30.03.2022 11:11, David Sommerseth wrote:

On 30/03/2022 10:51, David Sommerseth wrote:

On 29/03/2022 21:29, Timo Rothenpieler wrote:

---
This patch sits on top of the current dco branch, and will not apply to
latest master.

It solves the issue of dropping root privileges breaking dco and sitnl
due to missing NET_ADMIN capabilities.


  configure.ac   |  3 ++
  src/openvpn/init.c | 22 +-
  src/openvpn/platform.c | 65 +-
  src/openvpn/platform.h |  2 +-
  4 files changed, 89 insertions(+), 3 deletions(-)



Thanks a lot!  I've quickly looked through the code, and I have to NAK 
this approach:



+#ifdef HAVE_LINUX_CAPABILITIES
+#define SET_CAP_HELPER(data, set, cap) data[(cap)>>5].set |= 
1<<((cap)&31)

+
+static bool
+do_keep_caps(bool prepare)
+{
+    struct __user_cap_header_struct cap_hdr = { 
_LINUX_CAPABILITY_VERSION_3 };
+    struct __user_cap_data_struct cap_data[_LINUX_CAPABILITY_U32S_3] 
= {};

+
+    if (syscall(SYS_capget, _hdr, cap_data) < 0)


We should really use libcap or libcap-ng and not avoid using syscalls 
directly.


Is there any preference between the two? I initially used libcap, but 
wanted to avoid introducing another dependency.
But both libcap and libcap-ng seem to be widely adopted by distros, and 
there isn't a huge difference in boilerplate between them for this purpose.



This did not come out well.  Sorry about that.

We should really avoid using syscalls directly, as that binds us to 
certain APIs and bindings.


Newer kernels may also require additional adjustments in the future, to 
preserve the same behaviour.  Which means we need to maintain this code 
and also pay more attention to the security aspects of privilege 
management, like new vulnerabilities and exploits.


The libcap or libcap-ng libraries are used by far more applications, 
doing similar privilege management - and these libraries already pay 
attention to the security aspects of new vulnerabilities and exploits. 
The libcap-ng library is also recommended by more developers, due to its 
simpler API.


It is possible to argue that sitnl does low-level calls to the kernel as 
well.  But potential libraries had an API which was making everything 
far more complex on the OpenVPN side.  For libcap-ng at least, that is 
not the case; as the API it provides is pretty simple.


Shouldn't caps support also be enabled when sitnl is in use?
Given it also needs CAP_NET_ADMIN.



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Retain CAP_NET_ADMIN when dropping privileges

2022-03-29 Thread Timo Rothenpieler 

On 29.03.2022 21:29, Timo Rothenpieler wrote:

+static bool
+do_keep_caps(bool prepare)
+{
+struct __user_cap_header_struct cap_hdr = { _LINUX_CAPABILITY_VERSION_3 };
+struct __user_cap_data_struct cap_data[_LINUX_CAPABILITY_U32S_3] = {};
+
+if (syscall(SYS_capget, _hdr, cap_data) < 0)
+{
+msg(M_NONFATAL | M_ERRNO, "failed getting capabilities");
+return false;
+}
+
+if (prepare)
+{
+SET_CAP_HELPER(cap_data, permitted, CAP_NET_ADMIN);
+}
+else
+{
+SET_CAP_HELPER(cap_data, effective, CAP_NET_ADMIN);


This is missing something like the following:


/* Clamp permitted capabilities to effective ones.
 * Without doing this, the process can give itself root-like caps at 
any time. */
for (int i = 0; i < sizeof(cap_data)/sizeof(cap_data[0]); i++)
{
cap_data[i].permitted = cap_data[i].effective;
}


Without that, the permitted caps stay the full set of root caps, and the 
process can make them effective at any time.


Patch on GitHub is updated with that.


+}
+
+if (syscall(SYS_capset, _hdr, cap_data) < 0)
+{
+msg(M_NONFATAL | M_ERRNO, "failed setting %s capabilities", prepare ? 
"permitted" : "effective");
+return false;
+}
+
+if (prepare && prctl(PR_SET_KEEPCAPS, 1) < 0)
+{
+msg(M_NONFATAL | M_ERRNO, "failed setting keepcaps");
+return false;
+}
+
+return true;
+}



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [ovpn-dco] can not delete tun interface automatically if option "user nobody" is used

2022-03-29 Thread Timo Rothenpieler 

On 29.03.2022 12:48, Gert Doering wrote:

Hi,

On Tue, Mar 29, 2022 at 06:21:37PM +0800, Tony He wrote:

1. Add option "user nobody" to test ovpn-dco.
2. Start openvpn, below is the log. Then we will see tun0 is still
there after openvpn exit. We must use the command "ip link del tunX"
to delete. This is not friendly to end user.


Yes.  This is currently unsolved - if you tell openvpn to give up its
privileges, it will give up its privileges, and then it lacks privileges
to tear down the interface again.

This should be doable with linux net capabilities, but right now, we
have not investigated this option further.  So, for now, do not
use "user" together with DCO.


I just sent a patch ("Retain CAP_NET_ADMIN when dropping privileges") 
that solves this for me.

It retains the CAP_NET_ADMIN capability while switching user.

Patch sits on top of the dco branch, since it uses functions from it.

I'm not seeing the patch yet, but my mail server claims it got delivered.

The patch is also on Github, on top of the dco branch in my fork:
https://github.com/BtbN/openvpn/tree/dco


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] Retain CAP_NET_ADMIN when dropping privileges

2022-03-29 Thread Timo Rothenpieler 
---
This patch sits on top of the current dco branch, and will not apply to
latest master.

It solves the issue of dropping root privileges breaking dco and sitnl
due to missing NET_ADMIN capabilities.


 configure.ac   |  3 ++
 src/openvpn/init.c | 22 +-
 src/openvpn/platform.c | 65 +-
 src/openvpn/platform.h |  2 +-
 4 files changed, 89 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index 7199483a..220c63e5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -710,6 +710,9 @@ if test -z "${LIBPAM_LIBS}"; then
)
 fi
 
+AC_CHECK_HEADERS([linux/capability.h sys/syscall.h sys/prctl.h],
+ [AC_DEFINE(HAVE_LINUX_CAPABILITIES, 1, [Linux capability 
support available])])
+
 case "${with_mem_check}" in
valgrind)
AC_CHECK_HEADERS(
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 8818ba6f..13c07ff0 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1138,6 +1138,25 @@ possibly_become_daemon(const struct options *options)
 return ret;
 }
 
+/*
+ * Determine if we need to retain process capabilities. DCO and SITNL need it.
+ * Enforce it for DCO, but only try and soft-fail for SITNL to keep backwards 
compat.
+ */
+static int
+get_need_keep_caps(struct context *c)
+{
+if (dco_enabled(>options))
+{
+return 1;
+}
+
+#ifdef ENABLE_SITNL
+return -1;
+#else
+return 0;
+#endif
+}
+
 /*
  * Actually do UID/GID downgrade, chroot and SELinux context switching, if 
requested.
  */
@@ -1167,8 +1186,9 @@ do_uid_gid_chroot(struct context *c, bool no_delay)
 {
 if (no_delay)
 {
+int keep_caps = get_need_keep_caps(c);
 platform_group_set(>platform_state_group);
-platform_user_set(>platform_state_user);
+platform_user_set(>platform_state_user, keep_caps);
 }
 else if (c->first_time)
 {
diff --git a/src/openvpn/platform.c b/src/openvpn/platform.c
index 450f28ba..680ebb5f 100644
--- a/src/openvpn/platform.c
+++ b/src/openvpn/platform.c
@@ -43,6 +43,12 @@
 #include 
 #endif
 
+#ifdef HAVE_LINUX_CAPABILITIES
+#include 
+#include 
+#include 
+#endif
+
 /* Redefine the top level directory of the filesystem
  * to restrict access to files for security */
 void
@@ -91,17 +97,74 @@ platform_user_get(const char *username, struct 
platform_state_user *state)
 return ret;
 }
 
+#ifdef HAVE_LINUX_CAPABILITIES
+#define SET_CAP_HELPER(data, set, cap) data[(cap)>>5].set |= 1<<((cap)&31)
+
+static bool
+do_keep_caps(bool prepare)
+{
+struct __user_cap_header_struct cap_hdr = { _LINUX_CAPABILITY_VERSION_3 };
+struct __user_cap_data_struct cap_data[_LINUX_CAPABILITY_U32S_3] = {};
+
+if (syscall(SYS_capget, _hdr, cap_data) < 0)
+{
+msg(M_NONFATAL | M_ERRNO, "failed getting capabilities");
+return false;
+}
+
+if (prepare)
+{
+SET_CAP_HELPER(cap_data, permitted, CAP_NET_ADMIN);
+}
+else
+{
+SET_CAP_HELPER(cap_data, effective, CAP_NET_ADMIN);
+}
+
+if (syscall(SYS_capset, _hdr, cap_data) < 0)
+{
+msg(M_NONFATAL | M_ERRNO, "failed setting %s capabilities", prepare ? 
"permitted" : "effective");
+return false;
+}
+
+if (prepare && prctl(PR_SET_KEEPCAPS, 1) < 0)
+{
+msg(M_NONFATAL | M_ERRNO, "failed setting keepcaps");
+return false;
+}
+
+return true;
+}
+#else
+static bool
+do_keep_caps(bool prepare)
+{
+return false;
+}
+#endif
+
 void
-platform_user_set(const struct platform_state_user *state)
+platform_user_set(const struct platform_state_user *state, int keep_caps)
 {
 #if defined(HAVE_GETPWNAM) && defined(HAVE_SETUID)
 if (state->username && state->pw)
 {
+bool caps_prepared = keep_caps && do_keep_caps(true);
+
 if (setuid(state->pw->pw_uid))
 {
 msg(M_ERR, "setuid('%s') failed", state->username);
 }
 msg(M_INFO, "UID set to %s", state->username);
+
+if (caps_prepared && do_keep_caps(false))
+{
+msg(M_INFO, "Capabilities retained");
+}
+else if (keep_caps > 0)
+{
+msg(M_FATAL, "Failed retaining capabilities");
+}
 }
 #endif
 }
diff --git a/src/openvpn/platform.h b/src/openvpn/platform.h
index a3eec298..ef06da93 100644
--- a/src/openvpn/platform.h
+++ b/src/openvpn/platform.h
@@ -79,7 +79,7 @@ struct platform_state_group {
 
 bool platform_user_get(const char *username, struct platform_state_user 
*state);
 
-void platform_user_set(const struct platform_state_user *state);
+void platform_user_set(const struct platform_state_user *state, int keep_caps);
 
 bool platform_group_get(const char *groupname, struct platform_state_group 
*state);
 
-- 
2.25.1



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net

Re: [Openvpn-devel] [ovpn-dco] can not delete tun interface automatically if option "user nobody" is used

2022-03-29 Thread Timo Rothenpieler 

On 29.03.2022 12:21, Tony He wrote:

Hi,

1. Add option "user nobody" to test ovpn-dco.
2. Start openvpn, below is the log. Then we will see tun0 is still
there after openvpn exit. We must use the command "ip link del tunX"
to delete. This is not friendly to end user.


I think the dco only working if openvpn is running as root is a general 
issue right now.
Like, clients can't connect either, because it fails to communicate with 
the interface after dropping privileges.



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel