From 4a66798d4df99e1ff578ad847927eea7bef6a4c9 Mon Sep 17 00:00:00 2001 From: paladin-devops Date: Tue, 1 Feb 2022 23:22:29 -0500 Subject: [PATCH 1/3] Add check for OIDC provider to permit a non-exact redirect URI from OIDC client if it is the IPv4 or IPv6 loopback address. --- changelog/13871.txt | 3 ++ vault/identity_store_oidc_provider.go | 3 +- vault/identity_store_oidc_provider_util.go | 49 ++++++++++++++++++++++ 3 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 changelog/13871.txt create mode 100644 vault/identity_store_oidc_provider_util.go diff --git a/changelog/13871.txt b/changelog/13871.txt new file mode 100644 index 000000000000..5878fba9d858 --- /dev/null +++ b/changelog/13871.txt @@ -0,0 +1,3 @@ +```release-note:bug +oidc: Support dynamic port for loopback redirect URI +``` diff --git a/vault/identity_store_oidc_provider.go b/vault/identity_store_oidc_provider.go index d3a7a260de4c..2800f446292e 100644 --- a/vault/identity_store_oidc_provider.go +++ b/vault/identity_store_oidc_provider.go @@ -1513,7 +1513,8 @@ func (i *IdentityStore) pathOIDCAuthorize(ctx context.Context, req *logical.Requ if redirectURI == "" { return authResponse("", state, ErrAuthInvalidRequest, "redirect_uri parameter is required") } - if !strutil.StrListContains(client.RedirectURIs, redirectURI) { + + if !strutil.StrListContains(client.RedirectURIs, redirectURI) && !isValidRedirectURI(redirectURI, client.RedirectURIs) { return authResponse("", state, ErrAuthInvalidRedirectURI, "redirect_uri is not allowed for the client") } diff --git a/vault/identity_store_oidc_provider_util.go b/vault/identity_store_oidc_provider_util.go new file mode 100644 index 000000000000..05df8f8a347f --- /dev/null +++ b/vault/identity_store_oidc_provider_util.go @@ -0,0 +1,49 @@ +package vault + +import ( + "net/url" + "regexp" + "strings" +) + +func isValidRedirectURI(uri string, validUris []string) bool { + requestedUri, err := url.Parse(uri) + if err != nil { + return false + } + + for _, validUri := range validUris { + if strings.ToLower(validUri) == strings.ToLower(uri) || isLoopbackURI(requestedUri, validUri) { + return true + } + } + + return false +} + +func isLoopbackURI(requestUri *url.URL, validUri string) bool { + registeredUri, err := url.Parse(validUri) + if err != nil { + return false + } + + // Verifies that the valid URL is HTTP and is the loopback address before + // proceeding, otherwise return false + if registeredUri.Scheme != "http" || !isLoopbackAddress(registeredUri.Host) { + return false + } + + // Returns true if the path after the IP/port is the same + // Request URL and valid URL have already been validated as loopback + if requestUri.Scheme == "http" && isLoopbackAddress(requestUri.Host) && registeredUri.Path == requestUri.Path { + return true + } + + return false +} + +// Returns true if the address hostname is the IPv4 or IPv6 loopback address and ignores port +func isLoopbackAddress(address string) bool { + match, _ := regexp.MatchString("^(127.0.0.1|\\[::1\\])(:?)(\\d*)$", address) + return match +} From 2af1aa00140eb01d8fcb867e50f7e829c26e396d Mon Sep 17 00:00:00 2001 From: Joe <83741749+paladin-devops@users.noreply.github.com> Date: Sun, 6 Feb 2022 01:24:11 -0500 Subject: [PATCH 2/3] Update changelog/13871.txt Co-authored-by: Austin Gebauer <34121980+austingebauer@users.noreply.github.com> --- changelog/13871.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/13871.txt b/changelog/13871.txt index 5878fba9d858..c8c5103bbf3b 100644 --- a/changelog/13871.txt +++ b/changelog/13871.txt @@ -1,3 +1,3 @@ ```release-note:bug -oidc: Support dynamic port for loopback redirect URI +identity/oidc: Adds support for port-agnostic validation of loopback IP redirect URIs. ``` From e7cdfc2174a704f07fe2d712aaf505fa465a9f65 Mon Sep 17 00:00:00 2001 From: paladin-devops Date: Sun, 6 Feb 2022 01:34:00 -0500 Subject: [PATCH 3/3] Update redirectURI check to match that for the OIDC auth method. --- vault/identity_store_oidc_provider.go | 2 +- vault/identity_store_oidc_provider_util.go | 51 +++++++++------------- 2 files changed, 21 insertions(+), 32 deletions(-) diff --git a/vault/identity_store_oidc_provider.go b/vault/identity_store_oidc_provider.go index 2800f446292e..e3580a9e16e7 100644 --- a/vault/identity_store_oidc_provider.go +++ b/vault/identity_store_oidc_provider.go @@ -1514,7 +1514,7 @@ func (i *IdentityStore) pathOIDCAuthorize(ctx context.Context, req *logical.Requ return authResponse("", state, ErrAuthInvalidRequest, "redirect_uri parameter is required") } - if !strutil.StrListContains(client.RedirectURIs, redirectURI) && !isValidRedirectURI(redirectURI, client.RedirectURIs) { + if !validRedirect(redirectURI, client.RedirectURIs) { return authResponse("", state, ErrAuthInvalidRedirectURI, "redirect_uri is not allowed for the client") } diff --git a/vault/identity_store_oidc_provider_util.go b/vault/identity_store_oidc_provider_util.go index 05df8f8a347f..f5b0dadb8a94 100644 --- a/vault/identity_store_oidc_provider_util.go +++ b/vault/identity_store_oidc_provider_util.go @@ -2,48 +2,37 @@ package vault import ( "net/url" - "regexp" - "strings" + + "github.com/hashicorp/go-secure-stdlib/strutil" ) -func isValidRedirectURI(uri string, validUris []string) bool { - requestedUri, err := url.Parse(uri) +// validRedirect checks whether uri is in allowed using special handling for loopback uris. +// Ref: https://tools.ietf.org/html/rfc8252#section-7.3 +func validRedirect(uri string, allowed []string) bool { + inputURI, err := url.Parse(uri) if err != nil { return false } - for _, validUri := range validUris { - if strings.ToLower(validUri) == strings.ToLower(uri) || isLoopbackURI(requestedUri, validUri) { - return true - } + // if uri isn't a loopback, just string search the allowed list + if !strutil.StrListContains([]string{"localhost", "127.0.0.1", "::1"}, inputURI.Hostname()) { + return strutil.StrListContains(allowed, uri) } - return false -} - -func isLoopbackURI(requestUri *url.URL, validUri string) bool { - registeredUri, err := url.Parse(validUri) - if err != nil { - return false - } + // otherwise, search for a match in a port-agnostic manner, per the OAuth RFC. + inputURI.Host = inputURI.Hostname() - // Verifies that the valid URL is HTTP and is the loopback address before - // proceeding, otherwise return false - if registeredUri.Scheme != "http" || !isLoopbackAddress(registeredUri.Host) { - return false - } + for _, a := range allowed { + allowedURI, err := url.Parse(a) + if err != nil { + return false + } + allowedURI.Host = allowedURI.Hostname() - // Returns true if the path after the IP/port is the same - // Request URL and valid URL have already been validated as loopback - if requestUri.Scheme == "http" && isLoopbackAddress(requestUri.Host) && registeredUri.Path == requestUri.Path { - return true + if inputURI.String() == allowedURI.String() { + return true + } } return false } - -// Returns true if the address hostname is the IPv4 or IPv6 loopback address and ignores port -func isLoopbackAddress(address string) bool { - match, _ := regexp.MatchString("^(127.0.0.1|\\[::1\\])(:?)(\\d*)$", address) - return match -}