Repository: jkroepke/openvpn-auth-oauth2
Pull request #416 (2025-02-12 19:53:05)

mask password from openvpn clients in debug logs. (user password from SSO accounts not affected)
#### What this PR does / why we need it

#### Which issue this PR fixes

*(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #

- fixes #

#### Special notes for your reviewer

#### Particularly user-facing changes

#### Checklist

Complete these before marking the PR as `ready to review`:

<!-- [Place an '[x]' (no spaces) in all applicable fields.] -->

- [ ] [DCO](https://github.com/prometheus-community/helm-charts/blob/main/CONTRIBUTING.md#sign-off-your-work) signed
- [ ] The PR title has a summary of the changes
- [ ] The PR body has a summary to reflect any significant (and particularly user-facing) changes introduced by this PR


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/jkroepke/openvpn-auth-oauth2/pull/416?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jan-Otto+Kr%C3%B6pke) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 83.07%. Comparing base [(`4b51d04`)](https://app.codecov.io/gh/jkroepke/openvpn-auth-oauth2/commit/4b51d0465a5a041ef03dd500488bc3d36b9c63a0?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jan-Otto+Kr%C3%B6pke) to head [(`bcf68d2`)](https://app.codecov.io/gh/jkroepke/openvpn-auth-oauth2/commit/bcf68d206bab2082f43c974dddd4729381145fc7?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jan-Otto+Kr%C3%B6pke).
> Report is 1 commits behind head on main.

<details><summary>Additional details and impacted files</summary>


```diff
@@           Coverage Diff           @@
##             main     #416   +/-   ##
=======================================
  Coverage   83.07%   83.07%           
=======================================
  Files          50       50           
  Lines        2889     2889           
=======================================
  Hits         2400     2400           
  Misses        372      372           
  Partials      117      117           
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/jkroepke/openvpn-auth-oauth2/pull/416?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jan-Otto+Kr%C3%B6pke).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jan-Otto+Kr%C3%B6pke).


Patch:
diff --git a/internal/openvpn/handler.go b/internal/openvpn/handler.go
index ec54c0e..1c5b8de 100644
--- a/internal/openvpn/handler.go
+++ b/internal/openvpn/handler.go
@@ -7,12 +7,15 @@ import (
 	"fmt"
 	"io"
 	"log/slog"
+	"regexp"
 	"strings"
 	"time"
 
 	"github.com/jkroepke/openvpn-auth-oauth2/internal/openvpn/connection"
 )
 
+var reMaskClientPassword = regexp.MustCompile(`CLIENT:ENV,password=(.*)\r?\n`)
+
 // handlePassword enters the password on the OpenVPN management interface connection.
 func (c *Client) handlePassword(ctx context.Context) error {
 	buf := make([]byte, 15)
@@ -142,7 +145,7 @@ func (c *Client) handleMessage(ctx context.Context, message string) error {
 }
 
 func (c *Client) handleClientMessage(ctx context.Context, message string) error {
-	c.logger.LogAttrs(ctx, slog.LevelDebug, message)
+	c.logger.LogAttrs(ctx, slog.LevelDebug, reMaskClientPassword.ReplaceAllLiteralString(message, "CLIENT:ENV,password=***\r\n"))
 
 	client, err := connection.NewClient(c.conf, message)
 	if err != nil {


Test patch:
diff --git a/internal/openvpn/main_test.go b/internal/openvpn/main_test.go
index 5a40230..0c816d4 100644
--- a/internal/openvpn/main_test.go
+++ b/internal/openvpn/main_test.go
@@ -51,8 +51,6 @@ func TestClientInvalidServer(t *testing.T) {
 func TestClientFull(t *testing.T) {
 	t.Parallel()
 
-	logger := testutils.NewTestLogger()
-
 	confs := []struct {
 		name   string
 		conf   config.Config
@@ -204,6 +202,25 @@ func TestClientFull(t *testing.T) {
 			"client-auth-nt 0 1",
 			nil,
 		},
+		{
+			"client password mask",
+			config.Config{
+				HTTP: config.HTTP{
+					BaseURL: &config.URL{Scheme: "http", Host: "localhost"},
+					Secret:  testutils.Secret,
+				},
+				OpenVpn: config.OpenVpn{
+					CommonName: config.OpenVPNCommonName{
+						EnvironmentVariableName: "common_name",
+					},
+					Bypass:   config.OpenVpnBypass{CommonNames: []string{"bypass"}},
+					Password: "password",
+				},
+			},
+			">CLIENT:CONNECT,0,1\r\n>CLIENT:ENV,common_name=bypass\r\nCLIENT:ENV,password=important value\n>CLIENT:ENV,END\r\n",
+			"client-auth-nt 0 1",
+			nil,
+		},
 		{
 			"client established",
 			config.Config{
@@ -287,6 +304,8 @@ func TestClientFull(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
+			logger := testutils.NewTestLogger()
+
 			managementInterface, err := nettest.NewLocalListener("tcp")
 			require.NoError(t, err)
 
@@ -337,6 +356,10 @@ func TestClientFull(t *testing.T) {
 					assert.Equal(t, tt.expect, auth, logger.String())
 				}
 
+				if strings.Contains(tt.client, "CLIENT:ENV,password=") {
+					assert.Contains(t, logger.String(), `CLIENT:ENV,password=***`, logger.String())
+				}
+
 				testutils.SendMessage(t, conn, "SUCCESS: %s command succeeded\r\n", strings.SplitN(auth, " ", 2)[0])
 
 				if strings.Contains(auth, "client-deny") {
diff --git a/internal/utils/testutils/logger.go b/internal/utils/testutils/logger.go
index 67ae82c..c79844f 100644
--- a/internal/utils/testutils/logger.go
+++ b/internal/utils/testutils/logger.go
@@ -16,7 +16,9 @@ func NewTestLogger() *Logger {
 	syncBuffer.buffer.Grow(16 << 20)
 
 	return &Logger{
-		slog.New(slog.NewTextHandler(syncBuffer, nil)),
+		slog.New(slog.NewTextHandler(syncBuffer, &slog.HandlerOptions{
+			Level: slog.LevelDebug,
+		})),
 		syncBuffer,
 	}
 }