fix: prevent server side request forgery using Kubernetes storage

Signed-off-by: m.nabokikh <maksim.nabokikh@flant.com>
dexidp · May 20, 2022 · cde4ea4 · cde4ea4
1 parent 0270536
commit cde4ea4
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 10 deletions.
diff --git a/storage/kubernetes/client.go b/storage/kubernetes/client.go
@@ -17,6 +17,7 @@ import (
 	"net/http"
 	"os"
 	"path"
+	"regexp"
 	"strconv"
 	"strings"
 	"time"
@@ -82,22 +83,30 @@ func offlineTokenName(userID string, connID string, h func() hash.Hash) string {
 	return strings.TrimRight(encoding.EncodeToString(hash.Sum(nil)), "=")
 }
 
-func (cli *client) urlFor(apiVersion, namespace, resource, name string) string {
+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) urlFor(apiVersion, namespace, resource, name string) (string, error) {
 	basePath := "apis/"
 	if apiVersion == "v1" {
 		basePath = "api/"
 	}
 
+	if name != "" && (len(name) > kubeResourceMaxLen || !kubeResourceNameRegex.MatchString(name)) {
+		return "", fmt.Errorf(
+			"kubernetes resource name is not valid: %q, must match the pattern %s and be no longer than %d charactes",
+			name, kubeResourceNameRegex.String(), kubeResourceMaxLen)
+	}
+
 	var p string
 	if namespace != "" {
 		p = path.Join(basePath, apiVersion, "namespaces", namespace, resource, name)
 	} else {
 		p = path.Join(basePath, apiVersion, resource, name)
 	}
-	if strings.HasSuffix(cli.baseURL, "/") {
-		return cli.baseURL + p
-	}
-	return cli.baseURL + "/" + p
+
+	return strings.TrimSuffix(cli.baseURL, "/") + "/" + p, nil
 }
 
 // Define an error interface so we can get at the underlying status code if it's
@@ -164,7 +173,10 @@ func (cli *client) get(resource, name string, v interface{}) error {
 }
 
 func (cli *client) getResource(apiVersion, namespace, resource, name string, v interface{}) error {
-	url := cli.urlFor(apiVersion, namespace, resource, name)
+	url, err := cli.urlFor(apiVersion, namespace, resource, name)
+	if err != nil {
+		return err
+	}
 	resp, err := cli.client.Get(url)
 	if err != nil {
 		return err
@@ -190,7 +202,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
@@ -231,7 +246,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)
@@ -270,7 +288,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)

diff --git a/storage/kubernetes/storage_test.go b/storage/kubernetes/storage_test.go
@@ -125,7 +125,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,