Use Skylink Hash (pt2)

README.md

@@ -22,23 +22,18 @@ database of hashes.
 
 # AllowList
 
-The blocker service can only block skylinks which are not in the allow list.
-To add a skylink to the allow list, one has to manually query the database and
+The blocker service can only block hashes which are not in the allow list.
+To add a hash to the allow list, one has to manually query the database and
 perform the follow operation:
 
 ```
 db.getCollection('allowlist').insertOne({
-  skylink: "[INSERT V1 SKYLINK HERE]",
-  description: "[INSERT SKYLINK DESCRIPTION]",
+  hash: "[INSERT HASH OF V1 SKYLINK HERE]",
+  description: "[INSERT DESCRIPTION]",
   timestamp_added: new Date(),
 })
 ```
 
-The skylink is expected to be in the following form: `_B19BtlWtjjR7AD0DDzxYanvIhZ7cxXrva5tNNxDht1kaA`.
-So that's without portal and without the `sia://` prefix. The allow list is
-persisted as is, so not as a hash, for ease of use and because it is assumed the
-allowlist only holds non-abusive content.
-
 # Environment
 
 This service depends on the following environment variables:

api/api_test.go

@@ -26,13 +26,13 @@ func newAPITester(api *API) *apiTester {
 
 // newTestAPI returns a new API instance
 func newTestAPI(dbName string, client *SkydClient) (*API, error) {
+	// create database
+	db := database.NewTestDB(context.Background(), dbName)
+
 	// create a nil logger
 	logger := logrus.New()
 	logger.Out = ioutil.Discard
 
-	// create database
-	db := database.NewTestDB(context.Background(), dbName, logger)
-
 	// create the API
 	api, err := New(client, db, logger)
 	if err != nil {

api/handlers.go

@@ -41,12 +41,27 @@ const (
 	sortDescending = "desc"
 )
 
+var (
+	// errResolve is the error returned when we failed to resolve a skylink,
+	// indicating skyd failure
+	errResolve = errors.New("failed to resolve skylink")
+)
+
 type (
 	// BlockPOST describes a request to the /block endpoint.
 	BlockPOST struct {
 		Skylink  skylink  `json:"skylink"`
 		Reporter Reporter `json:"reporter"`
 		Tags     []string `json:"tags"`
+
+		// Hash represents the hash of the Skylink's merkle root. Either 'hash'
+		// or 'skylink' must be set. If both are set the Skylink's hash value
+		// must correspond with the hash.
+		//
+		// It is encouraged to use this field when possible as it allows
+		// services that interact with the blocker to only deal with hashes
+		// instead of skylinks.
+		Hash crypto.Hash `json:"hash"`
 	}
 
 	// BlocklistGET returns a list of blocked hashes
@@ -128,13 +143,13 @@ func (api *API) blocklistGET(w http.ResponseWriter, r *http.Request, _ httproute
 	// parse offset and limit parameters
 	sort, offset, limit, err := parseListParameters(r.URL.Query())
 	if err != nil {
-		skyapi.WriteError(w, skyapi.Error{err.Error()}, http.StatusBadRequest)
+		WriteError(w, err, http.StatusBadRequest)
 		return
 	}
 
 	blocked, more, err := api.staticDB.BlockedHashes(r.Context(), sort, offset, limit)
 	if err != nil {
-		skyapi.WriteError(w, skyapi.Error{err.Error()}, http.StatusInternalServerError)
+		WriteError(w, err, http.StatusInternalServerError)
 		return
 	}
 
@@ -181,7 +196,7 @@ func (api *API) blockPOST(w http.ResponseWriter, r *http.Request, _ httprouter.P
 	var body BlockPOST
 	err := json.NewDecoder(b).Decode(&body)
 	if err != nil {
-		skyapi.WriteError(w, skyapi.Error{err.Error()}, http.StatusBadRequest)
+		WriteError(w, err, http.StatusBadRequest)
 		return
 	}
 
@@ -213,7 +228,7 @@ func (api *API) blockWithPoWPOST(w http.ResponseWriter, r *http.Request, _ httpr
 	var body BlockWithPoWPOST
 	err := json.NewDecoder(b).Decode(&body)
 	if err != nil {
-		skyapi.WriteError(w, skyapi.Error{err.Error()}, http.StatusBadRequest)
+		WriteError(w, err, http.StatusBadRequest)
 		return
 	}
 
@@ -223,7 +238,7 @@ func (api *API) blockWithPoWPOST(w http.ResponseWriter, r *http.Request, _ httpr
 	// Verify the pow.
 	err = body.PoW.Verify()
 	if err != nil {
-		skyapi.WriteError(w, skyapi.Error{err.Error()}, http.StatusBadRequest)
+		WriteError(w, err, http.StatusBadRequest)
 		return
 	}
 
@@ -242,86 +257,113 @@ func (api *API) blockWithPoWGET(w http.ResponseWriter, r *http.Request, _ httpro
 // block handlers. It executes all code which is shared between the two
 // handlers.
 func (api *API) handleBlockRequest(ctx context.Context, w http.ResponseWriter, bp BlockPOST, sub string) {
-	// Decode the skylink, we can safely ignore the error here as LoadString
-	// will have been called by the JSON decoder
-	var skylink skymodules.Skylink
-	_ = skylink.LoadString(string(bp.Skylink))
-
-	// Resolve the skylink
-	resolved, err := api.staticSkydClient.ResolveSkylink(skylink)
-	if err == nil {
-		// replace the skylink with the resolved skylink
-		skylink = resolved
-	} else {
-		// in case of an error we log and continue with the given skylink
-		api.staticLogger.Errorf("failed to resolve skylink '%v', err: %v", skylink, err)
-	}
-
-	// Sanity check the skylink is a v1 skylink
-	if !skylink.IsSkylinkV1() {
-		skyapi.WriteError(w, skyapi.Error{"failed to resolve skylink"}, http.StatusInternalServerError)
+	// Resolve the post body into a hash
+	hash, err := api.resolveHash(bp)
+	if err != nil {
+		// return an internal server error if the resolve failed due to skyd
+		// either being down or behaving unexpectedly
+		code := http.StatusBadRequest
+		if errors.Contains(err, errResolve) {
+			code = http.StatusInternalServerError
+		}
+		WriteError(w, errors.AddContext(err, "failed to resolve hash"), code)
 		return
 	}
 
 	// Check whether the skylink is on the allow list
-	if api.isAllowListed(ctx, skylink) {
+	if api.isAllowListed(ctx, hash) {
 		skyapi.WriteJSON(w, statusResponse{"reported"})
 		return
 	}
 
-	// Block the link.
-	err = api.block(ctx, bp, skylink, sub, sub == "")
-	if errors.Contains(err, database.ErrSkylinkExists) {
-		skyapi.WriteJSON(w, statusResponse{"duplicate"})
-		return
-	}
-	if err != nil {
-		skyapi.WriteError(w, skyapi.Error{err.Error()}, http.StatusInternalServerError)
-		return
-	}
-	skyapi.WriteJSON(w, statusResponse{"reported"})
-}
-
-// block blocks a skylink
-func (api *API) block(ctx context.Context, bp BlockPOST, skylink skymodules.Skylink, sub string, unauthenticated bool) error {
-	// TODO: currently we still set the Skylink, as soon as this module is
-	// converted to work fully with hashes, the Skylink field needs to be
-	// dropped.
+	// Create a blocked skylink object
 	bs := &database.BlockedSkylink{
-		Skylink: skylink.String(),
-		Hash:    database.NewHash(skylink),
+		Hash: database.Hash{Hash: hash},
 		Reporter: database.Reporter{
 			Name:            bp.Reporter.Name,
 			Email:           bp.Reporter.Email,
 			OtherContact:    bp.Reporter.OtherContact,
 			Sub:             sub,
-			Unauthenticated: unauthenticated,
+			Unauthenticated: sub == "",
 		},
 		Tags:           bp.Tags,
 		TimestampAdded: time.Now().UTC(),
 	}
+
+	// Block the link.
 	api.staticLogger.Debugf("blocking hash %s", bs.Hash)
-	err := api.staticDB.CreateBlockedSkylink(ctx, bs)
+	err = api.staticDB.CreateBlockedSkylink(ctx, bs)
+	if errors.Contains(err, database.ErrSkylinkExists) {
+		skyapi.WriteJSON(w, statusResponse{"duplicate"})
+		return
+	}
 	if err != nil {
-		return err
+		WriteError(w, err, http.StatusInternalServerError)
+		return
 	}
 	api.staticLogger.Debugf("blocked hash %s", bs.Hash)
-	return nil
+	skyapi.WriteJSON(w, statusResponse{"reported"})
 }
 
 // isAllowListed returns true if the given skylink is on the allow list
 //
 // NOTE: the given skylink is expected to be a v1 skylink, meaning the caller of
 // this function should have tried to resolve the skylink beforehand
-func (api *API) isAllowListed(ctx context.Context, skylink skymodules.Skylink) bool {
-	allowlisted, err := api.staticDB.IsAllowListed(ctx, skylink.String())
+func (api *API) isAllowListed(ctx context.Context, hash crypto.Hash) bool {
+	allowlisted, err := api.staticDB.IsAllowListed(ctx, hash)
 	if err != nil {
 		api.staticLogger.Error("failed to verify skylink against the allow list", err)
 		return false
 	}
 	return allowlisted
 }
 
+// resolveHash resolves the given block post object into a hash. If a hash was
+// already given, it will simply return that. If a skylink was given, it will
+// try to resolve it first if necessary and return the hash of the v1 skylink.
+func (api *API) resolveHash(bp BlockPOST) (crypto.Hash, error) {
+	// validate the block post
+	err := bp.validate()
+	if err != nil {
+		return crypto.Hash{}, err
+	}
+
+	// if the hash is set, we are done
+	if bp.Hash != (crypto.Hash{}) {
+		return bp.Hash, nil
+	}
+
+	// decode the skylink
+	var skylink skymodules.Skylink
+	err = skylink.LoadString(string(bp.Skylink))
+	if err != nil {
+		return crypto.Hash{}, errors.AddContext(err, "failed to load skylink")
+	}
+
+	// resolve the skylink
+	skylink, err = api.staticSkydClient.ResolveSkylink(skylink)
+	if err != nil {
+		return crypto.Hash{}, errors.Compose(err, errResolve)
+	}
+
+	// sanity check the skylink is a v1 skylink
+	if !skylink.IsSkylinkV1() {
+		return crypto.Hash{}, errors.Compose(err, errResolve)
+	}
+
+	// return the hash
+	return crypto.HashObject(skylink.MerkleRoot()), nil
+}
+
+// validate returns an error if the block post object does not contain a hash or
+// skylink
+func (bp *BlockPOST) validate() error {
+	if bp.Hash == (crypto.Hash{}) && bp.Skylink == "" {
+		return errors.New("hash or skylink is required")
+	}
+	return nil
+}
+
 // extractSkylinkHash extracts the skylink hash from the given skylink that
 // might have protocol, path, etc. within it.
 func extractSkylinkHash(skylink string) (string, error) {
@@ -381,3 +423,8 @@ func parseListParameters(query url.Values) (int, int, int, error) {
 
 	return sort, offset, limit, nil
 }
+
+// WriteError wraps WriteError from the skyd node api
+func WriteError(w http.ResponseWriter, err error, code int) {
+	skyapi.WriteError(w, skyapi.Error{Message: err.Error()}, code)
+}

api/handlers_test.go

@@ -122,10 +122,18 @@ func testHandleBlockRequest(t *testing.T, server *httptest.Server) {
 	// create a response writer
 	w := newMockResponseWriter()
 
-	// allow list a skylink
+	// create skylink
+	var sl skymodules.Skylink
+	err = sl.LoadString(v1SkylinkStr)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// allowlist a skylink
+	hash := database.NewHash(sl)
 	err = api.staticDB.CreateAllowListedSkylink(ctx, &database.AllowListedSkylink{
-		Skylink:        v1SkylinkStr,
-		Description:    "test skylink",
+		Hash:           hash,
+		Description:    "test hash",
 		TimestampAdded: time.Now().UTC(),
 	})
 	if err != nil {
@@ -153,15 +161,10 @@ func testHandleBlockRequest(t *testing.T, server *httptest.Server) {
 		t.Fatal("unexpected error", err)
 	}
 	if resp.Status != "reported" {
-		t.Fatal("unexpected response status", resp.Status)
+		t.Fatal("unexpected response status", resp.Status, resp)
 	}
 
 	// assert the blocked skylink did not make it into the database
-	var sl skymodules.Skylink
-	err = sl.LoadString(v1SkylinkStr)
-	if err != nil {
-		t.Fatal(err)
-	}
 	doc, err := api.staticDB.FindByHash(ctx, database.NewHash(sl))
 	if err != nil {
 		t.Fatal("unexpected error", err)
@@ -171,8 +174,8 @@ func testHandleBlockRequest(t *testing.T, server *httptest.Server) {
 	}
 
 	// up until now we have asserted that the skylink gets resolved and the
-	// allow list gets checked, note that this is only meaningful if the below
-	// assertions pass also (happy path)
+	// allowlist gets checked, note that this is only meaningful if the below
+	// assertions also pass (happy path)
 
 	// load a random skylink
 	err = sl.LoadString("_B19BtlWtjjR7AD0DDzxYanvIhZ7cxXrva5tNNxDht1kaA")
@@ -261,8 +264,7 @@ func testHandleBlocklistGET(t *testing.T, server *httptest.Server) {
 		skylink := fmt.Sprintf("skylink_%d", i)
 		offset := time.Duration(i) * time.Second
 		err = api.staticDB.CreateBlockedSkylink(ctx, &database.BlockedSkylink{
-			Skylink: skylink,
-			Hash:    database.HashBytes([]byte(skylink)),
+			Hash: database.HashBytes([]byte(skylink)),
 			Reporter: database.Reporter{
 				Name: "John Doe",
 			},

blocker/blocker_test.go

@@ -127,13 +127,13 @@ func testBlockHashes(t *testing.T, server *httptest.Server) {
 
 // newTestBlocker returns a new blocker instance
 func newTestBlocker(ctx context.Context, dbName string, skydClient *api.SkydClient) (*Blocker, error) {
+	// create database
+	db := database.NewTestDB(context.Background(), dbName)
+
 	// create a nil logger
 	logger := logrus.New()
 	logger.Out = ioutil.Discard
 
-	// create database
-	db := database.NewTestDB(context.Background(), dbName, logger)
-
 	// create the blocker
 	blocker, err := New(skydClient, db, logger)
 	if err != nil {