From ea96a246fc9b959c9e1b87a4d18533011093a7e0 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 8 May 2017 10:42:49 -0700 Subject: [PATCH] audit: hash time.Time values in map fields This enables audit.Hash to hash time.Time values that may exist as direct fields in the map. This will error (instead of panic) for any time.Time values that don't occur within map values. For example, this does not support a time.Time within a slice. If that needs to be supported then modifications will need to be made. This also requires an update to reflectwalk (included in this PR). This is a minimal change that allows SkipEntry to signal to skip an entire struct. We do this because we don't want to walk any of time.Time since we handle it directly. --- audit/hashstructure.go | 43 +++++++++++++++ audit/hashstructure_test.go | 5 ++ .../mitchellh/reflectwalk/reflectwalk.go | 55 +++++++++++-------- vendor/vendor.json | 6 +- 4 files changed, 83 insertions(+), 26 deletions(-) diff --git a/audit/hashstructure.go b/audit/hashstructure.go index ea0899ee9731..3d38a0612615 100644 --- a/audit/hashstructure.go +++ b/audit/hashstructure.go @@ -1,8 +1,10 @@ package audit import ( + "errors" "reflect" "strings" + "time" "github.com/hashicorp/vault/helper/salt" "github.com/hashicorp/vault/helper/wrapping" @@ -141,6 +143,12 @@ type hashWalker struct { unknownKeys []string } +// hashTimeType stores a pre-computed reflect.Type for a time.Time so +// we can quickly compare in hashWalker.Struct. We create an empty/invalid +// time.Time{} so we don't need to incur any additional startup cost vs. +// Now() or Unix(). +var hashTimeType = reflect.TypeOf(time.Time{}) + func (w *hashWalker) Enter(loc reflectwalk.Location) error { w.loc = loc return nil @@ -188,6 +196,41 @@ func (w *hashWalker) SliceElem(i int, elem reflect.Value) error { return nil } +func (w *hashWalker) Struct(v reflect.Value) error { + // We are looking for time values. If it isn't one, ignore it. + if v.Type() != hashTimeType { + return nil + } + + // If we aren't in a map value, return an error to prevent a panic + if v.Interface() != w.lastValue.Interface() { + return errors.New("time.Time value in a non map key cannot be hashed for audits") + } + + // Override location to be a MapValue. loc is set to None since we + // already "entered" the struct. We could do better here by keeping + // a stack of locations and checking the last entry. + w.loc = reflectwalk.MapValue + + // Create a string value of the time. IMPORTANT: this must never change + // across Vault versions or the hash value of equivalent time.Time will + // change. + strVal := v.Interface().(time.Time).UTC().Format(time.RFC3339Nano) + + // Walk it as if it were a primitive value with the string value. + // This will replace the currenty map value (which is a time.Time). + if err := w.Primitive(reflect.ValueOf(strVal)); err != nil { + return err + } + + // Skip this entry so that we don't walk the struct. + return reflectwalk.SkipEntry +} + +func (w *hashWalker) StructField(reflect.StructField, reflect.Value) error { + return nil +} + func (w *hashWalker) Primitive(v reflect.Value) error { if w.Callback == nil { return nil diff --git a/audit/hashstructure_test.go b/audit/hashstructure_test.go index 6916d0d3a308..5e6dafa54e67 100644 --- a/audit/hashstructure_test.go +++ b/audit/hashstructure_test.go @@ -140,6 +140,10 @@ func TestHash(t *testing.T) { &logical.Response{ Data: map[string]interface{}{ "foo": "bar", + + // Responses can contain time values, so test that with + // a known fixed value. + "bar": time.Unix(1494264707, 0), }, WrapInfo: &wrapping.ResponseWrapInfo{ TTL: 60, @@ -151,6 +155,7 @@ func TestHash(t *testing.T) { &logical.Response{ Data: map[string]interface{}{ "foo": "hmac-sha256:f9320baf0249169e73850cd6156ded0106e2bb6ad8cab01b7bbbebe6d1065317", + "bar": "hmac-sha256:b09b815a7d1c3bbcf702f9c9a50ef6408d0935bea0154383a128ca8743eb06fc", }, WrapInfo: &wrapping.ResponseWrapInfo{ TTL: 60, diff --git a/vendor/github.com/mitchellh/reflectwalk/reflectwalk.go b/vendor/github.com/mitchellh/reflectwalk/reflectwalk.go index 33287088492a..d7ab7b6d782a 100644 --- a/vendor/github.com/mitchellh/reflectwalk/reflectwalk.go +++ b/vendor/github.com/mitchellh/reflectwalk/reflectwalk.go @@ -72,6 +72,7 @@ type PointerWalker interface { // SkipEntry can be returned from walk functions to skip walking // the value of this field. This is only valid in the following functions: // +// - Struct: skips all fields from being walked // - StructField: skips walking the struct value // var SkipEntry = errors.New("skip this entry") @@ -345,42 +346,50 @@ func walkStruct(v reflect.Value, w interface{}) (err error) { ew.Enter(Struct) } + skip := false if sw, ok := w.(StructWalker); ok { - if err = sw.Struct(v); err != nil { + err = sw.Struct(v) + if err == SkipEntry { + skip = true + err = nil + } + if err != nil { return } } - vt := v.Type() - for i := 0; i < vt.NumField(); i++ { - sf := vt.Field(i) - f := v.FieldByIndex([]int{i}) + if !skip { + vt := v.Type() + for i := 0; i < vt.NumField(); i++ { + sf := vt.Field(i) + f := v.FieldByIndex([]int{i}) + + if sw, ok := w.(StructWalker); ok { + err = sw.StructField(sf, f) + + // SkipEntry just pretends this field doesn't even exist + if err == SkipEntry { + continue + } - if sw, ok := w.(StructWalker); ok { - err = sw.StructField(sf, f) + if err != nil { + return + } + } - // SkipEntry just pretends this field doesn't even exist - if err == SkipEntry { - continue + ew, ok := w.(EnterExitWalker) + if ok { + ew.Enter(StructField) } + err = walk(f, w) if err != nil { return } - } - ew, ok := w.(EnterExitWalker) - if ok { - ew.Enter(StructField) - } - - err = walk(f, w) - if err != nil { - return - } - - if ok { - ew.Exit(StructField) + if ok { + ew.Exit(StructField) + } } } diff --git a/vendor/vendor.json b/vendor/vendor.json index ddcd78a8b032..7d340003edde 100644 --- a/vendor/vendor.json +++ b/vendor/vendor.json @@ -1129,10 +1129,10 @@ "revisionTime": "2017-03-07T20:11:23Z" }, { - "checksumSHA1": "67Y6z6rHipvOvFwCZZXqKH+TWao=", + "checksumSHA1": "KqsMqI+Y+3EFYPhyzafpIneaVCM=", "path": "github.com/mitchellh/reflectwalk", - "revision": "417edcfd99a4d472c262e58f22b4bfe97580f03e", - "revisionTime": "2017-01-10T16:52:07Z" + "revision": "8d802ff4ae93611b807597f639c19f76074df5c6", + "revisionTime": "2017-05-08T17:38:06Z" }, { "checksumSHA1": "BxxkAJ/Nm61PybCXvQIZJwyTj3Y=",