Require a signed URL to write to the local

upload-server/server/local.go

@@ -8,6 +8,7 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+	"time"
 
 	log "github.com/sirupsen/logrus"
 
@@ -20,8 +21,9 @@ const (
 )
 
 type local struct {
-	url string
-	dir string
+	url    string
+	dir    string
+	signer *signer
 }
 
 func configureLocalHandlers(mux *http.ServeMux) error {
@@ -44,9 +46,15 @@ func configureLocalHandlers(mux *http.ServeMux) error {
 		dir = envDir
 	}
 
+	uploadSigner, err := newSigner()
+	if err != nil {
+		return err
+	}
+
 	l := &local{
-		url: envURL,
-		dir: dir,
+		url:    envURL,
+		dir:    dir,
+		signer: uploadSigner,
 	}
 	mux.HandleFunc(types.GetURLPath, l.handlerGetUploadURL)
 	mux.HandleFunc(putURLPath+putHandler, l.handlePutRequest)
@@ -80,6 +88,7 @@ func (l *local) getUploadURL(objectKey string) (string, error) {
 		return "", fmt.Errorf("failed to parse upload URL: %w", err)
 	}
 	newURL := parsedUploadURL.JoinPath(parsedUploadURL.Path, putURLPath, objectKey)
+	newURL.RawQuery = l.signer.sign(objectKey, time.Now()).Encode()
 	return newURL.String(), nil
 }
 
@@ -91,13 +100,6 @@ func (l *local) handlePutRequest(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	r.Body = http.MaxBytesReader(w, r.Body, maxUploadSize)
-	body, err := io.ReadAll(r.Body)
-	if err != nil {
-		http.Error(w, "request body too large or failed to read", http.StatusRequestEntityTooLarge)
-		return
-	}
-
 	uploadDir := r.PathValue("dir")
 	if uploadDir == "" {
 		http.Error(w, "missing dir path", http.StatusBadRequest)
@@ -109,6 +111,19 @@ func (l *local) handlePutRequest(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	if err := l.signer.verify(uploadDir+"/"+uploadFile, r.URL.Query(), time.Now()); err != nil {
+		http.Error(w, "unauthorized", http.StatusUnauthorized)
+		log.Warnf("Rejected upload of %s/%s: %v", uploadDir, uploadFile, err)
+		return
+	}
+
+	r.Body = http.MaxBytesReader(w, r.Body, maxUploadSize)
+	body, err := io.ReadAll(r.Body)
+	if err != nil {
+		http.Error(w, "request body too large or failed to read", http.StatusRequestEntityTooLarge)
+		return
+	}
+
 	cleanBase := filepath.Clean(l.dir) + string(filepath.Separator)
 
 	dirPath := filepath.Clean(filepath.Join(l.dir, uploadDir))
@@ -125,14 +140,14 @@ func (l *local) handlePutRequest(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if err = os.MkdirAll(dirPath, 0750); err != nil {
+	if err = os.MkdirAll(dirPath, 0o750); err != nil {
 		http.Error(w, "failed to create upload dir", http.StatusInternalServerError)
 		log.Errorf("Failed to create upload dir: %v", err)
 		return
 	}
 
 	flags := os.O_WRONLY | os.O_CREATE | os.O_EXCL
-	f, err := os.OpenFile(filePath, flags, 0600)
+	f, err := os.OpenFile(filePath, flags, 0o600)
 	if err != nil {
 		if os.IsExist(err) {
 			http.Error(w, "file already exists", http.StatusConflict)

upload-server/server/local_test.go

@@ -8,12 +8,21 @@ import (
 	"os"
 	"path/filepath"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/require"
 
 	"github.com/netbirdio/netbird/upload-server/types"
 )
 
+const testSigningKey = "test-signing-key"
+
+func signedQuery(t *testing.T, objectKey string) string {
+	t.Helper()
+	s := &signer{key: []byte(testSigningKey)}
+	return s.sign(objectKey, time.Now()).Encode()
+}
+
 func Test_LocalHandlerGetUploadURL(t *testing.T) {
 	mockURL := "http://localhost:8080"
 	t.Setenv("SERVER_URL", mockURL)
@@ -37,7 +46,6 @@ func Test_LocalHandlerGetUploadURL(t *testing.T) {
 	require.Contains(t, response.URL, "test-file/")
 	require.NotEmpty(t, response.Key)
 	require.Contains(t, response.Key, "test-file/")
-
 }
 
 func Test_LocalHandlePutRequest(t *testing.T) {
@@ -45,13 +53,15 @@ func Test_LocalHandlePutRequest(t *testing.T) {
 	mockURL := "http://localhost:8080"
 	t.Setenv("SERVER_URL", mockURL)
 	t.Setenv("STORE_DIR", mockDir)
+	t.Setenv(signingKeyVar, testSigningKey)
 
 	mux := http.NewServeMux()
 	err := configureLocalHandlers(mux)
 	require.NoError(t, err)
 
 	fileContent := []byte("test file content")
-	req := httptest.NewRequest(http.MethodPut, putURLPath+"/uploads/test.txt", bytes.NewReader(fileContent))
+	req := httptest.NewRequest(http.MethodPut,
+		putURLPath+"/uploads/test.txt?"+signedQuery(t, "uploads/test.txt"), bytes.NewReader(fileContent))
 
 	rec := httptest.NewRecorder()
 	mux.ServeHTTP(rec, req)
@@ -69,13 +79,16 @@ func Test_LocalHandlePutRequest_PathTraversal(t *testing.T) {
 	mockURL := "http://localhost:8080"
 	t.Setenv("SERVER_URL", mockURL)
 	t.Setenv("STORE_DIR", mockDir)
+	t.Setenv(signingKeyVar, testSigningKey)
 
 	mux := http.NewServeMux()
 	err := configureLocalHandlers(mux)
 	require.NoError(t, err)
 
 	fileContent := []byte("malicious content")
-	req := httptest.NewRequest(http.MethodPut, putURLPath+"/uploads/%2e%2e%2f%2e%2e%2fetc%2fpasswd", bytes.NewReader(fileContent))
+	req := httptest.NewRequest(http.MethodPut,
+		putURLPath+"/uploads/%2e%2e%2f%2e%2e%2fetc%2fpasswd?"+signedQuery(t, "uploads/../../etc/passwd"),
+		bytes.NewReader(fileContent))
 
 	rec := httptest.NewRecorder()
 	mux.ServeHTTP(rec, req)
@@ -90,11 +103,13 @@ func Test_LocalHandlePutRequest_DirTraversal(t *testing.T) {
 	mockDir := t.TempDir()
 	t.Setenv("SERVER_URL", "http://localhost:8080")
 	t.Setenv("STORE_DIR", mockDir)
+	t.Setenv(signingKeyVar, testSigningKey)
 
-	l := &local{url: "http://localhost:8080", dir: mockDir}
+	l := &local{url: "http://localhost:8080", dir: mockDir, signer: &signer{key: []byte(testSigningKey)}}
 
 	body := bytes.NewReader([]byte("bad"))
-	req := httptest.NewRequest(http.MethodPut, putURLPath+"/x/evil.txt", body)
+	req := httptest.NewRequest(http.MethodPut,
+		putURLPath+"/x/evil.txt?"+signedQuery(t, "../../../tmp/evil.txt"), body)
 	req.SetPathValue("dir", "../../../tmp")
 	req.SetPathValue("file", "evil.txt")
 
@@ -111,17 +126,20 @@ func Test_LocalHandlePutRequest_DuplicateFile(t *testing.T) {
 	mockDir := t.TempDir()
 	t.Setenv("SERVER_URL", "http://localhost:8080")
 	t.Setenv("STORE_DIR", mockDir)
+	t.Setenv(signingKeyVar, testSigningKey)
 
 	mux := http.NewServeMux()
 	err := configureLocalHandlers(mux)
 	require.NoError(t, err)
 
-	req := httptest.NewRequest(http.MethodPut, putURLPath+"/dir/dup.txt", bytes.NewReader([]byte("first")))
+	req := httptest.NewRequest(http.MethodPut,
+		putURLPath+"/dir/dup.txt?"+signedQuery(t, "dir/dup.txt"), bytes.NewReader([]byte("first")))
 	rec := httptest.NewRecorder()
 	mux.ServeHTTP(rec, req)
 	require.Equal(t, http.StatusOK, rec.Code)
 
-	req = httptest.NewRequest(http.MethodPut, putURLPath+"/dir/dup.txt", bytes.NewReader([]byte("second")))
+	req = httptest.NewRequest(http.MethodPut,
+		putURLPath+"/dir/dup.txt?"+signedQuery(t, "dir/dup.txt"), bytes.NewReader([]byte("second")))
 	rec = httptest.NewRecorder()
 	mux.ServeHTTP(rec, req)
 	require.Equal(t, http.StatusConflict, rec.Code)
@@ -135,13 +153,15 @@ func Test_LocalHandlePutRequest_BodyTooLarge(t *testing.T) {
 	mockDir := t.TempDir()
 	t.Setenv("SERVER_URL", "http://localhost:8080")
 	t.Setenv("STORE_DIR", mockDir)
+	t.Setenv(signingKeyVar, testSigningKey)
 
 	mux := http.NewServeMux()
 	err := configureLocalHandlers(mux)
 	require.NoError(t, err)
 
 	largeBody := make([]byte, maxUploadSize+1)
-	req := httptest.NewRequest(http.MethodPut, putURLPath+"/dir/big.txt", bytes.NewReader(largeBody))
+	req := httptest.NewRequest(http.MethodPut,
+		putURLPath+"/dir/big.txt?"+signedQuery(t, "dir/big.txt"), bytes.NewReader(largeBody))
 	rec := httptest.NewRecorder()
 	mux.ServeHTTP(rec, req)
 

upload-server/server/signing.go

@@ -0,0 +1,85 @@
+package server
+
+import (
+	"crypto/hmac"
+	"crypto/rand"
+	"crypto/sha256"
+	"encoding/hex"
+	"fmt"
+	"net/url"
+	"os"
+	"strconv"
+	"time"
+
+	log "github.com/sirupsen/logrus"
+)
+
+const (
+	signingKeyVar = "NB_UPLOAD_SIGNING_KEY"
+
+	// signatureTTL matches the expiry the S3 backend puts on its presigned URLs.
+	signatureTTL = 15 * time.Minute
+
+	expiryParam    = "exp"
+	signatureParam = "sig"
+)
+
+type signer struct {
+	key []byte
+}
+
+func newSigner() (*signer, error) {
+	if env, ok := os.LookupEnv(signingKeyVar); ok {
+		if env == "" {
+			return nil, fmt.Errorf("%s is set but empty", signingKeyVar)
+		}
+		return &signer{key: []byte(env)}, nil
+	}
+
+	key := make([]byte, 32)
+	if _, err := rand.Read(key); err != nil {
+		return nil, fmt.Errorf("generate signing key: %w", err)
+	}
+	log.Infof("%s not set, generated an ephemeral upload signing key", signingKeyVar)
+
+	return &signer{key: key}, nil
+}
+
+// sign returns the query parameters that authorize an upload of objectKey.
+func (s *signer) sign(objectKey string, now time.Time) url.Values {
+	exp := now.Add(signatureTTL).Unix()
+
+	v := url.Values{}
+	v.Set(expiryParam, strconv.FormatInt(exp, 10))
+	v.Set(signatureParam, hex.EncodeToString(s.signature(objectKey, exp)))
+
+	return v
+}
+
+// verify reports whether query carries a still-valid signature over objectKey.
+func (s *signer) verify(objectKey string, query url.Values, now time.Time) error {
+	exp, err := strconv.ParseInt(query.Get(expiryParam), 10, 64)
+	if err != nil {
+		return fmt.Errorf("malformed %s parameter", expiryParam)
+	}
+
+	got, err := hex.DecodeString(query.Get(signatureParam))
+	if err != nil {
+		return fmt.Errorf("malformed %s parameter", signatureParam)
+	}
+
+	if !hmac.Equal(got, s.signature(objectKey, exp)) {
+		return fmt.Errorf("signature mismatch")
+	}
+	if now.Unix() > exp {
+		return fmt.Errorf("upload URL expired")
+	}
+
+	return nil
+}
+
+func (s *signer) signature(objectKey string, exp int64) []byte {
+	mac := hmac.New(sha256.New, s.key)
+	fmt.Fprintf(mac, "%s\n%d", objectKey, exp)
+	return mac.Sum(nil)
+}

upload-server/server/signing_test.go

@@ -0,0 +1,136 @@
+package server
+
+import (
+	"bytes"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/netbirdio/netbird/upload-server/types"
+)
+
+func newLocalMux(t *testing.T) (*http.ServeMux, string) {
+	t.Helper()
+
+	mockDir := t.TempDir()
+	t.Setenv("SERVER_URL", "http://localhost:8080")
+	t.Setenv("STORE_DIR", mockDir)
+	t.Setenv(signingKeyVar, testSigningKey)
+
+	mux := http.NewServeMux()
+	require.NoError(t, configureLocalHandlers(mux))
+
+	return mux, mockDir
+}
+
+func Test_LocalUploadURLRoundTrip(t *testing.T) {
+	mux, mockDir := newLocalMux(t)
+
+	getReq := httptest.NewRequest(http.MethodGet, types.GetURLPath+"?id=test-file", nil)
+	getReq.Header.Set(types.ClientHeader, types.ClientHeaderValue)
+	getRec := httptest.NewRecorder()
+	mux.ServeHTTP(getRec, getReq)
+	require.Equal(t, http.StatusOK, getRec.Code)
+
+	var response types.GetURLResponse
+	require.NoError(t, json.Unmarshal(getRec.Body.Bytes(), &response))
+
+	minted, err := url.Parse(response.URL)
+	require.NoError(t, err)
+	require.NotEmpty(t, minted.Query().Get(signatureParam))
+
+	content := []byte("bundle")
+	putRec := httptest.NewRecorder()
+	mux.ServeHTTP(putRec, httptest.NewRequest(http.MethodPut, minted.RequestURI(), bytes.NewReader(content)))
+	require.Equal(t, http.StatusOK, putRec.Code)
+
+	written, err := os.ReadFile(filepath.Join(mockDir, response.Key))
+	require.NoError(t, err)
+	require.Equal(t, content, written)
+}
+
+func Test_LocalHandlePutRequest_RejectsUnauthorized(t *testing.T) {
+	expired := &signer{key: []byte(testSigningKey)}
+
+	tests := []struct {
+		name  string
+		query string
+	}{
+		{
+			name:  "no signature",
+			query: "",
+		},
+		{
+			name:  "tampered signature",
+			query: "exp=99999999999&sig=deadbeef",
+		},
+		{
+			name:  "malformed signature",
+			query: "exp=99999999999&sig=not-hex",
+		},
+		{
+			// A signature is only good for the key it was minted for, so a URL
+			// handed out for one bundle cannot be replayed against another.
+			name:  "signature for a different object",
+			query: signedQuery(t, "dir/other.txt"),
+		},
+		{
+			name:  "expired signature",
+			query: expired.sign("dir/file.txt", time.Now().Add(-signatureTTL-time.Minute)).Encode(),
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			mux, mockDir := newLocalMux(t)
+
+			target := putURLPath + "/dir/file.txt"
+			if tc.query != "" {
+				target += "?" + tc.query
+			}
+
+			rec := httptest.NewRecorder()
+			mux.ServeHTTP(rec, httptest.NewRequest(http.MethodPut, target, bytes.NewReader([]byte("payload"))))
+
+			require.Equal(t, http.StatusUnauthorized, rec.Code)
+
+			_, err := os.Stat(filepath.Join(mockDir, "dir", "file.txt"))
+			require.True(t, os.IsNotExist(err), "unauthorized upload should not be written")
+		})
+	}
+}
+
+func Test_SignerRejectsForeignKey(t *testing.T) {
+	minted := (&signer{key: []byte("one key")}).sign("dir/file.txt", time.Now())
+
+	err := (&signer{key: []byte("another key")}).verify("dir/file.txt", minted, time.Now())
+	require.Error(t, err)
+}
+
+func Test_NewSignerGeneratesEphemeralKey(t *testing.T) {
+	// Registers the restore hook, then clears the value for this test only.
+	t.Setenv(signingKeyVar, "")
+	os.Unsetenv(signingKeyVar)
+
+	first, err := newSigner()
+	require.NoError(t, err)
+	second, err := newSigner()
+	require.NoError(t, err)
+
+	require.NotEqual(t, first.key, second.key)
+	require.Len(t, first.key, 32)
+}
+
+func Test_NewSignerRejectsEmptyKey(t *testing.T) {
+	t.Setenv(signingKeyVar, "")
+
+	_, err := newSigner()
+	require.Error(t, err)
+}