From 54f1946aba17b2faaebfeb3205262d4af3f9f54c Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Wed, 19 Oct 2022 18:38:09 +0200 Subject: [PATCH] OIDC: allow to skip JWT signature validation It's intended for special cases where providers,such as Azure, use the "none" algorithm Signed-off-by: Nicola Murino --- docs/full-configuration.md | 1 + go.mod | 2 +- go.sum | 4 ++-- internal/config/config.go | 27 +++++++++++++++++---------- internal/config/config_test.go | 5 +++++ internal/httpd/oidc.go | 7 ++++++- sftpgo.json | 1 + 7 files changed, 33 insertions(+), 14 deletions(-) diff --git a/docs/full-configuration.md b/docs/full-configuration.md index 9e89d61d..c1f1220d 100644 --- a/docs/full-configuration.md +++ b/docs/full-configuration.md @@ -292,6 +292,7 @@ The configuration file contains the following sections: - `role_field`, string. Defines the optional ID token claims field to map to a SFTPGo role. If the defined ID token claims field is set to `admin` the authenticated user is mapped to an SFTPGo admin. You don't need to specify this field if you want to use OpenID only for the Web Client UI. If the field is inside a nested structure, you can use the dot notation to traverse the structures. Default: blank. - `implicit_roles`, boolean. If set, the `role_field` is ignored and the SFTPGo role is assumed based on the login link used. Default: `false`. - `custom_fields`, list of strings. Custom token claims fields to pass to the pre-login hook. Default: empty. + - `insecure_skip_signature_check`, boolean. This setting causes SFTPGo to skip JWT signature validation. It's intended for special cases where providers, such as Azure, use the `none` algorithm. Skipping the signature validation can cause security issues. Default: `false`. - `debug`, boolean. If set, the received id tokens will be logged at debug level. Default: `false`. - `security`, struct. Defines security headers to add to HTTP responses and allows to restrict allowed hosts. The following parameters are supported: - `enabled`, boolean. Set to `true` to enable security configurations. Default: `false`. diff --git a/go.mod b/go.mod index 32375762..2e63ff59 100644 --- a/go.mod +++ b/go.mod @@ -44,7 +44,7 @@ require ( github.com/minio/sio v0.3.0 github.com/otiai10/copy v1.7.0 github.com/pires/go-proxyproto v0.6.2 - github.com/pkg/sftp v1.13.6-0.20221016140646-c4323ce9f25f + github.com/pkg/sftp v1.13.6-0.20221018182125-7da137aa03f0 github.com/pquerna/otp v1.3.0 github.com/prometheus/client_golang v1.13.0 github.com/robfig/cron/v3 v3.0.1 diff --git a/go.sum b/go.sum index b9e2e520..73190d2d 100644 --- a/go.sum +++ b/go.sum @@ -1341,8 +1341,8 @@ github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINE github.com/pkg/profile v1.2.1/go.mod h1:hJw3o1OdXxsrSjjVksARp5W95eeEaEfptyVZyv6JUPA= github.com/pkg/sftp v1.10.1/go.mod h1:lYOWFsE0bwd1+KfKJaKeuokY15vzFx25BLbzYYoAxZI= github.com/pkg/sftp v1.13.1/go.mod h1:3HaPG6Dq1ILlpPZRO0HVMrsydcdLt6HRDccSgb87qRg= -github.com/pkg/sftp v1.13.6-0.20221016140646-c4323ce9f25f h1:aNkWgLhT2d9JJFKom5OrPF50Bql+6sBtjU722M2aVWA= -github.com/pkg/sftp v1.13.6-0.20221016140646-c4323ce9f25f/go.mod h1:wHDZ0IZX6JcBYRK1TH9bcVq8G7TLpVHYIGJRFnmPfxg= +github.com/pkg/sftp v1.13.6-0.20221018182125-7da137aa03f0 h1:QJypP3NZEUt+ka49zyp/MSdpjjM9EYkg0WA1NZQaxT0= +github.com/pkg/sftp v1.13.6-0.20221018182125-7da137aa03f0/go.mod h1:wHDZ0IZX6JcBYRK1TH9bcVq8G7TLpVHYIGJRFnmPfxg= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/posener/complete v1.1.1/go.mod h1:em0nMJCgc9GFtwrmVmEMR/ZL6WyhyjMBndrE9hABlRI= diff --git a/internal/config/config.go b/internal/config/config.go index ab61f72a..c3c45dd7 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -116,16 +116,17 @@ var ( RenderOpenAPI: true, WebClientIntegrations: nil, OIDC: httpd.OIDC{ - ClientID: "", - ClientSecret: "", - ConfigURL: "", - RedirectBaseURL: "", - UsernameField: "", - RoleField: "", - ImplicitRoles: false, - Scopes: []string{"openid", "profile", "email"}, - CustomFields: []string{}, - Debug: false, + ClientID: "", + ClientSecret: "", + ConfigURL: "", + RedirectBaseURL: "", + UsernameField: "", + RoleField: "", + ImplicitRoles: false, + Scopes: []string{"openid", "profile", "email"}, + CustomFields: []string{}, + InsecureSkipSignatureCheck: false, + Debug: false, }, Security: httpd.SecurityConf{ Enabled: false, @@ -1520,6 +1521,12 @@ func getHTTPDOIDCFromEnv(idx int) (httpd.OIDC, bool) { isSet = true } + skipSignatureCheck, ok := lookupBoolFromEnv(fmt.Sprintf("SFTPGO_HTTPD__BINDINGS__%v__OIDC__INSECURE_SKIP_SIGNATURE_CHECK", idx)) + if ok { + result.InsecureSkipSignatureCheck = skipSignatureCheck + isSet = true + } + debug, ok := lookupBoolFromEnv(fmt.Sprintf("SFTPGO_HTTPD__BINDINGS__%v__OIDC__DEBUG", idx)) if ok { result.Debug = debug diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 9ed9492c..dd8c32c1 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1145,6 +1145,7 @@ func TestHTTPDBindingsFromEnv(t *testing.T) { os.Setenv("SFTPGO_HTTPD__BINDINGS__2__OIDC__SCOPES", "openid") os.Setenv("SFTPGO_HTTPD__BINDINGS__2__OIDC__IMPLICIT_ROLES", "1") os.Setenv("SFTPGO_HTTPD__BINDINGS__2__OIDC__CUSTOM_FIELDS", "field1,field2") + os.Setenv("SFTPGO_HTTPD__BINDINGS__2__OIDC__INSECURE_SKIP_SIGNATURE_CHECK", "1") os.Setenv("SFTPGO_HTTPD__BINDINGS__2__OIDC__DEBUG", "1") os.Setenv("SFTPGO_HTTPD__BINDINGS__2__SECURITY__ENABLED", "true") os.Setenv("SFTPGO_HTTPD__BINDINGS__2__SECURITY__ALLOWED_HOSTS", "*.example.com,*.example.net") @@ -1213,6 +1214,7 @@ func TestHTTPDBindingsFromEnv(t *testing.T) { os.Unsetenv("SFTPGO_HTTPD__BINDINGS__2__OIDC__SCOPES") os.Unsetenv("SFTPGO_HTTPD__BINDINGS__2__OIDC__IMPLICIT_ROLES") os.Unsetenv("SFTPGO_HTTPD__BINDINGS__2__OIDC__CUSTOM_FIELDS") + os.Unsetenv("SFTPGO_HTTPD__BINDINGS__2__OIDC__INSECURE_SKIP_SIGNATURE_CHECK") os.Unsetenv("SFTPGO_HTTPD__BINDINGS__2__OIDC__DEBUG") os.Unsetenv("SFTPGO_HTTPD__BINDINGS__2__SECURITY__ENABLED") os.Unsetenv("SFTPGO_HTTPD__BINDINGS__2__SECURITY__ALLOWED_HOSTS") @@ -1263,6 +1265,7 @@ func TestHTTPDBindingsFromEnv(t *testing.T) { require.False(t, bindings[0].Security.Enabled) require.Equal(t, 0, bindings[0].ClientIPHeaderDepth) require.Len(t, bindings[0].OIDC.Scopes, 3) + require.False(t, bindings[0].OIDC.InsecureSkipSignatureCheck) require.False(t, bindings[0].OIDC.Debug) require.Equal(t, 8000, bindings[1].Port) require.Equal(t, "127.0.0.1", bindings[1].Address) @@ -1277,6 +1280,7 @@ func TestHTTPDBindingsFromEnv(t *testing.T) { require.Equal(t, 1, bindings[1].HideLoginURL) require.Empty(t, bindings[1].OIDC.ClientID) require.Len(t, bindings[1].OIDC.Scopes, 3) + require.False(t, bindings[1].OIDC.InsecureSkipSignatureCheck) require.False(t, bindings[1].OIDC.Debug) require.False(t, bindings[1].Security.Enabled) require.Equal(t, "Web Admin", bindings[1].Branding.WebAdmin.Name) @@ -1316,6 +1320,7 @@ func TestHTTPDBindingsFromEnv(t *testing.T) { require.Len(t, bindings[2].OIDC.CustomFields, 2) require.Equal(t, "field1", bindings[2].OIDC.CustomFields[0]) require.Equal(t, "field2", bindings[2].OIDC.CustomFields[1]) + require.True(t, bindings[2].OIDC.InsecureSkipSignatureCheck) require.True(t, bindings[2].OIDC.Debug) require.True(t, bindings[2].Security.Enabled) require.Len(t, bindings[2].Security.AllowedHosts, 2) diff --git a/internal/httpd/oidc.go b/internal/httpd/oidc.go index 2fc69ef6..699109b6 100644 --- a/internal/httpd/oidc.go +++ b/internal/httpd/oidc.go @@ -90,6 +90,10 @@ type OIDC struct { Scopes []string `json:"scopes" mapstructure:"scopes"` // Custom token claims fields to pass to the pre-login hook CustomFields []string `json:"custom_fields" mapstructure:"custom_fields"` + // InsecureSkipSignatureCheck causes SFTPGo to skip JWT signature validation. + // It's intended for special cases where providers, such as Azure, use the "none" + // algorithm. Skipping the signature validation can cause security issues + InsecureSkipSignatureCheck bool `json:"insecure_skip_signature_check" mapstructure:"insecure_skip_signature_check"` // Debug enables the OIDC debug mode. In debug mode, the received id_token will be logged // at the debug level Debug bool `json:"debug" mapstructure:"debug"` @@ -160,7 +164,8 @@ func (o *OIDC) initialize() error { } o.provider = provider o.verifier = provider.Verifier(&oidc.Config{ - ClientID: o.ClientID, + ClientID: o.ClientID, + InsecureSkipSignatureCheck: o.InsecureSkipSignatureCheck, }) o.oauth2Config = &oauth2.Config{ ClientID: o.ClientID, diff --git a/sftpgo.json b/sftpgo.json index 1782ecb1..898d2598 100644 --- a/sftpgo.json +++ b/sftpgo.json @@ -280,6 +280,7 @@ "role_field": "", "implicit_roles": false, "custom_fields": [], + "insecure_skip_signature_check": false, "debug": false }, "security": {