From 6d20c8fce2992f1b0bc256dcd34c4cd5f1079b3d Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Tue, 30 Oct 2018 14:09:04 -0400 Subject: [PATCH] Add approle agent method removing secret ID file by default. (#5648) Also, massively update tests. --- command/agent/approle_end_to_end_test.go | 176 ++++++++++++++---- command/agent/auth/approle/approle.go | 104 ++++++++--- .../agent/autoauth/methods/approle.html.md | 18 +- 3 files changed, 236 insertions(+), 62 deletions(-) diff --git a/command/agent/approle_end_to_end_test.go b/command/agent/approle_end_to_end_test.go index 120db3618..6d8085750 100644 --- a/command/agent/approle_end_to_end_test.go +++ b/command/agent/approle_end_to_end_test.go @@ -2,6 +2,7 @@ package agent import ( "context" + "encoding/json" "io/ioutil" "os" "testing" @@ -21,6 +22,11 @@ import ( ) func TestAppRoleEndToEnd(t *testing.T) { + testAppRoleEndToEnd(t, false) + testAppRoleEndToEnd(t, true) +} + +func testAppRoleEndToEnd(t *testing.T, removeSecretIDFile bool) { var err error logger := logging.NewVaultLogger(log.Trace) coreConfig := &vault.CoreConfig{ @@ -52,55 +58,65 @@ func TestAppRoleEndToEnd(t *testing.T) { t.Fatal(err) } - _, err = client.Logical().Write("auth/approle/role/role1", map[string]interface{}{ + _, err = client.Logical().Write("auth/approle/role/test1", map[string]interface{}{ "bind_secret_id": "true", - "period": "300", + "token_ttl": "3s", + "token_max_ttl": "10s", }) if err != nil { t.Fatal(err) } - vaultResult, err := client.Logical().Write("auth/approle/role/role1/secret-id", nil) + resp, err := client.Logical().Write("auth/approle/role/test1/secret-id", nil) if err != nil { t.Fatal(err) } - secretID := vaultResult.Data["secret_id"].(string) + secretID1 := resp.Data["secret_id"].(string) - vaultResult, err = client.Logical().Read("auth/approle/role/role1/role-id") + resp, err = client.Logical().Read("auth/approle/role/test1/role-id") if err != nil { t.Fatal(err) } - roleID := vaultResult.Data["role_id"].(string) + roleID1 := resp.Data["role_id"].(string) + + _, err = client.Logical().Write("auth/approle/role/test2", map[string]interface{}{ + "bind_secret_id": "true", + "token_ttl": "3s", + "token_max_ttl": "10s", + }) + if err != nil { + t.Fatal(err) + } + + resp, err = client.Logical().Write("auth/approle/role/test2/secret-id", nil) + if err != nil { + t.Fatal(err) + } + secretID2 := resp.Data["secret_id"].(string) + + resp, err = client.Logical().Read("auth/approle/role/test2/role-id") + if err != nil { + t.Fatal(err) + } + roleID2 := resp.Data["role_id"].(string) rolef, err := ioutil.TempFile("", "auth.role-id.test.") if err != nil { t.Fatal(err) } role := rolef.Name() - defer rolef.Close() + rolef.Close() // WriteFile doesn't need it open defer os.Remove(role) - - t.Logf("input role_id_path: %s", role) - if err := ioutil.WriteFile(role, []byte(roleID), 0600); err != nil { - t.Fatal(err) - } else { - logger.Trace("wrote test role-id", "path", role) - } + t.Logf("input role_id_file_path: %s", role) secretf, err := ioutil.TempFile("", "auth.secret-id.test.") if err != nil { t.Fatal(err) } secret := secretf.Name() - defer secretf.Close() + secretf.Close() defer os.Remove(secret) - - t.Logf("input secret_id_path: %s", secret) - if err := ioutil.WriteFile(secret, []byte(secretID), 0600); err != nil { - t.Fatal(err) - } else { - logger.Trace("wrote test secret-id", "path", secret) - } + t.Logf("input secret_id_file_path: %s", secret) // We close these right away because we're just basically testing // permissions and finding a usable file name @@ -119,13 +135,18 @@ func TestAppRoleEndToEnd(t *testing.T) { }) defer timer.Stop() + conf := map[string]interface{}{ + "role_id_file_path": role, + "secret_id_file_path": secret, + } + if !removeSecretIDFile { + conf["remove_secret_id_file_after_reading"] = removeSecretIDFile + } + am, err := agentapprole.NewApproleAuthMethod(&auth.AuthConfig{ Logger: logger.Named("auth.approle"), MountPath: "auth/approle", - Config: map[string]interface{}{ - "role_id_path": role, - "secret_id_path": secret, - }, + Config: conf, }) if err != nil { t.Fatal(err) @@ -173,14 +194,103 @@ func TestAppRoleEndToEnd(t *testing.T) { t.Fatal("expected notexist err") } - token, err := client.Logical().Write("auth/approle/login", map[string]interface{}{ - "role_id": roleID, - "secret_id": secretID, - }) - if err != nil { + if err := ioutil.WriteFile(role, []byte(roleID1), 0600); err != nil { t.Fatal(err) + } else { + logger.Trace("wrote test role 1", "path", role) } - if token.Auth.ClientToken == "" { - t.Fatalf("expected a successful login") + + if err := ioutil.WriteFile(secret, []byte(secretID1), 0600); err != nil { + t.Fatal(err) + } else { + logger.Trace("wrote test secret 1", "path", secret) + } + + checkToken := func() string { + timeout := time.Now().Add(5 * time.Second) + for { + if time.Now().After(timeout) { + t.Fatal("did not find a written token after timeout") + } + val, err := ioutil.ReadFile(out) + if err == nil { + os.Remove(out) + if len(val) == 0 { + t.Fatal("written token was empty") + } + + _, err = os.Stat(secret) + switch { + case removeSecretIDFile && err == nil: + t.Fatal("secret file exists but was supposed to be removed") + case !removeSecretIDFile && err != nil: + t.Fatal("secret ID file does not exist but was not supposed to be removed") + } + + client.SetToken(string(val)) + secret, err := client.Auth().Token().LookupSelf() + if err != nil { + t.Fatal(err) + } + return secret.Data["entity_id"].(string) + } + time.Sleep(250 * time.Millisecond) + } + } + origEntity := checkToken() + + // Make sure it gets renewed + timeout := time.Now().Add(4 * time.Second) + for { + if time.Now().After(timeout) { + break + } + secret, err := client.Auth().Token().LookupSelf() + if err != nil { + t.Fatal(err) + } + ttl, err := secret.Data["ttl"].(json.Number).Int64() + if err != nil { + t.Fatal(err) + } + if ttl > 3 { + t.Fatalf("unexpected ttl: %v", secret.Data["ttl"]) + } + } + + // Write new values + if err := ioutil.WriteFile(role, []byte(roleID2), 0600); err != nil { + t.Fatal(err) + } else { + logger.Trace("wrote test role 2", "path", role) + } + + if err := ioutil.WriteFile(secret, []byte(secretID2), 0600); err != nil { + t.Fatal(err) + } else { + logger.Trace("wrote test secret 2", "path", secret) + } + + newEntity := checkToken() + if newEntity == origEntity { + t.Fatal("found same entity") + } + + timeout = time.Now().Add(4 * time.Second) + for { + if time.Now().After(timeout) { + break + } + secret, err := client.Auth().Token().LookupSelf() + if err != nil { + t.Fatal(err) + } + ttl, err := secret.Data["ttl"].(json.Number).Int64() + if err != nil { + t.Fatal(err) + } + if ttl > 3 { + t.Fatalf("unexpected ttl: %v", secret.Data["ttl"]) + } } } diff --git a/command/agent/auth/approle/approle.go b/command/agent/auth/approle/approle.go index a49f58c6c..398399558 100644 --- a/command/agent/auth/approle/approle.go +++ b/command/agent/auth/approle/approle.go @@ -5,20 +5,25 @@ import ( "errors" "fmt" "io/ioutil" - "log" + "os" "strings" + "github.com/hashicorp/errwrap" "github.com/hashicorp/go-hclog" "github.com/hashicorp/vault/api" "github.com/hashicorp/vault/command/agent/auth" + "github.com/hashicorp/vault/helper/parseutil" ) type approleMethod struct { logger hclog.Logger mountPath string - roleIDPath string - secretIDPath string + roleIDFilePath string + secretIDFilePath string + cachedRoleID string + cachedSecretID string + removeSecretIDFileAfterReading bool } func NewApproleAuthMethod(conf *auth.AuthConfig) (auth.AuthMethod, error) { @@ -30,51 +35,98 @@ func NewApproleAuthMethod(conf *auth.AuthConfig) (auth.AuthMethod, error) { } a := &approleMethod{ - logger: conf.Logger, - mountPath: conf.MountPath, + logger: conf.Logger, + mountPath: conf.MountPath, + removeSecretIDFileAfterReading: true, } - roleIDPathRaw, ok := conf.Config["role_id_path"] + roleIDFilePathRaw, ok := conf.Config["role_id_file_path"] if !ok { - return nil, errors.New("missing 'role_id_path' value") + return nil, errors.New("missing 'role_id_file_path' value") } - a.roleIDPath, ok = roleIDPathRaw.(string) + a.roleIDFilePath, ok = roleIDFilePathRaw.(string) if !ok { - return nil, errors.New("could not convert 'role_id_path' config value to string") + return nil, errors.New("could not convert 'role_id_file_path' config value to string") } - if a.roleIDPath == "" { - return nil, errors.New("'role_id_path' value is empty") + if a.roleIDFilePath == "" { + return nil, errors.New("'role_id_file_path' value is empty") } - secretIDPathRaw, ok := conf.Config["secret_id_path"] + secretIDFilePathRaw, ok := conf.Config["secret_id_file_path"] if !ok { - return nil, errors.New("missing 'secret_id_path' value") + return nil, errors.New("missing 'secret_id_file_path' value") } - a.secretIDPath, ok = secretIDPathRaw.(string) + a.secretIDFilePath, ok = secretIDFilePathRaw.(string) if !ok { - return nil, errors.New("could not convert 'secret_id_path' config value to string") + return nil, errors.New("could not convert 'secret_id_file_path' config value to string") } - if a.secretIDPath == "" { - return nil, errors.New("'secret_id_path' value is empty") + if a.secretIDFilePath == "" { + return nil, errors.New("'secret_id_file_path' value is empty") + } + + removeSecretIDFileAfterReadingRaw, ok := conf.Config["remove_secret_id_file_after_reading"] + if ok { + removeSecretIDFileAfterReading, err := parseutil.ParseBool(removeSecretIDFileAfterReadingRaw) + if err != nil { + return nil, errwrap.Wrapf("error parsing 'remove_secret_id_file_after_reading' value: {{err}}", err) + } + a.removeSecretIDFileAfterReading = removeSecretIDFileAfterReading } return a, nil } func (a *approleMethod) Authenticate(ctx context.Context, client *api.Client) (string, map[string]interface{}, error) { - a.logger.Trace("beginning authentication") - roleID, err := ioutil.ReadFile(a.roleIDPath) - if err != nil { - log.Fatal(err) + if _, err := os.Stat(a.roleIDFilePath); err == nil { + roleID, err := ioutil.ReadFile(a.roleIDFilePath) + if err != nil { + if a.cachedRoleID == "" { + return "", nil, errwrap.Wrapf("error reading role ID file and no cached role ID known: {{err}}", err) + } + a.logger.Warn("error reading role ID file", "error", err) + } + if len(roleID) == 0 { + if a.cachedRoleID == "" { + return "", nil, errors.New("role ID file empty and no cached role ID known") + } + a.logger.Warn("role ID file exists but read empty value, re-using cached value") + } else { + a.cachedRoleID = strings.TrimSpace(string(roleID)) + } } - secretID, err := ioutil.ReadFile(a.secretIDPath) - if err != nil { - log.Fatal(err) + if _, err := os.Stat(a.secretIDFilePath); err == nil { + secretID, err := ioutil.ReadFile(a.secretIDFilePath) + if err != nil { + if a.cachedSecretID == "" { + return "", nil, errwrap.Wrapf("error reading secret ID file and no cached secret ID known: {{err}}", err) + } + a.logger.Warn("error reading secret ID file", "error", err) + } + if len(secretID) == 0 { + if a.cachedSecretID == "" { + return "", nil, errors.New("secret ID file empty and no cached secret ID known") + } + a.logger.Warn("secret ID file exists but read empty value, re-using cached value") + } else { + a.cachedSecretID = strings.TrimSpace(string(secretID)) + if a.removeSecretIDFileAfterReading { + if err := os.Remove(a.secretIDFilePath); err != nil { + a.logger.Error("error removing secret ID file after reading", "error", err) + } + } + } + } + + if a.cachedRoleID == "" { + return "", nil, errors.New("no known role ID") + } + if a.cachedSecretID == "" { + return "", nil, errors.New("no known secret ID") } return fmt.Sprintf("%s/login", a.mountPath), map[string]interface{}{ - "role_id": strings.TrimSpace(string(roleID)), - "secret_id": strings.TrimSpace(string(secretID)), + "role_id": a.cachedRoleID, + "secret_id": a.cachedSecretID, }, nil } diff --git a/website/source/docs/agent/autoauth/methods/approle.html.md b/website/source/docs/agent/autoauth/methods/approle.html.md index 514eee467..85d2587af 100644 --- a/website/source/docs/agent/autoauth/methods/approle.html.md +++ b/website/source/docs/agent/autoauth/methods/approle.html.md @@ -9,11 +9,23 @@ description: |- # Vault Agent Auto-Auth AppRole Method -The `approle` method reads in a role-id/secret-id from a files and sends it to the [AppRole Auth +The `approle` method reads in a role ID and a secret ID from files and sends +the values to the [AppRole Auth method](https://www.vaultproject.io/docs/auth/approle.html). +The method caches values and it is safe to delete the role ID/secret ID files +after they have been read. In fact, by default, after reading the secret ID, +the agent will delete the file. New files or values written at the expected +locations will be used on next authentication and the new values will be +cached. + ## Configuration -* `role_id_path` `(string: required)` - The path to the file with role-id +* `role_id_file_path` `(string: required)` - The path to the file with role ID -* `secret_id_path` `(string: required)` - The path to the file with secret-id +* `secret_id_file_path` `(string: required)` - The path to the file with secret + ID + +* `remove_secret_id_file_after_reading` `(bool: optional, defaults to true)` - + This can be set to `false` to disable the default behavior of removing the + secret ID file after it's been read.