Validate upload server config at startup

upload-server/server/local.go

@@ -33,10 +33,13 @@ func configureLocalHandlers(mux *http.ServeMux, limiter *middleware.APIRateLimit
 	if !ok {
 		return fmt.Errorf("SERVER_URL environment variable is required")
 	}
-	_, err := url.Parse(envURL)
+	parsedURL, err := url.Parse(envURL)
 	if err != nil {
 		return fmt.Errorf("SERVER_URL environment variable is invalid: %w", err)
 	}
+	if parsedURL.Scheme != "https" {
+		return fmt.Errorf("SERVER_URL environment variable must use https, got %q", parsedURL.Scheme)
+	}
 
 	dir := defaultDir
 	envDir, ok := os.LookupEnv("STORE_DIR")

upload-server/server/local_test.go

@@ -15,7 +15,7 @@ import (
 	"github.com/netbirdio/netbird/upload-server/types"
 )
 
-const testSigningKey = "test-signing-key"
+const testSigningKey = "test-signing-key-with-enough-length"
 
 func signedQuery(t *testing.T, objectKey string) string {
 	t.Helper()
@@ -24,12 +24,12 @@ func signedQuery(t *testing.T, objectKey string) string {
 }
 
 func Test_LocalHandlerGetUploadURL(t *testing.T) {
-	mockURL := "http://localhost:8080"
+	mockURL := "https://localhost:8080"
 	t.Setenv("SERVER_URL", mockURL)
 	t.Setenv("STORE_DIR", t.TempDir())
 
 	mux := http.NewServeMux()
-	err := configureLocalHandlers(mux, newRateLimiter())
+	err := configureLocalHandlers(mux, newTestRateLimiter(t))
 	require.NoError(t, err)
 
 	req := httptest.NewRequest(http.MethodGet, types.GetURLPath+"?id=test-file", nil)
@@ -50,13 +50,13 @@ func Test_LocalHandlerGetUploadURL(t *testing.T) {
 
 func Test_LocalHandlePutRequest(t *testing.T) {
 	mockDir := t.TempDir()
-	mockURL := "http://localhost:8080"
+	mockURL := "https://localhost:8080"
 	t.Setenv("SERVER_URL", mockURL)
 	t.Setenv("STORE_DIR", mockDir)
 	t.Setenv(signingKeyVar, testSigningKey)
 
 	mux := http.NewServeMux()
-	err := configureLocalHandlers(mux, newRateLimiter())
+	err := configureLocalHandlers(mux, newTestRateLimiter(t))
 	require.NoError(t, err)
 
 	fileContent := []byte("test file content")
@@ -76,13 +76,13 @@ func Test_LocalHandlePutRequest(t *testing.T) {
 
 func Test_LocalHandlePutRequest_PathTraversal(t *testing.T) {
 	mockDir := t.TempDir()
-	mockURL := "http://localhost:8080"
+	mockURL := "https://localhost:8080"
 	t.Setenv("SERVER_URL", mockURL)
 	t.Setenv("STORE_DIR", mockDir)
 	t.Setenv(signingKeyVar, testSigningKey)
 
 	mux := http.NewServeMux()
-	err := configureLocalHandlers(mux, newRateLimiter())
+	err := configureLocalHandlers(mux, newTestRateLimiter(t))
 	require.NoError(t, err)
 
 	fileContent := []byte("malicious content")
@@ -101,11 +101,11 @@ func Test_LocalHandlePutRequest_PathTraversal(t *testing.T) {
 
 func Test_LocalHandlePutRequest_DirTraversal(t *testing.T) {
 	mockDir := t.TempDir()
-	t.Setenv("SERVER_URL", "http://localhost:8080")
+	t.Setenv("SERVER_URL", "https://localhost:8080")
 	t.Setenv("STORE_DIR", mockDir)
 	t.Setenv(signingKeyVar, testSigningKey)
 
-	l := &local{url: "http://localhost:8080", dir: mockDir, signer: &signer{key: []byte(testSigningKey)}}
+	l := &local{url: "https://localhost:8080", dir: mockDir, signer: &signer{key: []byte(testSigningKey)}}
 
 	body := bytes.NewReader([]byte("bad"))
 	req := httptest.NewRequest(http.MethodPut,
@@ -124,12 +124,12 @@ func Test_LocalHandlePutRequest_DirTraversal(t *testing.T) {
 
 func Test_LocalHandlePutRequest_DuplicateFile(t *testing.T) {
 	mockDir := t.TempDir()
-	t.Setenv("SERVER_URL", "http://localhost:8080")
+	t.Setenv("SERVER_URL", "https://localhost:8080")
 	t.Setenv("STORE_DIR", mockDir)
 	t.Setenv(signingKeyVar, testSigningKey)
 
 	mux := http.NewServeMux()
-	err := configureLocalHandlers(mux, newRateLimiter())
+	err := configureLocalHandlers(mux, newTestRateLimiter(t))
 	require.NoError(t, err)
 
 	req := httptest.NewRequest(http.MethodPut,
@@ -151,12 +151,12 @@ func Test_LocalHandlePutRequest_DuplicateFile(t *testing.T) {
 
 func Test_LocalHandlePutRequest_BodyTooLarge(t *testing.T) {
 	mockDir := t.TempDir()
-	t.Setenv("SERVER_URL", "http://localhost:8080")
+	t.Setenv("SERVER_URL", "https://localhost:8080")
 	t.Setenv("STORE_DIR", mockDir)
 	t.Setenv(signingKeyVar, testSigningKey)
 
 	mux := http.NewServeMux()
-	err := configureLocalHandlers(mux, newRateLimiter())
+	err := configureLocalHandlers(mux, newTestRateLimiter(t))
 	require.NoError(t, err)
 
 	largeBody := make([]byte, maxUploadSize+1)
@@ -170,3 +170,12 @@ func Test_LocalHandlePutRequest_BodyTooLarge(t *testing.T) {
 	_, err = os.Stat(filepath.Join(mockDir, "dir", "big.txt"))
 	require.True(t, os.IsNotExist(err))
 }
+
+func Test_ConfigureLocalHandlersRejectsPlaintextURL(t *testing.T) {
+	t.Setenv("SERVER_URL", "http://localhost:8080")
+	t.Setenv("STORE_DIR", t.TempDir())
+	t.Setenv(signingKeyVar, testSigningKey)
+
+	err := configureLocalHandlers(http.NewServeMux(), newTestRateLimiter(t))
+	require.Error(t, err)
+}

upload-server/server/ratelimit.go

@@ -11,16 +11,19 @@ import (
 const defaultUploadBurst = 100
 
 func newRateLimiter() *middleware.APIRateLimiter {
-	cfg, _ := middleware.RateLimiterConfigFromEnv()
+	cfg, enabled := middleware.RateLimiterConfigFromEnv()
 	if os.Getenv(middleware.RateLimitingBurstEnv) == "" {
 		cfg.Burst = defaultUploadBurst
 	}
 
-	limiter := middleware.NewAPIRateLimiter(cfg)
+	// Rate limiting is enabled by default unless explicitly disabled
-	if os.Getenv(middleware.RateLimitingEnabledEnv) == "false" {
+	if os.Getenv(middleware.RateLimitingEnabledEnv) == "" {
-		limiter.SetEnabled(false)
+		enabled = true
 	}
 
+	limiter := middleware.NewAPIRateLimiter(cfg)
+	limiter.SetEnabled(enabled)
+
 	log.Infof("Upload URL rate limiting: enabled=%t rate=%.0f/min burst=%d trusted_proxies=%q",
 		limiter.Enabled(), cfg.RequestsPerMinute, cfg.Burst, os.Getenv(middleware.RateLimitingTrustedProxiesEnv))
 

upload-server/server/ratelimit_test.go

@@ -11,6 +11,15 @@ import (
 	"github.com/netbirdio/netbird/upload-server/types"
 )
 
+func newTestRateLimiter(t *testing.T) *middleware.APIRateLimiter {
+	t.Helper()
+
+	limiter := newRateLimiter()
+	t.Cleanup(limiter.Stop)
+
+	return limiter
+}
+
 func getUploadURL(t *testing.T, mux *http.ServeMux) int {
 	t.Helper()
 
@@ -33,6 +42,7 @@ func Test_GetUploadURLIsRateLimited(t *testing.T) {
 }
 
 func Test_RateLimitingIsOnByDefault(t *testing.T) {
+	t.Setenv(middleware.RateLimitingEnabledEnv, "")
 	t.Setenv(middleware.RateLimitingBurstEnv, "1")
 	mux, _ := newLocalMux(t)
 
@@ -48,3 +58,14 @@ func Test_RateLimitingCanBeDisabled(t *testing.T) {
 	require.Equal(t, http.StatusOK, getUploadURL(t, mux))
 	require.Equal(t, http.StatusOK, getUploadURL(t, mux))
 }
+
+func Test_ServerStopIsIdempotent(t *testing.T) {
+	t.Setenv("SERVER_URL", "https://localhost:8080")
+	t.Setenv("STORE_DIR", t.TempDir())
+	t.Setenv(signingKeyVar, testSigningKey)
+
+	srv := NewServer()
+
+	require.NoError(t, srv.Stop())
+	require.NotPanics(t, func() { _ = srv.Stop() })
+}

upload-server/server/s3_test.go

@@ -90,7 +90,7 @@ func Test_S3HandlerGetUploadURL(t *testing.T) {
 	t.Setenv(bucketVar, bucketName)
 
 	mux := http.NewServeMux()
-	err = configureS3Handlers(mux, newRateLimiter())
+	err = configureS3Handlers(mux, newTestRateLimiter(t))
 	require.NoError(t, err)
 
 	req := httptest.NewRequest(http.MethodGet, types.GetURLPath+"?id=test-file", nil)

upload-server/server/signing.go

@@ -22,6 +22,8 @@ const (
 
 	expiryParam    = "exp"
 	signatureParam = "sig"
+
+	minSigningKeyLen = 32
 )
 
 type signer struct {
@@ -33,6 +35,9 @@ func newSigner() (*signer, error) {
 		if env == "" {
 			return nil, fmt.Errorf("%s is set but empty", signingKeyVar)
 		}
+		if len(env) < minSigningKeyLen {
+			return nil, fmt.Errorf("%s must be at least %d bytes", signingKeyVar, minSigningKeyLen)
+		}
 		return &signer{key: []byte(env)}, nil
 	}
 
@@ -71,7 +76,7 @@ func (s *signer) verify(objectKey string, query url.Values, now time.Time) error
 	if !hmac.Equal(got, s.signature(objectKey, exp)) {
 		return fmt.Errorf("signature mismatch")
 	}
-	if now.Unix() > exp {
+	if now.Unix() >= exp {
 		return fmt.Errorf("upload URL expired")
 	}
 

upload-server/server/signing_test.go

@@ -8,6 +8,7 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 
@@ -20,12 +21,12 @@ func newLocalMux(t *testing.T) (*http.ServeMux, string) {
 	t.Helper()
 
 	mockDir := t.TempDir()
-	t.Setenv("SERVER_URL", "http://localhost:8080")
+	t.Setenv("SERVER_URL", "https://localhost:8080")
 	t.Setenv("STORE_DIR", mockDir)
 	t.Setenv(signingKeyVar, testSigningKey)
 
 	mux := http.NewServeMux()
-	require.NoError(t, configureLocalHandlers(mux, newRateLimiter()))
+	require.NoError(t, configureLocalHandlers(mux, newTestRateLimiter(t)))
 
 	return mux, mockDir
 }
@@ -81,6 +82,10 @@ func Test_LocalHandlePutRequest_RejectsUnauthorized(t *testing.T) {
 			name:  "signature for a different object",
 			query: signedQuery(t, "dir/other.txt"),
 		},
+		{
+			name:  "signature expiring this second",
+			query: expired.sign("dir/file.txt", time.Now().Add(-signatureTTL)).Encode(),
+		},
 		{
 			name:  "expired signature",
 			query: expired.sign("dir/file.txt", time.Now().Add(-signatureTTL-time.Minute)).Encode(),
@@ -134,3 +139,10 @@ func Test_NewSignerRejectsEmptyKey(t *testing.T) {
 	_, err := newSigner()
 	require.Error(t, err)
 }
+
+func Test_NewSignerRejectsShortKey(t *testing.T) {
+	t.Setenv(signingKeyVar, strings.Repeat("a", minSigningKeyLen-1))
+
+	_, err := newSigner()
+	require.Error(t, err)
+}