From bdbd1930eda31eae6e999c92fab679ae6cd8dfbd Mon Sep 17 00:00:00 2001 From: Alex Crawford Date: Mon, 12 Jan 2015 12:25:28 -0800 Subject: [PATCH 1/5] config/validate: add rule for file paths --- config/validate/rules.go | 51 ++++++++++++++++++++++++----------- config/validate/rules_test.go | 37 +++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 15 deletions(-) diff --git a/config/validate/rules.go b/config/validate/rules.go index e1e6ad5..459877d 100644 --- a/config/validate/rules.go +++ b/config/validate/rules.go @@ -18,7 +18,9 @@ package validate import ( "fmt" + "path" "reflect" + "strings" "github.com/coreos/coreos-cloudinit/config" ) @@ -29,6 +31,7 @@ type rule func(config node, report *Report) var Rules []rule = []rule{ checkStructure, checkValidity, + checkWriteFiles, } // checkStructure compares the provided config to the empty config.CloudConfig @@ -67,6 +70,24 @@ func checkNodeStructure(n, g node, r *Report) { } } +// isCompatible determines if the type of kind n can be converted to the type +// of kind g in the context of YAML. This is not an exhaustive list, but its +// enough for the purposes of cloud-config validation. +func isCompatible(n, g reflect.Kind) bool { + switch g { + case reflect.String: + return n == reflect.String || n == reflect.Int || n == reflect.Float64 || n == reflect.Bool + case reflect.Struct: + return n == reflect.Struct || n == reflect.Map + case reflect.Float64: + return n == reflect.Float64 || n == reflect.Int + case reflect.Bool, reflect.Slice, reflect.Int: + return n == g + default: + panic(fmt.Sprintf("isCompatible(): unhandled kind %s", g)) + } +} + // checkValidity checks the value of every node in the provided config by // running config.AssertValid() on it. func checkValidity(cfg node, report *Report) { @@ -98,20 +119,20 @@ func checkNodeValidity(n, g node, r *Report) { } } -// isCompatible determines if the type of kind n can be converted to the type -// of kind g in the context of YAML. This is not an exhaustive list, but its -// enough for the purposes of cloud-config validation. -func isCompatible(n, g reflect.Kind) bool { - switch g { - case reflect.String: - return n == reflect.String || n == reflect.Int || n == reflect.Float64 || n == reflect.Bool - case reflect.Struct: - return n == reflect.Struct || n == reflect.Map - case reflect.Float64: - return n == reflect.Float64 || n == reflect.Int - case reflect.Bool, reflect.Slice, reflect.Int: - return n == g - default: - panic(fmt.Sprintf("isCompatible(): unhandled kind %s", g)) +// checkWriteFiles checks to make sure that the target file can actually be +// written. Note that this check is approximate (it only checks to see if the file +// is under /usr). +func checkWriteFiles(cfg node, report *Report) { + for _, f := range cfg.Child("write_files").children { + c := f.Child("path") + if !c.IsValid() { + continue + } + + d := path.Dir(c.String()) + switch { + case strings.HasPrefix(d, "/usr"): + report.Error(c.line, "file cannot be written to a read-only filesystem") + } } } diff --git a/config/validate/rules_test.go b/config/validate/rules_test.go index ab6d157..55f4387 100644 --- a/config/validate/rules_test.go +++ b/config/validate/rules_test.go @@ -249,3 +249,40 @@ func TestCheckValidity(t *testing.T) { } } } + +func TestCheckWriteFiles(t *testing.T) { + tests := []struct { + config string + + entries []Entry + }{ + {}, + { + config: "write_files:\n - path: /valid", + }, + { + config: "write_files:\n - path: /tmp/usr/valid", + }, + { + config: "write_files:\n - path: /usr/invalid", + entries: []Entry{{entryError, "file cannot be written to a read-only filesystem", 2}}, + }, + { + config: "write-files:\n - path: /tmp/../usr/invalid", + entries: []Entry{{entryError, "file cannot be written to a read-only filesystem", 2}}, + }, + } + + for i, tt := range tests { + r := Report{} + n, err := parseCloudConfig([]byte(tt.config), &r) + if err != nil { + panic(err) + } + checkWriteFiles(n, &r) + + if e := r.Entries(); !reflect.DeepEqual(tt.entries, e) { + t.Errorf("bad report (%d, %q): want %#v, got %#v", i, tt.config, tt.entries, e) + } + } +} From 571903cec62ecefa2230e85ede5a5e48061b7443 Mon Sep 17 00:00:00 2001 From: Alex Crawford Date: Mon, 12 Jan 2015 15:43:58 -0800 Subject: [PATCH 2/5] config/validate: add rule for etcd discovery token --- config/validate/rules.go | 14 ++++++++++++++ config/validate/rules_test.go | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/config/validate/rules.go b/config/validate/rules.go index 459877d..416e6ec 100644 --- a/config/validate/rules.go +++ b/config/validate/rules.go @@ -18,6 +18,7 @@ package validate import ( "fmt" + "net/url" "path" "reflect" "strings" @@ -29,11 +30,24 @@ type rule func(config node, report *Report) // Rules contains all of the validation rules. var Rules []rule = []rule{ + checkDiscoveryUrl, checkStructure, checkValidity, checkWriteFiles, } +// checkDiscoveryUrl verifies that the string is a valid url. +func checkDiscoveryUrl(cfg node, report *Report) { + c := cfg.Child("coreos").Child("etcd").Child("discovery") + if !c.IsValid() { + return + } + + if _, err := url.ParseRequestURI(c.String()); err != nil { + report.Warning(c.line, "discovery URL is not valid") + } +} + // checkStructure compares the provided config to the empty config.CloudConfig // structure. Each node is checked to make sure that it exists in the known // structure and that its type is compatible. diff --git a/config/validate/rules_test.go b/config/validate/rules_test.go index 55f4387..c767dff 100644 --- a/config/validate/rules_test.go +++ b/config/validate/rules_test.go @@ -21,6 +21,39 @@ import ( "testing" ) +func TestCheckDiscoveryUrl(t *testing.T) { + tests := []struct { + config string + + entries []Entry + }{ + {}, + { + config: "coreos:\n etcd:\n discovery: https://discovery.etcd.io/00000000000000000000000000000000", + }, + { + config: "coreos:\n etcd:\n discovery: http://custom.domain/mytoken", + }, + { + config: "coreos:\n etcd:\n discovery: disco", + entries: []Entry{{entryWarning, "discovery URL is not valid", 3}}, + }, + } + + for i, tt := range tests { + r := Report{} + n, err := parseCloudConfig([]byte(tt.config), &r) + if err != nil { + panic(err) + } + checkDiscoveryUrl(n, &r) + + if e := r.Entries(); !reflect.DeepEqual(tt.entries, e) { + t.Errorf("bad report (%d, %q): want %#v, got %#v", i, tt.config, tt.entries, e) + } + } +} + func TestCheckStructure(t *testing.T) { tests := []struct { config string From f61c08c246ea5c756d0bb5068c0ae4af112b17b6 Mon Sep 17 00:00:00 2001 From: Alex Crawford Date: Mon, 12 Jan 2015 16:09:21 -0800 Subject: [PATCH 3/5] config/validate: add rule for coreos.write_files --- config/validate/rules.go | 10 ++++++++++ config/validate/rules_test.go | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/config/validate/rules.go b/config/validate/rules.go index 416e6ec..f8397dc 100644 --- a/config/validate/rules.go +++ b/config/validate/rules.go @@ -34,6 +34,7 @@ var Rules []rule = []rule{ checkStructure, checkValidity, checkWriteFiles, + checkWriteFilesUnderCoreos, } // checkDiscoveryUrl verifies that the string is a valid url. @@ -150,3 +151,12 @@ func checkWriteFiles(cfg node, report *Report) { } } } + +// checkWriteFilesUnderCoreos checks to see if the 'write_files' node is a +// child of 'coreos' (it shouldn't be). +func checkWriteFilesUnderCoreos(cfg node, report *Report) { + c := cfg.Child("coreos").Child("write_files") + if c.IsValid() { + report.Info(c.line, "write_files doesn't belong under coreos") + } +} diff --git a/config/validate/rules_test.go b/config/validate/rules_test.go index c767dff..d37c181 100644 --- a/config/validate/rules_test.go +++ b/config/validate/rules_test.go @@ -319,3 +319,37 @@ func TestCheckWriteFiles(t *testing.T) { } } } + +func TestCheckWriteFilesUnderCoreos(t *testing.T) { + tests := []struct { + config string + + entries []Entry + }{ + {}, + { + config: "write_files:\n - path: /hi", + }, + { + config: "coreos:\n write_files:\n - path: /hi", + entries: []Entry{{entryInfo, "write_files doesn't belong under coreos", 2}}, + }, + { + config: "coreos:\n write-files:\n - path: /hyphen", + entries: []Entry{{entryInfo, "write_files doesn't belong under coreos", 2}}, + }, + } + + for i, tt := range tests { + r := Report{} + n, err := parseCloudConfig([]byte(tt.config), &r) + if err != nil { + panic(err) + } + checkWriteFilesUnderCoreos(n, &r) + + if e := r.Entries(); !reflect.DeepEqual(tt.entries, e) { + t.Errorf("bad report (%d, %q): want %#v, got %#v", i, tt.config, tt.entries, e) + } + } +} From 3c93938f8a80ea269baa727322d4da012c77730c Mon Sep 17 00:00:00 2001 From: Alex Crawford Date: Tue, 13 Jan 2015 16:24:59 -0800 Subject: [PATCH 4/5] decode: refactor file decoding into config package --- config/decode.go | 56 ++++++++++++++++++++++++++++++++++++++++++++++++ system/file.go | 54 +--------------------------------------------- 2 files changed, 57 insertions(+), 53 deletions(-) create mode 100644 config/decode.go diff --git a/config/decode.go b/config/decode.go new file mode 100644 index 0000000..f5847aa --- /dev/null +++ b/config/decode.go @@ -0,0 +1,56 @@ +package config + +import ( + "bytes" + "compress/gzip" + "encoding/base64" + "fmt" +) + +func DecodeBase64Content(content string) ([]byte, error) { + output, err := base64.StdEncoding.DecodeString(content) + + if err != nil { + return nil, fmt.Errorf("Unable to decode base64: %q", err) + } + + return output, nil +} + +func DecodeGzipContent(content string) ([]byte, error) { + gzr, err := gzip.NewReader(bytes.NewReader([]byte(content))) + + if err != nil { + return nil, fmt.Errorf("Unable to decode gzip: %q", err) + } + defer gzr.Close() + + buf := new(bytes.Buffer) + buf.ReadFrom(gzr) + + return buf.Bytes(), nil +} + +func DecodeContent(content string, encoding string) ([]byte, error) { + switch encoding { + case "": + return []byte(content), nil + + case "b64", "base64": + return DecodeBase64Content(content) + + case "gz", "gzip": + return DecodeGzipContent(content) + + case "gz+base64", "gzip+base64", "gz+b64", "gzip+b64": + gz, err := DecodeBase64Content(content) + + if err != nil { + return nil, err + } + + return DecodeGzipContent(string(gz)) + } + + return nil, fmt.Errorf("Unsupported encoding %q", encoding) +} diff --git a/system/file.go b/system/file.go index 22118c1..9454498 100644 --- a/system/file.go +++ b/system/file.go @@ -17,9 +17,6 @@ package system import ( - "bytes" - "compress/gzip" - "encoding/base64" "fmt" "io/ioutil" "log" @@ -50,61 +47,12 @@ func (f *File) Permissions() (os.FileMode, error) { return os.FileMode(perm), nil } -func DecodeBase64Content(content string) ([]byte, error) { - output, err := base64.StdEncoding.DecodeString(content) - - if err != nil { - return nil, fmt.Errorf("Unable to decode base64: %v", err) - } - - return output, nil -} - -func DecodeGzipContent(content string) ([]byte, error) { - gzr, err := gzip.NewReader(bytes.NewReader([]byte(content))) - - if err != nil { - return nil, fmt.Errorf("Unable to decode gzip: %v", err) - } - defer gzr.Close() - - buf := new(bytes.Buffer) - buf.ReadFrom(gzr) - - return buf.Bytes(), nil -} - -func DecodeContent(content string, encoding string) ([]byte, error) { - switch encoding { - case "": - return []byte(content), nil - - case "b64", "base64": - return DecodeBase64Content(content) - - case "gz", "gzip": - return DecodeGzipContent(content) - - case "gz+base64", "gzip+base64", "gz+b64", "gzip+b64": - gz, err := DecodeBase64Content(content) - - if err != nil { - return nil, err - } - - return DecodeGzipContent(string(gz)) - } - - return nil, fmt.Errorf("Unsupported encoding %s", encoding) - -} - func WriteFile(f *File, root string) (string, error) { fullpath := path.Join(root, f.Path) dir := path.Dir(fullpath) log.Printf("Writing file to %q", fullpath) - content, err := DecodeContent(f.Content, f.Encoding) + content, err := config.DecodeContent(f.Content, f.Encoding) if err != nil { return "", fmt.Errorf("Unable to decode %s (%v)", f.Path, err) From 551cbb1e5d765b9e87a5440d3dfebe4ddfd25300 Mon Sep 17 00:00:00 2001 From: Alex Crawford Date: Tue, 13 Jan 2015 16:28:47 -0800 Subject: [PATCH 5/5] config/validate: add rule for file encoding --- config/validate/rules.go | 17 +++++++++++++ config/validate/rules_test.go | 46 +++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/config/validate/rules.go b/config/validate/rules.go index f8397dc..abce8aa 100644 --- a/config/validate/rules.go +++ b/config/validate/rules.go @@ -31,6 +31,7 @@ type rule func(config node, report *Report) // Rules contains all of the validation rules. var Rules []rule = []rule{ checkDiscoveryUrl, + checkEncoding, checkStructure, checkValidity, checkWriteFiles, @@ -49,6 +50,22 @@ func checkDiscoveryUrl(cfg node, report *Report) { } } +// checkEncoding validates that, for each file under 'write_files', the +// content can be decoded given the specified encoding. +func checkEncoding(cfg node, report *Report) { + for _, f := range cfg.Child("write_files").children { + e := f.Child("encoding") + if !e.IsValid() { + continue + } + + c := f.Child("contents") + if _, err := config.DecodeContent(c.String(), e.String()); err != nil { + report.Error(c.line, fmt.Sprintf("contents cannot be decoded as %q", e.String())) + } + } +} + // checkStructure compares the provided config to the empty config.CloudConfig // structure. Each node is checked to make sure that it exists in the known // structure and that its type is compatible. diff --git a/config/validate/rules_test.go b/config/validate/rules_test.go index d37c181..85dffae 100644 --- a/config/validate/rules_test.go +++ b/config/validate/rules_test.go @@ -54,6 +54,52 @@ func TestCheckDiscoveryUrl(t *testing.T) { } } +func TestCheckEncoding(t *testing.T) { + tests := []struct { + config string + + entries []Entry + }{ + {}, + { + config: "write_files:\n - encoding: base64\n contents: aGVsbG8K", + }, + { + config: "write_files:\n - contents: !!binary aGVsbG8K", + }, + { + config: "write_files:\n - encoding: base64\n contents: !!binary aGVsbG8K", + entries: []Entry{{entryError, `contents cannot be decoded as "base64"`, 3}}, + }, + { + config: "write_files:\n - encoding: base64\n contents: !!binary YUdWc2JHOEsK", + }, + { + config: "write_files:\n - encoding: gzip\n contents: !!binary H4sIAOC3tVQAA8tIzcnJ5wIAIDA6NgYAAAA=", + }, + { + config: "write_files:\n - encoding: gzip+base64\n contents: H4sIAOC3tVQAA8tIzcnJ5wIAIDA6NgYAAAA=", + }, + { + config: "write_files:\n - encoding: custom\n contents: hello", + entries: []Entry{{entryError, `contents cannot be decoded as "custom"`, 3}}, + }, + } + + for i, tt := range tests { + r := Report{} + n, err := parseCloudConfig([]byte(tt.config), &r) + if err != nil { + panic(err) + } + checkEncoding(n, &r) + + if e := r.Entries(); !reflect.DeepEqual(tt.entries, e) { + t.Errorf("bad report (%d, %q): want %#v, got %#v", i, tt.config, tt.entries, e) + } + } +} + func TestCheckStructure(t *testing.T) { tests := []struct { config string