From ea6c082e85a4f218bc872ad090188ee516e2e382 Mon Sep 17 00:00:00 2001 From: Brian Bland Date: Mon, 5 Jan 2015 14:25:28 -0800 Subject: [PATCH] Minor cleanup/testing for HMAC upload tokens Changes configuration variable, lowercases private interface methods, adds token sanity tests. --- app.go | 2 +- configuration/configuration.go | 5 +- layerupload.go | 4 +- storage/layerupload.go | 2 + tokens.go | 16 ++--- tokens_test.go | 121 +++++++++++++++++++++++++++++++++ 6 files changed, 135 insertions(+), 15 deletions(-) create mode 100644 tokens_test.go diff --git a/app.go b/app.go index 1790b3ae..e1ffd8d5 100644 --- a/app.go +++ b/app.go @@ -64,7 +64,7 @@ func NewApp(configuration configuration.Configuration) *App { app.driver = driver app.services = storage.NewServices(app.driver) - app.tokenProvider = newHMACTokenProvider(configuration.Cluster.Secret) + app.tokenProvider = newHMACTokenProvider(configuration.HTTP.Secret) authType := configuration.Auth.Type() diff --git a/configuration/configuration.go b/configuration/configuration.go index 9d991dfa..1a37c2a7 100644 --- a/configuration/configuration.go +++ b/configuration/configuration.go @@ -32,13 +32,10 @@ type Configuration struct { HTTP struct { // Addr specifies the bind address for the registry instance. Addr string `yaml:"addr"` - } `yaml:"http"` - // Cluster contains configuration parameters for clustering the registry. - Cluster struct { // Secret specifies the secret key which HMAC tokens are created with. Secret string `yaml:"secret"` - } `yaml:"cluster"` + } `yaml:"http"` } // v0_1Configuration is a Version 0.1 Configuration struct diff --git a/layerupload.go b/layerupload.go index 865bf12c..b694a677 100644 --- a/layerupload.go +++ b/layerupload.go @@ -33,7 +33,7 @@ func layerUploadDispatcher(ctx *Context, r *http.Request) http.Handler { if luh.UUID != "" { luh.log = luh.log.WithField("uuid", luh.UUID) - state, err := ctx.tokenProvider.LayerUploadStateFromToken(r.FormValue("_state")) + state, err := ctx.tokenProvider.layerUploadStateFromToken(r.FormValue("_state")) if err != nil { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { logrus.Infof("error resolving upload: %v", err) @@ -172,7 +172,7 @@ func (luh *layerUploadHandler) CancelLayerUpload(w http.ResponseWriter, r *http. // left to the caller. func (luh *layerUploadHandler) layerUploadResponse(w http.ResponseWriter, r *http.Request) error { values := make(url.Values) - stateToken, err := luh.Context.tokenProvider.LayerUploadStateToToken(storage.LayerUploadState{Name: luh.Upload.Name(), UUID: luh.Upload.UUID(), Offset: luh.Upload.Offset()}) + stateToken, err := luh.Context.tokenProvider.layerUploadStateToToken(storage.LayerUploadState{Name: luh.Upload.Name(), UUID: luh.Upload.UUID(), Offset: luh.Upload.Offset()}) if err != nil { logrus.Infof("error building upload state token: %s", err) return err diff --git a/storage/layerupload.go b/storage/layerupload.go index 47c12501..3175a09e 100644 --- a/storage/layerupload.go +++ b/storage/layerupload.go @@ -59,6 +59,8 @@ type layerUploadStore interface { New(name string) (LayerUploadState, error) Open(uuid string) (layerFile, error) GetState(uuid string) (LayerUploadState, error) + // TODO: factor this method back in + // SaveState(lus LayerUploadState) error DeleteState(uuid string) error } diff --git a/tokens.go b/tokens.go index 46caf81a..276b896e 100644 --- a/tokens.go +++ b/tokens.go @@ -12,11 +12,11 @@ import ( // tokenProvider contains methods for serializing and deserializing state from token strings. type tokenProvider interface { - // LayerUploadStateFromToken retrieves the LayerUploadState for a given state token. - LayerUploadStateFromToken(stateToken string) (storage.LayerUploadState, error) + // layerUploadStateFromToken retrieves the LayerUploadState for a given state token. + layerUploadStateFromToken(stateToken string) (storage.LayerUploadState, error) - // LayerUploadStateToToken returns a token string representing the given LayerUploadState. - LayerUploadStateToToken(layerUploadState storage.LayerUploadState) (string, error) + // layerUploadStateToToken returns a token string representing the given LayerUploadState. + layerUploadStateToToken(layerUploadState storage.LayerUploadState) (string, error) } type hmacTokenProvider struct { @@ -27,8 +27,8 @@ func newHMACTokenProvider(secret string) tokenProvider { return &hmacTokenProvider{secret: secret} } -// LayerUploadStateFromToken deserializes the given HMAC stateToken and validates the prefix HMAC -func (ts *hmacTokenProvider) LayerUploadStateFromToken(stateToken string) (storage.LayerUploadState, error) { +// layerUploadStateFromToken deserializes the given HMAC stateToken and validates the prefix HMAC +func (ts *hmacTokenProvider) layerUploadStateFromToken(stateToken string) (storage.LayerUploadState, error) { var lus storage.LayerUploadState tokenBytes, err := base64.URLEncoding.DecodeString(stateToken) @@ -56,8 +56,8 @@ func (ts *hmacTokenProvider) LayerUploadStateFromToken(stateToken string) (stora return lus, nil } -// LayerUploadStateToToken serializes the given LayerUploadState to JSON with an HMAC prepended -func (ts *hmacTokenProvider) LayerUploadStateToToken(lus storage.LayerUploadState) (string, error) { +// layerUploadStateToToken serializes the given LayerUploadState to JSON with an HMAC prepended +func (ts *hmacTokenProvider) layerUploadStateToToken(lus storage.LayerUploadState) (string, error) { mac := hmac.New(sha256.New, []byte(ts.secret)) stateJSON := fmt.Sprintf("{\"Name\": \"%s\", \"UUID\": \"%s\", \"Offset\": %d}", lus.Name, lus.UUID, lus.Offset) mac.Write([]byte(stateJSON)) diff --git a/tokens_test.go b/tokens_test.go new file mode 100644 index 00000000..a447438a --- /dev/null +++ b/tokens_test.go @@ -0,0 +1,121 @@ +package registry + +import ( + "testing" + + "github.com/docker/distribution/storage" +) + +var layerUploadStates = []storage.LayerUploadState{ + { + Name: "hello", + UUID: "abcd-1234-qwer-0987", + Offset: 0, + }, + { + Name: "hello-world", + UUID: "abcd-1234-qwer-0987", + Offset: 0, + }, + { + Name: "h3ll0_w0rld", + UUID: "abcd-1234-qwer-0987", + Offset: 1337, + }, + { + Name: "ABCDEFG", + UUID: "ABCD-1234-QWER-0987", + Offset: 1234567890, + }, + { + Name: "this-is-A-sort-of-Long-name-for-Testing", + UUID: "dead-1234-beef-0987", + Offset: 8675309, + }, +} + +var secrets = []string{ + "supersecret", + "12345", + "a", + "SuperSecret", + "Sup3r... S3cr3t!", + "This is a reasonably long secret key that is used for the purpose of testing.", + "\u2603+\u2744", // snowman+snowflake +} + +// TestLayerUploadTokens constructs stateTokens from LayerUploadStates and +// validates that the tokens can be used to reconstruct the proper upload state. +func TestLayerUploadTokens(t *testing.T) { + tokenProvider := newHMACTokenProvider("supersecret") + + for _, testcase := range layerUploadStates { + token, err := tokenProvider.layerUploadStateToToken(testcase) + if err != nil { + t.Fatal(err) + } + + lus, err := tokenProvider.layerUploadStateFromToken(token) + if err != nil { + t.Fatal(err) + } + + assertLayerUploadStateEquals(t, testcase, lus) + } +} + +// TestHMACValidate ensures that any HMAC token providers are compatible if and +// only if they share the same secret. +func TestHMACValidation(t *testing.T) { + for _, secret := range secrets { + tokenProvider1 := newHMACTokenProvider(secret) + tokenProvider2 := newHMACTokenProvider(secret) + badTokenProvider := newHMACTokenProvider("DifferentSecret") + + for _, testcase := range layerUploadStates { + token, err := tokenProvider1.layerUploadStateToToken(testcase) + if err != nil { + t.Fatal(err) + } + + lus, err := tokenProvider2.layerUploadStateFromToken(token) + if err != nil { + t.Fatal(err) + } + + assertLayerUploadStateEquals(t, testcase, lus) + + _, err = badTokenProvider.layerUploadStateFromToken(token) + if err == nil { + t.Fatalf("Expected token provider to fail at retrieving state from token: %s", token) + } + + badToken, err := badTokenProvider.layerUploadStateToToken(testcase) + if err != nil { + t.Fatal(err) + } + + _, err = tokenProvider1.layerUploadStateFromToken(badToken) + if err == nil { + t.Fatalf("Expected token provider to fail at retrieving state from token: %s", badToken) + } + + _, err = tokenProvider2.layerUploadStateFromToken(badToken) + if err == nil { + t.Fatalf("Expected token provider to fail at retrieving state from token: %s", badToken) + } + } + } +} + +func assertLayerUploadStateEquals(t *testing.T, expected storage.LayerUploadState, received storage.LayerUploadState) { + if expected.Name != received.Name { + t.Fatalf("Expected Name=%q, Received Name=%q", expected.Name, received.Name) + } + if expected.UUID != received.UUID { + t.Fatalf("Expected UUID=%q, Received UUID=%q", expected.UUID, received.UUID) + } + if expected.Offset != received.Offset { + t.Fatalf("Expected Offset=%d, Received Offset=%d", expected.Offset, received.Offset) + } +}