cli: Only allow HTTPS URLs with `inject` (#7940)

The CLI may access manifests over insecure channels, potentially
allowing MITM-attacks to run arbitrary code.

This change modifies `inject` to only support `https` URLs to mitigate
this risk.

This change addresses a security review finding (`TOB-LKDTM-4`).

Signed-off-by: Oliver Gould <ver@buoyant.io>
This commit is contained in:
Oliver Gould 2022-02-23 09:26:27 -08:00 committed by GitHub
parent 25ef001ab2
commit 27d89a8e3a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 22 additions and 24 deletions

View File

@ -70,7 +70,7 @@ sub-folders, or coming from stdin.`,
kubectl get deploy -o yaml | linkerd inject - | kubectl apply -f - kubectl get deploy -o yaml | linkerd inject - | kubectl apply -f -
# Injecting a file from a remote URL # Injecting a file from a remote URL
linkerd inject http://url.to/yml | kubectl apply -f - linkerd inject https://url.to/yml | kubectl apply -f -
# Inject all the resources inside a folder and its sub-folders. # Inject all the resources inside a folder and its sub-folders.
linkerd inject <folder> | kubectl apply -f -`, linkerd inject <folder> | kubectl apply -f -`,

View File

@ -577,7 +577,7 @@ func TestInjectFilePath(t *testing.T) {
}) })
} }
func TestValidURL(t *testing.T) { func TestToURL(t *testing.T) {
// if the string follows a URL pattern, true has to be returned // if the string follows a URL pattern, true has to be returned
// if not false is returned // if not false is returned
@ -593,9 +593,9 @@ func TestValidURL(t *testing.T) {
} }
for url, expectedValue := range tests { for url, expectedValue := range tests {
value := isValidURL(url) _, ok := toURL(url)
if value != expectedValue { if ok != expectedValue {
t.Errorf("Result mismatch for %s. expected %v, but got %v", url, expectedValue, value) t.Errorf("Result mismatch for %s. expected %v, but got %v", url, expectedValue, ok)
} }
} }

View File

@ -10,6 +10,7 @@ import (
"net/url" "net/url"
"os" "os"
"path/filepath" "path/filepath"
"strings"
"github.com/linkerd/linkerd2/pkg/inject" "github.com/linkerd/linkerd2/pkg/inject"
corev1 "k8s.io/api/core/v1" corev1 "k8s.io/api/core/v1"
@ -142,17 +143,19 @@ func processList(bytes []byte, rt resourceTransformer) ([]byte, []inject.Report,
// Read all the resource files found in path into a slice of readers. // Read all the resource files found in path into a slice of readers.
// path can be either a file, directory or stdin. // path can be either a file, directory or stdin.
func read(path string) ([]io.Reader, error) { func read(path string) ([]io.Reader, error) {
var (
in []io.Reader
err error
)
if path == "-" { if path == "-" {
in = append(in, os.Stdin) return []io.Reader{os.Stdin}, nil
} else if isValidURL(path) { }
resp, err := http.Get(path)
if url, ok := toURL(path); ok {
if strings.ToLower(url.Scheme) != "https" {
return nil, fmt.Errorf("only HTTPS URLs are allowed")
}
resp, err := http.Get(url.String())
if err != nil { if err != nil {
return nil, err return nil, err
} }
defer resp.Body.Close()
if resp.StatusCode != http.StatusOK { if resp.StatusCode != http.StatusOK {
return nil, fmt.Errorf("unable to read URL %q, server reported %s, status code=%d", path, resp.Status, resp.StatusCode) return nil, fmt.Errorf("unable to read URL %q, server reported %s, status code=%d", path, resp.Status, resp.StatusCode)
@ -164,26 +167,21 @@ func read(path string) ([]io.Reader, error) {
if err != nil { if err != nil {
return nil, err return nil, err
} }
resp.Body.Close()
in = append(in, buf) return []io.Reader{buf}, nil
} else {
in, err = walk(path)
if err != nil {
return nil, err
}
} }
return in, nil return walk(path)
} }
// checks if the given string is a valid URL // checks if the given string is a valid URL
func isValidURL(path string) bool { func toURL(path string) (*url.URL, bool) {
u, err := url.ParseRequestURI(path) u, err := url.ParseRequestURI(path)
if err != nil { if err == nil && u.Host != "" && u.Scheme != "" {
return false return u, true
} }
return u.Host != "" && u.Scheme != "" return nil, false
} }
// walk walks the file tree rooted at path. path may be a file or a directory. // walk walks the file tree rooted at path. path may be a file or a directory.