From 898695e312ddbe670ac9e35c5b1ea0f4df6ab797 Mon Sep 17 00:00:00 2001 From: Xe Iaso Date: Fri, 22 Jul 2022 15:07:38 -0400 Subject: [PATCH] cmd/gitops-pusher: add etag cache file for the three version problem (#5124) This allows gitops-pusher to detect external ACL changes. I'm not sure what to call this problem, so I've been calling it the "three version problem" in my notes. The basic problem is that at any given time we only have two versions of the ACL file at any given point: the version in CONTROL and the one in the git repo. In order to check if there has been tampering of the ACL files in the admin panel, we need to have a _third_ version to compare against. In this case I am not storing the old ACL entirely (though that could be a reasonable thing to add in the future), but only its sha256sum. This allows us to detect if the shasum in control matches the shasum we expect, and if that expectation fails, then we can react accordingly. This will require additional configuration in CI, but I'm sure that can be done. Signed-off-by: Xe --- cmd/gitops-pusher/.gitignore | 1 + cmd/gitops-pusher/cache.go | 67 +++++++++++++++++++++++++++++ cmd/gitops-pusher/gitops-pusher.go | 69 ++++++++++++++++++++++++++---- 3 files changed, 129 insertions(+), 8 deletions(-) create mode 100644 cmd/gitops-pusher/.gitignore create mode 100644 cmd/gitops-pusher/cache.go diff --git a/cmd/gitops-pusher/.gitignore b/cmd/gitops-pusher/.gitignore new file mode 100644 index 000000000..504452249 --- /dev/null +++ b/cmd/gitops-pusher/.gitignore @@ -0,0 +1 @@ +version-cache.json diff --git a/cmd/gitops-pusher/cache.go b/cmd/gitops-pusher/cache.go new file mode 100644 index 000000000..b3646822e --- /dev/null +++ b/cmd/gitops-pusher/cache.go @@ -0,0 +1,67 @@ +// Copyright (c) 2022 Tailscale Inc & AUTHORS All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import ( + "encoding/json" + "os" +) + +// Cache contains cached information about the last time this tool was run. +// +// This is serialized to a JSON file that should NOT be checked into git. +// It should be managed with either CI cache tools or stored locally somehow. The +// exact mechanism is irrelevant as long as it is consistent. +// +// This allows gitops-pusher to detect external ACL changes. I'm not sure what to +// call this problem, so I've been calling it the "three version problem" in my +// notes. The basic problem is that at any given time we only have two versions +// of the ACL file at any given point. In order to check if there has been +// tampering of the ACL files in the admin panel, we need to have a _third_ version +// to compare against. +// +// In this case I am not storing the old ACL entirely (though that could be a +// reasonable thing to add in the future), but only its sha256sum. This allows +// us to detect if the shasum in control matches the shasum we expect, and if that +// expectation fails, then we can react accordingly. +type Cache struct { + PrevETag string // Stores the previous ETag of the ACL to allow +} + +// Save persists the cache to a given file. +func (c *Cache) Save(fname string) error { + os.Remove(fname) + fout, err := os.Create(fname) + if err != nil { + return err + } + defer fout.Close() + + return json.NewEncoder(fout).Encode(c) +} + +// LoadCache loads the cache from a given file. +func LoadCache(fname string) (*Cache, error) { + var result Cache + + fin, err := os.Open(fname) + if err != nil { + return nil, err + } + defer fin.Close() + + err = json.NewDecoder(fin).Decode(&result) + if err != nil { + return nil, err + } + + return &result, nil +} + +// Shuck removes the first and last character of a string, analogous to +// shucking off the husk of an ear of corn. +func Shuck(s string) string { + return s[1 : len(s)-1] +} diff --git a/cmd/gitops-pusher/gitops-pusher.go b/cmd/gitops-pusher/gitops-pusher.go index 5cc53f7c3..ea16cf5c6 100644 --- a/cmd/gitops-pusher/gitops-pusher.go +++ b/cmd/gitops-pusher/gitops-pusher.go @@ -27,11 +27,23 @@ import ( var ( rootFlagSet = flag.NewFlagSet("gitops-pusher", flag.ExitOnError) policyFname = rootFlagSet.String("policy-file", "./policy.hujson", "filename for policy file") + cacheFname = rootFlagSet.String("cache-file", "./version-cache.json", "filename for the previous known version hash") timeout = rootFlagSet.Duration("timeout", 5*time.Minute, "timeout for the entire CI run") githubSyntax = rootFlagSet.Bool("github-syntax", true, "use GitHub Action error syntax (https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-an-error-message)") + + modifiedExternallyFailure = make(chan struct{}, 1) ) -func apply(tailnet, apiKey string) func(context.Context, []string) error { +func modifiedExternallyError() { + if *githubSyntax { + fmt.Printf("::error file=%s,line=1,col=1,title=Policy File Modified Externally::The policy file was modified externally in the admin console.\n", *policyFname) + } else { + fmt.Printf("The policy file was modified externally in the admin console.\n") + } + modifiedExternallyFailure <- struct{}{} +} + +func apply(cache *Cache, tailnet, apiKey string) func(context.Context, []string) error { return func(ctx context.Context, args []string) error { controlEtag, err := getACLETag(ctx, tailnet, apiKey) if err != nil { @@ -43,8 +55,18 @@ func apply(tailnet, apiKey string) func(context.Context, []string) error { return err } + if cache.PrevETag == "" { + log.Println("no previous etag found, assuming local file is correct and recording that") + cache.PrevETag = Shuck(localEtag) + } + log.Printf("control: %s", controlEtag) log.Printf("local: %s", localEtag) + log.Printf("cache: %s", cache.PrevETag) + + if cache.PrevETag != controlEtag { + modifiedExternallyError() + } if controlEtag == localEtag { log.Println("no update needed, doing nothing") @@ -55,11 +77,13 @@ func apply(tailnet, apiKey string) func(context.Context, []string) error { return err } + cache.PrevETag = Shuck(localEtag) + return nil } } -func test(tailnet, apiKey string) func(context.Context, []string) error { +func test(cache *Cache, tailnet, apiKey string) func(context.Context, []string) error { return func(ctx context.Context, args []string) error { controlEtag, err := getACLETag(ctx, tailnet, apiKey) if err != nil { @@ -71,8 +95,18 @@ func test(tailnet, apiKey string) func(context.Context, []string) error { return err } + if cache.PrevETag == "" { + log.Println("no previous etag found, assuming local file is correct and recording that") + cache.PrevETag = Shuck(localEtag) + } + log.Printf("control: %s", controlEtag) log.Printf("local: %s", localEtag) + log.Printf("cache: %s", cache.PrevETag) + + if cache.PrevETag != controlEtag { + modifiedExternallyError() + } if controlEtag == localEtag { log.Println("no updates found, doing nothing") @@ -86,7 +120,7 @@ func test(tailnet, apiKey string) func(context.Context, []string) error { } } -func getChecksums(tailnet, apiKey string) func(context.Context, []string) error { +func getChecksums(cache *Cache, tailnet, apiKey string) func(context.Context, []string) error { return func(ctx context.Context, args []string) error { controlEtag, err := getACLETag(ctx, tailnet, apiKey) if err != nil { @@ -98,8 +132,14 @@ func getChecksums(tailnet, apiKey string) func(context.Context, []string) error return err } + if cache.PrevETag == "" { + log.Println("no previous etag found, assuming local file is correct and recording that") + cache.PrevETag = Shuck(localEtag) + } + log.Printf("control: %s", controlEtag) log.Printf("local: %s", localEtag) + log.Printf("cache: %s", cache.PrevETag) return nil } @@ -114,13 +154,22 @@ func main() { if !ok { log.Fatal("set envvar TS_API_KEY to your Tailscale API key") } + cache, err := LoadCache(*cacheFname) + if err != nil { + if os.IsNotExist(err) { + cache = &Cache{} + } else { + log.Fatalf("error loading cache: %v", err) + } + } + defer cache.Save(*cacheFname) applyCmd := &ffcli.Command{ Name: "apply", ShortUsage: "gitops-pusher [options] apply", ShortHelp: "Pushes changes to CONTROL", LongHelp: `Pushes changes to CONTROL`, - Exec: apply(tailnet, apiKey), + Exec: apply(cache, tailnet, apiKey), } testCmd := &ffcli.Command{ @@ -128,7 +177,7 @@ func main() { ShortUsage: "gitops-pusher [options] test", ShortHelp: "Tests ACL changes", LongHelp: "Tests ACL changes", - Exec: test(tailnet, apiKey), + Exec: test(cache, tailnet, apiKey), } cksumCmd := &ffcli.Command{ @@ -136,7 +185,7 @@ func main() { ShortUsage: "Shows checksums of ACL files", ShortHelp: "Fetch checksum of CONTROL's ACL and the local ACL for comparison", LongHelp: "Fetch checksum of CONTROL's ACL and the local ACL for comparison", - Exec: getChecksums(tailnet, apiKey), + Exec: getChecksums(cache, tailnet, apiKey), } root := &ffcli.Command{ @@ -157,6 +206,10 @@ func main() { fmt.Println(err) os.Exit(1) } + + if len(modifiedExternallyFailure) != 0 { + os.Exit(1) + } } func sumFile(fname string) (string, error) { @@ -176,7 +229,7 @@ func sumFile(fname string) (string, error) { return "", err } - return fmt.Sprintf("\"%x\"", h.Sum(nil)), nil + return fmt.Sprintf("%x", h.Sum(nil)), nil } func applyNewACL(ctx context.Context, tailnet, apiKey, policyFname, oldEtag string) error { @@ -315,5 +368,5 @@ func getACLETag(ctx context.Context, tailnet, apiKey string) (string, error) { return "", fmt.Errorf("wanted HTTP status code %d but got %d", want, got) } - return resp.Header.Get("ETag"), nil + return Shuck(resp.Header.Get("ETag")), nil }