Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Cache whether we've been initialized to reduce load on storage #7549

Merged
merged 3 commits into from
Oct 8, 2019
Merged

Cache whether we've been initialized to reduce load on storage #7549

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7f47d57
In order to prevent a potential DOS by querying the unauthenticated
ncabatoff Oct 2, 2019
12274d7
go mod vendor and tidy.
ncabatoff Oct 2, 2019
2abf1c2
Merge branch 'master' into cache-initialized-status
ncabatoff Oct 8, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ require (
github.com/joyent/triton-go v0.0.0-20190112182421-51ffac552869
github.com/keybase/go-crypto v0.0.0-20190403132359-d65b6b94177f
github.com/kr/pretty v0.1.0
github.com/kr/pty v1.1.3 // indirect
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please go mod tidy. This is really annoying because for some reason right now this keeps getting added on build and removed on go mod tidy. (Are you on go 1.12.10? I only noticed it after upgrading to that, although it may be unrelated.) However, for the moment we're not committing build-time only changes since it'll be pulled right back out on the next tidy.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My IDE (goland) also auto-adds it back, very annoying I agree.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try merging in master, it should be fixed (I no longer get on my diffs).

github.com/kr/text v0.1.0
github.com/lib/pq v1.2.0
github.com/mattn/go-colorable v0.1.2
Expand Down Expand Up @@ -120,6 +121,7 @@ require (
github.com/stretchr/testify v1.3.0
go.etcd.io/bbolt v1.3.2
go.etcd.io/etcd v0.0.0-20190412021913-f29b1ada1971
go.uber.org/atomic v1.4.0
golang.org/x/crypto v0.0.0-20190701094942-4def268fd1a4
golang.org/x/net v0.0.0-20190813141303-74dc4d7220e7
golang.org/x/oauth2 v0.0.0-20190402181905-9f3314589c9a
Expand Down
2 changes: 2 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,8 @@ go.opencensus.io v0.21.0 h1:mU6zScU4U1YAFPHEHYk+3JC4SY7JxgkqS10ZOSyksNg=
go.opencensus.io v0.21.0/go.mod h1:mSImk1erAIZhrmZN+AvHh14ztQfjbGwt4TtuofqLduU=
go.uber.org/atomic v1.3.2 h1:2Oa65PReHzfn29GpvgsYwloV9AVFHPDk8tYxt2c2tr4=
go.uber.org/atomic v1.3.2/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE=
go.uber.org/atomic v1.4.0 h1:cxzIVoETapQEqDhQu3QfnvXAV4AlzcvUCxkVUFw3+EU=
go.uber.org/atomic v1.4.0/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE=
go.uber.org/multierr v1.1.0 h1:HoEmRHQPVSqub6w2z2d2EOVs2fjyFRGyofhKuyDq0QI=
go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/0=
go.uber.org/zap v1.9.1 h1:XCJQEf3W6eZaVwhRBof6ImoYGJSITeKWsyeh3HFu/5o=
Expand Down
1 change: 1 addition & 0 deletions sdk/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ require (
golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3 // indirect
golang.org/x/sys v0.0.0-20190403152447-81d4e9dc473e
golang.org/x/text v0.3.1-0.20181227161524-e6919f6577db // indirect
google.golang.org/appengine v1.4.0 // indirect
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also here

google.golang.org/genproto v0.0.0-20190404172233-64821d5d2107 // indirect
google.golang.org/grpc v1.22.0
gopkg.in/asn1-ber.v1 v1.0.0-20181015200546-f715ec2f112d // indirect
Expand Down
11 changes: 10 additions & 1 deletion vault/barrier_aes_gcm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,13 @@ import (
"sync"
"time"

metrics "github.com/armon/go-metrics"
"github.com/armon/go-metrics"
"github.com/hashicorp/errwrap"
"github.com/hashicorp/vault/sdk/helper/jsonutil"
"github.com/hashicorp/vault/sdk/helper/strutil"
"github.com/hashicorp/vault/sdk/logical"
"github.com/hashicorp/vault/sdk/physical"
"go.uber.org/atomic"
)

const (
Expand Down Expand Up @@ -69,6 +70,8 @@ type AESGCMBarrier struct {
// future versioning of barrier implementations. It's var instead
// of const to allow for testing
currentAESGCMVersionByte byte

initialized atomic.Bool
}

// NewAESGCMBarrier is used to construct a new barrier that uses
Expand All @@ -86,12 +89,17 @@ func NewAESGCMBarrier(physical physical.Backend) (*AESGCMBarrier, error) {
// Initialized checks if the barrier has been initialized
// and has a master key set.
func (b *AESGCMBarrier) Initialized(ctx context.Context) (bool, error) {
if b.initialized.Load() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see why using Uber's atomic package is convenient here since we want to return bool, but I wonder if we could simply use the stdlib's atomic.LoadUint32 vs importing an additional library.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was Jeff's suggestion. He likes the API the library provides and thought this would be a convenient occasion to give it a try. I concurred.

return true, nil
}

// Read the keyring file
keys, err := b.backend.List(ctx, keyringPrefix)
if err != nil {
return false, errwrap.Wrapf("failed to check for initialization: {{err}}", err)
}
if strutil.StrListContains(keys, "keyring") {
b.initialized.Store(true)
return true, nil
}

Expand All @@ -100,6 +108,7 @@ func (b *AESGCMBarrier) Initialized(ctx context.Context) (bool, error) {
if err != nil {
return false, errwrap.Wrapf("failed to check for initialization: {{err}}", err)
}
b.initialized.Store(out != nil)
return out != nil, nil
}

Expand Down