From 6e4df6a7f27ecadbdec06fe92d915faabee33300 Mon Sep 17 00:00:00 2001 From: Yucheng Wu Date: Tue, 14 May 2019 14:49:38 +0800 Subject: [PATCH] fix CVE-2019-11244: `kubectl --http-cache=` creates world-writeable cached schema files --- .../discovery/cached/disk/cached_discovery.go | 4 +- .../cached/disk/cached_discovery_test.go | 27 ++++++++++ .../discovery/cached/disk/round_tripper.go | 3 ++ .../cached/disk/round_tripper_test.go | 52 +++++++++++++++++++ 4 files changed, 84 insertions(+), 2 deletions(-) diff --git a/staging/src/k8s.io/client-go/discovery/cached/disk/cached_discovery.go b/staging/src/k8s.io/client-go/discovery/cached/disk/cached_discovery.go index 9ede5016bc..fd8b61d158 100644 --- a/staging/src/k8s.io/client-go/discovery/cached/disk/cached_discovery.go +++ b/staging/src/k8s.io/client-go/discovery/cached/disk/cached_discovery.go @@ -172,7 +172,7 @@ func (d *CachedDiscoveryClient) getCachedFile(filename string) ([]byte, error) { } func (d *CachedDiscoveryClient) writeCachedFile(filename string, obj runtime.Object) error { - if err := os.MkdirAll(filepath.Dir(filename), 0755); err != nil { + if err := os.MkdirAll(filepath.Dir(filename), 0750); err != nil { return err } @@ -191,7 +191,7 @@ func (d *CachedDiscoveryClient) writeCachedFile(filename string, obj runtime.Obj return err } - err = os.Chmod(f.Name(), 0755) + err = os.Chmod(f.Name(), 0660) if err != nil { return err } diff --git a/staging/src/k8s.io/client-go/discovery/cached/disk/cached_discovery_test.go b/staging/src/k8s.io/client-go/discovery/cached/disk/cached_discovery_test.go index 3ddd4a98b1..3bb7fb8947 100644 --- a/staging/src/k8s.io/client-go/discovery/cached/disk/cached_discovery_test.go +++ b/staging/src/k8s.io/client-go/discovery/cached/disk/cached_discovery_test.go @@ -19,6 +19,7 @@ package disk import ( "io/ioutil" "os" + "path/filepath" "testing" "time" @@ -96,6 +97,32 @@ func TestNewCachedDiscoveryClient_TTL(t *testing.T) { assert.Equal(c.groupCalls, 2) } +func TestNewCachedDiscoveryClient_PathPerm(t *testing.T) { + assert := assert.New(t) + + d, err := ioutil.TempDir("", "") + assert.NoError(err) + os.RemoveAll(d) + defer os.RemoveAll(d) + + c := fakeDiscoveryClient{} + cdc := newCachedDiscoveryClient(&c, d, 1*time.Nanosecond) + cdc.ServerGroups() + + err = filepath.Walk(d, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if info.IsDir() { + assert.Equal(os.FileMode(0750), info.Mode().Perm()) + } else { + assert.Equal(os.FileMode(0660), info.Mode().Perm()) + } + return nil + }) + assert.NoError(err) +} + type fakeDiscoveryClient struct { groupCalls int resourceCalls int diff --git a/staging/src/k8s.io/client-go/discovery/cached/disk/round_tripper.go b/staging/src/k8s.io/client-go/discovery/cached/disk/round_tripper.go index 7e2a537a9a..1dfb8297d9 100644 --- a/staging/src/k8s.io/client-go/discovery/cached/disk/round_tripper.go +++ b/staging/src/k8s.io/client-go/discovery/cached/disk/round_tripper.go @@ -18,6 +18,7 @@ package disk import ( "net/http" + "os" "path/filepath" "github.com/gregjones/httpcache" @@ -35,6 +36,8 @@ type cacheRoundTripper struct { // corresponding requests. func newCacheRoundTripper(cacheDir string, rt http.RoundTripper) http.RoundTripper { d := diskv.New(diskv.Options{ + PathPerm: os.FileMode(0750), + FilePerm: os.FileMode(0660), BasePath: cacheDir, TempDir: filepath.Join(cacheDir, ".diskv-temp"), }) diff --git a/staging/src/k8s.io/client-go/discovery/cached/disk/round_tripper_test.go b/staging/src/k8s.io/client-go/discovery/cached/disk/round_tripper_test.go index 329be42a5d..13002c63d2 100644 --- a/staging/src/k8s.io/client-go/discovery/cached/disk/round_tripper_test.go +++ b/staging/src/k8s.io/client-go/discovery/cached/disk/round_tripper_test.go @@ -22,7 +22,10 @@ import ( "net/http" "net/url" "os" + "path/filepath" "testing" + + "github.com/stretchr/testify/assert" ) // copied from k8s.io/client-go/transport/round_trippers_test.go @@ -93,3 +96,52 @@ func TestCacheRoundTripper(t *testing.T) { t.Errorf("Invalid content read from cache %q", string(content)) } } + +func TestCacheRoundTripperPathPerm(t *testing.T) { + assert := assert.New(t) + + rt := &testRoundTripper{} + cacheDir, err := ioutil.TempDir("", "cache-rt") + os.RemoveAll(cacheDir) + defer os.RemoveAll(cacheDir) + + if err != nil { + t.Fatal(err) + } + cache := newCacheRoundTripper(cacheDir, rt) + + // First call, caches the response + req := &http.Request{ + Method: http.MethodGet, + URL: &url.URL{Host: "localhost"}, + } + rt.Response = &http.Response{ + Header: http.Header{"ETag": []string{`"123456"`}}, + Body: ioutil.NopCloser(bytes.NewReader([]byte("Content"))), + StatusCode: http.StatusOK, + } + resp, err := cache.RoundTrip(req) + if err != nil { + t.Fatal(err) + } + content, err := ioutil.ReadAll(resp.Body) + if err != nil { + t.Fatal(err) + } + if string(content) != "Content" { + t.Errorf(`Expected Body to be "Content", got %q`, string(content)) + } + + err = filepath.Walk(cacheDir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if info.IsDir() { + assert.Equal(os.FileMode(0750), info.Mode().Perm()) + } else { + assert.Equal(os.FileMode(0660), info.Mode().Perm()) + } + return nil + }) + assert.NoError(err) +}