Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP Experiment in handling Sensitive keys #6595

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions client/acl.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
metrics "github.com/armon/go-metrics"
lru "github.com/hashicorp/golang-lru"
"github.com/hashicorp/nomad/acl"
"github.com/hashicorp/nomad/helper/sensitive"
"github.com/hashicorp/nomad/nomad/structs"
)

Expand Down Expand Up @@ -69,12 +70,12 @@ func (c *cachedACLValue) Age() time.Duration {

// ResolveToken is used to translate an ACL Token Secret ID into
// an ACL object, nil if ACLs are disabled, or an error.
func (c *Client) ResolveToken(secretID string) (*acl.ACL, error) {
func (c *Client) ResolveToken(secretID sensitive.Sensitive) (*acl.ACL, error) {
a, _, err := c.resolveTokenAndACL(secretID)
return a, err
}

func (c *Client) resolveTokenAndACL(secretID string) (*acl.ACL, *structs.ACLToken, error) {
func (c *Client) resolveTokenAndACL(secretID sensitive.Sensitive) (*acl.ACL, *structs.ACLToken, error) {
// Fast-path if ACLs are disabled
if !c.config.ACLEnabled {
return nil, nil, nil
Expand Down Expand Up @@ -112,7 +113,7 @@ func (c *Client) resolveTokenAndACL(secretID string) (*acl.ACL, *structs.ACLToke
// resolveTokenValue is used to translate a secret ID into an ACL token with caching
// We use a local cache up to the TTL limit, and then resolve via a server. If we cannot
// reach a server, but have a cached value we extend the TTL to gracefully handle outages.
func (c *Client) resolveTokenValue(secretID string) (*structs.ACLToken, error) {
func (c *Client) resolveTokenValue(secretID sensitive.Sensitive) (*structs.ACLToken, error) {
// Hot-path the anonymous token
if secretID == "" {
return structs.AnonymousACLToken, nil
Expand Down Expand Up @@ -158,7 +159,7 @@ func (c *Client) resolveTokenValue(secretID string) (*structs.ACLToken, error) {
// We cache the policies locally, and fault them from a server as necessary. Policies
// are cached for a TTL, and then refreshed. If a server cannot be reached, the cache TTL
// will be ignored to gracefully handle outages.
func (c *Client) resolvePolicies(secretID string, policies []string) ([]*structs.ACLPolicy, error) {
func (c *Client) resolvePolicies(secretID sensitive.Sensitive, policies []string) ([]*structs.ACLPolicy, error) {
var out []*structs.ACLPolicy
var expired []*structs.ACLPolicy
var missing []string
Expand Down
3 changes: 2 additions & 1 deletion client/acl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/hashicorp/nomad/acl"
"github.com/hashicorp/nomad/client/config"
"github.com/hashicorp/nomad/helper/sensitive"
"github.com/hashicorp/nomad/helper/uuid"
"github.com/hashicorp/nomad/nomad/mock"
"github.com/hashicorp/nomad/nomad/structs"
Expand Down Expand Up @@ -161,7 +162,7 @@ func TestClient_ACL_ResolveToken(t *testing.T) {
}

// Test bad token
out4, err := c1.ResolveToken(uuid.Generate())
out4, err := c1.ResolveToken(sensitive.Sensitive(uuid.Generate()))
assert.Equal(t, structs.ErrTokenNotFound, err)
assert.Nil(t, out4)
}
9 changes: 5 additions & 4 deletions client/alloc_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/hashicorp/nomad/client/config"
cstructs "github.com/hashicorp/nomad/client/structs"
"github.com/hashicorp/nomad/helper/pluginutils/catalog"
"github.com/hashicorp/nomad/helper/sensitive"
"github.com/hashicorp/nomad/helper/uuid"
"github.com/hashicorp/nomad/nomad"
"github.com/hashicorp/nomad/nomad/mock"
Expand Down Expand Up @@ -764,7 +765,7 @@ func TestAlloc_ExecStreaming_ACL_Basic(t *testing.T) {

cases := []struct {
Name string
Token string
Token sensitive.Sensitive
ExpectedError string
}{
{
Expand Down Expand Up @@ -897,7 +898,7 @@ func TestAlloc_ExecStreaming_ACL_WithIsolation_Image(t *testing.T) {

cases := []struct {
Name string
Token string
Token sensitive.Sensitive
ExpectedError string
}{
{
Expand Down Expand Up @@ -1046,7 +1047,7 @@ func TestAlloc_ExecStreaming_ACL_WithIsolation_Chroot(t *testing.T) {

cases := []struct {
Name string
Token string
Token sensitive.Sensitive
ExpectedError string
}{
{
Expand Down Expand Up @@ -1190,7 +1191,7 @@ func TestAlloc_ExecStreaming_ACL_WithIsolation_None(t *testing.T) {

cases := []struct {
Name string
Token string
Token sensitive.Sensitive
ExpectedError string
}{
{
Expand Down
7 changes: 4 additions & 3 deletions client/allocrunner/taskrunner/template/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,8 @@ func newRunnerConfig(config *TaskTemplateManagerConfig,
// Setup the Consul config
if cc.ConsulConfig != nil {
conf.Consul.Address = &cc.ConsulConfig.Addr
conf.Consul.Token = &cc.ConsulConfig.Token
token := cc.ConsulConfig.Token.Plaintext()
conf.Consul.Token = &token

if cc.ConsulConfig.EnableSSL != nil && *cc.ConsulConfig.EnableSSL {
verify := cc.ConsulConfig.VerifySSL != nil && *cc.ConsulConfig.VerifySSL
Expand All @@ -644,8 +645,8 @@ func newRunnerConfig(config *TaskTemplateManagerConfig,
}
}

if cc.ConsulConfig.Auth != "" {
parts := strings.SplitN(cc.ConsulConfig.Auth, ":", 2)
if auth := cc.ConsulConfig.Auth.Plaintext(); auth != "" {
parts := strings.SplitN(auth, ":", 2)
if len(parts) != 2 {
return nil, fmt.Errorf("Failed to parse Consul Auth config")
}
Expand Down
7 changes: 4 additions & 3 deletions client/allocwatcher/alloc_watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/hashicorp/nomad/client/allocdir"
"github.com/hashicorp/nomad/client/config"
cstructs "github.com/hashicorp/nomad/client/structs"
"github.com/hashicorp/nomad/helper/sensitive"
"github.com/hashicorp/nomad/nomad/structs"
)

Expand Down Expand Up @@ -89,7 +90,7 @@ type Config struct {

// MigrateToken is used to migrate remote alloc dirs when ACLs are
// enabled.
MigrateToken string
MigrateToken sensitive.Sensitive

Logger hclog.Logger
}
Expand Down Expand Up @@ -338,7 +339,7 @@ type remotePrevAlloc struct {

// migrateToken allows a client to migrate data in an ACL-protected remote
// volume
migrateToken string
migrateToken sensitive.Sensitive
}

// IsWaiting returns true if there's a concurrent call inside Wait
Expand Down Expand Up @@ -529,7 +530,7 @@ func (p *remotePrevAlloc) migrateAllocDir(ctx context.Context, nodeAddr string)
}

url := fmt.Sprintf("/v1/client/allocation/%v/snapshot", p.prevAllocID)
qo := &nomadapi.QueryOptions{AuthToken: p.migrateToken}
qo := &nomadapi.QueryOptions{AuthToken: p.migrateToken.Plaintext()}
resp, err := apiClient.Raw().Response(url, qo)
if err != nil {
prevAllocDir.Destroy()
Expand Down
17 changes: 9 additions & 8 deletions client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (
"github.com/hashicorp/nomad/command/agent/consul"
"github.com/hashicorp/nomad/helper"
"github.com/hashicorp/nomad/helper/pool"
"github.com/hashicorp/nomad/helper/sensitive"
hstats "github.com/hashicorp/nomad/helper/stats"
"github.com/hashicorp/nomad/helper/tlsutil"
"github.com/hashicorp/nomad/helper/uuid"
Expand Down Expand Up @@ -646,7 +647,7 @@ func (c *Client) NodeID() string {
}

// secretNodeID returns the secret node ID for the given client
func (c *Client) secretNodeID() string {
func (c *Client) secretNodeID() sensitive.Sensitive {
return c.config.Node.SecretID
}

Expand Down Expand Up @@ -881,7 +882,7 @@ func (c *Client) computeAllocatedDeviceGroupStats(devices []*structs.AllocatedDe
// ValidateMigrateToken verifies that a token is for a specific client and
// allocation, and has been created by a trusted party that has privileged
// knowledge of the client's secret identifier
func (c *Client) ValidateMigrateToken(allocID, migrateToken string) bool {
func (c *Client) ValidateMigrateToken(allocID string, migrateToken sensitive.Sensitive) bool {
if !c.config.ACLEnabled {
return true
}
Expand Down Expand Up @@ -1185,7 +1186,7 @@ func (c *Client) NumAllocs() int {
// nodeID restores, or generates if necessary, a unique node ID and SecretID.
// The node ID is, if available, a persistent unique ID. The secret ID is a
// high-entropy random UUID.
func (c *Client) nodeID() (id, secret string, err error) {
func (c *Client) nodeID() (id string, secret sensitive.Sensitive, err error) {
var hostID string
hostInfo, err := host.Info()
if !c.config.NoHostUUID && err == nil {
Expand All @@ -1202,7 +1203,7 @@ func (c *Client) nodeID() (id, secret string, err error) {

// Do not persist in dev mode
if c.config.DevMode {
return hostID, uuid.Generate(), nil
return hostID, sensitive.Sensitive(uuid.Generate()), nil
}

// Attempt to read existing ID
Expand Down Expand Up @@ -1232,10 +1233,10 @@ func (c *Client) nodeID() (id, secret string, err error) {
}

if len(secretBuf) != 0 {
secret = string(secretBuf)
secret = sensitive.Sensitive(string(secretBuf))
} else {
// Generate new ID
secret = uuid.Generate()
secret = sensitive.Sensitive(uuid.Generate())

// Persist the ID
if err := ioutil.WriteFile(secretPath, []byte(secret), 0700); err != nil {
Expand Down Expand Up @@ -1866,7 +1867,7 @@ type allocUpdates struct {

// migrateTokens are a list of tokens necessary for when clients pull data
// from authorized volumes
migrateTokens map[string]string
migrateTokens map[string]sensitive.Sensitive
}

// watchAllocations is used to scan for updates to allocations
Expand Down Expand Up @@ -2243,7 +2244,7 @@ func (c *Client) updateAlloc(update *structs.Allocation) {
}

// addAlloc is invoked when we should add an allocation
func (c *Client) addAlloc(alloc *structs.Allocation, migrateToken string) error {
func (c *Client) addAlloc(alloc *structs.Allocation, migrateToken sensitive.Sensitive) error {
c.allocLock.Lock()
defer c.allocLock.Unlock()

Expand Down
3 changes: 2 additions & 1 deletion client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/hashicorp/nomad/client/state"
"github.com/hashicorp/nomad/command/agent/consul"
"github.com/hashicorp/nomad/helper/pluginutils/catalog"
"github.com/hashicorp/nomad/helper/sensitive"
"github.com/hashicorp/nomad/helper/testlog"
"github.com/hashicorp/nomad/helper/uuid"
"github.com/hashicorp/nomad/nomad"
Expand Down Expand Up @@ -970,7 +971,7 @@ func TestClient_ValidateMigrateToken_InvalidToken(t *testing.T) {
assert.Equal(c.ValidateMigrateToken("", ""), false)

alloc := mock.Alloc()
assert.Equal(c.ValidateMigrateToken(alloc.ID, alloc.ID), false)
assert.Equal(c.ValidateMigrateToken(alloc.ID, sensitive.Sensitive(alloc.ID)), false)
assert.Equal(c.ValidateMigrateToken(alloc.ID, ""), false)
}

Expand Down
9 changes: 5 additions & 4 deletions client/fs_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/hashicorp/nomad/client/config"
sframer "github.com/hashicorp/nomad/client/lib/streamframer"
cstructs "github.com/hashicorp/nomad/client/structs"
"github.com/hashicorp/nomad/helper/sensitive"
"github.com/hashicorp/nomad/helper/testlog"
"github.com/hashicorp/nomad/helper/uuid"
"github.com/hashicorp/nomad/nomad"
Expand Down Expand Up @@ -145,7 +146,7 @@ func TestFS_Stat_ACL(t *testing.T) {

cases := []struct {
Name string
Token string
Token sensitive.Sensitive
ExpectedError string
}{
{
Expand Down Expand Up @@ -278,7 +279,7 @@ func TestFS_List_ACL(t *testing.T) {

cases := []struct {
Name string
Token string
Token sensitive.Sensitive
ExpectedError string
}{
{
Expand Down Expand Up @@ -428,7 +429,7 @@ func TestFS_Stream_ACL(t *testing.T) {

cases := []struct {
Name string
Token string
Token sensitive.Sensitive
ExpectedError string
}{
{
Expand Down Expand Up @@ -1057,7 +1058,7 @@ func TestFS_Logs_ACL(t *testing.T) {

cases := []struct {
Name string
Token string
Token sensitive.Sensitive
ExpectedError string
}{
{
Expand Down
6 changes: 3 additions & 3 deletions client/vaultclient/vaultclient_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func TestVaultClient_TokenRenewals(t *testing.T) {
num := 5
tokens := make([]string, num)
for i := 0; i < num; i++ {
c.client.SetToken(v.Config.Token)
c.client.SetToken(v.Config.Token.Plaintext())

if err := c.client.SetAddress(v.Config.Addr); err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -251,7 +251,7 @@ func TestVaultClient_RenewNonRenewableLease(t *testing.T) {
Renewable: new(bool),
}

c.client.SetToken(v.Config.Token)
c.client.SetToken(v.Config.Token.Plaintext())

if err := c.client.SetAddress(v.Config.Addr); err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -293,7 +293,7 @@ func TestVaultClient_RenewNonexistentLease(t *testing.T) {
// Sleep a little while to ensure that the renewal loop is active
time.Sleep(time.Duration(testutil.TestMultiplier()) * time.Second)

c.client.SetToken(v.Config.Token)
c.client.SetToken(v.Config.Token.Plaintext())

if err := c.client.SetAddress(v.Config.Addr); err != nil {
t.Fatal(err)
Expand Down
2 changes: 1 addition & 1 deletion command/acl_policy_apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func TestACLPolicyApplyCommand(t *testing.T) {
assert.Equal(1, code)

// Apply a policy with a valid management token
os.Setenv("NOMAD_TOKEN", token.SecretID)
os.Setenv("NOMAD_TOKEN", token.SecretID.Plaintext())
code = cmd.Run([]string{"-address=" + url, "test-policy", f.Name()})
assert.Equal(0, code)

Expand Down
4 changes: 2 additions & 2 deletions command/acl_policy_delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,12 @@ func TestACLPolicyDeleteCommand(t *testing.T) {

// Delete the policy without a valid token fails
invalidToken := mock.ACLToken()
os.Setenv("NOMAD_TOKEN", invalidToken.SecretID)
os.Setenv("NOMAD_TOKEN", invalidToken.SecretID.Plaintext())
code := cmd.Run([]string{"-address=" + url, policy.Name})
assert.Equal(1, code)

// Delete the policy with a valid management token
os.Setenv("NOMAD_TOKEN", token.SecretID)
os.Setenv("NOMAD_TOKEN", token.SecretID.Plaintext())
code = cmd.Run([]string{"-address=" + url, policy.Name})
assert.Equal(0, code)

Expand Down
4 changes: 2 additions & 2 deletions command/acl_policy_info_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ func TestACLPolicyInfoCommand(t *testing.T) {

// Attempt to apply a policy without a valid management token
invalidToken := mock.ACLToken()
os.Setenv("NOMAD_TOKEN", invalidToken.SecretID)
os.Setenv("NOMAD_TOKEN", invalidToken.SecretID.Plaintext())
code := cmd.Run([]string{"-address=" + url, policy.Name})
assert.Equal(1, code)

// Apply a policy with a valid management token
os.Setenv("NOMAD_TOKEN", token.SecretID)
os.Setenv("NOMAD_TOKEN", token.SecretID.Plaintext())
code = cmd.Run([]string{"-address=" + url, policy.Name})
assert.Equal(0, code)

Expand Down
6 changes: 3 additions & 3 deletions command/acl_policy_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@ func TestACLPolicyListCommand(t *testing.T) {

// Attempt to list policies without a valid management token
invalidToken := mock.ACLToken()
code := cmd.Run([]string{"-address=" + url, "-token=" + invalidToken.SecretID})
code := cmd.Run([]string{"-address=" + url, "-token=" + invalidToken.SecretID.Plaintext()})
assert.Equal(1, code)

// Apply a policy with a valid management token
code = cmd.Run([]string{"-address=" + url, "-token=" + token.SecretID})
code = cmd.Run([]string{"-address=" + url, "-token=" + token.SecretID.Plaintext()})
assert.Equal(0, code)

// Check the output
Expand All @@ -54,7 +54,7 @@ func TestACLPolicyListCommand(t *testing.T) {
}

// List json
if code := cmd.Run([]string{"-address=" + url, "-token=" + token.SecretID, "-json"}); code != 0 {
if code := cmd.Run([]string{"-address=" + url, "-token=" + token.SecretID.Plaintext(), "-json"}); code != 0 {
t.Fatalf("expected exit 0, got: %d; %v", code, ui.ErrorWriter.String())
}
out = ui.OutputWriter.String()
Expand Down
2 changes: 1 addition & 1 deletion command/acl_token_create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func TestACLTokenCreateCommand(t *testing.T) {
assert.Equal(1, code)

// Request to create a new token with a valid management token
os.Setenv("NOMAD_TOKEN", token.SecretID)
os.Setenv("NOMAD_TOKEN", token.SecretID.Plaintext())
code = cmd.Run([]string{"-address=" + url, "-policy=foo", "-type=client"})
assert.Equal(0, code)

Expand Down
Loading