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.

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

Add the ability to view and list of leases metadata #2650

Merged
merged 18 commits into from
May 4, 2017
Merged
1 change: 1 addition & 0 deletions http/logical_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ func TestLogical_StandbyRedirect(t *testing.T) {
"ttl": json.Number("0"),
"creation_ttl": json.Number("0"),
"explicit_max_ttl": json.Number("0"),
"expire_time": nil,
},
"warnings": nilWarnings,
"wrap_info": nil,
Expand Down
2 changes: 2 additions & 0 deletions http/sys_generate_root_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,7 @@ func TestSysGenerateRoot_Update_OTP(t *testing.T) {
"ttl": json.Number("0"),
"path": "auth/token/root",
"explicit_max_ttl": json.Number("0"),
"expire_time": nil,
}

resp = testHttpGet(t, newRootToken, addr+"/v1/auth/token/lookup-self")
Expand Down Expand Up @@ -401,6 +402,7 @@ func TestSysGenerateRoot_Update_PGP(t *testing.T) {
"ttl": json.Number("0"),
"path": "auth/token/root",
"explicit_max_ttl": json.Number("0"),
"expire_time": nil,
}

resp = testHttpGet(t, newRootToken, addr+"/v1/auth/token/lookup-self")
Expand Down
41 changes: 23 additions & 18 deletions vault/expiration.go
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ func (m *ExpirationManager) Renew(leaseID string, increment time.Duration) (*log
}

// Check if the lease is renewable
if err := le.renewable(); err != nil {
if _, err := le.renewable(); err != nil {
return nil, err
}

Expand Down Expand Up @@ -450,7 +450,7 @@ func (m *ExpirationManager) RenewToken(req *logical.Request, source string, toke

// Check if the lease is renewable. Note that this also checks for a nil
// lease and errors in that case as well.
if err := le.renewable(); err != nil {
if _, err := le.renewable(); err != nil {
return logical.ErrorResponse(err.Error()), logical.ErrInvalidRequest
}

Expand Down Expand Up @@ -841,29 +841,34 @@ type leaseEntry struct {
}

// encode is used to JSON encode the lease entry
func (l *leaseEntry) encode() ([]byte, error) {
return json.Marshal(l)
func (le *leaseEntry) encode() ([]byte, error) {
return json.Marshal(le)
}

func (le *leaseEntry) renewable() error {
func (le *leaseEntry) renewable() (bool, error) {
var err error
switch {
// If there is no entry, cannot review
if le == nil || le.ExpireTime.IsZero() {
return fmt.Errorf("lease not found or lease is not renewable")
}

case le == nil || le.ExpireTime.IsZero():
err = fmt.Errorf("lease not found or lease is not renewable")
// Determine if the lease is expired
if le.ExpireTime.Before(time.Now()) {
return fmt.Errorf("lease expired")
}

case le.ExpireTime.Before(time.Now()):
err = fmt.Errorf("lease expired")
// Determine if the lease is renewable
if le.Secret != nil && !le.Secret.Renewable {
return fmt.Errorf("lease is not renewable")
case le.Secret != nil && !le.Secret.Renewable:
err = fmt.Errorf("lease is not renewable")
case le.Auth != nil && !le.Auth.Renewable:
err = fmt.Errorf("lease is not renewable")
}
if le.Auth != nil && !le.Auth.Renewable {
return fmt.Errorf("lease is not renewable")

if err != nil {
return false, err
}
return nil
return true, nil
}

func (le *leaseEntry) ttl() int64 {
return int64(le.ExpireTime.Sub(time.Now().Round(time.Second)).Seconds())
}

// decodeLeaseEntry is used to reverse encode and return a new entry
Expand Down
34 changes: 33 additions & 1 deletion vault/expiration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1075,7 +1075,8 @@ func TestLeaseEntry(t *testing.T) {
},
Secret: &logical.Secret{
LeaseOptions: logical.LeaseOptions{
TTL: time.Minute,
TTL: time.Minute,
Renewable: true,
},
},
IssueTime: time.Now(),
Expand All @@ -1095,6 +1096,37 @@ func TestLeaseEntry(t *testing.T) {
if !reflect.DeepEqual(out.Data, le.Data) {
t.Fatalf("got: %#v, expect %#v", out, le)
}

// Test renewability
le.ExpireTime = time.Time{}
if r, _ := le.renewable(); r {
t.Fatal("lease with zero expire time is not renewable")
}
le.ExpireTime = time.Now().Add(-1 * time.Hour)
if r, _ := le.renewable(); r {
t.Fatal("lease with expire time in the past is not renewable")
}
le.ExpireTime = time.Now().Add(1 * time.Hour)
if r, err := le.renewable(); !r {
t.Fatalf("lease with future expire time is renewable, err: %v", err)
}
le.Secret.LeaseOptions.Renewable = false
if r, _ := le.renewable(); r {
t.Fatal("secret is set to not be renewable but returns as renewable")
}
le.Secret = nil
le.Auth = &logical.Auth{
LeaseOptions: logical.LeaseOptions{
Renewable: true,
},
}
if r, err := le.renewable(); !r {
t.Fatalf("auth is renewable but is set to not be, err: %v", err)
}
le.Auth.LeaseOptions.Renewable = false
if r, _ := le.renewable(); r {
t.Fatal("auth is set to not be renewable but returns as renewable")
}
}

func TestExpiration_RevokeForce(t *testing.T) {
Expand Down
122 changes: 117 additions & 5 deletions vault/logical_system.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,16 @@ func NewSystemBackend(core *Core, config *logical.BackendConfig) (logical.Backen
Root: []string{
"auth/*",
"remount",
"revoke-prefix/*",
"audit",
"audit/*",
"raw/*",
"replication/primary/secondary-token",
"replication/reindex",
"rotate",
"config/auditing/*",
"leases/revoke-prefix/*",
"revoke-prefix/*",
"leases/lookup/*",
},

Unauthenticated: []string{
Expand Down Expand Up @@ -299,7 +301,43 @@ func NewSystemBackend(core *Core, config *logical.BackendConfig) (logical.Backen
},

&framework.Path{
Pattern: "renew" + framework.OptionalParamRegex("url_lease_id"),
Pattern: "leases/lookup/(?P<prefix>.+?)?",

Fields: map[string]*framework.FieldSchema{
"prefix": &framework.FieldSchema{
Type: framework.TypeString,
Description: strings.TrimSpace(sysHelp["leases-list-prefix"][0]),
},
},

Callbacks: map[logical.Operation]framework.OperationFunc{
logical.ListOperation: b.handleLeaseLookupList,
},

HelpSynopsis: strings.TrimSpace(sysHelp["leases"][0]),
HelpDescription: strings.TrimSpace(sysHelp["leases"][1]),
},

&framework.Path{
Pattern: "leases/lookup",
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want this to be a root protected path?

Copy link
Member

Choose a reason for hiding this comment

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

@briankassouf Since lease/lookup doesn't return sensitive information (e.g. the contents of internal data) and you have to know the lease ID ahead of time I don't think it needs to be root. In fact, I am actually thinking we may want to add it to the default policy. Listing on the other hand does divulge lease IDs which can be used to e.g. renew them, so that should be root protected.


Fields: map[string]*framework.FieldSchema{
"lease_id": &framework.FieldSchema{
Type: framework.TypeString,
Description: strings.TrimSpace(sysHelp["lease_id"][0]),
},
},

Callbacks: map[logical.Operation]framework.OperationFunc{
logical.UpdateOperation: b.handleLeaseLookup,
},

HelpSynopsis: strings.TrimSpace(sysHelp["leases"][0]),
HelpDescription: strings.TrimSpace(sysHelp["leases"][1]),
},

&framework.Path{
Pattern: "(leases/)?renew" + framework.OptionalParamRegex("url_lease_id"),

Fields: map[string]*framework.FieldSchema{
"url_lease_id": &framework.FieldSchema{
Expand All @@ -325,7 +363,7 @@ func NewSystemBackend(core *Core, config *logical.BackendConfig) (logical.Backen
},

&framework.Path{
Pattern: "revoke" + framework.OptionalParamRegex("url_lease_id"),
Pattern: "(leases/)?revoke" + framework.OptionalParamRegex("url_lease_id"),
Copy link
Contributor

Choose a reason for hiding this comment

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

👍


Fields: map[string]*framework.FieldSchema{
"url_lease_id": &framework.FieldSchema{
Expand All @@ -347,7 +385,7 @@ func NewSystemBackend(core *Core, config *logical.BackendConfig) (logical.Backen
},

&framework.Path{
Pattern: "revoke-force/(?P<prefix>.+)",
Pattern: "(leases/)?revoke-force/(?P<prefix>.+)",

Fields: map[string]*framework.FieldSchema{
"prefix": &framework.FieldSchema{
Expand All @@ -365,7 +403,7 @@ func NewSystemBackend(core *Core, config *logical.BackendConfig) (logical.Backen
},

&framework.Path{
Pattern: "revoke-prefix/(?P<prefix>.+)",
Pattern: "(leases/)?revoke-prefix/(?P<prefix>.+)",

Fields: map[string]*framework.FieldSchema{
"prefix": &framework.FieldSchema{
Expand Down Expand Up @@ -686,6 +724,7 @@ func NewSystemBackend(core *Core, config *logical.BackendConfig) (logical.Backen
HelpSynopsis: strings.TrimSpace(sysHelp["audited-headers-name"][0]),
HelpDescription: strings.TrimSpace(sysHelp["audited-headers-name"][1]),
},

&framework.Path{
Pattern: "config/auditing/request-headers$",

Expand Down Expand Up @@ -1274,6 +1313,61 @@ func (b *SystemBackend) handleTuneWriteCommon(
return nil, nil
}

// handleLease is use to view the metadata for a given LeaseID
Copy link
Member

Choose a reason for hiding this comment

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

s/handleLease/handleLeaseLookup

func (b *SystemBackend) handleLeaseLookup(
req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
leaseID := data.Get("lease_id").(string)
if leaseID == "" {
return logical.ErrorResponse("lease_id must be specified"),
logical.ErrInvalidRequest
}

Copy link
Member

Choose a reason for hiding this comment

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

Can we have a check for leaseID == "" here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do.

leaseTimes, err := b.Core.expiration.FetchLeaseTimes(leaseID)
if err != nil {
b.Backend.Logger().Error("sys: error retrieving lease", "lease_id", leaseID, "error", err)
return handleError(err)
}
if leaseTimes == nil {
return logical.ErrorResponse("invalid lease"), logical.ErrInvalidRequest
}

Copy link
Member

Choose a reason for hiding this comment

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

Can we add a comment here explaining why we are setting values to nil beforehand. This will avoid people removing it in future.

resp := &logical.Response{
Data: map[string]interface{}{
"id": leaseID,
"issue_time": leaseTimes.IssueTime,
"expire_time": nil,
"last_renewal": nil,
"ttl": int64(0),
},
}
renewable, _ := leaseTimes.renewable()
resp.Data["renewable"] = renewable

if !leaseTimes.LastRenewalTime.IsZero() {
Copy link
Member

Choose a reason for hiding this comment

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

Sorry for circling around on this. On a second thought, the idea of having fields with default values (like you were doing earlier) does seem like a sensible thing to do too. What might be of concern is that this API will not have deterministic number of response fields. That is okay I guess but I am not sure if that is a problem. Bringing this up so we can all agree on this and move forward.

resp.Data["last_renewal"] = leaseTimes.LastRenewalTime
}
if !leaseTimes.ExpireTime.IsZero() {
resp.Data["expire_time"] = leaseTimes.ExpireTime
resp.Data["ttl"] = leaseTimes.ttl()
}
return resp, nil
}

func (b *SystemBackend) handleLeaseLookupList(
req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
prefix := data.Get("prefix").(string)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a check to make sure prefix is not ""

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Empty prefix is valid and will list the top level keys.

if prefix != "" && !strings.HasSuffix(prefix, "/") {
prefix = prefix + "/"
}

keys, err := b.Core.expiration.idView.List(prefix)
if err != nil {
b.Backend.Logger().Error("sys: error listing leases", "prefix", prefix, "error", err)
return handleError(err)
}
return logical.ListResponse(keys), nil
}

// handleRenew is used to renew a lease with a given LeaseID
func (b *SystemBackend) handleRenew(
req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
Expand Down Expand Up @@ -2429,4 +2523,22 @@ This path responds to the following HTTP methods.
"Lists the headers configured to be audited.",
`Returns a list of headers that have been configured to be audited.`,
},

"leases": {
`View or list lease metadata.`,
`
This path responds to the following HTTP methods.

PUT /
Retrieve the metadata for the provided lease id.

LIST /<prefix>
Lists the leases for the named prefix.
`,
},

"leases-list-prefix": {
`The path to list leases under. Example: "aws/creds/deploy"`,
"",
},
}
Loading