Integrate password policies (#40)

* Added `password_policy` field to config that references the new `GeneratePasswordFromPolicy` available on `SystemView`
* Improved timeout handling on `retry` (adheres to context timeout while still differentiating between timed out and cancelled)
* Improved timeout assertion
* Updated go to 1.13.7
* Improved RNG handling

.travis.yml

@@ -1,6 +1,6 @@
 language: go
 go:
-  - "1.12"
+  - "1.13.7"
 env:
 - GO111MODULE=on
 script:

backend.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"strings"
 	"sync"
+	"time"
 
 	"github.com/hashicorp/vault/sdk/framework"
 	"github.com/hashicorp/vault/sdk/helper/locksutil"
@@ -80,6 +81,61 @@ func (b *azureSecretBackend) invalidate(ctx context.Context, key string) {
 	}
 }
 
+func (b *azureSecretBackend) getClient(ctx context.Context, s logical.Storage) (*client, error) {
+	b.lock.RLock()
+	unlockFunc := b.lock.RUnlock
+	defer func() { unlockFunc() }()
+
+	if b.client.Valid() {
+		return b.client, nil
+	}
+
+	b.lock.RUnlock()
+	b.lock.Lock()
+	unlockFunc = b.lock.Unlock
+
+	if b.client.Valid() {
+		return b.client, nil
+	}
+
+	config, err := b.getConfig(ctx, s)
+	if err != nil {
+		return nil, err
+	}
+
+	if b.settings == nil {
+		if config == nil {
+			config = new(azureConfig)
+		}
+
+		settings, err := b.getClientSettings(ctx, config)
+		if err != nil {
+			return nil, err
+		}
+		b.settings = settings
+	}
+
+	p, err := b.getProvider(b.settings)
+	if err != nil {
+		return nil, err
+	}
+
+	passwords := passwords{
+		policyGenerator: b.System(),
+		policyName:      config.PasswordPolicy,
+	}
+
+	c := &client{
+		provider:   p,
+		settings:   b.settings,
+		expiration: time.Now().Add(clientLifetime),
+		passwords:  passwords,
+	}
+	b.client = c
+
+	return c, nil
+}
+
 const backendHelp = `
 The Azure secrets backend dynamically generates Azure service
 principals. The SP credentials have a configurable lease and

client.go

@@ -33,61 +33,14 @@ type client struct {
 	provider   AzureProvider
 	settings   *clientSettings
 	expiration time.Time
+	passwords  passwords
 }
 
 // Valid returns whether the client defined and not expired.
 func (c *client) Valid() bool {
 	return c != nil && time.Now().Before(c.expiration)
 }
 
-func (b *azureSecretBackend) getClient(ctx context.Context, s logical.Storage) (*client, error) {
-	b.lock.RLock()
-	unlockFunc := b.lock.RUnlock
-	defer func() { unlockFunc() }()
-
-	if b.client.Valid() {
-		return b.client, nil
-	}
-
-	b.lock.RUnlock()
-	b.lock.Lock()
-	unlockFunc = b.lock.Unlock
-
-	if b.client.Valid() {
-		return b.client, nil
-	}
-
-	if b.settings == nil {
-		config, err := b.getConfig(ctx, s)
-		if err != nil {
-			return nil, err
-		}
-		if config == nil {
-			config = new(azureConfig)
-		}
-
-		settings, err := b.getClientSettings(ctx, config)
-		if err != nil {
-			return nil, err
-		}
-		b.settings = settings
-	}
-
-	p, err := b.getProvider(b.settings)
-	if err != nil {
-		return nil, err
-	}
-
-	c := &client{
-		provider:   p,
-		settings:   b.settings,
-		expiration: time.Now().Add(clientLifetime),
-	}
-	b.client = c
-
-	return c, nil
-}
-
 // createApp creates a new Azure application.
 // An Application is a needed to create service principals used by
 // the caller for authentication.
@@ -115,15 +68,15 @@ func (c *client) createApp(ctx context.Context) (app *graphrbac.Application, err
 func (c *client) createSP(
 	ctx context.Context,
 	app *graphrbac.Application,
-	duration time.Duration) (*graphrbac.ServicePrincipal, string, error) {
+	duration time.Duration) (svcPrinc *graphrbac.ServicePrincipal, password string, err error) {
 
 	// Generate a random key (which must be a UUID) and password
 	keyID, err := uuid.GenerateUUID()
 	if err != nil {
 		return nil, "", err
 	}
 
-	password, err := uuid.GenerateUUID()
+	password, err = c.passwords.generate(ctx)
 	if err != nil {
 		return nil, "", err
 	}
@@ -161,8 +114,8 @@ func (c *client) createSP(
 }
 
 // addAppPassword adds a new password to an App's credentials list.
-func (c *client) addAppPassword(ctx context.Context, appObjID string, duration time.Duration) (string, string, error) {
-	keyID, err := uuid.GenerateUUID()
+func (c *client) addAppPassword(ctx context.Context, appObjID string, duration time.Duration) (keyID string, password string, err error) {
+	keyID, err = uuid.GenerateUUID()
 	if err != nil {
 		return "", "", err
 	}
@@ -171,7 +124,7 @@ func (c *client) addAppPassword(ctx context.Context, appObjID string, duration t
 	// passwords. These must be UUIDs, so the three leading bytes will be used as an indicator.
 	keyID = "ffffff" + keyID[6:]
 
-	password, err := uuid.GenerateUUID()
+	password, err = c.passwords.generate(ctx)
 	if err != nil {
 		return "", "", err
 	}
@@ -443,22 +396,26 @@ func (b *azureSecretBackend) getClientSettings(ctx context.Context, config *azur
 // Delays are random but will average 5 seconds.
 func retry(ctx context.Context, f func() (interface{}, bool, error)) (interface{}, error) {
 	delayTimer := time.NewTimer(0)
-	endCh := time.NewTimer(retryTimeout).C
+	if _, hasTimeout := ctx.Deadline(); !hasTimeout {
+		var cancel func()
+		ctx, cancel = context.WithTimeout(ctx, retryTimeout)
+		defer cancel()
+	}
 
+	rng := rand.New(rand.NewSource(time.Now().UnixNano()))
 	for {
 		if result, done, err := f(); done {
 			return result, err
 		}
 
-		delay := time.Duration(2000+rand.Intn(6000)) * time.Millisecond
+		delay := time.Duration(2000+rng.Intn(6000)) * time.Millisecond
 		delayTimer.Reset(delay)
 
 		select {
 		case <-delayTimer.C:
-		case <-endCh:
-			return nil, errors.New("retry: timeout")
+			// Retry loop
 		case <-ctx.Done():
-			return nil, errors.New("retry: cancelled")
+			return nil, fmt.Errorf("retry failed: %w", ctx.Err())
 		}
 	}
 }

client_test.go

@@ -17,7 +17,7 @@ func TestRetry(t *testing.T) {
 		_, err := retry(context.Background(), func() (interface{}, bool, error) {
 			return nil, true, nil
 		})
-		nilErr(t, err)
+		assertErrorIsNil(t, err)
 	})
 
 	t.Run("Three retries", func(t *testing.T) {
@@ -39,7 +39,7 @@ func TestRetry(t *testing.T) {
 		if elapsed < 4 || elapsed > 16 {
 			t.Fatalf("expected time of 4-16 seconds, got: %f", elapsed)
 		}
-		nilErr(t, err)
+		assertErrorIsNil(t, err)
 	})
 
 	t.Run("Error on attempt", func(t *testing.T) {
@@ -59,18 +59,23 @@ func TestRetry(t *testing.T) {
 		}
 		t.Parallel()
 		start := time.Now()
-		_, err := retry(context.Background(), func() (interface{}, bool, error) {
+
+		timeout := 10 * time.Second
+		ctx, cancel := context.WithTimeout(context.Background(), timeout)
+		defer cancel()
+		called := 0
+		_, err := retry(ctx, func() (interface{}, bool, error) {
+			called++
 			return nil, false, nil
 		})
-		elapsed := time.Now().Sub(start).Seconds()
-		expected := retryTimeout.Seconds()
-
-		if elapsed < expected-2 || elapsed > expected+2 {
-			t.Fatalf("expected time of ~%f seconds, got: %f", expected, elapsed)
+		elapsed := time.Now().Sub(start)
+		if err == nil {
+			t.Fatalf("expected error, got nil")
 		}
-		if err == nil || !strings.Contains(err.Error(), "timeout") {
-			t.Fatalf("expected timeout error, got: %v", err)
+		if called == 0 {
+			t.Fatalf("retryable function was never called")
 		}
+		assertDuration(t, elapsed, timeout, 100*time.Millisecond)
 	})
 
 	t.Run("Cancellation", func(t *testing.T) {
@@ -91,8 +96,26 @@ func TestRetry(t *testing.T) {
 			t.Fatalf("expected time of ~7 seconds, got: %f", elapsed)
 		}
 
-		if err == nil || !strings.Contains(err.Error(), "cancelled") {
-			t.Fatalf("expected cancelled error, got: %v", err)
+		if err == nil {
+			t.Fatalf("expected err: got nil")
+		}
+		underlyingErr := errors.Unwrap(err)
+		if underlyingErr != context.Canceled {
+			t.Fatalf("expected %s, got: %v", context.Canceled, err)
 		}
 	})
 }
+
+// assertDuration with a certain amount of flex in the exact value
+func assertDuration(t *testing.T, actual, expected, delta time.Duration) {
+	t.Helper()
+
+	diff := actual - expected
+	if diff < 0 {
+		diff = -diff
+	}
+
+	if diff > delta {
+		t.Fatalf("Actual duration %s does not equal expected %s with delta %s", actual, expected, delta)
+	}
+}

go.mod

@@ -9,14 +9,27 @@ require (
 	github.com/Azure/go-autorest/autorest/date v0.2.0
 	github.com/Azure/go-autorest/autorest/to v0.3.0
 	github.com/Azure/go-autorest/autorest/validation v0.2.0 // indirect
+	github.com/fatih/color v1.9.0 // indirect
+	github.com/frankban/quicktest v1.10.0 // indirect
 	github.com/go-test/deep v1.0.2-0.20181118220953-042da051cf31
 	github.com/hashicorp/errwrap v1.0.0
-	github.com/hashicorp/go-hclog v0.12.0
-	github.com/hashicorp/go-multierror v1.0.0
+	github.com/hashicorp/go-hclog v0.14.1
+	github.com/hashicorp/go-multierror v1.1.0
+	github.com/hashicorp/go-retryablehttp v0.6.6 // indirect
+	github.com/hashicorp/go-rootcerts v1.0.2 // indirect
 	github.com/hashicorp/go-uuid v1.0.2
 	github.com/hashicorp/vault/api v1.0.5-0.20200215224050-f6547fa8e820
-	github.com/hashicorp/vault/sdk v0.1.14-0.20200215224050-f6547fa8e820
+	github.com/hashicorp/vault/sdk v0.1.14-0.20200527182800-ad90e0b39d2f
 	github.com/hashicorp/yamux v0.0.0-20181012175058-2f1d1f20f75d // indirect
-	golang.org/x/text v0.3.1-0.20181227161524-e6919f6577db // indirect
-	google.golang.org/genproto v0.0.0-20190404172233-64821d5d2107 // indirect
+	github.com/kr/text v0.2.0 // indirect
+	github.com/mattn/go-colorable v0.1.6 // indirect
+	github.com/mitchellh/mapstructure v1.3.2 // indirect
+	github.com/pierrec/lz4 v2.5.2+incompatible // indirect
+	golang.org/x/crypto v0.0.0-20200604202706-70a84ac30bf9 // indirect
+	golang.org/x/net v0.0.0-20200602114024-627f9648deb9 // indirect
+	golang.org/x/sys v0.0.0-20200602225109-6fdc65e7d980 // indirect
+	golang.org/x/text v0.3.2 // indirect
+	golang.org/x/time v0.0.0-20200416051211-89c76fbcd5d1 // indirect
+	google.golang.org/protobuf v1.24.0 // indirect
+	gopkg.in/square/go-jose.v2 v2.5.1 // indirect
 )