Feature/extra info debug (#28)

Summary:

 - tls not set for Basic server eventhough info passed
 - tls variables are no longer mandatory
 - clean all non signed headers when checking signature that covers specific headers
 - make sure to send error back when there is an upstream error
 - tests: allow self-signed certificates for the tests
 - misc change: more debug logging

More detailed

* tracing: extra info on header manipulations

* special handling content length

* test: add dummy test for local debugging.

* bugfix: Basic server was never enabling TLS

* bugfix: no conservatism when cleaning for validating signature

When we have to verify a signature and we are cleaning then we should clean everything that is not considered signed.

This change allows the cleaner to take in options to specify the type of cleaning and e make sure that when checking signature it always cleans.

* docker: example docker compose to run locally.

* logging: more detailed debug

Add information when doign debug logging. Including logging related to the aws signing.

* bugfix: allow self-signe certificates in tests also for presigned urls

* config: tls config parameters are no longer mandatory, they can be left empty for non-tls proxying

* bugfix: fail explicitly when upstream error
bugfix: fail explicitly when using unsupported chunked signing

Author: pvbouwel

aws/service/s3/handler-builder.go

@@ -14,6 +14,7 @@ import (
 	"github.com/VITObelgium/fakes3pp/constants"
 	"github.com/VITObelgium/fakes3pp/presign"
 	"github.com/VITObelgium/fakes3pp/requestctx"
+	"github.com/VITObelgium/fakes3pp/usererror"
 )
 
 // A handler builder builds http handlers
@@ -66,6 +67,21 @@ func justProxy(ctx context.Context, w http.ResponseWriter, r *http.Request, targ
 		writeS3ErrorResponse(ctx, w, ErrS3InternalError, nil)
 		return
 	}
+	payloadHash := r.Header.Get(constants.AmzContentSHAKey)
+	if payloadHash == "STREAMING-UNSIGNED-PAYLOAD-TRAILER" {
+		writeS3ErrorResponse(
+			ctx, 
+			w, 
+			ErrS3InternalError, 
+			usererror.New(
+				errors.New("unsupported encryption to be implemented so giving internal error to user"), 
+				`We do not support STREAMING-UNSIGNED-PAYLOAD-TRAILER yet.
+				For details or to upvote see https://github.com/VITObelgium/fakes3pp/issues/27
+				`),
+			)
+		return
+	}
+	slog.DebugContext(ctx, "Headers before signing", "headers", r.Header)
 	err = presign.SignWithCreds(ctx, r, creds, targetBackendId)
 	if err != nil {
 		slog.ErrorContext(ctx, "Could not sign request with permanent credentials", "error", err, "backendId", targetBackendId)
@@ -78,6 +94,7 @@ func justProxy(ctx context.Context, w http.ResponseWriter, r *http.Request, targ
 	resp, err := client.Do(r)
 	if err != nil {
 		slog.ErrorContext(ctx, "Error making request", "error", err)
+		writeS3ErrorResponse(ctx, w, ErrS3UpstreamError, nil)
 		return
 	}
 	defer resp.Body.Close()
@@ -93,7 +110,7 @@ func justProxy(ctx context.Context, w http.ResponseWriter, r *http.Request, targ
 	if err != nil {
 		slog.ErrorContext(ctx, "Context had error", "error", err, "context_error", ctx.Err())
 	} else {
-		slog.InfoContext(ctx, "End of proxying", "bytes", i, "error", err, "status", resp.Status, "headers", resp.Header)
+		slog.InfoContext(ctx, "End of proxying", "bytes", i, "error", err, "status", resp.Status, "headers", resp.Header, "trailer", r.Trailer)
 	}
 }
 

aws/service/s3/proxys3_test.go

@@ -328,7 +328,8 @@ func TestWithValidPresignedUrlOtherRegion(t *testing.T) {
 	if err != nil {
 		t.Errorf("client: error making http request: %s\n", err)
 	}
-	res, err := http.DefaultClient.Do(httpReq)
+	httpClient := testutils.BuildUnsafeHttpClientThatTrustsAnyCert(t)
+	res, err := httpClient.Do(httpReq)
 	if err != nil {
 		t.Errorf("client: error making http request: %s\n", err)
 	}
@@ -442,7 +443,7 @@ func performValidListObjectTestRequest(t testing.TB, s *S3Server, headerModifier
 	}
 	headerModifierPostSign(req.Header)
 
-	client := &http.Client{}
+	client := testutils.BuildUnsafeHttpClientThatTrustsAnyCert(t)
 	resp, err := client.Do(req)
 	if err != nil {
 		t.Errorf("Could not perform request: %s", err)

cmd/almost-e2e_test.go

@@ -7,6 +7,7 @@ import (
 	"io"
 	"log/slog"
 	"net/http"
+	"os"
 	"testing"
 	"time"
 
@@ -78,10 +79,12 @@ var testProviderFakeTesting string = fmt.Sprintf(`
 var testConfigFakeTesting string = fmt.Sprintf("providers:%s", testProviderFakeTesting)
 
 func TestMain(m *testing.M) {
-	envFiles = "../etc/.env"
-	loadEnvVarsFromDotEnv()
-	initConfig()
-	initializeTestLogging()
+	if os.Getenv("DEBUG_LOCAL_TEST") == "" {
+		envFiles = "../etc/.env"
+		loadEnvVarsFromDotEnv()
+		initConfig()
+		initializeTestLogging()
+	}
 	m.Run()
 }
 
@@ -394,7 +397,7 @@ func TestSigv4PresignedUrlsWork(t *testing.T) {
 		if err != nil {
 			t.Errorf("Did not expect error when signing url for %s. Got %s", backendRegion, err)
 		}
-		resp, err := http.Get(signedUri)
+		resp, err := testutils.BuildUnsafeHttpClientThatTrustsAnyCert(t).Get(signedUri)
 		if err != nil {
 			t.Errorf("Did not expect error when using signing url for %s. Got %s", backendRegion, err)
 		}
@@ -720,7 +723,7 @@ func TestSigv4PresignedUrlsFailWithOldSigningStrategy(t *testing.T) {
 			t.Errorf("Did not expect error when signing url for %s. Got %s", backendRegion, err)
 			t.FailNow()
 		}
-		resp, err := http.Get(signedUri)
+		resp, err := testutils.BuildUnsafeHttpClientThatTrustsAnyCert(t).Get(signedUri)
 		if err != nil {
 			t.Errorf("The get should have gone through but got an error: %s", err)
 		}
@@ -757,7 +760,7 @@ func TestSigv4PresignedUrlsSucceedWithOldSigningStrategyWhenBackwardsCompatibili
 		if err != nil {
 			t.Errorf("Did not expect error when signing url for %s. Got %s", backendRegion, err)
 		}
-		resp, err := http.Get(signedUri)
+		resp, err := testutils.BuildUnsafeHttpClientThatTrustsAnyCert(t).Get(signedUri)
 		if err != nil {
 			t.Errorf("Did not expect error when using signing url for %s. Got %s", backendRegion, err)
 		}

cmd/config.go

@@ -98,14 +98,14 @@ var envVarDefs = []envVarDef{
 	{
 		s3ProxyCertFile,
 		FAKES3PP_S3_PROXY_CERT_FILE,
-		true,
+		false,
 		"The certificate file used for tls server-side",
 		[]string{proxys3},
 	},
 	{
 		s3ProxyKeyFile,
 		FAKES3PP_S3_PROXY_KEY_FILE,
-		true,
+		false,
 		"The key file used for tls server-side",
 		[]string{proxys3},
 	},
@@ -140,14 +140,14 @@ var envVarDefs = []envVarDef{
 	{
 		stsProxyCertFile,
 		FAKES3PP_STS_PROXY_CERT_FILE,
-		true,
+		false,
 		"The certificate file used for tls server-side",
 		[]string{proxysts},
 	},
 	{
 		stsProxyKeyFile,
 		FAKES3PP_STS_PROXY_KEY_FILE,
-		true,
+		false,
 		"The key file used for tls server-side",
 		[]string{proxysts},
 	},

cmd/debug-local_test.go

@@ -0,0 +1,31 @@
+package cmd
+
+import (
+	"fmt"
+	"log/slog"
+	"os"
+	"testing"
+
+	"github.com/VITObelgium/fakes3pp/logging"
+	"github.com/VITObelgium/fakes3pp/server"
+)
+
+
+//This is a dummy test that can be used to spawn a server with a local config
+//Just set DEBUG_LOCAL_TEST to a directory that has a fakes3pp.env file and all the other
+//required config that you reference in that env file. Then if you debug this test you have a server
+//running with the debugger attached.
+func TestRunLocalDebugEndpoint(t *testing.T) {
+	if os.Getenv("DEBUG_LOCAL_TEST") == "" {
+		t.Skip("Skipping this test because DEBUG_LOCAL_TEST is empty.")
+	}
+	
+	testDir := os.Getenv("DEBUG_LOCAL_TEST")
+	envFiles = fmt.Sprintf("%s/fakes3pp.env", testDir)
+	loadEnvVarsFromDotEnv()
+	initConfig()
+	logging.InitializeLogging(slog.LevelDebug, nil, nil)
+
+	server.CreateAndStartSync(buildS3Server(), getServerOptsFromViper())
+	
+}

constants/aws-constants.go

@@ -40,6 +40,9 @@ const (
 	// ContentSHAKey is the SHA256 of request body
 	AmzContentSHAKey = "X-Amz-Content-Sha256"
 
+	//as per https://docs.aws.amazon.com/AmazonS3/latest/API/sigv4-streaming.html
+	AmzDecodedContentLength = "x-amz-decoded-content-length"
+
 	// TimeFormat is the time format to be used in the X-Amz-Date header or query parameter
 	TimeFormat = "20060102T150405Z"
 )

etc/docker-compose.yaml

@@ -0,0 +1,37 @@
+services:
+  s3proxy:
+    image: localhost/fakes3pp
+    networks:
+      - frontend
+    ports:
+      - "8443:8443"
+      - "8000:5555"
+    command: proxys3 --dot-env /etc/fakes3pp/.env.docker
+    volumes:
+      - ../etc.private:/etc/fakes3pp:Z
+
+    environment:
+      HOME: /root
+      LOG_LEVEL: DEBUG
+     
+
+  stsproxy:
+    image: localhost/fakes3pp
+
+    networks:
+      - frontend
+    ports:
+      - "8444:8444"
+      - "8001:5556"
+    command: proxysts --dot-env /etc/fakes3pp/.env.docker
+    volumes:
+      - ../etc.private:/etc/fakes3pp:Z
+    environment:
+      HOME: /root
+      FAKES3PP_METRICS_PORT: "8001"
+
+networks:
+  frontend:
+    # Specify driver options
+    driver: bridge
+

middleware/authentication.go

@@ -75,7 +75,7 @@ func handleAuthNPresigned(w http.ResponseWriter, r *http.Request, keyStorage uti
 
 	err = makeSureSessionTokenIsForAccessKey(creds.SessionToken, creds.AccessKeyID)
 	if err != nil {
-		err := fmt.Errorf("Error when making sure session token corresponds to used credential pair: %w", err)
+		err := fmt.Errorf("error when making sure session token corresponds to used credential pair: %w", err)
 		e.WriteErrorResponse(r.Context(), w, service.ErrAuthorizationHeaderMalformed, err)
 		return false
 	}
@@ -142,7 +142,7 @@ func handleAuthNNormal(w http.ResponseWriter, r *http.Request, keyStorage utils.
 
 	err = makeSureSessionTokenIsForAccessKey(sessionToken, accessKeyId)
 	if err != nil {
-		err := fmt.Errorf("Error when making sure session token corresponds to used credential pair: %w", err)
+		err := fmt.Errorf("error when making sure session token corresponds to used credential pair: %w", err)
 		e.WriteErrorResponse(r.Context(), w, service.ErrAuthorizationHeaderMalformed, err)
 		return false
 	}
@@ -174,6 +174,8 @@ func handleAuthNNormal(w http.ResponseWriter, r *http.Request, keyStorage utils.
 	passedSignature := r.Header.Get(constants.AuthorizationHeader)
 	//Cleaning could have removed content length
 	r.ContentLength = backupContentLength
+	slog.DebugContext(r.Context(), "ContentLength after manipualation", "ContentLength", r.ContentLength)
+
 
 	if calculatedSignature != passedSignature {
 		slog.DebugContext(
@@ -215,9 +217,11 @@ func getCredentialsFromRequest(r *http.Request) (accessKeyId, sessionToken strin
 //cleanHeadersThatAreNotSignedInAuthHeader removes headers which are potentially added along the way
 //
 func cleanHeadersThatAreNotSignedInAuthHeader(req *http.Request) {
+	slog.DebugContext(req.Context(), "Headers before manipualation", "Headers", req.Header)
+
 	signedHeaders := getSignedHeadersFromRequest(req)
 
-	presign.CleanHeadersTo(req.Context(), req, signedHeaders)
+	presign.CleanHeadersTo(req.Context(), req, signedHeaders, presign.CleanerOptions{})
 }
 
 const signedHeadersPrefix = "SignedHeaders="

middleware/observability.go

@@ -102,6 +102,11 @@ func LogMiddleware(requestLogLvl slog.Level, hc HealthChecker, promReg prometheu
 				"Request start",
 				getRequestCtxLogAttrs(rCtx)...
 			)
+			slog.DebugContext(
+				ctx, "Extra debug details", 
+				"contentLength", r.ContentLength,
+				"TransferEncoding", r.TransferEncoding,
+			)
 			defer logFinalRequestDetails(ctx, logLvl, startTime, rCtx)
 
 			if !wasHealthCheck{

presign/header-operations.go

@@ -19,6 +19,15 @@ var cleanableHeaders = map[string]bool{
 	"amz-sdk-invocation-id": true, //Added by AWS SDKs after signing
 	"amz-sdk-request": true, //Added by AWS SDKs after signing
 	"content-length": true,
+	"Sec-Fetch-User": true,
+	"Priority": true,
+	"Te": true,
+	"Accept": true,
+	"Upgrade-Insecure-Requests": true,
+	"Sec-Fetch-Dest": true,
+	"Sec-Fetch-Mode": true,
+	"Sec-Fetch-Site": true,
+	"Accept-Language": true,
 }
 
 //It is not always clear which headers are OK to skip cleaning. These headers
@@ -37,7 +46,14 @@ func isCleanable(headerName string) bool {
 	return false
 }
 
-func CleanHeadersTo(ctx context.Context, req *http.Request, toKeep map[string]string) {
+type CleanerOptions struct{
+	//If set we do not check against a list of headers that is safe to clean
+	//We just clean all that are not explicitly set to keep
+	AlwaysClean bool
+}
+
+//
+func CleanHeadersTo(ctx context.Context, req *http.Request, toKeep map[string]string, opts CleanerOptions) {
 	var cleaned = []string{}
 	var skipped = []string{}
 	var signed = []string{}
@@ -55,11 +71,17 @@ func CleanHeadersTo(ctx context.Context, req *http.Request, toKeep map[string]st
 			signed = append(signed, header)
 			continue
 		}
-		if isCleanable(header) {
+		if isCleanable(header) || opts.AlwaysClean {
 			//If content-length is to be cleaned it should
 			//also be <=0 otherwise it is taken in the signature
 			//-1 means unknown so let's fall back to that
 			if headerLC == "content-length" {
+				slog.DebugContext(
+					req.Context(),
+					"Cleaning Content-Length",
+					"header", req.Header.Get(header),
+					"reqValue", req.ContentLength,
+				)
 				req.ContentLength = -1
 			}
 			req.Header.Del(header)

presign/s3v4.go

@@ -3,6 +3,7 @@ package presign
 import (
 	"context"
 	"errors"
+	"fmt"
 	"log/slog"
 	"net/http"
 	"strconv"
@@ -12,6 +13,7 @@ import (
 	"github.com/VITObelgium/fakes3pp/requestutils"
 	"github.com/aws/aws-sdk-go-v2/aws"
 	v4 "github.com/aws/aws-sdk-go-v2/aws/signer/v4"
+	"github.com/aws/smithy-go/logging"
 )
 
 //This file just contains helpers to presign for S3 with sigv4
@@ -20,14 +22,31 @@ func PreSignRequestWithCreds(ctx context.Context, req *http.Request, expiryInSec
 	if expiryInSeconds <= 0 {
 		return "", nil, errors.New("expiryInSeconds must be bigger than 0 for presigned requests")
 	}
-	signer := v4.NewSigner()
+	signer := getSigner(ctx)
 
 	ctx, creds, req, payloadHash, service, region, signingTime := GetS3SignRequestParams(ctx, req, expiryInSeconds, signingTime, creds, defaultRegion)
 	return signer.PresignHTTP(ctx, creds, req, payloadHash, service, region, signingTime)
 }
 
+func getLogger(ctx context.Context, l *slog.Logger) logging.LoggerFunc {
+	var f logging.LoggerFunc = func(classification logging.Classification, format string, v ...interface{}) {
+		if len(classification) != 0 {
+			format = string(classification) + " " + format
+		}
+	
+		l.DebugContext(ctx, "SigninLogging", "msg", fmt.Sprintf(format, v...))
+	}
+	
+	return f
+}
+
+func getSigner(ctx context.Context) *v4.Signer {
+	return v4.NewSigner(func(signer *v4.SignerOptions){signer.LogSigning = true; signer.Logger = getLogger(ctx, slog.Default())})
+}
+
+
 func SignRequestWithCreds(ctx context.Context, req *http.Request, expiryInSeconds int, signingTime time.Time, creds aws.Credentials, defaultRegion string) (err error){
-	signer := v4.NewSigner()
+	signer := getSigner(ctx)
 
 	ctx, creds, req, payloadHash, service, region, signingTime := GetS3SignRequestParams(ctx, req, expiryInSeconds, signingTime, creds, defaultRegion)
 	return signer.SignHTTP(ctx, creds, req, payloadHash, service, region, signingTime)

presign/s3v4query.go

@@ -89,7 +89,7 @@ func (u presignedUrlS3V4Query) GetPresignedUrlDetails(ctx context.Context, deriv
 	if c.Header.Get("Host") == "" {
 		c.Header.Add("Host", c.Host)
 	}
-	CleanHeadersTo(ctx, c, u.getSignedHeaders())
+	CleanHeadersTo(ctx, c, u.getSignedHeaders(), CleanerOptions{AlwaysClean: true})
 	defaultRegion := ""  // A Sigv4 always has a region specified as part of the X-amz-credentials parameter so no fallback needed.
 	signedUri, _, err := PreSignRequestWithCreds(ctx, c, expirySeconds, signDate, creds, defaultRegion)
 	if err != nil {