Skip to content

Commit

Permalink
feat: add explicit_max_ttl to azure secrets (#199)
Browse files Browse the repository at this point in the history 
Add `explicit_max_ttl` to Azure role attributes. If set, Application
Secrets in Azure AD will be created with a maximum lifetime equal to
`explicit_max_ttl` instead of the hard-coded 10-year default in effect
until now.

Leases are renewable unless or until the remaining Azure-side lifetime
is shorter than the role's configured TTL. Marking a lease as
non-renewable signals to clients that they must obtain a new
lease/secret when the existing one is approaching the limit that was
originally set through `explicit_max_ttl`.

Fixes #178
Fixes VAULT-12316
  • Loading branch information
@gsantos-hc
gsantos-hc authored Jul 3, 2024
1 parent b9b349b commit bfe49f2
Show file tree
Hide file tree
Showing 7 changed files with 164 additions and 34 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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

0 comments on commit bfe49f2

Please sign in to comment.