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

feat: explicit max ttl for secrets #199

Merged
merged 1 commit into from
Jul 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
## Unreleased

FEATURES:
* Adds ability to limit the lifetime of service principal secrets in Azure through `explicit_max_ttl` on roles ([GH-199](https://github.com/hashicorp/vault-plugin-secrets-azure/pull/199))

## v0.19.0

IMPROVEMENTS:
Expand Down
24 changes: 13 additions & 11 deletions backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@ import (
)

const (
defaultLeaseTTLHr = 1 * time.Hour
maxLeaseTTLHr = 12 * time.Hour
defaultTestTTL = 300
defaultTestMaxTTL = 3600
defaultLeaseTTLHr = 1 * time.Hour
maxLeaseTTLHr = 12 * time.Hour
defaultTestTTL = 300
defaultTestMaxTTL = 3600
defaultTestExplicitMaxTTL = 7200
)

var (
Expand Down Expand Up @@ -59,13 +60,14 @@ func getTestBackendMocked(t *testing.T, initConfig bool) (*azureSecretBackend, l

if initConfig {
cfg := map[string]interface{}{
"subscription_id": generateUUID(),
"tenant_id": generateUUID(),
"client_id": testClientID,
"client_secret": testClientSecret,
"environment": "AZURECHINACLOUD",
"ttl": defaultTestTTL,
"max_ttl": defaultTestMaxTTL,
"subscription_id": generateUUID(),
"tenant_id": generateUUID(),
"client_id": testClientID,
"client_secret": testClientSecret,
"environment": "AZURECHINACLOUD",
"ttl": defaultTestTTL,
"max_ttl": defaultTestMaxTTL,
"explicit_max_ttl": defaultTestExplicitMaxTTL,
}

testConfigCreate(t, b, config.StorageView, cfg, false)
Expand Down
17 changes: 10 additions & 7 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,16 +80,18 @@ func (c *client) createAppWithName(ctx context.Context, rolename string, signInA
func (c *client) createSP(
ctx context.Context,
app api.Application,
duration time.Duration) (spID string, password string, err error) {
duration time.Duration) (spID string, password string, endDate time.Time, err error) {

type idPass struct {
ID string
Password string
EndDate time.Time
}

resultRaw, err := retry(ctx, func() (interface{}, bool, error) {
now := time.Now()
spID, password, err := c.provider.CreateServicePrincipal(ctx, app.AppID, now, now.Add(duration))
endDate := now.Add(duration)
spID, password, err := c.provider.CreateServicePrincipal(ctx, app.AppID, now, endDate)

// Propagation delays within Azure can cause this error occasionally, so don't quit on it.
if err != nil && (strings.Contains(err.Error(), errInvalidApplicationObject)) {
Expand All @@ -99,32 +101,33 @@ func (c *client) createSP(
result := idPass{
ID: spID,
Password: password,
EndDate: endDate,
}

return result, true, err
})

if err != nil {
return "", "", fmt.Errorf("error creating service principal: %w", err)
return "", "", time.Time{}, fmt.Errorf("error creating service principal: %w", err)
}

result := resultRaw.(idPass)

return result.ID, result.Password, nil
return result.ID, result.Password, result.EndDate, nil
}

// addAppPassword adds a new password to an App's credentials list.
func (c *client) addAppPassword(ctx context.Context, appObjID string, expiresIn time.Duration) (string, string, error) {
func (c *client) addAppPassword(ctx context.Context, appObjID string, expiresIn time.Duration) (string, string, time.Time, error) {
exp := time.Now().Add(expiresIn)
resp, err := c.provider.AddApplicationPassword(ctx, appObjID, "vault-plugin-secrets-azure", exp)
if err != nil {
if strings.Contains(err.Error(), "size of the object has exceeded its limit") {
err = errors.New("maximum number of Application passwords reached")
}
return "", "", fmt.Errorf("error updating credentials: %w", err)
return "", "", time.Time{}, fmt.Errorf("error updating credentials: %w", err)
}

return resp.KeyID, resp.SecretText, nil
return resp.KeyID, resp.SecretText, resp.EndDate, nil
}

// deleteAppPassword removes a password, if present, from an App's credentials list.
Expand Down
30 changes: 28 additions & 2 deletions path_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ type roleEntry struct {
Tags []string `json:"tags"`
TTL time.Duration `json:"ttl"`
MaxTTL time.Duration `json:"max_ttl"`
ExplicitMaxTTL time.Duration `json:"explicit_max_ttl"`
PermanentlyDelete bool `json:"permanently_delete"`
PersistApp bool `json:"persist_app"`

Expand Down Expand Up @@ -106,6 +107,10 @@ func pathsRole(b *azureSecretBackend) []*framework.Path {
Type: framework.TypeDurationSecond,
Description: "Maximum time a service principal. If not set or set to 0, will use system default.",
},
"explicit_max_ttl": {
Type: framework.TypeDurationSecond,
Description: "Maximum lifetime of the lease and service principal. If not set or set to 0, will use the system default.",
},
"permanently_delete": {
Type: framework.TypeBool,
Description: "Indicates whether new application objects should be permanently deleted. If not set, objects will not be permanently deleted.",
Expand Down Expand Up @@ -207,10 +212,24 @@ func (b *azureSecretBackend) pathRoleUpdate(ctx context.Context, req *logical.Re
role.MaxTTL = time.Duration(d.Get("max_ttl").(int)) * time.Second
}

if explicitMaxTTLRaw, ok := d.GetOk("explicit_max_ttl"); ok {
role.ExplicitMaxTTL = time.Duration(explicitMaxTTLRaw.(int)) * time.Second
} else if req.Operation == logical.CreateOperation {
role.ExplicitMaxTTL = time.Duration(d.Get("explicit_max_ttl").(int)) * time.Second
}

if role.MaxTTL != 0 && role.TTL > role.MaxTTL {
return logical.ErrorResponse("ttl cannot be greater than max_ttl"), nil
}

if role.ExplicitMaxTTL != 0 && role.TTL > role.ExplicitMaxTTL {
return logical.ErrorResponse("ttl cannot be greater than explicit_max_ttl"), nil
}

if role.ExplicitMaxTTL != 0 && role.MaxTTL > role.ExplicitMaxTTL {
return logical.ErrorResponse("max_ttl cannot be greater than explicit_max_ttl"), nil
}

// load and verify deletion options
if permanentlyDeleteRaw, ok := d.GetOk("permanently_delete"); ok {
role.PermanentlyDelete = permanentlyDeleteRaw.(bool)
Expand Down Expand Up @@ -463,8 +482,14 @@ func (b *azureSecretBackend) createPersistedApp(ctx context.Context, req *logica
return fmt.Errorf("error writing WAL: %w", err)
}

// TODO: should we expire the PW?
spObjID, _, err := c.createSP(ctx, app, spExpiration)
// Determine SP duration
spDuration := spExpiration
if role.ExplicitMaxTTL != 0 {
spDuration = role.ExplicitMaxTTL
}

// Create the SP
spObjID, _, _, err := c.createSP(ctx, app, spDuration)
if err != nil {
return err
}
Expand Down Expand Up @@ -520,6 +545,7 @@ func (b *azureSecretBackend) pathRoleRead(ctx context.Context, req *logical.Requ
Data: map[string]interface{}{
"ttl": r.TTL / time.Second,
"max_ttl": r.MaxTTL / time.Second,
"explicit_max_ttl": r.ExplicitMaxTTL / time.Second,
"azure_roles": r.AzureRoles,
"azure_groups": r.AzureGroups,
"application_object_id": r.ApplicationObjectID,
Expand Down
35 changes: 25 additions & 10 deletions path_roles_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ func TestRoleCreate(t *testing.T) {
}]`),
"ttl": int64(0),
"max_ttl": int64(0),
"explicit_max_ttl": int64(0),
"application_object_id": "",
"permanently_delete": true,
"persist_app": false,
Expand Down Expand Up @@ -74,6 +75,7 @@ func TestRoleCreate(t *testing.T) {
}]`),
"ttl": int64(300),
"max_ttl": int64(3000),
"explicit_max_ttl": int64(3000),
"application_object_id": "",
"permanently_delete": true,
"persist_app": false,
Expand Down Expand Up @@ -124,6 +126,7 @@ func TestRoleCreate(t *testing.T) {
}]`),
"ttl": int64(0),
"max_ttl": int64(0),
"explicit_max_ttl": int64(0),
"application_object_id": "",
"persist_app": true,
}
Expand Down Expand Up @@ -151,6 +154,7 @@ func TestRoleCreate(t *testing.T) {
}]`),
"ttl": int64(300),
"max_ttl": int64(3000),
"explicit_max_ttl": int64(3000),
"application_object_id": "",
"persist_app": true,
}
Expand Down Expand Up @@ -198,6 +202,7 @@ func TestRoleCreate(t *testing.T) {
"application_object_id": "00000000-0000-0000-0000-000000000000",
"ttl": int64(300),
"max_ttl": int64(3000),
"explicit_max_ttl": int64(3000),
"azure_roles": "[]",
"azure_groups": "[]",
"sign_in_audience": "PersonalMicrosoftAccount",
Expand Down Expand Up @@ -232,13 +237,14 @@ func TestRoleCreate(t *testing.T) {
"persist_app": false,
}

// Verify that ttl and max_ttl are 0 if not provided
// Verify that ttl, max_ttl and explicit_max_ttl are 0 if not provided
// and that permanently_delete is false if not provided
name := generateUUID()
testRoleCreate(t, b, s, name, testRole)

testRole["ttl"] = int64(0)
testRole["max_ttl"] = int64(0)
testRole["explicit_max_ttl"] = int64(0)
testRole["permanently_delete"] = false

resp, err := testRoleRead(t, b, s, name)
Expand All @@ -253,16 +259,21 @@ func TestRoleCreate(t *testing.T) {

const skip = -999
tests := []struct {
ttl int64
maxTTL int64
expError bool
ttl int64
maxTTL int64
explicitMaxTTL int64
expError bool
}{
{5, 10, false},
{5, skip, false},
{skip, 10, false},
{100, 100, false},
{101, 100, true},
{101, 0, false},
{5, 10, skip, false},
{5, skip, skip, false},
{skip, 10, skip, false},
{100, 100, skip, false},
{101, 100, skip, true},
{101, 0, skip, false},
{5, 10, 10, false},
{10, skip, 5, true},
{10, 10, 5, true},
{skip, skip, 10, false},
}

for i, test := range tests {
Expand All @@ -276,6 +287,9 @@ func TestRoleCreate(t *testing.T) {
if test.maxTTL != skip {
role["max_ttl"] = test.maxTTL
}
if test.explicitMaxTTL != skip {
role["explicit_max_ttl"] = test.explicitMaxTTL
}
name := generateUUID()
resp, err := b.HandleRequest(context.Background(), &logical.Request{
Operation: logical.CreateOperation,
Expand Down Expand Up @@ -864,4 +878,5 @@ func convertRespTypes(data map[string]interface{}) {
}
data["ttl"] = int64(data["ttl"].(time.Duration))
data["max_ttl"] = int64(data["max_ttl"].(time.Duration))
data["explicit_max_ttl"] = int64(data["explicit_max_ttl"].(time.Duration))
}
38 changes: 34 additions & 4 deletions path_service_principal.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const (
)

// SPs will be created with a far-future expiration in Azure
// unless `explicit_max_ttl` is set in the role
var spExpiration = 10 * 365 * 24 * time.Hour

func secretServicePrincipal(b *azureSecretBackend) *framework.Secret {
Expand Down Expand Up @@ -97,6 +98,9 @@ func (b *azureSecretBackend) pathSPRead(ctx context.Context, req *logical.Reques

resp.Secret.TTL = role.TTL
resp.Secret.MaxTTL = role.MaxTTL
if role.ExplicitMaxTTL != 0 && (role.ExplicitMaxTTL < role.MaxTTL || role.MaxTTL == 0) {
resp.Secret.MaxTTL = role.ExplicitMaxTTL
}
return resp, nil
}

Expand All @@ -122,8 +126,14 @@ func (b *azureSecretBackend) createSPSecret(ctx context.Context, s logical.Stora
return nil, fmt.Errorf("error writing WAL: %w", err)
}

// Determine SP duration
spDuration := spExpiration
if role.ExplicitMaxTTL != 0 {
spDuration = role.ExplicitMaxTTL
}

// Create a service principal associated with the new App
spID, password, err := c.createSP(ctx, app, spExpiration)
spID, password, endDate, err := c.createSP(ctx, app, spDuration)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -175,6 +185,7 @@ func (b *azureSecretBackend) createSPSecret(ctx context.Context, s logical.Stora
"group_membership_ids": groupObjectIDs(role.AzureGroups),
"role": roleName,
"permanently_delete": role.PermanentlyDelete,
"key_end_date": endDate.Format(time.RFC3339Nano),
}

return b.Secret(SecretTypeSP).Response(data, internalData), nil
Expand All @@ -186,7 +197,13 @@ func (b *azureSecretBackend) createStaticSPSecret(ctx context.Context, c *client
lock.Lock()
defer lock.Unlock()

keyID, password, err := c.addAppPassword(ctx, role.ApplicationObjectID, spExpiration)
// Determine SP duration
spDuration := spExpiration
if role.ExplicitMaxTTL != 0 {
spDuration = role.ExplicitMaxTTL
}

keyID, password, endDate, err := c.addAppPassword(ctx, role.ApplicationObjectID, spDuration)
if err != nil {
return nil, err
}
Expand All @@ -198,6 +215,7 @@ func (b *azureSecretBackend) createStaticSPSecret(ctx context.Context, c *client
internalData := map[string]interface{}{
"app_object_id": role.ApplicationObjectID,
"key_id": keyID,
"key_end_date": endDate.Format(time.RFC3339Nano),
"role": roleName,
}

Expand All @@ -219,9 +237,21 @@ func (b *azureSecretBackend) spRenew(ctx context.Context, req *logical.Request,
return nil, nil
}

// Determine remaining lifetime of SP secret in Azure
keyEndDateRaw, ok := req.Secret.InternalData["key_end_date"]
if !ok {
return nil, errors.New("internal data 'key_end_date' not found")
}
keyEndDate, err := time.Parse(time.RFC3339Nano, keyEndDateRaw.(string))
if err != nil {
return nil, errors.New("cannot parse 'key_end_date' to timestamp")
}
keyLifetime := time.Until(keyEndDate)

resp := &logical.Response{Secret: req.Secret}
resp.Secret.TTL = role.TTL
resp.Secret.MaxTTL = role.MaxTTL
resp.Secret.TTL = min(role.TTL, keyLifetime)
resp.Secret.MaxTTL = min(role.MaxTTL, keyLifetime)
resp.Secret.Renewable = role.TTL < keyLifetime // Lease cannot be renewed beyond service-side endDate

return resp, nil
}
Expand Down
Loading