Skip to content

Commit

Permalink
Check common name in case sensitive mode by default (#187)
Browse files Browse the repository at this point in the history
  • Loading branch information
jkroepke authored Feb 19, 2024
1 parent bf2788b commit 5455923
Show file tree
Hide file tree
Showing 6 changed files with 167 additions and 39 deletions.
7 changes: 5 additions & 2 deletions docs/Configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ oauth2:
# - "phr"
# - "phrh"
common-name: ""
common-name-case-sensitive: false
# groups:
# - "test"
# - "test2"
Expand Down Expand Up @@ -171,6 +172,8 @@ Usage of openvpn-auth-oauth2:
oauth2 required acr values. Comma separated list. Example: phr,phrh (env: CONFIG_OAUTH2_VALIDATE_ACR)
--oauth2.validate.common-name string
validate common_name from OpenVPN with IDToken claim. For example: preferred_username or sub (env: CONFIG_OAUTH2_VALIDATE_COMMON__NAME)
--oauth2.validate.common-name-case-sensitive
If true, openvpn-auth-oauth2 will validate the common case in sensitive mode (env: CONFIG_OAUTH2_VALIDATE_COMMON__NAME__CASE__SENSITIVE)
--oauth2.validate.groups value
oauth2 required user groups. If multiple groups are configured, the user needs to be least in one group. Comma separated list. Example: group1,group2,group3 (env: CONFIG_OAUTH2_VALIDATE_GROUPS)
--oauth2.validate.ipaddr
Expand All @@ -189,8 +192,8 @@ Usage of openvpn-auth-oauth2:
bypass oauth authentication for CNs. Comma separated list. (env: CONFIG_OPENVPN_BYPASS_COMMON__NAMES)
--openvpn.common-name.environment-variable string
Name of the environment variable in the OpenVPN management interface which contains the common name. If username-as-common-name is enabled, this should be set to 'username' to use the username as common name. Other values like 'X509_0_emailAddress' are supported. See https://openvpn.net/community-resources/reference-manual-for-openvpn-2-6/#environmental-variables for more information. (env: CONFIG_OPENVPN_COMMON__NAME_ENVIRONMENT__VARIABLE) (default "common_name")
--openvpn.common-name.mode string
If common names are too long, use md5/sha1 to hash them or omit to skip them. If omit, oauth2.validate.common-name does not work anymore. Values: [plain,omit] (env: CONFIG_OPENVPN_COMMON__NAME_MODE) (default "plain")
--openvpn.common-name.mode value
If common names are too long, use md5/sha1 to hash them or omit to skip them. If omit, oauth2.validate.common-name does not work anymore. Values: [plain,omit] (env: CONFIG_OPENVPN_COMMON__NAME_MODE) (default plain)
--openvpn.pass-through.address string
The address of the pass-through socket. Must start with unix:// or tcp:// (env: CONFIG_OPENVPN_PASS__THROUGH_ADDRESS) (default "unix:/run/openvpn-auth-oauth2/server.sock")
--openvpn.pass-through.enabled
Expand Down
9 changes: 7 additions & 2 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,9 @@ func FlagSet(name string) *flag.FlagSet {
"Other values like 'X509_0_emailAddress' are supported. "+
"See https://openvpn.net/community-resources/reference-manual-for-openvpn-2-6/#environmental-variables for more information.",
)
flagSet.String(
flagSet.TextVar(new(OpenVPNCommonNameMode),
"openvpn.common-name.mode",
Defaults.OpenVpn.CommonName.Mode.String(),
Defaults.OpenVpn.CommonName.Mode,
"If common names are too long, use md5/sha1 to hash them or omit to skip them. "+
"If omit, oauth2.validate.common-name does not work anymore. Values: [plain,omit]",
)
Expand Down Expand Up @@ -278,6 +278,11 @@ func FlagSet(name string) *flag.FlagSet {
Defaults.OAuth2.Validate.CommonName,
"validate common_name from OpenVPN with IDToken claim. For example: preferred_username or sub",
)
flagSet.Bool(
"oauth2.validate.common-name-case-sensitive",
Defaults.OAuth2.Validate.CommonNameCaseSensitive,
"If true, openvpn-auth-oauth2 will validate the common case in sensitive mode",
)
flagSet.TextVar(new(StringSlice),
"oauth2.scopes",
Defaults.OAuth2.Scopes,
Expand Down
12 changes: 7 additions & 5 deletions internal/config/load_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ oauth2:
secret: "test"
validate:
common-name: "preffered_username"
common-name-case-sensitive: true
groups:
- "test"
- "test2"
Expand Down Expand Up @@ -191,11 +192,12 @@ http:
Secret: "1jd93h5b6s82lf03jh5b2hf9",
},
Validate: config.OAuth2Validate{
CommonName: "preffered_username",
IPAddr: true,
Issuer: false,
Groups: []string{"test", "test2"},
Roles: []string{"test", "test2"},
CommonName: "preffered_username",
CommonNameCaseSensitive: true,
IPAddr: true,
Issuer: false,
Groups: []string{"test", "test2"},
Roles: []string{"test", "test2"},
},
},
},
Expand Down
13 changes: 7 additions & 6 deletions internal/config/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,13 @@ type OAuth2Endpoints struct {
}

type OAuth2Validate struct {
Acr StringSlice `koanf:"acr"`
Groups StringSlice `koanf:"groups"`
Roles StringSlice `koanf:"roles"`
IPAddr bool `koanf:"ipaddr"`
Issuer bool `koanf:"issuer"`
CommonName string `koanf:"common-name"`
Acr StringSlice `koanf:"acr"`
Groups StringSlice `koanf:"groups"`
Roles StringSlice `koanf:"roles"`
IPAddr bool `koanf:"ipaddr"`
Issuer bool `koanf:"issuer"`
CommonName string `koanf:"common-name"`
CommonNameCaseSensitive bool `koanf:"common-name-case-sensitive"`
}

type OAuth2Refresh struct {
Expand Down
7 changes: 7 additions & 0 deletions internal/oauth2/providers/generic/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"slices"
"strings"

"github.com/jkroepke/openvpn-auth-oauth2/internal/config"
"github.com/jkroepke/openvpn-auth-oauth2/internal/oauth2/idtoken"
Expand Down Expand Up @@ -85,6 +86,12 @@ func (p *Provider) CheckCommonName(session state.State, tokens *oidc.Tokens[*idt
}

tokenCommonName = utils.TransformCommonName(p.Conf.OpenVpn.CommonName.Mode, tokenCommonName)

if !p.Conf.OAuth2.Validate.CommonNameCaseSensitive {
session.CommonName = strings.ToLower(session.CommonName)
tokenCommonName = strings.ToLower(tokenCommonName)
}

if tokenCommonName != session.CommonName {
return fmt.Errorf("common_name %w: openvpn client: %s - oidc token: %s",
ErrMismatch, session.CommonName, tokenCommonName)
Expand Down
158 changes: 134 additions & 24 deletions internal/oauth2/providers/generic/check_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package generic_test

import (
"context"
"errors"
"net/http"
"testing"

Expand Down Expand Up @@ -150,17 +151,139 @@ func TestValidateCommonName(t *testing.T) {

for _, tt := range []struct {
name string
tokenClaim string
tokenCommonName string
requiredCommonName string
commonNameMode config.OpenVPNCommonNameMode
err string
conf config.Config
err error
}{
{"sub empty", "sub", "apple", "", config.CommonNameModePlain, "common_name mismatch: openvpn client is empty"},
{"sub required", "sub", "apple", "apple", config.CommonNameModePlain, ""},
{"sub required wrong", "sub", "pear", "apple", config.CommonNameModePlain, "common_name mismatch: openvpn client: apple - oidc token: pear"},
{"nonexists claim", "nonexists", "pear", "apple", config.CommonNameModePlain, "missing claim: nonexists"},
{"sub omit", "sub", "apple", config.CommonNameModeOmitValue, config.CommonNameModeOmit, "common_name mismatch: openvpn client is empty"},
{
"sub empty",
"apple",
"",
config.Config{
OpenVpn: config.OpenVpn{
CommonName: config.OpenVPNCommonName{
Mode: config.CommonNameModePlain,
},
},
OAuth2: config.OAuth2{
Validate: config.OAuth2Validate{
CommonName: "sub",
},
},
},
errors.New("common_name mismatch: openvpn client is empty"),
},
{
"sub required",
"apple",
"apple",
config.Config{
OpenVpn: config.OpenVpn{
CommonName: config.OpenVPNCommonName{
Mode: config.CommonNameModePlain,
},
},
OAuth2: config.OAuth2{
Validate: config.OAuth2Validate{
CommonName: "sub",
},
},
},
nil,
},
{
"sub required case insensitive",
"APPLE",
"apple",
config.Config{
OpenVpn: config.OpenVpn{
CommonName: config.OpenVPNCommonName{
Mode: config.CommonNameModePlain,
},
},
OAuth2: config.OAuth2{
Validate: config.OAuth2Validate{
CommonName: "sub",
CommonNameCaseSensitive: false,
},
},
},
nil,
},
{
"sub required wrong case insensitive",
"APPLE",
"apple",
config.Config{
OpenVpn: config.OpenVpn{
CommonName: config.OpenVPNCommonName{
Mode: config.CommonNameModePlain,
},
},
OAuth2: config.OAuth2{
Validate: config.OAuth2Validate{
CommonName: "sub",
CommonNameCaseSensitive: true,
},
},
},
errors.New("common_name mismatch: openvpn client: apple - oidc token: APPLE"),
},
{
"sub required wrong",
"pear",
"apple",
config.Config{
OpenVpn: config.OpenVpn{
CommonName: config.OpenVPNCommonName{
Mode: config.CommonNameModePlain,
},
},
OAuth2: config.OAuth2{
Validate: config.OAuth2Validate{
CommonName: "sub",
},
},
},
errors.New("common_name mismatch: openvpn client: apple - oidc token: pear"),
},
{
"nonexists claim",
"pear",
"apple",
config.Config{
OpenVpn: config.OpenVpn{
CommonName: config.OpenVPNCommonName{
Mode: config.CommonNameModePlain,
},
},
OAuth2: config.OAuth2{
Validate: config.OAuth2Validate{
CommonName: "nonexists",
},
},
},
errors.New("missing claim: nonexists"),
},
{
"sub omit",
"apple",
config.CommonNameModeOmitValue,
config.Config{
OpenVpn: config.OpenVpn{
CommonName: config.OpenVPNCommonName{
Mode: config.CommonNameModeOmit,
},
},
OAuth2: config.OAuth2{
Validate: config.OAuth2Validate{
CommonName: "sub",
},
},
},
errors.New("common_name mismatch: openvpn client is empty"),
},
} {
tt := tt
t.Run(tt.name, func(t *testing.T) {
Expand All @@ -174,32 +297,19 @@ func TestValidateCommonName(t *testing.T) {
},
}

conf := config.Config{
OpenVpn: config.OpenVpn{
CommonName: config.OpenVPNCommonName{
Mode: tt.commonNameMode,
},
},
OAuth2: config.OAuth2{
Validate: config.OAuth2Validate{
CommonName: tt.tokenClaim,
},
},
}

session := state.State{
CommonName: tt.requiredCommonName,
}

provider, err := generic.NewProvider(context.Background(), conf, http.DefaultClient)
provider, err := generic.NewProvider(context.Background(), tt.conf, http.DefaultClient)
require.NoError(t, err)

err = provider.CheckCommonName(session, token)
if tt.err == "" {
if tt.err == nil {
require.NoError(t, err)
} else {
require.Error(t, err)
assert.Equal(t, tt.err, err.Error())
assert.EqualError(t, tt.err, err.Error())
}
})
}
Expand Down

0 comments on commit 5455923

Please sign in to comment.