From aa2d6e6e6c7434058c4b1a46d4354391ed4d96d0 Mon Sep 17 00:00:00 2001 From: Jakub Guzik Date: Fri, 19 Mar 2021 00:09:18 +0100 Subject: Fix volumes and networks list/prune filters in http api This is the continuation work started in #9711. It turns out that list/prune commands for volumes in libpod/compat api have very dangerous error handling when broken filter input is supplied. Problem also affects network list/prune in libpod. This commit unifies filter handling across libpod/compat api and adds sanity apiv2 testcases. Signed-off-by: Jakub Guzik --- pkg/api/handlers/compat/events.go | 54 ++-------------------------- pkg/api/handlers/compat/networks.go | 26 ++++---------- pkg/api/handlers/compat/volumes.go | 27 ++++---------- pkg/api/handlers/libpod/networks.go | 29 ++++++--------- pkg/api/handlers/libpod/volumes.go | 32 ++++++----------- pkg/util/filters.go | 70 +++++++++++++++++++++++++++++++++++++ test/apiv2/30-volumes.at | 20 +++++++++++ test/apiv2/35-networks.at | 22 +++++++++++- 8 files changed, 148 insertions(+), 132 deletions(-) diff --git a/pkg/api/handlers/compat/events.go b/pkg/api/handlers/compat/events.go index 9e82831d7..dd0a9e7a9 100644 --- a/pkg/api/handlers/compat/events.go +++ b/pkg/api/handlers/compat/events.go @@ -1,69 +1,19 @@ package compat import ( - "encoding/json" - "fmt" "net/http" "github.com/containers/podman/v3/libpod" "github.com/containers/podman/v3/libpod/events" "github.com/containers/podman/v3/pkg/api/handlers/utils" "github.com/containers/podman/v3/pkg/domain/entities" + "github.com/containers/podman/v3/pkg/util" "github.com/gorilla/schema" jsoniter "github.com/json-iterator/go" "github.com/pkg/errors" "github.com/sirupsen/logrus" ) -// filtersFromRequests extracts the "filters" parameter from the specified -// http.Request. The parameter can either be a `map[string][]string` as done -// in new versions of Docker and libpod, or a `map[string]map[string]bool` as -// done in older versions of Docker. We have to do a bit of Yoga to support -// both - just as Docker does as well. -// -// Please refer to https://github.com/containers/podman/issues/6899 for some -// background. -func filtersFromRequest(r *http.Request) ([]string, error) { - var ( - compatFilters map[string]map[string]bool - filters map[string][]string - libpodFilters []string - raw []byte - ) - - if _, found := r.URL.Query()["filters"]; found { - raw = []byte(r.Form.Get("filters")) - } else if _, found := r.URL.Query()["Filters"]; found { - raw = []byte(r.Form.Get("Filters")) - } else { - return []string{}, nil - } - - // Backwards compat with older versions of Docker. - if err := json.Unmarshal(raw, &compatFilters); err == nil { - for filterKey, filterMap := range compatFilters { - for filterValue, toAdd := range filterMap { - if toAdd { - libpodFilters = append(libpodFilters, fmt.Sprintf("%s=%s", filterKey, filterValue)) - } - } - } - return libpodFilters, nil - } - - if err := json.Unmarshal(raw, &filters); err != nil { - return nil, err - } - - for filterKey, filterSlice := range filters { - for _, filterValue := range filterSlice { - libpodFilters = append(libpodFilters, fmt.Sprintf("%s=%s", filterKey, filterValue)) - } - } - - return libpodFilters, nil -} - // NOTE: this endpoint serves both the docker-compatible one and the new libpod // one. func GetEvents(w http.ResponseWriter, r *http.Request) { @@ -92,7 +42,7 @@ func GetEvents(w http.ResponseWriter, r *http.Request) { fromStart = true } - libpodFilters, err := filtersFromRequest(r) + libpodFilters, err := util.FiltersFromRequest(r) if err != nil { utils.Error(w, "failed to parse parameters", http.StatusBadRequest, errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String())) return diff --git a/pkg/api/handlers/compat/networks.go b/pkg/api/handlers/compat/networks.go index 7e06cad66..77ed548d8 100644 --- a/pkg/api/handlers/compat/networks.go +++ b/pkg/api/handlers/compat/networks.go @@ -17,6 +17,7 @@ import ( "github.com/containers/podman/v3/pkg/domain/entities" "github.com/containers/podman/v3/pkg/domain/infra/abi" networkid "github.com/containers/podman/v3/pkg/network" + "github.com/containers/podman/v3/pkg/util" "github.com/docker/docker/api/types" dockerNetwork "github.com/docker/docker/api/types/network" "github.com/gorilla/schema" @@ -181,18 +182,12 @@ func findPluginByName(plugins []*libcni.NetworkConfig, pluginType string) ([]byt func ListNetworks(w http.ResponseWriter, r *http.Request) { runtime := r.Context().Value("runtime").(*libpod.Runtime) - filters, err := filtersFromRequest(r) + filterMap, err := util.PrepareFilters(r) if err != nil { - utils.Error(w, "Something went wrong.", http.StatusBadRequest, errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String())) + utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String())) return } - filterMap := map[string][]string{} - for _, filter := range filters { - split := strings.SplitN(filter, "=", 2) - if len(split) > 1 { - filterMap[split[0]] = append(filterMap[split[0]], split[1]) - } - } + config, err := runtime.GetConfig() if err != nil { utils.InternalServerError(w, err) @@ -208,7 +203,7 @@ func ListNetworks(w http.ResponseWriter, r *http.Request) { reports := []*types.NetworkResource{} logrus.Debugf("netNames: %q", strings.Join(netNames, ", ")) for _, name := range netNames { - report, err := getNetworkResourceByNameOrID(name, runtime, filterMap) + report, err := getNetworkResourceByNameOrID(name, runtime, *filterMap) if err != nil { utils.InternalServerError(w, err) return @@ -401,22 +396,15 @@ func Disconnect(w http.ResponseWriter, r *http.Request) { // Prune removes unused networks func Prune(w http.ResponseWriter, r *http.Request) { runtime := r.Context().Value("runtime").(*libpod.Runtime) - filters, err := filtersFromRequest(r) + filterMap, err := util.PrepareFilters(r) if err != nil { utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "Decode()")) return } - filterMap := map[string][]string{} - for _, filter := range filters { - split := strings.SplitN(filter, "=", 2) - if len(split) > 1 { - filterMap[split[0]] = append(filterMap[split[0]], split[1]) - } - } ic := abi.ContainerEngine{Libpod: runtime} pruneOptions := entities.NetworkPruneOptions{ - Filters: filterMap, + Filters: *filterMap, } pruneReports, err := ic.NetworkPrune(r.Context(), pruneOptions) if err != nil { diff --git a/pkg/api/handlers/compat/volumes.go b/pkg/api/handlers/compat/volumes.go index d2febc615..42ece643b 100644 --- a/pkg/api/handlers/compat/volumes.go +++ b/pkg/api/handlers/compat/volumes.go @@ -5,7 +5,6 @@ import ( "encoding/json" "net/http" "net/url" - "strings" "time" "github.com/containers/podman/v3/libpod" @@ -14,6 +13,7 @@ import ( "github.com/containers/podman/v3/pkg/api/handlers/utils" "github.com/containers/podman/v3/pkg/domain/filters" "github.com/containers/podman/v3/pkg/domain/infra/abi/parse" + "github.com/containers/podman/v3/pkg/util" docker_api_types "github.com/docker/docker/api/types" docker_api_types_volume "github.com/docker/docker/api/types/volume" "github.com/gorilla/schema" @@ -22,16 +22,10 @@ import ( func ListVolumes(w http.ResponseWriter, r *http.Request) { var ( - decoder = r.Context().Value("decoder").(*schema.Decoder) runtime = r.Context().Value("runtime").(*libpod.Runtime) ) - query := struct { - Filters map[string][]string `schema:"filters"` - }{ - // override any golang type defaults - } - - if err := decoder.Decode(&query, r.URL.Query()); err != nil { + filtersMap, err := util.PrepareFilters(r) + if err != nil { utils.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError, errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String())) return @@ -39,14 +33,14 @@ func ListVolumes(w http.ResponseWriter, r *http.Request) { // Reject any libpod specific filters since `GenerateVolumeFilters()` will // happily parse them for us. - for filter := range query.Filters { + for filter := range *filtersMap { if filter == "opts" { utils.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError, errors.Errorf("unsupported libpod filters passed to docker endpoint")) return } } - volumeFilters, err := filters.GenerateVolumeFilters(query.Filters) + volumeFilters, err := filters.GenerateVolumeFilters(*filtersMap) if err != nil { utils.InternalServerError(w, err) return @@ -265,20 +259,13 @@ func PruneVolumes(w http.ResponseWriter, r *http.Request) { var ( runtime = r.Context().Value("runtime").(*libpod.Runtime) ) - filtersList, err := filtersFromRequest(r) + filterMap, err := util.PrepareFilters(r) if err != nil { utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "Decode()")) return } - filterMap := map[string][]string{} - for _, filter := range filtersList { - split := strings.SplitN(filter, "=", 2) - if len(split) > 1 { - filterMap[split[0]] = append(filterMap[split[0]], split[1]) - } - } - f := (url.Values)(filterMap) + f := (url.Values)(*filterMap) filterFuncs, err := filters.GenerateVolumeFilters(f) if err != nil { utils.Error(w, "Something when wrong.", http.StatusInternalServerError, errors.Wrapf(err, "failed to parse filters for %s", f.Encode())) diff --git a/pkg/api/handlers/libpod/networks.go b/pkg/api/handlers/libpod/networks.go index 19c9ed658..5417f778e 100644 --- a/pkg/api/handlers/libpod/networks.go +++ b/pkg/api/handlers/libpod/networks.go @@ -10,6 +10,7 @@ import ( "github.com/containers/podman/v3/pkg/api/handlers/utils" "github.com/containers/podman/v3/pkg/domain/entities" "github.com/containers/podman/v3/pkg/domain/infra/abi" + "github.com/containers/podman/v3/pkg/util" "github.com/gorilla/schema" "github.com/pkg/errors" ) @@ -45,20 +46,15 @@ func CreateNetwork(w http.ResponseWriter, r *http.Request) { } func ListNetworks(w http.ResponseWriter, r *http.Request) { runtime := r.Context().Value("runtime").(*libpod.Runtime) - decoder := r.Context().Value("decoder").(*schema.Decoder) - query := struct { - Filters map[string][]string `schema:"filters"` - }{ - // override any golang type defaults - } - if err := decoder.Decode(&query, r.URL.Query()); err != nil { - utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, + filterMap, err := util.PrepareFilters(r) + if err != nil { + utils.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError, errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String())) return } options := entities.NetworkListOptions{ - Filters: query.Filters, + Filters: *filterMap, } ic := abi.ContainerEngine{Libpod: runtime} reports, err := ic.NetworkList(r.Context(), options) @@ -78,7 +74,7 @@ func RemoveNetwork(w http.ResponseWriter, r *http.Request) { // override any golang type defaults } if err := decoder.Decode(&query, r.URL.Query()); err != nil { - utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, + utils.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError, errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String())) return } @@ -111,7 +107,7 @@ func InspectNetwork(w http.ResponseWriter, r *http.Request) { // override any golang type defaults } if err := decoder.Decode(&query, r.URL.Query()); err != nil { - utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, + utils.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError, errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String())) return } @@ -178,20 +174,15 @@ func ExistsNetwork(w http.ResponseWriter, r *http.Request) { // Prune removes unused networks func Prune(w http.ResponseWriter, r *http.Request) { runtime := r.Context().Value("runtime").(*libpod.Runtime) - decoder := r.Context().Value("decoder").(*schema.Decoder) - query := struct { - Filters map[string][]string `schema:"filters"` - }{ - // override any golang type defaults - } - if err := decoder.Decode(&query, r.URL.Query()); err != nil { + filterMap, err := util.PrepareFilters(r) + if err != nil { utils.Error(w, "Something went wrong.", http.StatusInternalServerError, err) return } pruneOptions := entities.NetworkPruneOptions{ - Filters: query.Filters, + Filters: *filterMap, } ic := abi.ContainerEngine{Libpod: runtime} pruneReports, err := ic.NetworkPrune(r.Context(), pruneOptions) diff --git a/pkg/api/handlers/libpod/volumes.go b/pkg/api/handlers/libpod/volumes.go index a602e6744..442b53d1e 100644 --- a/pkg/api/handlers/libpod/volumes.go +++ b/pkg/api/handlers/libpod/volumes.go @@ -13,6 +13,7 @@ import ( "github.com/containers/podman/v3/pkg/domain/filters" "github.com/containers/podman/v3/pkg/domain/infra/abi" "github.com/containers/podman/v3/pkg/domain/infra/abi/parse" + "github.com/containers/podman/v3/pkg/util" "github.com/gorilla/schema" "github.com/pkg/errors" ) @@ -29,7 +30,7 @@ func CreateVolume(w http.ResponseWriter, r *http.Request) { } input := entities.VolumeCreateOptions{} if err := decoder.Decode(&query, r.URL.Query()); err != nil { - utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, + utils.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError, errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String())) return } @@ -95,22 +96,16 @@ func InspectVolume(w http.ResponseWriter, r *http.Request) { func ListVolumes(w http.ResponseWriter, r *http.Request) { var ( - decoder = r.Context().Value("decoder").(*schema.Decoder) runtime = r.Context().Value("runtime").(*libpod.Runtime) ) - query := struct { - Filters map[string][]string `schema:"filters"` - }{ - // override any golang type defaults - } - - if err := decoder.Decode(&query, r.URL.Query()); err != nil { - utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, + filterMap, err := util.PrepareFilters(r) + if err != nil { + utils.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError, errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String())) return } - volumeFilters, err := filters.GenerateVolumeFilters(query.Filters) + volumeFilters, err := filters.GenerateVolumeFilters(*filterMap) if err != nil { utils.InternalServerError(w, err) return @@ -148,19 +143,13 @@ func PruneVolumes(w http.ResponseWriter, r *http.Request) { func pruneVolumesHelper(r *http.Request) ([]*reports.PruneReport, error) { var ( runtime = r.Context().Value("runtime").(*libpod.Runtime) - decoder = r.Context().Value("decoder").(*schema.Decoder) ) - query := struct { - Filters map[string][]string `schema:"filters"` - }{ - // override any golang type defaults - } - - if err := decoder.Decode(&query, r.URL.Query()); err != nil { + filterMap, err := util.PrepareFilters(r) + if err != nil { return nil, err } - f := (url.Values)(query.Filters) + f := (url.Values)(*filterMap) filterFuncs, err := filters.GenerateVolumeFilters(f) if err != nil { return nil, err @@ -172,6 +161,7 @@ func pruneVolumesHelper(r *http.Request) ([]*reports.PruneReport, error) { } return reports, nil } + func RemoveVolume(w http.ResponseWriter, r *http.Request) { var ( runtime = r.Context().Value("runtime").(*libpod.Runtime) @@ -184,7 +174,7 @@ func RemoveVolume(w http.ResponseWriter, r *http.Request) { } if err := decoder.Decode(&query, r.URL.Query()); err != nil { - utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, + utils.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError, errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String())) return } diff --git a/pkg/util/filters.go b/pkg/util/filters.go index bf16f89e3..51b2c5331 100644 --- a/pkg/util/filters.go +++ b/pkg/util/filters.go @@ -1,6 +1,10 @@ package util import ( + "encoding/json" + "fmt" + "net/http" + "strings" "time" "github.com/containers/podman/v3/pkg/timetype" @@ -23,3 +27,69 @@ func ComputeUntilTimestamp(filter string, filterValues []string) (time.Time, err } return time.Unix(seconds, nanoseconds), nil } + +// filtersFromRequests extracts the "filters" parameter from the specified +// http.Request. The parameter can either be a `map[string][]string` as done +// in new versions of Docker and libpod, or a `map[string]map[string]bool` as +// done in older versions of Docker. We have to do a bit of Yoga to support +// both - just as Docker does as well. +// +// Please refer to https://github.com/containers/podman/issues/6899 for some +// background. +func FiltersFromRequest(r *http.Request) ([]string, error) { + var ( + compatFilters map[string]map[string]bool + filters map[string][]string + libpodFilters []string + raw []byte + ) + + if _, found := r.URL.Query()["filters"]; found { + raw = []byte(r.Form.Get("filters")) + } else if _, found := r.URL.Query()["Filters"]; found { + raw = []byte(r.Form.Get("Filters")) + } else { + return []string{}, nil + } + + // Backwards compat with older versions of Docker. + if err := json.Unmarshal(raw, &compatFilters); err == nil { + for filterKey, filterMap := range compatFilters { + for filterValue, toAdd := range filterMap { + if toAdd { + libpodFilters = append(libpodFilters, fmt.Sprintf("%s=%s", filterKey, filterValue)) + } + } + } + return libpodFilters, nil + } + + if err := json.Unmarshal(raw, &filters); err != nil { + return nil, err + } + + for filterKey, filterSlice := range filters { + for _, filterValue := range filterSlice { + libpodFilters = append(libpodFilters, fmt.Sprintf("%s=%s", filterKey, filterValue)) + } + } + + return libpodFilters, nil +} + +// PrepareFilters prepares a *map[string][]string of filters to be later searched +// in lipod and compat API to get desired filters +func PrepareFilters(r *http.Request) (*map[string][]string, error) { + filtersList, err := FiltersFromRequest(r) + if err != nil { + return nil, err + } + filterMap := map[string][]string{} + for _, filter := range filtersList { + split := strings.SplitN(filter, "=", 2) + if len(split) > 1 { + filterMap[split[0]] = append(filterMap[split[0]], split[1]) + } + } + return &filterMap, nil +} diff --git a/test/apiv2/30-volumes.at b/test/apiv2/30-volumes.at index 1a40b3cdf..18ff31100 100644 --- a/test/apiv2/30-volumes.at +++ b/test/apiv2/30-volumes.at @@ -86,14 +86,34 @@ t DELETE libpod/volumes/foo1 404 \ .message~.* \ .response=404 +#compat api list volumes sanity checks +t GET volumes?filters='garb1age}' 500 \ + .cause="invalid character 'g' looking for beginning of value" +t GET volumes?filters='{"label":["testl' 500 \ + .cause="unexpected end of JSON input" + +#libpod api list volumes sanity checks +t GET libpod/volumes/json?filters='garb1age}' 500 \ + .cause="invalid character 'g' looking for beginning of value" +t GET libpod/volumes/json?filters='{"label":["testl' 500 \ + .cause="unexpected end of JSON input" + # Prune volumes - bad filter input t POST volumes/prune?filters='garb1age}' 500 \ .cause="invalid character 'g' looking for beginning of value" +t POST libpod/volumes/prune?filters='garb1age}' 500 \ + .cause="invalid character 'g' looking for beginning of value" ## Prune volumes with label matching 'testlabel1=testonly' t POST libpod/volumes/prune?filters='{"label":["testlabel1=testonly"]}' 200 t GET libpod/volumes/json?filters='{"label":["testlabel1=testonly"]}' 200 length=0 +## Prune volumes with label illformed label +t POST volumes/prune?filters='{"label":["tes' 500 \ + .cause="unexpected end of JSON input" +t POST libpod/volumes/prune?filters='{"label":["tes' 500 \ + .cause="unexpected end of JSON input" + ## Prune volumes with label matching 'testlabel' t POST libpod/volumes/prune?filters='{"label":["testlabel"]}' 200 t GET libpod/volumes/json?filters='{"label":["testlabel"]}' 200 length=0 diff --git a/test/apiv2/35-networks.at b/test/apiv2/35-networks.at index 6c3a34ece..21840a42d 100644 --- a/test/apiv2/35-networks.at +++ b/test/apiv2/35-networks.at @@ -80,9 +80,29 @@ t POST networks/create Name=net3\ IPAM='{"Config":[]}' 201 # network delete docker t DELETE networks/net3 204 -# Prune networks compat api - bad filter input +#compat api list networks sanity checks +t GET networks?filters='garb1age}' 500 \ + .cause="invalid character 'g' looking for beginning of value" +t GET networks?filters='{"label":["testl' 500 \ + .cause="unexpected end of JSON input" + +#libpod api list networks sanity checks +t GET libpod/networks/json?filters='garb1age}' 500 \ + .cause="invalid character 'g' looking for beginning of value" +t GET libpod/networks/json?filters='{"label":["testl' 500 \ + .cause="unexpected end of JSON input" + +# Prune networks compat api t POST networks/prune?filters='garb1age}' 500 \ .cause="invalid character 'g' looking for beginning of value" +t POST networks/prune?filters='{"label":["tes' 500 \ + .cause="unexpected end of JSON input" + +# Prune networks libpod api +t POST libpod/networks/prune?filters='garb1age}' 500 \ + .cause="invalid character 'g' looking for beginning of value" +t POST libpod/networks/prune?filters='{"label":["tes' 500 \ + .cause="unexpected end of JSON input" # prune networks using filter - compat api t POST networks/prune?filters='{"label":["xyz"]}' 200 -- cgit v1.2.3-54-g00ecf