fix: prevent server side request forgery using Kubernetes storage (#2479)

Signed-off-by: m.nabokikh <maksim.nabokikh@flant.com>

storage/kubernetes/client.go

@@ -18,6 +18,7 @@ import (
 	"net/url"
 	"os"
 	"path"
+	"regexp"
 	"strconv"
 	"strings"
 	"time"
@@ -83,14 +84,26 @@ func offlineTokenName(userID string, connID string, h func() hash.Hash) string {
 	return strings.TrimRight(encoding.EncodeToString(hash.Sum(nil)), "=")
 }
 
+const kubeResourceMaxLen = 63
+
+var kubeResourceNameRegex = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`)
+
 func (cli *client) urlForWithParams(
 	apiVersion, namespace, resource, name string, params url.Values,
-) string {
+) (string, error) {
 	basePath := "apis/"
 	if apiVersion == "v1" {
 		basePath = "api/"
 	}
 
+	if name != "" && (len(name) > kubeResourceMaxLen || !kubeResourceNameRegex.MatchString(name)) {
+		// The actual name can be found in auth request or auth code objects and equals to the state value
+		return "", fmt.Errorf(
+			"invalid kubernetes resource name: must match the pattern %s and be no longer than %d charactes",
+			kubeResourceNameRegex.String(),
+			kubeResourceMaxLen)
+	}
+
 	var p string
 	if namespace != "" {
 		p = path.Join(basePath, apiVersion, "namespaces", namespace, resource, name)
@@ -105,13 +118,13 @@ func (cli *client) urlForWithParams(
 	}
 
 	if strings.HasSuffix(cli.baseURL, "/") {
-		return cli.baseURL + p + paramsSuffix
+		return cli.baseURL + p + paramsSuffix, nil
 	}
 
-	return cli.baseURL + "/" + p + paramsSuffix
+	return cli.baseURL + "/" + p + paramsSuffix, nil
 }
 
-func (cli *client) urlFor(apiVersion, namespace, resource, name string) string {
+func (cli *client) urlFor(apiVersion, namespace, resource, name string) (string, error) {
 	return cli.urlForWithParams(apiVersion, namespace, resource, name, url.Values{})
 }
 
@@ -191,13 +204,20 @@ func (cli *client) getURL(url string, v interface{}) error {
 }
 
 func (cli *client) getResource(apiVersion, namespace, resource, name string, v interface{}) error {
-	return cli.getURL(cli.urlFor(apiVersion, namespace, resource, name), v)
+	u, err := cli.urlFor(apiVersion, namespace, resource, name)
+	if err != nil {
+		return err
+	}
+	return cli.getURL(u, v)
 }
 
 func (cli *client) listN(resource string, v interface{}, n int) error {
 	params := url.Values{}
 	params.Add("limit", fmt.Sprintf("%d", n))
-	u := cli.urlForWithParams(cli.apiVersion, cli.namespace, resource, "", params)
+	u, err := cli.urlForWithParams(cli.apiVersion, cli.namespace, resource, "", params)
+	if err != nil {
+		return err
+	}
 	return cli.getURL(u, v)
 }
 
@@ -215,7 +235,10 @@ func (cli *client) postResource(apiVersion, namespace, resource string, v interf
 		return fmt.Errorf("marshal object: %v", err)
 	}
 
-	url := cli.urlFor(apiVersion, namespace, resource, "")
+	url, err := cli.urlFor(apiVersion, namespace, resource, "")
+	if err != nil {
+		return err
+	}
 	resp, err := cli.client.Post(url, "application/json", bytes.NewReader(body))
 	if err != nil {
 		return err
@@ -256,7 +279,10 @@ func (cli *client) detectKubernetesVersion() error {
 }
 
 func (cli *client) delete(resource, name string) error {
-	url := cli.urlFor(cli.apiVersion, cli.namespace, resource, name)
+	url, err := cli.urlFor(cli.apiVersion, cli.namespace, resource, name)
+	if err != nil {
+		return err
+	}
 	req, err := http.NewRequest("DELETE", url, nil)
 	if err != nil {
 		return fmt.Errorf("create delete request: %v", err)
@@ -295,7 +321,11 @@ func (cli *client) put(resource, name string, v interface{}) error {
 		return fmt.Errorf("marshal object: %v", err)
 	}
 
-	url := cli.urlFor(cli.apiVersion, cli.namespace, resource, name)
+	url, err := cli.urlFor(cli.apiVersion, cli.namespace, resource, name)
+	if err != nil {
+		return err
+	}
+
 	req, err := http.NewRequest("PUT", url, bytes.NewReader(body))
 	if err != nil {
 		return fmt.Errorf("create patch request: %v", err)

storage/kubernetes/storage_test.go

@@ -129,7 +129,11 @@ func TestURLFor(t *testing.T) {
 
 	for _, test := range tests {
 		c := &client{baseURL: test.baseURL}
-		got := c.urlFor(test.apiVersion, test.namespace, test.resource, test.name)
+		got, err := c.urlFor(test.apiVersion, test.namespace, test.resource, test.name)
+		if err != nil {
+			t.Errorf("got error: %v", err)
+		}
+
 		if got != test.want {
 			t.Errorf("(&client{baseURL:%q}).urlFor(%q, %q, %q, %q): expected %q got %q",
 				test.baseURL,