Skip to content

Commit

Permalink
audit: hash time.Time values in map fields
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mitchellh committed May 8, 2017
1 parent dc2794d commit ea96a24
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 26 deletions.
43 changes: 43 additions & 0 deletions audit/hashstructure.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package audit

import (
"errors"
"reflect"
"strings"
"time"

"github.com/hashicorp/vault/helper/salt"
"github.com/hashicorp/vault/helper/wrapping"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions audit/hashstructure_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
55 changes: 32 additions & 23 deletions vendor/github.com/mitchellh/reflectwalk/reflectwalk.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions vendor/vendor.json
Original file line number Diff line number Diff line change
Expand Up @@ -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=",
Expand Down

0 comments on commit ea96a24

Please sign in to comment.