From fb86904902994d34c5a75a0d22c825847add074c Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Thu, 19 Jan 2017 13:40:32 -0800 Subject: [PATCH 01/10] Check capabilities, allow creation against role Check the capabilities of the Vault token to ensure it is valid and also allow targetting of a role that the token is not from. --- command/agent/config-test-fixtures/basic.hcl | 1 + command/agent/config_parse.go | 1 + command/agent/config_parse_test.go | 1 + nomad/structs/config/vault.go | 10 ++ nomad/vault.go | 150 ++++++++++++++++++- 5 files changed, 156 insertions(+), 7 deletions(-) diff --git a/command/agent/config-test-fixtures/basic.hcl b/command/agent/config-test-fixtures/basic.hcl index 0059210f2..587d68423 100644 --- a/command/agent/config-test-fixtures/basic.hcl +++ b/command/agent/config-test-fixtures/basic.hcl @@ -121,6 +121,7 @@ vault { key_file = "/path/to/key/file" tls_server_name = "foobar" tls_skip_verify = true + create_from_role = "test_role" } tls { http = true diff --git a/command/agent/config_parse.go b/command/agent/config_parse.go index e7a78edf5..9dfbf6aaf 100644 --- a/command/agent/config_parse.go +++ b/command/agent/config_parse.go @@ -710,6 +710,7 @@ func parseVaultConfig(result **config.VaultConfig, list *ast.ObjectList) error { "ca_file", "ca_path", "cert_file", + "create_from_role", "key_file", "tls_server_name", "tls_skip_verify", diff --git a/command/agent/config_parse_test.go b/command/agent/config_parse_test.go index bbb858e5a..8747926bb 100644 --- a/command/agent/config_parse_test.go +++ b/command/agent/config_parse_test.go @@ -129,6 +129,7 @@ func TestConfig_Parse(t *testing.T) { Addr: "127.0.0.1:9500", AllowUnauthenticated: &trueValue, Enabled: &falseValue, + Role: "test_role", TLSCaFile: "/path/to/ca/file", TLSCaPath: "/path/to/ca", TLSCertFile: "/path/to/cert/file", diff --git a/nomad/structs/config/vault.go b/nomad/structs/config/vault.go index a958661c6..42115c1a9 100644 --- a/nomad/structs/config/vault.go +++ b/nomad/structs/config/vault.go @@ -30,6 +30,13 @@ type VaultConfig struct { // lifetime. Token string `mapstructure:"token"` + // Role sets the role in which to create tokens from. The Token given to + // Nomad does not have to be created from this role but must have "update" + // capability on "auth/token/create/". If this value is + // unset and the token is created from a role, the value is defaulted to the + // role the token is from. + Role string `mapstructure:"create_from_role"` + // AllowUnauthenticated allows users to submit jobs requiring Vault tokens // without providing a Vault token proving they have access to these // policies. @@ -99,6 +106,9 @@ func (a *VaultConfig) Merge(b *VaultConfig) *VaultConfig { if b.Token != "" { result.Token = b.Token } + if b.Role != "" { + result.Role = b.Role + } if b.TaskTokenTTL != "" { result.TaskTokenTTL = b.TaskTokenTTL } diff --git a/nomad/vault.go b/nomad/vault.go index 1c71afc74..ff867e12e 100644 --- a/nomad/vault.go +++ b/nomad/vault.go @@ -47,6 +47,42 @@ const ( // vaultRevocationIntv is the interval at which Vault tokens that failed // initial revocation are retried vaultRevocationIntv = 5 * time.Minute + + // vaultCapabilitiesLookupPath is the path to lookup the capabilities of + // ones token. + vaultCapabilitiesLookupPath = "/sys/capabilities-self" + + // vaultCapabilitiesCapability is the expected capability of Nomad's Vault + // token on the the path. + vaultCapabilitiesCapability = "update" + + // vaultTokenLookupPath is the path used to lookup a token + vaultTokenLookupPath = "auth/token/lookup" + + // vaultTokenLookupCapability is the expected capability Nomad's + // Vault token should have on the path + vaultTokenLookupCapability = "update" + + // vaultTokenRevokePath is the path used to revoke a token + vaultTokenRevokePath = "/auth/token/revoke-accessor/*" + + // vaultTokenRevokeCapability is the expected capability Nomad's + // Vault token should have on the path + vaultTokenRevokeCapability = "update" + + // vaultRoleLookupPath is the path to lookup a role + vaultRoleLookupPath = "auth/token/roles/%s" + + // vaultRoleLookupCapability is the the expected capability Nomad's Vault + // token should have on the path + vaultRoleLookupCapability = "read" + + // vaultRoleCreatePath is the path to create a token from a role + vaultTokenRoleCreatePath = "auth/token/create/%s" + + // vaultTokenRoleCreateCapability is the the expected capability Nomad's Vault + // token should have on the path + vaultTokenRoleCreateCapability = "update" ) var ( @@ -478,7 +514,7 @@ func (v *vaultClient) renew() error { func (v *vaultClient) getWrappingFn() func(operation, path string) string { createPath := "auth/token/create" if !v.tokenData.Root { - createPath = fmt.Sprintf("auth/token/create/%s", v.tokenData.Role) + createPath = fmt.Sprintf("auth/token/create/%s", v.getRole()) } return func(operation, path string) string { @@ -516,7 +552,27 @@ func (v *vaultClient) parseSelfToken() error { } } + data.Root = root + v.tokenData = &data + + // The criteria that must be met for the token to be valid are as follows: + // 1) If token is non-root or is but has a creation ttl + // a) The token must be renewable + // b) Token must have a non-zero TTL + // 2) Must have update capability for "auth/token/lookup/" (used to verify incoming tokens) + // 3) Must have update capability for "/auth/token/revoke-accessor/" (used to revoke unneeded tokens) + // 4) If configured to create tokens against a role: + // a) Must have read capability for "auth/token/roles/" + // c) Role must: + // 1) Not allow orphans + // 2) Must allow tokens to be renewed + // 3) Must not have an explicit max TTL + // 4) Must have non-zero period + // 5) If not configured against a role, the token must be root + var mErr multierror.Error + role := v.getRole() if !root { // All non-root tokens must be renewable if !data.Renewable { @@ -534,7 +590,7 @@ func (v *vaultClient) parseSelfToken() error { } // There must be a valid role since we aren't root - if data.Role == "" { + if role == "" { multierror.Append(&mErr, fmt.Errorf("token role name must be set when not using a root token")) } @@ -548,18 +604,98 @@ func (v *vaultClient) parseSelfToken() error { } } + // Check we have the correct capabilities + if err := v.validateCapabilities(role, root); err != nil { + multierror.Append(&mErr, err) + } + // If given a role validate it - if data.Role != "" { - if err := v.validateRole(data.Role); err != nil { + if role != "" { + if err := v.validateRole(role); err != nil { multierror.Append(&mErr, err) } } - data.Root = root - v.tokenData = &data return mErr.ErrorOrNil() } +// getRole returns the role name to be used when creating tokens +func (v *vaultClient) getRole() string { + if v.config.Role != "" { + return v.config.Role + } + + return v.tokenData.Role +} + +// validateCapabilities checks that Nomad's Vault token has the correct +// capabilities. +func (v *vaultClient) validateCapabilities(role string, root bool) error { + // Check if the token can lookup capabilities. + var mErr multierror.Error + _, err := v.hasCapability(vaultCapabilitiesLookupPath, vaultCapabilitiesCapability) + if err != nil { + // Check if there is a permission denied + if vaultUnrecoverableError.MatchString(err.Error()) { + // Since we can't read permissions, we just log a warning that we + // can't tell if the Vault token will work + msg := fmt.Sprintf("Can not lookup token capabilities. "+ + "As such certain operations may fail in the future. "+ + "Please give Nomad a Vault token with %q on %q", + vaultCapabilitiesCapability, vaultCapabilitiesLookupPath) + v.logger.Printf("[WARN] vault: %s", msg) + return nil + } else { + multierror.Append(&mErr, err) + } + } + + // verify is a helper function that verifies the token has the capability on + // the given path and adds an issue to the error + verify := func(path, capability string) { + ok, err := v.hasCapability(path, capability) + if err != nil { + multierror.Append(&mErr, err) + } else if !ok { + multierror.Append(&mErr, fmt.Errorf("token doesn't have %q capability on %q", capability, path)) + } + } + + // Check if we are verifying incoming tokens + if !v.config.AllowsUnauthenticated() { + verify(vaultTokenLookupPath, vaultTokenLookupCapability) + } + + // Verify we can revoke tokens + verify(vaultTokenRevokePath, vaultTokenRevokeCapability) + + // If we are using a role verify the capability + if role != "" { + // Verify we can read the role + verify(fmt.Sprintf(vaultRoleLookupPath, role), vaultRoleLookupCapability) + + // Verify we can create from the role + verify(fmt.Sprintf(vaultTokenRoleCreatePath, role), vaultTokenRoleCreateCapability) + } + + return mErr.ErrorOrNil() +} + +// hasCapability takes a path and returns whether the token has the required +// capability at the given path. +func (v *vaultClient) hasCapability(path string, required string) (bool, error) { + caps, err := v.client.Sys().CapabilitiesSelf(path) + if err != nil { + return false, err + } + for _, c := range caps { + if c == required { + return true, nil + } + } + return false, nil +} + // validateRole contacts Vault and checks that the given Vault role is valid for // the purposes of being used by Nomad func (v *vaultClient) validateRole(role string) error { @@ -684,7 +820,7 @@ func (v *vaultClient) CreateToken(ctx context.Context, a *structs.Allocation, ta secret, err = v.auth.Create(req) } else { // Make the token using the role - secret, err = v.auth.CreateWithRole(req, v.tokenData.Role) + secret, err = v.auth.CreateWithRole(req, v.getRole()) } // Determine whether it is unrecoverable From 735b5f9ca403a556205373342d37ce69a890ae58 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Thu, 19 Jan 2017 17:12:33 -0800 Subject: [PATCH 02/10] Vendor vault/api --- .../hashicorp/vault/api/auth_token.go | 66 ++++++++++++++--- .../github.com/hashicorp/vault/api/client.go | 31 ++++---- .../github.com/hashicorp/vault/api/logical.go | 71 +++++++++++++++++-- .../hashicorp/vault/api/ssh_agent.go | 15 +++- .../hashicorp/vault/api/sys_init.go | 1 + vendor/vendor.json | 6 +- 6 files changed, 158 insertions(+), 32 deletions(-) diff --git a/vendor/github.com/hashicorp/vault/api/auth_token.go b/vendor/github.com/hashicorp/vault/api/auth_token.go index 1901ea110..aff10f410 100644 --- a/vendor/github.com/hashicorp/vault/api/auth_token.go +++ b/vendor/github.com/hashicorp/vault/api/auth_token.go @@ -25,6 +25,21 @@ func (c *TokenAuth) Create(opts *TokenCreateRequest) (*Secret, error) { return ParseSecret(resp.Body) } +func (c *TokenAuth) CreateOrphan(opts *TokenCreateRequest) (*Secret, error) { + r := c.c.NewRequest("POST", "/v1/auth/token/create-orphan") + if err := r.SetJSONBody(opts); err != nil { + return nil, err + } + + resp, err := c.c.RawRequest(r) + if err != nil { + return nil, err + } + defer resp.Body.Close() + + return ParseSecret(resp.Body) +} + func (c *TokenAuth) CreateWithRole(opts *TokenCreateRequest, roleName string) (*Secret, error) { r := c.c.NewRequest("POST", "/v1/auth/token/create/"+roleName) if err := r.SetJSONBody(opts); err != nil { @@ -41,7 +56,12 @@ func (c *TokenAuth) CreateWithRole(opts *TokenCreateRequest, roleName string) (* } func (c *TokenAuth) Lookup(token string) (*Secret, error) { - r := c.c.NewRequest("GET", "/v1/auth/token/lookup/"+token) + r := c.c.NewRequest("POST", "/v1/auth/token/lookup") + if err := r.SetJSONBody(map[string]interface{}{ + "token": token, + }); err != nil { + return nil, err + } resp, err := c.c.RawRequest(r) if err != nil { @@ -53,8 +73,12 @@ func (c *TokenAuth) Lookup(token string) (*Secret, error) { } func (c *TokenAuth) LookupAccessor(accessor string) (*Secret, error) { - r := c.c.NewRequest("POST", "/v1/auth/token/lookup-accessor/"+accessor) - + r := c.c.NewRequest("POST", "/v1/auth/token/lookup-accessor") + if err := r.SetJSONBody(map[string]interface{}{ + "accessor": accessor, + }); err != nil { + return nil, err + } resp, err := c.c.RawRequest(r) if err != nil { return nil, err @@ -77,10 +101,11 @@ func (c *TokenAuth) LookupSelf() (*Secret, error) { } func (c *TokenAuth) Renew(token string, increment int) (*Secret, error) { - r := c.c.NewRequest("PUT", "/v1/auth/token/renew/"+token) - - body := map[string]interface{}{"increment": increment} - if err := r.SetJSONBody(body); err != nil { + r := c.c.NewRequest("PUT", "/v1/auth/token/renew") + if err := r.SetJSONBody(map[string]interface{}{ + "token": token, + "increment": increment, + }); err != nil { return nil, err } @@ -113,7 +138,12 @@ func (c *TokenAuth) RenewSelf(increment int) (*Secret, error) { // RevokeAccessor revokes a token associated with the given accessor // along with all the child tokens. func (c *TokenAuth) RevokeAccessor(accessor string) error { - r := c.c.NewRequest("POST", "/v1/auth/token/revoke-accessor/"+accessor) + r := c.c.NewRequest("POST", "/v1/auth/token/revoke-accessor") + if err := r.SetJSONBody(map[string]interface{}{ + "accessor": accessor, + }); err != nil { + return err + } resp, err := c.c.RawRequest(r) if err != nil { return err @@ -126,7 +156,13 @@ func (c *TokenAuth) RevokeAccessor(accessor string) error { // RevokeOrphan revokes a token without revoking the tree underneath it (so // child tokens are orphaned rather than revoked) func (c *TokenAuth) RevokeOrphan(token string) error { - r := c.c.NewRequest("PUT", "/v1/auth/token/revoke-orphan/"+token) + r := c.c.NewRequest("PUT", "/v1/auth/token/revoke-orphan") + if err := r.SetJSONBody(map[string]interface{}{ + "token": token, + }); err != nil { + return err + } + resp, err := c.c.RawRequest(r) if err != nil { return err @@ -136,7 +172,9 @@ func (c *TokenAuth) RevokeOrphan(token string) error { return nil } -// RevokeSelf revokes the token making the call +// RevokeSelf revokes the token making the call. The `token` parameter is kept +// for backwards compatibility but is ignored; only the client's set token has +// an effect. func (c *TokenAuth) RevokeSelf(token string) error { r := c.c.NewRequest("PUT", "/v1/auth/token/revoke-self") resp, err := c.c.RawRequest(r) @@ -152,7 +190,13 @@ func (c *TokenAuth) RevokeSelf(token string) error { // the entire tree underneath -- all of its child tokens, their child tokens, // etc. func (c *TokenAuth) RevokeTree(token string) error { - r := c.c.NewRequest("PUT", "/v1/auth/token/revoke/"+token) + r := c.c.NewRequest("PUT", "/v1/auth/token/revoke") + if err := r.SetJSONBody(map[string]interface{}{ + "token": token, + }); err != nil { + return err + } + resp, err := c.c.RawRequest(r) if err != nil { return err diff --git a/vendor/github.com/hashicorp/vault/api/client.go b/vendor/github.com/hashicorp/vault/api/client.go index 179d2bdc9..88a8ea4f9 100644 --- a/vendor/github.com/hashicorp/vault/api/client.go +++ b/vendor/github.com/hashicorp/vault/api/client.go @@ -48,7 +48,7 @@ type Config struct { redirectSetup sync.Once // MaxRetries controls the maximum number of times to retry when a 5xx error - // occurs. Set to 0 or less to disable retrying. + // occurs. Set to 0 or less to disable retrying. Defaults to 0. MaxRetries int } @@ -99,12 +99,10 @@ func DefaultConfig() *Config { config.Address = v } - config.MaxRetries = pester.DefaultClient.MaxRetries - return config } -// ConfigureTLS takes a set of TLS configurations and applies those to the HTTP client. +// ConfigureTLS takes a set of TLS configurations and applies those to the the HTTP client. func (c *Config) ConfigureTLS(t *TLSConfig) error { if c.HttpClient == nil { @@ -289,6 +287,11 @@ func (c *Client) SetAddress(addr string) error { return nil } +// Address returns the Vault URL the client is configured to connect to +func (c *Client) Address() string { + return c.addr.String() +} + // SetWrappingLookupFunc sets a lookup function that returns desired wrap TTLs // for a given operation and path func (c *Client) SetWrappingLookupFunc(lookupFunc WrappingLookupFunc) { @@ -327,17 +330,19 @@ func (c *Client) NewRequest(method, path string) *Request { Params: make(map[string][]string), } + var lookupPath string + switch { + case strings.HasPrefix(path, "/v1/"): + lookupPath = strings.TrimPrefix(path, "/v1/") + case strings.HasPrefix(path, "v1/"): + lookupPath = strings.TrimPrefix(path, "v1/") + default: + lookupPath = path + } if c.wrappingLookupFunc != nil { - var lookupPath string - switch { - case strings.HasPrefix(path, "/v1/"): - lookupPath = strings.TrimPrefix(path, "/v1/") - case strings.HasPrefix(path, "v1/"): - lookupPath = strings.TrimPrefix(path, "v1/") - default: - lookupPath = path - } req.WrapTTL = c.wrappingLookupFunc(method, lookupPath) + } else { + req.WrapTTL = DefaultWrappingLookupFunc(method, lookupPath) } return req diff --git a/vendor/github.com/hashicorp/vault/api/logical.go b/vendor/github.com/hashicorp/vault/api/logical.go index fb8288e73..0d5e7d495 100644 --- a/vendor/github.com/hashicorp/vault/api/logical.go +++ b/vendor/github.com/hashicorp/vault/api/logical.go @@ -3,6 +3,8 @@ package api import ( "bytes" "fmt" + "net/http" + "os" "github.com/hashicorp/vault/helper/jsonutil" ) @@ -11,6 +13,26 @@ const ( wrappedResponseLocation = "cubbyhole/response" ) +var ( + // The default TTL that will be used with `sys/wrapping/wrap`, can be + // changed + DefaultWrappingTTL = "5m" + + // The default function used if no other function is set, which honors the + // env var and wraps `sys/wrapping/wrap` + DefaultWrappingLookupFunc = func(operation, path string) string { + if os.Getenv(EnvVaultWrapTTL) != "" { + return os.Getenv(EnvVaultWrapTTL) + } + + if (operation == "PUT" || operation == "POST") && path == "sys/wrapping/wrap" { + return DefaultWrappingTTL + } + + return "" + } +) + // Logical is used to perform logical backend operations on Vault. type Logical struct { c *Client @@ -38,7 +60,10 @@ func (c *Logical) Read(path string) (*Secret, error) { } func (c *Logical) List(path string) (*Secret, error) { - r := c.c.NewRequest("GET", "/v1/"+path) + r := c.c.NewRequest("LIST", "/v1/"+path) + // Set this for broader compatibility, but we use LIST above to be able to + // handle the wrapping lookup function + r.Method = "GET" r.Params.Set("list", "true") resp, err := c.c.RawRequest(r) if resp != nil { @@ -93,10 +118,48 @@ func (c *Logical) Delete(path string) (*Secret, error) { } func (c *Logical) Unwrap(wrappingToken string) (*Secret, error) { - origToken := c.c.Token() - defer c.c.SetToken(origToken) + var data map[string]interface{} + if wrappingToken != "" { + if c.c.Token() == "" { + c.c.SetToken(wrappingToken) + } else if wrappingToken != c.c.Token() { + data = map[string]interface{}{ + "token": wrappingToken, + } + } + } - c.c.SetToken(wrappingToken) + r := c.c.NewRequest("PUT", "/v1/sys/wrapping/unwrap") + if err := r.SetJSONBody(data); err != nil { + return nil, err + } + + resp, err := c.c.RawRequest(r) + if resp != nil { + defer resp.Body.Close() + } + if err != nil { + if resp != nil && resp.StatusCode != 404 { + return nil, err + } + } + if resp == nil { + return nil, nil + } + + switch resp.StatusCode { + case http.StatusOK: // New method is supported + return ParseSecret(resp.Body) + case http.StatusNotFound: // Fall back to old method + default: + return nil, nil + } + + if wrappingToken != "" { + origToken := c.c.Token() + defer c.c.SetToken(origToken) + c.c.SetToken(wrappingToken) + } secret, err := c.Read(wrappedResponseLocation) if err != nil { diff --git a/vendor/github.com/hashicorp/vault/api/ssh_agent.go b/vendor/github.com/hashicorp/vault/api/ssh_agent.go index 5a8192ae9..729fd99c4 100644 --- a/vendor/github.com/hashicorp/vault/api/ssh_agent.go +++ b/vendor/github.com/hashicorp/vault/api/ssh_agent.go @@ -62,6 +62,7 @@ type SSHHelperConfig struct { AllowedCidrList string `hcl:"allowed_cidr_list"` AllowedRoles string `hcl:"allowed_roles"` TLSSkipVerify bool `hcl:"tls_skip_verify"` + TLSServerName string `hcl:"tls_server_name"` } // SetTLSParameters sets the TLS parameters for this SSH agent. @@ -70,6 +71,7 @@ func (c *SSHHelperConfig) SetTLSParameters(clientConfig *Config, certPool *x509. InsecureSkipVerify: c.TLSSkipVerify, MinVersion: tls.VersionTLS12, RootCAs: certPool, + ServerName: c.TLSServerName, } transport := cleanhttp.DefaultTransport() @@ -77,6 +79,16 @@ func (c *SSHHelperConfig) SetTLSParameters(clientConfig *Config, certPool *x509. clientConfig.HttpClient.Transport = transport } +// Returns true if any of the following conditions are true: +// * CA cert is configured +// * CA path is configured +// * configured to skip certificate verification +// * TLS server name is configured +// +func (c *SSHHelperConfig) shouldSetTLSParameters() bool { + return c.CACert != "" || c.CAPath != "" || c.TLSServerName != "" || c.TLSSkipVerify +} + // NewClient returns a new client for the configuration. This client will be used by the // vault-ssh-helper to communicate with Vault server and verify the OTP entered by user. // If the configuration supplies Vault SSL certificates, then the client will @@ -89,7 +101,7 @@ func (c *SSHHelperConfig) NewClient() (*Client, error) { clientConfig.Address = c.VaultAddr // Check if certificates are provided via config file. - if c.CACert != "" || c.CAPath != "" || c.TLSSkipVerify { + if c.shouldSetTLSParameters() { rootConfig := &rootcerts.Config{ CAFile: c.CACert, CAPath: c.CAPath, @@ -145,6 +157,7 @@ func ParseSSHHelperConfig(contents string) (*SSHHelperConfig, error) { "allowed_cidr_list", "allowed_roles", "tls_skip_verify", + "tls_server_name", } if err := checkHCLKeys(list, valid); err != nil { return nil, multierror.Prefix(err, "ssh_helper:") diff --git a/vendor/github.com/hashicorp/vault/api/sys_init.go b/vendor/github.com/hashicorp/vault/api/sys_init.go index d307f732b..f824ab7dd 100644 --- a/vendor/github.com/hashicorp/vault/api/sys_init.go +++ b/vendor/github.com/hashicorp/vault/api/sys_init.go @@ -38,6 +38,7 @@ type InitRequest struct { RecoveryShares int `json:"recovery_shares"` RecoveryThreshold int `json:"recovery_threshold"` RecoveryPGPKeys []string `json:"recovery_pgp_keys"` + RootTokenPGPKey string `json:"root_token_pgp_key"` } type InitStatusResponse struct { diff --git a/vendor/vendor.json b/vendor/vendor.json index 7667e51ec..fec434164 100644 --- a/vendor/vendor.json +++ b/vendor/vendor.json @@ -774,10 +774,10 @@ "revisionTime": "2016-08-21T23:40:57Z" }, { - "checksumSHA1": "JH8wmQ8cWdn7mYu1T7gJ3IMIrec=", + "checksumSHA1": "31yBeS6U3xm7VJ7ZvDxRgBxXP0A=", "path": "github.com/hashicorp/vault/api", - "revision": "182ba68a9589d4cef95234134aaa498a686e3de3", - "revisionTime": "2016-08-21T23:40:57Z" + "revision": "f4adc7fa960ed8e828f94bc6785bcdbae8d1b263", + "revisionTime": "2016-12-16T21:07:16Z" }, { "checksumSHA1": "5lR6EdY0ARRdKAq3hZcL38STD8Q=", From ace50cfa19d3df6360a0e311952b177048e38bbc Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Thu, 19 Jan 2017 17:21:46 -0800 Subject: [PATCH 03/10] closer on the tests --- nomad/vault.go | 74 ++++++++++++++++++++++++--------------------- nomad/vault_test.go | 10 +++--- 2 files changed, 45 insertions(+), 39 deletions(-) diff --git a/nomad/vault.go b/nomad/vault.go index ff867e12e..495b2b6e5 100644 --- a/nomad/vault.go +++ b/nomad/vault.go @@ -52,42 +52,42 @@ const ( // ones token. vaultCapabilitiesLookupPath = "/sys/capabilities-self" - // vaultCapabilitiesCapability is the expected capability of Nomad's Vault - // token on the the path. - vaultCapabilitiesCapability = "update" - // vaultTokenLookupPath is the path used to lookup a token vaultTokenLookupPath = "auth/token/lookup" - // vaultTokenLookupCapability is the expected capability Nomad's - // Vault token should have on the path - vaultTokenLookupCapability = "update" - // vaultTokenRevokePath is the path used to revoke a token - vaultTokenRevokePath = "/auth/token/revoke-accessor/*" - - // vaultTokenRevokeCapability is the expected capability Nomad's - // Vault token should have on the path - vaultTokenRevokeCapability = "update" + vaultTokenRevokePath = "auth/token/revoke-accessor/*" // vaultRoleLookupPath is the path to lookup a role vaultRoleLookupPath = "auth/token/roles/%s" - // vaultRoleLookupCapability is the the expected capability Nomad's Vault - // token should have on the path - vaultRoleLookupCapability = "read" - // vaultRoleCreatePath is the path to create a token from a role vaultTokenRoleCreatePath = "auth/token/create/%s" - - // vaultTokenRoleCreateCapability is the the expected capability Nomad's Vault - // token should have on the path - vaultTokenRoleCreateCapability = "update" ) var ( // vaultUnrecoverableError matches unrecoverable errors vaultUnrecoverableError = regexp.MustCompile(`Code:\s+40(0|3|4)`) + + // vaultCapabilitiesCapability is the expected capability of Nomad's Vault + // token on the the path. + vaultCapabilitiesCapability = []string{"update", "root"} + + // vaultTokenLookupCapability is the expected capability Nomad's + // Vault token should have on the path + vaultTokenLookupCapability = []string{"update", "root"} + + // vaultTokenRevokeCapability is the expected capability Nomad's + // Vault token should have on the path + vaultTokenRevokeCapability = []string{"update", "root"} + + // vaultRoleLookupCapability is the the expected capability Nomad's Vault + // token should have on the path + vaultRoleLookupCapability = []string{"read", "root"} + + // vaultTokenRoleCreateCapability is the the expected capability Nomad's Vault + // token should have on the path + vaultTokenRoleCreateCapability = []string{"update", "root"} ) // VaultClient is the Servers interface for interfacing with Vault @@ -533,7 +533,7 @@ func (v *vaultClient) getWrappingFn() func(operation, path string) string { func (v *vaultClient) parseSelfToken() error { // Get the initial lease duration auth := v.client.Auth().Token() - self, err := auth.LookupSelf() + self, err := auth.Lookup(v.client.Token()) if err != nil { return fmt.Errorf("failed to lookup Vault periodic token: %v", err) } @@ -633,7 +633,7 @@ func (v *vaultClient) getRole() string { func (v *vaultClient) validateCapabilities(role string, root bool) error { // Check if the token can lookup capabilities. var mErr multierror.Error - _, err := v.hasCapability(vaultCapabilitiesLookupPath, vaultCapabilitiesCapability) + _, _, err := v.hasCapability(vaultCapabilitiesLookupPath, vaultCapabilitiesCapability) if err != nil { // Check if there is a permission denied if vaultUnrecoverableError.MatchString(err.Error()) { @@ -650,14 +650,15 @@ func (v *vaultClient) validateCapabilities(role string, root bool) error { } } - // verify is a helper function that verifies the token has the capability on - // the given path and adds an issue to the error - verify := func(path, capability string) { - ok, err := v.hasCapability(path, capability) + // verify is a helper function that verifies the token has one of the + // capabilities on the given path and adds an issue to the error + verify := func(path string, requiredCaps []string) { + ok, caps, err := v.hasCapability(path, requiredCaps) if err != nil { multierror.Append(&mErr, err) } else if !ok { - multierror.Append(&mErr, fmt.Errorf("token doesn't have %q capability on %q", capability, path)) + multierror.Append(&mErr, + fmt.Errorf("token must have one of the following capabilities %v on %q; has %v", requiredCaps, path, caps)) } } @@ -681,19 +682,22 @@ func (v *vaultClient) validateCapabilities(role string, root bool) error { return mErr.ErrorOrNil() } -// hasCapability takes a path and returns whether the token has the required -// capability at the given path. -func (v *vaultClient) hasCapability(path string, required string) (bool, error) { +// hasCapability takes a path and returns whether the token has at least one of +// the required capabilities on the given path. It also returns the set of +// capabilities the token does have as well as any error that occured. +func (v *vaultClient) hasCapability(path string, required []string) (bool, []string, error) { caps, err := v.client.Sys().CapabilitiesSelf(path) if err != nil { - return false, err + return false, nil, err } for _, c := range caps { - if c == required { - return true, nil + for _, r := range required { + if c == r { + return true, caps, nil + } } } - return false, nil + return false, caps, nil } // validateRole contacts Vault and checks that the given Vault role is valid for diff --git a/nomad/vault_test.go b/nomad/vault_test.go index d1a9188c4..65c12ed70 100644 --- a/nomad/vault_test.go +++ b/nomad/vault_test.go @@ -26,15 +26,15 @@ const ( capabilities = ["create", "update"] } -path "auth/token/lookup/*" { - capabilities = ["read"] +path "auth/token/lookup" { + capabilities = ["update"] } path "auth/token/roles/test" { capabilities = ["read"] } -path "/auth/token/revoke-accessor/*" { +path "auth/token/revoke-accessor/*" { capabilities = ["update"] } ` @@ -227,7 +227,9 @@ func testVaultRoleAndToken(v *testutil.TestVault, t *testing.T, data map[string] // Create a new token with the role a := v.Client.Auth().Token() - req := vapi.TokenCreateRequest{} + req := vapi.TokenCreateRequest{ + Policies: []string{"auth"}, + } s, err := a.CreateWithRole(&req, "test") if err != nil { t.Fatalf("failed to create child token: %v", err) From 7d1ec25d092fd8eca4ca826f9eab25cefe3ffc45 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Fri, 20 Jan 2017 10:06:47 -0800 Subject: [PATCH 04/10] Test pass --- nomad/vault.go | 2 +- nomad/vault_test.go | 92 ++++++++++++++++++++++----------------------- 2 files changed, 47 insertions(+), 47 deletions(-) diff --git a/nomad/vault.go b/nomad/vault.go index 495b2b6e5..fdee7e440 100644 --- a/nomad/vault.go +++ b/nomad/vault.go @@ -56,7 +56,7 @@ const ( vaultTokenLookupPath = "auth/token/lookup" // vaultTokenRevokePath is the path used to revoke a token - vaultTokenRevokePath = "auth/token/revoke-accessor/*" + vaultTokenRevokePath = "auth/token/revoke-accessor" // vaultRoleLookupPath is the path to lookup a role vaultRoleLookupPath = "auth/token/roles/%s" diff --git a/nomad/vault_test.go b/nomad/vault_test.go index 65c12ed70..fd6f801ea 100644 --- a/nomad/vault_test.go +++ b/nomad/vault_test.go @@ -34,12 +34,53 @@ path "auth/token/roles/test" { capabilities = ["read"] } -path "auth/token/revoke-accessor/*" { +path "auth/token/revoke-accessor" { capabilities = ["update"] } ` ) +// defaultTestVaultRoleAndToken creates a test Vault role and returns a token +// created in that role +func defaultTestVaultRoleAndToken(v *testutil.TestVault, t *testing.T, rolePeriod int) string { + d := make(map[string]interface{}, 2) + d["allowed_policies"] = "auth" + d["period"] = rolePeriod + return testVaultRoleAndToken(v, t, d, []string{"auth"}) +} + +// testVaultRoleAndToken creates a test Vault role with the specified data after +// creating the auth policy. It then returns a token created in that role with +// the passed policies +func testVaultRoleAndToken(v *testutil.TestVault, t *testing.T, data map[string]interface{}, policies []string) string { + // Build the auth policy + sys := v.Client.Sys() + if err := sys.PutPolicy("auth", authPolicy); err != nil { + t.Fatalf("failed to create auth policy: %v", err) + } + + // Build a role + l := v.Client.Logical() + l.Write("auth/token/roles/test", data) + + // Create a new token with the role + a := v.Client.Auth().Token() + req := vapi.TokenCreateRequest{ + Policies: policies, + } + s, err := a.CreateWithRole(&req, "test") + if err != nil { + t.Fatalf("failed to create child token: %v", err) + } + + // Get the client token + if s == nil || s.Auth == nil { + t.Fatalf("bad secret response: %+v", s) + } + + return s.Auth.ClientToken +} + func TestVaultClient_BadConfig(t *testing.T) { conf := &config.VaultConfig{} logger := log.New(os.Stderr, "", log.LstdFlags) @@ -102,7 +143,7 @@ func TestVaultClient_ValidateRole(t *testing.T) { "renewable": true, "explicit_max_ttl": 10, } - v.Config.Token = testVaultRoleAndToken(v, t, data) + v.Config.Token = testVaultRoleAndToken(v, t, data, nil) logger := log.New(os.Stderr, "", log.LstdFlags) v.Config.ConnectionRetryIntv = 100 * time.Millisecond @@ -203,46 +244,6 @@ func TestVaultClient_SetConfig(t *testing.T) { } } -// defaultTestVaultRoleAndToken creates a test Vault role and returns a token -// created in that role -func defaultTestVaultRoleAndToken(v *testutil.TestVault, t *testing.T, rolePeriod int) string { - d := make(map[string]interface{}, 2) - d["allowed_policies"] = "auth" - d["period"] = rolePeriod - return testVaultRoleAndToken(v, t, d) -} - -// testVaultRoleAndToken creates a test Vault role with the specified data and -// returns a token created in that role -func testVaultRoleAndToken(v *testutil.TestVault, t *testing.T, data map[string]interface{}) string { - // Build the auth policy - sys := v.Client.Sys() - if err := sys.PutPolicy("auth", authPolicy); err != nil { - t.Fatalf("failed to create auth policy: %v", err) - } - - // Build a role - l := v.Client.Logical() - l.Write("auth/token/roles/test", data) - - // Create a new token with the role - a := v.Client.Auth().Token() - req := vapi.TokenCreateRequest{ - Policies: []string{"auth"}, - } - s, err := a.CreateWithRole(&req, "test") - if err != nil { - t.Fatalf("failed to create child token: %v", err) - } - - // Get the client token - if s == nil || s.Auth == nil { - t.Fatalf("bad secret response: %+v", s) - } - - return s.Auth.ClientToken -} - func TestVaultClient_RenewalLoop(t *testing.T) { v := testutil.NewTestVault(t).Start() defer v.Stop() @@ -848,16 +849,15 @@ func TestVaultClient_RevokeTokens_Role(t *testing.T) { } // Lookup the token and make sure we get an error + if purged != 2 { + t.Fatalf("Expected purged 2; got %d", purged) + } if s, err := auth.Lookup(t1.Auth.ClientToken); err == nil { t.Fatalf("Revoked token lookup didn't fail: %+v", s) } if s, err := auth.Lookup(t2.Auth.ClientToken); err == nil { t.Fatalf("Revoked token lookup didn't fail: %+v", s) } - - if purged != 2 { - t.Fatalf("Expected purged 2; got %d", purged) - } } func waitForConnection(v *vaultClient, t *testing.T) { From faa50b851efa88462c20dd49023faaf02f8b1b7a Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Fri, 20 Jan 2017 10:26:25 -0800 Subject: [PATCH 05/10] Cleanup errors/comments --- nomad/vault.go | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/nomad/vault.go b/nomad/vault.go index fdee7e440..6f80f4544 100644 --- a/nomad/vault.go +++ b/nomad/vault.go @@ -70,23 +70,28 @@ var ( vaultUnrecoverableError = regexp.MustCompile(`Code:\s+40(0|3|4)`) // vaultCapabilitiesCapability is the expected capability of Nomad's Vault - // token on the the path. + // token on the the path. The token must have at least one of the + // capabilities. vaultCapabilitiesCapability = []string{"update", "root"} // vaultTokenLookupCapability is the expected capability Nomad's - // Vault token should have on the path + // Vault token should have on the path. The token must have at least one of + // the capabilities. vaultTokenLookupCapability = []string{"update", "root"} // vaultTokenRevokeCapability is the expected capability Nomad's - // Vault token should have on the path + // Vault token should have on the path. The token must have at least one of + // the capabilities. vaultTokenRevokeCapability = []string{"update", "root"} // vaultRoleLookupCapability is the the expected capability Nomad's Vault - // token should have on the path + // token should have on the path. The token must have at least one of the + // capabilities. vaultRoleLookupCapability = []string{"read", "root"} // vaultTokenRoleCreateCapability is the the expected capability Nomad's Vault - // token should have on the path + // token should have on the path. The token must have at least one of the + // capabilities. vaultTokenRoleCreateCapability = []string{"update", "root"} ) @@ -552,6 +557,7 @@ func (v *vaultClient) parseSelfToken() error { } } + // Store the token data data.Root = root v.tokenData = &data @@ -641,7 +647,8 @@ func (v *vaultClient) validateCapabilities(role string, root bool) error { // can't tell if the Vault token will work msg := fmt.Sprintf("Can not lookup token capabilities. "+ "As such certain operations may fail in the future. "+ - "Please give Nomad a Vault token with %q on %q", + "Please give Nomad a Vault token with one of the following "+ + "capabilities %q on %q so that the required capabilities can be verified", vaultCapabilitiesCapability, vaultCapabilitiesLookupPath) v.logger.Printf("[WARN] vault: %s", msg) return nil @@ -658,7 +665,7 @@ func (v *vaultClient) validateCapabilities(role string, root bool) error { multierror.Append(&mErr, err) } else if !ok { multierror.Append(&mErr, - fmt.Errorf("token must have one of the following capabilities %v on %q; has %v", requiredCaps, path, caps)) + fmt.Errorf("token must have one of the following capabilities %q on %q; has %v", requiredCaps, path, caps)) } } From 76dbc4aee14dca7c6f9fd86c614ecf996a7e435c Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Fri, 20 Jan 2017 14:23:50 -0800 Subject: [PATCH 06/10] verify we can renew ourselves --- nomad/vault.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/nomad/vault.go b/nomad/vault.go index 6f80f4544..235a3d4e2 100644 --- a/nomad/vault.go +++ b/nomad/vault.go @@ -52,6 +52,9 @@ const ( // ones token. vaultCapabilitiesLookupPath = "/sys/capabilities-self" + // vaultTokenRenewPath is the path used to renew our token + vaultTokenRenewPath = "auth/token/renew-self" + // vaultTokenLookupPath is the path used to lookup a token vaultTokenLookupPath = "auth/token/lookup" @@ -74,6 +77,11 @@ var ( // capabilities. vaultCapabilitiesCapability = []string{"update", "root"} + // vaultTokenRenewCapability is the expected capability Nomad's + // Vault token should have on the path. The token must have at least one of + // the capabilities. + vaultTokenRenewCapability = []string{"update", "root"} + // vaultTokenLookupCapability is the expected capability Nomad's // Vault token should have on the path. The token must have at least one of // the capabilities. @@ -458,7 +466,7 @@ func (v *vaultClient) renewalLoop() { // Set base values and add some backoff - v.logger.Printf("[DEBUG] vault: got error or bad auth, so backing off: %v", err) + v.logger.Printf("[WARN] vault: got error or bad auth, so backing off: %v", err) switch { case backoff < 5: backoff = 5 @@ -674,6 +682,9 @@ func (v *vaultClient) validateCapabilities(role string, root bool) error { verify(vaultTokenLookupPath, vaultTokenLookupCapability) } + // Verify we can renew our selves tokens + verify(vaultTokenRenewPath, vaultTokenRenewCapability) + // Verify we can revoke tokens verify(vaultTokenRevokePath, vaultTokenRevokeCapability) From 442d775fb2607a3c0bbb9a861fd34b8383de0459 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Sat, 21 Jan 2017 17:33:35 -0800 Subject: [PATCH 07/10] Test new functionality --- nomad/vault.go | 23 ++-- nomad/vault_test.go | 255 +++++++++++++++++++++++++++++++++++++++----- 2 files changed, 245 insertions(+), 33 deletions(-) diff --git a/nomad/vault.go b/nomad/vault.go index 235a3d4e2..c885f111e 100644 --- a/nomad/vault.go +++ b/nomad/vault.go @@ -526,8 +526,9 @@ func (v *vaultClient) renew() error { // getWrappingFn returns an appropriate wrapping function for Nomad Servers func (v *vaultClient) getWrappingFn() func(operation, path string) string { createPath := "auth/token/create" - if !v.tokenData.Root { - createPath = fmt.Sprintf("auth/token/create/%s", v.getRole()) + role := v.getRole() + if role != "" { + createPath = fmt.Sprintf("auth/token/create/%s", role) } return func(operation, path string) string { @@ -733,10 +734,11 @@ func (v *vaultClient) validateRole(role string) error { // Read and parse the fields var data struct { - ExplicitMaxTtl int `mapstructure:"explicit_max_ttl"` - Orphan bool - Period int - Renewable bool + ExplicitMaxTtl int `mapstructure:"explicit_max_ttl"` + Orphan bool + Period int + Renewable bool + DisallowedPolicies []string `mapstructure:"disallowed_policies"` } if err := mapstructure.WeakDecode(rsecret.Data, &data); err != nil { return fmt.Errorf("failed to parse Vault role's data block: %v", err) @@ -760,6 +762,12 @@ func (v *vaultClient) validateRole(role string) error { multierror.Append(&mErr, fmt.Errorf("Role must have a non-zero period to make tokens periodic.")) } + for _, d := range data.DisallowedPolicies { + if d == "default" { + multierror.Append(&mErr, fmt.Errorf("Role can not disallow allow default policy")) + } + } + return mErr.ErrorOrNil() } @@ -837,7 +845,8 @@ func (v *vaultClient) CreateToken(ctx context.Context, a *structs.Allocation, ta // token or a role based token var secret *vapi.Secret var err error - if v.tokenData.Root { + role := v.getRole() + if v.tokenData.Root && role == "" { req.Period = v.childTTL secret, err = v.auth.Create(req) } else { diff --git a/nomad/vault_test.go b/nomad/vault_test.go index fd6f801ea..682d989ac 100644 --- a/nomad/vault_test.go +++ b/nomad/vault_test.go @@ -21,9 +21,10 @@ import ( ) const ( - // authPolicy is a policy that allows token creation operations - authPolicy = `path "auth/token/create/test" { - capabilities = ["create", "update"] + // nomadRoleManagementPolicy is a policy that allows nomad to manage tokens + nomadRoleManagementPolicy = ` +path "auth/token/renew-self" { + capabilities = ["update"] } path "auth/token/lookup" { @@ -37,26 +38,74 @@ path "auth/token/roles/test" { path "auth/token/revoke-accessor" { capabilities = ["update"] } +` + + // tokenLookupPolicy allows a token to be looked up + tokenLookupPolicy = ` +path "auth/token/lookup" { + capabilities = ["update"] +} +` + + // nomadRoleCreatePolicy gives the ability to create the role and derive tokens + // from the test role + nomadRoleCreatePolicy = ` +path "auth/token/create/test" { + capabilities = ["create", "update"] +} +` + + // secretPolicy gives access to the secret mount + secretPolicy = ` +path "secret/*" { + capabilities = ["create", "read", "update", "delete", "list"] +} ` ) -// defaultTestVaultRoleAndToken creates a test Vault role and returns a token +// defaultTestVaultWhitelistRoleAndToken creates a test Vault role and returns a token // created in that role -func defaultTestVaultRoleAndToken(v *testutil.TestVault, t *testing.T, rolePeriod int) string { +func defaultTestVaultWhitelistRoleAndToken(v *testutil.TestVault, t *testing.T, rolePeriod int) string { + vaultPolicies := map[string]string{ + "nomad-role-create": nomadRoleCreatePolicy, + "nomad-role-management": nomadRoleManagementPolicy, + } d := make(map[string]interface{}, 2) - d["allowed_policies"] = "auth" + d["allowed_policies"] = "nomad-role-create,nomad-role-management" d["period"] = rolePeriod - return testVaultRoleAndToken(v, t, d, []string{"auth"}) + return testVaultRoleAndToken(v, t, vaultPolicies, d, + []string{"nomad-role-create", "nomad-role-management"}) } -// testVaultRoleAndToken creates a test Vault role with the specified data after -// creating the auth policy. It then returns a token created in that role with -// the passed policies -func testVaultRoleAndToken(v *testutil.TestVault, t *testing.T, data map[string]interface{}, policies []string) string { - // Build the auth policy +// defaultTestVaultBlacklistRoleAndToken creates a test Vault role using +// disallowed_policies and returns a token created in that role +func defaultTestVaultBlacklistRoleAndToken(v *testutil.TestVault, t *testing.T, rolePeriod int) string { + vaultPolicies := map[string]string{ + "nomad-role-create": nomadRoleCreatePolicy, + "nomad-role-management": nomadRoleManagementPolicy, + "secrets": secretPolicy, + } + + // XXX if root is included it works but if not it doesn't. Seems like a + // Vault bug + d := make(map[string]interface{}, 2) + d["disallowed_policies"] = "nomad-role-create,root" + d["period"] = rolePeriod + return testVaultRoleAndToken(v, t, vaultPolicies, d, + []string{"default", "nomad-role-create", "nomad-role-management"}) +} + +// testVaultRoleAndToken writes the vaultPolicies to vault and then creates a +// test role with the passed data. After that it derives a token from the role +// with the tokenPolicies +func testVaultRoleAndToken(v *testutil.TestVault, t *testing.T, vaultPolicies map[string]string, + data map[string]interface{}, tokenPolicies []string) string { + // Write the policies sys := v.Client.Sys() - if err := sys.PutPolicy("auth", authPolicy); err != nil { - t.Fatalf("failed to create auth policy: %v", err) + for p, data := range vaultPolicies { + if err := sys.PutPolicy(p, data); err != nil { + t.Fatalf("failed to create %q policy: %v", p, err) + } } // Build a role @@ -66,7 +115,7 @@ func testVaultRoleAndToken(v *testutil.TestVault, t *testing.T, data map[string] // Create a new token with the role a := v.Client.Auth().Token() req := vapi.TokenCreateRequest{ - Policies: policies, + Policies: tokenPolicies, } s, err := a.CreateWithRole(&req, "test") if err != nil { @@ -137,13 +186,17 @@ func TestVaultClient_ValidateRole(t *testing.T) { defer v.Stop() // Set the configs token in a new test role + vaultPolicies := map[string]string{ + "nomad-role-create": nomadRoleCreatePolicy, + "nomad-role-management": nomadRoleManagementPolicy, + } data := map[string]interface{}{ "allowed_policies": "default,root", "orphan": true, "renewable": true, "explicit_max_ttl": 10, } - v.Config.Token = testVaultRoleAndToken(v, t, data, nil) + v.Config.Token = testVaultRoleAndToken(v, t, vaultPolicies, data, nil) logger := log.New(os.Stderr, "", log.LstdFlags) v.Config.ConnectionRetryIntv = 100 * time.Millisecond @@ -180,6 +233,59 @@ func TestVaultClient_ValidateRole(t *testing.T) { } } +func TestVaultClient_ValidateToken(t *testing.T) { + v := testutil.NewTestVault(t).Start() + defer v.Stop() + + // Set the configs token in a new test role + vaultPolicies := map[string]string{ + "nomad-role-create": nomadRoleCreatePolicy, + "token-lookup": tokenLookupPolicy, + } + data := map[string]interface{}{ + "allowed_policies": "token-lookup,nomad-role-create", + "period": 10, + } + v.Config.Token = testVaultRoleAndToken(v, t, vaultPolicies, data, []string{"token-lookup", "nomad-role-create"}) + + logger := log.New(os.Stderr, "", log.LstdFlags) + v.Config.ConnectionRetryIntv = 100 * time.Millisecond + client, err := NewVaultClient(v.Config, logger, nil) + if err != nil { + t.Fatalf("failed to build vault client: %v", err) + } + defer client.Stop() + + // Wait for an error + var conn bool + var connErr error + testutil.WaitForResult(func() (bool, error) { + conn, connErr = client.ConnectionEstablished() + if conn { + return false, fmt.Errorf("Should not connect") + } + + if connErr == nil { + return false, fmt.Errorf("expect an error") + } + + return true, nil + }, func(err error) { + t.Fatalf("bad: %v", err) + }) + + errStr := connErr.Error() + if !strings.Contains(errStr, vaultTokenRevokePath) { + t.Fatalf("Expect orphan error") + } + if !strings.Contains(errStr, fmt.Sprintf(vaultRoleLookupPath, "test")) { + t.Fatalf("Expect explicit max ttl error") + } + if !strings.Contains(errStr, "token must have one of the following") { + t.Fatalf("Expect explicit max ttl error") + } +} + func TestVaultClient_SetActive(t *testing.T) { v := testutil.NewTestVault(t).Start() defer v.Stop() @@ -217,7 +323,7 @@ func TestVaultClient_SetConfig(t *testing.T) { defer v2.Stop() // Set the configs token in a new test role - v2.Config.Token = defaultTestVaultRoleAndToken(v2, t, 20) + v2.Config.Token = defaultTestVaultWhitelistRoleAndToken(v2, t, 20) logger := log.New(os.Stderr, "", log.LstdFlags) client, err := NewVaultClient(v.Config, logger, nil) @@ -239,7 +345,7 @@ func TestVaultClient_SetConfig(t *testing.T) { waitForConnection(client, t) - if client.tokenData == nil || len(client.tokenData.Policies) != 2 { + if client.tokenData == nil || len(client.tokenData.Policies) != 3 { t.Fatalf("unexpected token: %v", client.tokenData) } } @@ -249,7 +355,7 @@ func TestVaultClient_RenewalLoop(t *testing.T) { defer v.Stop() // Set the configs token in a new test role - v.Config.Token = defaultTestVaultRoleAndToken(v, t, 5) + v.Config.Token = defaultTestVaultWhitelistRoleAndToken(v, t, 5) // Start the client logger := log.New(os.Stderr, "", log.LstdFlags) @@ -389,7 +495,7 @@ func TestVaultClient_LookupToken_Role(t *testing.T) { defer v.Stop() // Set the configs token in a new test role - v.Config.Token = defaultTestVaultRoleAndToken(v, t, 5) + v.Config.Token = defaultTestVaultWhitelistRoleAndToken(v, t, 5) logger := log.New(os.Stderr, "", log.LstdFlags) client, err := NewVaultClient(v.Config, logger, nil) @@ -412,7 +518,7 @@ func TestVaultClient_LookupToken_Role(t *testing.T) { t.Fatalf("failed to parse policies: %v", err) } - expected := []string{"auth", "default"} + expected := []string{"default", "nomad-role-create", "nomad-role-management"} if !reflect.DeepEqual(policies, expected) { t.Fatalf("Unexpected policies; got %v; want %v", policies, expected) } @@ -546,12 +652,12 @@ func TestVaultClient_CreateToken_Root(t *testing.T) { } } -func TestVaultClient_CreateToken_Role(t *testing.T) { +func TestVaultClient_CreateToken_Whitelist_Role(t *testing.T) { v := testutil.NewTestVault(t).Start() defer v.Stop() // Set the configs token in a new test role - v.Config.Token = defaultTestVaultRoleAndToken(v, t, 5) + v.Config.Token = defaultTestVaultWhitelistRoleAndToken(v, t, 5) // Start the client logger := log.New(os.Stderr, "", log.LstdFlags) @@ -593,12 +699,109 @@ func TestVaultClient_CreateToken_Role(t *testing.T) { } } +func TestVaultClient_CreateToken_Root_Target_Role(t *testing.T) { + v := testutil.NewTestVault(t).Start() + defer v.Stop() + + // Create the test role + defaultTestVaultWhitelistRoleAndToken(v, t, 5) + + // Target the test role + v.Config.Role = "test" + + // Start the client + logger := log.New(os.Stderr, "", log.LstdFlags) + client, err := NewVaultClient(v.Config, logger, nil) + if err != nil { + t.Fatalf("failed to build vault client: %v", err) + } + client.SetActive(true) + defer client.Stop() + + waitForConnection(client, t) + + // Create an allocation that requires a Vault policy + a := mock.Alloc() + task := a.Job.TaskGroups[0].Tasks[0] + task.Vault = &structs.Vault{Policies: []string{"default"}} + + s, err := client.CreateToken(context.Background(), a, task.Name) + if err != nil { + t.Fatalf("CreateToken failed: %v", err) + } + + // Ensure that created secret is a wrapped token + if s == nil || s.WrapInfo == nil { + t.Fatalf("Bad secret: %#v", s) + } + + d, err := time.ParseDuration(vaultTokenCreateTTL) + if err != nil { + t.Fatalf("bad: %v", err) + } + + if s.WrapInfo.WrappedAccessor == "" { + t.Fatalf("Bad accessor: %v", s.WrapInfo.WrappedAccessor) + } else if s.WrapInfo.Token == "" { + t.Fatalf("Bad token: %v", s.WrapInfo.WrappedAccessor) + } else if s.WrapInfo.TTL != int(d.Seconds()) { + t.Fatalf("Bad ttl: %v", s.WrapInfo.WrappedAccessor) + } +} + +func TestVaultClient_CreateToken_Blacklist_Role(t *testing.T) { + v := testutil.NewTestVault(t).Start() + defer v.Stop() + + // Set the configs token in a new test role + v.Config.Token = defaultTestVaultBlacklistRoleAndToken(v, t, 5) + + // Start the client + logger := log.New(os.Stderr, "", log.LstdFlags) + client, err := NewVaultClient(v.Config, logger, nil) + if err != nil { + t.Fatalf("failed to build vault client: %v", err) + } + client.SetActive(true) + defer client.Stop() + + waitForConnection(client, t) + + // Create an allocation that requires a Vault policy + a := mock.Alloc() + task := a.Job.TaskGroups[0].Tasks[0] + task.Vault = &structs.Vault{Policies: []string{"secrets"}} + + s, err := client.CreateToken(context.Background(), a, task.Name) + if err != nil { + t.Fatalf("CreateToken failed: %v", err) + } + + // Ensure that created secret is a wrapped token + if s == nil || s.WrapInfo == nil { + t.Fatalf("Bad secret: %#v", s) + } + + d, err := time.ParseDuration(vaultTokenCreateTTL) + if err != nil { + t.Fatalf("bad: %v", err) + } + + if s.WrapInfo.WrappedAccessor == "" { + t.Fatalf("Bad accessor: %v", s.WrapInfo.WrappedAccessor) + } else if s.WrapInfo.Token == "" { + t.Fatalf("Bad token: %v", s.WrapInfo.WrappedAccessor) + } else if s.WrapInfo.TTL != int(d.Seconds()) { + t.Fatalf("Bad ttl: %v", s.WrapInfo.WrappedAccessor) + } +} + func TestVaultClient_CreateToken_Role_InvalidToken(t *testing.T) { v := testutil.NewTestVault(t).Start() defer v.Stop() // Set the configs token in a new test role - defaultTestVaultRoleAndToken(v, t, 5) + defaultTestVaultWhitelistRoleAndToken(v, t, 5) v.Config.Token = "foo-bar" // Start the client @@ -637,7 +840,7 @@ func TestVaultClient_CreateToken_Role_Unrecoverable(t *testing.T) { defer v.Stop() // Set the configs token in a new test role - v.Config.Token = defaultTestVaultRoleAndToken(v, t, 5) + v.Config.Token = defaultTestVaultWhitelistRoleAndToken(v, t, 5) // Start the client logger := log.New(os.Stderr, "", log.LstdFlags) @@ -799,7 +1002,7 @@ func TestVaultClient_RevokeTokens_Role(t *testing.T) { defer v.Stop() // Set the configs token in a new test role - v.Config.Token = defaultTestVaultRoleAndToken(v, t, 5) + v.Config.Token = defaultTestVaultWhitelistRoleAndToken(v, t, 5) purged := 0 purge := func(accessors []*structs.VaultAccessor) error { From dd4b959df9657bec6a296c9c47811f034b28d587 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Sat, 21 Jan 2017 17:53:30 -0800 Subject: [PATCH 08/10] update website --- website/source/data/vault/nomad-server-policy.hcl | 2 +- website/source/docs/agent/configuration/vault.html.md | 6 ++++++ website/source/docs/vault-integration/index.html.md | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/website/source/data/vault/nomad-server-policy.hcl b/website/source/data/vault/nomad-server-policy.hcl index 95a5fbe22..d1190af7e 100644 --- a/website/source/data/vault/nomad-server-policy.hcl +++ b/website/source/data/vault/nomad-server-policy.hcl @@ -1,6 +1,6 @@ # Allow creating tokens under the role path "auth/token/create/nomad-server" { - capabilities = ["create", "update"] + capabilities = ["update"] } # Allow looking up the role diff --git a/website/source/docs/agent/configuration/vault.html.md b/website/source/docs/agent/configuration/vault.html.md index e1e86abf0..a491d6348 100644 --- a/website/source/docs/agent/configuration/vault.html.md +++ b/website/source/docs/agent/configuration/vault.html.md @@ -47,6 +47,12 @@ vault { - `enabled` `(bool: false)` - Specifies if the Vault integration should be activated. +- `create_from_role` `(string: "")` - Specifies the role to create tokens from. + The token given to Nomad does not have to be created from this role but must + have "update" capability on "auth/token/create/" path in + Vault. If this value is unset and the token is created from a role, the value + is defaulted to the role the token is from. + - `task_token_ttl` `(string: "")` - Specifies the TTL of created tokens when using a root token. This is specified using a label suffix like "30s" or "1h". diff --git a/website/source/docs/vault-integration/index.html.md b/website/source/docs/vault-integration/index.html.md index 7221e4653..7baff34b0 100644 --- a/website/source/docs/vault-integration/index.html.md +++ b/website/source/docs/vault-integration/index.html.md @@ -69,7 +69,7 @@ when creating the role with the Vault endpoint `/auth/token/roles/`: ```hcl # Allow creating tokens under the role path "auth/token/create/nomad-server" { - capabilities = ["create", "update"] + capabilities = ["update"] } # Allow looking up the role From faf9745256c5e1c01b8042d0d791002b70c5eb9e Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Mon, 23 Jan 2017 10:40:28 -0800 Subject: [PATCH 09/10] Fix blacklist test --- nomad/vault_test.go | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/nomad/vault_test.go b/nomad/vault_test.go index 682d989ac..e17c3bce0 100644 --- a/nomad/vault_test.go +++ b/nomad/vault_test.go @@ -86,13 +86,27 @@ func defaultTestVaultBlacklistRoleAndToken(v *testutil.TestVault, t *testing.T, "secrets": secretPolicy, } - // XXX if root is included it works but if not it doesn't. Seems like a - // Vault bug + // Create the role d := make(map[string]interface{}, 2) - d["disallowed_policies"] = "nomad-role-create,root" + d["disallowed_policies"] = "nomad-role-create" d["period"] = rolePeriod - return testVaultRoleAndToken(v, t, vaultPolicies, d, - []string{"default", "nomad-role-create", "nomad-role-management"}) + testVaultRoleAndToken(v, t, vaultPolicies, d, []string{"default"}) + + // Create a token that can use the role + a := v.Client.Auth().Token() + req := &vapi.TokenCreateRequest{ + Policies: []string{"nomad-role-create", "nomad-role-management"}, + } + s, err := a.Create(req) + if err != nil { + t.Fatalf("failed to create child token: %v", err) + } + + if s == nil || s.Auth == nil { + t.Fatalf("bad secret response: %+v", s) + } + + return s.Auth.ClientToken } // testVaultRoleAndToken writes the vaultPolicies to vault and then creates a @@ -755,6 +769,7 @@ func TestVaultClient_CreateToken_Blacklist_Role(t *testing.T) { // Set the configs token in a new test role v.Config.Token = defaultTestVaultBlacklistRoleAndToken(v, t, 5) + v.Config.Role = "test" // Start the client logger := log.New(os.Stderr, "", log.LstdFlags) From 94ed50aa594b08d2ed6a907a4483b85403984107 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Mon, 23 Jan 2017 11:46:27 -0800 Subject: [PATCH 10/10] Prefer looking up using self path and remove checking for default policy --- nomad/vault.go | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/nomad/vault.go b/nomad/vault.go index c885f111e..a78c82210 100644 --- a/nomad/vault.go +++ b/nomad/vault.go @@ -58,6 +58,9 @@ const ( // vaultTokenLookupPath is the path used to lookup a token vaultTokenLookupPath = "auth/token/lookup" + // vaultTokenLookupSelfPath is the path used to lookup self token + vaultTokenLookupSelfPath = "auth/token/lookup-self" + // vaultTokenRevokePath is the path used to revoke a token vaultTokenRevokePath = "auth/token/revoke-accessor" @@ -87,6 +90,11 @@ var ( // the capabilities. vaultTokenLookupCapability = []string{"update", "root"} + // vaultTokenLookupSelfCapability is the expected capability Nomad's + // Vault token should have on the path. The token must have at least one of + // the capabilities. + vaultTokenLookupSelfCapability = []string{"update", "root"} + // vaultTokenRevokeCapability is the expected capability Nomad's // Vault token should have on the path. The token must have at least one of // the capabilities. @@ -547,10 +555,18 @@ func (v *vaultClient) getWrappingFn() func(operation, path string) string { func (v *vaultClient) parseSelfToken() error { // Get the initial lease duration auth := v.client.Auth().Token() - self, err := auth.Lookup(v.client.Token()) + var self *vapi.Secret + + // Try looking up the token using the self endpoint + secret, err := auth.LookupSelf() if err != nil { - return fmt.Errorf("failed to lookup Vault periodic token: %v", err) + // Try looking up our token directly + self, err = auth.Lookup(v.client.Token()) + if err != nil { + return fmt.Errorf("failed to lookup Vault periodic token: %v", err) + } } + self = secret // Read and parse the fields var data tokenData @@ -734,11 +750,10 @@ func (v *vaultClient) validateRole(role string) error { // Read and parse the fields var data struct { - ExplicitMaxTtl int `mapstructure:"explicit_max_ttl"` - Orphan bool - Period int - Renewable bool - DisallowedPolicies []string `mapstructure:"disallowed_policies"` + ExplicitMaxTtl int `mapstructure:"explicit_max_ttl"` + Orphan bool + Period int + Renewable bool } if err := mapstructure.WeakDecode(rsecret.Data, &data); err != nil { return fmt.Errorf("failed to parse Vault role's data block: %v", err) @@ -762,12 +777,6 @@ func (v *vaultClient) validateRole(role string) error { multierror.Append(&mErr, fmt.Errorf("Role must have a non-zero period to make tokens periodic.")) } - for _, d := range data.DisallowedPolicies { - if d == "default" { - multierror.Append(&mErr, fmt.Errorf("Role can not disallow allow default policy")) - } - } - return mErr.ErrorOrNil() }