diff --git a/lib/auth/github.go b/lib/auth/github.go index c2b92bbffc75d..e94463d880c63 100644 --- a/lib/auth/github.go +++ b/lib/auth/github.go @@ -26,9 +26,7 @@ import ( "io" "log/slog" "net/http" - "net/netip" "net/url" - "slices" "strings" "time" @@ -153,7 +151,12 @@ func (a *Server) CreateGithubAuthRequest(ctx context.Context, req types.GithubAu // checked, as they will point the browser away from the IdP or the web UI // after the authentication is done if !req.CreateWebSession { - if err := ValidateClientRedirect(req.ClientRedirectURL, req.SSOTestFlow, connector.GetClientRedirectSettings()); err != nil { + ceremonyType := sso.CeremonyTypeLogin + if req.SSOTestFlow { + ceremonyType = sso.CeremonyTypeTest + } + + if err := sso.ValidateClientRedirect(req.ClientRedirectURL, ceremonyType, connector.GetClientRedirectSettings()); err != nil { return nil, trace.Wrap(err, InvalidClientRedirectErrorMessage) } } @@ -1005,8 +1008,6 @@ func (a *Server) createGithubUser(ctx context.Context, p *CreateUserParams, dryR return user, nil } -const unknownRedirectHostnameErrMsg = "unknown custom client redirect URL hostname" - // ValidateClientRedirect checks a desktop client redirect URL for SSO logins // against some (potentially nil) settings from an auth connector; in the // current implementation, that means either "http" schema with a hostname of @@ -1018,105 +1019,14 @@ const unknownRedirectHostnameErrMsg = "unknown custom client redirect URL hostna // list is non-empty URLs in both the "http" and "https" schema are allowed // if the hostname is an IP address that is contained in a specified CIDR // range on any port. +// +// TODO(Joerger): Replaced by [sso.ValidateClientRedirect], remove once /e no longer depends on it func ValidateClientRedirect(clientRedirect string, ssoTestFlow bool, settings *types.SSOClientRedirectSettings) error { - if clientRedirect == "" { - // empty redirects are non-functional and harmless, so we allow them as - // they're used a lot in test code - return nil - } - - u, err := url.Parse(clientRedirect) - if err != nil { - return trace.Wrap(err, "parsing client redirect URL") - } - - if u.Opaque != "" { - return trace.BadParameter("unexpected opaque client redirect URL") - } - if u.User != nil { - return trace.BadParameter("unexpected userinfo in client redirect URL") - } - if u.Fragment != "" || u.RawFragment != "" { - return trace.BadParameter("unexpected fragment in client redirect URL") - } - - // For Web MFA redirect, we expect a relative path. The proxy handling the SSO callback - // will will redirect to itself with this relative path. - if u.Path == sso.WebMFARedirect { - if u.IsAbs() { - return trace.BadParameter("invalid scheme in client redirect URL for SSO MFA") - } - if u.Hostname() != "" { - return trace.BadParameter("invalid host name in client redirect URL for SSO MFA") - } - return nil - } - - if u.EscapedPath() != "/callback" { - return trace.BadParameter("invalid path in client redirect URL") - } - if q, err := url.ParseQuery(u.RawQuery); err != nil { - return trace.Wrap(err, "parsing query in client redirect URL") - } else if len(q) != 1 || len(q["secret_key"]) != 1 { - return trace.BadParameter("malformed query parameters in client redirect URL") - } - - // we checked everything but u.Scheme and u.Host now - if u.Scheme != "http" && u.Scheme != "https" { - return trace.BadParameter("invalid scheme in client redirect URL") - } - - // allow HTTP redirects to local addresses - allowedHTTPLocalAddrs := []string{"localhost", "127.0.0.1", "::1"} - if u.Scheme == "http" && slices.Contains(allowedHTTPLocalAddrs, u.Hostname()) { - return nil - } - + ceremonyType := sso.CeremonyTypeLogin if ssoTestFlow { - return trace.AccessDenied("custom client redirect URLs are not allowed in SSO test") + ceremonyType = sso.CeremonyTypeTest } - - if settings == nil { - return trace.AccessDenied("%s", unknownRedirectHostnameErrMsg) - } - - // allow HTTP or HTTPS redirects from IPs in specified CIDR ranges - hostIP, err := netip.ParseAddr(u.Hostname()) - if err == nil { - hostIP = hostIP.Unmap() - - for _, cidrStr := range settings.InsecureAllowedCidrRanges { - cidr, err := netip.ParsePrefix(cidrStr) - if err != nil { - slog.WarnContext(context.Background(), "error parsing OIDC connector CIDR prefix", "cidr", cidrStr, "err", err) - continue - } - if cidr.Contains(hostIP) { - return nil - } - } - } - - if u.Scheme == "https" { - switch u.Port() { - default: - return trace.BadParameter("invalid port in client redirect URL") - case "", "443": - } - - for _, expression := range settings.AllowedHttpsHostnames { - ok, err := utils.MatchString(u.Hostname(), expression) - if err != nil { - slog.WarnContext(context.Background(), "error compiling OIDC connector allowed HTTPS hostname regex", "regex", expression, "err", err) - continue - } - if ok { - return nil - } - } - } - - return trace.AccessDenied("%s", unknownRedirectHostnameErrMsg) + return sso.ValidateClientRedirect(clientRedirect, ceremonyType, settings) } // populateGithubClaims builds a GithubClaims using queried diff --git a/lib/auth/github_test.go b/lib/auth/github_test.go index ad5ab2f35bf11..5dfe31c3aec40 100644 --- a/lib/auth/github_test.go +++ b/lib/auth/github_test.go @@ -43,7 +43,6 @@ import ( "github.com/gravitational/teleport/lib/authz" "github.com/gravitational/teleport/lib/backend" "github.com/gravitational/teleport/lib/backend/memory" - "github.com/gravitational/teleport/lib/client/sso" "github.com/gravitational/teleport/lib/events" "github.com/gravitational/teleport/lib/events/eventstest" "github.com/gravitational/teleport/lib/loginrule" @@ -611,207 +610,3 @@ func TestBuildAPIEndpoint(t *testing.T) { }) } } - -func TestValidateClientRedirect(t *testing.T) { - t.Run("StandardLocalhost", func(t *testing.T) { - for _, goodURL := range []string{ - "http://127.0.0.1/callback", - "http://127.0.0.1:1234/callback", - "http://[::1]/callback", - "http://[::1]:1234/callback", - "http://localhost/callback", - "http://localhost:1234/callback", - } { - const ssoTestFlowFalse = false - var defaultSettings *types.SSOClientRedirectSettings - require.NoError(t, ValidateClientRedirect(goodURL+"?secret_key=", ssoTestFlowFalse, defaultSettings)) - } - }) - - t.Run("InvalidLocalhostLike", func(t *testing.T) { - for _, badURL := range []string{ - "http://127.0.0.1:12345/notcallback", - "http://127a0.0.1/callback", - "http://127a0.0.1:1234/callback", - "http://::1/callback", - "http://notlocalhost/callback", - "http://sub.localhost/callback", - "http://localhost.com/callback", - "http://127.0.0.1.example.com/callback", - "http://[::1].example.com/callback", - "http://username@127.0.0.1:12345/callback", - "http://@localhost:12345/callback", - "http://localhost@example.com/callback", - "http://127.0.0.1:12345@example.com/callback", - "https://127.0.0.1:12345/callback", - "https://localhost:12345/callback", - "https://localhost/callback", - "ftp://localhost/callback", - "ftp://127.0.0.1/callback", - "ftp://[::1]/callback", - } { - const ssoTestFlowFalse = false - var defaultSettings *types.SSOClientRedirectSettings - require.Error(t, ValidateClientRedirect(badURL+"?secret_key=", ssoTestFlowFalse, defaultSettings)) - } - }) - - t.Run("BadQuery", func(t *testing.T) { - for _, badURL := range []string{ - "http://127.0.0.1:12345/callback", - "http://127.0.0.1:12345/callback?secret=a", - "http://127.0.0.1:12345/callback?secret_key=a&foo=b", - } { - const ssoTestFlowFalse = false - var defaultSettings *types.SSOClientRedirectSettings - require.Error(t, ValidateClientRedirect(badURL, ssoTestFlowFalse, defaultSettings)) - } - }) - - t.Run("AllowedHttpsHostnames", func(t *testing.T) { - for _, goodURL := range []string{ - "https://allowed.domain.invalid/callback", - "https://foo.allowed.with.subdomain.invalid/callback", - "https://but.no.subsubdomain.invalid/callback", - } { - const ssoTestFlowFalse = false - settings := &types.SSOClientRedirectSettings{ - AllowedHttpsHostnames: []string{ - "allowed.domain.invalid", - "*.allowed.with.subdomain.invalid", - "^[-a-zA-Z0-9]+.no.subsubdomain.invalid$", - }, - } - require.NoError(t, ValidateClientRedirect(goodURL+"?secret_key=", ssoTestFlowFalse, settings)) - } - - for _, badURL := range []string{ - "https://allowed.domain.invalid/notcallback", - "https://allowed.domain.invalid:12345/callback", - "http://allowed.domain.invalid/callback", - "https://not.allowed.domain.invalid/callback", - "https://allowed.with.subdomain.invalid/callback", - "https://i.said.no.subsubdomain.invalid/callback", - } { - const ssoTestFlowFalse = false - settings := &types.SSOClientRedirectSettings{ - AllowedHttpsHostnames: []string{ - "allowed.domain.invalid", - "*.allowed.with.subdomain.invalid", - "^[-a-zA-Z0-9]+.no.subsubdomain.invalid", - }, - } - require.Error(t, ValidateClientRedirect(badURL+"?secret_key=", ssoTestFlowFalse, settings)) - } - }) - - t.Run("InsecureAllowedCidrRanges", func(t *testing.T) { - for _, goodURL := range []string{ - "http://192.168.0.27/callback", - "https://192.168.0.27/callback", - "http://192.168.0.27:1337/callback", - "https://192.168.0.27:1337/callback", - "http://[2001:db8::aaaa:bbbb]/callback", - "https://[2001:db8::aaaa:bbbb]/callback", - "http://[2001:db8::aaaa:bbbb]:1337/callback", - "https://[2001:db8::aaaa:bbbb]:1337/callback", - "http://[2001:db8::1]/callback", - "https://[2001:db8::1]/callback", - "http://[2001:db8::1]:1337/callback", - "https://[2001:db8::1]:1337/callback", - } { - const ssoTestFlowFalse = false - settings := &types.SSOClientRedirectSettings{ - InsecureAllowedCidrRanges: []string{ - "192.168.0.0/24", - "2001:db8::/96", - }, - } - require.NoError(t, ValidateClientRedirect(goodURL+"?secret_key=", ssoTestFlowFalse, settings)) - } - - for _, badURL := range []string{ - "http://192.168.1.1/callback", - "https://192.168.1.1/callback", - "http://192.168.1.1:80/callback", - "https://192.168.1.1:443/callback", - "http://[2001:db8::1:aaaa:bbbb]/callback", - "https://[2001:db8::1:aaaa:bbbb]/callback", - "http://[2001:db8::1:aaaa:bbbb]:80/callback", - "https://[2001:db8::1:aaaa:bbbb]:443/callback", - "http://[2001:db9::]/callback", - "https://[2001:db9::]/callback", - "http://not.an.ip/callback", - "https://not.an.ip/callback", - "http://192.168.0.27/nocallback", - "https://192.168.0.27/nocallback", - "http://[2001:db8::1]/notacallback", - "https://[2001:db8::1]/notacallback", - } { - const ssoTestFlowFalse = false - settings := &types.SSOClientRedirectSettings{ - InsecureAllowedCidrRanges: []string{ - "192.168.0.0/24", - "2001:db8::/96", - }, - } - require.Error(t, ValidateClientRedirect(badURL+"?secret_key=", ssoTestFlowFalse, settings)) - } - }) - - t.Run("SSOTestFlow", func(t *testing.T) { - for _, goodURL := range []string{ - "http://127.0.0.1:12345/callback", - } { - const ssoTestFlowTrue = true - settings := &types.SSOClientRedirectSettings{ - AllowedHttpsHostnames: []string{ - "allowed.domain.invalid", - }, - } - require.NoError(t, ValidateClientRedirect(goodURL+"?secret_key=", ssoTestFlowTrue, settings)) - } - - for _, badURL := range []string{ - "https://allowed.domain.invalid/callback", - "http://allowed.domain.invalid/callback", - } { - const ssoTestFlowTrue = true - settings := &types.SSOClientRedirectSettings{ - AllowedHttpsHostnames: []string{ - "allowed.domain.invalid", - }, - } - require.Error(t, ValidateClientRedirect(badURL+"?secret_key=", ssoTestFlowTrue, settings)) - } - }) - - t.Run("SSOMFAWeb", func(t *testing.T) { - const ssoTestFlowFalse = false - settings := &types.SSOClientRedirectSettings{ - AllowedHttpsHostnames: []string{ - "allowed.domain.invalid", - }, - } - - // Only allow web mfa redirect as a relative path. - require.NoError(t, ValidateClientRedirect(sso.WebMFARedirect+"?channel_id=", ssoTestFlowFalse, settings)) - - for _, badURL := range []string{ - "localhost:12345" + sso.WebMFARedirect, - "http://localhost:12345" + sso.WebMFARedirect, - "https://localhost:12345" + sso.WebMFARedirect, - "127.0.0.1:12345" + sso.WebMFARedirect, - "http://127.0.0.1:12345" + sso.WebMFARedirect, - "https://127.0.0.1:12345" + sso.WebMFARedirect, - "allowed.domain.invalid" + sso.WebMFARedirect, - "http://allowed.domain.invalid" + sso.WebMFARedirect, - "https://allowed.domain.invalid" + sso.WebMFARedirect, - "not.allowed.domain.invalid" + sso.WebMFARedirect, - "http://not.allowed.domain.invalid" + sso.WebMFARedirect, - "https://not.allowed.domain.invalid" + sso.WebMFARedirect, - } { - require.Error(t, ValidateClientRedirect(badURL+"?channel_id=", ssoTestFlowFalse, settings)) - } - }) -} diff --git a/lib/client/sso/redirector.go b/lib/client/sso/redirector.go index 24310ee97169d..4f1be8324b8dc 100644 --- a/lib/client/sso/redirector.go +++ b/lib/client/sso/redirector.go @@ -28,10 +28,12 @@ import ( "net" "net/http" "net/http/httptest" + "net/netip" "net/url" "os" "os/exec" "runtime" + "slices" "strings" "time" @@ -41,6 +43,7 @@ import ( "github.com/gravitational/teleport" "github.com/gravitational/teleport/api/constants" apidefaults "github.com/gravitational/teleport/api/defaults" + "github.com/gravitational/teleport/api/types" apiutils "github.com/gravitational/teleport/api/utils" "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/lib/auth/authclient" @@ -502,3 +505,153 @@ func (rd *Redirector) wrapCallback(fn func(http.ResponseWriter, *http.Request) ( } }) } + +// CeremonyType is the type of SSO ceremony. +type CeremonyType int + +const ( + // CeremonyTypeLogin ceremonies are performed during standard SSO login flows. + CeremonyTypeLogin CeremonyType = iota + // CeremonyTypeMFA ceremonies are performed during MFA flows for SSO users if SSO MFA + // is enabled for their origin SSO connector. + CeremonyTypeMFA + // CeremonyTypeTest ceremonies are performed by administrators with "tctl sso test". + CeremonyTypeTest +) + +// ValidateClientRedirect validates a redirect URL provided by the client to +// consume the SSO response at the end of the SSO Ceremony. This validation +// differs depending on the ceremony type. Some checks are auth connector +// specific and others depend on the ceremony type (MFA, Login, or Test). +// +// For Web MFA ceremonies, the redirect URL should be [WebMFARedirect] as a relative +// path with no custom values outside of query parameters. +// +// For Login, Test, and non-web MFA ceremonies, the redirect URL should have: +// - a path of "/callback" +// - "http" scheme +// - a hostname of "localhost", "127.0.0.1", or "::1" +// - any port +// +// For non test ceremonies, If "allowed_https_hostnames" list is non-empty, +// the redirect URL can instead have: +// - a path of "/callback" +// - "https" scheme +// - a hostname that matches one in the "allowed_https_hostnames" list +// - either empty or 443 port +// +// For non test ceremonies, If the "insecure_allowed_cidr_ranges" list is non-empty, URLs in both the +// "http" and "https" schema are allowed if the hostname is an IP address that is contained in a +// specified CIDR range on any port. +func ValidateClientRedirect(clientRedirect string, ceremonyType CeremonyType, settings *types.SSOClientRedirectSettings) error { + // Warning to developers and reviewers: this validation function is critical to sso security + // and any changes to it should be carefully considered from a vulnerability point of view. + + if clientRedirect == "" { + // empty redirects are non-functional and harmless, so we allow them as + // they're used a lot in test code + return nil + } + + u, err := url.Parse(clientRedirect) + if err != nil { + return trace.Wrap(err, "parsing client redirect URL") + } + + if u.Opaque != "" { + return trace.BadParameter("unexpected opaque client redirect URL") + } + if u.User != nil { + return trace.BadParameter("unexpected userinfo in client redirect URL") + } + if u.Fragment != "" || u.RawFragment != "" { + return trace.BadParameter("unexpected fragment in client redirect URL") + } + + // For Web MFA redirect, we expect a relative path. The proxy handling the SSO callback + // will redirect to itself with this relative path. + if u.Path == WebMFARedirect { + if ceremonyType != CeremonyTypeMFA { + return trace.BadParameter("the web mfa redirect path, \"/web/sso_confirm\", cannot be used for non-MFA flows") + } + if u.IsAbs() { + return trace.BadParameter("invalid client redirect URL for SSO MFA") + } + if u.Hostname() != "" { + return trace.BadParameter("invalid client redirect URL for SSO MFA") + } + if q, err := url.ParseQuery(u.RawQuery); err != nil { + return trace.Wrap(err, "parsing query in client redirect URL") + } else if len(q) != 1 || len(q["channel_id"]) != 1 { + return trace.BadParameter("malformed query parameters in client redirect URL") + } + return nil + } + + if u.EscapedPath() != "/callback" { + return trace.BadParameter("invalid path in client redirect URL") + } + if q, err := url.ParseQuery(u.RawQuery); err != nil { + return trace.Wrap(err, "parsing query in client redirect URL") + } else if len(q) != 1 || len(q["secret_key"]) != 1 { + return trace.BadParameter("malformed query parameters in client redirect URL") + } + + // we checked everything but u.Scheme and u.Host now + if u.Scheme != "http" && u.Scheme != "https" { + return trace.BadParameter("invalid scheme in client redirect URL") + } + + // allow HTTP redirects to local addresses + allowedHTTPLocalAddrs := []string{"localhost", "127.0.0.1", "::1"} + if u.Scheme == "http" && slices.Contains(allowedHTTPLocalAddrs, u.Hostname()) { + return nil + } + + if ceremonyType == CeremonyTypeTest { + return trace.AccessDenied("custom client redirect URLs are not allowed in SSO test") + } + + const unknownRedirectHostnameErrMsg = "unknown custom client redirect URL hostname" + if settings == nil { + return trace.AccessDenied("%s", unknownRedirectHostnameErrMsg) + } + + // allow HTTP or HTTPS redirects from IPs in specified CIDR ranges + hostIP, err := netip.ParseAddr(u.Hostname()) + if err == nil { + hostIP = hostIP.Unmap() + + for _, cidrStr := range settings.InsecureAllowedCidrRanges { + cidr, err := netip.ParsePrefix(cidrStr) + if err != nil { + slog.WarnContext(context.Background(), "error parsing OIDC connector CIDR prefix", "cidr", cidrStr, "err", err) + continue + } + if cidr.Contains(hostIP) { + return nil + } + } + } + + if u.Scheme == "https" { + switch u.Port() { + default: + return trace.BadParameter("invalid port in client redirect URL") + case "", "443": + } + + for _, expression := range settings.AllowedHttpsHostnames { + ok, err := utils.MatchString(u.Hostname(), expression) + if err != nil { + slog.WarnContext(context.Background(), "error compiling OIDC connector allowed HTTPS hostname regex", "regex", expression, "err", err) + continue + } + if ok { + return nil + } + } + } + + return trace.AccessDenied("%s", unknownRedirectHostnameErrMsg) +} diff --git a/lib/client/sso/redirector_test.go b/lib/client/sso/redirector_test.go index e684caf2e98ff..f3226356eee7f 100644 --- a/lib/client/sso/redirector_test.go +++ b/lib/client/sso/redirector_test.go @@ -21,6 +21,7 @@ package sso_test import ( "context" "errors" + "fmt" "io" "net/http" "net/http/httptest" @@ -30,6 +31,7 @@ import ( "github.com/stretchr/testify/require" + "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/lib/client/sso" "github.com/gravitational/teleport/lib/web" @@ -244,6 +246,259 @@ func TestRedirector(t *testing.T) { } } +func TestValidateClientRedirect(t *testing.T) { + t.Run("StandardLocalhost", func(t *testing.T) { + for _, goodURL := range []string{ + "http://127.0.0.1/callback", + "http://127.0.0.1:1234/callback", + "http://[::1]/callback", + "http://[::1]:1234/callback", + "http://localhost/callback", + "http://localhost:1234/callback", + } { + for name, ceremonyType := range map[string]sso.CeremonyType{ + "Login": sso.CeremonyTypeLogin, + "MFA": sso.CeremonyTypeMFA, + "Test": sso.CeremonyTypeTest, + } { + t.Run(fmt.Sprintf("Ceremony%v", name), func(t *testing.T) { + var defaultSettings *types.SSOClientRedirectSettings + require.NoError(t, sso.ValidateClientRedirect(goodURL+"?secret_key=", ceremonyType, defaultSettings)) + }) + } + } + }) + + t.Run("InvalidLocalhostLike", func(t *testing.T) { + for _, badURL := range []string{ + "http://127.0.0.1:12345/notcallback", + "http://127a0.0.1/callback", + "http://127a0.0.1:1234/callback", + "http://::1/callback", + "http://notlocalhost/callback", + "http://sub.localhost/callback", + "http://localhost.com/callback", + "http://127.0.0.1.example.com/callback", + "http://[::1].example.com/callback", + "http://username@127.0.0.1:12345/callback", + "http://@localhost:12345/callback", + "http://localhost@example.com/callback", + "http://127.0.0.1:12345@example.com/callback", + "https://127.0.0.1:12345/callback", + "https://localhost:12345/callback", + "https://localhost/callback", + "ftp://localhost/callback", + "ftp://127.0.0.1/callback", + "ftp://[::1]/callback", + } { + for name, ceremonyType := range map[string]sso.CeremonyType{ + "Login": sso.CeremonyTypeLogin, + "MFA": sso.CeremonyTypeMFA, + "Test": sso.CeremonyTypeTest, + } { + t.Run(fmt.Sprintf("Ceremony%v", name), func(t *testing.T) { + var defaultSettings *types.SSOClientRedirectSettings + require.Error(t, sso.ValidateClientRedirect(badURL+"?secret_key=", ceremonyType, defaultSettings)) + }) + } + } + }) + + t.Run("BadQuery", func(t *testing.T) { + for _, badURL := range []string{ + "http://127.0.0.1:12345/callback", + "http://127.0.0.1:12345/callback?secret=a", + "http://127.0.0.1:12345/callback?secret_key=a&foo=b", + } { + for name, ceremonyType := range map[string]sso.CeremonyType{ + "Login": sso.CeremonyTypeLogin, + "MFA": sso.CeremonyTypeMFA, + "Test": sso.CeremonyTypeTest, + } { + t.Run(fmt.Sprintf("Ceremony%v", name), func(t *testing.T) { + var defaultSettings *types.SSOClientRedirectSettings + require.Error(t, sso.ValidateClientRedirect(badURL, ceremonyType, defaultSettings)) + }) + } + } + }) + + t.Run("AllowedHttpsHostnames", func(t *testing.T) { + for _, goodURL := range []string{ + "https://allowed.domain.invalid/callback", + "https://foo.allowed.with.subdomain.invalid/callback", + "https://but.no.subsubdomain.invalid/callback", + } { + settings := &types.SSOClientRedirectSettings{ + AllowedHttpsHostnames: []string{ + "allowed.domain.invalid", + "*.allowed.with.subdomain.invalid", + "^[-a-zA-Z0-9]+.no.subsubdomain.invalid$", + }, + } + for name, ceremonyType := range map[string]sso.CeremonyType{ + "Login": sso.CeremonyTypeLogin, + "MFA": sso.CeremonyTypeMFA, + } { + t.Run(fmt.Sprintf("Ceremony%v", name), func(t *testing.T) { + require.NoError(t, sso.ValidateClientRedirect(goodURL+"?secret_key=", ceremonyType, settings)) + }) + } + } + + for _, badURL := range []string{ + "https://allowed.domain.invalid/notcallback", + "https://allowed.domain.invalid:12345/callback", + "http://allowed.domain.invalid/callback", + "https://not.allowed.domain.invalid/callback", + "https://allowed.with.subdomain.invalid/callback", + "https://i.said.no.subsubdomain.invalid/callback", + } { + settings := &types.SSOClientRedirectSettings{ + AllowedHttpsHostnames: []string{ + "allowed.domain.invalid", + "*.allowed.with.subdomain.invalid", + "^[-a-zA-Z0-9]+.no.subsubdomain.invalid", + }, + } + for name, ceremonyType := range map[string]sso.CeremonyType{ + "Login": sso.CeremonyTypeLogin, + "MFA": sso.CeremonyTypeMFA, + } { + t.Run(fmt.Sprintf("Ceremony%v", name), func(t *testing.T) { + require.Error(t, sso.ValidateClientRedirect(badURL+"?secret_key=", ceremonyType, settings)) + }) + } + } + }) + + t.Run("InsecureAllowedCidrRanges", func(t *testing.T) { + for _, goodURL := range []string{ + "http://192.168.0.27/callback", + "https://192.168.0.27/callback", + "http://192.168.0.27:1337/callback", + "https://192.168.0.27:1337/callback", + "http://[2001:db8::aaaa:bbbb]/callback", + "https://[2001:db8::aaaa:bbbb]/callback", + "http://[2001:db8::aaaa:bbbb]:1337/callback", + "https://[2001:db8::aaaa:bbbb]:1337/callback", + "http://[2001:db8::1]/callback", + "https://[2001:db8::1]/callback", + "http://[2001:db8::1]:1337/callback", + "https://[2001:db8::1]:1337/callback", + } { + settings := &types.SSOClientRedirectSettings{ + InsecureAllowedCidrRanges: []string{ + "192.168.0.0/24", + "2001:db8::/96", + }, + } + for name, ceremonyType := range map[string]sso.CeremonyType{ + "Login": sso.CeremonyTypeLogin, + "MFA": sso.CeremonyTypeMFA, + } { + t.Run(fmt.Sprintf("Ceremony%v", name), func(t *testing.T) { + require.NoError(t, sso.ValidateClientRedirect(goodURL+"?secret_key=", ceremonyType, settings)) + }) + } + } + + for _, badURL := range []string{ + "http://192.168.1.1/callback", + "https://192.168.1.1/callback", + "http://192.168.1.1:80/callback", + "https://192.168.1.1:443/callback", + "http://[2001:db8::1:aaaa:bbbb]/callback", + "https://[2001:db8::1:aaaa:bbbb]/callback", + "http://[2001:db8::1:aaaa:bbbb]:80/callback", + "https://[2001:db8::1:aaaa:bbbb]:443/callback", + "http://[2001:db9::]/callback", + "https://[2001:db9::]/callback", + "http://not.an.ip/callback", + "https://not.an.ip/callback", + "http://192.168.0.27/nocallback", + "https://192.168.0.27/nocallback", + "http://[2001:db8::1]/notacallback", + "https://[2001:db8::1]/notacallback", + } { + settings := &types.SSOClientRedirectSettings{ + InsecureAllowedCidrRanges: []string{ + "192.168.0.0/24", + "2001:db8::/96", + }, + } + for name, ceremonyType := range map[string]sso.CeremonyType{ + "Login": sso.CeremonyTypeLogin, + "MFA": sso.CeremonyTypeMFA, + } { + t.Run(fmt.Sprintf("Ceremony%v", name), func(t *testing.T) { + require.Error(t, sso.ValidateClientRedirect(badURL+"?secret_key=", ceremonyType, settings)) + }) + } + } + }) + + t.Run("SSOTestFlow", func(t *testing.T) { + for _, goodURL := range []string{ + "http://127.0.0.1:12345/callback", + } { + settings := &types.SSOClientRedirectSettings{ + AllowedHttpsHostnames: []string{ + "allowed.domain.invalid", + }, + } + require.NoError(t, sso.ValidateClientRedirect(goodURL+"?secret_key=", sso.CeremonyTypeTest, settings)) + } + + for _, badURL := range []string{ + "https://allowed.domain.invalid/callback", + "http://allowed.domain.invalid/callback", + } { + settings := &types.SSOClientRedirectSettings{ + AllowedHttpsHostnames: []string{ + "allowed.domain.invalid", + }, + } + require.Error(t, sso.ValidateClientRedirect(badURL+"?secret_key=", sso.CeremonyTypeTest, settings)) + } + }) + + t.Run("SSOMFAWeb", func(t *testing.T) { + settings := &types.SSOClientRedirectSettings{ + AllowedHttpsHostnames: []string{ + "allowed.domain.invalid", + }, + } + + // Only allow web mfa redirect as a relative path. + require.NoError(t, sso.ValidateClientRedirect(sso.WebMFARedirect+"?channel_id=", sso.CeremonyTypeMFA, settings)) + + for _, badURL := range []string{ + "localhost:12345" + sso.WebMFARedirect, + "http://localhost:12345" + sso.WebMFARedirect, + "https://localhost:12345" + sso.WebMFARedirect, + "127.0.0.1:12345" + sso.WebMFARedirect, + "http://127.0.0.1:12345" + sso.WebMFARedirect, + "https://127.0.0.1:12345" + sso.WebMFARedirect, + "allowed.domain.invalid" + sso.WebMFARedirect, + "http://allowed.domain.invalid" + sso.WebMFARedirect, + "https://allowed.domain.invalid" + sso.WebMFARedirect, + "not.allowed.domain.invalid" + sso.WebMFARedirect, + "http://not.allowed.domain.invalid" + sso.WebMFARedirect, + "https://not.allowed.domain.invalid" + sso.WebMFARedirect, + } { + require.Error(t, sso.ValidateClientRedirect(badURL+"?channel_id=", sso.CeremonyTypeMFA, settings)) + } + + // channel_id query parameter must be set. + require.Error(t, sso.ValidateClientRedirect(sso.WebMFARedirect, sso.CeremonyTypeMFA, settings)) + + // Don't allow web mfa redirect for non-mfa ceremonies. + require.Error(t, sso.ValidateClientRedirect(sso.WebMFARedirect+"?channel_id=", sso.CeremonyTypeLogin, settings)) + require.Error(t, sso.ValidateClientRedirect(sso.WebMFARedirect+"?channel_id=", sso.CeremonyTypeTest, settings)) + }) +} + // create a mock proxy server which echos the final proxy redirect destination page. e.g. sso success page. func newMockProxy(t *testing.T) *httptest.Server { mux := http.NewServeMux()