From 9fdb79acfbb807ff43db090fc3d5b945723043fc Mon Sep 17 00:00:00 2001 From: Thomas Mitchell Date: Wed, 15 Feb 2023 20:05:47 -0500 Subject: [PATCH] add support for escaping colon and caret in paths --- main.go | 6 +-- tests | 8 +-- vault/proxy.go | 14 ++--- vault/tree.go | 13 +++-- vault/utils.go | 43 ++++++++++++--- vault/utils_test.go | 124 ++++++++++++++++++++++++++++++++++++++++++++ vault/vault.go | 23 ++++---- 7 files changed, 195 insertions(+), 36 deletions(-) create mode 100644 vault/utils_test.go diff --git a/main.go b/main.go index 340df00..a942d65 100644 --- a/main.go +++ b/main.go @@ -89,7 +89,7 @@ func connect(auth bool) *vault.Vault { return v } -//Exits program with error if no Vault targeted +// Exits program with error if no Vault targeted func getVaultURL() string { ret := os.Getenv("VAULT_ADDR") if ret == "" { @@ -2061,7 +2061,7 @@ paths/keys. if !opt.List.Quick { for i := range paths { if !strings.HasSuffix(paths[i], "/") { - fullpath := path + "/" + paths[i] + fullpath := path + "/" + vault.EscapePathSegment(paths[i]) mountVersion, err := v.MountVersion(fullpath) if err != nil { return err @@ -4463,7 +4463,7 @@ func recursively(cmd string, args ...string) bool { return y == "y" || y == "yes" } -//For versions of safe 0.10+ +// For versions of safe 0.10+ // Older versions just use a map[string]map[string]string type exportFormat struct { ExportVersion uint `json:"export_version"` diff --git a/tests b/tests index a46f146..57f9852 100755 --- a/tests +++ b/tests @@ -12,10 +12,10 @@ root_token= unseal_key= declare -a versions=( - "1.3.10" - "1.4.7" - "1.5.7" - "1.6.3" + "1.9.10" + "1.10.10" + "1.11.7" + "1.12.3" ) case $OSTYPE in diff --git a/vault/proxy.go b/vault/proxy.go index 4bfef60..4141b98 100644 --- a/vault/proxy.go +++ b/vault/proxy.go @@ -141,8 +141,8 @@ func getEnvironmentVariable(variables ...string) string { return "" } -//SOCKS5SSHConfig contains configuration variables for setting up a SOCKS5 -//proxy to be tunneled through an SSH connection. +// SOCKS5SSHConfig contains configuration variables for setting up a SOCKS5 +// proxy to be tunneled through an SSH connection. type SOCKS5SSHConfig struct { Host string User string @@ -151,7 +151,7 @@ type SOCKS5SSHConfig struct { SkipHostKeyValidation bool } -//StartSSHTunnel makes an SSH connection according to the given config. It +// StartSSHTunnel makes an SSH connection according to the given config. It // returns an SSH client if it was successful and an error otherwise. func StartSSHTunnel(conf SOCKS5SSHConfig) (*ssh.Client, error) { hostKeyCallback := ssh.InsecureIgnoreHostKey() @@ -186,9 +186,9 @@ func StartSSHTunnel(conf SOCKS5SSHConfig) (*ssh.Client, error) { return ssh.Dial("tcp", conf.Host, sshConfig) } -//StartSOCKS5SSH makes an SSH connection according to the given config, starts -//a local SOCKS5 server on a random port, and then returns the proxy -//address if the connection was successful and an error if it was unsuccessful. +// StartSOCKS5SSH makes an SSH connection according to the given config, starts +// a local SOCKS5 server on a random port, and then returns the proxy +// address if the connection was successful and an error if it was unsuccessful. func StartSOCKS5Server(dialFn func(string, string) (net.Conn, error)) (string, error) { socks5Server, err := socks5.New(&socks5.Config{ Dial: noopDialContext(dialFn), @@ -302,7 +302,7 @@ func writeKnownHosts(knownHostsFile, hostname string, key ssh.PublicKey) error { fileInfo, err := f.Stat() if err != nil { - return fmt.Errorf("Could no retrieve info for file `%s'") + return fmt.Errorf("Could not retrieve info for file `%s': %s", knownHostsFile, err) } if fileInfo.Size() != 0 { diff --git a/vault/tree.go b/vault/tree.go index b41cffe..cb3afb8 100644 --- a/vault/tree.go +++ b/vault/tree.go @@ -12,7 +12,7 @@ import ( "github.com/starkandwayne/goutils/tree" ) -//This is a synchronized queue that specifically works with our tree algorithm, +// This is a synchronized queue that specifically works with our tree algorithm, // in which the workers that pull work off the queue are also responsible for // populating the queue. This is because of the recursive nature of the tree // population. All workers are released when all workers are simultaneously @@ -142,7 +142,7 @@ func (v *Vault) ConstructSecrets(path string, opts TreeOpts) (s Secrets, err err return s, nil } -//This does not keep the list in a sorted order. Sort afterward +// This does not keep the list in a sorted order. Sort afterward func (s *Secrets) purgeWhereLatestVersionDeleted() { for i := 0; i < len(*s); i++ { if len((*s)[i].Versions) == 0 || (*s)[i].Versions[len((*s)[i].Versions)-1].State != SecretStateAlive { @@ -368,7 +368,7 @@ func (v *Vault) constructTree(path string, opts TreeOpts) (*secretTree, error) { return ret, err } -//Only use this for the base for the initial node of the tree. You can infer +// Only use this for the base for the initial node of the tree. You can infer // type much faster than this if you know the operation that retrieved it in the // first place. func (t *secretTree) populateNodeType(v *Vault) error { @@ -459,7 +459,12 @@ func (s Secrets) Paths() []string { for i := range s { if len(s[i].Versions) > 0 { for _, key := range s[i].Versions[len(s[i].Versions)-1].Data.Keys() { - ret = append(ret, fmt.Sprintf("%s:%s", s[i].Path, key)) + ret = append(ret, + fmt.Sprintf("%s:%s", + EscapePathSegment(s[i].Path), + EscapePathSegment(key), + ), + ) } } else { ret = append(ret, s[i].Path) diff --git a/vault/utils.go b/vault/utils.go index ee363de..5403972 100644 --- a/vault/utils.go +++ b/vault/utils.go @@ -8,32 +8,59 @@ import ( "strings" ) +var keyColonRegexp = regexp.MustCompile(`[^\\](:)`) +var versionCaretRegexp = regexp.MustCompile(`[^\\](\^)`) + // ParsePath splits the given path string into its respective secret path -// and contained key parts +// and contained key parts func ParsePath(path string) (secret, key string, version uint64) { secret = path - if idx := strings.LastIndex(path, "^"); idx >= 0 { - versionString := path[idx+1:] - var err error + var err error + + matches := versionCaretRegexp.FindAllStringSubmatchIndex(path, -1) + if len(matches) > 0 { //if there exists a version caret + caretIdx := matches[len(matches)-1] + caretStart, caretEnd := caretIdx[len(caretIdx)-2], caretIdx[len(caretIdx)-1] + versionString := path[caretEnd:] version, err = strconv.ParseUint(versionString, 10, 64) if err == nil { - path = path[:idx] + path = path[:caretStart] secret = path } } - if idx := strings.LastIndex(path, ":"); idx >= 0 { - secret = path[:idx] - key = path[idx+1:] + matches = keyColonRegexp.FindAllStringSubmatchIndex(path, -1) + if len(matches) > 0 { //if there exists a path colon + colonIdx := matches[len(matches)-1] + colonStart, colonEnd := colonIdx[len(colonIdx)-2], colonIdx[len(colonIdx)-1] + key = path[colonEnd:] + secret = path[:colonStart] } + //unescape escaped characters + secret = strings.ReplaceAll(secret, `\:`, ":") + secret = strings.ReplaceAll(secret, `\^`, "^") + key = strings.ReplaceAll(key, `\:`, ":") + key = strings.ReplaceAll(key, `\^`, "^") + secret = Canonicalize(secret) return } +// EscapePathSegment is the reverse of ParsePath for an output secret or key +// segment; whereas that function unescapes colons and carets, this function +// reescapes them so that they can be run through that function again. +func EscapePathSegment(segment string) string { + segment = strings.ReplaceAll(segment, ":", `\:`) + segment = strings.ReplaceAll(segment, "^", `\^`) + return segment +} + // EncodePath creates a safe-friendly canonical path for the given arguments func EncodePath(path, key string, version uint64) string { + path = EscapePathSegment(path) if key != "" { + key = EscapePathSegment(key) path += ":" + key } diff --git a/vault/utils_test.go b/vault/utils_test.go new file mode 100644 index 0000000..111f067 --- /dev/null +++ b/vault/utils_test.go @@ -0,0 +1,124 @@ +package vault_test + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + "github.com/starkandwayne/safe/vault" +) + +var _ = Describe("Utils", func() { + Describe("ParsePath", func() { + var inPath, inKey, inVersion string + var outPath, outKey string + var outVersion uint64 + + var expPath, expKey string + var expVersion uint64 + + JustBeforeEach(func() { + var fullInPath string = inPath + if inKey != "" { + fullInPath = fullInPath + ":" + inKey + } + if inVersion != "" { + fullInPath = fullInPath + "^" + inVersion + } + outPath, outKey, outVersion = vault.ParsePath(fullInPath) + }) + + AfterEach(func() { + inPath, inKey, inVersion = "", "", "" + outPath, outKey = "", "" + outVersion = 0 + expPath, expKey = "", "" + expVersion = 0 + }) + + assertPathValues := func() { + It("should have the expected values", func() { + By("having the correct path value") + Expect(outPath).To(Equal(expPath)) + + By("having the correct key value") + Expect(outKey).To(Equal(expKey)) + + By("having the correct version value") + Expect(outVersion).To(Equal(expVersion)) + }) + } + + type ioStruct struct{ in, out, desc string } + + paths := []ioStruct{ + {"secret/foo", "secret/foo", "that is basic"}, + {`secret/f\:oo`, "secret/f:oo", "that has an escaped colon"}, + {`secret/f\^oo`, "secret/f^oo", "that has an escaped caret"}, + } + + keys := []ioStruct{ + {"bar", "bar", "that is basic"}, + {`b\:ar`, "b:ar", "that has an escaped colon"}, + {`b\^ar`, "b^ar", "that has an escaped caret"}, + } + + Context("with a path", func() { + for i := range paths { + path := paths[i] + Context(path.desc, func() { + BeforeEach(func() { + inPath, expPath = path.in, path.out + }) + + assertPathValues() + + Context("with a key", func() { + for j := range keys { + key := keys[j] + Context(key.desc, func() { + BeforeEach(func() { + inKey, expKey = key.in, key.out + }) + + assertPathValues() + + Context("with a version", func() { + Context("that is zero", func() { + BeforeEach(func() { + inVersion, expVersion = "0", 0 + }) + + assertPathValues() + }) + + Context("that is positive", func() { + BeforeEach(func() { + inVersion, expVersion = "21", 21 + }) + + assertPathValues() + }) + }) + }) + } + }) + }) + } + }) + + Context("with a path that has an unescaped colon and a key", func() { + BeforeEach(func() { + inPath, inKey = "secret:foo", "bar" + expPath, expKey = "secret:foo", "bar" + }) + + assertPathValues() + }) + + Context("with a path that has an unescaped caret and a version", func() { + BeforeEach(func() { + inPath, inVersion = "secret^foo", "2" + expPath, expVersion = "secret^foo", 2 + }) + }) + }) +}) diff --git a/vault/vault.go b/vault/vault.go index 27f409c..1c30bb7 100644 --- a/vault/vault.go +++ b/vault/vault.go @@ -134,7 +134,6 @@ func (v *Vault) Curl(method string, path string, body []byte) (*http.Response, e // If there is nothing at that path, a nil *Secret will be returned, with no // error. func (v *Vault) Read(path string) (secret *Secret, err error) { - path = Canonicalize(path) path, key, version := ParsePath(path) secret = NewSecret() @@ -190,11 +189,15 @@ func (v *Vault) List(path string) (paths []string, err error) { // Write takes a Secret and writes it to the Vault at the specified path. func (v *Vault) Write(path string, s *Secret) error { - path = Canonicalize(path) - if strings.Contains(path, ":") { + path, key, version := ParsePath(path) + if key != "" { return fmt.Errorf("cannot write to paths in /path:key notation") } + if version != 0 { + return fmt.Errorf("cannot write to paths in /path^version notation") + } + if s.Empty() { return v.deleteIfPresent(path, DeleteOpts{}) } @@ -207,7 +210,7 @@ func (v *Vault) Write(path string, s *Secret) error { return err } -//errIfFolder returns an error with your provided message if the given path is a folder. +// errIfFolder returns an error with your provided message if the given path is a folder. // Can also throw an error if contacting the backend failed, in which case that error // is returned. func (v *Vault) errIfFolder(path, message string, args ...interface{}) error { @@ -316,8 +319,8 @@ func (v *Vault) verifySecretExists(path string) error { return err } -//DeleteTree recursively deletes the leaf nodes beneath the given root until -//the root has no children, and then deletes that. +// DeleteTree recursively deletes the leaf nodes beneath the given root until +// the root has no children, and then deletes that. func (v *Vault) DeleteTree(root string, opts DeleteOpts) error { root = Canonicalize(root) @@ -486,13 +489,13 @@ func (v *Vault) deleteSpecificKey(path string) error { return v.Write(secretPath, secret) } -//DeleteVersions marks the given versions of the given secret as deleted for +// DeleteVersions marks the given versions of the given secret as deleted for // a v2 backend or actually deletes it for a v1 backend. func (v *Vault) DeleteVersions(path string, versions []uint) error { return v.client.Delete(path, &vaultkv.KVDeleteOpts{Versions: versions, V1Destroy: true}) } -//DestroyVersions irrevocably destroys the given versions of the given secret +// DestroyVersions irrevocably destroys the given versions of the given secret func (v *Vault) DestroyVersions(path string, versions []uint) error { return v.client.Destroy(path, versions) } @@ -530,7 +533,7 @@ func (v *Vault) Undelete(path string) error { return v.Client().Undelete(secret, []uint{uint(version)}) } -//deleteIfPresent first checks to see if there is a Secret at the given path, +// deleteIfPresent first checks to see if there is a Secret at the given path, // and if so, it deletes it. Otherwise, no error is thrown func (v *Vault) deleteIfPresent(path string, opts DeleteOpts) error { secretpath, _, _ := ParsePath(path) @@ -699,7 +702,7 @@ func (v *Vault) Copy(oldpath, newpath string, opts MoveCopyOpts) error { return nil } -//MoveCopyTree will recursively copy all nodes from the root to the new location. +// MoveCopyTree will recursively copy all nodes from the root to the new location. // This function will get confused about 'secret:key' syntax, so don't let those // get routed here - they don't make sense for a recursion anyway. func (v *Vault) MoveCopyTree(oldRoot, newRoot string, f func(string, string, MoveCopyOpts) error, opts MoveCopyOpts) error {