feat: now make hmac secret subscription mandatory

Only receive request with X-Hub-Signature header and resub with hmac secret BREAKING CHANGE: mandatory config added, see README.md for new required configs
worksinmagic · Sep 25, 2020 · 1690321 · 1690321
1 parent f0eadb9
commit 1690321
Show file tree

Hide file tree

Showing 4 changed files with 138 additions and 26 deletions.
diff --git a/plugin/autosubscribefeed/autosubscribefeed.go b/plugin/autosubscribefeed/autosubscribefeed.go
@@ -2,8 +2,10 @@ package autosubscribefeed
 
 import (
 	"context"
+	"fmt"
 	"net/http"
 	"net/url"
+	"strings"
 	"time"
 
 	"github.com/pkg/errors"
@@ -20,6 +22,9 @@ const (
 	HubMode              = "hub.mode"
 	HubVerify            = "hub.verify"
 	HubVerificationToken = "hub.verify_token"
+	HubSecret            = "hub.secret"
+
+	ErrResubscribeFormat = "failed to resubscribe for topic %s with error '%v'"
 )
 
 var (
@@ -29,21 +34,23 @@ var (
 type Subscriber struct {
 	resubInterval     time.Duration
 	targetAddr        string
-	topic             string
+	topics            []string
 	callbackAddr      string
 	verificationToken string
+	hmacSecret        string
 
 	logger ytfeed.Logger
 	client *http.Client
 }
 
-func New(logger ytfeed.Logger, verificationToken, targetAddr, topic, callbackAddr string, resubInterval time.Duration) (s *Subscriber) {
+func New(logger ytfeed.Logger, verificationToken, hmacSecret, targetAddr, callbackAddr string, topics []string, resubInterval time.Duration) (s *Subscriber) {
 	s = &Subscriber{}
 	s.resubInterval = resubInterval
 	s.targetAddr = targetAddr
 	s.callbackAddr = callbackAddr
 	s.verificationToken = verificationToken
-	s.topic = topic
+	s.hmacSecret = hmacSecret
+	s.topics = topics
 	s.client = &http.Client{}
 	s.client.Timeout = DefaultTimeout
 	s.logger = logger
@@ -69,25 +76,52 @@ func (s *Subscriber) Subscribe(ctx context.Context) (err error) {
 	}
 }
 
+type ErrorSub struct {
+	Topic string
+	Err   error
+}
+
 func (s *Subscriber) subscribe() (err error) {
-	data := url.Values{}
-	data.Set(HubTopic, s.topic)
-	data.Set(HubCallback, s.callbackAddr)
-	data.Set(HubMode, DefaultHubMode)
-	data.Set(HubVerify, DefaultHubVerify)
-	data.Set(HubVerificationToken, s.verificationToken)
-
-	var resp *http.Response
-	resp, err = s.client.PostForm(s.targetAddr, data)
-	if err != nil {
-		return
-	}
-	if resp.StatusCode >= http.StatusBadRequest {
-		err = errors.Wrapf(ErrFailedToSubscribeFeed, "HTTP status %d", resp.StatusCode)
-		return
+	failedReqs := make([]ErrorSub, 0, 8)
+
+	for _, topic := range s.topics {
+		data := url.Values{}
+		data.Set(HubTopic, topic)
+		data.Set(HubCallback, s.callbackAddr)
+		data.Set(HubMode, DefaultHubMode)
+		data.Set(HubVerify, DefaultHubVerify)
+		data.Set(HubVerificationToken, s.verificationToken)
+		data.Set(HubSecret, s.hmacSecret)
+
+		var resp *http.Response
+		resp, err = s.client.PostForm(s.targetAddr, data)
+		if err != nil {
+			failedReqs = append(failedReqs, ErrorSub{
+				Topic: topic,
+				Err:   err,
+			})
+			continue
+		}
+		if resp.StatusCode >= http.StatusBadRequest {
+			err = errors.Wrapf(ErrFailedToSubscribeFeed, "HTTP status %d", resp.StatusCode)
+			failedReqs = append(failedReqs, ErrorSub{
+				Topic: topic,
+				Err:   err,
+			})
+			continue
+		}
+
+		s.logger.Infof("Resubscribed to topic %s with callback address %s", topic, s.callbackAddr)
 	}
 
-	s.logger.Infof("Resubscribed to topic %s with callback address %s", s.topic, s.callbackAddr)
+	if len(failedReqs) > 0 {
+		errMessages := make([]string, 0, len(failedReqs))
+		for _, f := range failedReqs {
+			errMessages = append(errMessages, fmt.Sprintf(ErrResubscribeFormat, f.Topic, f.Err))
+		}
+
+		err = errors.New(strings.Join(errMessages, ","))
+	}
 
 	return
 }
diff --git a/plugin/autosubscribefeed/autosubscribefeed_test.go b/plugin/autosubscribefeed/autosubscribefeed_test.go
@@ -34,13 +34,14 @@ func TestSubscriber(t *testing.T) {
 
 	logger := mock.NewMockLogger(ctrl)
 	verificationToken := "mytoken"
+	secret := "mysecret"
 	targetAddr := ts.URL
-	topic := "mytopic"
+	topics := []string{"mytopic", "yourtopic"}
 	callbackAddr := "http://localhost:9876"
 	resubInterval := 100 * time.Millisecond
 
 	t.Run("success", func(t *testing.T) {
-		s := New(logger, verificationToken, targetAddr, topic, callbackAddr, resubInterval)
+		s := New(logger, verificationToken, secret, targetAddr, callbackAddr, topics, resubInterval)
 		require.NotNil(t, s)
 
 		customHTTPClient := &http.Client{}
@@ -49,6 +50,12 @@ func TestSubscriber(t *testing.T) {
 		ctx, cancel := context.WithTimeout(context.TODO(), 200*time.Millisecond)
 		defer cancel()
 
+		// do this twice because we have two topics
+		logger.EXPECT().Infof(
+			gomock.AssignableToTypeOf("resubscribed"),
+			gomock.AssignableToTypeOf("topic"),
+			gomock.AssignableToTypeOf("callback address"),
+		)
 		logger.EXPECT().Infof(
 			gomock.AssignableToTypeOf("resubscribed"),
 			gomock.AssignableToTypeOf("topic"),
@@ -60,13 +67,14 @@ func TestSubscriber(t *testing.T) {
 	})
 
 	t.Run("failed", func(t *testing.T) {
-		s := New(logger, verificationToken, targetAddr, wrongTopic, callbackAddr, resubInterval)
+		topics := []string{wrongTopic}
+		s := New(logger, verificationToken, secret, targetAddr, callbackAddr, topics, resubInterval)
 		require.NotNil(t, s)
 
 		customHTTPClient := &http.Client{}
 		s.SetHTTPClient(customHTTPClient)
 
-		ctx, cancel := context.WithTimeout(context.TODO(), 200*time.Millisecond)
+		ctx, cancel := context.WithTimeout(context.TODO(), 150*time.Millisecond)
 		defer cancel()
 
 		logger.EXPECT().Errorf(

diff --git a/rss/rss.go b/rss/rss.go
@@ -3,8 +3,12 @@ package rss
 import (
 	"bytes"
 	"context"
+	"crypto/hmac"
+	"crypto/sha1"
+	"encoding/hex"
 	"encoding/json"
 	"fmt"
+	"hash"
 	"io"
 	"io/ioutil"
 	"net/http"
@@ -18,7 +22,7 @@ const (
 	YoutubeSubscriptionTopicPrefix = "https://www.youtube.com/xml/feeds/videos.xml?channel_id="
 )
 
-func Handler(ctx context.Context, logger ytfeed.Logger, verificationToken string, dataHandlers ...ytfeed.DataHandlerFunc) func(w http.ResponseWriter, req *http.Request) {
+func Handler(ctx context.Context, logger ytfeed.Logger, verificationToken, hmacSecret string, dataHandlers ...ytfeed.DataHandlerFunc) func(w http.ResponseWriter, req *http.Request) {
 	return func(w http.ResponseWriter, req *http.Request) {
 		defer req.Body.Close()
 
@@ -79,9 +83,22 @@ func Handler(ctx context.Context, logger ytfeed.Logger, verificationToken string
 			}
 			originalMessage := string(tmpRaw)
 
+			hmacHasher := hmac.New(sha1.New, []byte(hmacSecret))
+			var verified bool
+			verified, err = VerifyDataFeed(hmacHasher, req.Header.Get("X-Hub-Signature"), string(tmpRaw))
+			if err != nil {
+				w.WriteHeader(http.StatusForbidden)
+				fmt.Fprintf(w, "INVALID AUTHENTICATED CONTENT DISTRIBUTION SIGNATURE: %v", err)
+				return
+			}
+			if !verified {
+				w.WriteHeader(http.StatusForbidden)
+				fmt.Fprintf(w, "INVALID AUTHENTICATED CONTENT DISTRIBUTION SIGNATURE: not verified")
+				return
+			}
+
 			jbuf, err := xj.Convert(bytes.NewReader(tmpRaw))
 			if err != nil {
-				logger.Warnf("Failed to convert XML input to JSON: %v", err)
 				w.WriteHeader(http.StatusBadRequest)
 				fmt.Fprintf(w, "INVALID XML INPUT: %v", err)
 				return
@@ -124,3 +141,19 @@ func Handler(ctx context.Context, logger ytfeed.Logger, verificationToken string
 func IsYoutubeSubscriptionTopic(topic string) bool {
 	return strings.HasPrefix(topic, YoutubeSubscriptionTopicPrefix)
 }
+
+func VerifyDataFeed(hmacHasher hash.Hash, hmacHeader, xmlData string) (verified bool, err error) {
+	var hmacData []byte
+	hmacData, err = hex.DecodeString(strings.ReplaceAll(hmacHeader, "sha1=", ""))
+	if err != nil {
+		return
+	}
+	_, err = hmacHasher.Write([]byte(xmlData))
+	if err != nil {
+		return
+	}
+
+	verified = hmac.Equal(hmacData, hmacHasher.Sum(nil))
+
+	return
+}
diff --git a/rss/rss_test.go b/rss/rss_test.go
@@ -28,6 +28,10 @@ const (
 	<published>2012-08-26T01:51:49+00:00</published>
 	<updated>2020-03-22T11:53:58.790881024+00:00</updated>
    </entry>`
+
+	sampleDataHmacHex = "sha1=b6996541be31719101d047bdcef1f1762d83f743"
+
+	hmacSecret = "mysecret"
 )
 
 func TestRss(t *testing.T) {
@@ -40,7 +44,7 @@ func TestRss(t *testing.T) {
 		require.NotNil(t, data)
 	}
 
-	handler := Handler(context.TODO(), logger, verificationToken, dataHandler)
+	handler := Handler(context.TODO(), logger, verificationToken, hmacSecret, dataHandler)
 	require.NotNil(t, handler)
 
 	t.Run("method GET", func(t *testing.T) {
@@ -190,6 +194,7 @@ func TestRss(t *testing.T) {
 			if err != nil {
 				panic(err)
 			}
+			req.Header.Set("X-Hub-Signature", sampleDataHmacHex)
 			rec := httptest.NewRecorder()
 
 			logger.EXPECT().Infof(
@@ -202,13 +207,45 @@ func TestRss(t *testing.T) {
 			require.Equal(t, http.StatusCreated, rec.Result().StatusCode)
 		})
 
+		t.Run("feed failed invalid hmac not hex", func(t *testing.T) {
+			body := bytes.NewBufferString(`{"data":"data"}`)
+			addr := "http://localhost:8080/"
+			req, err := http.NewRequest(http.MethodPost, addr, body)
+			if err != nil {
+				panic(err)
+			}
+			req.Header.Set("X-Hub-Signature", "random")
+			rec := httptest.NewRecorder()
+
+			handler(rec, req)
+
+			require.Equal(t, http.StatusForbidden, rec.Result().StatusCode)
+		})
+
+		t.Run("feed failed invalid hmac not verified", func(t *testing.T) {
+			body := bytes.NewBufferString(`{"data":"data"}`)
+			addr := "http://localhost:8080/"
+			req, err := http.NewRequest(http.MethodPost, addr, body)
+			if err != nil {
+				panic(err)
+			}
+			req.Header.Set("X-Hub-Signature", sampleDataHmacHex)
+			rec := httptest.NewRecorder()
+
+			handler(rec, req)
+
+			require.Equal(t, http.StatusForbidden, rec.Result().StatusCode)
+		})
+
 		t.Run("feed failed invalid XML", func(t *testing.T) {
 			body := bytes.NewBufferString(`{"data":"data"}`)
+			wrongDataHex := "sha1=87c70090fbae726b9ddc0383c2819040622d30e2"
 			addr := "http://localhost:8080/"
 			req, err := http.NewRequest(http.MethodPost, addr, body)
 			if err != nil {
 				panic(err)
 			}
+			req.Header.Set("X-Hub-Signature", wrongDataHex)
 			rec := httptest.NewRecorder()
 
 			logger.EXPECT().Errorf(