luxolus
/
open-vault
2
0
Fork 0
Code Issues 1 Pull Requests Releases Activity

agent: allow AppRole Auto-Auth when bind_secret_id = false (#6324) 

* agent: allow AppRole Auto-Auth when bind_secret_id = false
This commit is contained in:
Alessandro De Blasis 2019-04-01 21:27:54 +01:00 committed by Jeff Mitchell
parent 74ed0bd97c
commit c96362d466
3 changed files with 293 additions and 113 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

337
command/agent/approle_end_to_end_test.go
View File

@ -3,6 +3,7 @@ package agent
import (
"context"
"encoding/json"
"fmt"
"io/ioutil"
"os"
"testing"
@ -22,16 +23,43 @@ import (
)
func TestAppRoleEndToEnd(t *testing.T) {
t.Run("preserve_secret_id_file", func(t *testing.T) {
testAppRoleEndToEnd(t, false)
})
t.Run("remove_secret_id_file", func(t *testing.T) {
testAppRoleEndToEnd(t, true)
})
testCases := []struct {
removeSecretIDFile bool
bindSecretID bool
secretIDLess bool
expectToken bool
}{
//default behaviour => token expected
{false, true, false, true},
{true, true, false, true},
//bindSecretID=false, wrong secret provided => token expected
//(vault ignores the supplied secret_id if bind_secret_id=false)
{false, false, false, true},
{true, false, false, true},
//bindSecretID=false, secret not provided => token expected
{false, false, true, true},
{true, false, true, true},
//bindSecretID=true, secret not provided => token not expected
{false, true, true, false},
{true, true, true, false},
}
for _, tc := range testCases {
secretFileAction := "preserve"
if tc.removeSecretIDFile {
secretFileAction = "remove"
}
t.Run(fmt.Sprintf("%s_secret_id_file bindSecretID=%v secretIDLess=%v expectToken=%v", secretFileAction, tc.bindSecretID, tc.secretIDLess, tc.expectToken), func(t *testing.T) {
testAppRoleEndToEnd(t, tc.removeSecretIDFile, tc.bindSecretID, tc.secretIDLess, tc.expectToken)
})
}
}
func testAppRoleEndToEnd(t *testing.T, removeSecretIDFile bool) {
func testAppRoleEndToEnd(t *testing.T, removeSecretIDFile bool, bindSecretID bool, secretIDLess bool, expectToken bool) {
var err error
logger := logging.NewVaultLogger(log.Trace)
coreConfig := &vault.CoreConfig{
@ -63,42 +91,53 @@ func testAppRoleEndToEnd(t *testing.T, removeSecretIDFile bool) {
t.Fatal(err)
}
_, err = client.Logical().Write("auth/approle/role/test1", map[string]interface{}{
"bind_secret_id": "true",
_, err = client.Logical().Write("auth/approle/role/test1", addConstraints(!bindSecretID, map[string]interface{}{
"bind_secret_id": bindSecretID,
"token_ttl": "3s",
"token_max_ttl": "10s",
})
}))
logger.Trace("vault configured with", "bind_secret_id", bindSecretID)
if err != nil {
t.Fatal(err)
}
resp, err := client.Logical().Write("auth/approle/role/test1/secret-id", nil)
if err != nil {
t.Fatal(err)
secret := ""
secretID1 := ""
secretID2 := ""
if bindSecretID {
resp, err := client.Logical().Write("auth/approle/role/test1/secret-id", nil)
if err != nil {
t.Fatal(err)
}
secretID1 = resp.Data["secret_id"].(string)
} else {
logger.Trace("skipped write to auth/approle/role/test1/secret-id")
}
secretID1 := resp.Data["secret_id"].(string)
resp, err = client.Logical().Read("auth/approle/role/test1/role-id")
resp, err := client.Logical().Read("auth/approle/role/test1/role-id")
if err != nil {
t.Fatal(err)
}
roleID1 := resp.Data["role_id"].(string)
_, err = client.Logical().Write("auth/approle/role/test2", map[string]interface{}{
"bind_secret_id": "true",
_, err = client.Logical().Write("auth/approle/role/test2", addConstraints(!bindSecretID, map[string]interface{}{
"bind_secret_id": bindSecretID,
"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)
if bindSecretID {
resp, err = client.Logical().Write("auth/approle/role/test2/secret-id", nil)
if err != nil {
t.Fatal(err)
}
secretID2 = resp.Data["secret_id"].(string)
} else {
logger.Trace("skipped write to auth/approle/role/test2/secret-id")
}
secretID2 := resp.Data["secret_id"].(string)
resp, err = client.Logical().Read("auth/approle/role/test2/role-id")
if err != nil {
t.Fatal(err)
@ -113,16 +152,18 @@ func testAppRoleEndToEnd(t *testing.T, removeSecretIDFile bool) {
rolef.Close() // WriteFile doesn't need it open
defer os.Remove(role)
t.Logf("input role_id_file_path: %s", role)
secretf, err := ioutil.TempFile("", "auth.secret-id.test.")
if err != nil {
t.Fatal(err)
if bindSecretID {
secretf, err := ioutil.TempFile("", "auth.secret-id.test.")
if err != nil {
t.Fatal(err)
}
secret = secretf.Name()
secretf.Close()
defer os.Remove(secret)
t.Logf("input secret_id_file_path: %s", secret)
} else {
logger.Trace("skipped writing tempfile auth.secret-id.test.")
}
secret := secretf.Name()
secretf.Close()
defer os.Remove(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
ouf, err := ioutil.TempFile("", "auth.tokensink.test.")
@ -140,14 +181,34 @@ func testAppRoleEndToEnd(t *testing.T, removeSecretIDFile bool) {
})
defer timer.Stop()
secretFromAgent := secret
if secretIDLess {
secretFromAgent = ""
}
if !bindSecretID && !secretIDLess {
logger.Trace("agent is providing an invalid secret that should be ignored")
secretf, err := ioutil.TempFile("", "auth.secret-id.test.")
if err != nil {
t.Fatal(err)
}
secretFromAgent = secretf.Name()
secretf.Close()
defer os.Remove(secretFromAgent)
//if the token is empty, auth.approle would fail reporting the error
if err := ioutil.WriteFile(secretFromAgent, []byte("wrong-secret"), 0600); err != nil {
t.Fatal(err)
} else {
logger.Trace("wrote secret_id_file_path with wrong-secret", "path", secretFromAgent)
}
}
conf := map[string]interface{}{
"role_id_file_path": role,
"secret_id_file_path": secret,
"secret_id_file_path": secretFromAgent,
}
logger.Trace("agent configured with", "conf", conf)
if !removeSecretIDFile {
conf["remove_secret_id_file_after_reading"] = removeSecretIDFile
}
am, err := agentapprole.NewApproleAuthMethod(&auth.AuthConfig{
Logger: logger.Named("auth.approle"),
MountPath: "auth/approle",
@ -205,17 +266,24 @@ func testAppRoleEndToEnd(t *testing.T, removeSecretIDFile bool) {
logger.Trace("wrote test role 1", "path", role)
}
if err := ioutil.WriteFile(secret, []byte(secretID1), 0600); err != nil {
t.Fatal(err)
if bindSecretID {
if err := ioutil.WriteFile(secret, []byte(secretID1), 0600); err != nil {
t.Fatal(err)
} else {
logger.Trace("wrote test secret 1", "path", secret)
}
} else {
logger.Trace("wrote test secret 1", "path", secret)
logger.Trace("skipped writing test secret 1")
}
checkToken := func() string {
timeout := time.Now().Add(10 * time.Second)
for {
if time.Now().After(timeout) {
t.Fatal("did not find a written token after timeout")
if expectToken {
t.Fatal("did not find a written token after timeout")
}
return ""
}
val, err := ioutil.ReadFile(out)
if err == nil {
@ -223,15 +291,15 @@ func testAppRoleEndToEnd(t *testing.T, removeSecretIDFile bool) {
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")
if !secretIDLess {
_, err = os.Stat(secretFromAgent)
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 {
@ -243,6 +311,13 @@ func testAppRoleEndToEnd(t *testing.T, removeSecretIDFile bool) {
}
}
origEntity := checkToken()
if !expectToken && origEntity != "" {
t.Fatal("did not expect a token to be written: " + origEntity)
}
if !expectToken && origEntity == "" {
logger.Trace("skipping entities comparison as we are not expecting tokens to be written")
return
}
// Make sure it gets renewed
timeout := time.Now().Add(4 * time.Second)
@ -270,10 +345,14 @@ func testAppRoleEndToEnd(t *testing.T, removeSecretIDFile bool) {
logger.Trace("wrote test role 2", "path", role)
}
if err := ioutil.WriteFile(secret, []byte(secretID2), 0600); err != nil {
t.Fatal(err)
if bindSecretID {
if err := ioutil.WriteFile(secret, []byte(secretID2), 0600); err != nil {
t.Fatal(err)
} else {
logger.Trace("wrote test secret 2", "path", secret)
}
} else {
logger.Trace("wrote test secret 2", "path", secret)
logger.Trace("skipped writing test secret 2")
}
newEntity := checkToken()
@ -301,6 +380,29 @@ func testAppRoleEndToEnd(t *testing.T, removeSecretIDFile bool) {
}
func TestAppRoleWithWrapping(t *testing.T) {
testCases := []struct {
bindSecretID bool
secretIDLess bool
expectToken bool
}{
//default behaviour => token expected
{true, false, true},
//bindSecretID=false, wrong secret provided, wrapping_path provided => token not expected
//(wrapping token is not valid or does not exist)
{false, false, false},
//bindSecretID=false, no secret provided, wrapping_path provided but ignored => token expected
{false, true, true},
}
for _, tc := range testCases {
t.Run(fmt.Sprintf("bindSecretID=%v secretIDLess=%v expectToken=%v", tc.bindSecretID, tc.secretIDLess, tc.expectToken), func(t *testing.T) {
testAppRoleWithWrapping(t, tc.bindSecretID, tc.secretIDLess, tc.expectToken)
})
}
}
func testAppRoleWithWrapping(t *testing.T, bindSecretID bool, secretIDLess bool, expectToken bool) {
var err error
logger := logging.NewVaultLogger(log.Trace)
coreConfig := &vault.CoreConfig{
@ -333,11 +435,11 @@ func TestAppRoleWithWrapping(t *testing.T) {
t.Fatal(err)
}
_, err = client.Logical().Write("auth/approle/role/test1", map[string]interface{}{
"bind_secret_id": "true",
_, err = client.Logical().Write("auth/approle/role/test1", addConstraints(!bindSecretID, map[string]interface{}{
"bind_secret_id": bindSecretID,
"token_ttl": "3s",
"token_max_ttl": "10s",
})
}))
if err != nil {
t.Fatal(err)
}
@ -349,13 +451,18 @@ func TestAppRoleWithWrapping(t *testing.T) {
return ""
})
resp, err := client.Logical().Write("auth/approle/role/test1/secret-id", nil)
if err != nil {
t.Fatal(err)
secret := ""
secretID1 := ""
if bindSecretID {
resp, err := client.Logical().Write("auth/approle/role/test1/secret-id", nil)
if err != nil {
t.Fatal(err)
}
secretID1 = resp.WrapInfo.Token
} else {
logger.Trace("skipped write to auth/approle/role/test1/secret-id")
}
secretID1 := resp.WrapInfo.Token
resp, err = client.Logical().Read("auth/approle/role/test1/role-id")
resp, err := client.Logical().Read("auth/approle/role/test1/role-id")
if err != nil {
t.Fatal(err)
}
@ -370,14 +477,18 @@ func TestAppRoleWithWrapping(t *testing.T) {
defer os.Remove(role)
t.Logf("input role_id_file_path: %s", role)
secretf, err := ioutil.TempFile("", "auth.secret-id.test.")
if err != nil {
t.Fatal(err)
if bindSecretID {
secretf, err := ioutil.TempFile("", "auth.secret-id.test.")
if err != nil {
t.Fatal(err)
}
secret = secretf.Name()
secretf.Close()
defer os.Remove(secret)
t.Logf("input secret_id_file_path: %s", secret)
} else {
logger.Trace("skipped writing tempfile auth.secret-id.test.")
}
secret := secretf.Name()
secretf.Close()
defer os.Remove(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
@ -396,12 +507,33 @@ func TestAppRoleWithWrapping(t *testing.T) {
})
defer timer.Stop()
secretFromAgent := secret
if secretIDLess {
secretFromAgent = ""
}
if !bindSecretID && !secretIDLess {
logger.Trace("agent is providing an invalid secret that should be ignored")
secretf, err := ioutil.TempFile("", "auth.secret-id.test.")
if err != nil {
t.Fatal(err)
}
secretFromAgent = secretf.Name()
secretf.Close()
defer os.Remove(secretFromAgent)
//if the token is empty, auth.approle would fail reporting the error
if err := ioutil.WriteFile(secretFromAgent, []byte("wrong-secret"), 0600); err != nil {
t.Fatal(err)
} else {
logger.Trace("wrote secret_id_file_path with wrong-secret", "path", secretFromAgent)
}
}
conf := map[string]interface{}{
"role_id_file_path": role,
"secret_id_file_path": secret,
"secret_id_file_path": secretFromAgent,
"secret_id_response_wrapping_path": "auth/approle/role/test1/secret-id",
"remove_secret_id_file_after_reading": true,
}
logger.Trace("agent configured with", "conf", conf)
am, err := agentapprole.NewApproleAuthMethod(&auth.AuthConfig{
Logger: logger.Named("auth.approle"),
@ -460,17 +592,26 @@ func TestAppRoleWithWrapping(t *testing.T) {
logger.Trace("wrote test role 1", "path", role)
}
if err := ioutil.WriteFile(secret, []byte(secretID1), 0600); err != nil {
t.Fatal(err)
if bindSecretID {
logger.Trace("WRITING TO auth.secret-id.test.", "secret", secret, "secretID1", secretID1)
if err := ioutil.WriteFile(secret, []byte(secretID1), 0600); err != nil {
t.Fatal(err)
} else {
logger.Trace("wrote test secret 1", "path", secret)
}
} else {
logger.Trace("wrote test secret 1", "path", secret)
logger.Trace("skipped writing test secret 1")
}
checkToken := func() string {
timeout := time.Now().Add(10 * time.Second)
for {
if time.Now().After(timeout) {
t.Fatal("did not find a written token after timeout")
if expectToken {
t.Fatal("did not find a written token after timeout")
}
return ""
}
val, err := ioutil.ReadFile(out)
if err == nil {
@ -478,9 +619,10 @@ func TestAppRoleWithWrapping(t *testing.T) {
if len(val) == 0 {
t.Fatal("written token was empty")
}
if _, err := os.Stat(secret); err == nil {
t.Fatal("secret ID file does not exist but was not supposed to be removed")
if !secretIDLess {
if _, err := os.Stat(secret); err == nil {
t.Fatal("secret ID file does not exist but was not supposed to be removed")
}
}
client.SetToken(string(val))
@ -494,6 +636,15 @@ func TestAppRoleWithWrapping(t *testing.T) {
}
}
origEntity := checkToken()
logger.Trace("cheking token", "origEntity", origEntity)
if !expectToken && origEntity != "" {
t.Fatal("did not expect a token to be written: " + origEntity)
}
if !expectToken && origEntity == "" {
logger.Trace("skipping entities comparison as we are not expecting tokens to be written")
return
}
// Make sure it gets renewed
timeout := time.Now().Add(4 * time.Second)
@ -516,15 +667,21 @@ func TestAppRoleWithWrapping(t *testing.T) {
// Write new values
client.SetToken(origToken)
resp, err = client.Logical().Write("auth/approle/role/test1/secret-id", nil)
if err != nil {
t.Fatal(err)
}
secretID2 := resp.WrapInfo.Token
if err := ioutil.WriteFile(secret, []byte(secretID2), 0600); err != nil {
t.Fatal(err)
logger.Trace("origToken set into client", "origToken", origToken)
if bindSecretID {
resp, err = client.Logical().Write("auth/approle/role/test1/secret-id", nil)
if err != nil {
t.Fatal(err)
}
secretID2 := resp.WrapInfo.Token
if err := ioutil.WriteFile(secret, []byte(secretID2), 0600); err != nil {
t.Fatal(err)
} else {
logger.Trace("wrote test secret 2", "path", secret)
}
} else {
logger.Trace("wrote test secret 2", "path", secret)
logger.Trace("skipped writing test secret 2")
}
newEntity := checkToken()
@ -550,3 +707,17 @@ func TestAppRoleWithWrapping(t *testing.T) {
}
}
}
func addConstraints(add bool, cfg map[string]interface{}) map[string]interface{} {
if add {
//extraConstraints to add when bind_secret_id=false (otherwise Vault would fail with: "at least one constraint should be enabled on the role")
extraConstraints := map[string]interface{}{
"secret_id_bound_cidrs": "127.0.0.1/32",
"token_bound_cidrs": "127.0.0.1/32",
}
for k, v := range extraConstraints {
cfg[k] = v
}
}
return cfg
}

63
command/agent/auth/approle/approle.go
View File

@ -54,34 +54,33 @@ func NewApproleAuthMethod(conf *auth.AuthConfig) (auth.AuthMethod, error) {
}
secretIDFilePathRaw, ok := conf.Config["secret_id_file_path"]
if !ok {
return nil, errors.New("missing 'secret_id_file_path' value")
}
a.secretIDFilePath, ok = secretIDFilePathRaw.(string)
if !ok {
return nil, errors.New("could not convert 'secret_id_file_path' config value to string")
}
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
}
secretIDResponseWrappingPathRaw, ok := conf.Config["secret_id_response_wrapping_path"]
if ok {
a.secretIDResponseWrappingPath, ok = secretIDResponseWrappingPathRaw.(string)
a.secretIDFilePath, ok = secretIDFilePathRaw.(string)
if !ok {
return nil, errors.New("could not convert 'secret_id_response_wrapping_path' config value to string")
return nil, errors.New("could not convert 'secret_id_file_path' config value to string")
}
if a.secretIDResponseWrappingPath == "" {
return nil, errors.New("'secret_id_response_wrapping_path' value is empty")
if a.secretIDFilePath == "" {
return a, nil
}
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
}
secretIDResponseWrappingPathRaw, ok := conf.Config["secret_id_response_wrapping_path"]
if ok {
a.secretIDResponseWrappingPath, ok = secretIDResponseWrappingPathRaw.(string)
if !ok {
return nil, errors.New("could not convert 'secret_id_response_wrapping_path' config value to string")
}
if a.secretIDResponseWrappingPath == "" {
return nil, errors.New("'secret_id_response_wrapping_path' value is empty")
}
}
}
@ -106,6 +105,17 @@ func (a *approleMethod) Authenticate(ctx context.Context, client *api.Client) (s
a.cachedRoleID = strings.TrimSpace(string(roleID))
}
}
if a.cachedRoleID == "" {
return "", nil, errors.New("no known role ID")
}
if a.secretIDFilePath == "" {
return fmt.Sprintf("%s/login", a.mountPath), map[string]interface{}{
"role_id": a.cachedRoleID,
}, nil
}
if _, err := os.Stat(a.secretIDFilePath); err == nil {
secretID, err := ioutil.ReadFile(a.secretIDFilePath)
if err != nil {
@ -180,9 +190,6 @@ func (a *approleMethod) Authenticate(ctx context.Context, client *api.Client) (s
}
}
if a.cachedRoleID == "" {
return "", nil, errors.New("no known role ID")
}
if a.cachedSecretID == "" {
return "", nil, errors.New("no known secret ID")
}

6
website/source/docs/agent/autoauth/methods/approle.html.md
View File

@ -23,8 +23,10 @@ cached.
* `role_id_file_path` `(string: required)` - The path to the file with role ID
* `secret_id_file_path` `(string: required)` - The path to the file with secret
ID
* `secret_id_file_path` `(string: optional)` - The path to the file with secret
ID.
If not set, only the `role-id` will be used. \
In that case, the AppRole should have `bind_secret_id` set to `false` otherwise Vault Agent wouldn't be able to login.
* `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