Repository: avenga/couper
Pull request #147 (2021-03-16 09:27:56)

Add new strip-secure-cookies setting


Patch:
diff --git a/DOCKER.md b/DOCKER.md
index 240358a9..11ec967e 100644
--- a/DOCKER.md
+++ b/DOCKER.md
@@ -26,6 +26,7 @@ docker run --rm -p 8080:8080 -v `pwd`:/htdocs avenga/couper
 | COUPER_HEALTH_PATH    | `/healthz`   | Path for health-check requests for all servers and ports.   |
 | COUPER_NO_PROXY_FROM_ENV | `false` | Disables the connect hop to configured [proxy via environment](https://godoc.org/golang.org/x/net/http/httpproxy). |
 | COUPER_REQUEST_ID_FORMAT    | `common`   | If set to `uuid4` a rfc4122 uuid is used for `req.id` and related log fields.   |
+| COUPER_SECURE_COOKIES | `""` | If set to `"strip"`, the `Secure` flag is removed from all `Set-Cookie` HTTP header fields. |
 | COUPER_ACCESS_LOG_PARENT_FIELD | `""`  | An option for `json` log format to add all log fields as child properties. |
 | COUPER_ACCESS_LOG_TYPE_VALUE | `couper_access`  | Value for the log field `type`. |
 | COUPER_ACCESS_LOG_REQUEST_HEADERS | `User-Agent, Accept, Referer`  | A comma separated list of header names whose values should be logged. |
diff --git a/command/run.go b/command/run.go
index 09e8257e..5a0a0e75 100644
--- a/command/run.go
+++ b/command/run.go
@@ -3,6 +3,7 @@ package command
 import (
 	"context"
 	"flag"
+	"fmt"
 
 	"github.com/avenga/couper/config/env"
 
@@ -32,10 +33,16 @@ func (r Run) Execute(args Args, config *config.Couper, logEntry *logrus.Entry) e
 	set.BoolVar(&config.Settings.XForwardedHost, "xfh", config.Settings.XForwardedHost, "-xfh")
 	set.BoolVar(&config.Settings.NoProxyFromEnv, "no-proxy-from-env", config.Settings.NoProxyFromEnv, "-no-proxy-from-env")
 	set.StringVar(&config.Settings.RequestIDFormat, "request-id-format", config.Settings.RequestIDFormat, "-request-id-format uuid4")
+	set.StringVar(&config.Settings.SecureCookies, "secure-cookies", config.Settings.SecureCookies, "-secure-cookies strip")
 	if err := set.Parse(args.Filter(set)); err != nil {
 		return err
 	}
 
+	if config.Settings.SecureCookies != "" &&
+		config.Settings.SecureCookies != server.SecureCookiesStrip {
+		return fmt.Errorf("Invalid value for the -secure-cookies flag given. Only 'strip' is allowed.")
+	}
+
 	env.Decode(config.Settings)
 
 	timings := runtime.DefaultTimings
diff --git a/config/settings.go b/config/settings.go
index ab0ed1de..90cc258f 100644
--- a/config/settings.go
+++ b/config/settings.go
@@ -17,5 +17,6 @@ type Settings struct {
 	LogFormat       string `hcl:"log_format,optional"`
 	NoProxyFromEnv  bool   `hcl:"no_proxy_from_env,optional"`
 	RequestIDFormat string `hcl:"request_id_format,optional"`
+	SecureCookies   string `hcl:"secure_cookies,optional"`
 	XForwardedHost  bool   `hcl:"xfh,optional"`
 }
diff --git a/docs/README.md b/docs/README.md
index dc2489ce..fc3cafda 100644
--- a/docs/README.md
+++ b/docs/README.md
@@ -780,12 +780,13 @@ gateway instance.
 | *context*           | Root of the configuration file. | |
 | *label*             | Not impplemented. | |
 | **Attributes**      | **Description** | **Default** |
-| `health_path`       | health path which is available for all configured server and ports | `/healthz` |
+| `health_path`       | Health path which is available for all configured server and ports | `/healthz` |
 | `no_proxy_from_env` | Disables the connect hop to configured [proxy via environment](https://godoc.org/golang.org/x/net/http/httpproxy). | `false` |
-| `default_port`      | port which will be used if not explicitly specified per host within the [`hosts`](#server-block) list | `8080` |
-| `log_format`        | switch for tab/field based colored view or json log lines | `common` |
-| `xfh`               | option to use the `X-Forwarded-Host` header as the request host | `false` |
-| `request_id_format` | if set to `uuid4` a rfc4122 uuid is used for `req.id` and related log fields | `common` |
+| `default_port`      | Port which will be used if not explicitly specified per host within the [`hosts`](#server-block) list | `8080` |
+| `log_format`        | Switch for tab/field based colored view or json log lines | `common` |
+| `xfh`               | Option to use the `X-Forwarded-Host` header as the request host | `false` |
+| `request_id_format` | If set to `uuid4` a rfc4122 uuid is used for `req.id` and related log fields | `common` |
+| `secure_cookies`    | If set to `"strip"`, the `Secure` flag is removed from all `Set-Cookie` HTTP header fields. | `""` |
 
 ### Health-Check
 
diff --git a/server/cookies.go b/server/cookies.go
new file mode 100644
index 00000000..db36f091
--- /dev/null
+++ b/server/cookies.go
@@ -0,0 +1,52 @@
+package server
+
+import (
+	"net/http"
+	"regexp"
+	"strings"
+)
+
+const (
+	SecureCookiesStrip = "strip"
+	SecureCookieAV     = "Secure"
+	setCookieHeader    = "Set-Cookie"
+)
+
+var regexSplitSetCookie = regexp.MustCompile(`([^;]+);?`)
+
+func stripSecureCookies(header http.Header) {
+	list := header.Values(setCookieHeader)
+	header.Del(setCookieHeader)
+
+	for _, original := range list {
+		parts, isSecure := parseSetCookieHeader(original)
+
+		if !isSecure {
+			header.Add(setCookieHeader, original) // Unchanged
+		} else {
+			header.Add(setCookieHeader, strings.Join(parts, "; "))
+		}
+	}
+}
+
+// parseSetCookieHeader splits the given Set-Cookie HTTP header field value
+// and always removes the <Secure> flag. If the <Secure> flag was present, the
+// second return value is set to <true>, otherwise to <false>.
+func parseSetCookieHeader(setCookie string) ([]string, bool) {
+	var parts []string
+	var isSecure bool
+
+	for _, m := range regexSplitSetCookie.FindAllStringSubmatch(setCookie, -1) {
+		part := strings.TrimSpace(m[1])
+
+		if strings.ToLower(part) == strings.ToLower(SecureCookieAV) {
+			isSecure = true
+
+			continue
+		}
+
+		parts = append(parts, part)
+	}
+
+	return parts, isSecure
+}
diff --git a/server/http.go b/server/http.go
index cad01f7f..128b21af 100644
--- a/server/http.go
+++ b/server/http.go
@@ -171,6 +171,7 @@ func (s *HTTPServer) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
 		transport.ReClientSupportsGZ.MatchString(
 			req.Header.Get(transport.AcceptEncodingHeader),
 		),
+		s.settings.SecureCookies,
 	)
 	rw = w
 
diff --git a/server/rw_wrapper.go b/server/rw_wrapper.go
index bdda6e07..fecbd174 100644
--- a/server/rw_wrapper.go
+++ b/server/rw_wrapper.go
@@ -26,14 +26,16 @@ type RWWrapper struct {
 	headerBuffer  *bytes.Buffer
 	httpStatus    []byte
 	httpLineDelim []byte
+	secureCookies string
 	statusWritten bool
 }
 
 // NewRWWrapper creates a new RWWrapper object.
-func NewRWWrapper(rw http.ResponseWriter, useGZ bool) *RWWrapper {
+func NewRWWrapper(rw http.ResponseWriter, useGZ bool, secureCookies string) *RWWrapper {
 	w := &RWWrapper{
-		rw:           rw,
-		headerBuffer: &bytes.Buffer{},
+		rw:            rw,
+		headerBuffer:  &bytes.Buffer{},
+		secureCookies: secureCookies,
 	}
 
 	if useGZ {
@@ -126,6 +128,10 @@ func (w *RWWrapper) configureHeader() {
 		w.rw.Header().Del(transport.ContentLengthHeader)
 		w.rw.Header().Set(transport.ContentEncodingHeader, transport.GzipName)
 	}
+
+	if w.secureCookies == SecureCookiesStrip {
+		stripSecureCookies(w.rw.Header())
+	}
 }
 
 func (w *RWWrapper) parseStatusCode(p []byte) int {


Test patch:
diff --git a/handler/endpoint_test.go b/handler/endpoint_test.go
index 775a1ee3..375dd96f 100644
--- a/handler/endpoint_test.go
+++ b/handler/endpoint_test.go
@@ -99,7 +99,7 @@ func TestEndpoint_RoundTrip_Eval(t *testing.T) {
 			*req = *req.WithContext(evalCtx.WithClientRequest(req))
 
 			rec := httptest.NewRecorder()
-			rw := server.NewRWWrapper(rec, false) // crucial for working ep due to res.Write()
+			rw := server.NewRWWrapper(rec, false, "") // crucial for working ep due to res.Write()
 			ep.ServeHTTP(rw, req)
 			rec.Flush()
 			res := rec.Result()
@@ -205,7 +205,7 @@ func TestEndpoint_RoundTripContext_Variables_json_body(t *testing.T) {
 				*req = *req.WithContext(eval.NewContext(nil).WithClientRequest(req))
 
 				rec := httptest.NewRecorder()
-				rw := server.NewRWWrapper(rec, false) // crucial for working ep due to res.Write()
+				rw := server.NewRWWrapper(rec, false, "") // crucial for working ep due to res.Write()
 				ep.ServeHTTP(rw, req)
 				rec.Flush()
 				//res := rec.Result()
@@ -286,7 +286,7 @@ func TestEndpoint_RoundTripContext_Null_Eval(t *testing.T) {
 			*req = *req.WithContext(ctx)
 
 			rec := httptest.NewRecorder()
-			rw := server.NewRWWrapper(rec, false) // crucial for working ep due to res.Write()
+			rw := server.NewRWWrapper(rec, false, "") // crucial for working ep due to res.Write()
 			ep.ServeHTTP(rw, req)
 			rec.Flush()
 			res := rec.Result()
diff --git a/server/cookies_internal_test.go b/server/cookies_internal_test.go
new file mode 100644
index 00000000..74b54a08
--- /dev/null
+++ b/server/cookies_internal_test.go
@@ -0,0 +1,34 @@
+package server
+
+import (
+	"net/http"
+	"reflect"
+	"testing"
+)
+
+func TestCookies_StripSecureCookies(t *testing.T) {
+	header := http.Header{}
+
+	header.Add(setCookieHeader, "name=1; HttpOnly")
+	header.Add(setCookieHeader, "name=2; Path=path; Secure; HttpOnly")
+	header.Add(setCookieHeader, "name=2; Path=path; Secure; HttpOnly;")
+	header.Add(setCookieHeader, "name=3; Path=path; HttpOnly; Secure;")
+	header.Add(setCookieHeader, "name=4; Path=path; HttpOnly; secure")
+	header.Add(setCookieHeader, "name=secure; Path=path; HttpOnly")
+	header.Add(setCookieHeader, "name=Secure; Path=path; HttpOnly;")
+
+	stripSecureCookies(header)
+
+	exp := http.Header{}
+	exp.Add(setCookieHeader, "name=1; HttpOnly")
+	exp.Add(setCookieHeader, "name=2; Path=path; HttpOnly")
+	exp.Add(setCookieHeader, "name=2; Path=path; HttpOnly")
+	exp.Add(setCookieHeader, "name=3; Path=path; HttpOnly")
+	exp.Add(setCookieHeader, "name=4; Path=path; HttpOnly")
+	exp.Add(setCookieHeader, "name=secure; Path=path; HttpOnly")
+	exp.Add(setCookieHeader, "name=Secure; Path=path; HttpOnly;")
+
+	if !reflect.DeepEqual(header, exp) {
+		t.Errorf("Expected \n'%v', got: \n'%v'", exp, header)
+	}
+}
diff --git a/server/cookies_test.go b/server/cookies_test.go
new file mode 100644
index 00000000..98f43746
--- /dev/null
+++ b/server/cookies_test.go
@@ -0,0 +1,55 @@
+package server_test
+
+import (
+	"net"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+	"time"
+
+	"github.com/avenga/couper/internal/test"
+)
+
+func TestCookies_IntegrationStrip(t *testing.T) {
+	helper := test.New(t)
+	seenCh := make(chan struct{})
+
+	origin := httptest.NewUnstartedServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
+		rw.Header().Set("Set-Cookie", "n=v; Path=path; Secure")
+		rw.WriteHeader(http.StatusOK)
+
+		go func() {
+			seenCh <- struct{}{}
+		}()
+	}))
+	ln, err := net.Listen("tcp4", testProxyAddr[7:])
+	helper.Must(err)
+	origin.Listener = ln
+	origin.Start()
+	defer func() {
+		origin.Close()
+		ln.Close()
+		time.Sleep(time.Second)
+	}()
+
+	confPath := "testdata/settings/01_couper.hcl"
+	shutdown, _ := newCouper(confPath, test.New(t))
+	defer shutdown()
+
+	req, err := http.NewRequest(http.MethodGet, "http://anyserver:8080", nil)
+	helper.Must(err)
+
+	res, err := newClient().Do(req)
+	helper.Must(err)
+
+	if v := res.Header.Get("Set-Cookie"); v != "n=v; Path=path" {
+		t.Errorf("Unexpected Set-Cookie header given: %s", v)
+	}
+
+	timer := time.NewTimer(time.Second)
+	select {
+	case <-timer.C:
+		t.Error("Origin request failed")
+	case <-seenCh:
+	}
+}
diff --git a/server/testdata/settings/01_couper.hcl b/server/testdata/settings/01_couper.hcl
new file mode 100644
index 00000000..99a9692a
--- /dev/null
+++ b/server/testdata/settings/01_couper.hcl
@@ -0,0 +1,15 @@
+server "settings" {
+  api {
+    endpoint "/" {
+      proxy {
+        backend {
+          origin = "http://example.com"
+        }
+      }
+    }
+  }
+}
+
+settings {
+  secure_cookies = "strip"
+}