From 586b4cc5abb0c8554fcf8db95f1761b323756f27 Mon Sep 17 00:00:00 2001 From: Aaron Boodman Date: Sat, 21 Sep 2013 01:54:07 -0700 Subject: [PATCH] Check IsTransitive in share handler and add tests. Also fix a bug in interpretation of via chain, which caused sharing to be overly permissive. BUG=https://code.google.com/p/camlistore/issues/detail?id=226 Change-Id: I4459a67d1b40e4a50f111ce708dbfcbd975f3f15 --- pkg/schema/blob.go | 11 ++++ pkg/server/share.go | 18 ++++--- pkg/server/share_test.go | 111 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 132 insertions(+), 8 deletions(-) create mode 100644 pkg/server/share_test.go diff --git a/pkg/schema/blob.go b/pkg/schema/blob.go index 8e87a63e0..6a00a381f 100644 --- a/pkg/schema/blob.go +++ b/pkg/schema/blob.go @@ -264,6 +264,17 @@ func (bb *Builder) SetShareExpiration(t time.Time) { } } +func (bb *Builder) SetShareIsTransitive(b bool) { + if bb.Type() != "claim" || bb.m["claimType"] != "share" { + panic("called SetShareIsTransitive on non-share") + } + if !b { + delete(bb.m, "transitive") + } else { + bb.m["transitive"] = true + } +} + // SetRawStringField sets a raw string field in the underlying map. func (bb *Builder) SetRawStringField(key, value string) *Builder { bb.m[key] = value diff --git a/pkg/server/share.go b/pkg/server/share.go index 2867b9e65..7fca9e0c6 100644 --- a/pkg/server/share.go +++ b/pkg/server/share.go @@ -41,8 +41,6 @@ const fetchFailureDelay = 200 * time.Millisecond // ShareHandler handles the requests for "share" (and shared) blobs. type shareHandler struct { - blobRoot string - fetcher blob.StreamingFetcher } @@ -59,12 +57,10 @@ func newShareFromConfig(ld blobserver.Loader, conf jsonconfig.Obj) (h http.Handl return } - share := &shareHandler{ - blobRoot: blobRoot, - } - bs, err := ld.GetStorage(share.blobRoot) + share := &shareHandler{} + bs, err := ld.GetStorage(blobRoot) if err != nil { - return nil, fmt.Errorf("Share handler's blobRoot of %q error: %v", share.blobRoot, err) + return nil, fmt.Errorf("Share handler's blobRoot of %q error: %v", blobRoot, err) } fetcher, ok := bs.(blob.StreamingFetcher) if !ok { @@ -137,6 +133,7 @@ func handleGetViaSharing(conn http.ResponseWriter, req *http.Request, return } if share.IsExpired() { + log.Print("Share is expired") auth.SendUnauthorized(conn, req) return } @@ -146,6 +143,11 @@ func handleGetViaSharing(conn http.ResponseWriter, req *http.Request, auth.SendUnauthorized(conn, req) return } + if len(fetchChain) > 2 && !share.IsTransitive() { + log.Print("Share is not transitive") + auth.SendUnauthorized(conn, req) + return + } case len(fetchChain) - 1: // Last one is fine (as long as its path up to here has been proven, and it's // not the first thing in the chain) @@ -166,7 +168,7 @@ func handleGetViaSharing(conn http.ResponseWriter, req *http.Request, return } saught := fetchChain[i+1].String() - if bytes.IndexAny(slurpBytes, saught) == -1 { + if bytes.Index(slurpBytes, []byte(saught)) == -1 { log.Printf("Fetch chain %d of %s failed; no reference to %s", i, br.String(), saught) auth.SendUnauthorized(conn, req) diff --git a/pkg/server/share_test.go b/pkg/server/share_test.go new file mode 100644 index 000000000..8cbb0a3ef --- /dev/null +++ b/pkg/server/share_test.go @@ -0,0 +1,111 @@ +/* +Copyright 2013 Google Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package server + +import ( + "log" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "camlistore.org/pkg/blob" + "camlistore.org/pkg/httputil" + "camlistore.org/pkg/schema" + "camlistore.org/pkg/test" + . "camlistore.org/pkg/test/asserts" +) + +func TestHandleGetViaSharing(t *testing.T) { + // TODO(aa): It would be good if we could test that we are failing for + // the right reason for all of these (some kind of internal error code). + + sto := &test.Fetcher{} + handler := &httputil.PrefixHandler{"/", &shareHandler{sto}} + wr := &httptest.ResponseRecorder{} + + get := func(path string) *httptest.ResponseRecorder { + wr = httptest.NewRecorder() + req, _ := http.NewRequest("GET", "http://unused/"+path, nil) + handler.ServeHTTP(wr, req) + return wr + } + + content := "monkey" + contentRef := blob.SHA1FromString(content) + + // For the purposes of following the via chain, the only thing that + // matters is that the content of each link contains the name of the + // next link. + link := contentRef.String() + linkRef := blob.SHA1FromString(link) + + share := schema.NewShareRef(schema.ShareHaveRef, linkRef, false). + SetSigner(blob.SHA1FromString("irrelevant")). + SetRawStringField("camliSig", "alsounused") + + log.Print("Should fail because first link does not exist") + get(share.Blob().BlobRef().String()) + ExpectInt(t, 401, wr.Code, "") + + log.Print("Should fail because share target does not match next link") + sto.ReceiveBlob(share.Blob().BlobRef(), strings.NewReader(share.Blob().JSON())) + get(contentRef.String() + "?via=" + share.Blob().BlobRef().String()) + ExpectInt(t, 401, wr.Code, "") + + log.Print("Should fail because first link is not a share") + sto.ReceiveBlob(linkRef, strings.NewReader(link)) + get(linkRef.String()) + ExpectInt(t, 401, wr.Code, "") + log.Print("Should successfully fetch share") + get(share.Blob().BlobRef().String()) + ExpectInt(t, 200, wr.Code, "") + + log.Print("Should successfully fetch link via share") + get(linkRef.String() + "?via=" + share.Blob().BlobRef().String()) + ExpectInt(t, 200, wr.Code, "") + + log.Print("Should fail because share is not transitive") + get(contentRef.String() + "?via=" + share.Blob().BlobRef().String() + "," + linkRef.String()) + ExpectInt(t, 401, wr.Code, "") + + log.Print("Should fail because link content does not contain target") + share.SetShareIsTransitive(true) + sto.ReceiveBlob(share.Blob().BlobRef(), strings.NewReader(share.Blob().JSON())) + get(linkRef.String() + "?via=" + share.Blob().BlobRef().String() + "," + linkRef.String()) + ExpectInt(t, 401, wr.Code, "") + + log.Print("Should successfully fetch content via link via share") + sto.ReceiveBlob(contentRef, strings.NewReader(content)) + get(contentRef.String() + "?via=" + share.Blob().BlobRef().String() + "," + linkRef.String()) + ExpectInt(t, 200, wr.Code, "") + + log.Print("Should fail because share is expired") + share.SetShareExpiration(time.Now().Add(-time.Duration(10) * time.Minute)) + sto.ReceiveBlob(share.Blob().BlobRef(), strings.NewReader(share.Blob().JSON())) + get(contentRef.String() + "?via=" + share.Blob().BlobRef().String() + "," + linkRef.String()) + ExpectInt(t, 401, wr.Code, "") + + log.Print("Should succeed because share has not expired") + share.SetShareExpiration(time.Now().Add(time.Duration(10) * time.Minute)) + sto.ReceiveBlob(share.Blob().BlobRef(), strings.NewReader(share.Blob().JSON())) + get(contentRef.String() + "?via=" + share.Blob().BlobRef().String() + "," + linkRef.String()) + ExpectInt(t, 200, wr.Code, "") + + // TODO(aa): assemble +}