From 0d535aea95d2b5f0df5ad4296e6e21ffd45ee552 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Fri, 13 Oct 2017 10:59:13 -0700 Subject: [PATCH] base64 migrate token HTTP header values must be ASCII. Also constant time compare tokens and test the generate and compare helper functions. --- client/client.go | 9 +-------- nomad/eval_endpoint_test.go | 23 +++++++++++++++++++++++ nomad/node_endpoint.go | 26 ++++++++++++++++++++++---- 3 files changed, 46 insertions(+), 12 deletions(-) diff --git a/client/client.go b/client/client.go index 27f1f2c4e..796d6e6ed 100644 --- a/client/client.go +++ b/client/client.go @@ -33,7 +33,6 @@ import ( vaultapi "github.com/hashicorp/vault/api" "github.com/mitchellh/hashstructure" "github.com/shirou/gopsutil/host" - "golang.org/x/crypto/blake2b" ) const ( @@ -534,13 +533,7 @@ func (c *Client) ValidateMigrateToken(allocID, migrateToken string) bool { return true } - h, err := blake2b.New512([]byte(c.secretNodeID())) - if err != nil { - return false - } - h.Write([]byte(allocID)) - expectedMigrateToken := string(h.Sum(nil)) - return expectedMigrateToken == migrateToken + return nomad.CompareMigrateToken(allocID, c.secretNodeID(), migrateToken) } // GetAllocFS returns the AllocFS interface for the alloc dir of an allocation diff --git a/nomad/eval_endpoint_test.go b/nomad/eval_endpoint_test.go index f46f4e1d9..6955182c5 100644 --- a/nomad/eval_endpoint_test.go +++ b/nomad/eval_endpoint_test.go @@ -1,6 +1,7 @@ package nomad import ( + "encoding/base64" "fmt" "reflect" "strings" @@ -1008,3 +1009,25 @@ func TestEvalEndpoint_Reblock(t *testing.T) { t.Fatalf("ReblockEval didn't insert eval into the blocked eval tracker") } } + +// TestGenerateMigrateToken asserts the migrate token is valid for use in HTTP +// headers and CompareMigrateToken works as expected. +func TestGenerateMigrateToken(t *testing.T) { + assert := assert.New(t) + allocID := uuid.Generate() + nodeSecret := uuid.Generate() + token, err := GenerateMigrateToken(allocID, nodeSecret) + assert.Nil(err) + _, err = base64.URLEncoding.DecodeString(token) + assert.Nil(err) + + assert.True(CompareMigrateToken(allocID, nodeSecret, token)) + assert.False(CompareMigrateToken("x", nodeSecret, token)) + assert.False(CompareMigrateToken(allocID, "x", token)) + assert.False(CompareMigrateToken(allocID, nodeSecret, "x")) + + token2, err := GenerateMigrateToken("x", nodeSecret) + assert.Nil(err) + assert.False(CompareMigrateToken(allocID, nodeSecret, token2)) + assert.True(CompareMigrateToken("x", nodeSecret, token2)) +} diff --git a/nomad/node_endpoint.go b/nomad/node_endpoint.go index 8ad951045..6500c0def 100644 --- a/nomad/node_endpoint.go +++ b/nomad/node_endpoint.go @@ -2,6 +2,8 @@ package nomad import ( "context" + "crypto/subtle" + "encoding/base64" "fmt" "strings" "sync" @@ -659,15 +661,31 @@ func (n *Node) GetAllocs(args *structs.NodeSpecificRequest, return n.srv.blockingRPC(&opts) } -// generateMigrateToken will create a token for a client to access an +// GenerateMigrateToken will create a token for a client to access an // authenticated volume of another client to migrate data for sticky volumes. -func generateMigrateToken(allocID, nodeSecretID string) (string, error) { +func GenerateMigrateToken(allocID, nodeSecretID string) (string, error) { h, err := blake2b.New512([]byte(nodeSecretID)) if err != nil { return "", err } h.Write([]byte(allocID)) - return string(h.Sum(nil)), nil + return base64.URLEncoding.EncodeToString(h.Sum(nil)), nil +} + +// CompareMigrateToken returns true if two migration tokens can be computed and +// are equal. +func CompareMigrateToken(allocID, nodeSecretID, otherMigrateToken string) bool { + h, err := blake2b.New512([]byte(nodeSecretID)) + if err != nil { + return false + } + h.Write([]byte(allocID)) + + otherBytes, err := base64.URLEncoding.DecodeString(otherMigrateToken) + if err != nil { + return false + } + return subtle.ConstantTimeCompare(h.Sum(nil), otherBytes) == 1 } // GetClientAllocs is used to request a lightweight list of alloc modify indexes @@ -743,7 +761,7 @@ func (n *Node) GetClientAllocs(args *structs.NodeSpecificRequest, continue } - token, err := generateMigrateToken(prevAllocation.ID, allocNode.SecretID) + token, err := GenerateMigrateToken(prevAllocation.ID, allocNode.SecretID) if err != nil { return err }