From ab673e0cb7d2756f95d9cf019dc385a348ca911e Mon Sep 17 00:00:00 2001 From: Austin Gebauer Date: Wed, 27 May 2020 14:19:48 -0700 Subject: [PATCH 1/4] fix: use mount from path parameter given during authn --- backend_test.go | 2 +- cli.go | 7 ++++++- path_login.go | 4 ++-- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/backend_test.go b/backend_test.go index 87b59c2..dc7c62d 100644 --- a/backend_test.go +++ b/backend_test.go @@ -225,7 +225,7 @@ func TestBackEnd_ValidateInstancePrincipalLoginNonExistentRole(t *testing.T) { func makeRequestAndValidateResponse(t *testing.T, cmdMap map[string]string, expectFailure bool, expectedTTL time.Duration, expectedPolicies []string) { role := cmdMap["role"] - path := fmt.Sprintf(PathBaseFormat, role) + path := fmt.Sprintf(PathBaseFormat, "oci", role) signingPath := PathVersionBase + path backend, config, err := initTest(t) diff --git a/cli.go b/cli.go index 8f3b4ff..e25f81f 100644 --- a/cli.go +++ b/cli.go @@ -50,13 +50,18 @@ Configuration: } func (h *CLIHandler) Auth(c *api.Client, m map[string]string) (*api.Secret, error) { + mount, ok := m["mount"] + if !ok { + mount = "oci" + } + role, ok := m["role"] if !ok { return nil, fmt.Errorf("Enter the role") } role = strings.ToLower(role) - path := fmt.Sprintf(PathBaseFormat, role) + path := fmt.Sprintf(PathBaseFormat, mount, role) signingPath := PathVersionBase + path loginData, err := CreateLoginData(c.Address(), m, signingPath) diff --git a/path_login.go b/path_login.go index 6484f13..4f02413 100644 --- a/path_login.go +++ b/path_login.go @@ -17,7 +17,7 @@ import ( // These constants store the required http path & method information for validating the signed request const ( PathVersionBase = "/v1" - PathBaseFormat = "/auth/oci/login/%s" + PathBaseFormat = "/auth/%s/login/%s" PathLoginMethod = "get" ) @@ -78,7 +78,7 @@ func (b *backend) pathLoginUpdate(ctx context.Context, req *logical.Request, dat authenticateRequestHeaders := requestHeaders.(http.Header) // Find the targetUrl and Method - finalLoginPath := PathVersionBase + fmt.Sprintf(PathBaseFormat, roleName) + finalLoginPath := PathVersionBase + fmt.Sprintf(PathBaseFormat, "oci", roleName) method, targetUrl, err := requestTargetToMethodURL(authenticateRequestHeaders[HdrRequestTarget], PathLoginMethod, finalLoginPath) if err != nil { return unauthorizedLogicalResponse(req, b.Logger(), err) From 8a7b2247db3581dd43e5fffa48715be980db8056 Mon Sep 17 00:00:00 2001 From: Austin Gebauer Date: Fri, 29 May 2020 14:45:37 -0700 Subject: [PATCH 2/4] fix: use regex for mount path segment matching --- path_login.go | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/path_login.go b/path_login.go index 4f02413..cc44288 100644 --- a/path_login.go +++ b/path_login.go @@ -4,21 +4,24 @@ package ociauth import ( "context" "fmt" + "net/http" + "regexp" + "strings" + "unicode" + log "github.com/hashicorp/go-hclog" "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/logical" "github.com/oracle/oci-go-sdk/common" "github.com/pkg/errors" - "net/http" - "strings" - "unicode" ) // These constants store the required http path & method information for validating the signed request const ( - PathVersionBase = "/v1" - PathBaseFormat = "/auth/%s/login/%s" - PathLoginMethod = "get" + PathVersionBase = "/v1" + PathBaseFormat = "/auth/%s/login/%s" + PathLoginMethod = "get" + URLMountSegmentRegex = "[[:ascii:]]+" ) // Signing Header constants @@ -78,8 +81,9 @@ func (b *backend) pathLoginUpdate(ctx context.Context, req *logical.Request, dat authenticateRequestHeaders := requestHeaders.(http.Header) // Find the targetUrl and Method - finalLoginPath := PathVersionBase + fmt.Sprintf(PathBaseFormat, "oci", roleName) - method, targetUrl, err := requestTargetToMethodURL(authenticateRequestHeaders[HdrRequestTarget], PathLoginMethod, finalLoginPath) + finalLoginPathRegex := fmt.Sprintf("^%s%s$", PathVersionBase, + fmt.Sprintf(PathBaseFormat, URLMountSegmentRegex, roleName)) + method, targetUrl, err := requestTargetToMethodURL(authenticateRequestHeaders[HdrRequestTarget], PathLoginMethod, finalLoginPathRegex) if err != nil { return unauthorizedLogicalResponse(req, b.Logger(), err) } @@ -213,12 +217,13 @@ func unauthorizedLogicalResponse(req *logical.Request, logger log.Logger, err er return logical.RespondWithStatusCode(nil, req, http.StatusUnauthorized) } -func requestTargetToMethodURL(requestTarget []string, expectedMethod string, expectedUrl string) (method string, url string, err error) { +func requestTargetToMethodURL(requestTarget []string, expectedMethod string, expectedURLRegex string) (method string, url string, err error) { if len(requestTarget) == 0 { return "", "", errors.New("no (request-target) specified in header") } parts := strings.FieldsFunc(requestTarget[0], unicode.IsSpace) - if len(parts) != 2 || strings.ToLower(parts[0]) != expectedMethod || strings.ToLower(parts[1]) != expectedUrl { + match, err := regexp.MatchString(expectedURLRegex, strings.ToLower(parts[1])) + if len(parts) != 2 || strings.ToLower(parts[0]) != expectedMethod || err != nil || !match { return "", "", errors.New("incorrect (request-target) specified in header") } return parts[0], parts[1], nil From 4b0bd3555f2eefaccce7b4fe08f86c2d3be70aab Mon Sep 17 00:00:00 2001 From: Austin Gebauer Date: Mon, 1 Jun 2020 12:11:08 -0700 Subject: [PATCH 3/4] fix: check path segments using url path split --- path_login.go | 39 +++++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/path_login.go b/path_login.go index cc44288..e616a7b 100644 --- a/path_login.go +++ b/path_login.go @@ -5,7 +5,6 @@ import ( "context" "fmt" "net/http" - "regexp" "strings" "unicode" @@ -18,10 +17,12 @@ import ( // These constants store the required http path & method information for validating the signed request const ( - PathVersionBase = "/v1" - PathBaseFormat = "/auth/%s/login/%s" - PathLoginMethod = "get" - URLMountSegmentRegex = "[[:ascii:]]+" + PathVersionBase = "/v1" + PathBaseFormat = "/auth/%s/login/%s" + PathLoginMethod = "get" + PathSegmentAuth = "auth" + PathSegmentLogin = "login" + PathSegmentVersion = "v1" ) // Signing Header constants @@ -81,9 +82,7 @@ func (b *backend) pathLoginUpdate(ctx context.Context, req *logical.Request, dat authenticateRequestHeaders := requestHeaders.(http.Header) // Find the targetUrl and Method - finalLoginPathRegex := fmt.Sprintf("^%s%s$", PathVersionBase, - fmt.Sprintf(PathBaseFormat, URLMountSegmentRegex, roleName)) - method, targetUrl, err := requestTargetToMethodURL(authenticateRequestHeaders[HdrRequestTarget], PathLoginMethod, finalLoginPathRegex) + method, targetUrl, err := requestTargetToMethodURL(authenticateRequestHeaders[HdrRequestTarget], roleName) if err != nil { return unauthorizedLogicalResponse(req, b.Logger(), err) } @@ -217,15 +216,31 @@ func unauthorizedLogicalResponse(req *logical.Request, logger log.Logger, err er return logical.RespondWithStatusCode(nil, req, http.StatusUnauthorized) } -func requestTargetToMethodURL(requestTarget []string, expectedMethod string, expectedURLRegex string) (method string, url string, err error) { +func requestTargetToMethodURL(requestTarget []string, roleName string) (method string, url string, err error) { if len(requestTarget) == 0 { return "", "", errors.New("no (request-target) specified in header") } + errHeader := errors.New("incorrect (request-target) specified in header") + + // Ensure both the request method and URL path are present in the (request-target) header parts := strings.FieldsFunc(requestTarget[0], unicode.IsSpace) - match, err := regexp.MatchString(expectedURLRegex, strings.ToLower(parts[1])) - if len(parts) != 2 || strings.ToLower(parts[0]) != expectedMethod || err != nil || !match { - return "", "", errors.New("incorrect (request-target) specified in header") + if len(parts) != 2 { + return "", "", errHeader + } + + // Validate the request method + if strings.ToLower(parts[0]) != PathLoginMethod { + return "", "", errHeader } + + // Validate the URL path by inspecting its segments. + // The path mount segment of the URL is not validated. + segments := strings.Split(parts[1], "/") + if len(segments) < 5 || segments[0] != PathSegmentVersion || segments[1] != PathSegmentAuth || + segments[len(segments)-2] != PathSegmentLogin || segments[len(segments)-1] != roleName { + return "", "", errHeader + } + return parts[0], parts[1], nil } From b1222fd25e071f936b79c66a3a2c8a1186db73d7 Mon Sep 17 00:00:00 2001 From: Austin Gebauer Date: Wed, 10 Jun 2020 20:34:41 -0700 Subject: [PATCH 4/4] fix: format of request-target header and validation --- cli.go | 1 + path_login.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/cli.go b/cli.go index e25f81f..0966d66 100644 --- a/cli.go +++ b/cli.go @@ -54,6 +54,7 @@ func (h *CLIHandler) Auth(c *api.Client, m map[string]string) (*api.Secret, erro if !ok { mount = "oci" } + mount = strings.TrimSuffix(mount, "/") role, ok := m["role"] if !ok { diff --git a/path_login.go b/path_login.go index e616a7b..85e1f07 100644 --- a/path_login.go +++ b/path_login.go @@ -235,7 +235,7 @@ func requestTargetToMethodURL(requestTarget []string, roleName string) (method s // Validate the URL path by inspecting its segments. // The path mount segment of the URL is not validated. - segments := strings.Split(parts[1], "/") + segments := strings.Split(strings.TrimPrefix(parts[1], "/"), "/") if len(segments) < 5 || segments[0] != PathSegmentVersion || segments[1] != PathSegmentAuth || segments[len(segments)-2] != PathSegmentLogin || segments[len(segments)-1] != roleName { return "", "", errHeader