diff --git a/internal/servers/controller/handler.go b/internal/servers/controller/handler.go index 1e2f444d6..a013d9f5c 100644 --- a/internal/servers/controller/handler.go +++ b/internal/servers/controller/handler.go @@ -20,6 +20,7 @@ import ( "github.com/hashicorp/boundary/internal/servers/controller/handlers/sessions" "github.com/hashicorp/boundary/internal/servers/controller/handlers/targets" "github.com/hashicorp/boundary/sdk/strutil" + "github.com/hashicorp/go-cleanhttp" "github.com/hashicorp/shared-secure-libs/configutil" "github.com/hashicorp/boundary/internal/servers/controller/handlers" @@ -53,8 +54,9 @@ func (c *Controller) handler(props HandlerProperties) (http.Handler, error) { corsWrappedHandler := wrapHandlerWithCors(mux, props) commonWrappedHandler := wrapHandlerWithCommonFuncs(corsWrappedHandler, c, props) + printablePathCheckHandler := cleanhttp.PrintablePathCheckHandler(commonWrappedHandler, nil) - return commonWrappedHandler, nil + return printablePathCheckHandler, nil } func handleGrpcGateway(c *Controller, props HandlerProperties) (http.Handler, error) { diff --git a/internal/servers/controller/handler_test.go b/internal/servers/controller/handler_test.go index 2b372c8b2..c3497e05e 100644 --- a/internal/servers/controller/handler_test.go +++ b/internal/servers/controller/handler_test.go @@ -110,7 +110,8 @@ func TestHandleImplementedPaths(t *testing.T) { "v1/hosts/someid", "v1/roles", "v1/roles/someid", - "v1/scopes", + "400_v1/sc\u200Bopes", + "200_v1/scopes", "v1/scopes/someid", "v1/sessions/", "v1/sessions/someid", @@ -185,11 +186,28 @@ func TestHandleImplementedPaths(t *testing.T) { } { for _, p := range paths { t.Run(fmt.Sprintf("%s/%s", verb, p), func(t *testing.T) { + + var expCode int + if !strings.HasPrefix(p, "v1/") { + sp := strings.Split(p, "_") + require.Len(t, sp, 2) + switch sp[0] { + case "400": + expCode = http.StatusBadRequest + case "200": + expCode = http.StatusOK + } + p = sp[1] + } + url := fmt.Sprintf("%s/%s", c.ApiAddrs()[0], p) req, err := http.NewRequest(verb, url, nil) require.NoError(t, err) resp, err := http.DefaultClient.Do(req) require.NoError(t, err) + if expCode != 0 { + assert.Equal(t, expCode, resp.StatusCode) + } assert.NotEqualf(t, resp.StatusCode, http.StatusNotFound, "Got response %v, wanted not 404", resp.StatusCode) }) } diff --git a/internal/servers/controller/handlers/verifiers.go b/internal/servers/controller/handlers/verifiers.go index 0b48c2ea0..ba7633262 100644 --- a/internal/servers/controller/handlers/verifiers.go +++ b/internal/servers/controller/handlers/verifiers.go @@ -3,6 +3,7 @@ package handlers import ( "regexp" "strings" + "unicode" "github.com/golang/protobuf/ptypes/timestamp" "github.com/golang/protobuf/ptypes/wrappers" @@ -30,10 +31,26 @@ func ValidateCreateRequest(i ApiResource, fn CustomValidatorFunc) error { badFields["id"] = "This is a read only field." } if i.GetName() != nil { - i.GetName().Value = strings.TrimSpace(i.GetName().GetValue()) + trimmed := strings.TrimSpace(i.GetName().GetValue()) + switch { + case trimmed == "": + badFields["name"] = "Cannot set empty string as name" + case !ValidNameDescription(trimmed): + badFields["name"] = "Name contains unprintable characters" + default: + i.GetName().Value = trimmed + } } if i.GetDescription() != nil { - i.GetDescription().Value = strings.TrimSpace(i.GetDescription().GetValue()) + trimmed := strings.TrimSpace(i.GetDescription().GetValue()) + switch { + case trimmed == "": + badFields["description"] = "Cannot set empty string as description" + case !ValidNameDescription(trimmed): + badFields["description"] = "Description contains unprintable characters" + default: + i.GetDescription().Value = trimmed + } } if i.GetCreatedTime() != nil { badFields["created_time"] = "This is a read only field." @@ -74,17 +91,23 @@ func ValidateUpdateRequest(prefix string, r UpdateRequest, i ApiResource, fn Cus } if i.GetName() != nil { trimmed := strings.TrimSpace(i.GetName().GetValue()) - if trimmed == "" { + switch { + case trimmed == "": badFields["name"] = "Cannot set empty string as name" - } else { + case !ValidNameDescription(trimmed): + badFields["name"] = "Name contains unprintable characters" + default: i.GetName().Value = trimmed } } if i.GetDescription() != nil { trimmed := strings.TrimSpace(i.GetDescription().GetValue()) - if trimmed == "" { + switch { + case trimmed == "": badFields["description"] = "Cannot set empty string as description" - } else { + case !ValidNameDescription(trimmed): + badFields["description"] = "Description contains unprintable characters" + default: i.GetDescription().Value = trimmed } } @@ -157,3 +180,15 @@ func ValidId(prefix, id string) bool { id = strings.TrimPrefix(id, prefix) return !reInvalidID.Match([]byte(id)) } + +func ValidNameDescription(in string) bool { + idx := strings.IndexFunc(in, func(c rune) bool { + return !unicode.IsPrint(c) + }) + + if idx != -1 { + return false + } + + return true +} diff --git a/internal/servers/controller/handlers/verifiers_test.go b/internal/servers/controller/handlers/verifiers_test.go index 568cf71de..4514bbf80 100644 --- a/internal/servers/controller/handlers/verifiers_test.go +++ b/internal/servers/controller/handlers/verifiers_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/require" "google.golang.org/protobuf/types/known/fieldmaskpb" "google.golang.org/protobuf/types/known/timestamppb" + "google.golang.org/protobuf/types/known/wrapperspb" ) func errorIncludesFields(t *testing.T, err error, wantFields []string) { @@ -38,6 +39,11 @@ func TestValidId(t *testing.T) { assert.False(t, ValidId("short", "prefix_short")) } +func TestValidNameDescription(t *testing.T) { + assert.True(t, ValidNameDescription("foobar")) + assert.False(t, ValidNameDescription("foo\u200Bbar")) +} + func TestValidateGetRequest(t *testing.T) { cases := []struct { name string @@ -228,6 +234,36 @@ func TestValidateCreateRequest(t *testing.T) { }, badFields: []string{"id", "created_time", "version", "test"}, }, + { + name: "good name", + item: &pb.User{ + Name: wrapperspb.String("foobar"), + }, + valFn: NoopValidatorFn, + }, + { + name: "bad name", + item: &pb.User{ + Name: wrapperspb.String("foo\u200Bbar"), + }, + valFn: NoopValidatorFn, + badFields: []string{"name"}, + }, + { + name: "good description", + item: &pb.User{ + Description: wrapperspb.String("foobar"), + }, + valFn: NoopValidatorFn, + }, + { + name: "bad description", + item: &pb.User{ + Description: wrapperspb.String("foo\u200Bbar"), + }, + valFn: NoopValidatorFn, + badFields: []string{"description"}, + }, } for _, tc := range cases { @@ -290,6 +326,60 @@ func TestValidateUpdateRequest(t *testing.T) { valFn: NoopValidatorFn, badFields: []string{"id"}, }, + { + name: "good name", + prefix: "prefix", + req: &pbs.UpdateUserRequest{ + Id: "prefix_something", + UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"anything"}}, + }, + item: &pb.User{ + Version: 1, + Name: wrapperspb.String("foobar"), + }, + valFn: NoopValidatorFn, + }, + { + name: "bad name", + prefix: "prefix", + req: &pbs.UpdateUserRequest{ + Id: "prefix_something", + UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"anything"}}, + }, + item: &pb.User{ + Version: 1, + Name: wrapperspb.String("foo\u200Bbar"), + }, + valFn: NoopValidatorFn, + badFields: []string{"name"}, + }, + { + name: "good description", + prefix: "prefix", + req: &pbs.UpdateUserRequest{ + Id: "prefix_something", + UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"anything"}}, + }, + item: &pb.User{ + Version: 1, + Description: wrapperspb.String("foobar"), + }, + valFn: NoopValidatorFn, + }, + { + name: "bad description", + prefix: "prefix", + req: &pbs.UpdateUserRequest{ + Id: "prefix_something", + UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"anything"}}, + }, + item: &pb.User{ + Version: 1, + Description: wrapperspb.String("foo\u200Bbar"), + }, + valFn: NoopValidatorFn, + badFields: []string{"description"}, + }, { name: "missing version", prefix: "prefix",